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

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

[Jakub Jelinek]

Hi!

The following patch doesn't actually fix the print_mem_ref bugs, I've kept
it for now as broken as it was, but at least improves the cases where
we can unambiguously map back MEM[&something + off] into some particular
reference (e.g. something.foo[1].bar etc.).
In the distant past I think we were folding such MEM_REFs back to
COMPONENT_REFs and ARRAY_REFs, but we've stopped doing that.  But for
diagnostics that is what the user actually want to see IMHO.
So on the attached testcase, instead of printing what is in left column
it prints what is in right column:
((int*)t) + 3		t.u.b
((int*)t) + 6		t.u.e.i
((int*)t) + 8		t.v
s + 1			s[1]
Of course, print_mem_ref needs to be also fixed to avoid printing the
nonsense it is printing right now, t is a structure type, so it can't be
cast to int* in C and in C++ only using some user operator, and
the result of what it printed is a pointer, while the uninitialized reads
are int.

I was hoping Martin would fix that, but given his comment in the PR I think
I'll fix it myself tomorrow.

Anyway, this patch is useful on its own.  Bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?

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

	PR tree-optimization/98597
	* c-pretty-print.c (c_fold_indirect_ref_for_warn): New function.
	(print_mem_ref): Call it.

	* gcc.dg/uninit-40.c: New test.

--- 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)
 	{
--- gcc/testsuite/gcc.dg/uninit-40.c.jj	2021-01-13 15:23:47.366134905 +0100
+++ gcc/testsuite/gcc.dg/uninit-40.c	2021-01-13 15:23:15.208500279 +0100
@@ -0,0 +1,31 @@
+/* PR tree-optimization/98597 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wuninitialized" } */
+
+union U { double d; int i; float f; };
+struct S { char a; int b; char c; unsigned d; union U e; };
+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 *pt = (char *) &t;
+  q[0] = bar (pt + __builtin_offsetof (struct T, u.b));	/* { dg-warning "'t\\.u\\.b' is used uninitialized" } */
+  q[1] = bar (pt + __builtin_offsetof (struct T, u.e));	/* { dg-warning "'t\\.u\\.e\\.i' is used uninitialized" } */
+  q[2] = bar (pt + __builtin_offsetof (struct T, v));	/* { dg-warning "'t\\.v' is used uninitialized" } */
+  int s[3];
+  s[0] = 1;
+  char *ps = (char *) s;
+  q[3] = bar (ps + sizeof (int));			/* { dg-warning "'s\\\[1\\\]' is used uninitialized" } */
+}

	Jakub

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

[Richard Biener]

On Wed, 13 Jan 2021, Jakub Jelinek wrote:

> Hi!
> 
> The following patch doesn't actually fix the print_mem_ref bugs, I've kept
> it for now as broken as it was, but at least improves the cases where
> we can unambiguously map back MEM[&something + off] into some particular
> reference (e.g. something.foo[1].bar etc.).
> In the distant past I think we were folding such MEM_REFs back to
> COMPONENT_REFs and ARRAY_REFs, but we've stopped doing that.

Yeah, because it has different semantics - *(((int *)t + 3)
accesses an int object while t.u.b accesses a 't' object from the TBAA
perspective.

>  But for
> diagnostics that is what the user actually want to see IMHO.
> So on the attached testcase, instead of printing what is in left column
> it prints what is in right column:
> ((int*)t) + 3		t.u.b
> ((int*)t) + 6		t.u.e.i
> ((int*)t) + 8		t.v
> s + 1			s[1]

so while that's "nice" in general, for TBAA diagnostics it might actually
be misleading.

I wonder whether we absolutely need to print a C expression here.
We could print, instead of *((int *)t + 3), "access to a memory
object of type 'int' at offset 12 bytes from 't'", thus explain
in plain english.

That said, *((int *)t + 3) is exactly what the access is,
semantically.  There's the case of a mismatch of the access type
and the TBAA type which we cannot write down in C terms but maybe
we want to have a builtin for this?  __builtin_access (ptr, lvalue-type,
tbaa-type)?

> Of course, print_mem_ref needs to be also fixed to avoid printing the
> nonsense it is printing right now, t is a structure type, so it can't be
> cast to int* in C and in C++ only using some user operator, and
> the result of what it printed is a pointer, while the uninitialized reads
> are int.
> 
> I was hoping Martin would fix that, but given his comment in the PR I think
> I'll fix it myself tomorrow.
> 
> Anyway, this patch is useful on its own.  Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?

In the light of Martins patch this is probably reasonable but still
the general direction is wrong (which is why I didn't approve Martins
original patch).  I'm also somewhat disappointed we're breaking this
so late in the cycle.

c_fold_indirect_ref_for_warn doesn't look like it is especially
careful about error recovery issues (error_mark_node in random
places of the trees).  Maybe that never happens.

Thanks,
Richard.

> 2021-01-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/98597
> 	* c-pretty-print.c (c_fold_indirect_ref_for_warn): New function.
> 	(print_mem_ref): Call it.
> 
> 	* gcc.dg/uninit-40.c: New test.
> 
> --- 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)
>  	{
> --- gcc/testsuite/gcc.dg/uninit-40.c.jj	2021-01-13 15:23:47.366134905 +0100
> +++ gcc/testsuite/gcc.dg/uninit-40.c	2021-01-13 15:23:15.208500279 +0100
> @@ -0,0 +1,31 @@
> +/* PR tree-optimization/98597 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wuninitialized" } */
> +
> +union U { double d; int i; float f; };
> +struct S { char a; int b; char c; unsigned d; union U e; };
> +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 *pt = (char *) &t;
> +  q[0] = bar (pt + __builtin_offsetof (struct T, u.b));	/* { dg-warning "'t\\.u\\.b' is used uninitialized" } */
> +  q[1] = bar (pt + __builtin_offsetof (struct T, u.e));	/* { dg-warning "'t\\.u\\.e\\.i' is used uninitialized" } */
> +  q[2] = bar (pt + __builtin_offsetof (struct T, v));	/* { dg-warning "'t\\.v' is used uninitialized" } */
> +  int s[3];
> +  s[0] = 1;
> +  char *ps = (char *) s;
> +  q[3] = bar (ps + sizeof (int));			/* { dg-warning "'s\\\[1\\\]' is used uninitialized" } */
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

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

[Jakub Jelinek]

On Thu, Jan 14, 2021 at 08:43:54AM +0100, Richard Biener wrote:
> >  But for
> > diagnostics that is what the user actually want to see IMHO.
> > So on the attached testcase, instead of printing what is in left column
> > it prints what is in right column:
> > ((int*)t) + 3		t.u.b
> > ((int*)t) + 6		t.u.e.i
> > ((int*)t) + 8		t.v
> > s + 1			s[1]
> 
> so while that's "nice" in general, for TBAA diagnostics it might actually
> be misleading.
> 
> I wonder whether we absolutely need to print a C expression here.

I'm afraid yes, because it is not a toplevel routine, but something called
from the c-family pretty-printers, so it can be in the middle of arbitrary
C/C++ expressions.  And printing
(3 * (access to a memory object of type 'int' at offset 12 bytes from 't') + 31) * 42
would be just weird.

> We could print, instead of *((int *)t + 3), "access to a memory
> object of type 'int' at offset 12 bytes from 't'", thus explain
> in plain english.
> 
> That said, *((int *)t + 3) is exactly what the access is,

*((int *)&t + 3) actually, the code I haven't touched has multiple bugs.

The user generally doesn't know the exact layout of the structures,
and especially with C++ templates it is extremely hard to figure that out,
so even when we could print verbose text it would be helpful to give a hint
(in your text something like (which falls into 't.u.b')).
I don't see how we can print both the MEM_REF type and TBAA type in a way
that would be understandable to the user.

Could we print
t.u.b
if the TBAA type is compatible with the type of the reference and perhaps
*(int*)&t.u.b
if it is incompatible?
From the aliasing perspective that is still different, but we don't print
the TBAA type anyway.

> In the light of Martins patch this is probably reasonable but still
> the general direction is wrong (which is why I didn't approve Martins
> original patch).  I'm also somewhat disappointed we're breaking this
> so late in the cycle.

I'm too.

> c_fold_indirect_ref_for_warn doesn't look like it is especially
> careful about error recovery issues (error_mark_node in random
> places of the trees).  Maybe that never happens.

I've created it by copying and adjusting the C++ cxx_fold_indirect_ref_1
which had those error_mark_node checks in there (haven't verified if
they are strictly necessary or not), but as the diagnostic code isn't used
solely during middle-end, but also in the FEs and I remember several cases
where the types had error marks within the types in there.
The function seemed to be too short and after the changes too different
from cxx_fold_indirect_ref_1, which contains some very C++ specific parts,
handling of the active union member or empty bases (there is a pending PR
for it) etc.

	Jakub

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

[Jakub Jelinek]

On Thu, Jan 14, 2021 at 09:28:31AM +0100, Jakub Jelinek via Gcc-patches wrote:
> I'm afraid yes, because it is not a toplevel routine, but something called
> from the c-family pretty-printers, so it can be in the middle of arbitrary
> C/C++ expressions.  And printing
> (3 * (access to a memory object of type 'int' at offset 12 bytes from 't') + 31) * 42
> would be just weird.
> 
> > We could print, instead of *((int *)t + 3), "access to a memory
> > object of type 'int' at offset 12 bytes from 't'", thus explain
> > in plain english.
> > 
> > That said, *((int *)t + 3) is exactly what the access is,
> 
> *((int *)&t + 3) actually, the code I haven't touched has multiple bugs.
> 
> The user generally doesn't know the exact layout of the structures,
> and especially with C++ templates it is extremely hard to figure that out,
> so even when we could print verbose text it would be helpful to give a hint
> (in your text something like (which falls into 't.u.b')).
> I don't see how we can print both the MEM_REF type and TBAA type in a way
> that would be understandable to the user.
> 
> Could we print
> t.u.b
> if the TBAA type is compatible with the type of the reference and perhaps
> *(int*)&t.u.b
> if it is incompatible?
> >From the aliasing perspective that is still different, but we don't print
> the TBAA type anyway.

There is another option I forgot about, but perhaps it is too verbose.
Print
*(int*)((char*)&t + offsetof (struct T, u.b))
so like
*(int*)((char*)&t + 12)
but print the offset in a more user-friendly way.

	Jakub

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

[Richard Biener]

On Thu, 14 Jan 2021, Jakub Jelinek wrote:

> On Thu, Jan 14, 2021 at 09:28:31AM +0100, Jakub Jelinek via Gcc-patches wrote:
> > I'm afraid yes, because it is not a toplevel routine, but something called
> > from the c-family pretty-printers, so it can be in the middle of arbitrary
> > C/C++ expressions.  And printing
> > (3 * (access to a memory object of type 'int' at offset 12 bytes from 't') + 31) * 42
> > would be just weird.
> > 
> > > We could print, instead of *((int *)t + 3), "access to a memory
> > > object of type 'int' at offset 12 bytes from 't'", thus explain
> > > in plain english.
> > > 
> > > That said, *((int *)t + 3) is exactly what the access is,
> > 
> > *((int *)&t + 3) actually, the code I haven't touched has multiple bugs.
> > 
> > The user generally doesn't know the exact layout of the structures,
> > and especially with C++ templates it is extremely hard to figure that out,
> > so even when we could print verbose text it would be helpful to give a hint
> > (in your text something like (which falls into 't.u.b')).
> > I don't see how we can print both the MEM_REF type and TBAA type in a way
> > that would be understandable to the user.
> > 
> > Could we print
> > t.u.b
> > if the TBAA type is compatible with the type of the reference and perhaps
> > *(int*)&t.u.b
> > if it is incompatible?
> > >From the aliasing perspective that is still different, but we don't print
> > the TBAA type anyway.

True.  As said we could simply add a GCC extension to write a MEM_REF
in source and print that syntax ... then it would be valid (GCC) C/C++.

> There is another option I forgot about, but perhaps it is too verbose.
> Print
> *(int*)((char*)&t + offsetof (struct T, u.b))

or rather offsetof (struct T, u) to not single out a specific union
member?

Richard.

> so like
> *(int*)((char*)&t + 12)
> but print the offset in a more user-friendly way.

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

[Jakub Jelinek]

On Thu, Jan 14, 2021 at 11:05:40AM +0100, Richard Biener wrote:
> > > Could we print
> > > t.u.b
> > > if the TBAA type is compatible with the type of the reference and perhaps
> > > *(int*)&t.u.b
> > > if it is incompatible?
> > > >From the aliasing perspective that is still different, but we don't print
> > > the TBAA type anyway.
> 
> True.  As said we could simply add a GCC extension to write a MEM_REF
> in source and print that syntax ... then it would be valid (GCC) C/C++.

But even if we do that unless people are familiar with that extension they
wouldn't know what it means (and they didn't write it in that way in their
source).

