Comment by OskarS | Hacker News Reader

Comment by OskarS | Hacker News Reader

Parent Full thread

So the problem here is that in this line:

      if( value < (double) LONG_MIN || value > (double) LONG_MAX)
          return( 0);

The cast of LONG_MAX to double rounds upwards (which is allowed by the standard, which says that rounding direction is "implementation defined"), so that "value > (double) LONG_MAX" is false, right? Even though the mathematical value of "value" is larger than LONG_MAX?

Which then leads to this line:

   l_val = (long) value;

Where value is cast to a long despite being outside of the range of longs, thus causing undefined behavior. So to be clear, BOTH of the -O0 and -O3 versions are correct, since both invoke undefined behaviour.

When -O0 and -O3 give different results, either at least one of them is incorrect and you've stumbled on a compiler bug, or both of them are correct and you're invoking UB (the far more common situation).

EDIT: no, I think I misunderstood it: it's not that value is larger than (double) LONG_MAX, it IS (double)LONG_MAX, so of course "value > (double)LONG_MAX" is false.

The problem is that the "(long)((double)LONG_MAX))" is undefined behaviour on implementations that round (double)LONG_MAX upwards instead of downwards. Which is allowed by the standard. Ok, cool :)

This ^^^^ is the core issue. It's insidious that LONG_MAX is too big to be exactly representable as a double.

The answer by ambrop7 below solves a different problem but appears to be correct. The reason escaped me at first and is super subtle. The trick is that LONG_MAX is 2^63 - 1, not 2^63. And the subtlety is that 2^63 is guaranteed to be exactly representable in IEEE double because it is an even power of 2, which 2^63-1 is not.

I don't care much for runtime ldexp() anyhow. So I'd be tempted to just pre-compute the exact limits -2^63 and nextafter(+2^63, 0) and encode them as doubles manually (omitting some #if method of portably determining the width of LONG):

  #define FLONG_MIN 9223372036854775808.0 // exact -2^63
  #define FLONG_MAX 9223372036854774784.0 // exact nextbefore(2^63)

  if (value < FLONG_MIN || value > FLONG_MAX)
     return(0);

Then the UB is avoided. I think the rest of the module works as is. Now, I question whether the author really wants integers between 2^53 and 2^63 to sparsely return true. So it might be better to just change the whole design to use +-2^53 as a hard limit, for trivially guaranteed round trip of the entire range and dispense with these nasty edge cases.

Huh. So, how does one check that a cast from double to long would succeed in C? Just go and do lround()+errno check?

I think the most correct way would be to explicitly set the rounding mode to down or towards-zero, then calculate largest_double_that_fits = (double) LONG_MAX, then check value <= largest_double_that_fits. Since largest_double_that_fits is guarenteed to be <= LONG_MAX, and value <= largest_double_that_fits, it follows that value <= LONG_MAX.

This is not right. The current rounding mode is not guaranteed to apply to integer to floating point conversions. Quoting C99, section 6.3.1.4, paragraph 2:

"When a value of integer type is converted to a real floating type, if the value being converted can be represented exactly in the new type, it is unchanged. If the value being converted is in the range of values that can be represented but cannot be represented exactly, the result is either the nearest higher or nearest lower representable value, chosen in an implementation-defined manner. If the value being converted is outside the range of values that can be represented, the behavior is undefined."

See it says implementation-defined manner, not according to the current rounding mode.

Test case:

    #pragma STDC FENV_ACCESS ON
    #include <stdio.h>
    #include <stdint.h>
    #include <fenv.h>

    int main()
    {
        fesetround(FE_DOWNWARD);
        printf("%f\n", (double)UINT64_MAX);
        fesetround(FE_UPWARD);
        printf("%f\n", (double)UINT64_MAX);
        return 0;
    }

    $ gcc -std=c99 -frounding-math x.c -lm
    $ ./a.out 
    18446744073709551616.000000
    18446744073709551616.000000

In both cases it rounded up.

I stand corrected, thanks! What a bizzare choice when floating point standards already have a well-defined system for determining how to round things.

Here's a reliable/portable solution:

    bool double_to_uint64 (double x, uint64_t *out)
    {
        double y = trunc(x);
        if (y >= 0.0 && y < ldexp(1.0, 64)) {
            *out = y;
            return true;
        } else {
            return false;
        }
    }

If you need different rounding behavior, just change trunc() to round(), floor() or ceil(). Note that it is important that the result is produced by converting the rounded double (y) to an integer type, not the original value (x).

Explanation:

- we first round the value to an integer (but still a floating point value),

- we then check that this integer is in the valid range of the target integer type by comparing it with exact integer values (0 and 2^N),

- if the check passes, then converting this integer to the target integer type is safe, and if the check fails, then conversion is not possible.

Of course if you literally need to convert to "long" you have a problem because the width of "long" is not known, but that is a rather different concern. I argue types with unknown width like "long" should almost never be used anyway.

(based on my answer here: https://stackoverflow.com/questions/8905246/how-to-check-if-...)

Reply on news.ycombinator.com