public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
@ 2021-01-08  8:20 marxin at gcc dot gnu.org
  2021-01-08  8:20 ` [Bug tree-optimization/98597] " marxin at gcc dot gnu.org
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-01-08  8:20 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 98597
           Summary: [11 Regression] ICE in print_mem_ref since
                    r11-6508-gabb1b6058c09a7c0
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: msebor at gcc dot gnu.org
  Target Milestone: ---

One more ICE I found:

$ cat x.C
struct shared_count {
  shared_count() {}
  shared_count(shared_count &r) : pi(r.pi) {}
  int pi;
};
struct shared_ptr {
  int ptr;
  shared_count refcount;
};
struct Bar {
  Bar(int, shared_ptr);
};
void g() {
  shared_ptr foo;
  Bar(0, foo);
}

$ g++ x.C -O2 -Wuninitialized -c
In copy constructor ‘shared_ptr::shared_ptr(shared_ptr&)’,
    inlined from ‘void g()’ at x.C:15:13:
x.C:6:8: warning: ‘foo.shared_ptr::ptr’ is used uninitialized [-Wuninitialized]
    6 | struct shared_ptr {
      |        ^~~~~~~~~~
x.C: In function ‘void g()’:
x.C:14:14: note: ‘foo’ declared here
   14 |   shared_ptr foo;
      |              ^~~
‘
during GIMPLE pass: uninit
Segmentation fault
   13 | void g() {
      |      ^
0x10bf64f crash_signal
        /home/marxin/Programming/gcc/gcc/toplev.c:327
0xbc57bc print_mem_ref
        /home/marxin/Programming/gcc/gcc/c-family/c-pretty-print.c:1851
0x9c4b2f dump_expr
        /home/marxin/Programming/gcc/gcc/cp/error.c:2366
0x9c97d0 expr_to_string(tree_node*)
        /home/marxin/Programming/gcc/gcc/cp/error.c:3187
0x9c9f0c cp_printer
        /home/marxin/Programming/gcc/gcc/cp/error.c:4355
0x1c30a8c pp_format(pretty_printer*, text_info*)
        /home/marxin/Programming/gcc/gcc/pretty-print.c:1475
0x1c148e6 diagnostic_report_diagnostic(diagnostic_context*, diagnostic_info*)
        /home/marxin/Programming/gcc/gcc/diagnostic.c:1216
0x1c16e48 diagnostic_impl
        /home/marxin/Programming/gcc/gcc/diagnostic.c:1366
0x1c16e48 warning_at(unsigned int, int, char const*, ...)
        /home/marxin/Programming/gcc/gcc/diagnostic.c:1503
0x12d0e16 maybe_warn_operand
        /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:418
0x12d423f warn_uninitialized_vars
        /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:657
0x12d83f2 execute
        /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:3019
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
@ 2021-01-08  8:20 ` marxin at gcc dot gnu.org
  2021-01-11 23:29 ` slyfox at gcc dot gnu.org
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-01-08  8:20 UTC (permalink / raw)
  To: gcc-bugs

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.2.0
   Last reconfirmed|                            |2021-01-08
      Known to fail|                            |11.0
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=98578
     Ever confirmed|0                           |1
           Priority|P3                          |P1
   Target Milestone|---                         |11.0
             Status|UNCONFIRMED                 |NEW

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
  2021-01-08  8:20 ` [Bug tree-optimization/98597] " marxin at gcc dot gnu.org
@ 2021-01-11 23:29 ` slyfox at gcc dot gnu.org
  2021-01-12  8:43 ` marxin at gcc dot gnu.org
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: slyfox at gcc dot gnu.org @ 2021-01-11 23:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
I also found a similar crash (from `RTL pass: expand` for some reason) in
print_mem_ref:

