[PATCH] Avoid UB in insn-emit.c (PR tree-optimization/79345)

Jakub Jelinek jakub@redhat.com
Wed Mar 1 19:33:00 GMT 2017


On Wed, Mar 01, 2017 at 03:03:29PM +0100, Richard Biener wrote:
> The patch adds better limiting to that interface and fixes one
> false positive in fixed-value.c.  Two other false positives are
> fixed by the wide-int.h patch posted a few hours ago and a patch
> to genemit from Jakub.

Here is that patch.  Right now insn-emit.c for match_scratch
operands in expanders emits
  rtx operand7 ATTRIBUTE_UNUSED;
...
  rtx operands[8];
  operands[0] = operand0;
...
  // but no operands[7] = something here;
...
  // C code from *.md file (which typically doesn't touch operands[7])
...
  operand7 = operands[7];
  (void) operand7;
...
  // generated code that doesn't use operand7
This triggers -Wuninitialized warning with Richard's patch and is really UB,
even when we actually don't use it and so hopefully optimize away.
The following patch just removes all operand7 and operands[7] references
from the generated code (i.e. operands that are for match_scratch).

Usually match_scratch numbers come after match_operand/match_dup etc.
numbers, but as can be seen, there are few spots where that is not the case.
The patch adds verification of this requirement in genemit and then fixes
the issues it has diagnosed.

Bootstrapped/regtested on x86_64-linux and i686-linux, plus tested with
make s-emit in a cross to
{powerpc64,aarch64,armv7hl,sparc64,s390x,cris,sh,ia64,hppa,mips}-linux, ok
for trunk?

2017-03-01  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/79345
	* gensupport.h (struct pattern_stats): Add min_scratch_opno field.
	* gensupport.c (get_pattern_stats_1) <case MATCH_SCRATCH>: Update it.
	(get_pattern_stats): Initialize it.
	* genemit.c (gen_expand): Verify match_scratch numbers come after
	match_operand/match_dup numbers.
	* config/i386/i386.md (<s>mul<mode>3_highpart): Swap match_dup and
	match_scratch numbers.
	* config/i386/sse.md (avx2_gathersi<mode>, avx2_gatherdi<mode>):
	Likewise.
	* config/s390/s390.md (trunctdsd2): Likewise.

--- gcc/gensupport.h.jj	2017-01-01 12:45:38.000000000 +0100
+++ gcc/gensupport.h	2017-03-01 12:06:21.816440102 +0100
@@ -199,7 +199,8 @@ struct pattern_stats
   /* The largest match_dup, match_op_dup or match_par_dup number found.  */
   int max_dup_opno;
 
-  /* The largest match_scratch number found.  */
+  /* The smallest and largest match_scratch number found.  */
+  int min_scratch_opno;
   int max_scratch_opno;
 
   /* The number of times match_dup, match_op_dup or match_par_dup appears
--- gcc/gensupport.c.jj	2017-01-05 22:10:31.000000000 +0100
+++ gcc/gensupport.c	2017-03-01 12:21:24.830327207 +0100
@@ -3000,6 +3000,10 @@ get_pattern_stats_1 (struct pattern_stat
       break;
 
     case MATCH_SCRATCH:
+      if (stats->min_scratch_opno == -1)
+	stats->min_scratch_opno = XINT (x, 0);
+      else
+	stats->min_scratch_opno = MIN (stats->min_scratch_opno, XINT (x, 0));
       stats->max_scratch_opno = MAX (stats->max_scratch_opno, XINT (x, 0));
       break;
 
@@ -3032,6 +3036,7 @@ get_pattern_stats (struct pattern_stats
 
   stats->max_opno = -1;
   stats->max_dup_opno = -1;
+  stats->min_scratch_opno = -1;
   stats->max_scratch_opno = -1;
   stats->num_dups = 0;
 
--- gcc/genemit.c.jj	2017-01-01 12:45:35.000000000 +0100
+++ gcc/genemit.c	2017-03-01 12:16:28.391343302 +0100
@@ -448,6 +448,10 @@ gen_expand (md_rtx_info *info)
 
   /* Find out how many operands this function has.  */
   get_pattern_stats (&stats, XVEC (expand, 1));
+  if (stats.min_scratch_opno != -1
+      && stats.min_scratch_opno <= MAX (stats.max_opno, stats.max_dup_opno))
+    fatal_at (info->loc, "define_expand for %s needs to have match_scratch "
+			 "numbers above all other operands", XSTR (expand, 0));
 
   /* Output the function name and argument declarations.  */
   printf ("rtx\ngen_%s (", XSTR (expand, 0));
@@ -479,8 +483,6 @@ gen_expand (md_rtx_info *info)
      make a local variable.  */
   for (i = stats.num_generator_args; i <= stats.max_dup_opno; i++)
     printf ("  rtx operand%d;\n", i);
-  for (; i <= stats.max_scratch_opno; i++)
-    printf ("  rtx operand%d ATTRIBUTE_UNUSED;\n", i);
   printf ("  rtx_insn *_val = 0;\n");
   printf ("  start_sequence ();\n");
 
