[Bug tree-optimization/103161] New: [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[Bug tree-optimization/103161] New: [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

            Bug ID: 103161
           Summary: [12 Regression] Better ranges cause
                    builtin-sprintf-warn-16.c failure
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: law at gcc dot gnu.org
  Target Milestone: ---

On a variety of platforms builtin-sprintf-warn-16.c has started failing since
converting strlen to use Ranger.

Tests that now fail, but worked before (6 tests):

or1k-sim: gcc.dg/tree-ssa/builtin-sprintf-warn-16.c  (test for warnings, line
142)
or1k-sim: gcc.dg/tree-ssa/builtin-sprintf-warn-16.c  (test for warnings, line
243)
or1k-sim: gcc.dg/tree-ssa/builtin-sprintf-warn-16.c (test for excess errors)

The excess errors and line 142 failure are due to getting tighter ranges out of
Ranger which seems to have confused the wrap-around bits in the sprintf
warnings.

A reduced testcase for or1k-elf that you can trigger with a cross:

# 0 "k.c"
# 0 "<built-in>"
# 0 "<command-line>"
# 1 "k.c"






typedef unsigned int size_t;
typedef unsigned int wchar_t;

void sink (void*);
void* get_value (void);
# 22 "k.c"
extern char buf[1];






typedef signed long long sint128_t;
typedef unsigned long long uint128_t;


const sint128_t sint128_max
  = (sint128_t)1 << (sizeof sint128_max * 8 - 2);
const sint128_t uint128_max = (uint128_t)-1;


void test_width_var (void)
{

  {


    extern unsigned w;
    if (w < 5 || (unsigned)-1 - 7 < w)
      w = 5;

    __builtin_sprintf (buf + 1, "%*u", w, *(int*)get_value ()); sink (buf);
  }
}


The relevant bits from the .strlen dump:

Old:
! j.c:52: __builtin_sprintf: objsize = 0, fmtstr = "%*u"
!   Directive 1 at offset 0: "%*u", width in range [0, 2147483648]
!     Result: 1, 1, 2147483648, 2147483648 (1, 1, 2147483648, 2147483648)
    Directive 2 at offset 3: "", length = 1

New:
! k.c:52: __builtin_sprintf: objsize = 0, fmtstr = "%*u"
!   Directive 1 at offset 0: "%*u", width in range [5, 4294967288]
!     Result: 5, 5, -1, 4294967288 (5, 5, -1, -1)
    Directive 2 at offset 3: "", length = 1


AFAICT we've got a tighter range from Ranger, which in turn is confusing some
of the wrap-around logic.

Martin, can you take a look a this?

[Bug tree-optimization/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

--- Comment #1 from Jeffrey A. Law <law at gcc dot gnu.org> ---
I suspect the same underlying issue is affecting the test on line #243 as well.

[Bug tree-optimization/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
   Target Milestone|---                         |12.0

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug tree-optimization/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-11-10
     Ever confirmed|0                           |1

[Bug tree-optimization/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=102831
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |msebor at gcc dot gnu.org

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
Confirmed.  It's a failure in what looks like a paranoia test that tries to
verify that the sprintf pass isn't making overly aggressive assumptions about
the width field.  The sprintf pass calls get_range_expr() to determine the
range.  It runs as part of the strlen pass which still uses EVRP for ranges. 
It's strange that the range is different for some targets than others (the IL
is otherwise identical).  Since the code is using the same EVRP analyzer as
before I'd expect the GIMPLE statement passed to get_range_expr() to be ignored
and the range to be "global" for the whole function body.  I haven't looked
into this in enough detail to understand what's different.  As of now this is
just as puzzling to me as the false positive reported in pr102831.  I'd expect
converting the strlen pass to use ranger to fix this.  I plan to work on it
sometime soon.

[Bug tree-optimization/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #3)
> Confirmed.  It's a failure in what looks like a paranoia test that tries to
> verify that the sprintf pass isn't making overly aggressive assumptions
> about the width field.  The sprintf pass calls get_range_expr() to determine
> the range.  It runs as part of the strlen pass which still uses EVRP for
> ranges.  It's strange that the range is different for some targets than
> others (the IL is otherwise identical).  Since the code is using the same
> EVRP analyzer as before I'd expect the GIMPLE statement passed to
> get_range_expr() to be ignored and the range to be "global" for the whole
> function body.  I haven't looked into this in enough detail to understand
> what's different.  As of now this is just as puzzling to me as the false
> positive reported in pr102831.  I'd expect converting the strlen pass to use
> ranger to fix this.  I plan to work on it sometime soon.

strlen is no longer using evrp.  The only remaining uses of the evrp engine are
DOM and evrp running in classic mode (which is off by default):

$ grep evrp_range_analyzer * -l
gimple-ssa-evrp-analyze.c
gimple-ssa-evrp-analyze.h
gimple-ssa-evrp.c
tree-ssa-dom.c

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|tree-optimization           |testsuite

--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
Great!  With the strlen conversion to ranger
(g:6b8b959675a3e14cfdd2145bd62e4260eb193765) the test now fails on x86_64 as
well:

Excess errors:
/src/gcc/master/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-16.c:142:5:
warning: '%*u' directive writing 5 or more bytes into a region of size 0
[-Wformat-overflow=]

while on or1k-elf:

Excess errors:
/src/gcc/master/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-16.c:142:5:
warning: '%*u' directive writing 5 or more bytes into a region of size 0
[-Wformat-overflow=]
/src/gcc/master/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-16.c:243:5:
warning: '%.*u' directive writing 7 or more bytes into a region of size 0
[-Wformat-overflow=]

I think the test unwittingly depends on GCC not inferring a range from a
conditional (although the intent is clearly that it does).  It uses an unsigned
int as the width argument to sprintf which is undefined, but I suspect it does
it as an attempt to create a signed anti-range.  So this might simply be a
problem with the test.  The difference between EVRP and Ranger can be seen in
the test case below (function g() corresponds to what the test does, and with
EVRP the range for w in it is the same as in f() on some targets like or1k-elf
and on others it's varying):

$ cat pr103161.c && gcc-11 -O2 -S -Wall -fdump-tree-strlen=/dev/stdout
pr103161.c
extern char a[1];

int f (int i, unsigned w)
{
  if (w < 5)
    w = 5;

  return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5, INT_MAX]
}

int g (int i)
{
  extern unsigned w;

  if (w < 5)
    w = 5;

  return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [0, INT_MAX]
}

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

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }
pr103161.c:8: __builtin_sprintf: objsize = 0, fmtstr = "%*i"
  Directive 1 at offset 0: "%*i", width in range [5, 4294967295]
pr103161.c: In function ‘f’:
pr103161.c:8:37: warning: ‘%*i’ directive writing 5 or more bytes into a region
of size 0 [-Wformat-overflow=]
    8 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5,
INT_MAX]
      |                                     ^~~
    Result: 5, 5, -1, 4294967295 (5, 5, -1, -1)
  Directive 2 at offset 3: "", length = 1
pr103161.c:8:10: note: ‘__builtin_sprintf’ output 6 or more bytes into a
destination of size 0
    8 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5,
INT_MAX]
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Discarding out-of-bounds return value 0.

int f (int i, unsigned int w)
{
  unsigned int _2;
  int _6;

  <bb 2> [local count: 1073741824]:
  _2 = MAX_EXPR <w_1(D), 5>;
  _6 = __builtin_sprintf (&MEM <char[1]> [(void *)&a + 1B], "%*i", _2, i_4(D));
  return _6;

}



;; Function g (g, funcdef_no=1, decl_uid=1948, cgraph_uid=2, symbol_order=1)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2 3 4
;; 2 succs { 3 4 }
;; 3 succs { 4 }
;; 4 succs { 1 }
pr103161.c:18: __builtin_sprintf: objsize = 0, fmtstr = "%*i"
  Directive 1 at offset 0: "%*i", width in range [0, 2147483648]
pr103161.c: In function ‘g’:
pr103161.c:18:37: warning: ‘%*i’ directive writing between 1 and 2147483648
bytes into a region of size 0 [-Wformat-overflow=]
   18 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [0,
INT_MAX]
      |                                     ^~~
    Result: 1, 1, 2147483648, 2147483648 (1, 1, 2147483648, 2147483648)
  Directive 2 at offset 3: "", length = 1
pr103161.c:18:10: note: ‘__builtin_sprintf’ output between 2 and 2147483649
bytes into a destination of size 0
   18 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [0,
INT_MAX]
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Discarding out-of-bounds return value 0.

int g (int i)
{
  unsigned int w.0_1;
  int _8;
  unsigned int prephitmp_9;

  <bb 2> [local count: 1073741824]:
  w.0_1 = w;
  if (w.0_1 <= 4)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:
  w = 5;

  <bb 4> [local count: 1073741824]:
  # prephitmp_9 = PHI <w.0_1(2), 5(3)>
  _8 = __builtin_sprintf (&MEM <char[1]> [(void *)&a + 1B], "%*i", prephitmp_9,
i_6(D));
  return _8;

}

While with today's trunk:

$ gcc-12 -O2  -S -Wall -fdump-tree-strlen=/dev/stdout pr103161.c
extern char a[1];

int f (int i, unsigned w)
{
  if (w < 5)
    w = 5;

  return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5, INT_MAX]
}

int g (int i)
{
  extern unsigned w;

  if (w < 5)
    w = 5;

  return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5, INT_MAX]
}

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

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }
pr103161.c:8: __builtin_sprintf: objsize = 0, fmtstr = "%*i"
  Directive 1 at offset 0: "%*i", width in range [5, 4294967295]
