Re: [PATCH] ubsan: missed -fsanitize=bounds for compound ops [PR108060]

[Jakub Jelinek <jakub@redhat.com>]

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