Bug 52904 - -Wstrict-overflow false alarm with bounded loop
Summary: -Wstrict-overflow false alarm with bounded loop
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, missed-optimization, xfail
Depends on:
Blocks:
 
Reported: 2012-04-08 08:58 UTC by Paul Eggert
Modified: 2016-08-17 09:13 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-04-10 00:00:00


Attachments
simplified version of Emacs code, illustrating the bug (154 bytes, text/plain)
2012-04-08 08:58 UTC, Paul Eggert
Details
gcc -v output, for test case (654 bytes, text/plain)
2012-04-08 08:59 UTC, Paul Eggert
Details
simplified version of Emacs code, illustrating the bug (154 bytes, text/plain)
2012-04-08 09:02 UTC, Paul Eggert
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Eggert 2012-04-08 08:58:51 UTC
Created attachment 27113 [details]
simplified version of Emacs code, illustrating the bug

I ran into this problem when trying to build GNU Emacs with -Wstrict-overflow.

Compiling the attached program 'v.i' with the following command:

gcc -c -Wstrict-overflow -O2 v.i

generates the diagnostic:

v.i: In function 'wait_reading_process_output':
v.i:14:6: error: assuming signed overflow does not occur when simplifying conditional to constant [-Werror=strict-overflow]

The diagnostic is obviously incorrect, since the variable 'nfds' cannot possibly exceed 1024.

I will also attach the output of "gcc -v -c -Wstrict-overflow -O2 v.i".
Comment 1 Paul Eggert 2012-04-08 08:59:43 UTC
Created attachment 27114 [details]
gcc -v output, for test case
Comment 2 Paul Eggert 2012-04-08 09:02:22 UTC
Created attachment 27115 [details]
simplified version of Emacs code, illustrating the bug

