Re: [patch] Fix segfault caused by spurious constant overflow

[Eric Botcazou <ebotcazou@adacore.com>]

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

> Hmm, ISTR we had such mitigations in place (or have) elsewhere keying
> on the most significant bit set instead of power-of-two.  But your case
> likely recurses and runs into the extract_multiv limiting to eventually
> stop, even for (N + 4) * 8, right?  If so shouldn't we prevent this
> even for !TYPE_OVERFLOW_WRAPS?  Also
> 
> +         && !(tree_fits_shwi_p (c)
> +              && exact_log2 (absu_hwi (tree_to_shwi (c))) > 0))
> 
> is better written as
> 
>            && exact_log2 (wi::to_wide (c)) > 0

It turns out that pow2p_hwi can be used instead and is cheaper, so I have 
changed both extract_muldiv_1 and fold_plusminus_mult_expr to using it.


	* fold-const.c (extract_muldiv_1) <PLUS_EXPR>: Do not distribute a
	multiplication by a power-of-two value.
	(fold_plusminus_mult_expr): Use pow2p_hwi to detect a power-of-two value
	and turn the modulo operation into a masking operation.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 2239 bytes --]

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 271694)
+++ fold-const.c	(working copy)
@@ -6475,8 +6475,12 @@ extract_muldiv_1 (tree t, tree c, enum t
 	 apply the distributive law to commute the multiply and addition
 	 if the multiplication of the constants doesn't overflow
 	 and overflow is defined.  With undefined overflow
-	 op0 * c might overflow, while (op0 + orig_op1) * c doesn't.  */
-      if (code == MULT_EXPR && TYPE_OVERFLOW_WRAPS (ctype))
+	 op0 * c might overflow, while (op0 + orig_op1) * c doesn't.
+	 But fold_plusminus_mult_expr would factor back any power-of-two
+	 value so do not distribute in the first place in this case.  */
+      if (code == MULT_EXPR
+	  && TYPE_OVERFLOW_WRAPS (ctype)
+	  && !(tree_fits_shwi_p (c) && pow2p_hwi (absu_hwi (tree_to_shwi (c)))))
 	return fold_build2 (tcode, ctype,
 			    fold_build2 (code, ctype,
 					 fold_convert (ctype, op0),
@@ -7124,14 +7128,13 @@ fold_plusminus_mult_expr (location_t loc
   /* No identical multiplicands; see if we can find a common
      power-of-two factor in non-power-of-two multiplies.  This
      can help in multi-dimensional array access.  */
-  else if (tree_fits_shwi_p (arg01)
-	   && tree_fits_shwi_p (arg11))
+  else if (tree_fits_shwi_p (arg01) && tree_fits_shwi_p (arg11))
     {
-      HOST_WIDE_INT int01, int11, tmp;
+      HOST_WIDE_INT int01 = tree_to_shwi (arg01);
+      HOST_WIDE_INT int11 = tree_to_shwi (arg11);
+      HOST_WIDE_INT tmp;
       bool swap = false;
       tree maybe_same;
-      int01 = tree_to_shwi (arg01);
-      int11 = tree_to_shwi (arg11);
 
       /* Move min of absolute values to int11.  */
       if (absu_hwi (int01) < absu_hwi (int11))
@@ -7144,7 +7147,10 @@ fold_plusminus_mult_expr (location_t loc
       else
 	maybe_same = arg11;
 
-      if (exact_log2 (absu_hwi (int11)) > 0 && int01 % int11 == 0
+      unsigned HOST_WIDE_INT factor = absu_hwi (int11);
+      if (factor > 1
+	  && pow2p_hwi (factor)
+	  && (int01 & (factor - 1)) == 0
 	  /* The remainder should not be a constant, otherwise we
 	     end up folding i * 4 + 2 to (i * 2 + 1) * 2 which has
 	     increased the number of multiplications necessary.  */