Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-07-09 Thread Martin Sebor via Gcc-patches

On 6/29/20 1:19 AM, Richard Biener wrote:

On Mon, Jun 29, 2020 at 1:08 AM Martin Sebor  wrote:


On 6/23/20 1:12 AM, Richard Biener wrote:

On Tue, Jun 23, 2020 at 12:22 AM Martin Sebor via Gcc-patches
 wrote:


On 6/22/20 12:55 PM, Jason Merrill wrote:

On 6/22/20 1:25 PM, Martin Sebor wrote:

The attached fix parallels the one for the equivalent C bug 95580
where the pretty printers don't correctly handle MEM_REF arguments
with type void* or other pointers to an incomplete type.

The incorrect handling was exposed by the recent change to
-Wuninitialized which includes such expressions in diagnostics.



+if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
+  if (!integer_onep (size))
+{
+  pp_cxx_left_paren (pp);
+  dump_type (pp, ptr_type_node, flags);
+  pp_cxx_right_paren (pp);
+}


Don't we want to print the cast if the pointer target type is incomplete?


I suppose, yes, although after some more testing I think what should
be output is the type of the access.  The target pointer type isn't
meaningful (at least not in this case).

Here's what the warning looks like in C for the test case in
gcc.dg/pr95580.c:

 warning: ‘*((void *)(p)+1)’ may be used uninitialized

and like this in C++:

 warning: ‘*(p +1)’ may be used uninitialized

The +1 is a byte offset, which is correct given that incrementing
a void* in GCC is the same as adding 1 to the byte address, but
dereferencing a void* doesn't correspond to what's going on in
the source.

Even for a complete type (with size greater than 1), printing
the type of the argument plus a byte offset is wrong.  It ends
up with this for the C++ test case from 95768:

 warning: ‘*((int*) +4)’ is used uninitialized

when the access is actually ‘*((int*) +1)’

So it seems to me for MEM_REF, to make the output meaningful,
it's the type of the access (i.e., the MEM_REF type) that should
be printed here, and the offset should either be in elements of
the accessed type, i.e.,

 warning: ‘*((int*) +1)’ is used uninitialized

or, if the access is misaligned, the argument should first be
cast to char*, the offset added, and the result then cast to
the access type, like this:

 warning: ‘*(T*)((char*) +1)’ is used uninitialized

The attached revised and less than fully tested patch implements
this for C++ only for now.  If we agree on this approach I'll see
about making the corresponding change in C.


Note that there is no C/C++ way of fully expressing MEM_REF
semantics.  __MEM  ((T *)p + 1) is not actually
*(int *)((char *)p + 1) because that does not reflect that the
effective type of the lvalue when TBAA is concerned is 'T'
rather than 'int'.


What form would you say is closest to the C/C++ semantics, or
likely the most useful to users, that GCC could print instead?


Hmm, I'd try *() maybe?  Because there's
no C/C++ that can express what GIMPLE can do here.  Of course
pattern matching the exact cases we can handle like your patch
is an improvement (but as said the TBAA issue is still present).


"pointer derived from" would be misleading because of C++ class
derivation.  But more important, I think the output needs to
reflect what the warning actually is based on.  Leaving out
salient details like types or offsets from warnings about
out-of-bounds accesses makes analyzing them more difficult,
both for users and for us during initial triage.


Note for MEM_REF the offset is always
a constant byte offset but it indeed does not have to be a
multiple of the MEM_REF type size.

I wonder whether printing the MEM_REF in full provides
any real diagnostic value in the more "obfuscated" cases.


I'm not sure what obfuscated cases you're thinking of, or what
you mean by printing it in full.


I think that printing ‘*(T*)((char*) +1)’ is likely going
to confuse users because they cannot match this to a source
location.  If we have a source location we should have caret
diagnostics.


  I instrumented the code to
