[Bug ipa/97264] New: [11 Regression] -fpa-modref breaks va_arg on glibc

[Bug ipa/97264] New: [11 Regression] -fpa-modref breaks va_arg on glibc

[slyfox at gcc dot gnu.org]

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

            Bug ID: 97264
           Summary: [11 Regression] -fpa-modref breaks va_arg on glibc
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
                CC: hubicka at gcc dot gnu.org, marxin at gcc dot gnu.org
  Target Milestone: ---

Initially spotted the problem on glibc form master where fscanf() breaks in
complex ways.

I tried to minimize fscanf() down to a single file. Looks like -fno-ipa-modref
changes crashing to non-crashing program.

Full example:

// extracted from glibc's __vfscanf_internal()

void __isoc99_fscanf__ (const char *format, ...) __attribute__((noipa));
void __vfscanf_internal__ (const char *format, __builtin_va_list argptr) 
__attribute__((noipa));

#define isd(c) ({ int __c = (c); __c >= '0' && __c <= '9'; })

static int
read_int (const unsigned char **pstr)
{
  // read first digit '1'
  int retval = **pstr - '0';

  // skip digits: executes 0 times and advances pointer once
  while (isd (*++(*pstr)))
      ;

  return retval;
}

void __vfscanf_internal__ (const char *f, __builtin_va_list argptr)
{
  /* assume input: "1<>\0" */
  while (*f != '\0')
    {
      const unsigned char ** pstr = (const unsigned char **) &f;
      // skip digit '1'
      read_int (pstr);

      /* Find the conversion specifier.  */
      f++; // skip '<'

      char * str = __builtin_va_arg(argptr, char *);

      f++; // skip ">"

      *str++ = '?';
    }
}

void __isoc99_fscanf__ (const char *format, ...)
{
  __builtin_va_list arg;

  __builtin_va_start (arg, format);
  __vfscanf_internal__ (format, arg);
  __builtin_va_end (arg);
}

int main (void)
{
    char key[10];
    __isoc99_fscanf__ ("1<>", key);
    return 0;
}

How to crash:

$ gcc-11.0.0 -O2 uname.c -o uname11 && ./uname11
Segmentation fault (core dumped)
$ gcc-11.0.0 -O2 uname.c -o uname11 -fno-ipa-modref && ./uname11
<ok>

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[slyfox at gcc dot gnu.org]

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

--- Comment #1 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Created attachment 49295
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49295&action=edit
uname.c

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
   Target Milestone|---                         |11.0

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
a-t5.c.036t.fre1:ipa-modref: in __vfscanf_internal__/1, call to read_int/0 does
not clobber f 2->2
a-t5.c.036t.fre1:ipa-modref: in __vfscanf_internal__/1, call to read_int/0 does
not clobber f 2->2

I think this is 'const char *' vs. 'const unsigned char *' being TBAA
incompatible.  Changing read_int to take 'const char *pstr' fixes
things (and changing the type in __vfscanf_internal__ as well).

So this seems to be a latent bug in glibc unless somehow C makes those
pointers magically compatible wrt TBAA.  Joseph?

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[slyfox at gcc dot gnu.org]

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

--- Comment #3 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Oh, that makes sense.

> void __vfscanf_internal__ (const char *f, __builtin_va_list argptr)
> {
>   /* assume input: "1<>\0" */
>   while (*f != '\0')
>     {
>       const unsigned char ** pstr = (const unsigned char **) &f;

comes from
https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/vfscanf-internal.c;h=95b46dcbeb55b1724b396f02a940f3047259b926;hb=HEAD#l489
:

"""
 274 int
 275 __vfscanf_internal (FILE *s, const char *format, va_list argptr,
 276                     unsigned int mode_flags)
...
 487       if (ISDIGIT ((UCHAR_T) *f))
 488         {
 489           argpos = read_int ((const UCHAR_T **) &f);
"""

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[rguenther at suse dot de]

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

--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 1 Oct 2020, slyfox at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97264
> 
> --- Comment #3 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
> Oh, that makes sense.
> 
> > void __vfscanf_internal__ (const char *f, __builtin_va_list argptr)
> > {
> >   /* assume input: "1<>\0" */
> >   while (*f != '\0')
> >     {
> >       const unsigned char ** pstr = (const unsigned char **) &f;
> 
> comes from
> https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/vfscanf-internal.c;h=95b46dcbeb55b1724b396f02a940f3047259b926;hb=HEAD#l489
> :
> 
> """
>  274 int
>  275 __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>  276                     unsigned int mode_flags)
> ...
>  487       if (ISDIGIT ((UCHAR_T) *f))
>  488         {
>  489           argpos = read_int ((const UCHAR_T **) &f);
> """

>From this little context it eventually makes sense to declare
'f' as const unsigned char * in this function.

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[schwab@linux-m68k.org]

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

--- Comment #5 from Andreas Schwab <schwab@linux-m68k.org> ---
Why doesn't gcc warn about that?

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andreas Schwab from comment #5)
> Why doesn't gcc warn about that?

It does:

unsigned char **q;
void foo (char *p)
{
  q = (unsigned char **)&p;
}

> gcc t.c -fstrict-aliasing -Wstrict-aliasing=2 -S
t.i: In function 'foo':
t.i:4:3: warning: dereferencing type-punned pointer will break strict-aliasing
rules [-Wstrict-aliasing]
   q = (unsigned char **)&p;
   ^

note the default level of -Wstrict-aliasing when enabled is 3 which will
not warn when the pointer is not dereferenced in the same expression.

[Bug ipa/97264] [11 Regression] -fpa-modref breaks va_arg on glibc

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Invalid.