```c++
struct QQmlRefCount {
  void release() const;
  virtual ~QQmlRefCount();
};
QQmlRefCount::~QQmlRefCount() {}
void QQmlRefCount::release() const { delete this; }
struct QQmlJavaScriptExpression {
  virtual int sourceLocation();
};
struct QQmlBoundSignalExpression : QQmlJavaScriptExpression, QQmlRefCount {};
struct QQmlProfilerDefinitions {
  enum RangeType { HandlingSignal };
};
struct QQmlProfiler : QQmlProfilerDefinitions {
  struct RefLocation {
    RefLocation() {
      switch (locationType)
      case HandlingSignal:
        boundSignal->release();
    }
    RangeType locationType;
    QQmlBoundSignalExpression *boundSignal;
  };
  void startCompiling() { RefLocation(); }
};
struct QQmlCompilingProfiler {
  QQmlProfiler QQmlCompilingProfiler_profiler;
  QQmlCompilingProfiler(int *) {
    QQmlCompilingProfiler_profiler.startCompiling();
  }
};
int notifyComplete_blob;
void QQmlDataBlobnotifyComplete() {
  QQmlCompilingProfiler prof(&notifyComplete_blob);
}
```

```
$ x86_64-pc-linux-gnu-g++ -O2 -std=c++1z -c a.cpp.cpp

during RTL pass: expand
In function 'void QQmlDataBlobnotifyComplete()':
Segmentation fault
    5 | QQmlRefCount::~QQmlRefCount() {}
      |                                ^
0x120986f crash_signal
        ../../gcc/gcc/toplev.c:327
0x7fbb6ee06b6f ???
       
/usr/src/debug/sys-libs/glibc-2.32-r7/glibc-2.32/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
0x9122bb print_mem_ref
        ../../gcc/gcc/c-family/c-pretty-print.c:1851
0x912c07 c_pretty_printer::unary_expression(tree_node*)
        ../../gcc/gcc/c-family/c-pretty-print.c:2005
0x596063 dump_expr
        ../../gcc/gcc/cp/error.c:2421
...
```

I poked slightly at the crash in gdb:

print_mem_ref():
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/c-family/c-pretty-print.c#l1812

(gdb) call debug_generic_expr(e)
MEM[(struct QQmlBoundSignalExpression * *)&D.2397 + 8B]

(gdb) call debug_generic_expr(arg)
D.2397

1836   const bool addr = TREE_CODE (arg) == ADDR_EXPR;
1837   if (addr)
1838     {
1839       arg = TREE_OPERAND (arg, 0);
1840       if (byte_off == 0)
1841         {
1842           pp->expression (arg);
1843           return;
1844         }
1845     }

Does not match as an address and falls through (should it?).

1847   tree access_type = TREE_TYPE (e);
1848   if (TREE_CODE (access_type) == ARRAY_TYPE)
1849     access_type = TREE_TYPE (access_type);
1850   tree arg_type = TREE_TYPE (TREE_TYPE (arg));
1851   if (TREE_CODE (arg_type) == ARRAY_TYPE)
1852     arg_type = TREE_TYPE (arg_type);

(gdb) call debug_generic_expr(access_type)
struct QQmlBoundSignalExpression *

(gdb) call TREE_CODE (access_type)
$1 = POINTER_TYPE

(gdb) call debug_generic_expr(TREE_TYPE (arg))
struct RefLocation

(gdb) call TREE_TYPE (TREE_TYPE (arg))
$3 = (tree) 0x0

Null deref at '1851   if (TREE_CODE (arg_type) == ARRAY_TYPE)'.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
  2021-01-08  8:20 ` [Bug tree-optimization/98597] " marxin at gcc dot gnu.org
  2021-01-11 23:29 ` slyfox at gcc dot gnu.org
@ 2021-01-12  8:43 ` marxin at gcc dot gnu.org
  2021-01-12  8:43 ` marxin at gcc dot gnu.org
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-01-12  8:43 UTC (permalink / raw)
  To: gcc-bugs

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

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
> ```c++
> struct QQmlRefCount {
>   void release() const;
>   virtual ~QQmlRefCount();
> };
> QQmlRefCount::~QQmlRefCount() {}
> void QQmlRefCount::release() const { delete this; }
> struct QQmlJavaScriptExpression {
>   virtual int sourceLocation();
> };
> struct QQmlBoundSignalExpression : QQmlJavaScriptExpression, QQmlRefCount {};
> struct QQmlProfilerDefinitions {
>   enum RangeType { HandlingSignal };
> };
> struct QQmlProfiler : QQmlProfilerDefinitions {
>   struct RefLocation {
>     RefLocation() {
>       switch (locationType)
>       case HandlingSignal:
>         boundSignal->release();
>     }
>     RangeType locationType;
>     QQmlBoundSignalExpression *boundSignal;
>   };
>   void startCompiling() { RefLocation(); }
> };
> struct QQmlCompilingProfiler {
>   QQmlProfiler QQmlCompilingProfiler_profiler;
>   QQmlCompilingProfiler(int *) {
>     QQmlCompilingProfiler_profiler.startCompiling();
>   }
> };
> int notifyComplete_blob;
> void QQmlDataBlobnotifyComplete() {
>   QQmlCompilingProfiler prof(&notifyComplete_blob);
> }
> ```
> 

