[PATCH] Fix recent DEBUG_BEGIN_STMT related regressions (PR debug/83547)

[PATCH] Fix recent DEBUG_BEGIN_STMT related regressions (PR debug/83547)

[Jakub Jelinek]

Hi!

The recent change to clear TREE_SIDE_EFFECTS on STATEMENT_LIST containing
DEBUG_BEGIN_STMTs and a single other statement without TREE_SIDE_EFFECTS on
it breaks the C stmt expr handling.  The problem is that it assumes if
TREE_SIDE_EFFECTS is clear on a STATEMENT_LIST then that means the
STATEMENT_LIST is empty, i.e. ({ }), which is no longer the case.

Fixed by skipping over DEBUG_BEGIN_STMTs from the tail to find the last
non-DEBUG_BEGIN_STMT stmt (if any, and if none, treat it like ({ }) ),
and also not processing the last non-DEBUG_BEGIN_STMT for -Wunused-value
if it is followed by DEBUG_BEGIN_STMTs.

In addition to this, the patch includes a fix from my earlier patch, where
alloc_stmt_list TREE_SIDE_EFFECTS bit is inconsistent, depending on whether
it is reused from the cache (then TREE_SIDE_EFFECTS was clear), or newly
allocated (then it was set).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-12-22  Jakub Jelinek  <jakub@redhat.com>

	PR debug/83547
	* tree-iterator.c (alloc_stmt_list): Start with cleared
	TREE_SIDE_EFFECTS regardless whether a new STATEMENT_LIST is allocated
	or old one reused.
c/
	* c-typeck.c (c_finish_stmt_expr): Ignore !TREE_SIDE_EFFECTS as
	indicator of ({ }), instead skip all trailing DEBUG_BEGIN_STMTs first,
	and consider empty ones if there are no other stmts.  For
	-Wunused-value walk all statements before the one only followed by
	DEBUG_BEGIN_STMTs.
testsuite/
	* gcc.c-torture/compile/pr83547.c: New test.

--- gcc/tree-iterator.c.jj	2017-12-18 14:57:11.000000000 +0100
+++ gcc/tree-iterator.c	2017-12-22 12:18:49.017432720 +0100
@@ -41,7 +41,10 @@ alloc_stmt_list (void)
       TREE_SET_CODE (list, STATEMENT_LIST);
     }
   else
-    list = make_node (STATEMENT_LIST);
+    {
+      list = make_node (STATEMENT_LIST);
+      TREE_SIDE_EFFECTS (list) = 0;
+    }
   TREE_TYPE (list) = void_type_node;
   return list;
 }
--- gcc/c/c-typeck.c.jj	2017-12-19 18:09:05.000000000 +0100
+++ gcc/c/c-typeck.c	2017-12-22 12:29:01.213576464 +0100
@@ -10751,17 +10751,21 @@ c_finish_stmt_expr (location_t loc, tree
  continue_searching:
   if (TREE_CODE (last) == STATEMENT_LIST)
     {
-      tree_stmt_iterator i;
+      tree_stmt_iterator l = tsi_last (last);
+
+      while (!tsi_end_p (l) && TREE_CODE (tsi_stmt (l)) == DEBUG_BEGIN_STMT)
+	tsi_prev (&l);
 
       /* This can happen with degenerate cases like ({ }).  No value.  */
-      if (!TREE_SIDE_EFFECTS (last))
+      if (tsi_end_p (l))
 	return body;
 
       /* If we're supposed to generate side effects warnings, process
 	 all of the statements except the last.  */
       if (warn_unused_value)
 	{
-	  for (i = tsi_start (last); !tsi_one_before_end_p (i); tsi_next (&i))
+	  for (tree_stmt_iterator i = tsi_start (last);
+	       tsi_stmt (i) != tsi_stmt (l); tsi_next (&i))
 	    {
 	      location_t tloc;
 	      tree t = tsi_stmt (i);
@@ -10770,13 +10774,7 @@ c_finish_stmt_expr (location_t loc, tree
 	      emit_side_effect_warnings (tloc, t);
 	    }
 	}
-      else
-	i = tsi_last (last);
-      if (TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
-	do
-	  tsi_prev (&i);
-	while (TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT);
-      last_p = tsi_stmt_ptr (i);
+      last_p = tsi_stmt_ptr (l);
       last = *last_p;
     }
 
--- gcc/testsuite/gcc.c-torture/compile/pr83547.c.jj	2017-12-22 12:36:06.503125477 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr83547.c	2017-12-22 12:35:45.000000000 +0100
@@ -0,0 +1,16 @@
+/* PR debug/83547 */
+
+void
+foo (void)
+{
+  if (({ 0; }))
+    ;
+  if (({ 0; 0; }))
+    ;
+  if (({ }))		/* { dg-error "void value not ignored as it ought to be" } */
+    ;
+  if (({ 0; { 0; } }))	/* { dg-error "void value not ignored as it ought to be" } */
+    ;
+  if (({ 0; {} }))	/* { dg-error "void value not ignored as it ought to be" } */
+    ;
+}

	Jakub

Re: [PATCH] Fix recent DEBUG_BEGIN_STMT related regressions (PR debug/83547)

[Joseph Myers]

On Fri, 22 Dec 2017, Jakub Jelinek wrote:

> Hi!
> 
> The recent change to clear TREE_SIDE_EFFECTS on STATEMENT_LIST containing
> DEBUG_BEGIN_STMTs and a single other statement without TREE_SIDE_EFFECTS on
> it breaks the C stmt expr handling.  The problem is that it assumes if
> TREE_SIDE_EFFECTS is clear on a STATEMENT_LIST then that means the
> STATEMENT_LIST is empty, i.e. ({ }), which is no longer the case.
> 
> Fixed by skipping over DEBUG_BEGIN_STMTs from the tail to find the last
> non-DEBUG_BEGIN_STMT stmt (if any, and if none, treat it like ({ }) ),
> and also not processing the last non-DEBUG_BEGIN_STMT for -Wunused-value
> if it is followed by DEBUG_BEGIN_STMTs.
> 
> In addition to this, the patch includes a fix from my earlier patch, where
> alloc_stmt_list TREE_SIDE_EFFECTS bit is inconsistent, depending on whether
> it is reused from the cache (then TREE_SIDE_EFFECTS was clear), or newly
> allocated (then it was set).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.  (This looks the issue causing glibc builds with current mainline to 
fall over 
<https://sourceware.org/ml/libc-testresults/2017-q4/msg00538.html>.)

-- 
Joseph S. Myers
joseph@codesourcery.com