Bug 80776 - -Wformat-overflow false positive for %d on integer bounded by __builtin_unreachable
Summary: -Wformat-overflow false positive for %d on integer bounded by __builtin_unrea...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 7.1.0
: P3 normal
Target Milestone: ---
Assignee: Richard Biener
URL:
Keywords: diagnostic, missed-optimization
Depends on:
Blocks: Wformat-overflow
  Show dependency treegraph
 
Reported: 2017-05-16 07:43 UTC by Paul Eggert
Modified: 2023-10-25 03:36 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 10.0, 6.4.0, 7.5.0, 8.4.0, 9.2.0
Last reconfirmed: 2020-03-04 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Eggert 2017-05-16 07:43:29 UTC
I found this problem when compiling Emacs with GCC 7.1.0 on x86-64. Emacs uses __builtin_unreachable to let the compiler know about ranges and thereby suppress false alarms. Here is a stripped-down example of the bug:

  extern int sprintf (char *restrict, const char *restrict, ...)
    __attribute__ ((__nothrow__));
  extern int foo (void);

  int
  Fgenerate_new_buffer_name (void)
  {
    char number[2];
    int i = foo ();
    if (i < 0)
      __builtin_unreachable ();
    if (i >= 2)
      __builtin_unreachable ();
    return sprintf (number, "%d", i);
  }

Compile this with:

  gcc -c -Wformat-overflow -O2  t.i

The output is:

t.i: In function 'Fgenerate_new_buffer_name':
t.i:14:28: warning: '%d' directive writing between 1 and 10 bytes into a region\
 of size 2 [-Wformat-overflow=]
   return sprintf (number, "%d", i);
                            ^~
t.i:14:27: note: directive argument in the range [0, 2147483647]
   return sprintf (number, "%d", i);
                           ^~~~

The diagnostic is incorrect, as the directive argument I is in the range [0, 1]. Changing the two ifs to read just "if (i < 0 || i >= 2) __builtin_unreachable ();" works around this particular bug, but other variants show similar problems.
Comment 1 Richard Biener 2017-05-16 09:10:12 UTC
Possibly the walk in remove_range_assertions visits the latter before the former block but in principle we do have code to handle this there.

Confirmed also as missed VRP optimization.
Comment 2 Joseph S. Myers 2017-11-21 16:57:28 UTC
The following, reduced from an unsuccessful attempt to avoid a spurious -Wformat-overflow warning in glibc, produces a spurious warning with current GCC and looks like the same bug or something very similar.

char buf[15];

void
f (int a, int b, int c, int d, int e, int f)
{
  if (a < -999 || a > 9999
      || b < 1 || b > 12
      || c < 1 || c > 31
      || d < 0 || d > 23
      || e < 0 || e > 59
      || f < 0 || f > 60)
    __builtin_unreachable ();
  __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
}

u.c: In function 'f':
u.c:13:44: warning: '%02d' directive writing between 2 and 10 bytes into a region of size 5 [-Wformat-overflow=]
   __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
                                            ^~~~
u.c:13:27: note: directive argument in the range [0, 2147483647]
   __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
u.c:13:27: note: directive argument in the range [0, 2147483647]
u.c:13:3: note: '__builtin_sprintf' output between 15 and 31 bytes into a destination of size 15
   __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 3 Paul Eggert 2017-11-26 00:02:46 UTC
(In reply to Richard Biener from comment #1)
> Possibly the walk in remove_range_assertions visits the latter before the
> former block but in principle we do have code to handle this there.

I just ran into the same problem again, with the following code derived from GNU Emacs, and it suggests that your diagnosis is correct. Perhaps this can be used in a test case once the bug is fixed. If I remove the "if (! (0 <= i)) __builtin_unreachable ();" the bogus warning goes away, which suggests that the earlier test is hiding the later one somehow. This is with GCC 7.2.1 20170915 (Red Hat 7.2.1-2) on x86-64.

extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__)) __attribute__ ((__artificial__)) int
__attribute__ ((__nothrow__ , __leaf__)) sprintf (char *__restrict __s, const char *__restrict __fmt, ...)
{
  return __builtin___sprintf_chk (__s, 2 - 1,
      __builtin_object_size (__s, 2 > 1), __fmt, __builtin_va_arg_pack ());
}
char number[sizeof "999999"];
int somerandom (void);
void
Foo (void)
{
  int i = somerandom ();
  if (! (0 <= i))
    __builtin_unreachable ();
  if (! (0 <= i && i <= 999999))
    __builtin_unreachable ();
  sprintf (number, "%d", i);
}

