Latest commit introduces undefined behavior in hgcd2.c

Niels Möller nisse at lysator.liu.se
Wed Sep 18 19:37:45 UTC 2019


tg at gmplib.org (Torbjörn Granlund) writes:

> I think it is a false positive.  The result of the shifted value is
> masked when the shift count is not in range.

The line in question is

  dh = (dh << dcnt) + (-(dcnt > 0) & (dl >> (GMP_LIMB_BITS - dcnt)));

Should be fine if shift by 64 is "implementation defined", but a
problem, at least in theory, if it is "undefined behavior". I'm afraid I
don't know these fine details of the C spec by heart.

Is it reasonable to change it to

  #define LIMB_SHIFT_MASK (GMP_LIMB_BITS - 1)
  dh = (dh << dcnt) + (-(dcnt > 0) & (dl >> (LIMB_SHIFT_MASK & - dcnt)));

I'd expect compilers for common archs will generate the same code. It
depends on GMP_LIMB_BITS being a power of 2, though.

Vaguely related: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57157. But
unlike rotate, an extra masking step seems necessary.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.


More information about the gmp-bugs mailing list