[PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

[Bin.Cheng]

On Wed, Aug 27, 2014 at 6:34 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 27/08/14 11:08, Bin Cheng wrote:
>> Hi
>> As reported in bug pr62151, combine pass may wrongly delete necessary
>> instruction in function distribute_notes thus leaving register
>> uninitialized.  This patch is to fix the issue by checking if i2 immediately
>> modifies the register in REG_DEAD note.  If yes, set tem_insn accordingly to
>> start finding right place for note distribution from i2.
>>
>> I once sent a RFC patch at
>> https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01718.html, but got no
>> comments,  here I added some comments in this patch to make it a formal one.
>>
>>
>> I tested the original patch, and will re-test it against the latest code
>> later.  So is it OK?  Any comments will be appreciated.
>>
>
> Isn't this the sort of sequence that combinable_i3pat is supposed to reject?
Hi Richard,
I think it's not.  combinable_i3pat checks cases in which i1 feeds to
i3 directly, while i2 kills reg_use in i1.  For this case the feeding
chain is "i0->i1->i2->i3", the combination is valid and beneficial.
What needs to be handled is if i2dest is anticipated after i3.  If
not, then i2 could be deleted; if yes, i2 should be preserved.
Moreover, following constant propagation could delete i2 after
propagating the new i2src into i4.  Note combine pass already handles
this kind of case using variable added_sets_2 in function try_combine.
The problem is in distribute_notes which mis-deleted i2.

I added one test case in the updated patch.

Thanks,
bin
>
-------------- next part --------------
Index: gcc/testsuite/gcc.c-torture/execute/pr62151.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr62151.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/pr62151.c	(revision 0)
@@ -0,0 +1,41 @@
+/* PR rtl-optimization/62151 */
+
+int a, c, d, e, f, g, h, i;
+short b;
+
+int
+fn1 ()
+{
+  b = 0;
+  for (;;)
+    {
+      int j[2];
+      j[f] = 0;
+      if (h)
+	d = 0;
+      else
+	{
+	  for (; f; f++)
+	    ;
+	  for (a = 0; a < 1; a++)
+	    for (;;)
+	      {
+		i = b & ((b ^ 1) & 83647) ? b : b - 1;
+		g = 1 ? i : 0;
+		e = j[0];
+		if (c)
+		  break;
+		return 0;
+	      }
+	}
+    }
+}
+
+int
+main ()
+{
+  fn1 ();
+  if (g != -1) 
+    __builtin_abort (); 
+  return 0;
+}
Index: gcc/combine.c
===================================================================
--- gcc/combine.c	(revision 214413)
+++ gcc/combine.c	(working copy)
@@ -13499,7 +13499,38 @@ distribute_notes (rtx notes, rtx_insn *from_insn,
 		       || rtx_equal_p (XEXP (note, 0), elim_i1)
 		       || rtx_equal_p (XEXP (note, 0), elim_i0))
 		break;
-	      tem_insn = i3;
+
+	      /* See PR62151.
+		 It's possible to have below situation:
+		   i0: r1 <- const_0
+		   i1: r2 <- r1 op_1 const_1
+		       REG_DEAD r1
+		   i2: r1 <- r2 op_2 const_2
+		       REG_DEAD r2
+		   i3: r3 <- r1
+		   i4: r4 <- r1
+
+		 It is transformed into below code before distributing
+		 the REG_DEAD note in i1:
+		   i0: NOTE_INSN_DELETED
+		   i1: NOTE_INSN_DELETED
+		   i2: r1 <- const_combined
+		   i3: r3 <- const_combined
+		   i4: r4 <- r1
+
+		 We need to check if i2 immediately modifies r1 otherwise
+		 i2 would be deleted by below code when distributing
+		 REG_DEAD note, leaving r1 in i4 uninitialied.
+
+		 We set TEM_INSN to i2 for this case indicating that we
+		 need to find right place for distribution from i2.
+		 */
+	      if (from_insn && i2
+		  && from_insn != i2 && from_insn != i3
+		  && reg_set_p (XEXP (note, 0), PATTERN (i2)))
+		tem_insn = i2;
+	      else
+		tem_insn = i3;
 	    }
 
 	  if (place == 0)