> > There is another option I forgot about, but perhaps it is too verbose.
> > Print
> > *(int*)((char*)&t + offsetof (struct T, u.b))
> 
> or rather offsetof (struct T, u) to not single out a specific union
> member?

Sure, I can just get rid of the UNION_TYPE handling from the function,
or use it only if the TBAA access type is compatible.

	Jakub

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

[Richard Biener]

On Thu, 14 Jan 2021, Jakub Jelinek wrote:

> On Thu, Jan 14, 2021 at 11:05:40AM +0100, Richard Biener wrote:
> > > > Could we print
> > > > t.u.b
> > > > if the TBAA type is compatible with the type of the reference and perhaps
> > > > *(int*)&t.u.b
> > > > if it is incompatible?
> > > > >From the aliasing perspective that is still different, but we don't print
> > > > the TBAA type anyway.
> > 
> > True.  As said we could simply add a GCC extension to write a MEM_REF
> > in source and print that syntax ... then it would be valid (GCC) C/C++.
> 
> But even if we do that unless people are familiar with that extension they
> wouldn't know what it means (and they didn't write it in that way in their
> source).

I'd just use it in case we can't express it in the C/C++ way (thus when
the TBAA type differs).  We already print {ref-all} in type dumping
for example, which isn't C/C++ either.

> > > There is another option I forgot about, but perhaps it is too verbose.
> > > Print
> > > *(int*)((char*)&t + offsetof (struct T, u.b))
> > 
> > or rather offsetof (struct T, u) to not single out a specific union
> > member?
> 
> Sure, I can just get rid of the UNION_TYPE handling from the function,
> or use it only if the TBAA access type is compatible.

Sure.

I wonder if we can use some pp flags to tell whether we're being
called from FE or middle-end diagnostics and thus whether we
should try to produce source-like expressions or can expect
weird GIMPLE IL derived expressions.

Richard.

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

[Martin Sebor]

