Date: Mon, 31 Dec 2018 20:54:26 -0800 From: Steve Kargl <sgk@troutmask.apl.washington.edu> To: Pedro Giffuni <pfg@freebsd.org> Cc: freebsd-numerics@freebsd.org Subject: Re: Undefined Behavior in lib/msun/src/e_pow.c (was Re: New math library from ARM) Message-ID: <20190101045425.GA5767@troutmask.apl.washington.edu> In-Reply-To: <06c8b6a2-ed26-f255-3947-c79b593a9dea@FreeBSD.org> References: <797a7755-db93-1b9c-f3b9-8850d948e098@FreeBSD.org> <20181231151904.GB823@troutmask.apl.washington.edu> <20181231152230.GC823@troutmask.apl.washington.edu> <06c8b6a2-ed26-f255-3947-c79b593a9dea@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190101045425.GA5767>