Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 16 Nov 2019 23:03:19 +0800
From:      Li-Wen Hsu <lwhsu@freebsd.org>
To:        sgk@troutmask.apl.washington.edu
Cc:        Jeff Walden <jwalden@mit.edu>, freebsd-numerics@freebsd.org
Subject:   Re: UB in various hypot() implementations (left-shifting a negative number)
Message-ID:  <CAKBkRUyAspA6eagg5wNuo9R1U=5QZfEfuF2BBp3=AvyrnFA0ZA@mail.gmail.com>
In-Reply-To: <20191113232233.GA64847@troutmask.apl.washington.edu>
References:  <ecf6c3ed-dea7-ae4f-77fa-a6114b688fec@mit.edu> <20191113232233.GA64847@troutmask.apl.washington.edu>

next in thread | previous in thread | raw e-mail | index | archive | help
I did a quick test, it seems one regression test fails:

lwhsu@x1c:/usr/tests/lib/msun > kyua debug csqrt_test:main
1..18
ok 1 - csqrt
ok 2 - csqrt
ok 3 - csqrt
ok 4 - csqrt
ok 5 - csqrt
ok 6 - csqrt
ok 7 - csqrt
ok 8 - csqrt
ok 9 - csqrt
ok 10 - csqrt
ok 11 - csqrt
ok 12 - csqrt
ok 13 - csqrt
ok 14 - csqrt
ok 15 - csqrt
ok 16 - csqrt
Assertion failed: (creall(result) =3D=3D ldexpl(14 * 0x1p-4, exp / 2)),
function test_overflow, file
/usr/home/lwhsu/freebsd-src/lib/msun/tests/csqrt_test.c, line 236.
Process with PID 18700 exited with signal 6 and dumped core;
attempting to gather stack trace
[New LWP 101179]
Core was generated by `/usr/tests/lib/msun/csqrt_test'.
Program terminated with signal SIGABRT, Aborted.
#0  thr_kill () at thr_kill.S:3
3       RSYSCALL(thr_kill)
#0  thr_kill () at thr_kill.S:3
#1  0x00000008004543a4 in __raise (s=3D6) at /usr/src/lib/libc/gen/raise.c:=
52
#2  0x00000008003c3029 in abort () at /usr/src/lib/libc/stdlib/abort.c:67
#3  0x0000000800440e71 in __assert (func=3D<optimized out>,
file=3D<optimized out>, line=3D<optimized out>, failedexpr=3D<optimized
out>) at /usr/src/lib/libc/gen/assert.c:51
#4  0x00000000002040bd in test_overflow (maxexp=3D16384) at
/usr/home/lwhsu/freebsd-src/lib/msun/tests/csqrt_test.c:236
#5  0x0000000000202a91 in main () at
/usr/home/lwhsu/freebsd-src/lib/msun/tests/csqrt_test.c:363
GDB exited successfully
Files left in work directory after failure: csqrt_test.core
csqrt_test:main  ->  broken: Received signal 6

I haven't checked the details, but I definitely need experts in this
field to help.

Thanks,
Li-Wen


On Thu, Nov 14, 2019 at 7:22 AM Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
>
> Looks good to me.  Just need to convince someone to commit it.
>
> --
> steve
>
>
> On Wed, Nov 13, 2019 at 02:43:04PM -0800, Jeff Walden wrote:
> >
> > Just wanted to note here that I filed https://reviews.freebsd.org/D2235=
4 to fix a bit of undefined behavior in the hypot() function implementation=
s.
> >
> > `hypot(x, y)` computes `sqrt(x*x + y*y)`, with IEEE-754-aware precision=
.  For very large or small `x` or `y`, the naive implementation would lose =
precision -- so in such cases the calculation is performed after multiplyin=
g the numbers by a very large (or very small) power of two, then the multip=
lication is undone at end.
> >
> > Undoing the multiplication involves multiplying a quantity `w` by `2**k=
`, where `k` (which may be positive or negative) was computed depending on =
the particular `x` and `y` values provided.  Current algorithms generally t=
ake `t1=3D0.0`, extract the high word, add `k<<20` or `k<<23` to it to appr=
opriately bump the exponent, then overwrite `t1`'s high word.  But it seems=
 equally effective to take `t1=3D0.0`, then write `(1023+k)<<20` or `(127+k=
)<<23` to it for identical effect -- and `k` is never so negative to comput=
e a negative value.  My changes do this.  (I wish there were named constant=
s I could use for all these numbers, but as there don't seem to be any, mag=
ic numbers seems like the best I can do.)
> >
> > Errors in these changes would most likely produce a power of two off by=
 a factor of two, so *probably* testing any inputs that would happen to inv=
oke these code paths should be adequate testing.  I'm fixing this in order =
to upstream a likely fix in the SpiderMonkey JavaScript engine for this, so=
 the (double, double) algorithm/change is the only one I have (purely manua=
lly) tested.  Eyeballs on the other functions' changes especially appreciat=
ed!
> >
> > Jeff
> > _______________________________________________
> > freebsd-numerics@freebsd.org mailing list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-numerics
> > To unsubscribe, send any mail to "freebsd-numerics-unsubscribe@freebsd.=
org"
>
> --
> Steve
> 20170425 https://www.youtube.com/watch?v=3DVWUpyCsUKR4
> 20161221 https://www.youtube.com/watch?v=3DIbCHE-hONow
> _______________________________________________
> freebsd-numerics@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-numerics
> To unsubscribe, send any mail to "freebsd-numerics-unsubscribe@freebsd.or=
g"



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAKBkRUyAspA6eagg5wNuo9R1U=5QZfEfuF2BBp3=AvyrnFA0ZA>