[PATCH] correct the representation of ADDR_EXPR involving pointer to array [PR 90694]

[PATCH] correct the representation of ADDR_EXPR involving pointer to array [PR 90694]

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

Given a poiner to array p, tree dumps for expressions like &(*p)[2]
actually show &*p[2].  That's not right -- the parentheses are
important to differentiate indexing into the array the pointer
points to from indexing into the pointer.

The attached patch adjusts the tree pretty printer to add the parens
when the pointer points to an array.

Tested on x86_64-linux.

Martin

[-- Attachment #2: gcc-90694.diff --]
[-- Type: text/x-patch, Size: 1846 bytes --]

PR middle-end/90694 - incorrect representation of ADDR_EXPR involving a pointer to array

gcc/ChangeLog:

	PR middle-end/90694
	* tree-pretty-print.c (dump_generic_node): Add parentheses.

gcc/testsuite/ChangeLog:

	PR middle-end/90694
	* gcc.dg/tree-ssa/dump-5.c: New test.

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dump-5.c b/gcc/testsuite/gcc.dg/tree-ssa/dump-5.c
new file mode 100644
index 00000000000..6807b5e9ef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dump-5.c
@@ -0,0 +1,15 @@
+/* PR middle-end/90694 - incorrect representation of ADDR_EXPR involving
+   a pointer to array
+   { dg-do compile }
+   { dg-options "-fdump-tree-original" } */
+
+typedef char A8[8];
+
+unsigned f (A8 *pa)
+{
+  return __builtin_strlen (&(*pa)[2]);
+}
+
+/* Veriy the expression is correct in the dump:
+  { dg-final { scan-tree-dump-not "\\\&\\\*pa\\\[2\\\]" "original" } }
+  { dg-final { scan-tree-dump "\\\&\\\(\\\*pa\\\)\\\[2\\\]" "original" } } */
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 6645a646617..30d1d65e6bc 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1676,9 +1676,17 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
 	  {
 	    if (TREE_CODE (TREE_OPERAND (node, 0)) != ADDR_EXPR)
 	      {
+		/* Enclose pointers to arrays in parentheses.  */
+		tree op0 = TREE_OPERAND (node, 0);
+		tree op0type = TREE_TYPE (op0);
+		if (POINTER_TYPE_P (op0type)
+		    && TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
+		  pp_left_paren (pp);
 		pp_star (pp);
-		dump_generic_node (pp, TREE_OPERAND (node, 0),
-				   spc, flags, false);
+		dump_generic_node (pp, op0, spc, flags, false);
+		if (POINTER_TYPE_P (op0type)
+		    && TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
+		  pp_right_paren (pp);
 	      }
 	    else
 	      dump_generic_node (pp,

Re: [PATCH] correct the representation of ADDR_EXPR involving pointer to array [PR 90694]

[Jeff Law]

On 5/31/19 1:56 PM, Martin Sebor wrote:
> Given a poiner to array p, tree dumps for expressions like &(*p)[2]
> actually show &*p[2].  That's not right -- the parentheses are
> important to differentiate indexing into the array the pointer
> points to from indexing into the pointer.
> 
> The attached patch adjusts the tree pretty printer to add the parens
> when the pointer points to an array.
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> gcc-90694.diff
> 
> PR middle-end/90694 - incorrect representation of ADDR_EXPR involving a pointer to array
> 
> gcc/ChangeLog:
> 
> 	PR middle-end/90694
> 	* tree-pretty-print.c (dump_generic_node): Add parentheses.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR middle-end/90694
> 	* gcc.dg/tree-ssa/dump-5.c: New test.
OK.  I'm going to assume that the gimple parser already does the right
thing since it's supposed to already handle C expressions correctly.

Jeff

Re: [PATCH] correct the representation of ADDR_EXPR involving pointer to array [PR 90694]

[Richard Biener]

On Fri, May 31, 2019 at 10:50 PM Jeff Law <law@redhat.com> wrote:
>
> On 5/31/19 1:56 PM, Martin Sebor wrote:
> > Given a poiner to array p, tree dumps for expressions like &(*p)[2]
> > actually show &*p[2].  That's not right -- the parentheses are
> > important to differentiate indexing into the array the pointer
> > points to from indexing into the pointer.
> >
> > The attached patch adjusts the tree pretty printer to add the parens
> > when the pointer points to an array.
> >
> > Tested on x86_64-linux.
> >
> > Martin
> >
> > gcc-90694.diff
> >
> > PR middle-end/90694 - incorrect representation of ADDR_EXPR involving a pointer to array
> >
> > gcc/ChangeLog:
> >
> >       PR middle-end/90694
> >       * tree-pretty-print.c (dump_generic_node): Add parentheses.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       PR middle-end/90694
> >       * gcc.dg/tree-ssa/dump-5.c: New test.
> OK.  I'm going to assume that the gimple parser already does the right
> thing since it's supposed to already handle C expressions correctly.

-gimple dumping doesn't elide dumping MEM_REFs to plain * so you
see

  _1 = &__MEM <char[8]> (pa)[2];
  _2 = __builtin_strlen (_1);

I think the GIMPLE FE accepts *p as dereference in source, sth I
should eventually fix (it likewise accepts ->).  It then just does
what the C frontend does since it shares most of its parsing.

Richard.

> Jeff