pr103161.c: In function ‘f’:
pr103161.c:8:37: warning: ‘%*i’ directive writing 5 or more bytes into a region
of size 0 [-Wformat-overflow=]
    8 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5,
INT_MAX]
      |                                     ^~~
    Result: 5, 5, -1, 4294967295 (5, 5, -1, -1)
  Directive 2 at offset 3: "", length = 1
pr103161.c:8:10: note: ‘__builtin_sprintf’ output 6 or more bytes into a
destination of size 0
    8 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5,
INT_MAX]
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Discarding out-of-bounds return value 0.

int f (int i, unsigned int w)
{
  unsigned int _2;
  int _6;

  <bb 2> [local count: 1073741824]:
  _2 = MAX_EXPR <w_1(D), 5>;
  _6 = __builtin_sprintf (&MEM <char[1]> [(void *)&a + 1B], "%*i", _2, i_4(D));
  return _6;

}



;; Function g (g, funcdef_no=1, decl_uid=1983, cgraph_uid=2, symbol_order=1)

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2 3 4
;; 2 succs { 3 4 }
;; 3 succs { 4 }
;; 4 succs { 1 }
pr103161.c:18: __builtin_sprintf: objsize = 0, fmtstr = "%*i"
  Directive 1 at offset 0: "%*i", width in range [5, 4294967295]