Re-uploading the test case, marked as "text/plain" this time, so it's easier to download.
Comment 3 Andrew Pinski 2012-04-08 10:05:26 UTC
(In reply to comment #2)
> Created attachment 27115 [details]
> simplified version of Emacs code, illustrating the bug
> 
> Re-uploading the test case, marked as "text/plain" this time, so it's easier to
> download.

You did not have to re-upload the testcase, you can change the MIME type of the atttachment after the fact.
Comment 4 Richard Biener 2012-04-10 12:16:54 UTC
It's the

  if (nfds < 0)

conditional that we warn on.  We cannot currently derive an upper bound
for nfds based on the number of iterations of the loop as it is not
an induction variable we can analyze.

int
wait_reading_process_output (void)
{
  int nfds = 0;
  int channel;
  for (channel = 0; channel < 1024; ++channel)
    {
      if (foo (channel))
	nfds++;
    }
  if (nfds < 0)
    return 1;

This is in some way a missed optimization in value-range analysis as well.
Comment 5 Laurent Rineau 2013-12-12 17:12:26 UTC
With the following gcc version:
  gcc (GCC) 4.8.2 20131017 (Red Hat 4.8.2-1)

I have a similar result:

$ gcc -c -Wstrict-overflow -O2 v.i
v.i: In function ‘wait_reading_process_output’:
v.i:14:6: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   if (nfds < 0)
      ^

That diagnostic seems right, according to the documentation of -Wstrict-overflow.
Comment 6 Paul Eggert 2013-12-12 18:12:14 UTC
> That diagnostic seems right, according to the documentation of -Wstrict-overflow.

The diagnostic is "right" only in the sense that it is correctly reporting
that GCC does not deduce that signed overflow cannot possibly occur in
this function.  It is not "right" in the common sense that a programmer
would ordinarily want, i.e., that the program may have a bug because
signed overflow might lead to undefined behavior.  (Surely the diagnostic
is supposed to be for the benefit of programmers trying to find potential
bugs in their programs, not for the benefit of GCC maintainers trying
to explain how GCC works internally.  :-)
Comment 7 Laurent Rineau 2013-12-12 18:19:14 UTC
In the test case, nfds cannot overflow, because of two reasons:
  - nfds is only incremented from 0, and -fstrict-overflow allows gcc to suppose it will not overflow,
  - the number of iterations of the loop cannot allow nfds to overflow, even without -fstrict-overflow.

Gcc warns that the test (nfds < 0) is useless, because of -fstrict-overflow. The developer has two solutions:
  - remove that test,
  - or compile with -fno-strict-overflow.
Comment 8 Paul Eggert 2013-12-12 19:34:16 UTC
On 12/12/2013 10:19 AM, Laurent.Rineau__gcc at normalesup dot org wrote:
> The developer has two solutions:
>   - remove that test,
>   - or compile with -fno-strict-overflow.

Sure, and because of this problem, GNU Emacs has chosen #2, that
is, Emacs doesn't use -Wstrict-overflow any more.  (Removing
the test would unnecessarily complicate Emacs, since the
test is needed on some platforms that are conditionally
compiled away in this stripped-down example.)

A goal of -Wstrict-overflow, at least at the 1 level, is to
not generate false alarms, so that it's generally useful in
real programs.  This bug report gives one example where the
goal is not being met.  Emacs currently has a half dozen or
so such examples of this (please see below for what the
current Emacs bzr trunk would output, if we enabled this
warning) and I thought that the GCC developers would find it
useful to see one of them.  If you're not interested, that's
OK too; Emacs will continue to not use -Wstrict-overflow.

In file included from dispnew.c:26:0:
dispnew.c: In function ‘update_window’:
lisp.h:749:30: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   return num < lower ? lower : num <= upper ? num : upper;

dispnew.c: In function ‘update_frame_1’:
dispnew.c:4490:19: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   pause_p = 0 < i && i < FRAME_LINES (f) - 1;
fileio.c: In function ‘Finsert_file_contents’:
fileio.c:3630:11: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
        if (nread < 0)
           ^
fileio.c:3632:16: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
        else if (nread > 0)
                ^
eval.c: In function ‘backtrace_eval_unrewind’:
eval.c:3496:3: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   for (; distance > 0; distance--)
   ^
intervals.c: In function ‘offset_intervals’:
intervals.c:1364:6: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   if (length == TOTAL_LENGTH (tree))
      ^
intervals.c:1379:9: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   while (left_to_delete > 0)
         ^
Comment 9 kugan 2014-08-08 10:21:19 UTC
Incorrect warning is gone from the latest trunk build. VRP is now able to detect the correct range. This bug can now be closed.

----------------------------------------------------
Value ranges after VRP:

channel_8: [1, 1023]  EQUIVALENCES: { channel_10 } (1 elements)
channel_10: [1, 1024]
channel_15: [0, 1023]
.MEM_16: VARYING


Removing basic block 5
wait_reading_process_output ()
{
  int channel;
  int nfds;

  <bb 2>:

  <bb 3>:
  # channel_15 = PHI <channel_10(3), 0(2)>
  foo (channel_15);
  channel_10 = channel_15 + 1;
  if (channel_10 != 1024)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 4>:
  return 0;

}
Comment 10 Manuel López-Ibáñez 2014-08-08 15:05:17 UTC
(In reply to kugan from comment #9)
> Incorrect warning is gone from the latest trunk build. VRP is now able to
> detect the correct range. This bug can now be closed.

Since it was probably fixed by accident, it will get unfixed by accident again.
If you don't add the testcase to the regression testsuite, this will happen again:

https://gcc.gnu.org/wiki/HowToPrepareATestcase

If you add a testcase pointing to this PR, when it fails people can come back here and read the discussion.
Comment 11 kugan 2014-08-08 22:16:01 UTC
I agree. I will post a patch to add this test-case and let the maintainers decide if this is necessary.
Comment 12 kugan 2014-08-18 06:29:08 UTC
Author: kugan
Date: Mon Aug 18 06:28:35 2014
New Revision: 214084

URL: https://gcc.gnu.org/viewcvs?rev=214084&root=gcc&view=rev
Log:
gcc/testsuite
2014-08-18  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR tree-optimization/52904
	* gcc.dg/pr52904.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/pr52904.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 13 Richard Biener 2016-08-17 09:13:54 UTC
Re-opening because I am fixing a bug that makes the diagnostic re-appear (it was "fixed" by a bug that caused us to meet [0, +INF] and [1, +INF(OVF)] as [0, +INF]
rather than [0, +INF(OVF)]).