This also started with r11-6508-gabb1b6058c09a7c0.

@Martin: Can you please take a look?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2021-01-12  8:43 ` marxin at gcc dot gnu.org
@ 2021-01-12  8:43 ` marxin at gcc dot gnu.org
  2021-01-12 18:39 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-01-12  8:43 UTC (permalink / raw)
  To: gcc-bugs

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2021-01-12  8:43 ` marxin at gcc dot gnu.org
@ 2021-01-12 18:39 ` jakub at gcc dot gnu.org
  2021-01-12 19:02 ` jakub at gcc dot gnu.org
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-12 18:39 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The bug is clear:
  const bool addr = TREE_CODE (arg) == ADDR_EXPR;
  if (addr)
    {
      arg = TREE_OPERAND (arg, 0);
...
    }
makes arg sometimes pointer and sometimes what the pointer points to (which can
be another pointer or something completely different).
So obviously
  tree arg_type = TREE_TYPE (TREE_TYPE (arg));
can't work reliably.
Either the arg = TREE_OPERAND (arg, 0); should be done only if byte_off == 0,
or everything else that uses TREE_TYPE (TREE_TYPE (arg)) needs to be changed to
do something different based on whether addr is true or false.
I'd note that having the same variable represent different levels of
indirection in the same code depending on some circumstances is a maintainance
nightmare.
E.g. instead of having a bool whether arg was originally ADDR_EXPR or not, it
might be better to keep arg as it is and have another tree that would represent
what arg points to if it is ADDR_EXPR, otherwise NULL_TREE.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2021-01-12 18:39 ` jakub at gcc dot gnu.org
@ 2021-01-12 19:02 ` jakub at gcc dot gnu.org
  2021-01-12 20:03 ` cvs-commit at gcc dot gnu.org
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-12 19:02 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Also, it seems the shortcut for MEM_REF [&var, 0] is only correct if the types
of var and MEM_REF are compatible, if I have say
struct S { int a, int b; } var;
then if MEM[&var, 0] has int type, then it shouldn't be printed as var, but as
*(int *)&var or so.
The patch has been committed without any patch review from what I can see, and
it really shows up that it hasn't been reviewed.
I think best would be to revert it, then fix all issues it has and then submit
for patch review.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2021-01-12 19:02 ` jakub at gcc dot gnu.org
@ 2021-01-12 20:03 ` cvs-commit at gcc dot gnu.org
  2021-01-12 20:10 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-01-12 20:03 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 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:5a9cfad2de92f2d65585774acb524b3fa17621b5

commit r11-6621-g5a9cfad2de92f2d65585774acb524b3fa17621b5
Author: Martin Sebor <msebor@redhat.com>
Date:   Tue Jan 12 12:58:27 2021 -0700

    Avoid a couple more ICEs in print_mem_ref (PR c/98597).

    Resolves:
    PR c/98597 - ICE in -Wuninitialized printing a MEM_REF
    PR c/98592 - ICE in gimple_canonical_types_compatible_p while formatting

    gcc/c-family/ChangeLog:

            PR c/98597
            PR c/98592
            * c-pretty-print.c (print_mem_ref): Avoid assuming MEM_REF operand
            has pointer type.  Remove redundant code.  Avoid calling
            gimple_canonical_types_compatible_p.

    gcc/testsuite/ChangeLog:

            PR c/98597
            PR c/98592
            * g++.dg/warn/Wuninitialized-13.C: New test.
             gcc.dg/uninit-39.c: New test.

            #

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2021-01-12 20:03 ` cvs-commit at gcc dot gnu.org
@ 2021-01-12 20:10 ` jakub at gcc dot gnu.org
  2021-01-13 11:49 ` jakub at gcc dot gnu.org
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-12 20:10 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This is not a proper fix, you avoid the ICEs perhaps, but keep printing
completely bogus output for many cases.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2021-01-12 20:10 ` jakub at gcc dot gnu.org
@ 2021-01-13 11:49 ` jakub at gcc dot gnu.org
  2021-01-13 13:49 ` jakub at gcc dot gnu.org
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-13 11:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Example where the new code gets it all wrong
struct S { char a; int b; char c; unsigned d; };
struct T { char t; struct S u; int v; };
typedef short U[2][2];
void baz (U *);

