https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112727
--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> --- The releases/gcc-13 branch has been updated by Jakub Jelinek <ja...@gcc.gnu.org>: https://gcc.gnu.org/g:a982b9cb690a163434f1ac5a0901548b594205f2 commit r13-8160-ga982b9cb690a163434f1ac5a0901548b594205f2 Author: Jakub Jelinek <ja...@redhat.com> Date: Fri Dec 8 20:56:48 2023 +0100 c++: Unshare folded SAVE_EXPR arguments during cp_fold [PR112727] The following testcase is miscompiled because two ubsan instrumentations run into each other. The first one is the shift instrumentation. Before the C++ FE calls it, it wraps the 2 shift arguments with cp_save_expr, so that side-effects in them aren't evaluated multiple times. And, ubsan_instrument_shift itself uses unshare_expr on any uses of the operands to make sure further modifications in them don't affect other copies of them (the only not unshared ones are the one the caller then uses for the actual operation after the instrumentation, which means there is no tree sharing). Now, if there are side-effects in the first operand like say function call, cp_save_expr wraps it into a SAVE_EXPR, and ubsan_instrument_shift in this mode emits something like if (..., SAVE_EXPR <foo ()>, SAVE_EXPR <op1> > const) __ubsan_handle_shift_out_of_bounds (..., SAVE_EXPR <foo ()>, ...); and caller adds SAVE_EXPR <foo ()> << SAVE_EXPR <op1> after it in a COMPOUND_EXPR. So far so good. If there are no side-effects and cp_save_expr doesn't create SAVE_EXPR, everything is ok as well because of the unshare_expr. We have if (..., SAVE_EXPR <op1> > const) __ubsan_handle_shift_out_of_bounds (..., ptr->something[i], ...); and ptr->something[i] << SAVE_EXPR <op1> where ptr->something[i] is unshared. In the testcase below, the !x->s[j] ? 1 : 0 expression is wrapped initially into a SAVE_EXPR though, and unshare_expr doesn't unshare SAVE_EXPRs nor anything used in them for obvious reasons, so we end up with: if (..., SAVE_EXPR <!(bool) VIEW_CONVERT_EXPR<const struct S *>(x)->s[j] ? 1 : 0>, SAVE_EXPR <op1> > const) __ubsan_handle_shift_out_of_bounds (..., SAVE_EXPR <!(bool) VIEW_CONVERT_EXPR<const struct S *>(x)->s[j] ? 1 : 0>, ...); and SAVE_EXPR <!(bool) VIEW_CONVERT_EXPR<const struct S *>(x)->s[j] ? 1 : 0> << SAVE_EXPR <op1> So far good as well. But later during cp_fold of the SAVE_EXPR we find out that VIEW_CONVERT_EXPR<const struct S *>(x)->s[j] ? 0 : 1 is actually invariant (has TREE_READONLY set) and so cp_fold simplifies the above to if (..., SAVE_EXPR <op1> > const) __ubsan_handle_shift_out_of_bounds (..., (bool) VIEW_CONVERT_EXPR<const struct S *>(x)->s[j] ? 0 : 1, ...); and ((bool) VIEW_CONVERT_EXPR<const struct S *>(x)->s[j] ? 0 : 1) << SAVE_EXPR <op1> with the s[j] ARRAY_REFs and other expressions shared in between the two uses (and obviously the expression optimized away from the COMPOUND_EXPR in the if condition. Then comes another ubsan instrumentation at genericization time, this time to instrument the ARRAY_REFs with strict bounds checking, and replaces the s[j] in there with s[.UBSAN_BOUNDS (0B, SAVE_EXPR<j>, 8), SAVE_EXPR<j>] As the trees are shared, it does that just once though. And as the if body is gimplified first, the SAVE_EXPR<j> is evaluated inside of the if body and when it is used again after the if, it uses a potentially uninitialized value of j.1 (always uninitialized if the shift count isn't out of bounds). The following patch fixes that by unshare_expr unsharing the folded argument of a SAVE_EXPR if we've folded the SAVE_EXPR into an invariant and it is used more than once. 2023-12-08 Jakub Jelinek <ja...@redhat.com> PR sanitizer/112727 * cp-gimplify.cc (cp_fold): If SAVE_EXPR has been previously folded, unshare_expr what is returned. * c-c++-common/ubsan/pr112727.c: New test. (cherry picked from commit 6ddaf06e375e1c15dcda338697ab6ea457e6f497)
