On 8/28/25 9:48 AM, Sirui Mu wrote:
Hello,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.
Thanks! Though it looks like you don't have a copyright assignment or DCO certification; see https://gcc.gnu.org/contribute.html#legal for more information.
Tested on x86_64-linux. --- gcc/cp/typeck.cc | 26 ++++++++++--------- .../g++.dg/cpp1z/array-condition-expr.C | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+), 12 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..340edc2ffc5 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -3987,6 +3987,18 @@ 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); + 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 +4078,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,8 +4153,6 @@ 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);
If we're going to remove these tests whether first is set, I'd prefer to also move the declaration of first down...
return ret; }@@ -4158,8 +4160,8 @@ cp_build_array_ref (location_t loc, tree array, tree idx,tree ar = cp_default_conversion (array, complain); tree ind = cp_default_conversion (idx, complain);
...to about here. Or we can leave the tests alone, they should be easy for the compiler to optimize away.
- 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(); +}
