Date: Tue, 1 Jan 2019 10:32: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: <20190101183226.GA11443@troutmask.apl.washington.edu> In-Reply-To: <9cdce84b-2cff-d752-67a9-4c9ef391cc2c@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> <20190101045425.GA5767@troutmask.apl.washington.edu> <d01c8770-5431-0634-4ecc-37069ff76458@FreeBSD.org> <20190101055225.GA5982@troutmask.apl.washington.edu> <e6bd09b3-35c6-922b-1b4d-de27e424a627@FreeBSD.org> <9cdce84b-2cff-d752-67a9-4c9ef391cc2c@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jan 01, 2019 at 10:01:45AM -0500, Pedro Giffuni wrote:
>
> >> This would be easy
> >> to see if all declarations are grouped together. In additional,
> >> don't you need to include stdint.h to get uint32_t?
> >
> > Well, my patch did compile, but I will match the existing style if
> > the change is accepted/desired.
Yeah, I found that out myself. I cannot find where stdint.h is
being pulled into the build. It seems that one of the included
headers may have some namespace pollution.
> >> % grep uint32_t /usr/src/lib/msun/src/e_pow.c
> >> % grep u_int32_t /usr/src/lib/msun/src/e_pow.c
> >> u_int32_t lx,ly;
> >> n = ((u_int32_t)hx>>31)-1;
> >>
> >> Code churn to placate lint for an event that cannot occur
> >> seems dubious to me.
> >>
> > UBsan should be able to detect it. I think Undefined Behavior is
> > considered a portability bug.
> >
> > There is no urgency but it is still a bug.
> >
> In the new patch: the first j comparison has to be made against an
> unsigned value anyways and the second comparison will never be against
> the magical 31 so there is no need to cast that one.
>
I'll defer to Bruce on which patch he prefers. If bde doesn't
weigh in, then choose which ever you want. Me, I would do
Index: src/e_pow.c
===================================================================
--- src/e_pow.c (revision 2135)
+++ src/e_pow.c (working copy)
@@ -104,7 +104,7 @@ __ieee754_pow(double x, double y)
double y1,t1,t2,r,s,t,u,v,w;
int32_t i,j,k,yisint,n;
int32_t hx,hy,ix,iy;
- u_int32_t lx,ly;
+ u_int32_t jj,lx,ly;
EXTRACT_WORDS(hx,lx,x);
EXTRACT_WORDS(hy,ly,y);
@@ -132,8 +132,8 @@ __ieee754_pow(double x, double y)
else if(iy>=0x3ff00000) {
k = (iy>>20)-0x3ff; /* exponent */
if(k>20) {
- j = ly>>(52-k);
- if((j<<(52-k))==ly) yisint = 2-(j&1);
+ jj = ly>>(52-k);
+ if((jj<<(52-k))==ly) yisint = 2-(j&1);
} else if(ly==0) {
j = iy>>(20-k);
if((j<<(20-k))==iy) yisint = 2-(j&1);
--
Steve
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190101183226.GA11443>