Created attachment 40722 [details] gcc -m32 -Woverflow incorrectly complains about this Compile the attached program with: gcc -m32 -S too-large.c and it responds: too-large.c: In function ‘too_large’: too-large.c:5:18: warning: integer overflow in expression [-Woverflow] return 32768 * 65536L < x; Since the expression 32768 * 65536L cannot be executed on this platform (and this is by design, the code tests whether the multiplication is advisable before trying it), the warning is a false alarm. Bruno Haible originally reported this problem in bug-gnulib, here: http://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00041.html As this sort of thing is common in portable code that is checking for overflow correctly, I suggest disabling -Woverflow tests inside unreachable expressions.
Confirmed. Note we don't warn for "0 ? 32768 * 65536L < x : 1", because c_parser_conditional_expression sets c_inhibit_evaluation_warnings if the cond is truthvalue_false_node. So I think we should do the same in c_parser_if_statement.
Generalized extended testcase: int fn1 (long x) { if (0) return __INT_MAX__ + 1; if (x || 0) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ if (1 || 0) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ if (0 && 0) return __INT_MAX__ + 1; if (0) return __INT_MAX__ + 1; else return 0; if (0) return 0; else return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ if (0) { if (x) return __INT_MAX__ + 1; } if (0) { if (x) return __INT_MAX__ + 1; } else { if (x) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ } if (0) if (0) return __INT_MAX__ + 1; if (1) return 0; else return __INT_MAX__ + 1; if (1) if (0) return __INT_MAX__ + 1; if (1) { if (x) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ } else { if (x) return __INT_MAX__ + 1; } if (0) return __INT_MAX__ + 1; else if (0) return __INT_MAX__ + 1; else return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ if (0) return __INT_MAX__ + 1; else if (1) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ else return __INT_MAX__ + 1; if (1) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ else if (0) return __INT_MAX__ + 1; return 0 ? __INT_MAX__ + 1 : 1; } void fn2 (int x) { if (0) x <<= sizeof (int) * __CHAR_BIT__; if (0) x <<= -1; if (0) x >>= sizeof (int) * __CHAR_BIT__; if (0) x >>= -1; }
(In reply to Marek Polacek from comment #2) > int > fn1 (long x) > { > if (0) > return __INT_MAX__ + 1; > > if (x || 0) > return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ > > if (1 || 0) > return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ The rest of fn1 is dead code (at least a good dataflow analysis would detect it). How about splitting fn1 into several functions? int fn11 (long x) { if (0) return __INT_MAX__ + 1; return 0; } int fn12 (long x) { if (x || 0) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ return 0; } int fn13 (long x) { if (1 || 0) return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ return 0; } int fn14 (long x) { if (0 && 0) return __INT_MAX__ + 1; return 0; } int fn15 (long x) { if (0) return __INT_MAX__ + 1; else return 0; } int fn16 (long x) { if (0) return 0; else return __INT_MAX__ + 1; /* { dg-warning "integer overflow" } */ } etc.
I might, but the front ends, where the warning is taking place, can't see that the function always returns early.
I think a similar issue was discussed in the past. The warnings are justified because implementations are allowed (even encouraged) to evaluate constant expressions during translation rather than runtime. As a result, the translation of a program that contains an overflowing constant expression has undefined behavior.
A prior discussion on this subject is in bug 68971.
> the translation of a program that contains an overflowing constant expression has undefined behavior Sure, but the programs in question do not contain constant expressions in sense of the C standard. They contain expressions that consist entirely of constants, which is a different thing. For example, assuming 32-bit int the following program contains the constant expression 'INT_MAX + 1' and the C standard requires a diagnostic for the overflow: #include <limits.h> int F (void) { if (0) { static int too_big = INT_MAX + 1; return too_big != 0; } return 0; } In contrast, the following program is valid C code and its behavior is well-defined because 'INT_MAX + 1' is not a constant expression (in the C-standard sense) and it is never evaluated: #include <limits.h> int G (void) { if (0) { int too_big = INT_MAX + 1; return too_big != 0; } return 0; } This bug report is about the latter kind of program. In practice these are often useful programs and GCC's overflow diagnostics are false alarms.
I understand the distinction, but I don't think it would be helpful to try to make it in the implementation of the warning, for a few reasons: 1) It's too subtle for non-expert programmers to understand. 2) It's unclear under what conditions the warning should or should not be issued. What if the controlling expression evaluates to zero but is not a constant expression? With or without optimization? 3) Avoiding the warning would require removing the implementation from the front end and adding it to the middle-end, which tends to lead to inconsistencies and both false positives and negatives. (This isn't an argument against also handling the overflow in the middle-end, as -Wstrict-overflow does, but rather one against removing it from the front end.) 4) There are a number of similar warnings that behave the same way (e.g., -Wshift-negative-value, -Wshift-count-overflow and -Wshift-overflow). GCC should be consistent in handling all these and so all the others would need to change as well. 5) There is an easy way to rewrite the code to avoid the warning: int too_large (long x) { #if INT_MAX < LONG_MAX // or in GCC 7, INT_SIZE < LONG_SIZE return 32768 * 65536L < x; #else return 0; #endif }
> 1) It's too subtle for non-expert programmers to understand. Actually, it's typically easy for non-experts to follow this. For example, although GCC falsely warns about this: /*1*/ if (0) return INT_MAX + 1; else return 0; GCC handles this correctly: /*2*/ return 0 ? INT_MAX + 1 : 0; If /*1*/ were really "too subtle" for non-experts, /*2*/ would also be "too subtle". In practice, though, /*2*/ is working well and is not "too subtle", and GCC should be able to make /*1*/ work as well. > 2) It's unclear under what conditions the warning should or should not be > issued. As a first approximation, have GCC treat /*1*/ like it treats /*2*/. > What if the controlling expression evaluates to zero but is not a > constant expression? Do what /*2*/ does. > With or without optimization? Again, do what /*2*/ does. > 3) Avoiding the warning would require removing the implementation from the > front end and adding it to the middle-end, which tends to lead to > inconsistencies and both false positives and negatives. (This isn't an > argument against also handling the overflow in the middle-end, as > -Wstrict-overflow does, but rather one against removing it from the front end.) Perhaps, given GCC's current internal structure it's nontrivial to fix GCC to eliminate the false alarms. However, this doesn't affect the fact that they are false alarms, and that problems in this area are due to problems in GCC's internal structure rather than being problems intrinsic to the analysis of C programs. > 4) There are a number of similar warnings that behave the same way (e.g., > -Wshift-negative-value, -Wshift-count-overflow and -Wshift-overflow). GCC > should be consistent in handling all these and so all the others would need to > change as well. Yes, that sounds right. > 5) There is an easy way to rewrite the code to avoid the warning: > > int too_large (long x) > { > #if INT_MAX < LONG_MAX // or in GCC 7, INT_SIZE < LONG_SIZE > return 32768 * 65536L < x; > #else > return 0; > #endif > } Although #if works for this particular example, it does not work with modern programming styles that try to avoid the preprocessor for various good reasons. Worse, #if does not work for expressions that contain constants that are not visible to the C proprocessor. In the following example, GCC reports a false alarm and this cannot be handled by the preprocessor because the preprocessor cannot see the values of ELTS and MULTIPLIER. Another example: sizeof is commonly involved in overflow checks, and the preprocessor cannot handle sizeof. #include <limits.h> /* Assume these are supplied by some header elsewhere, as enums. */ enum { ELTS = 1000000000, MULTIPLIER = 10 }; /* Return ELTS times MULTIPLIER. If the result would overflow, return the closest representable value. */ int ELTS_times_MULTIPLIER_saturated (void) { enum { a = ELTS, b = MULTIPLIER }; if (b < 0) { if (a < 0) { if (INT_MAX / b <= a) return INT_MAX; } else { if (b != -1 && INT_MIN / b < a) return INT_MIN; } } else if (b != 0) { if (a < 0) { if (a < INT_MIN / b) return INT_MIN; } else { if (INT_MAX / b < a) return INT_MAX; } } return a * b; }
This is arguably the same as or similar to bug 4210 and its duplicates.
Yes, bug 4210 looks like a duplicate. The test case from attachment 40722 [details] recast in the context of that enhancement request looks like this: int too_large (long x) { const int b = sizeof (int) < sizeof (long) if (b) return 32768 * 65536L < x; return 0; } An even less straightforward example might be one where b is an element of an array initialized (or assigned to) from constants, or, for a type with more than two values, in some set that excludes the tested value, or the result of a function call with constant arguments expanded inline by the optimizer. Etc. (Note that in all of these examples the warning is expected to do more than what it's intended and documented to do, namely "warn about compile-time overflow in constant expressions.") Deferring the warning until later stages of optimization in an effort to expand the set of contexts in which it recognizes that a constant expression cannot be evaluated at runtime would quite likely come at the cost of a growing rate of both false negatives and other false positives resulting from the very transformations the smarter warning would have to depend on.
If the proposed change would introduce significant problems with false positives or false negatives, then surely GCC already has these problems in conditional expressions. These problems ought to be addressed regardless of what GCC does with conditional statements. What false-positive and/or false-negative problems are these? Should they have bug reports? I'm still mystified as to why a conditional statement should generate false positives that the equivalent conditional expression does not generate. This makes little sense from the programmer's point of view.
The false positives/negatives I'm concerned about would result from moving the warning from the front-end to the middle-end. This would be necessary to handle some of the non-trivial cases I mentioned in comment #11. In the front-end the warning only sees as far as the end of a full expression. The front-end does no data flow analysis beyond attempting to evaluate expressions. In the middle-end, to expose dependencies between statements (such as the dependency of expr on cond in "if (cond) { expr; }" the warning would depend on various transformations done by optimization passes (such as constant propagation and dead code elimination). Some of these transformations could also simplify or remove expressions that involve constant overflows, leading to false negatives, and others could introduce them even though they didn't exist in the original code (e.g., along new code paths that may never be even executed), causing false positives. Bugs certainly should be raised for these problems (and many have been) but because of the complex interactions between the transformations they tend to be quite difficult to fix, and some can't be fixed at all. The net result would be a trade-off between one set of fairly simple bugs or complaints vs another, much more complex bugs. There are a number of examples of these trade-offs in Bugzilla with even more discussion on gcc-patches. This doesn't mean that warnings shouldn't be implemented in the middle-end, just that they aren't going to be perfect there either, or make everyone happy (in fact, simple warnings like -Woverflow tend to be less controversial than data flow warnings because they are for the most part decidable.) In any case, since bug 4210 already tracks the enhancement you would like to see I propose to close this as a duplicate of that bug. Please let me know if you disagree, otherwise I'll go ahead and resolve it as such.
Thanks, please feel free to mark this as a duplicate of Bug#4210. I plan to follow up there.
*** This bug has been marked as a duplicate of bug 4210 ***