[Bug tree-optimization/102238] New: alias_offset in gimple-ssa-sprintf.c is broken

[Bug tree-optimization/102238] New: alias_offset in gimple-ssa-sprintf.c is broken

[pinskia at gcc dot gnu.org]

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

            Bug ID: 102238
           Summary: alias_offset in gimple-ssa-sprintf.c is broken
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---

Take:
/* 
   { dg-do compile }
   { dg-options "-O2 -Wno-format-overflow -Wrestrict -ftrack-macro-expansion=0"
}
*/
void sink (int);
#define S10 "0123456789"
extern char a2[2][22];
#define T(d, ...) do {                                  \
    char a[22] = S10;                                   \
    sink (__builtin_sprintf ((d), __VA_ARGS__));        \
  } while (0)
struct S {
  char a[4];
  char b[4];
};
struct S2 {
  struct S s_1;
  struct S s_2;
  struct S sa3[3];
};
struct S3 {
  struct S2 s2_1;
  struct S2 s2_2;

  struct {
    struct {
      struct {
        struct S sa_3[3];
      } a_1[3];
    } a_2[3][3];
  } a_3[3][3][3];

  char fa[];
};
void test_struct_member_array (struct S3 *s3, int i)
{
  char *a = (char*)s3;
  char *d;
  const char *s;
  d = s3->s2_2.s_1.a;
  s = s3->s2_2.s_1.a;
  T (d, "%s", a + 40);   /* { dg-warning "overlaps" } */
  T (d, "%s", a + 41);   /* { dg-warning "may overlap" } */
  T (d, "%s", a + 42);   /* { dg-warning "may overlap" } */
}

----- CUT -----

This should warn but does not.
The reason is alias_offset does not do a good job at all figuring out field
offsets.

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |102216

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I ran into this while fixing PR 102216.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102216
[Bug 102216] [12 Regression] missed optimization causing Warray-bounds

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[msebor at gcc dot gnu.org]

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

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

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

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
The source and the destination arrays in the test case do not overlap so there
is no warning.  The preprocessed output makes it easier to see than the
original (the destination of the call is the a member of the object pointed to
by the function argument s3 and the %s argument the local array 'a'):

gcc -E pr102238.c
# 0 "pr102238.c"
# 0 "<built-in>"
# 0 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 0 "<command-line>" 2
# 1 "pr102238.c"
void sink (int);

extern char a2[2][22];




struct S {
  char a[4];
  char b[4];
};
struct S2 {
  struct S s_1;
  struct S s_2;
  struct S sa3[3];
};
struct S3 {
  struct S2 s2_1;
  struct S2 s2_2;

  struct {
    struct {
      struct {
        struct S sa_3[3];
      } a_1[3];
    } a_2[3][3];
  } a_3[3][3][3];

  char fa[];
};
void test_struct_member_array (struct S3 *s3, int i)
{
  char *a = (char*)s3;
  char *d = s3->s2_2.s_1.a;
  const char *s = s3->s2_2.s_1.a;

  do { char a[22] = "0123456789"; sink (__builtin_sprintf ((d), "%s", a + 40));
} while (0);
  do { char a[22] = "0123456789"; sink (__builtin_sprintf ((d), "%s", a + 41));
} while (0);
  do { char a[22] = "0123456789"; sink (__builtin_sprintf ((d), "%s", a + 42));
} while (0);
}

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #2)
> The source and the destination arrays in the test case do not overlap so
> there is no warning.  The preprocessed output makes it easier to see than
> the original (the destination of the call is the a member of the object
> pointed to by the function argument s3 and the %s argument the local array
> 'a'):

Silly mistake on my part, it is still buggy:

/* 
   { dg-do compile }
   { dg-options "-O2 -Wno-format-overflow -Wrestrict -ftrack-macro-expansion=0"
}
*/
void sink (int);
#define S10 "0123456789"
extern char a2[2][22];
#define T(d, ...) do {                                  \
    char a[tttttttt22] = S10;                           \
    sink (__builtin_sprintf ((d), __VA_ARGS__));        \
  } while (0)
struct S {
  char a[4];
  char b[4];
};
struct S2 {
  struct S s_1;
  struct S s_2;
  struct S sa3[3];
};
struct S3 {
  struct S2 s2_1;
  struct S2 s2_2;

  struct {
    struct {
      struct {
        struct S sa_3[3];
      } a_1[3];
    } a_2[3][3];
  } a_3[3][3][3];

