On Thu, Mar 09, 2023 at 08:12:47AM +0000, Richard Biener wrote:
> I think this is a reasonable way to address the regression, so OK.
It is true that both C and C++ (including c++14_down and c++17 and later
where the latter have different ordering rules) evaluate the lhs of
MODIFY_EXPR after rhs, so conceptually this patch makes sense.
But I wonder why we do in ubsan_maybe_instrument_array_ref:
if (e != NULL_TREE)
{
tree t = copy_node (*expr_p);
TREE_OPERAND (t, 1) = build2 (COMPOUND_EXPR, TREE_TYPE (op1),
e, op1);
*expr_p = t;
}
rather than modification of the ARRAY_REF's operand in place. If we
did that, we wouldn't really care about the order, shared tree would
be instrumented once, with SAVE_EXPR in there making sure we don't
compute that multiple times. Is that because the 2 copies could
have side-effects and we do want to evaluate those multiple times?
Jakub