static inline int
bar (char *p)
{
  return *(int *) p;
}

void
foo (int *q)
{
  struct T t;
  t.t = 1;
  t.u.c = 2;
  char *p = (char *) &t;
  q[0] = bar (p + __builtin_offsetof (struct T, u.b));
  q[1] = bar (p + __builtin_offsetof (struct T, u.d));
  q[2] = bar (p + __builtin_offsetof (struct T, v));
  int s[3];
  s[0] = 1;
  char *r = (char *) s;
  q[3] = bar (r + sizeof (int));
  U u[2], v[2];
  u[0][0][0] = 1;
  __builtin_memcpy (&v[1], &u[1], sizeof (U));
  baz (&v[1]);
}

It is true that old MEM_REF printing wasn't correct either:
test.c:19:8: warning: ‘*((void *)&t+8)’ is used uninitialized in this function
[-Wuninitialized]
test.c:20:8: warning: ‘*((void *)&t+16)’ is used uninitialized in this function
[-Wuninitialized]
test.c:21:8: warning: ‘*((void *)&t+20)’ is used uninitialized in this function
[-Wuninitialized]
test.c:25:8: warning: ‘*((void *)&s+4)’ is used uninitialized in this function
[-Wuninitialized]
test.c:28:3: warning: ‘*((void *)&u+8)’ is used uninitialized in this function
[-Wuninitialized]
- it would have been ugly but correct if it used char * instead of void * and
then added another cast to the MEM_REF type.
The new code prints:
test.c:19:8: warning: ‘((int*)t) + 2’ is used uninitialized [-Wuninitialized]
test.c:20:8: warning: ‘((int*)t) + 4’ is used uninitialized [-Wuninitialized]
test.c:21:8: warning: ‘((int*)t) + 5’ is used uninitialized [-Wuninitialized]
test.c:25:8: warning: ‘s + 1’ is used uninitialized [-Wuninitialized]
test.c:28:3: warning: ‘((long unsigned int*)u) + 1’ is used uninitialized
[-Wuninitialized]
Huh!  (int*)t is invalid C (in C++ perhaps there could be an operator for it),
and the result is a pointer anyway.  s + 1 is valid, but again s + 1 is not
uninitialized, it is s[1].  Similarly for u.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2021-01-13 11:49 ` jakub at gcc dot gnu.org
@ 2021-01-13 13:49 ` jakub at gcc dot gnu.org
  2021-01-13 14:05 ` jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-13 13:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, before starting to fix all the bugs in the print_mem_ref routine, I think
we should add something like:
--- gcc/c-family/c-pretty-print.c.jj    2021-01-13 08:02:09.425498954 +0100
+++ gcc/c-family/c-pretty-print.c       2021-01-13 14:31:34.048591447 +0100
@@ -1809,6 +1809,103 @@ pp_c_call_argument_list (c_pretty_printe
   pp_c_right_paren (pp);
 }

+/* Try to fold *(type *)&op into op.fld.fld2[1] if possible.
+   Only used for printing expressions.  Should punt if ambiguous
+   (e.g. in unions).  */
+
+static tree
+c_fold_indirect_ref_for_warn (location_t loc, tree type, tree op,
+                             offset_int off)
+{
+  tree optype = TREE_TYPE (op);
+  if (off == 0)
+    {
+      if (lang_hooks.types_compatible_p (optype, type))
+       return op;
+      /* Also handle conversion to an empty base class, which
+        is represented with a NOP_EXPR.  */
+      /* *(foo *)&complexfoo => __real__ complexfoo */
+      else if (TREE_CODE (optype) == COMPLEX_TYPE
+              && lang_hooks.types_compatible_p (type, TREE_TYPE (optype)))
+       return build1_loc (loc, REALPART_EXPR, type, op);
+    }
+  /* ((foo*)&complexfoo)[1] => __imag__ complexfoo */
+  else if (TREE_CODE (optype) == COMPLEX_TYPE
+          && lang_hooks.types_compatible_p (type, TREE_TYPE (optype))
+          && tree_to_uhwi (TYPE_SIZE_UNIT (type)) == off)
+    return build1_loc (loc, IMAGPART_EXPR, type, op);
+  /* ((foo *)&fooarray)[x] => fooarray[x] */
+  if (TREE_CODE (optype) == ARRAY_TYPE
+      && TYPE_SIZE_UNIT (TREE_TYPE (optype))
+      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (optype))) == INTEGER_CST
+      && !integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (optype))))
+    {
+      tree type_domain = TYPE_DOMAIN (optype);
+      tree min_val = size_zero_node;
+      if (type_domain && TYPE_MIN_VALUE (type_domain))
+       min_val = TYPE_MIN_VALUE (type_domain);
+      offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (optype)));
+      offset_int idx = off / el_sz;
+      offset_int rem = off % el_sz;
+      if (TREE_CODE (min_val) == INTEGER_CST)
+       {
+         tree index
+           = wide_int_to_tree (sizetype, idx + wi::to_offset (min_val));
+         op = build4_loc (loc, ARRAY_REF, TREE_TYPE (optype), op, index,
+                          NULL_TREE, NULL_TREE);
+         return c_fold_indirect_ref_for_warn (loc, type, op, rem);
+       }
+    }
+  /* ((foo *)&struct_with_foo_field)[x] => COMPONENT_REF */
+  else if (TREE_CODE (optype) == RECORD_TYPE)
+    {
+      for (tree field = TYPE_FIELDS (optype);
+          field; field = DECL_CHAIN (field))
+       if (TREE_CODE (field) == FIELD_DECL
+           && TREE_TYPE (field) != error_mark_node
+           && TYPE_SIZE_UNIT (TREE_TYPE (field))
+           && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (field))) == INTEGER_CST)
+         {
+           tree pos = byte_position (field);
+           if (TREE_CODE (pos) != INTEGER_CST)
+             continue;
+           offset_int upos = wi::to_offset (pos);
+           offset_int el_sz
+             = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (field)));
+           if (upos <= off && off < upos + el_sz)
+             {
+               tree cop = build3_loc (loc, COMPONENT_REF, TREE_TYPE (field),
+                                      op, field, NULL_TREE);
+               if (tree ret = c_fold_indirect_ref_for_warn (loc, type, cop,
+                                                            off - upos))
+                 return ret;
+             }
+         }
+    }
+  /* Similarly for unions, but in this case try to be very conservative,
+     only match if some field has type compatible with type and it is the
+     only such field.  */
+  else if (TREE_CODE (optype) == UNION_TYPE)
+    {
+      tree fld = NULL_TREE;
+      for (tree field = TYPE_FIELDS (optype);
+          field; field = DECL_CHAIN (field))
+       if (TREE_CODE (field) == FIELD_DECL
+           && TREE_TYPE (field) != error_mark_node
+           && lang_hooks.types_compatible_p (TREE_TYPE (field), type))
+         {
+           if (fld)
+             return NULL_TREE;
+           else
+             fld = field;
+         }
+      if (fld)
+       return build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), op, fld,
+                          NULL_TREE);
+    }
+
+  return NULL_TREE;
+}
 /* Print the MEM_REF expression REF, including its type and offset.
    Apply casts as necessary if the type of the access is different
    from the type of the accessed object.  Produce compact output
@@ -1836,6 +1933,14 @@ print_mem_ref (c_pretty_printer *pp, tre
   const bool addr = TREE_CODE (arg) == ADDR_EXPR;
   if (addr)
     {
+      tree op
+       = c_fold_indirect_ref_for_warn (EXPR_LOCATION (e), TREE_TYPE (e),
+                                       TREE_OPERAND (arg, 0), byte_off);
+      if (op)
+       {
+         pp->expression (op);
+         return;
+       }
       arg = TREE_OPERAND (arg, 0);
       if (byte_off == 0)
        {
which in itself will fix 3 out of the 5 cases in the above testcase to the IMHO
optimum form,
t.u.b, t.v and s[1] are valid C/C++ and accurate descriptions on what is
uninitialized.
Now, a question is if this c_fold_indirect_ref_for_warn needs to return
non-NULL only on type match (for unions I'd prefer to), or also whenever the
type just fully fits into the range.  In the latter case I guess it should have
the byte offset passed by reference and update it on non-NULL return to be
relative to what it returned.  E.g. for the t.u.d which is read through
incompatible type,
it could still return t.u.d and set byte_off to 0, but e.g. for *(int *)(p +
__builtin_offsetof (struct T, u) + 1) it would need to stop at t.u and byte_off
1.
In this case pp->expression (op) should be called only if byte_off == 0 &&
lang_hooks.types.types_compatible (TREE_TYPE (e), TREE_TYPE (op)) as the fast
path, but code later on could print e.g. *(unsigned *)&t.u.d or *(int *)((char
*)&t.u + 1).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2021-01-13 13:49 ` jakub at gcc dot gnu.org
@ 2021-01-13 14:05 ` jakub at gcc dot gnu.org
  2021-01-13 14:15 ` jakub at gcc dot gnu.org
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-13 14:05 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That would be then following (though I didn't want to touch the rest of
print_mem_ref so the patch doesn't remember op anywhere and resets byte_off
too).
--- gcc/c-family/c-pretty-print.c.jj    2021-01-13 08:02:09.425498954 +0100
+++ gcc/c-family/c-pretty-print.c       2021-01-13 15:02:57.860329998 +0100
@@ -1809,6 +1809,113 @@ pp_c_call_argument_list (c_pretty_printe
   pp_c_right_paren (pp);
 }