On 1/14/21 12:43 AM, Richard Biener wrote:
> On Wed, 13 Jan 2021, Jakub Jelinek wrote:
> 
>> Hi!
>>
>> The following patch doesn't actually fix the print_mem_ref bugs, I've kept
>> it for now as broken as it was, but at least improves the cases where
>> we can unambiguously map back MEM[&something + off] into some particular
>> reference (e.g. something.foo[1].bar etc.).
>> In the distant past I think we were folding such MEM_REFs back to
>> COMPONENT_REFs and ARRAY_REFs, but we've stopped doing that.
> 
> Yeah, because it has different semantics - *(((int *)t + 3)
> accesses an int object while t.u.b accesses a 't' object from the TBAA
> perspective.
> 
>>   But for
>> diagnostics that is what the user actually want to see IMHO.
>> So on the attached testcase, instead of printing what is in left column
>> it prints what is in right column:
>> ((int*)t) + 3		t.u.b
>> ((int*)t) + 6		t.u.e.i
>> ((int*)t) + 8		t.v
>> s + 1			s[1]
> 
> so while that's "nice" in general, for TBAA diagnostics it might actually
> be misleading.
> 
> I wonder whether we absolutely need to print a C expression here.
> We could print, instead of *((int *)t + 3), "access to a memory
> object of type 'int' at offset 12 bytes from 't'", thus explain
> in plain english.
> 
> That said, *((int *)t + 3) is exactly what the access is,
> semantically.  There's the case of a mismatch of the access type
> and the TBAA type which we cannot write down in C terms but maybe
> we want to have a builtin for this?  __builtin_access (ptr, lvalue-type,
> tbaa-type)?
> 
>> Of course, print_mem_ref needs to be also fixed to avoid printing the
>> nonsense it is printing right now, t is a structure type, so it can't be
>> cast to int* in C and in C++ only using some user operator, and
>> the result of what it printed is a pointer, while the uninitialized reads
>> are int.
>>
>> I was hoping Martin would fix that, but given his comment in the PR I think
>> I'll fix it myself tomorrow.
>>
>> Anyway, this patch is useful on its own.  Bootstrapped/regtested on
>> x86_64-linux and i686-linux, ok for trunk?
> 
> In the light of Martins patch this is probably reasonable but still
> the general direction is wrong (which is why I didn't approve Martins
> original patch).  I'm also somewhat disappointed we're breaking this
> so late in the cycle.

So am I.  I didn't test this change as exhaustively as I could and
(in light of the poor test coverage) should have.  That's my bad.
FWIW, I did do it for the first patch (by instrumenting GCC and
formatting every MEM_REF it came across), but it didn't occur to
me to do it this time around.  I have now completed this testing
(it found one more ICE elsewhere that I'll fix soon).

That said, as I mentioned in the patch submission, most middle end
warnings don't format MEM_REFs.  -Wuninitialized is an outlier here.
Most middle end warnings about invalid accesses print the decl or
allocation call with either a type or size of the access, followed
by either an array index or a byte offset in to the target of
the access.  For consistency I'd like to converge most middle end
warnings on the same style (formatted by the same code).  When
that happens, the MEM_REF format should become much less important,
so I wouldn't invest too much effort into perfecting it.

Martin

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

[Jakub Jelinek]

On Thu, Jan 14, 2021 at 10:49:42AM -0700, Martin Sebor wrote:
> > In the light of Martins patch this is probably reasonable but still
> > the general direction is wrong (which is why I didn't approve Martins
> > original patch).  I'm also somewhat disappointed we're breaking this
> > so late in the cycle.
> 
> So am I.  I didn't test this change as exhaustively as I could and
> (in light of the poor test coverage) should have.  That's my bad.
> FWIW, I did do it for the first patch (by instrumenting GCC and
> formatting every MEM_REF it came across), but it didn't occur to
> me to do it this time around.  I have now completed this testing
> (it found one more ICE elsewhere that I'll fix soon).

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).

Is this ok for trunk if it passes bootstrap/regtest?

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

	PR tree-optimization/98597
	* c-pretty-print.c (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.

--- gcc/c-family/c-pretty-print.c.jj	2021-01-13 15:27:09.822834600 +0100
+++ gcc/c-family/c-pretty-print.c	2021-01-14 19:02:21.299138891 +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
@@ -1833,59 +1940,79 @@ print_mem_ref (c_pretty_printer *pp, tre
   offset_int elt_idx = 0;
   /* True to include a cast to char* (for a nonzero final BYTE_OFF).  */
   bool char_cast = false;
-  const bool addr = TREE_CODE (arg) == ADDR_EXPR;
-  if (addr)
+  tree op = NULL_TREE;
+  bool array_ref_only = false;
+  if (TREE_CODE (arg) == ADDR_EXPR)
     {
-      arg = TREE_OPERAND (arg, 0);
-      if (byte_off == 0)
+      op = c_fold_indirect_ref_for_warn (EXPR_LOCATION (e), TREE_TYPE (e),
+					 TREE_OPERAND (arg, 0), byte_off);
+      /* Try to fold it back to component, array ref or their combination,
+	 but print it only if the types and TBAA types are compatible.  */
+      if (op
+	  && byte_off == 0
+	  && lang_hooks.types_compatible_p (TREE_TYPE (e), TREE_TYPE (op))
+	  && get_deref_alias_set (TREE_OPERAND (e, 1)) == get_alias_set (op))
 	{
-	  pp->expression (arg);
+	  pp->expression (op);
 	  return;
 	}
+      if (op == NULL_TREE)
+	op = TREE_OPERAND (arg, 0);
+      /* If the types or TBAA types are incompatible, undo the
+	 UNION_TYPE handling from c_fold_indirect_ref_for_warn, and similarly
+	 undo __real__/__imag__ the code below doesn't try to handle.  */
+      if (op != TREE_OPERAND (arg, 0)
+	  && ((TREE_CODE (op) == COMPONENT_REF
+	       && TREE_CODE (TREE_TYPE (TREE_OPERAND (op, 0))) == UNION_TYPE)
+	      || TREE_CODE (op) == REALPART_EXPR
+	      || TREE_CODE (op) == IMAGPART_EXPR))
+	op = TREE_OPERAND (op, 0);
+      if (op != TREE_OPERAND (arg, 0))
+	{
+	  array_ref_only = true;
+	  for (tree ref = op; ref != TREE_OPERAND (arg, 0);
+	       ref = TREE_OPERAND (ref, 0))
+	    if (TREE_CODE (ref) != ARRAY_REF)
+	      {
+		array_ref_only = false;
+		break;
+	      }
+	}
     }
 
   tree access_type = TREE_TYPE (e);
-  if (TREE_CODE (access_type) == ARRAY_TYPE)
-    access_type = TREE_TYPE (access_type);
-  tree arg_type = TREE_TYPE (arg);
-  if (POINTER_TYPE_P (arg_type))
-    arg_type = TREE_TYPE (arg_type);
-  if (TREE_CODE (arg_type) == ARRAY_TYPE)
-    arg_type = TREE_TYPE (arg_type);
+  tree arg_type = TREE_TYPE (TREE_TYPE (arg));
   if (tree access_size = TYPE_SIZE_UNIT (access_type))
-    if (TREE_CODE (access_size) == INTEGER_CST)
+    if (byte_off != 0
+	&& TREE_CODE (access_size) == INTEGER_CST
+	&& !integer_zerop (access_size))
       {
-	/* For naturally aligned accesses print the nonzero offset
-	   in units of the accessed type, in the form of an index.
-	   For unaligned accesses also print the residual byte offset.  */
 	offset_int asize = wi::to_offset (access_size);
-	offset_int szlg2 = wi::floor_log2 (asize);
-
-	elt_idx = byte_off >> szlg2;
-	byte_off = byte_off - (elt_idx << szlg2);
+	elt_idx = byte_off / asize;
+	byte_off = byte_off % asize;
       }
 
   /* True to include a cast to the accessed type.  */
-  const bool access_cast = VOID_TYPE_P (arg_type)
-    || TYPE_MAIN_VARIANT (access_type) != TYPE_MAIN_VARIANT (arg_type);
+  const bool access_cast
+    = ((op && op != TREE_OPERAND (arg, 0))
+       || VOID_TYPE_P (arg_type)
+       || !lang_hooks.types_compatible_p (access_type, arg_type));
+  const bool has_off = byte_off != 0 || (op && op != TREE_OPERAND (arg, 0));
 
-  if (byte_off != 0)
+  if (has_off && (byte_off != 0 || !array_ref_only))
     {
       /* When printing the byte offset for a pointer to a type of
 	 a different size than char, include a cast to char* first,
 	 before printing the cast to a pointer to the accessed type.  */
-      offset_int arg_size = 0;
-      if (tree size = TYPE_SIZE (arg_type))
-	arg_size = wi::to_offset (size);
-      if (arg_size != BITS_PER_UNIT)
+      tree size = TYPE_SIZE (arg_type);
+      if (size == NULL_TREE
+	  || TREE_CODE (size) != INTEGER_CST
+	  || wi::to_wide (size) != BITS_PER_UNIT)
 	char_cast = true;
     }
 
   if (elt_idx == 0)
-    {
-      if (!addr)
-	pp_c_star (pp);
-    }
+    pp_c_star (pp);
   else if (access_cast || char_cast)
     pp_c_left_paren (pp);
 
@@ -1895,25 +2022,63 @@ print_mem_ref (c_pretty_printer *pp, tre
 	 with the type of the referenced object (or if the object
 	 is typeless).  */
       pp_c_left_paren (pp);
-      pp->type_id (access_type);
-      pp_c_star (pp);
+      pp->type_id (build_pointer_type (access_type));
       pp_c_right_paren (pp);
     }
 
-  if (byte_off != 0)
+  if (has_off)
     pp_c_left_paren (pp);
 
   if (char_cast)
     {
-      /* Include a cast to char*.  */
+      /* Include a cast to char *.  */
       pp_c_left_paren (pp);
-      pp->type_id (char_type_node);
-      pp_c_star (pp);
+      pp->type_id (string_type_node);
       pp_c_right_paren (pp);
     }
 
   pp->unary_expression (arg);
 
+  if (op && op != TREE_OPERAND (arg, 0))
+    {
+      auto_vec<tree, 16> refs;
+      tree ref;
+      unsigned i;
+      bool array_refs = true;
+      for (ref = op; ref != TREE_OPERAND (arg, 0); ref = TREE_OPERAND (ref, 0))
+	refs.safe_push (ref);
+      FOR_EACH_VEC_ELT_REVERSE (refs, i, ref)
+	if (array_refs && TREE_CODE (ref) == ARRAY_REF)
+	  {
+	    pp_c_left_bracket (pp);
+	    pp->expression (TREE_OPERAND (ref, 1));
+	    pp_c_right_bracket (pp);
+	  }
+	else
+	  {
+	    if (array_refs)
+	      {
+		array_refs = false;
+		pp_string (pp, " + offsetof");
+		pp_c_left_paren (pp);
+		pp->type_id (TREE_TYPE (TREE_OPERAND (ref, 0)));
+		pp_comma (pp);
+	      }
+	    else if (TREE_CODE (ref) == COMPONENT_REF)
+	      pp_c_dot (pp);
+	    if (TREE_CODE (ref) == COMPONENT_REF)
+	      pp->expression (TREE_OPERAND (ref, 1));
+	    else
+	      {
+		pp_c_left_bracket (pp);
+		pp->expression (TREE_OPERAND (ref, 1));
+		pp_c_right_bracket (pp);
+	      }
+	  }
+      if (!array_refs)
+	pp_c_right_paren (pp);
+    }
+
   if (byte_off != 0)
     {
       pp_space (pp);
@@ -1921,25 +2086,20 @@ print_mem_ref (c_pretty_printer *pp, tre
       pp_space (pp);
       tree off = wide_int_to_tree (ssizetype, byte_off);
       pp->constant (off);
-      pp_c_right_paren (pp);
     }
+
+  if (has_off)
+    pp_c_right_paren (pp);
+
   if (elt_idx != 0)
     {
       if (access_cast || char_cast)
 	pp_c_right_paren (pp);
 
-      if (addr)
-	{
-	  pp_space (pp);
-	  pp_plus (pp);
-	  pp_space (pp);
-	}
-      else
-	pp_c_left_bracket (pp);
+      pp_c_left_bracket (pp);
       tree idx = wide_int_to_tree (ssizetype, elt_idx);
       pp->constant (idx);
-      if (!addr)
-	pp_c_right_bracket (pp);
+      pp_c_right_bracket (pp);
     }
 }
 
--- gcc/testsuite/gcc.dg/uninit-38.c.jj	2021-01-07 09:34:09.725641679 +0100
+++ gcc/testsuite/gcc.dg/uninit-38.c	2021-01-14 17:43:33.442029629 +0100
@@ -29,28 +29,28 @@ void sink (void*, ...);
   }						\
   typedef void dummy_type
 
-T (int, 0, 0);      // { dg-warning "'\\*\\(int\\*\\)p' is used uninitialized" }
-T (int, 0, 1);      // { dg-warning "'\\*\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)'" }
-T (int, 0, 2);      // { dg-warning "'\\*\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)'" }
-T (int, 0, 3);      // { dg-warning "'\\*\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)'" }
-T (int, 0, 4);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[1]'" }
-T (int, 0, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[1]'" }
-T (int, 0, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)\\)\\\[1]'" }
-T (int, 0, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)\\)\\\[1]'" }
-T (int, 0, 8);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[2]'" }
-T (int, 0, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[2]'" }
-
-
-T (int, 1, 0);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[1]' is used uninitialized" }
-T (int, 1, 1);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[1]'" }
-T (int, 1, 2);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)\\)\\\[1]'" }
-T (int, 1, 3);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)\\)\\\[1]'" }
-T (int, 1, 4);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[2]'" }
-T (int, 1, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[2]'" }
-T (int, 1, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)\\)\\\[2]'" }
-T (int, 1, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)\\)\\\[2]'" }
-T (int, 1, 8);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[3]'" }
-T (int, 1, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[3]'" }
+T (int, 0, 0);      // { dg-warning "'\\*\\(int \\*\\)p' is used uninitialized" }
+T (int, 0, 1);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)'" }
+T (int, 0, 2);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)'" }
+T (int, 0, 3);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)'" }
+T (int, 0, 4);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[1]'" }
+T (int, 0, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[1]'" }
+T (int, 0, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)\\)\\\[1]'" }
+T (int, 0, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)\\)\\\[1]'" }
+T (int, 0, 8);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[2]'" }
+T (int, 0, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[2]'" }
+
+
+T (int, 1, 0);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[1]' is used uninitialized" }
+T (int, 1, 1);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[1]'" }
+T (int, 1, 2);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)\\)\\\[1]'" }
+T (int, 1, 3);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)\\)\\\[1]'" }
+T (int, 1, 4);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[2]'" }
+T (int, 1, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[2]'" }
+T (int, 1, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)\\)\\\[2]'" }
+T (int, 1, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)\\)\\\[2]'" }
+T (int, 1, 8);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[3]'" }
+T (int, 1, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[3]'" }
 
 #undef T
 #define T(Type, idx, off)			\
@@ -63,25 +63,25 @@ T (int, 1, 9);      // { dg-warning "'\\
   }						\
   typedef void dummy_type
 
