Date: Mon, 12 Feb 2018 22:24:56 -0800 From: Eitan Adler <lists@eitanadler.com> To: freebsd-numerics@freebsd.org Subject: signed overflow in atan2 Message-ID: <CAF6rxgkDVfdN6hKEfrMj6WcNKJHZggq33e9zbM6kaaiXgkoChA@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
source: https://github.com/freebsd/freebsd/pull/130 `atan2(y, x)`'s special case for `x == 1.0` (in which case the result must be `atan(y)`) is unnecessarily complicated #130 As a component of atan2(y, x), the case of x == 1.0 is farmed out to atan(y). The current implementation of this comparison is vulnerable to signed integer underflow (that is, undefined behavior), and it's performed in a somewhat more complicated way than it need be. Change it to not be quite so cute, rather directly comparing the high/low bits of x to the specific IEEE-754 bit pattern that encodes 1.0. Note that while there are three different e_atan* files in the relevant directory, only this one needs fixing. e_atan2f.c already compares against the full bit pattern encoding 1.0f, while e_atan2l.cuses bitwise-ands/ors/nots and so doesn't require a change. (I ran into this in Mozilla's SpiderMonkey embedding of fdlibm, where a test of the behavior of Math.atan2(1, -0) triggered clang's signed-overflow-sanitizer runtime error.) ----- patch: if(((ix|((lx|-lx)>>31))>0x7ff00000)|| ((iy|((ly|-ly)>>31))>0x7ff00000)) /* x or y is NaN */ return x+y; - if((hx-0x3ff00000|lx)==0) return atan(y); /* x=1.0 */ + if(hx==0x3ff00000&&lx==0) return atan(y); /* x=1.0 */ m = ((hy>>31)&1)|((hx>>30)&2); /* 2*sign(x)+sign(y) */ /* when y = 0 */ ---- Is this correct? Any objections to be committing ? -- Eitan Adler
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF6rxgkDVfdN6hKEfrMj6WcNKJHZggq33e9zbM6kaaiXgkoChA>