On 8/28/25 12:33 PM, Sirui Mu wrote:
Hello,
This is the 2nd version of the patch. Changes made since the initial version:
- Implemented Jason's suggestions, move the declaration of first down.
- Add signed-off-by to the commit.
Please preserve the rationale in later revisions of the patch; revision
notes can go at the top, followed by a scissors line (-- 8< --), then
the commit message.
Please wrap lines in the commit message at 76 columns so that they still
fit in 80 columns with the 4 spaces of left padding from 'git log'. The
subject line should be even shorter; I aim for 50 characters after
"[PATCH] ", but this isn't a strict rule.
The commit message also needs ChangeLog entries; git gcc-commit-mklog
automates most of that.
This time, I've gone ahead and made these adjustments, and pushed the
following:
Thanks again.
From 78d19ea3fea308a08d2844de88d43154465daa78 Mon Sep 17 00:00:00 2001
From: Sirui Mu <msrlanc...@gmail.com>
Date: Thu, 28 Aug 2025 21:48:24 +0800
Subject: [PATCH] c++: array subscript with COND_EXPR as the array
To: gcc-patches@gcc.gnu.org
The following minimum reproducer would miscompile with vanilla gcc:
extern int x[10], y[10];
bool g();
void f() { 0[g() ? x : y] = 1; }
gcc would mistakenly treat the subexpression (g() ? x : y) as a prvalue and
move that array to stack. The following assignment would then write to the
stack instead of to the global arrays. When optimizations are enabled, this
assignment is discarded by dse and gcc generates the following code for the
f function:
"_Z1fi":
jmp "_Z1gv"
The miscompilation requires all the following conditions to be met:
- The array subscription expression is written as idx[array], instead of
the usual form array[idx];
- The "array" part must be a ternary expression (COND_EXPR in gcc tree)
and it must be an lvalue.
- The code must be compiled with -fstrong-eval-order which is the default
for -std=c++17 or later.
The cause of the issue lies in cp_build_array_ref, where it mistakenly
generates a COND_EXPR with ARRAY_TYPE to the IL when all the criteria above
are met. This patch tries to resolve this issue. It moves the
canonicalization step that transforms idx[array] to array[idx] early in
cp_build_array_ref to ensure we handle these two forms of array subscription
consistently.
Tested on x86_64-linux.
gcc/cp/ChangeLog:
* typeck.cc (cp_build_array_ref): Handle 0[arr] earlier.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1z/array-condition-expr.C: New test.
Signed-off-by: Sirui Mu <msrlanc...@gmail.com>
---
gcc/cp/typeck.cc | 31 +++++++++++--------
.../g++.dg/cpp1z/array-condition-expr.C | 26 ++++++++++++++++
2 files changed, 44 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1z/array-condition-expr.C
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 41a3f4cb7cb..ccfe50d3c37 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -3973,7 +3973,6 @@ tree
cp_build_array_ref (location_t loc, tree array, tree idx,
tsubst_flags_t complain)
{
- tree first = NULL_TREE;
tree ret;
if (idx == 0)
@@ -3987,6 +3986,21 @@ cp_build_array_ref (location_t loc, tree array, tree idx,
|| TREE_TYPE (idx) == error_mark_node)
return error_mark_node;
+ /* 0[array] */
+ if (TREE_CODE (TREE_TYPE (idx)) == ARRAY_TYPE)
+ {
+ std::swap (array, idx);
+
+ tree first = NULL_TREE;
+ if (flag_strong_eval_order == 2 && TREE_SIDE_EFFECTS (array))
+ idx = first = save_expr (idx);
+ ret = cp_build_array_ref (loc, array, idx, complain);
+
+ if (first)
+ ret = build2 (COMPOUND_EXPR, TREE_TYPE (ret), first, ret);
+ return ret;
+ }
+
/* If ARRAY is a COMPOUND_EXPR or COND_EXPR, move our reference
inside it. */
switch (TREE_CODE (array))
@@ -4066,14 +4080,6 @@ cp_build_array_ref (location_t loc, tree array, tree idx,
bool non_lvalue = convert_vector_to_array_for_subscript (loc, &array, idx);
- /* 0[array] */
- if (TREE_CODE (TREE_TYPE (idx)) == ARRAY_TYPE)
- {
- std::swap (array, idx);
- if (flag_strong_eval_order == 2 && TREE_SIDE_EFFECTS (array))
- idx = first = save_expr (idx);
- }
-
if (TREE_CODE (TREE_TYPE (array)) == ARRAY_TYPE)
{
tree rval, type;
@@ -4149,17 +4155,16 @@ cp_build_array_ref (location_t loc, tree array, tree idx,
protected_set_expr_location (ret, loc);
if (non_lvalue)
ret = non_lvalue_loc (loc, ret);
- if (first)
- ret = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (ret), first, ret);
return ret;
}
{
tree ar = cp_default_conversion (array, complain);
tree ind = cp_default_conversion (idx, complain);
+ tree first = NULL_TREE;
- if (!processing_template_decl
- && !first && flag_strong_eval_order == 2 && TREE_SIDE_EFFECTS (ind))
+ if (!processing_template_decl && flag_strong_eval_order == 2
+ && TREE_SIDE_EFFECTS (ind))
ar = first = save_expr (ar);
/* Put the integer in IND to simplify error checking. */
diff --git a/gcc/testsuite/g++.dg/cpp1z/array-condition-expr.C b/gcc/testsuite/g++.dg/cpp1z/array-condition-expr.C
new file mode 100644
index 00000000000..6b59d7c9fe4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/array-condition-expr.C
@@ -0,0 +1,26 @@
+// { dg-do run { target c++17 } }
+
+int x[10];
+int y[10];
+bool c() { return true; }
+
+void f(int i, int v)
+{
+ (c() ? x : y)[i] = v;
+}
+
+void g(int i, int v)
+{
+ i[c() ? x : y] = v;
+}
+
+int main()
+{
+ f(0, 1);
+ if (x[0] != 1)
+ __builtin_abort();
+
+ g(0, 2);
+ if (x[0] != 2)
+ __builtin_abort();
+}
--
2.50.1