-T (int, 0, 0);      // { dg-warning "'\\*\\(int\\*\\)a' is used uninitialized" }
-T (int, 0, 1);      // { dg-warning "'\\*\\(int\\*\\)\\(a \\+ 1\\)'" }
-T (int, 0, 2);      // { dg-warning "'\\*\\(int\\*\\)\\(a \\+ 2\\)'" }
-T (int, 0, 3);      // { dg-warning "'\\*\\(int\\*\\)\\(a \\+ 3\\)'" }
-T (int, 0, 4);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[1]'" }
-T (int, 0, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[1]'" }
-T (int, 0, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 2\\)\\)\\\[1]'" }
-T (int, 0, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 3\\)\\)\\\[1]'" }
-T (int, 0, 8);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[2]'" }
-T (int, 0, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[2]'" }
-
-
-T (int, 1, 0);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[1]' is used uninitialized" }
-T (int, 1, 1);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[1]'" }
-T (int, 1, 2);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 2\\)\\)\\\[1]'" }
-T (int, 1, 3);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 3\\)\\)\\\[1]'" }
-T (int, 1, 4);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[2]'" }
-T (int, 1, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[2]'" }
-T (int, 1, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 2\\)\\)\\\[2]'" }
-T (int, 1, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 3\\)\\)\\\[2]'" }
-T (int, 1, 8);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[3]'" }
-T (int, 1, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[3]'" }
+T (int, 0, 0);      // { dg-warning "'\\*\\(int \\*\\)a' is used uninitialized" }
+T (int, 0, 1);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)'" }
+T (int, 0, 2);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)'" }
+T (int, 0, 3);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)'" }
+T (int, 0, 4);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[1]'" }
+T (int, 0, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[1]'" }
+T (int, 0, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)\\)\\\[1]'" }
+T (int, 0, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)\\)\\\[1]'" }
+T (int, 0, 8);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[2]'" }
+T (int, 0, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[2]'" }
+
+
+T (int, 1, 0);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[1]' is used uninitialized" }
+T (int, 1, 1);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[1]'" }
+T (int, 1, 2);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)\\)\\\[1]'" }
+T (int, 1, 3);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)\\)\\\[1]'" }
+T (int, 1, 4);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[2]'" }
+T (int, 1, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[2]'" }
+T (int, 1, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)\\)\\\[2]'" }
+T (int, 1, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)\\)\\\[2]'" }
+T (int, 1, 8);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[3]'" }
+T (int, 1, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[3]'" }
--- gcc/testsuite/gcc.dg/uninit-40.c.jj	2021-01-14 13:17:58.483432786 +0100
+++ gcc/testsuite/gcc.dg/uninit-40.c	2021-01-14 19:03:08.039606122 +0100
@@ -0,0 +1,50 @@
+/* PR tree-optimization/98597 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wuninitialized" } */
+
+union U { double d; int i; float f; };
+struct S { char a; int b; char c; unsigned d; union U e; int f[3]; unsigned g[3]; };
+struct T { char t; struct S u; int v; };
+typedef short V[2][2];
+void baz (V *);
+
+static inline int
+bar (char *p)
+{
+  return *(int *) p;
+}
+
+void
+foo (int *q)
+{
+  struct T t;
+  t.t = 1;
+  t.u.c = 2;
+  char *pt = (char *) &t;
+  q[0] = bar (pt + __builtin_offsetof (struct T, u.b));	/* { dg-warning "'t\\.u\\.b' is used uninitialized" } */
+  q[1] = bar (pt + __builtin_offsetof (struct T, u.e));	/* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&t \\+ offsetof\\(struct T, u\\.e\\)\\)' is used uninitialized" } */
+  q[2] = bar (pt + __builtin_offsetof (struct T, v));	/* { dg-warning "'t\\.v' is used uninitialized" } */
+  q[3] = bar (pt + __builtin_offsetof (struct T, u.d));	/* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&t \\+ offsetof\\(struct T, u\\.d\\)\\)' is used uninitialized" } */
+  q[4] = bar (pt + __builtin_offsetof (struct T, u.f[2])); /* { dg-warning "'t\\.u\\.f\\\[2\\\]' is used uninitialized" } */
+  q[5] = bar (pt + __builtin_offsetof (struct T, u.g[2])); /* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&t \\+ offsetof\\(struct T, u\\.g\\\[2\\\]\\)\\)' is used uninitialized" } */
+  int s[3];
+  s[0] = 1;
+  char *ps = (char *) s;
+  q[6] = bar (ps + sizeof (int));			/* { dg-warning "'s\\\[1\\\]' is used uninitialized" } */
+  unsigned w[2][2];
+  w[0][0] = 1;
+  char *pw = (char *) w;
+  q[7] = bar (pw + 3 * sizeof (unsigned));		/* { dg-warning "'\\*\\(int \\*\\)\\(&w\\\[1\\\]\\\[1\\\]\\)' is used uninitialized" } */
+  struct T x[3][3];
+  x[0][0].t = 1;
+  char *px = (char *) x;
+  q[8] = bar (px + 5 * sizeof (struct T) + __builtin_offsetof (struct T, u.b));	/* { dg-warning "'x\\\[1\\\]\\\[2\\\]\\.u\\.b' is used uninitialized" } */
+  q[9] = bar (px + 6 * sizeof (struct T) + __builtin_offsetof (struct T, u.d));	/* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&x\\\[2\\\]\\\[0\\\] \\+ offsetof\\(struct T, u\\.d\\)\\)' is used uninitialized" } */
+#if defined(__i386__) || defined(__x86_64__)
+  /* memcpy folding is too target dependent to test it everywhere.  */
+  V u[2], v[2];
+  u[0][0][0] = 1;
+  __builtin_memcpy (&v[1], &u[1], sizeof (V));		/* { dg-warning "'\\*\\(\(long \)?long unsigned int \\*\\)\\(&u\\\[1\\\]\\\[0\\\]\\\[0\\\]\\)' is used uninitialized" "" { target i?86-*-* x86_64-*-* } } */
+  baz (&v[1]);
+#endif
+}


	Jakub

Re: [PATCH] c-family, v2: Improve MEM_REF printing for diagnostics [PR98597]

[Jakub Jelinek]

On Thu, Jan 14, 2021 at 07:26:36PM +0100, Jakub Jelinek via Gcc-patches wrote:
> Is this ok for trunk if it passes bootstrap/regtest?

So, x86_64-linux bootstrap unfortunately broke due to the -march=i486
changes, but at least i686-linux bootstrap succeeded and shows 2
regressions.

One is on g++.dg/gomp/allocate-2.C, which used to print:
allocate-2.C:9:36: error: user defined reduction not found for ‘s’
but now prints:
allocate-2.C:9:36: error: user defined reduction not found for ‘*&s’
because of -O0 and therefore -fno-strict-aliasing.
The problem is that for !flag_strict_aliasing get_deref_alias_set returns 0
and so the:
&& get_deref_alias_set (TREE_OPERAND (e, 1)) == get_alias_set (op)
check fails.  So, shall the code use
&& (!flag_no_strict_aliasing
    || get_deref_alias_set (TREE_OPERAND (e, 1)) == get_alias_set (op))
instead, or
get_alias_set (TREE_TYPE (TREE_TYPE (TREE_OPERAND (e, 1))))
== get_alias_set (op)
?
The other is on gcc.dg/gomp/_Atomic-3.c test, where we used to print
_Atomic-3.c:22:34: error: ‘_Atomic’ ‘k’ in ‘reduction’ clause
but now print
_Atomic-3.c:22:34: error: ‘_Atomic’ ‘*(_Atomic int (*)[4])(&k[0])’ in ‘reduction’ clause
Apparently in this case the C FE considers the two _Atomic int [4] types
incompatible, one is created through
c_build_qualified_type (type=<array_type 0x7fffea186a80>, type_quals=8, orig_qual_type=<tree 0x0>, orig_qual_indirect=1)
on an int [4] type, i.e. adding _Atomic qualifier to an unqualified array
type, and the other is created through
build_array_type (elt_type=<integer_type 0x7fffea186540 int>, index_type=<integer_type 0x7fffea186498>, typeless_storage=false)
i.e. creating an array with _Atomic int elements.
That seems like a C FE bug to me.

Anyway, I can fix or workaround that by:
--- gcc/c/c-typeck.c.jj	2021-01-04 10:25:49.651111329 +0100
+++ gcc/c/c-typeck.c	2021-01-15 09:53:29.590611264 +0100
@@ -13979,7 +13979,9 @@ c_finish_omp_clauses (tree clauses, enum
 	      size = size_binop (MINUS_EXPR, size, size_one_node);
 	      size = save_expr (size);
 	      tree index_type = build_index_type (size);
-	      tree atype = build_array_type (type, index_type);
+	      tree atype = build_array_type (TYPE_MAIN_VARIANT (type),
+					     index_type);
+	      atype = c_build_qualified_type (atype, TYPE_QUALS (type));
 	      tree ptype = build_pointer_type (type);
 	      if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE)
 		t = build_fold_addr_expr (t);
and then we're back to the above allocate-2.C issue, i.e. at -O0
we still print *&k rather than k.

And another question is if in case we punted because of the TBAA check
we shouldn't just force printing the access type, so never print
*&k but print instead *(access type)&k.

	Jakub

Re: [PATCH] c-family, v2: Improve MEM_REF printing for diagnostics [PR98597]

[Richard Biener]

On Thu, 14 Jan 2021, Jakub Jelinek wrote:

> On Thu, Jan 14, 2021 at 10:49:42AM -0700, Martin Sebor wrote:
> > > In the light of Martins patch this is probably reasonable but still
> > > the general direction is wrong (which is why I didn't approve Martins
> > > original patch).  I'm also somewhat disappointed we're breaking this
> > > so late in the cycle.
> > 
> > So am I.  I didn't test this change as exhaustively as I could and
> > (in light of the poor test coverage) should have.  That's my bad.
> > FWIW, I did do it for the first patch (by instrumenting GCC and
> > formatting every MEM_REF it came across), but it didn't occur to
> > me to do it this time around.  I have now completed this testing
> > (it found one more ICE elsewhere that I'll fix soon).
> 
> 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).
> 
> Is this ok for trunk if it passes bootstrap/regtest?

OK.

Thanks,
Richard.

> 2021-01-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/98597
> 	* c-pretty-print.c (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.
> 
> --- gcc/c-family/c-pretty-print.c.jj	2021-01-13 15:27:09.822834600 +0100
> +++ gcc/c-family/c-pretty-print.c	2021-01-14 19:02:21.299138891 +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
> @@ -1833,59 +1940,79 @@ print_mem_ref (c_pretty_printer *pp, tre
>    offset_int elt_idx = 0;
>    /* True to include a cast to char* (for a nonzero final BYTE_OFF).  */
>    bool char_cast = false;
> -  const bool addr = TREE_CODE (arg) == ADDR_EXPR;
> -  if (addr)
> +  tree op = NULL_TREE;
> +  bool array_ref_only = false;
> +  if (TREE_CODE (arg) == ADDR_EXPR)
>      {
> -      arg = TREE_OPERAND (arg, 0);
> -      if (byte_off == 0)
> +      op = c_fold_indirect_ref_for_warn (EXPR_LOCATION (e), TREE_TYPE (e),
> +					 TREE_OPERAND (arg, 0), byte_off);
> +      /* Try to fold it back to component, array ref or their combination,
> +	 but print it only if the types and TBAA types are compatible.  */
> +      if (op
> +	  && byte_off == 0
> +	  && lang_hooks.types_compatible_p (TREE_TYPE (e), TREE_TYPE (op))
> +	  && get_deref_alias_set (TREE_OPERAND (e, 1)) == get_alias_set (op))
>  	{
> -	  pp->expression (arg);
> +	  pp->expression (op);
>  	  return;
>  	}
> +      if (op == NULL_TREE)
> +	op = TREE_OPERAND (arg, 0);
> +      /* If the types or TBAA types are incompatible, undo the
> +	 UNION_TYPE handling from c_fold_indirect_ref_for_warn, and similarly
> +	 undo __real__/__imag__ the code below doesn't try to handle.  */
> +      if (op != TREE_OPERAND (arg, 0)
> +	  && ((TREE_CODE (op) == COMPONENT_REF
> +	       && TREE_CODE (TREE_TYPE (TREE_OPERAND (op, 0))) == UNION_TYPE)
> +	      || TREE_CODE (op) == REALPART_EXPR
> +	      || TREE_CODE (op) == IMAGPART_EXPR))
> +	op = TREE_OPERAND (op, 0);
> +      if (op != TREE_OPERAND (arg, 0))
> +	{
> +	  array_ref_only = true;
> +	  for (tree ref = op; ref != TREE_OPERAND (arg, 0);
> +	       ref = TREE_OPERAND (ref, 0))
> +	    if (TREE_CODE (ref) != ARRAY_REF)
> +	      {
> +		array_ref_only = false;
> +		break;
> +	      }
> +	}
>      }
>  
>    tree access_type = TREE_TYPE (e);
> -  if (TREE_CODE (access_type) == ARRAY_TYPE)
> -    access_type = TREE_TYPE (access_type);
> -  tree arg_type = TREE_TYPE (arg);
> -  if (POINTER_TYPE_P (arg_type))
> -    arg_type = TREE_TYPE (arg_type);
> -  if (TREE_CODE (arg_type) == ARRAY_TYPE)
> -    arg_type = TREE_TYPE (arg_type);
> +  tree arg_type = TREE_TYPE (TREE_TYPE (arg));
>    if (tree access_size = TYPE_SIZE_UNIT (access_type))
> -    if (TREE_CODE (access_size) == INTEGER_CST)
> +    if (byte_off != 0
> +	&& TREE_CODE (access_size) == INTEGER_CST
> +	&& !integer_zerop (access_size))
>        {
> -	/* For naturally aligned accesses print the nonzero offset
> -	   in units of the accessed type, in the form of an index.
> -	   For unaligned accesses also print the residual byte offset.  */
>  	offset_int asize = wi::to_offset (access_size);
> -	offset_int szlg2 = wi::floor_log2 (asize);
> -
> -	elt_idx = byte_off >> szlg2;
> -	byte_off = byte_off - (elt_idx << szlg2);
> +	elt_idx = byte_off / asize;
> +	byte_off = byte_off % asize;
>        }
>  
>    /* True to include a cast to the accessed type.  */
> -  const bool access_cast = VOID_TYPE_P (arg_type)
> -    || TYPE_MAIN_VARIANT (access_type) != TYPE_MAIN_VARIANT (arg_type);
> +  const bool access_cast
> +    = ((op && op != TREE_OPERAND (arg, 0))
> +       || VOID_TYPE_P (arg_type)
> +       || !lang_hooks.types_compatible_p (access_type, arg_type));
> +  const bool has_off = byte_off != 0 || (op && op != TREE_OPERAND (arg, 0));
>  
> -  if (byte_off != 0)
> +  if (has_off && (byte_off != 0 || !array_ref_only))
>      {
>        /* When printing the byte offset for a pointer to a type of
>  	 a different size than char, include a cast to char* first,
>  	 before printing the cast to a pointer to the accessed type.  */
> -      offset_int arg_size = 0;
> -      if (tree size = TYPE_SIZE (arg_type))
> -	arg_size = wi::to_offset (size);
> -      if (arg_size != BITS_PER_UNIT)
> +      tree size = TYPE_SIZE (arg_type);
> +      if (size == NULL_TREE
> +	  || TREE_CODE (size) != INTEGER_CST
> +	  || wi::to_wide (size) != BITS_PER_UNIT)
>  	char_cast = true;
>      }
>  
>    if (elt_idx == 0)
> -    {
> -      if (!addr)
> -	pp_c_star (pp);
> -    }
> +    pp_c_star (pp);
>    else if (access_cast || char_cast)
>      pp_c_left_paren (pp);
>  
> @@ -1895,25 +2022,63 @@ print_mem_ref (c_pretty_printer *pp, tre
>  	 with the type of the referenced object (or if the object
>  	 is typeless).  */
>        pp_c_left_paren (pp);
> -      pp->type_id (access_type);
> -      pp_c_star (pp);
> +      pp->type_id (build_pointer_type (access_type));
>        pp_c_right_paren (pp);
>      }
>  
> -  if (byte_off != 0)
> +  if (has_off)
>      pp_c_left_paren (pp);
>  
>    if (char_cast)
>      {
> -      /* Include a cast to char*.  */
> +      /* Include a cast to char *.  */
>        pp_c_left_paren (pp);
> -      pp->type_id (char_type_node);
> -      pp_c_star (pp);
> +      pp->type_id (string_type_node);
>        pp_c_right_paren (pp);
>      }
>  
>    pp->unary_expression (arg);
>  
> +  if (op && op != TREE_OPERAND (arg, 0))
> +    {
> +      auto_vec<tree, 16> refs;
> +      tree ref;
> +      unsigned i;
> +      bool array_refs = true;
> +      for (ref = op; ref != TREE_OPERAND (arg, 0); ref = TREE_OPERAND (ref, 0))
> +	refs.safe_push (ref);
> +      FOR_EACH_VEC_ELT_REVERSE (refs, i, ref)
> +	if (array_refs && TREE_CODE (ref) == ARRAY_REF)
> +	  {
> +	    pp_c_left_bracket (pp);
> +	    pp->expression (TREE_OPERAND (ref, 1));
> +	    pp_c_right_bracket (pp);
> +	  }
> +	else
> +	  {
> +	    if (array_refs)
> +	      {
> +		array_refs = false;
> +		pp_string (pp, " + offsetof");
> +		pp_c_left_paren (pp);
> +		pp->type_id (TREE_TYPE (TREE_OPERAND (ref, 0)));
> +		pp_comma (pp);
> +	      }
> +	    else if (TREE_CODE (ref) == COMPONENT_REF)
> +	      pp_c_dot (pp);
> +	    if (TREE_CODE (ref) == COMPONENT_REF)
> +	      pp->expression (TREE_OPERAND (ref, 1));
> +	    else
> +	      {
> +		pp_c_left_bracket (pp);
> +		pp->expression (TREE_OPERAND (ref, 1));
> +		pp_c_right_bracket (pp);
> +	      }
> +	  }
> +      if (!array_refs)
> +	pp_c_right_paren (pp);
> +    }
> +
>    if (byte_off != 0)
>      {
>        pp_space (pp);
> @@ -1921,25 +2086,20 @@ print_mem_ref (c_pretty_printer *pp, tre
>        pp_space (pp);
>        tree off = wide_int_to_tree (ssizetype, byte_off);
>        pp->constant (off);
> -      pp_c_right_paren (pp);
>      }
> +
> +  if (has_off)
> +    pp_c_right_paren (pp);
> +
>    if (elt_idx != 0)
>      {
>        if (access_cast || char_cast)
>  	pp_c_right_paren (pp);
>  
> -      if (addr)
> -	{
> -	  pp_space (pp);
> -	  pp_plus (pp);
> -	  pp_space (pp);
> -	}
> -      else
> -	pp_c_left_bracket (pp);
> +      pp_c_left_bracket (pp);
>        tree idx = wide_int_to_tree (ssizetype, elt_idx);
>        pp->constant (idx);
> -      if (!addr)
> -	pp_c_right_bracket (pp);
> +      pp_c_right_bracket (pp);
>      }
>  }
>  
> --- gcc/testsuite/gcc.dg/uninit-38.c.jj	2021-01-07 09:34:09.725641679 +0100
> +++ gcc/testsuite/gcc.dg/uninit-38.c	2021-01-14 17:43:33.442029629 +0100
> @@ -29,28 +29,28 @@ void sink (void*, ...);
>    }						\
>    typedef void dummy_type
>  
> -T (int, 0, 0);      // { dg-warning "'\\*\\(int\\*\\)p' is used uninitialized" }
> -T (int, 0, 1);      // { dg-warning "'\\*\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)'" }
> -T (int, 0, 2);      // { dg-warning "'\\*\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)'" }
> -T (int, 0, 3);      // { dg-warning "'\\*\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)'" }
> -T (int, 0, 4);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[1]'" }
> -T (int, 0, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[1]'" }
> -T (int, 0, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)\\)\\\[1]'" }
> -T (int, 0, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)\\)\\\[1]'" }
> -T (int, 0, 8);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[2]'" }
> -T (int, 0, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[2]'" }
> -
> -
> -T (int, 1, 0);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[1]' is used uninitialized" }
> -T (int, 1, 1);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[1]'" }
> -T (int, 1, 2);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)\\)\\\[1]'" }
> -T (int, 1, 3);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)\\)\\\[1]'" }
> -T (int, 1, 4);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[2]'" }
> -T (int, 1, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[2]'" }
> -T (int, 1, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 2\\)\\)\\\[2]'" }
> -T (int, 1, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 3\\)\\)\\\[2]'" }
> -T (int, 1, 8);      // { dg-warning "'\\(\\(int\\*\\)p\\)\\\[3]'" }
> -T (int, 1, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(\\(char\\*\\)p \\+ 1\\)\\)\\\[3]'" }
> +T (int, 0, 0);      // { dg-warning "'\\*\\(int \\*\\)p' is used uninitialized" }
> +T (int, 0, 1);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)'" }
> +T (int, 0, 2);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)'" }
> +T (int, 0, 3);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)'" }
> +T (int, 0, 4);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[1]'" }
> +T (int, 0, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[1]'" }
> +T (int, 0, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)\\)\\\[1]'" }
> +T (int, 0, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)\\)\\\[1]'" }
> +T (int, 0, 8);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[2]'" }
> +T (int, 0, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[2]'" }
> +
> +
> +T (int, 1, 0);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[1]' is used uninitialized" }
> +T (int, 1, 1);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[1]'" }
> +T (int, 1, 2);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)\\)\\\[1]'" }
> +T (int, 1, 3);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)\\)\\\[1]'" }
> +T (int, 1, 4);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[2]'" }
> +T (int, 1, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[2]'" }
> +T (int, 1, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 2\\)\\)\\\[2]'" }
> +T (int, 1, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 3\\)\\)\\\[2]'" }
> +T (int, 1, 8);      // { dg-warning "'\\(\\(int \\*\\)p\\)\\\[3]'" }
> +T (int, 1, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)p \\+ 1\\)\\)\\\[3]'" }
>  
>  #undef T
>  #define T(Type, idx, off)			\
> @@ -63,25 +63,25 @@ T (int, 1, 9);      // { dg-warning "'\\
>    }						\
>    typedef void dummy_type
>  
> -T (int, 0, 0);      // { dg-warning "'\\*\\(int\\*\\)a' is used uninitialized" }
> -T (int, 0, 1);      // { dg-warning "'\\*\\(int\\*\\)\\(a \\+ 1\\)'" }
> -T (int, 0, 2);      // { dg-warning "'\\*\\(int\\*\\)\\(a \\+ 2\\)'" }
> -T (int, 0, 3);      // { dg-warning "'\\*\\(int\\*\\)\\(a \\+ 3\\)'" }
> -T (int, 0, 4);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[1]'" }
> -T (int, 0, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[1]'" }
> -T (int, 0, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 2\\)\\)\\\[1]'" }
> -T (int, 0, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 3\\)\\)\\\[1]'" }
> -T (int, 0, 8);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[2]'" }
> -T (int, 0, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[2]'" }
> -
> -
> -T (int, 1, 0);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[1]' is used uninitialized" }
> -T (int, 1, 1);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[1]'" }
> -T (int, 1, 2);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 2\\)\\)\\\[1]'" }
> -T (int, 1, 3);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 3\\)\\)\\\[1]'" }
> -T (int, 1, 4);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[2]'" }
> -T (int, 1, 5);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[2]'" }
> -T (int, 1, 6);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 2\\)\\)\\\[2]'" }
> -T (int, 1, 7);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 3\\)\\)\\\[2]'" }
> -T (int, 1, 8);      // { dg-warning "'\\(\\(int\\*\\)a\\)\\\[3]'" }
> -T (int, 1, 9);      // { dg-warning "'\\(\\(int\\*\\)\\(a \\+ 1\\)\\)\\\[3]'" }
> +T (int, 0, 0);      // { dg-warning "'\\*\\(int \\*\\)a' is used uninitialized" }
> +T (int, 0, 1);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)'" }
> +T (int, 0, 2);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)'" }
> +T (int, 0, 3);      // { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)'" }
> +T (int, 0, 4);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[1]'" }
> +T (int, 0, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[1]'" }
> +T (int, 0, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)\\)\\\[1]'" }
> +T (int, 0, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)\\)\\\[1]'" }
> +T (int, 0, 8);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[2]'" }
> +T (int, 0, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[2]'" }
> +
> +
> +T (int, 1, 0);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[1]' is used uninitialized" }
> +T (int, 1, 1);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[1]'" }
> +T (int, 1, 2);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)\\)\\\[1]'" }
> +T (int, 1, 3);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)\\)\\\[1]'" }
> +T (int, 1, 4);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[2]'" }
> +T (int, 1, 5);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[2]'" }
> +T (int, 1, 6);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 2\\)\\)\\\[2]'" }
> +T (int, 1, 7);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 3\\)\\)\\\[2]'" }
> +T (int, 1, 8);      // { dg-warning "'\\(\\(int \\*\\)a\\)\\\[3]'" }
> +T (int, 1, 9);      // { dg-warning "'\\(\\(int \\*\\)\\(\\(char \\*\\)a \\+ 1\\)\\)\\\[3]'" }
> --- gcc/testsuite/gcc.dg/uninit-40.c.jj	2021-01-14 13:17:58.483432786 +0100
> +++ gcc/testsuite/gcc.dg/uninit-40.c	2021-01-14 19:03:08.039606122 +0100
> @@ -0,0 +1,50 @@
> +/* PR tree-optimization/98597 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wuninitialized" } */
> +
> +union U { double d; int i; float f; };
> +struct S { char a; int b; char c; unsigned d; union U e; int f[3]; unsigned g[3]; };
> +struct T { char t; struct S u; int v; };
> +typedef short V[2][2];
> +void baz (V *);
> +
> +static inline int
> +bar (char *p)
> +{
> +  return *(int *) p;
> +}
> +
> +void
> +foo (int *q)
> +{
> +  struct T t;
> +  t.t = 1;
> +  t.u.c = 2;
> +  char *pt = (char *) &t;
> +  q[0] = bar (pt + __builtin_offsetof (struct T, u.b));	/* { dg-warning "'t\\.u\\.b' is used uninitialized" } */
> +  q[1] = bar (pt + __builtin_offsetof (struct T, u.e));	/* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&t \\+ offsetof\\(struct T, u\\.e\\)\\)' is used uninitialized" } */
> +  q[2] = bar (pt + __builtin_offsetof (struct T, v));	/* { dg-warning "'t\\.v' is used uninitialized" } */
> +  q[3] = bar (pt + __builtin_offsetof (struct T, u.d));	/* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&t \\+ offsetof\\(struct T, u\\.d\\)\\)' is used uninitialized" } */
> +  q[4] = bar (pt + __builtin_offsetof (struct T, u.f[2])); /* { dg-warning "'t\\.u\\.f\\\[2\\\]' is used uninitialized" } */
> +  q[5] = bar (pt + __builtin_offsetof (struct T, u.g[2])); /* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&t \\+ offsetof\\(struct T, u\\.g\\\[2\\\]\\)\\)' is used uninitialized" } */
> +  int s[3];
> +  s[0] = 1;
> +  char *ps = (char *) s;
> +  q[6] = bar (ps + sizeof (int));			/* { dg-warning "'s\\\[1\\\]' is used uninitialized" } */
> +  unsigned w[2][2];
> +  w[0][0] = 1;
> +  char *pw = (char *) w;
> +  q[7] = bar (pw + 3 * sizeof (unsigned));		/* { dg-warning "'\\*\\(int \\*\\)\\(&w\\\[1\\\]\\\[1\\\]\\)' is used uninitialized" } */
> +  struct T x[3][3];
> +  x[0][0].t = 1;
> +  char *px = (char *) x;
> +  q[8] = bar (px + 5 * sizeof (struct T) + __builtin_offsetof (struct T, u.b));	/* { dg-warning "'x\\\[1\\\]\\\[2\\\]\\.u\\.b' is used uninitialized" } */
> +  q[9] = bar (px + 6 * sizeof (struct T) + __builtin_offsetof (struct T, u.d));	/* { dg-warning "'\\*\\(int \\*\\)\\(\\(char \\*\\)&x\\\[2\\\]\\\[0\\\] \\+ offsetof\\(struct T, u\\.d\\)\\)' is used uninitialized" } */
> +#if defined(__i386__) || defined(__x86_64__)
> +  /* memcpy folding is too target dependent to test it everywhere.  */
> +  V u[2], v[2];
> +  u[0][0][0] = 1;
> +  __builtin_memcpy (&v[1], &u[1], sizeof (V));		/* { dg-warning "'\\*\\(\(long \)?long unsigned int \\*\\)\\(&u\\\[1\\\]\\\[0\\\]\\\[0\\\]\\)' is used uninitialized" "" { target i?86-*-* x86_64-*-* } } */
> +  baz (&v[1]);
> +#endif
> +}
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH] c-family, v2: Improve MEM_REF printing for diagnostics [PR98597]