@@ -516,7 +518,7 @@ gen_expand (md_rtx_info *info)
 	 (unless we aren't going to use them at all).  */
       if (XVEC (expand, 1) != 0)
 	{
-	  for (i = 0; i < stats.num_operand_vars; i++)
+	  for (i = 0; i <= MAX (stats.max_opno, stats.max_dup_opno); i++)
 	    {
 	      printf ("    operand%d = operands[%d];\n", i, i);
 	      printf ("    (void) operand%d;\n", i);
--- gcc/config/i386/i386.md.jj	2017-02-22 18:15:48.000000000 +0100
+++ gcc/config/i386/i386.md	2017-03-01 12:23:22.882736837 +0100
@@ -7364,11 +7364,11 @@ (define_expand "<s>mul<mode>3_highpart"
 			   (match_operand:SWI48 1 "nonimmediate_operand"))
 			 (any_extend:<DWI>
 			   (match_operand:SWI48 2 "register_operand")))
-		       (match_dup 4))))
-	      (clobber (match_scratch:SWI48 3))
+		       (match_dup 3))))
+	      (clobber (match_scratch:SWI48 4))
 	      (clobber (reg:CC FLAGS_REG))])]
   ""
-  "operands[4] = GEN_INT (GET_MODE_BITSIZE (<MODE>mode));")
+  "operands[3] = GEN_INT (GET_MODE_BITSIZE (<MODE>mode));")
 
 (define_insn "*<s>muldi3_highpart_1"
   [(set (match_operand:DI 0 "register_operand" "=d")
--- gcc/config/i386/sse.md.jj	2017-02-07 16:41:32.000000000 +0100
+++ gcc/config/i386/sse.md	2017-03-01 12:24:57.193471018 +0100
@@ -18873,7 +18873,7 @@ (define_expand "avx2_gathersi<mode>"
 		   (unspec:VEC_GATHER_MODE
 		     [(match_operand:VEC_GATHER_MODE 1 "register_operand")
 		      (mem:<ssescalarmode>
-			(match_par_dup 7
+			(match_par_dup 6
 			  [(match_operand 2 "vsib_address_operand")
 			   (match_operand:<VEC_GATHER_IDXSI>
 			      3 "register_operand")
@@ -18881,10 +18881,10 @@ (define_expand "avx2_gathersi<mode>"
 		      (mem:BLK (scratch))
 		      (match_operand:VEC_GATHER_MODE 4 "register_operand")]
 		     UNSPEC_GATHER))
-	      (clobber (match_scratch:VEC_GATHER_MODE 6))])]
+	      (clobber (match_scratch:VEC_GATHER_MODE 7))])]
   "TARGET_AVX2"
 {
-  operands[7]
+  operands[6]
     = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, operands[2], operands[3],
 					operands[5]), UNSPEC_VSIBADDR);
 })
@@ -18934,7 +18934,7 @@ (define_expand "avx2_gatherdi<mode>"
 		   (unspec:VEC_GATHER_MODE
 		     [(match_operand:<VEC_GATHER_SRCDI> 1 "register_operand")
 		      (mem:<ssescalarmode>
-			(match_par_dup 7
+			(match_par_dup 6
 			  [(match_operand 2 "vsib_address_operand")
 			   (match_operand:<VEC_GATHER_IDXDI>
 			      3 "register_operand")
@@ -18942,10 +18942,10 @@ (define_expand "avx2_gatherdi<mode>"
 		      (mem:BLK (scratch))
 		      (match_operand:<VEC_GATHER_SRCDI> 4 "register_operand")]
 		     UNSPEC_GATHER))
-	      (clobber (match_scratch:VEC_GATHER_MODE 6))])]
+	      (clobber (match_scratch:VEC_GATHER_MODE 7))])]
   "TARGET_AVX2"
 {
-  operands[7]
+  operands[6]
     = gen_rtx_UNSPEC (Pmode, gen_rtvec (3, operands[2], operands[3],
 					operands[5]), UNSPEC_VSIBADDR);
 })
--- gcc/config/s390/s390.md.jj	2017-02-06 13:32:14.000000000 +0100
+++ gcc/config/s390/s390.md	2017-03-01 12:45:07.184184616 +0100
@@ -5074,15 +5074,15 @@ (define_insn "truncddsd2"
 
 (define_expand "trunctdsd2"
   [(parallel
-    [(set (match_dup 3)
+    [(set (match_dup 2)
 	  (float_truncate:DD (match_operand:TD 1 "register_operand" "")))
      (unspec:DI [(const_int DFP_RND_PREP_FOR_SHORT_PREC)] UNSPEC_ROUND)
-     (clobber (match_scratch:TD 2 ""))])
+     (clobber (match_scratch:TD 3 ""))])
    (set (match_operand:SD 0 "register_operand" "")
-	(float_truncate:SD (match_dup 3)))]
+	(float_truncate:SD (match_dup 2)))]
   "TARGET_HARD_DFP"
 {
-  operands[3] = gen_reg_rtx (DDmode);
+  operands[2] = gen_reg_rtx (DDmode);
 })
 
 ;
  

	Jakub



More information about the Gcc-patches mailing list