mpz_get_si: possible undefined behaviour? [Was: Micro-GMP]

[Vincent Lefevre]

On 2018-12-03 23:47:00 +0100, Marco Bodrato wrote:
> If LONG_MIN == -LONG_MAX-1, this step is correct.
> But, what about the LONG_MIN == -LONG_MAX case?
> The result is undefined, isn't it?

Yes.

> Which value we should actually return when the given mpz does not
> fit in the target type is not so well defined nor important,

One issue is that the function is poorly specified, if not buggy:

 -- Function: signed long int mpz_get_si (const mpz_t OP)
     If OP fits into a `signed long int' return the value of OP.
     Otherwise return the least significant part of OP, with the same
     sign as OP.

But on LONG_MAX + 1, mpz_get_si will give 0:

  if (size > 0)
    return zl & LONG_MAX;

while mathematically (and for mpz_sgn), 0 does not have the same sign
as a positive integer.

> but triggering an undefined behaviour seems a bad idea anyway.
> 
> 
> I'd suggest to substitute "-1" with "(LONG_MIN + LONG_MAX)", I mean
> something like:
> 
> return (LONG_MIN + LONG_MAX) -
>        (long) ((zp[0] - (-LONG_MAX - LONG_MIN)) & LONG_MAX);
> 
> 
> Comments about this analysis or better solutions?

I would write for clarity:

  long c = LONG_MIN + LONG_MAX;  /* -1 or 0 */
  /* ... */
  return c - (long) ((zp[0] - (-c)) & LONG_MAX);

and update the "Otherwise" case of the description, to say that the
returned value can be 0.

-- 
Vincent Lefèvre <vincent at vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)