Menu
▾
▴
1 Attachments
#2883 "2**100" returns 0 due to compiler optimizations
Version: 6.0.5
Compiler: Clang 21+
print 2**100
Expected output: 1.26765060022823e+30
Actual output: 0
After some testing and debugging, I found Clang has assumed that signed integer overflow (undefined behavior) can never happen (if no -fwrapv flag set), and further optimized away the integer_power_overflow branch (at least -O1 level) since LLVM 21.
f_power() in internal.c:
...
intgr_t tprev, t;
intgr_t tmag = llabs(a.v.int_val);
tprev = t = 1;
for (i = 0; i < b.v.int_val; i++) {
tprev = t;
t *= tmag; // signed integer overflow (UB)
if (t < tprev) // -> if (false), optimized away
goto integer_power_overflow; // never
}
...
I made a patch (based on 6.1 b9821fb) to fix it, but not sure whether there is a better solution.
FWIW: my Windows binary compiled with clang 22.1.7 does not exhibit this problem.
I can reproduce this on linux with clang 22.1.8
Very annoying that an explicit test for overflow is optimized away because "overflow cannot happen".
Did you test or at least consider whether the patched code will be accepted by older compilers? How old? As I recall, "inline" was not accepted in c89. But we bumped the stated requirement for building gnuplot 6.0 to c99, so I guess the question is specifically whether this is accepted by c99.
Following up on myself after testing with gcc 4.3.2 (not-yet-official c99 support) on an old machine.
Inclusion of the new function from C files works fine. Inclusion from C++ files fails due to some problem with type definitions in the standard headers.
If I patch around that, the code in the main program seems to work as intended.
This was obviously fixed in gcc or maybe in the C++ standard at some later date since compilation shows no problem with recent versions of gcc. I'm not sure how much trouble we want to go to in order to support old gcc versions, but if there's a simple fix that would be good.
Here's a matching addition to fix f_prod(). This one is needed for current gcc, even aside from the issue of clang compiler options.
I did not find any other affected routines, but I suppose there could be some. The core code does not make much use of 64-bit integer arithmetic.
Two edge cases mistakenly treated as overflow:
The same bug in
f_sum():Ugh. I am extremely embarrassed that I missed this pitfall when I first added support for 64 bit integers. Everywhere that this code pattern appears is potentially susceptible to errors due to loss of precision:
(double)(<FOO>.v.int_val)This pitfall is intrinsic to the IEEE floating point representation, so it is relevant not just for recent clang or gcc compiler options. In some places the loss of precision affects possible overflow; in others it does not.
So I am now thinking we need the full set of your gp_* built-ins. I suggest that they go in a new header file, maybe
overflow.h, rather than insyscfg.h. It seems that as of now only internal.c needs to include this header. That bypasses the issue of old C++ compilers failing to accept it (shown to be a problem with gcc 4.3.2).Last edit: Ethan Merritt 4 days ago
I moved them into
overflow.has suggested, and licensed it under BSD 2-Clause.Looks good.
The only other thing I see is that when you add a file to the package, it needs to be listed in .../src/Makefile.am gnuplot_SOURCES so that the automake system tracks it and includes it in the source package from "make dist".