Undefined Behavior in lib/msun/src/e_pow.c (was Re: New math library from ARM)

[Steve Kargl]

On Mon, Dec 31, 2018 at 10:32:06PM -0500, Pedro Giffuni wrote:
> Hmm ...
> 
> Looking at the changes in musl's libc I found this issue which seems 
> real (although somewhat theoretical):
> 
> https://git.musl-libc.org/cgit/musl/commit/src/math?id=688d3da0f1730daddbc954bbc2d27cc96ceee04c
> 
> Is the attached patch acceptable?
> 
> Also, their code is bit different here:
> 
> https://git.musl-libc.org/cgit/musl/commit/src/math?id=282b1cd26649d69de038111f5876853df6ddc345
> 
> but we may also have to check fmaf(-0x1.26524ep-54, -0x1.cb7868p+11, 
> 0x1.d10f5ep-29).
> 
> Cheers,
> 
> Pedro.
> 
> Index: lib/msun/src/e_pow.c
> ===================================================================
> --- lib/msun/src/e_pow.c	(revision 342665)
> +++ lib/msun/src/e_pow.c	(working copy)
> @@ -130,6 +130,7 @@
>  	if(hx<0) {
>  	    if(iy>=0x43400000) yisint = 2; /* even integer y */
>  	    else if(iy>=0x3ff00000) {
> +		uint32_t j;	/* Avoid UB in bit operations below. */
>  		k = (iy>>20)-0x3ff;	   /* exponent */
>  		if(k>20) {
>  		    j = ly>>(52-k);


I'll defer to Bruce on this.  My only comments are
1) declarations belong at the top of the file where all declarations occur
2) j is already declared as int32_t
3) uint32_t should be written as u_int32_t.

Are you sure that UB occurs?  Or, is this an attempt to
placate a static analysis tool that only see shifting of
a signed type?  Do you need to make a similar change to
e_powf.c?

-- 
Steve