$ gcc -Wformat-overflow -O2 -S v.i
v.i: In function ‘Foo’:
v.i:17:21: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 7 [-Wformat-overflow=]
   sprintf (number, "%d", i);
                     ^~
v.i:17:20: note: directive argument in the range [0, 2147483647]
   sprintf (number, "%d", i);
                    ^~~~
v.i:4:10: note: ‘__builtin___sprintf_chk’ output between 2 and 11 bytes into a destination of size 7
   return __builtin___sprintf_chk (__s, 2 - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       __builtin_object_size (__s, 2 > 1), __fmt, __builtin_va_arg_pack ());
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 4 Richard Biener 2017-11-27 10:48:53 UTC
For this case it is

  <bb 2> [100.00%]:
  # RANGE [0, 2147483647] NONZERO 2147483647
  # USE = nonlocal { D.1796 } (nonlocal, escaped, interposable)
  # CLB = nonlocal { D.1796 } (nonlocal, escaped, interposable)
  i_4 = somerandom ();
  # RANGE [0, 4294967295]
  _1 = (unsigned int) i_4;
  # RANGE [0, 1]
  _7 = _1 > 999999;
  if (_7 != 0)
    goto <bb 3>; [0.04%]
  else
    goto <bb 4>; [99.96%]

  <bb 3> [0.08%]:
  __builtin_unreachable ();

  <bb 4> [99.92%]:
  # USE = nonlocal { D.1796 } (nonlocal, escaped, interposable)
  # CLB = nonlocal { D.1796 } (nonlocal, escaped, interposable)
  __builtin___sprintf_chk (&number, 1, 7, "%d", i_4);
  return;


that is not handled because we don't insert ASSERT_EXPRs for i_4 (we don't
do that for uses appearing in PHI merge position because we don't insert
PHIs).

The DOM algorithm would get this right but it doesn't have the
remove_range_assertions () "trick" of handling __builtin_unreachable ().

Let me see if I can manage to fix that.
Comment 5 Richard Biener 2017-11-28 14:58:43 UTC
Author: rguenth
Date: Tue Nov 28 14:58:11 2017
New Revision: 255201

URL: https://gcc.gnu.org/viewcvs?rev=255201&root=gcc&view=rev
Log:
2017-11-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80776
	* gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info):
	Declare.
	* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info):
	New function.
	(evrp_range_analyzer::record_ranges_from_incoming_edges):
	If the incoming edge is an effective fallthru because the other
	edge only reaches a __builtin_unreachable () then record ranges
	derived from the controlling condition in SSA info.
	(evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info.
	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.

	* gcc.dg/pr80776-1.c: New testcase.
	* gcc.dg/pr80776-2.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/pr80776-1.c
    trunk/gcc/testsuite/gcc.dg/pr80776-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-ssa-evrp-analyze.c
    trunk/gcc/gimple-ssa-evrp-analyze.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c
    trunk/gcc/tree-vrp.h
Comment 6 Richard Biener 2017-11-28 14:59:35 UTC
The original case is now fixed.
Comment 7 Martin Sebor 2020-03-04 17:43:28 UTC
Reconfirming that test case in comment #2 still triggers the warning in GCC 10.  The dump shows that the warning thinks the fifth directive (%02d) generates between 2 and 10 bytes of output.  The symptoms feel similar to pr94021 but I haven't looked into whether they underlying root causes are related.

    Result: 2, 10, 10, 10 (12, 20, 20, 20)

$ gcc -O2 -S -Wall -fdump-tree-strlen=/dev/stdout pr80776-c2.c 

;; Function f (f, funcdef_no=0, decl_uid=1936, cgraph_uid=1, symbol_order=1)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }
pr80776-c2.c:14: __builtin_sprintf: objsize = 15, fmtstr = "%04d%02d%02d%02d%02d%02d"
  Directive 1 at offset 0: "%04d"
    Result: 4, 4, 4, 4 (4, 4, 4, 4)
  Directive 2 at offset 4: "%02d"
    Result: 2, 2, 2, 2 (6, 6, 6, 6)
  Directive 3 at offset 8: "%02d"
    Result: 2, 2, 2, 2 (8, 8, 8, 8)
  Directive 4 at offset 12: "%02d"
    Result: 2, 2, 2, 2 (10, 10, 10, 10)
  Directive 5 at offset 16: "%02d"
