This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix scalbn, scalbln integer overflow


On 03/02/2012 02:40 AM, Joseph S. Myers wrote:
Bug 10135 reports an integer overflow problem in scalblnl (which
version is unspecified).

The code in ldbl-96 is an example that illustrates the problem in all
the variants of scalbn and scalbln:

         k = k+n;
         if (__builtin_expect(n>  50000 || k>  0x7ffe, 0))
           return huge*__copysignl(huge,x); /* overflow  */
         if (__builtin_expect(n<  -50000, 0))
           return tiny*__copysignl(tiny,x);

Here k is an exponent, type int32_t or int64_t depending on the
implementation, which is fairly close to 0 (may be negative for
subnormals); n is an argument to the function, of type int for scalbn
and long for scalbln.  The expression k+n may overflow (either
positive or negative) except in the case where k is int64_t and n is
32-bit; the conversion storing it back in k may truncate the value if
n is 64-bit long and k is 32-bit.  Apart from undefined behavior on
integer overflow, the "k>  0x7ffe" comparison may thereby produce
incorrect results (so in particular causing the function to return
infinity instead of zero as reported in the bug).

The patch in the bug proposes reordering the tests so that the n<
-50000 test comes first.  This doesn't actually strictly avoid the
undefined behavior.  This patch makes the -50000 test come first, then
the 50000 test, then only after that test does it compute k+n (now
known not to overflow) and use that in a comparison.  It leaves the
compiler to eliminate redundancy from the two computations of k+n in
the source code, as I think this is the cleanest way to write the
code.  Testcases are added that showed the problem before the patch.

I suggest to add a comment before k=k+n stating why it's not done earlier in the code so that the next person might not accidentally "optimize" it.


The patch is fine,

Thanks,
Andreas
--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]