]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c-family: Tweak ptr +- (expr +- cst) FE optimization [PR120837]
authorJakub Jelinek <jakub@redhat.com>
Fri, 4 Jul 2025 05:50:12 +0000 (07:50 +0200)
committerJakub Jelinek <jakub@gcc.gnu.org>
Fri, 4 Jul 2025 05:50:12 +0000 (07:50 +0200)
The following testcase is miscompiled with -fsanitize=undefined but we
introduce UB into the IL even without that flag.

The optimization ptr +- (expr +- cst) when expr/cst have undefined
overflow into (ptr +- cst) +- expr is sometimes simply not valid,
without careful analysis on what ptr points to we don't know if it
is valid to do (ptr +- cst) pointer arithmetics.
E.g. on the testcase, ptr points to start of an array (actually
conditionally one or another) and cst is -1, so ptr - 1 is invalid
pointer arithmetics, while ptr + (expr - 1) can be valid if expr
is at runtime always > 1 and smaller than size of the array ptr points
to + 1.

Unfortunately, removing this 1992-ish optimization altogether causes
FAIL: c-c++-common/restrict-2.c  -Wc++-compat   scan-tree-dump-times lim2 "Moving statement" 11
FAIL: gcc.dg/tree-ssa/copy-headers-5.c scan-tree-dump ch2 "is now do-while loop"
FAIL: gcc.dg/tree-ssa/copy-headers-5.c scan-tree-dump-times ch2 "  if " 3
FAIL: gcc.dg/vect/pr57558-2.c scan-tree-dump vect "vectorized 1 loops"
FAIL: gcc.dg/vect/pr57558-2.c -flto -ffat-lto-objects  scan-tree-dump vect "vectorized 1 loops"
regressions (restrict-2.c also for C++ in all std modes).  I've been thinking
about some match.pd optimization for signed integer addition/subtraction of
constant followed by widening integral conversion followed by multiplication
or left shift, but that wouldn't help 32-bit arches.

So, instead at least for now, the following patch keeps doing the
optimization, just doesn't perform it in pointer arithmetics.
pointer_int_sum itself actually adds the multiplication by size_exp,
so ptr + expr is turned into ptr p+ expr * size_exp,
so this patch will try to optimize
ptr + (expr +- cst)
into
ptr p+ ((sizetype)expr * size_exp +- (sizetype)cst * size_exp)
and
ptr - (expr +- cst)
into
ptr p+ -((sizetype)expr * size_exp +- (sizetype)cst * size_exp)

2025-07-04  Jakub Jelinek  <jakub@redhat.com>

PR c/120837
* c-common.cc (pointer_int_sum): Rewrite the intop PLUS_EXPR or
MINUS_EXPR optimization into extension of both intop operands,
their separate multiplication and then addition/subtraction followed
by rest of pointer_int_sum handling after the multiplication.

* gcc.dg/ubsan/pr120837.c: New test.

gcc/c-family/c-common.cc
gcc/testsuite/gcc.dg/ubsan/pr120837.c [new file with mode: 0644]

index 9d6929824dccaa2782c635919249be6e0fcc9de1..362dc506f7850cc12fc133de499bd1a66c5eeafa 100644 (file)
@@ -3438,20 +3438,41 @@ pointer_int_sum (location_t loc, enum tree_code resultcode,
         an overflow error if the constant is negative but INTOP is not.  */
       && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (intop))
          || (TYPE_PRECISION (TREE_TYPE (intop))
-             == TYPE_PRECISION (TREE_TYPE (ptrop)))))
-    {
-      enum tree_code subcode = resultcode;
-      tree int_type = TREE_TYPE (intop);
-      if (TREE_CODE (intop) == MINUS_EXPR)
-       subcode = (subcode == PLUS_EXPR ? MINUS_EXPR : PLUS_EXPR);
-      /* Convert both subexpression types to the type of intop,
-        because weird cases involving pointer arithmetic
-        can result in a sum or difference with different type args.  */
-      ptrop = build_binary_op (EXPR_LOCATION (TREE_OPERAND (intop, 1)),
-                              subcode, ptrop,
-                              convert (int_type, TREE_OPERAND (intop, 1)),
-                              true);
-      intop = convert (int_type, TREE_OPERAND (intop, 0));
+             == TYPE_PRECISION (TREE_TYPE (ptrop))))
+      && TYPE_PRECISION (TREE_TYPE (intop)) <= TYPE_PRECISION (sizetype))
+    {
+      tree intop0 = TREE_OPERAND (intop, 0);
+      tree intop1 = TREE_OPERAND (intop, 1);
+      if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype)
+         || TYPE_UNSIGNED (TREE_TYPE (intop)) != TYPE_UNSIGNED (sizetype))
+       {
+         tree optype = c_common_type_for_size (TYPE_PRECISION (sizetype),
+                                               TYPE_UNSIGNED (sizetype));
+         intop0 = convert (optype, intop0);
+         intop1 = convert (optype, intop1);
+       }
+      tree t = fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (intop0), intop0,
+                               convert (TREE_TYPE (intop0), size_exp));
+      intop0 = convert (sizetype, t);
+      if (TREE_OVERFLOW_P (intop0) && !TREE_OVERFLOW (t))
+       intop0 = wide_int_to_tree (TREE_TYPE (intop0), wi::to_wide (intop0));
+      t = fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (intop1), intop1,
+                          convert (TREE_TYPE (intop1), size_exp));
+      intop1 = convert (sizetype, t);
+      if (TREE_OVERFLOW_P (intop1) && !TREE_OVERFLOW (t))
+       intop1 = wide_int_to_tree (TREE_TYPE (intop1), wi::to_wide (intop1));
+      intop = build_binary_op (EXPR_LOCATION (intop), TREE_CODE (intop),
+                              intop0, intop1, true);
+
+      /* Create the sum or difference.  */
+      if (resultcode == MINUS_EXPR)
+       intop = fold_build1_loc (loc, NEGATE_EXPR, sizetype, intop);
+
+      ret = fold_build_pointer_plus_loc (loc, ptrop, intop);
+
+      fold_undefer_and_ignore_overflow_warnings ();
+
+      return ret;
     }
 
   /* Convert the integer argument to a type the same size as sizetype
diff --git a/gcc/testsuite/gcc.dg/ubsan/pr120837.c b/gcc/testsuite/gcc.dg/ubsan/pr120837.c
new file mode 100644 (file)
index 0000000..97c85c7
--- /dev/null
@@ -0,0 +1,32 @@
+/* PR c/120837 */
+/* { dg-do run } */
+/* { dg-options "-O1 -fsanitize=undefined -fno-sanitize-recover=undefined" } */
+
+[[gnu::noipa]] void
+bar (void **x, void **y)
+{
+  x[0] = 0;
+  x[1] = 0;
+  x[2] = 0;
+  y[0] = 0;
+  y[1] = 0;
+  y[2] = 0;
+  y[3] = 0;
+  y[4] = 0;
+}
+
+[[gnu::noipa]] void *
+foo (int x, int y)
+{
+  void *a[3];
+  void *b[5];
+  bar (a, b);
+  return (x > y ? b : a)[y - 1];
+}
+
+int
+main ()
+{
+  if (foo (2, 1) != 0)
+    __builtin_abort ();
+}