[Bug middle-end/97858] New: [11 regression] Bogus warnings about va_list during profiledbootstrap

[Bug middle-end/97858] New: [11 regression] Bogus warnings about va_list during profiledbootstrap

[hubicka at gcc dot gnu.org]

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

            Bug ID: 97858
           Summary: [11 regression] Bogus warnings about va_list during
                    profiledbootstrap
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hubicka at gcc dot gnu.org
  Target Milestone: ---

During profiledbootstrap we get the following warnings:
../libcpp/../../libcpp/mkdeps.c: In function ‘munge.constprop’:
../libcpp/../../libcpp/mkdeps.c:176:13: warning: ‘MEM[(struct 
*)&args].reg_save_area’ may be used uninitialized [-Wmaybe-uninitialized]
  176 |         str = va_arg (args, const char *);
      |             ^
../libcpp/../../libcpp/mkdeps.c:120:11: note: ‘MEM[(struct 
*)&args].reg_save_area’ was declared here
  120 |   va_list args;
      |           ^
../libcpp/../../libcpp/mkdeps.c:176:13: warning: ‘MEM[(struct 
*)&args].overflow_arg_area’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  176 |         str = va_arg (args, const char *);
      |             ^
../libcpp/../../libcpp/mkdeps.c:120:11: note: ‘MEM[(struct 
*)&args].overflow_arg_area’ was declared here
  120 |   va_list args;
      |           ^
../libcpp/../../libcpp/mkdeps.c:176:13: warning: ‘MEM[(struct 
*)&args].gp_offset’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  176 |         str = va_arg (args, const char *);
      |             ^
../libcpp/../../libcpp/mkdeps.c:120:11: note: ‘MEM[(struct  *)&args].gp_offset’
was declared here
  120 |   va_list args;
      |           ^

This seems to be due to conditional initialization of va_list:
static const char *                                                             
munge (const char *str, const char *trail = NULL, ...)                          
{                                                                               
  static unsigned alloc;                                                        
  static char *buf;                                                             
  unsigned dst = 0;                                                             
  va_list args;                                                                 
  if (trail)                                                                    
    va_start (args, trail);                                                     

but it does not make much sense to me to warn about internals of va_arg
iplementation at first place.  It is not user visible.

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |bootstrap

--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
With profiledbootstrap and no-lto this turns into error
../../libcpp/mkdeps.c: In function ‘const char* munge.constprop(const char*,
const char*, ...)’:
../../libcpp/mkdeps.c:176:13: error: ‘args.__va_list_tag::reg_save_area’ may be
used uninitialized [-Werror=maybe-uninitialized]
  176 |         str = va_arg (args, const char *);
      |             ^
../../libcpp/mkdeps.c:176:13: error: ‘args.__va_list_tag::overflow_arg_area’
may be used uninitialized in this function [-Werror=maybe-uninitialized]
../../libcpp/mkdeps.c:176:13: error: ‘args.__va_list_tag::gp_offset’ may be
used uninitialized in this function [-Werror=maybe-uninitialized]
cc1plus: all warnings being treated as errors


I do not see an easy workaround since starting va_list unconditionally would be
more expensive then current code.

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |nathan at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Why would be the mkdeps.c ever performance critical?
But also, the function is only called with 1 (+ one defaulted to NULL) or 3
(last one NULL) arguments, so can't it be just changed to
static const char *
munge (const char *str, const char *tail = NULL)
and not use any varargs?  I don't see any changes to the calls to munge in the
modules patchset.
Or are there any short term plans to use it with more arguments?

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |11.0
   Target Milestone|---                         |11.0
           Keywords|                            |diagnostic

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
The va_args gimple expansion could be marked no-warning but of course any
uninit use can be real there and we should simply be "better" (somehow) in
diagnosing the root cause...

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[nathan at gcc dot gnu.org]

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

Nathan Sidwell <nathan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |nathan at gcc dot gnu.org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-11-17
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #4 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
I'll remove the va_arg use along the lines Jakub suggests.  It's possible the
original plan had more than one optional arg (yeah, that's the story I'll go
with)

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Nathan Sidwell <nathan@gcc.gnu.org>:

https://gcc.gnu.org/g:d7ab349c44f30bed90b03b45865f6c7c5de1dfd8

commit r11-5095-gd7ab349c44f30bed90b03b45865f6c7c5de1dfd8
Author: Nathan Sidwell <nathan@acm.org>
Date:   Tue Nov 17 06:45:18 2020 -0800

    preprocessor: Fix profiled bootstrap warning [pr97858]

    As Jakub points out, we only ever pass a single variadic parm (if at
    all), so just an optional arg is fine.

            PR preprocessor/97858
            libcpp/
            * mkdeps.c (munge): Drop varadic args, we only ever use one.

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[nathan at gcc dot gnu.org]

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

Nathan Sidwell <nathan at gcc dot gnu.org> changed:

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

--- Comment #6 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
Fixed d7ab349c44f

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[hubicka at ucw dot cz]

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

--- Comment #7 from Jan Hubicka <hubicka at ucw dot cz> ---
> Fixed d7ab349c44f
Thanks, my original intention was to mostly track the fact that we do not want
to warn about fields of va_list type that is internal to compiler though
:)

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, so one enhancement request would be for va_list fields don't warn about
the internal fields, but warn about use of uninitialized va_list itself.
And the second thing is if the
if (tail)
  va_start (ap, tail);
