Date: Sat, 16 Nov 2019 13:14:19 -0800 From: Steve Kargl <sgk@troutmask.apl.washington.edu> To: Dimitry Andric <dim@freebsd.org> Cc: freebsd-numerics@freebsd.org Subject: Re: UB in various hypot() implementations (left-shifting a negative, number) Message-ID: <20191116211419.GA40056@troutmask.apl.washington.edu> In-Reply-To: <a3fe6fb4-8f33-6d10-64d4-e722bfebcc85@FreeBSD.org> References: <a3fe6fb4-8f33-6d10-64d4-e722bfebcc85@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Nov 16, 2019 at 09:40:40PM +0100, Dimitry Andric wrote: > On 2019-11-16 18:02:29 UTC, Steve Kargl wrote: > > The patch for hypotl() is likely unneeded as there is no shifting > > of a negative integer in the code being changed. Does csqrt_test.c > > pass without Jeff's patch? > > Yes, it passes. > > > This is the original code > > > > u_int32_t high; > > t1 = 1.0; > > GET_HIGH_WORD(high,t1); > > SET_HIGH_WORD(t1,high+DESW(k)); > > > > high + DESW(k) = high + k > > = 16383 + k > > > > and this is the code after the patch > > > > t1 = 0.0; > > SET_HIGH_WORD(t1,ESW(k)); > > > > ESW(k) = MAX_EXP - 1 + k > > = LDBL_MAX_EXP - 1 + k > > = 16384 - 1 + k > > = 16383 + k > > > > So, in principle there is no functional change. > > What about t1 changing from 1.0 to 0? If I revert just that line, all > tests do pass, including the csqrt test. > Well, clearly, the patch to e_hypotl.c is wrong. It clear the significand when t1 = 0 whereas t1 = 1 leaves one bit set in the significand. Simply looking at the value of t1 under a poor man's debugger shows the difference. Adding "printf("%Le %La\n", t1, t1);" after the SET_HIGH_WORD gives 2.962347e-2493 0x1p-8280 <-- t1 = 1 0.000000e+00 0x1p-8280 <-- t1 = 0 for hypotl(ldexpl(1.1,-16000), ldexpl(2.1, -16000)). -- Steve 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20191116211419.GA40056>