pr103161.c: In function ‘g’:
pr103161.c:18:37: warning: ‘%*i’ directive writing 5 or more bytes into a
region of size 0 [-Wformat-overflow=]
   18 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5,
INT_MAX]
      |                                     ^~~
    Result: 5, 5, -1, 4294967295 (5, 5, -1, -1)
  Directive 2 at offset 3: "", length = 1
pr103161.c:18:10: note: ‘__builtin_sprintf’ output 6 or more bytes into a
destination of size 0
   18 |   return __builtin_sprintf (a + 1, "%*i", w, i);   // w's range is [5,
INT_MAX]
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Discarding out-of-bounds return value 0.

int g (int i)
{
  unsigned int w.0_1;
  int _8;
  unsigned int prephitmp_9;

  <bb 2> [local count: 1073741824]:
  w.0_1 = w;
  if (w.0_1 <= 4)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:
  w = 5;

  <bb 4> [local count: 1073741824]:
  # prephitmp_9 = PHI <w.0_1(2), 5(3)>
  _8 = __builtin_sprintf (&MEM <char[1]> [(void *)&a + 1B], "%*i", prephitmp_9,
i_6(D));
  return _8;

}

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

--- Comment #6 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #5)
> Great!  With the strlen conversion to ranger
> (g:6b8b959675a3e14cfdd2145bd62e4260eb193765) the test now fails on x86_64 as
> well:

I didn't see any regressions on x86-64 testing, except
builtin-sprintf-warn-22.c which I xfailed.

> I think the test unwittingly depends on GCC not inferring a range from a
> conditional (although the intent is clearly that it does).  It uses an
> unsigned int as the width argument to sprintf which is undefined, but I
> suspect it does it as an attempt to create a signed anti-range.  So this

If the code uses multi-ranges there should be no anti-ranges.  Anti-ranges do
not exist in the new world, and reference to them should be removed.

I still see them in:
gimple-ssa-warn-restrict.c
tree-ssa-strlen.c
builtins.c
ipa-fnsummary.c

(well and in ipa-prop.c, but they're kinda hijacking it for their own
purposes).

The remaining uses are internal to the legacy implementation (vrvalues,
tree-vrp, etc) so they're ok.

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

--- Comment #7 from Martin Sebor <msebor at gcc dot gnu.org> ---
I bet you can't see the failures because they depend on fixes in a patch that
hasn't been committed yet:
  https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583735.html

But I wonder if there actually is something else wrong besides the test making
bad assumptions.  I glossed over an important detail in the test and reduced it
too far to capture the difference between the or1k-elf and x86_64 IL.  This is
closer to what the test does:

extern char a[1];

int f (void);

int g (void)
{
  extern unsigned w;

  if (w < 5)
    w = 5;

  return __builtin_sprintf (a + 1, "%*i", w, f ());   <<< call f() to get value
}

The call to f() is what in x86_64 resets w's range (because w is extern and the
call to f() might overwrite it).  But with the or1k-elf target, that's not what
happens.  The IL shows that the call is made after the store to w and the value
used for the width in the sprintf() call is the one before the call.  That
seems like a bug, and that's the reason for the difference in the warning on
the two targets:

int g ()
{
  unsigned int w.0_1;
  int _3;
  int _9;
  unsigned int prephitmp_10;

  <bb 2> [local count: 1073741824]:
  w.0_1 = w;
  if (w.0_1 <= 4)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:
  w = 5;

  <bb 4> [local count: 1073741824]:
  # prephitmp_10 = PHI <w.0_1(2), 5(3)>               <<< w used in sprintf()
  _3 = f ();                                          <<< call to f() might
clobber w
  _9 = __builtin_sprintf (&MEM <char[1]> [(void *)&a + 1B], "%*i",
prephitmp_10, _3);
  return _9;

}

The x86_64 IL looks correct:

  <bb 4> [local count: 1073741824]:
  _2 = f ();
  w.1_3 = w;
  _9 = __builtin_sprintf (&MEM <char[1]> [(void *)&a + 1B], "%*i", w.1_3, _2);
  return _9;

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

--- Comment #8 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
https://en.cppreference.com/w/cpp/language/eval_order

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

--- Comment #9 from Martin Sebor <msebor at gcc dot gnu.org> ---
Aaah, never mind.  The test depends on the unspecified order of argument
evaluation.  Doh!

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:7c8a416da86c175bcb5fa7b49abf3cf343bc068f

commit r12-5134-g7c8a416da86c175bcb5fa7b49abf3cf343bc068f
Author: Martin Sebor <msebor@redhat.com>
Date:   Wed Nov 10 11:36:16 2021 -0700

    Adjust test to avoid target-specific failures [PR103161].

    Resolves:
    PR testsuite/103161 - Better ranges cause builtin-sprintf-warn-16.c failure

    gcc/testsuite:
            PR testsuite/103161
            * gcc.dg/tree-ssa/builtin-sprintf-warn-16.c: Avoid relying on
            argument evaluation order.  Cast width and precision to signed
            to avoid undefined behavior.

[Bug testsuite/103161] [12 Regression] Better ranges cause builtin-sprintf-warn-16.c failure

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103161

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #11 from Martin Sebor <msebor at gcc dot gnu.org> ---
Fixed.