[Richard Biener]

On Fri, 15 Jan 2021, Jakub Jelinek wrote:

> On Thu, Jan 14, 2021 at 07:26:36PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > Is this ok for trunk if it passes bootstrap/regtest?
> 
> So, x86_64-linux bootstrap unfortunately broke due to the -march=i486
> changes, but at least i686-linux bootstrap succeeded and shows 2
> regressions.
> 
> One is on g++.dg/gomp/allocate-2.C, which used to print:
> allocate-2.C:9:36: error: user defined reduction not found for ‘s’
> but now prints:
> allocate-2.C:9:36: error: user defined reduction not found for ‘*&s’
> because of -O0 and therefore -fno-strict-aliasing.
> The problem is that for !flag_strict_aliasing get_deref_alias_set returns 0
> and so the:
> && get_deref_alias_set (TREE_OPERAND (e, 1)) == get_alias_set (op)
> check fails.  So, shall the code use
> && (!flag_no_strict_aliasing
>     || get_deref_alias_set (TREE_OPERAND (e, 1)) == get_alias_set (op))
> instead, or
> get_alias_set (TREE_TYPE (TREE_TYPE (TREE_OPERAND (e, 1))))
> == get_alias_set (op)
> ?

Elsewhere we use

      tree decl = TREE_OPERAND (TREE_OPERAND (*t, 0), 0);
      tree alias_type = TREE_TYPE (TREE_OPERAND (*t, 1));
