[Bug c/103835] New: Bogus sprintf warnings

[Bug c/103835] New: Bogus sprintf warnings

[lavr at ncbi dot nlm.nih.gov]

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

            Bug ID: 103835
           Summary: Bogus sprintf warnings
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lavr at ncbi dot nlm.nih.gov
  Target Milestone: ---

Please address these warnings because they create more noise than they help!

$ cat test.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


const char* fun(char* buf, const char* pfx, int a, int b)
{
    sprintf(buf, "%sa = %d\n"
                 "%sb = %d\n",
                 pfx, a, pfx, b);
    return buf;
}


int main(int argc, char* argv[])
{
    char buf[500];
    const char* str;
    strcpy(buf, "\t");
    str = fun(buf + strlen(buf) + 1, buf, atoi(argv[1]), atoi(argv[2]));
    printf("%s\n", str);
    return 0;
}

$ gcc --version
gcc (GCC) 11.2.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc -Wall -O6 test.c
test.c: In function ‘main’:
test.c:8:21: warning: ‘a = ’ directive writing 4 bytes into a region of size
between 0 and 499 [-Wformat-overflow=]
    8 |     sprintf(buf, "%sa = %d\n"
      |                     ^~~~
test.c:8:5: note: ‘sprintf’ output between 13 and 1031 bytes into a destination
of size 499
    8 |     sprintf(buf, "%sa = %d\n"
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
    9 |                  "%sb = %d\n",
      |                  ~~~~~~~~~~~~~
   10 |                  pfx, a, pfx, b);
      |                  ~~~~~~~~~~~~~~~
test.c:8:5: warning: ‘sprintf’ arguments 3, 5 may overlap destination object
‘buf’ [-Wrestrict]
test.c:17:10: note: destination object referenced by ‘restrict’-qualified
argument 1 was declared here
   17 |     char buf[500];
      |          ^~~

It's clear that the destination buffer will NOT overlap with anything related
to "pfx" in the fun() function.  Is also clear that output will NOT contain
that many characters that the warning claims (up to 1031).  If GCC can't
estimate the length for sure, it's better NOT to emit any warnings, rather than
printing this annoying noise.

Please be mindful of your users -- and their time to re-analyze the code that
suddenly is now flagged with these senseless warnings, only to realize that
it's all red herring.

Thank you

[Bug tree-optimization/103835] Bogus sprintf warnings

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-12-26
           Keywords|                            |diagnostic,
                   |                            |missed-optimization
     Ever confirmed|0                           |1
          Component|c                           |tree-optimization
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks like atoi is not know and GCC thinks atoi will write into buf.
But that does not fix the restrict warning which does not use string length
analysis to figure out it does not overlap.

Confirmed.

[Bug tree-optimization/103835] Bogus sprintf warnings

[lavr at ncbi dot nlm.nih.gov]

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

--- Comment #2 from lavr at ncbi dot nlm.nih.gov ---
Irrespective of whether atoi() is known, printing an "int" (or two) will never
produce this many characters...  This, however, also seems to have triggered
some weird logic that took the entire size of buf[] as a possible size of "pfx"
on output; hence, the warning noted 1000+ characters to be printed.

[Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Bogus sprintf warnings      |bogus sprintf warnings due
                   |                            |to missing strlen
                   |                            |propagation
             Blocks|                            |83819

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The warnings in comment #0 are caused by a limitation of the strlen/sprintf
passes: their less than perfect integration with the range (or constant)
propagation machinery.  The issue can be seen in the simplified test case
below.  In f() the constant strlen() result isn't propagated into the addition
in d_5 = &a + n_4; at the time the warning runs (within the strlen pass), and
so it triggers as expected.  In g() it is propagated and the warning doesn't
trigger.

To avoid the warnings the strlen pass needs to set the range of strlen results
and propagate them through the IL so they can be determined in computations in
subsequent statements (like sprintf).

$ cat pr103835.c && gcc -O2 -S -Wall -fdump-tree-strlen=/dev/stdout pr103835.c
char a[8];

int f (void)
{
  __builtin_strcpy (a, "f");

  __SIZE_TYPE__ n = __builtin_strlen (a) + 1;

  char *d = a + n;

 __builtin_sprintf (d, "%s", a);

  return n;
}

int g (void)
{
  __builtin_strcpy (a, "g");

  __SIZE_TYPE__ n = __builtin_strlen ("g") + 1;

  char *d = a + n;

 __builtin_sprintf (d, "%s", a);

  return n;
}


;; Function f (f, funcdef_no=0, decl_uid=1980, 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 }
pr103835.c:11: __builtin_sprintf: objsize = 6, fmtstr = "%s"
  Directive 1 at offset 0: "%s"
    Result: 1, 1, 1, 1 (1, 1, 1, 1)
  Directive 2 at offset 2: "", length = 1
pr103835.c: In function ‘f’:
pr103835.c:11:2: warning: ‘__builtin_sprintf’ argument 3 may overlap
destination object ‘a’ [-Wrestrict]
   11 |  __builtin_sprintf (d, "%s", a);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pr103835.c:1:6: note: destination object referenced by ‘restrict’-qualified
argument 1 was declared here
    1 | char a[8];
      |      ^

int f ()
{
  char * d;
  long unsigned int n;
  long unsigned int _1;
  int _7;

  <bb 2> [local count: 1073741824]:
  __builtin_memcpy (&a, "f", 2);
  _1 = 1;
  n_4 = _1 + 1;
  d_5 = &a + n_4;
  __builtin_strcpy (d_5, &a);
  _7 = (int) n_4;
  return _7;

}



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

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }
pr103835.c:24: __builtin_sprintf: objsize = 6, fmtstr = "%s"
  Directive 1 at offset 0: "%s"
    Result: 1, 1, 1, 1 (1, 1, 1, 1)
  Directive 2 at offset 2: "", length = 1
  Substituting 1 for return value.

int g ()
{
  <bb 2> [local count: 1073741824]:
  __builtin_memcpy (&a, "g", 2);
  __builtin_strcpy (&MEM <char[8]> [(void *)&a + 2B], &a);
  return 2;

}


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83819
[Bug 83819] [meta-bug] missing strlen optimizations

[Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation

[lavr at ncbi dot nlm.nih.gov]

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

--- Comment #4 from lavr at ncbi dot nlm.nih.gov ---
Thank you for investigating this!

Like I said, it's better NOT to emit any warnings if some machinery in the
compiler is not perfect enough to recognize the danger correctly (as it used to
be the case in previous versions and so they were silent in this respect).

There is the second warning there, too, which seems not being addressed by the
discussion.  Not only the count (499) is not given correctly (but maybe it's
because of the constant propagation that was mentioned), there's the second
part (the note), which follows, and which looks extremely bad and unintelligent
w.r.t. the output size calculation:

test.c:8:21: warning: ‘a = ’ directive writing 4 bytes into a region of size
between 0 and 499 [-Wformat-overflow=]
    8 |     sprintf(buf, "%sa = %d\n"
      |                     ^~~~
test.c:8:5: note: ‘sprintf’ output between 13 and 1031 bytes into a destination
of size 499

Is this the same cause, too?

[Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation

[msebor at gcc dot gnu.org]

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

--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
It's the same cause.  When the -Wformat-overflow and -truncation warnings are
unable to compute the exact length of a string argument to a directive like %s
they use the size of the array the string is stored in as a worst case estimate
(if they can determine the size of the array).  In other words, they assume the
string is as long as fits in the array (minus 1 for the terminating nul).  See
the example below.  In simple cases specifying a precision (e.g., "%.4s") to
constrain the amount of output can be used to avoid the potential overflow and
suppress the warning.  This is difficult to do with multiple string directives;
with those, using snprintf is the preferred alternative.  To avoid the
-Wformat-truncation warning for the equivalent snprintf calls it's necessary to
check the function's return value.

The sprintf pass keeps a running total of the ranges of the bytes formatted by
each directive, and the warning tries to strike a balance between providing too
little and too much detail to understand why it triggers.  This is tricky as
the range of total bytes on output includes the sum of more and more
directives.  We discussed various approaches to improve this some time ago (see
pr77696) but didn't converge on one that made everyone happy.

$ cat a.c && gcc -O2 -S -Wall a.c
char a[8], b[8];

void f (void)
{
  __builtin_sprintf (a, "%s%i", b, 123);
}
a.c: In function ‘f’:
a.c:5:28: warning: ‘%i’ directive writing 3 bytes into a region of size between
1 and 8 [-Wformat-overflow=]
    5 |   __builtin_sprintf (a, "%s%i", b, 123);
      |                            ^~
a.c:5:3: note: ‘__builtin_sprintf’ output between 4 and 11 bytes into a
destination of size 8
    5 |   __builtin_sprintf (a, "%s%i", b, 123);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The decision when to warn and when not is difficult.  All these warnings depend
on various optimizations, and those are never perfect.  The warnings make a few
basic assumptions (e.g., unreachable code is eliminated) and sometimes apply
heuristics like the string length above.  Both of these can lead to false
positives in addition to false negatives.