+/* Try to fold *(type *)&op into op.fld.fld2[1] if possible.
+   Only used for printing expressions.  Should punt if ambiguous
+   (e.g. in unions).  */
+
+static tree
+c_fold_indirect_ref_for_warn (location_t loc, tree type, tree op,
+                             offset_int &off)
+{
+  tree optype = TREE_TYPE (op);
+  if (off == 0)
+    {
+      if (lang_hooks.types_compatible_p (optype, type))
+       return op;
+      /* *(foo *)&complexfoo => __real__ complexfoo */
+      else if (TREE_CODE (optype) == COMPLEX_TYPE
+              && lang_hooks.types_compatible_p (type, TREE_TYPE (optype)))
+       return build1_loc (loc, REALPART_EXPR, type, op);
+    }
+  /* ((foo*)&complexfoo)[1] => __imag__ complexfoo */
+  else if (TREE_CODE (optype) == COMPLEX_TYPE
+          && lang_hooks.types_compatible_p (type, TREE_TYPE (optype))
+          && tree_to_uhwi (TYPE_SIZE_UNIT (type)) == off)
+    {
+      off = 0;
+      return build1_loc (loc, IMAGPART_EXPR, type, op);
+    }
+  /* ((foo *)&fooarray)[x] => fooarray[x] */
+  if (TREE_CODE (optype) == ARRAY_TYPE
+      && TYPE_SIZE_UNIT (TREE_TYPE (optype))
+      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (optype))) == INTEGER_CST
+      && !integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (optype))))
+    {
+      tree type_domain = TYPE_DOMAIN (optype);
+      tree min_val = size_zero_node;
+      if (type_domain && TYPE_MIN_VALUE (type_domain))
+       min_val = TYPE_MIN_VALUE (type_domain);
+      offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (optype)));
+      offset_int idx = off / el_sz;
+      offset_int rem = off % el_sz;
+      if (TREE_CODE (min_val) == INTEGER_CST)
+       {
+         tree index
+           = wide_int_to_tree (sizetype, idx + wi::to_offset (min_val));
+         op = build4_loc (loc, ARRAY_REF, TREE_TYPE (optype), op, index,
+                          NULL_TREE, NULL_TREE);
+         off = rem;
+         if (tree ret = c_fold_indirect_ref_for_warn (loc, type, op, off))
+           return ret;
+         return op;
+       }
+    }
+  /* ((foo *)&struct_with_foo_field)[x] => COMPONENT_REF */
+  else if (TREE_CODE (optype) == RECORD_TYPE)
+    {
+      for (tree field = TYPE_FIELDS (optype);
+          field; field = DECL_CHAIN (field))
+       if (TREE_CODE (field) == FIELD_DECL
+           && TREE_TYPE (field) != error_mark_node
+           && TYPE_SIZE_UNIT (TREE_TYPE (field))
+           && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (field))) == INTEGER_CST)
+         {
+           tree pos = byte_position (field);
+           if (TREE_CODE (pos) != INTEGER_CST)
+             continue;
+           offset_int upos = wi::to_offset (pos);
+           offset_int el_sz
+             = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (field)));
+           if (upos <= off && off < upos + el_sz)
+             {
+               tree cop = build3_loc (loc, COMPONENT_REF, TREE_TYPE (field),
+                                      op, field, NULL_TREE);
+               off = off - upos;
+               if (tree ret = c_fold_indirect_ref_for_warn (loc, type, cop,
+                                                            off))
+                 return ret;
+               return cop;
+             }
+         }
+    }
+  /* Similarly for unions, but in this case try to be very conservative,
+     only match if some field has type compatible with type and it is the
+     only such field.  */
+  else if (TREE_CODE (optype) == UNION_TYPE)
+    {
+      tree fld = NULL_TREE;
+      for (tree field = TYPE_FIELDS (optype);
+          field; field = DECL_CHAIN (field))
+       if (TREE_CODE (field) == FIELD_DECL
+           && TREE_TYPE (field) != error_mark_node
+           && lang_hooks.types_compatible_p (TREE_TYPE (field), type))
+         {
+           if (fld)
+             return NULL_TREE;
+           else
+             fld = field;
+         }
+      if (fld)
+       {
+         off = 0;
+         return build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), op, fld,
+                            NULL_TREE);
+       }
+    }
+
+  return NULL_TREE;
+}
+
 /* Print the MEM_REF expression REF, including its type and offset.
    Apply casts as necessary if the type of the access is different
    from the type of the accessed object.  Produce compact output
@@ -1836,6 +1943,17 @@ print_mem_ref (c_pretty_printer *pp, tre
   const bool addr = TREE_CODE (arg) == ADDR_EXPR;
   if (addr)
     {
+      tree op
+       = c_fold_indirect_ref_for_warn (EXPR_LOCATION (e), TREE_TYPE (e),
+                                       TREE_OPERAND (arg, 0), byte_off);
+      if (op
+         && byte_off == 0
+         && lang_hooks.types_compatible_p (TREE_TYPE (e), TREE_TYPE (op)))
+       {
+         pp->expression (op);
+         return;
+       }
+      byte_off = wi::to_offset (TREE_OPERAND (e, 1));
       arg = TREE_OPERAND (arg, 0);
       if (byte_off == 0)
        {

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2021-01-13 14:05 ` jakub at gcc dot gnu.org
@ 2021-01-13 14:15 ` jakub at gcc dot gnu.org
  2021-01-13 16:18 ` msebor at gcc dot gnu.org
  2021-01-15 18:22 ` cvs-commit at gcc dot gnu.org
  13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-01-13 14:15 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, I'm not convinced it is ok to use TREE_TYPE (TREE_TYPE arg)) in the !addr
case for anything at all even for warnings, as in GIMPLE pointer conversions
are useless and therefore it very well could be completely unrelated pointer
type (in the more lucky case say void * from say the memory allocation, but if
less lucky...).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2021-01-13 14:15 ` jakub at gcc dot gnu.org
@ 2021-01-13 16:18 ` msebor at gcc dot gnu.org
  2021-01-15 18:22 ` cvs-commit at gcc dot gnu.org
  13 siblings, 0 replies; 15+ messages in thread
From: msebor at gcc dot gnu.org @ 2021-01-13 16:18 UTC (permalink / raw)
  To: gcc-bugs

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

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> ---
The ICE has been fixed.  Please open a new bug to track improvements that are
unrelated to the ICE.  Or, if you want to repurpose this bug for your own
changes assign it to yourself.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug tree-optimization/98597] [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0
  2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2021-01-13 16:18 ` msebor at gcc dot gnu.org
@ 2021-01-15 18:22 ` cvs-commit at gcc dot gnu.org
  13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-01-15 18:22 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r11-6729-gadb520606ce3e1e1f8aa8c5d0c59a5f3196fc545
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jan 15 19:20:29 2021 +0100

    c-family: Improve MEM_REF printing for diagnostics [PR98597]

    Ok, here is an updated patch which fixes what I found, and implements what
    has been discussed on the mailing list and on IRC, i.e. if the types
    are compatible as well as alias sets are same, then it prints
    what c_fold_indirect_ref_for_warn managed to create, otherwise it uses
    that info for printing offsets using offsetof (except when it starts
    with ARRAY_REFs, because one can't have offsetof (struct T[2][2],
[1][0].x.y)

    The uninit-38.c test (which was the only one I believe which had tests on
the
    exact spelling of MEM_REF printing) contains mainly changes to have space
    before * for pointer types (as that is how the C pretty-printers normally
    print types, int * rather than int*), plus what might be considered a
    regression from what Martin printed, but it is actually a correctness fix.

    When the arg is a pointer with type pointer to VLA with char element type
    (let's say the pointer is p), which is what happens in several of the
    uninit-38.c tests, omitting the (char *) cast is incorrect, as p + 1
    is not the 1 byte after p, but pointer to the end of the VLA.
    It only happened to work because of the hacks (which I don't like at all
    and are dangerous, DECL_ARTIFICIAL var names with dot inside can be pretty
    much anything, e.g. a lot of passes construct their helper vars from some
    prefix that designates intended use of the var plus numeric suffix), where
    the a.1 pointer to VLA is printed as a which if one is lucky happens to be
    a variable with VLA type (rather than pointer to it), and for such vars
    a + 1 is indeed &a[0] + 1 rather than &a + 1.  But if we want to do this
    reliably, we'd need to make sure it comes from VLA (e.g. verify that the
    SSA_NAME is defined to __builtin_alloca_with_align and that there exists
    a corresponding VAR_DECL with DECL_VALUE_EXPR that has the a.1 variable
    in it).

    2021-01-15  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/98597
            * c-pretty-print.c: Include options.h.
            (c_fold_indirect_ref_for_warn): New function.
            (print_mem_ref): Use it.  If it returns something that has
compatible
            type and is TBAA compatible with zero offset, print it and return,
            otherwise print it using offsetof syntax or array ref syntax.  Fix
up
            printing if MEM_REFs first operand is ADDR_EXPR, or when the first
            argument has pointer to array type.  Print pointers using the
standard
            formatting.

            * gcc.dg/uninit-38.c: Expect a space in between type name and
asterisk.
            Expect for now a (char *) cast for VLAs.
            * gcc.dg/uninit-40.c: New test.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-01-15 18:22 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08  8:20 [Bug tree-optimization/98597] New: [11 Regression] ICE in print_mem_ref since r11-6508-gabb1b6058c09a7c0 marxin at gcc dot gnu.org
2021-01-08  8:20 ` [Bug tree-optimization/98597] " marxin at gcc dot gnu.org
2021-01-11 23:29 ` slyfox at gcc dot gnu.org
2021-01-12  8:43 ` marxin at gcc dot gnu.org
2021-01-12  8:43 ` marxin at gcc dot gnu.org
2021-01-12 18:39 ` jakub at gcc dot gnu.org
2021-01-12 19:02 ` jakub at gcc dot gnu.org
2021-01-12 20:03 ` cvs-commit at gcc dot gnu.org
2021-01-12 20:10 ` jakub at gcc dot gnu.org
2021-01-13 11:49 ` jakub at gcc dot gnu.org
2021-01-13 13:49 ` jakub at gcc dot gnu.org
2021-01-13 14:05 ` jakub at gcc dot gnu.org
2021-01-13 14:15 ` jakub at gcc dot gnu.org
2021-01-13 16:18 ` msebor at gcc dot gnu.org
2021-01-15 18:22 ` cvs-commit at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).