...
          /* Same TBAA behavior with -fstrict-aliasing.  */
          && !TYPE_REF_CAN_ALIAS_ALL (alias_type)
          && (TYPE_MAIN_VARIANT (TREE_TYPE (decl))
              == TYPE_MAIN_VARIANT (TREE_TYPE (alias_type)))

to guard eliding of the MEM_REF.  So maybe use this form which doesn't
depend on alias sets.

> The other is on gcc.dg/gomp/_Atomic-3.c test, where we used to print
> _Atomic-3.c:22:34: error: ‘_Atomic’ ‘k’ in ‘reduction’ clause
> but now print
> _Atomic-3.c:22:34: error: ‘_Atomic’ ‘*(_Atomic int (*)[4])(&k[0])’ in ‘reduction’ clause
> Apparently in this case the C FE considers the two _Atomic int [4] types
> incompatible, one is created through
> c_build_qualified_type (type=<array_type 0x7fffea186a80>, type_quals=8, orig_qual_type=<tree 0x0>, orig_qual_indirect=1)
> on an int [4] type, i.e. adding _Atomic qualifier to an unqualified array
> type, and the other is created through
> build_array_type (elt_type=<integer_type 0x7fffea186540 int>, index_type=<integer_type 0x7fffea186498>, typeless_storage=false)
> i.e. creating an array with _Atomic int elements.
> That seems like a C FE bug to me.
> 
> Anyway, I can fix or workaround that by:
> --- gcc/c/c-typeck.c.jj	2021-01-04 10:25:49.651111329 +0100
> +++ gcc/c/c-typeck.c	2021-01-15 09:53:29.590611264 +0100
> @@ -13979,7 +13979,9 @@ c_finish_omp_clauses (tree clauses, enum
>  	      size = size_binop (MINUS_EXPR, size, size_one_node);
>  	      size = save_expr (size);
>  	      tree index_type = build_index_type (size);
> -	      tree atype = build_array_type (type, index_type);
> +	      tree atype = build_array_type (TYPE_MAIN_VARIANT (type),
> +					     index_type);
> +	      atype = c_build_qualified_type (atype, TYPE_QUALS (type));
>  	      tree ptype = build_pointer_type (type);
>  	      if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE)
>  		t = build_fold_addr_expr (t);
> and then we're back to the above allocate-2.C issue, i.e. at -O0
> we still print *&k rather than k.
> 
> And another question is if in case we punted because of the TBAA check
> we shouldn't just force printing the access type, so never print
> *&k but print instead *(access type)&k.

I guess so.

As said, I'm not a fan of too much magic here.  A MEM_REF is what it is...

Richard.