print every MEM_REF in that comes up in warn_uninitialized_vars
and rebuilt GCC.  There are 17,456 distinct instances so I didn't
review them all but those I did look at all look reasonable.
Probably the least useful are those that mention  by
itself (i.e.,  or *).  Those with an offset
are more informative (e.g., *((access**) +1).  In
a few the offset is very large, such as *((unsigned int*)sp
+4611686018427387900), but that doesn't seem like a problem.
I'd be happy to share the result.


Here +4611686018427387900 should be printed as -4, MEM_REF
offsets are to be interpreted as signed.


Sure, converting the offset to signed sounds like a good idea.





I'd also not print  but .


I also don't find  helpful, but I don't see 
as an improvement.  I think printing the SSA variable would be
more informative here since its name is usually related to
the variable it was derived from in the source.  But making that
change (or any other like it) feels like too much feature 

Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-06-29 Thread Richard Biener via Gcc-patches
On Mon, Jun 29, 2020 at 1:08 AM Martin Sebor  wrote:
>
> On 6/23/20 1:12 AM, Richard Biener wrote:
> > On Tue, Jun 23, 2020 at 12:22 AM Martin Sebor via Gcc-patches
> >  wrote:
> >>
> >> On 6/22/20 12:55 PM, Jason Merrill wrote:
> >>> On 6/22/20 1:25 PM, Martin Sebor wrote:
>  The attached fix parallels the one for the equivalent C bug 95580
>  where the pretty printers don't correctly handle MEM_REF arguments
>  with type void* or other pointers to an incomplete type.
> 
>  The incorrect handling was exposed by the recent change to
>  -Wuninitialized which includes such expressions in diagnostics.
> >>>
>  +if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
>  +  if (!integer_onep (size))
>  +{
>  +  pp_cxx_left_paren (pp);
>  +  dump_type (pp, ptr_type_node, flags);
>  +  pp_cxx_right_paren (pp);
>  +}
> >>>
> >>> Don't we want to print the cast if the pointer target type is incomplete?
> >>
> >> I suppose, yes, although after some more testing I think what should
> >> be output is the type of the access.  The target pointer type isn't
> >> meaningful (at least not in this case).
> >>
> >> Here's what the warning looks like in C for the test case in
> >> gcc.dg/pr95580.c:
> >>
> >> warning: ‘*((void *)(p)+1)’ may be used uninitialized
> >>
> >> and like this in C++:
> >>
> >> warning: ‘*(p +1)’ may be used uninitialized
> >>
> >> The +1 is a byte offset, which is correct given that incrementing
> >> a void* in GCC is the same as adding 1 to the byte address, but
> >> dereferencing a void* doesn't correspond to what's going on in
> >> the source.
> >>
> >> Even for a complete type (with size greater than 1), printing
> >> the type of the argument plus a byte offset is wrong.  It ends
> >> up with this for the C++ test case from 95768:
> >>
> >> warning: ‘*((int*) +4)’ is used uninitialized
> >>
> >> when the access is actually ‘*((int*) +1)’
> >>
> >> So it seems to me for MEM_REF, to make the output meaningful,
> >> it's the type of the access (i.e., the MEM_REF type) that should
> >> be printed here, and the offset should either be in elements of
> >> the accessed type, i.e.,
> >>
> >> warning: ‘*((int*) +1)’ is used uninitialized
> >>
> >> or, if the access is misaligned, the argument should first be
> >> cast to char*, the offset added, and the result then cast to
> >> the access type, like this:
> >>
> >> warning: ‘*(T*)((char*) +1)’ is used uninitialized
> >>
> >> The attached revised and less than fully tested patch implements
> >> this for C++ only for now.  If we agree on this approach I'll see
> >> about making the corresponding change in C.
> >
> > Note that there is no C/C++ way of fully expressing MEM_REF
> > semantics.  __MEM  ((T *)p + 1) is not actually
> > *(int *)((char *)p + 1) because that does not reflect that the
> > effective type of the lvalue when TBAA is concerned is 'T'
> > rather than 'int'.
>
> What form would you say is closest to the C/C++ semantics, or
> likely the most useful to users, that GCC could print instead?

Hmm, I'd try *() maybe?  Because there's
no C/C++ that can express what GIMPLE can do here.  Of course
pattern matching the exact cases we can handle like your patch
is an improvement (but as said the TBAA issue is still present).

> > Note for MEM_REF the offset is always
> > a constant byte offset but it indeed does not have to be a
> > multiple of the MEM_REF type size.
> >
> > I wonder whether printing the MEM_REF in full provides
> > any real diagnostic value in the more "obfuscated" cases.
>
> I'm not sure what obfuscated cases you're thinking of, or what
> you mean by printing it in full.

I think that printing ‘*(T*)((char*) +1)’ is likely going
to confuse users because they cannot match this to a source
location.  If we have a source location we should have caret
diagnostics.

>  I instrumented the code to
> print every MEM_REF in that comes up in warn_uninitialized_vars
> and rebuilt GCC.  There are 17,456 distinct instances so I didn't
> review them all but those I did look at all look reasonable.
> Probably the least useful are those that mention  by
> itself (i.e.,  or *).  Those with an offset
> are more informative (e.g., *((access**) +1).  In
> a few the offset is very large, such as *((unsigned int*)sp
> +4611686018427387900), but that doesn't seem like a problem.
> I'd be happy to share the result.

Here +4611686018427387900 should be printed as -4, MEM_REF
offsets are to be interpreted as signed.

> >
> > I'd also not print  but .
>
> I also don't find  helpful, but I don't see 
> as an improvement.  I think printing the SSA variable would be
> more informative here since its name is usually related to
> the variable it was derived from in the source.  But making that
> change (or any other like it) feels like too much feature creep
> for this fix.  I'd be happy to do it in a follow up if we 

Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-06-28 Thread Martin Sebor via Gcc-patches

On 6/23/20 1:12 AM, Richard Biener wrote:

On Tue, Jun 23, 2020 at 12:22 AM Martin Sebor via Gcc-patches
 wrote:


On 6/22/20 12:55 PM, Jason Merrill wrote:

On 6/22/20 1:25 PM, Martin Sebor wrote:

The attached fix parallels the one for the equivalent C bug 95580
where the pretty printers don't correctly handle MEM_REF arguments
with type void* or other pointers to an incomplete type.

The incorrect handling was exposed by the recent change to
-Wuninitialized which includes such expressions in diagnostics.



+if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
+  if (!integer_onep (size))
+{
+  pp_cxx_left_paren (pp);
+  dump_type (pp, ptr_type_node, flags);
+  pp_cxx_right_paren (pp);
+}


Don't we want to print the cast if the pointer target type is incomplete?


I suppose, yes, although after some more testing I think what should
be output is the type of the access.  The target pointer type isn't
meaningful (at least not in this case).

Here's what the warning looks like in C for the test case in
gcc.dg/pr95580.c:

warning: ‘*((void *)(p)+1)’ may be used uninitialized

and like this in C++:

warning: ‘*(p +1)’ may be used uninitialized

The +1 is a byte offset, which is correct given that incrementing
a void* in GCC is the same as adding 1 to the byte address, but
dereferencing a void* doesn't correspond to what's going on in
the source.

Even for a complete type (with size greater than 1), printing
the type of the argument plus a byte offset is wrong.  It ends
up with this for the C++ test case from 95768:

warning: ‘*((int*) +4)’ is used uninitialized

when the access is actually ‘*((int*) +1)’

So it seems to me for MEM_REF, to make the output meaningful,
it's the type of the access (i.e., the MEM_REF type) that should
be printed here, and the offset should either be in elements of
the accessed type, i.e.,

warning: ‘*((int*) +1)’ is used uninitialized

or, if the access is misaligned, the argument should first be
cast to char*, the offset added, and the result then cast to
the access type, like this:

warning: ‘*(T*)((char*) +1)’ is used uninitialized

The attached revised and less than fully tested patch implements
this for C++ only for now.  If we agree on this approach I'll see
about making the corresponding change in C.


Note that there is no C/C++ way of fully expressing MEM_REF
semantics.  __MEM  ((T *)p + 1) is not actually
*(int *)((char *)p + 1) because that does not reflect that the
effective type of the lvalue when TBAA is concerned is 'T'
rather than 'int'.


What form would you say is closest to the C/C++ semantics, or
likely the most useful to users, that GCC could print instead?


Note for MEM_REF the offset is always
a constant byte offset but it indeed does not have to be a
multiple of the MEM_REF type size.

I wonder whether printing the MEM_REF in full provides
any real diagnostic value in the more "obfuscated" cases.


I'm not sure what obfuscated cases you're thinking of, or what
you mean by printing it in full.  I instrumented the code to
print every MEM_REF in that comes up in warn_uninitialized_vars
and rebuilt GCC.  There are 17,456 distinct instances so I didn't
review them all but those I did look at all look reasonable.
Probably the least useful are those that mention  by
itself (i.e.,  or *).  Those with an offset
are more informative (e.g., *((access**) +1).  In
a few the offset is very large, such as *((unsigned int*)sp
+4611686018427387900), but that doesn't seem like a problem.
I'd be happy to share the result.



I'd also not print  but .


I also don't find  helpful, but I don't see 
as an improvement.  I think printing the SSA variable would be
more informative here since its name is usually related to
the variable it was derived from in the source.  But making that
change (or any other like it) feels like too much feature creep
for this fix.  I'd be happy to do it in a follow up if we agree
it's a good idea.

Either way, please let me know if the patch is okay as is or,
if not, what type it should mention.

Martin


Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-06-23 Thread Richard Biener via Gcc-patches
On Tue, Jun 23, 2020 at 12:22 AM Martin Sebor via Gcc-patches
 wrote:
>
> On 6/22/20 12:55 PM, Jason Merrill wrote:
> > On 6/22/20 1:25 PM, Martin Sebor wrote:
> >> The attached fix parallels the one for the equivalent C bug 95580
> >> where the pretty printers don't correctly handle MEM_REF arguments
> >> with type void* or other pointers to an incomplete type.
> >>
> >> The incorrect handling was exposed by the recent change to
> >> -Wuninitialized which includes such expressions in diagnostics.
> >
> >> +if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
> >> +  if (!integer_onep (size))
> >> +{
> >> +  pp_cxx_left_paren (pp);
> >> +  dump_type (pp, ptr_type_node, flags);
> >> +  pp_cxx_right_paren (pp);
> >> +}
> >
> > Don't we want to print the cast if the pointer target type is incomplete?
>
> I suppose, yes, although after some more testing I think what should
> be output is the type of the access.  The target pointer type isn't
> meaningful (at least not in this case).
>
> Here's what the warning looks like in C for the test case in
> gcc.dg/pr95580.c:
>
>warning: ‘*((void *)(p)+1)’ may be used uninitialized
>
> and like this in C++:
>
>warning: ‘*(p +1)’ may be used uninitialized
>
> The +1 is a byte offset, which is correct given that incrementing
> a void* in GCC is the same as adding 1 to the byte address, but
> dereferencing a void* doesn't correspond to what's going on in
> the source.
>
> Even for a complete type (with size greater than 1), printing
> the type of the argument plus a byte offset is wrong.  It ends
> up with this for the C++ test case from 95768:
>
>warning: ‘*((int*) +4)’ is used uninitialized
>
> when the access is actually ‘*((int*) +1)’
>
> So it seems to me for MEM_REF, to make the output meaningful,
> it's the type of the access (i.e., the MEM_REF type) that should
> be printed here, and the offset should either be in elements of
> the accessed type, i.e.,
>
>warning: ‘*((int*) +1)’ is used uninitialized
>
> or, if the access is misaligned, the argument should first be
> cast to char*, the offset added, and the result then cast to
> the access type, like this:
>
>warning: ‘*(T*)((char*) +1)’ is used uninitialized
>
> The attached revised and less than fully tested patch implements
> this for C++ only for now.  If we agree on this approach I'll see
> about making the corresponding change in C.

Note that there is no C/C++ way of fully expressing MEM_REF
semantics.  __MEM  ((T *)p + 1) is not actually
*(int *)((char *)p + 1) because that does not reflect that the
effective type of the lvalue when TBAA is concerned is 'T'
rather than 'int'.  Note for MEM_REF the offset is always
a constant byte offset but it indeed does not have to be a
multiple of the MEM_REF type size.

I wonder whether printing the MEM_REF in full provides
any real diagnostic value in the more "obfuscated" cases.

I'd also not print  but .

Richard.

> Martin


Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-06-22 Thread Martin Sebor via Gcc-patches

On 6/22/20 12:55 PM, Jason Merrill wrote:

On 6/22/20 1:25 PM, Martin Sebor wrote:

The attached fix parallels the one for the equivalent C bug 95580
where the pretty printers don't correctly handle MEM_REF arguments
with type void* or other pointers to an incomplete type.

The incorrect handling was exposed by the recent change to
-Wuninitialized which includes such expressions in diagnostics.



+    if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
+  if (!integer_onep (size))
+    {
+  pp_cxx_left_paren (pp);
+  dump_type (pp, ptr_type_node, flags);
+  pp_cxx_right_paren (pp);
+    }


Don't we want to print the cast if the pointer target type is incomplete?


I suppose, yes, although after some more testing I think what should
be output is the type of the access.  The target pointer type isn't
meaningful (at least not in this case).

Here's what the warning looks like in C for the test case in
gcc.dg/pr95580.c:

  warning: ‘*((void *)(p)+1)’ may be used uninitialized

and like this in C++:

  warning: ‘*(p +1)’ may be used uninitialized

The +1 is a byte offset, which is correct given that incrementing
a void* in GCC is the same as adding 1 to the byte address, but
dereferencing a void* doesn't correspond to what's going on in
the source.

Even for a complete type (with size greater than 1), printing
the type of the argument plus a byte offset is wrong.  It ends
up with this for the C++ test case from 95768:

  warning: ‘*((int*) +4)’ is used uninitialized

when the access is actually ‘*((int*) +1)’

So it seems to me for MEM_REF, to make the output meaningful,
it's the type of the access (i.e., the MEM_REF type) that should
be printed here, and the offset should either be in elements of
the accessed type, i.e.,

  warning: ‘*((int*) +1)’ is used uninitialized

or, if the access is misaligned, the argument should first be
cast to char*, the offset added, and the result then cast to
the access type, like this:

  warning: ‘*(T*)((char*) +1)’ is used uninitialized

The attached revised and less than fully tested patch implements
this for C++ only for now.  If we agree on this approach I'll see
about making the corresponding change in C.

Martin
PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage

gcc/cp/ChangeLog:

	PR c++/95768
	* error.c (dump_expr): Handle sizeless operand types such as void*.

gcc/testsuite/ChangeLog:

	PR c++/95768
	* g++.dg/pr95768.C: New test.

diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 0d6375e5e14..ea1f3232e3d 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -2374,32 +2374,63 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
   break;
 
 case MEM_REF:
-  if (TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR
-	  && integer_zerop (TREE_OPERAND (t, 1)))
-	dump_expr (pp, TREE_OPERAND (TREE_OPERAND (t, 0), 0), flags);
-  else
-	{
-	  pp_cxx_star (pp);
-	  if (!integer_zerop (TREE_OPERAND (t, 1)))
-	{
-	  pp_cxx_left_paren (pp);
-	  if (!integer_onep (TYPE_SIZE_UNIT
- (TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0))
-		{
-		  pp_cxx_left_paren (pp);
-		  dump_type (pp, ptr_type_node, flags);
-		  pp_cxx_right_paren (pp);
-		}
-	}
-	  dump_expr (pp, TREE_OPERAND (t, 0), flags);
-	  if (!integer_zerop (TREE_OPERAND (t, 1)))
-	{
-	  pp_cxx_ws_string (pp, "+");
-	  dump_expr (pp, fold_convert (ssizetype, TREE_OPERAND (t, 1)),
- flags);
-	  pp_cxx_right_paren (pp);
-	}
-	}
+  {
+	tree type = TREE_TYPE (t);
+	tree arg = TREE_OPERAND (t, 0);
+	tree offset = TREE_OPERAND (t, 1);
+	bool zero_offset = integer_zerop (offset);
+
+	if (TREE_CODE (arg) == ADDR_EXPR && zero_offset)
+	  dump_expr (pp, TREE_OPERAND (arg, 0), flags);
+	else
+	  {
+	pp_cxx_star (pp);
+	if (!zero_offset)
+	  {
+		pp_cxx_left_paren (pp);
+		pp_cxx_left_paren (pp);
+		dump_type (pp, type, flags);
+		pp_cxx_star (pp);
+		pp_cxx_right_paren (pp);
+
+		bool byte_offset = true;
+
+		if (tree size = TYPE_SIZE_UNIT (type))
+		  {
+		/* For naturally aligned accesses print the nonzero
+		   offset in units of the access type.  For unaligned
+		   accesses print a byte offset instead.  */
+		offset_int wsiz = wi::to_offset (size);
+		offset_int woff = wi::to_offset (offset);
+		offset_int szlg2 = wi::floor_log2 (wsiz);
+		offset_int eltoff = woff >> szlg2;
+		if (eltoff << szlg2 == woff)
+		  {
+			offset = wide_int_to_tree (ssizetype, eltoff);
+			byte_offset = false;
+		  }
+		  }
+
+		if (byte_offset)
+		  {
+		/* When printing the byte offset include a cast to
+		   a character type first, before printing the cast
+		   to the access type.  */
+		pp_cxx_left_paren (pp);
+		dump_type (pp, char_type_node, flags);
+		pp_cxx_star (pp);
+		pp_cxx_right_paren (pp);
+		  }
+	  }
+	dump_expr (pp, arg, flags);
+	if (!zero_offset)
+	  {
+		pp_cxx_ws_string 

Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-06-22 Thread Jason Merrill via Gcc-patches

On 6/22/20 1:25 PM, Martin Sebor wrote:

The attached fix parallels the one for the equivalent C bug 95580
where the pretty printers don't correctly handle MEM_REF arguments
with type void* or other pointers to an incomplete type.

The incorrect handling was exposed by the recent change to
-Wuninitialized which includes such expressions in diagnostics.



+   if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
+ if (!integer_onep (size))
+   {
+ pp_cxx_left_paren (pp);
+ dump_type (pp, ptr_type_node, flags);
+ pp_cxx_right_paren (pp);
+   }


Don't we want to print the cast if the pointer target type is incomplete?

Jason



[PATCH] handle MEM_REF with void* arguments (PR c++/95768)

2020-06-22 Thread Martin Sebor via Gcc-patches

The attached fix parallels the one for the equivalent C bug 95580
where the pretty printers don't correctly handle MEM_REF arguments
with type void* or other pointers to an incomplete type.

The incorrect handling was exposed by the recent change to
-Wuninitialized which includes such expressions in diagnostics.

Martin
PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage

gcc/cp/ChangeLog:

	PR c++/95768
	* error.c (dump_expr): Handle sizeless operand types such as void*.

gcc/testsuite/ChangeLog:

	PR c++/95768
	* g++.dg/pr95768.C: New test.

diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 0d6375e5e14..3a7254fdce1 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -2374,32 +2374,37 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
   break;
 
 case MEM_REF:
-  if (TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR
-	  && integer_zerop (TREE_OPERAND (t, 1)))
-	dump_expr (pp, TREE_OPERAND (TREE_OPERAND (t, 0), 0), flags);
-  else
-	{
-	  pp_cxx_star (pp);
-	  if (!integer_zerop (TREE_OPERAND (t, 1)))
-	{
-	  pp_cxx_left_paren (pp);
-	  if (!integer_onep (TYPE_SIZE_UNIT
- (TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0))
-		{
-		  pp_cxx_left_paren (pp);
-		  dump_type (pp, ptr_type_node, flags);
-		  pp_cxx_right_paren (pp);
-		}
-	}
-	  dump_expr (pp, TREE_OPERAND (t, 0), flags);
-	  if (!integer_zerop (TREE_OPERAND (t, 1)))
-	{
-	  pp_cxx_ws_string (pp, "+");
-	  dump_expr (pp, fold_convert (ssizetype, TREE_OPERAND (t, 1)),
- flags);
-	  pp_cxx_right_paren (pp);
-	}
-	}
+  {
+	tree arg = TREE_OPERAND (t, 0);
+	tree offset = TREE_OPERAND (t, 1);
+
+	if (TREE_CODE (arg) == ADDR_EXPR
+	&& integer_zerop (offset))
+	  dump_expr (pp, TREE_OPERAND (arg, 0), flags);
+	else
+	  {
+	pp_cxx_star (pp);
+	if (!integer_zerop (offset))
+	  {
+		pp_cxx_left_paren (pp);
+		tree argtype = TREE_TYPE (arg);
+		if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
+		  if (!integer_onep (size))
+		{
+		  pp_cxx_left_paren (pp);
+		  dump_type (pp, ptr_type_node, flags);
+		  pp_cxx_right_paren (pp);
+		}
+	  }
+	dump_expr (pp, arg, flags);
+	if (!integer_zerop (offset))
+	  {
+		pp_cxx_ws_string (pp, "+");
+		dump_expr (pp, fold_convert (ssizetype, offset), flags);
+		pp_cxx_right_paren (pp);
+	  }
+	  }
+  }
   break;
 
 case NEGATE_EXPR:
diff --git a/gcc/testsuite/g++.dg/pr95768.C b/gcc/testsuite/g++.dg/pr95768.C
new file mode 100644
index 000..babfcb49bf8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr95768.C
@@ -0,0 +1,32 @@
+/* PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+extern "C" void *malloc (__SIZE_TYPE__);
+
+struct f
+{
+  int i;
+  static int e (int);
+  void operator= (int) { e (i); }
+};
+
+struct m {
+  int i;
+  f length;
+};
+
+struct n {
+  m *o() { return (m *)this; }
+};
+
+struct p {
+  n *header;
+  p () {
+header = (n *)malloc (0);
+m b = *header->o();   // { dg-warning "-Wuninitialized" }
+b.length = 0;
+  }
+};
+
+void detach2() { p(); }