pr80776-c2.c: In function ‘f’:
pr80776-c2.c:14:44: warning: ‘%02d’ directive writing between 2 and 10 bytes into a region of size 5 [-Wformat-overflow=]
   14 |   __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
      |                                            ^~~~
pr80776-c2.c:14:27: note: directive argument in the range [0, 2147483647]
   14 |   __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
    Result: 2, 10, 10, 10 (12, 20, 20, 20)
  Directive 6 at offset 20: "%02d"
pr80776-c2.c:14:27: note: directive argument in the range [0, 60]
    Result: 2, 2, 2, 2 (14, 22, 22, 22)
  Directive 7 at offset 24: "", length = 1
pr80776-c2.c:14:3: note: ‘__builtin_sprintf’ output between 15 and 23 bytes into a destination of size 15
   14 |   __builtin_sprintf (buf, "%04d%02d%02d%02d%02d%02d", a, b, c, d, e, f);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

f (int a, int b, int c, int d, int e, int f)
{
  <bb 2> [local count: 1073741824]:
  __builtin_sprintf (&buf, "%04d%02d%02d%02d%02d%02d", a_14(D), b_15(D), c_16(D), d_17(D), e_18(D), f_19(D));
  return;

}
Comment 8 Andrew Pinski 2023-06-21 04:28:17 UTC
(In reply to Martin Sebor from comment #7)
> Reconfirming that test case in comment #2 still triggers the warning in GCC
> 10.  The dump shows that the warning thinks the fifth directive (%02d)
> generates between 2 and 10 bytes of output.  The symptoms feel similar to
> pr94021 but I haven't looked into whether they underlying root causes are
> related.

That testcase looks to be fixed in GCC 13:
/app/example.cpp:13: __builtin_sprintf: objsize = 15, fmtstr = "%04d%02d%02d%02d%02d%02d"
  Directive 1 at offset 0: "%04d"
    Result: 4, 4, 4, 4 (4, 4, 4, 4)
  Directive 2 at offset 4: "%02d"
    Result: 2, 2, 2, 2 (6, 6, 6, 6)
  Directive 3 at offset 8: "%02d"
    Result: 2, 2, 2, 2 (8, 8, 8, 8)
  Directive 4 at offset 12: "%02d"
    Result: 2, 2, 2, 2 (10, 10, 10, 10)
  Directive 5 at offset 16: "%02d"
    Result: 2, 2, 2, 2 (12, 12, 12, 12)
  Directive 6 at offset 20: "%02d"
    Result: 2, 2, 2, 2 (14, 14, 14, 14)
  Directive 7 at offset 24: "", length = 1
  Substituting 14 for return value.



GCC 13:
  # RANGE [irange] int [-999, 9999]
  intD.6 a_14(D) = aD.2739;
  # RANGE [irange] int [1, 12] NONZERO 0xf
  intD.6 b_15(D) = bD.2740;
  # RANGE [irange] int [1, 31] NONZERO 0x1f
  intD.6 c_16(D) = cD.2741;
  # RANGE [irange] int [0, 23] NONZERO 0x1f
  intD.6 d_17(D) = dD.2742;
  # RANGE [irange] int [0, 59] NONZERO 0x3f
  intD.6 e_18(D) = eD.2743;
  # RANGE [irange] int [0, 60] NONZERO 0x3f
  intD.6 f_19(D) = fD.2744;


GCC 12:

  # RANGE [-999, 9999]
  intD.6 a_14(D) = aD.1979;
  # RANGE [1, 12] NONZERO 15
  intD.6 b_15(D) = bD.1980;
  # RANGE [1, 31] NONZERO 31
  intD.6 c_16(D) = cD.1981;
  # RANGE [0, 23] NONZERO 31
  intD.6 d_17(D) = dD.1982;
  # RANGE [0, 2147483647] NONZERO 2147483647
  intD.6 e_18(D) = eD.1983;
  # RANGE [0, 2147483647] NONZERO 2147483647
  intD.6 f_19(D) = fD.1984;


So yes this has been fixed for GCC 13.
Comment 9 Andrew Pinski 2023-06-21 04:28:35 UTC
.
Comment 10 Andrew Pinski 2023-10-25 03:35:53 UTC
I am going to reopen this because the VRP issue has not been solved.

You can reproduce the same issue adding -fno-tree-reassoc to the testcase.

It just happens reassociating combined with the find_duplicates part of the PRE pass allows it to be optimized.