for (bool first = true; str; first = false)
{
  use (str);
  if (first)
    str = tail;
  else
    str = va_arg (ap, char *);
}
if (tail)
  va_end (ap);
is something uninit infrastructure should understand as never doing va_arg if
tail (the condition guarding va_start) is NULL.

[Bug bootstrap/97858] [11 regression] Bogus warnings about va_list during profiledbootstrap

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That said, can't reproduce with simplified:
void
foo (char *str, char *tail, ...)
{
  __builtin_va_list ap;
  if (tail)
    __builtin_va_start (ap, tail);
  for (int first = 1; str; first = 0)
    {
      *str = ' ';
      if (first)
        str = tail;
      else
        str = __builtin_va_arg (ap, char *);
    }
  if (tail)
    __builtin_va_end (ap);
}

[Bug middle-end/97858] [11 regression] Bogus warnings about va_list

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
           Severity|normal                      |enhancement
          Component|bootstrap                   |middle-end
            Summary|[11 regression] Bogus       |[11 regression] Bogus
                   |warnings about va_list      |warnings about va_list
                   |during profiledbootstrap    |
         Resolution|FIXED                       |---

--- Comment #10 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I have updated the PR to track the enhancements to -Wmaybe-uninitalized.

[Bug middle-end/97858] [11 regression] Bogus warnings about va_list

[hubicka at gcc dot gnu.org]

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

--- Comment #11 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Jakub,
with code patterns

if (foo)
  ininit var
...
if (foo)
  use var

the false positive really depends on how far we do jump threading and similar
transforms.

[Bug middle-end/97858] Bogus warnings about va_list

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11 regression] Bogus       |Bogus warnings about
                   |warnings about va_list      |va_list
   Target Milestone|11.0                        |---

[Bug middle-end/97858] avoid mentioning va_list internal fields in -Wuninitialized

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2020-11-17 00:00:00         |2021-4-9
            Summary|Bogus warnings about        |avoid mentioning va_list
                   |va_list                     |internal fields in
                   |                            |-Wuninitialized
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #12 from Martin Sebor <msebor at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #7)
> Thanks, my original intention was to mostly track the fact that we do not
> want
> to warn about fields of va_list type that is internal to compiler though
> :)

To be clear: for the following test case you suggest to issue just the one
warning below:

$ cat pr97858.c && gcc -O2  -S -Wall pr97858.c
void f (__builtin_va_list);

int g (int i, ...)
{
  __builtin_va_list ap;
  // __builtin_va_start (ap, i);

  int x = __builtin_va_arg (ap, int);
  __builtin_va_end (ap);

  return x;
}
pr97858.c: In function ‘g’:
pr97858.c:8:7: warning: ‘ap’ is used uninitialized [-Wuninitialized]
    8 |   int x = __builtin_va_arg (ap, int);
      |       ^
pr97858.c:5:21: note: ‘ap’ declared here
    5 |   __builtin_va_list ap;
      |                     ^~

as opposed to the three GCC issues now:

pr97858.c: In function ‘g’:
pr97858.c:8:7: warning: ‘ap[0].gp_offset’ is used uninitialized
[-Wuninitialized]
    8 |   int x = __builtin_va_arg (ap, int);
      |       ^
pr97858.c:5:21: note: ‘ap’ declared here
    5 |   __builtin_va_list ap;
      |                     ^~
pr97858.c:8:7: warning: ‘ap[0].reg_save_area’ may be used uninitialized
[-Wmaybe-uninitialized]
    8 |   int x = __builtin_va_arg (ap, int);
      |       ^
pr97858.c:5:21: note: ‘ap’ declared here
    5 |   __builtin_va_list ap;
      |                     ^~
pr97858.c:8:7: warning: ‘ap[0].overflow_arg_area’ may be used uninitialized
[-Wmaybe-uninitialized]
    8 |   int x = __builtin_va_arg (ap, int);
      |       ^
pr97858.c:5:21: note: ‘ap’ declared here
    5 |   __builtin_va_list ap;
      |                     ^~