  char fa[];
};
void test_struct_member_array (struct S3 *s3, int i)
{
  char *a1 = (char*)s3;
  char *d;
  const char *s;
  d = s3->s2_2.s_1.a;
  s = s3->s2_2.s_1.a;
  T (d, "%s", a1 + 40);   /* { dg-warning "overlaps" } */
  T (d, "%s", a1 + 41);   /* { dg-warning "may overlap" } */
  T (d, "%s", a1 + 42);   /* { dg-warning "may overlap" } */
}

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> (In reply to Martin Sebor from comment #2)
> > The source and the destination arrays in the test case do not overlap so
> > there is no warning.  The preprocessed output makes it easier to see than
> > the original (the destination of the call is the a member of the object
> > pointed to by the function argument s3 and the %s argument the local array
> > 'a'):
> 
> Silly mistake on my part, it is still buggy:

You can add 
  _Static_assert (offsetof(struct S3, s2_2.s_1.a) == 40);

To see that 40 is the offiset of s2_2.s_1.a.

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|102216                      |84774
   Last reconfirmed|                            |2021-09-16
             Status|UNCONFIRMED                 |NEW
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=102216
     Ever confirmed|0                           |1

--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
Confirmed with the simplified test case below.  DOM3 is the last pass to run
before strlen.  The strlen output shows the sprintf call is being folded to
strcpy.  I'm not sure what the relationship of this bug is to pr102216 but a
missing -Wrestrict cannot be a cause of a missed optimization.  I've moved
pr102216 to See Also.

$ cat pr102238.c && gcc -O2 -S -Wall -fdump-tree-dom3=/dev/stdout pr102238.c
struct A
{
  char a[4];
};

struct B
{
  struct A a1, a2;
};

void f (struct B *p)
{
  enum { off = __builtin_offsetof (struct B, a2.a) };

  char *s = (char*)p + off;
  char *d = p->a2.a;

  __builtin_sprintf (d, "%s", s);   // missing -Wrestrict
}

;; Function f (f, funcdef_no=0, decl_uid=1983, 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 }
void f (struct B * p)
{
  char * d;
  char * s;

  <bb 2> [local count: 1073741824]:
  s_2 = p_1(D) + 4;
  d_3 = &p_1(D)->a2.a;
  __builtin_sprintf (d_3, "%s", s_2);
  return;

}


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84774
[Bug 84774] [meta-bug] bogus/missing -Wrestrict
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102216
[Bug 102216] [12 Regression] missed optimization causing Warray-bounds

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |msebor at gcc dot gnu.org

--- Comment #6 from Martin Sebor <msebor at gcc dot gnu.org> ---
Let me handle this.

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |102216

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #5)
> Confirmed with the simplified test case below.  DOM3 is the last pass to run
> before strlen.  The strlen output shows the sprintf call is being folded to
> strcpy.  I'm not sure what the relationship of this bug is to pr102216 but a
> missing -Wrestrict cannot be a cause of a missed optimization.  I've moved
> pr102216 to See Also.

This issue is blocking the patch which I have created for PR 102216 and that is
how I found this issue so yes it is still blocking PR 102216.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102216
[Bug 102216] [12 Regression] missed optimization causing Warray-bounds

[Bug tree-optimization/102238] alias_offset in gimple-ssa-sprintf.c is broken

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|102216                      |

--- Comment #8 from Martin Sebor <msebor at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> This issue is blocking the patch which I have created for PR 102216 and that
> is how I found this issue so yes it is still blocking PR 102216.

I don't see what this problem has to do with pr102216 or how it's blocking your
progress on it.  pr102216 is a GCC 12 regression that apparently results in a
-Warray-bounds false positive.  The problem here is a false negative due to a
limitation that was introduced in GCC 10 (in r278098) with the initial
implementation of -Wrestrict for sprintf.  The two have nothing in common that
I can see.

Is your fix causing an existing test to fail because it changes the IL in a way
that prevents the warning from triggering?  (That shouldn't make it a blocker
as the test could be xfailed until this is fixed, but if you need me to do
something you need to explain what and why.)


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102216
[Bug 102216] [12 Regression] missed optimization causing Warray-bounds

[Bug tree-optimization/102238] missing -Wrestrict for sprintf into the same member array as argument plus offset

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
   Target Milestone|---                         |12.0
            Summary|alias_offset in             |missing -Wrestrict for
                   |gimple-ssa-sprintf.c is     |sprintf into the same
                   |broken                      |member array as argument
                   |                            |plus offset
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=102919

--- Comment #9 from Martin Sebor <msebor at gcc dot gnu.org> ---
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-October/582446.html

[Bug tree-optimization/102238] missing -Wrestrict for sprintf into the same member array as argument plus offset

[cvs-commit at gcc dot gnu.org]

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

--- 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:1ff4dbddcf74203a1e16316b18e12f9e1b5085f0

commit r12-4727-g1ff4dbddcf74203a1e16316b18e12f9e1b5085f0
Author: Martin Sebor <msebor@redhat.com>
Date:   Tue Oct 26 14:40:33 2021 -0600

    Improve/correct detection of overlapping aggregates [PR102238, PR102919].

    Resolves:
    PR tree-optimization/102238 - alias_offset in gimple-ssa-sprintf.c is
broken
    PR tree-optimization/102919 - spurious -Wrestrict warning for sprintf into
the same member array as argument plus offset

    gcc/ChangeLog:

            PR tree-optimization/102238
            PR tree-optimization/102919
            * gimple-ssa-sprintf.c (get_string_length): Add an argument.
            (array_elt_at_offset): Move to pointer-query.
            (set_aggregate_size_and_offset): New function.
            (field_at_offset):  Move to pointer-query.
            (get_origin_and_offset): Rename...
            (get_origin_and_offset_r): this.  Add an argument.  Make aggregate
            handling more robust.
            (get_origin_and_offset): New.
            (alias_offset): Add an argument.
            (format_string): Use subobject size determined by
get_origin_and_offset.
            * pointer-query.cc (field_at_offset): Move from
gimple-ssa-sprintf.c.
            Improve/correct handling of aggregates.
            (array_elt_at_offset): Same.
            * pointer-query.h (field_at_offset): Declare.
            (array_elt_at_offset): Declare.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/102238
            PR tree-optimization/102919
            * gcc.dg/tree-ssa/builtin-sprintf-warn-23.c: Remove warnings.
            * gcc.dg/Wrestrict-23.c: New test.

[Bug tree-optimization/102238] missing -Wrestrict for sprintf into the same member array as argument plus offset

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.