Hi!
The following testcase is miscompiled since the introduction of UBSan,
cp_build_array_ref COND_EXPR handling replaces
(cond ? a : b)[idx] with cond ? a[idx] : b[idx], but if there are
SAVE_EXPRs inside of idx, they will be evaluated just in one of the
branches and the other uses uninitialized temporaries.
Fixed by using cp_save_expr and if needed, evaluating it before the
condition; for constant indexes this shouldn't change anything, and for
larger expressions in idx I think the patch should result in smaller
generated code, no need to duplicate all the evaluation in each of the
branches.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2025-06-27 Jakub Jelinek <ja...@redhat.com>
PR c++/120471
* typeck.cc (cp_build_array_ref) <case COND_EXPR>: Use cp_save_expr
on idx. If it has TREE_SIDE_EFFECTS, add it as lhs of COMPOUND_EXPR
in first COND_EXPR argument. Formatting fixes.
* g++.dg/ubsan/pr120471.C: New test.
--- gcc/cp/typeck.cc.jj 2025-06-17 13:19:03.040958319 +0200
+++ gcc/cp/typeck.cc 2025-06-27 14:25:54.657852009 +0200
@@ -4001,13 +4001,14 @@ cp_build_array_ref (location_t loc, tree
}
case COND_EXPR:
- ret = build_conditional_expr
- (loc, TREE_OPERAND (array, 0),
- cp_build_array_ref (loc, TREE_OPERAND (array, 1), idx,
- complain),
- cp_build_array_ref (loc, TREE_OPERAND (array, 2), idx,
- complain),
- complain);
+ tree op0, op1, op2;
+ idx = cp_save_expr (idx);
+ op0 = TREE_OPERAND (array, 0);
+ if (TREE_SIDE_EFFECTS (idx))
+ op0 = build_compound_expr (loc, idx, op0);
+ op1 = cp_build_array_ref (loc, TREE_OPERAND (array, 1), idx, complain);
+ op2 = cp_build_array_ref (loc, TREE_OPERAND (array, 2), idx, complain);
+ ret = build_conditional_expr (loc, op0, op1, op2, complain);
protected_set_expr_location (ret, loc);
return ret;
--- gcc/testsuite/g++.dg/ubsan/pr120471.C.jj 2025-06-27 14:31:31.340547836
+0200
+++ gcc/testsuite/g++.dg/ubsan/pr120471.C 2025-06-27 14:31:12.347790637
+0200
@@ -0,0 +1,21 @@
+// PR c++/120471
+// { dg-do run }
+// { dg-options "-fsanitize=undefined" }
+
+volatile int b[1], a[1];
+
+void
+foo (int x)
+{
+ volatile int c = 21;
+ volatile int v = (x % 2 ? b : a)[c % 3];
+ if (v != 0)
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ foo (1);
+ foo (2);
+}
Jakub
