On Sat, Aug 03, 2013 at 12:24:32PM -0400, Jason Merrill wrote: > Where are the SAVE_EXPRs coming from? It doesn't seem to me that x > needs to be wrapped in a SAVE_EXPR at all in this case. For cases > where the SAVE_EXPR is needed and not used in the test, you could > add the SAVE_EXPR before the condition with a COMPOUND_EXPR.
Those SAVE_EXPRs are coming from c-typeck.c in this case: if (flag_sanitize & SANITIZE_UNDEFINED && current_function_decl != 0 && (doing_div_or_mod || doing_shift)) { /* OP0 and/or OP1 might have side-effects. */ op0 = c_save_expr (op0); op1 = c_save_expr (op1); I've resorted to creating (SAVE_EXPR <x>, ...) in the condition in case we're in C89, i.e. creating COMPOUND_EXPR in the condition itself, it seems to work pretty well. Firstly, I added explicit (void) cast via NOP_EXPR (as 'if ((void) x, y)' doesn't trigger -Wunused-value warning), but that didn't seem to be necessary... Does that look up to snuff to you? Thanks, 2013-08-05 Marek Polacek <pola...@redhat.com> * c-ubsan.c (ubsan_instrument_shift): Properly evaluate SAVE_EXPR even in the C89 mode. * gcc.dg/ubsan/save-expr-1.c: New test. --- gcc/c-family/c-ubsan.c.mp 2013-08-05 13:13:49.245693141 +0200 +++ gcc/c-family/c-ubsan.c 2013-08-05 13:13:53.170709181 +0200 @@ -131,6 +131,12 @@ ubsan_instrument_shift (location_t loc, build_int_cst (type0, 0)); tt = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, x, tt); } + + /* In case we have a SAVE_EXPR in a conditional context, we need to + make sure it gets evaluated before the condition. */ + if (!c_dialect_cxx () && !flag_isoc99) + t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t); + tree data = ubsan_create_data ("__ubsan_shift_data", loc, ubsan_type_descriptor (type0), ubsan_type_descriptor (type1), NULL_TREE); --- gcc/testsuite/gcc.dg/ubsan/save-expr-1.c.mp 2013-08-05 13:14:51.057960067 +0200 +++ gcc/testsuite/gcc.dg/ubsan/save-expr-1.c 2013-08-05 13:17:44.582675494 +0200 @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=shift -std=c89 -Wall -Werror -O" } */ + +static int x; +int +main (void) +{ + int o = 1; + int y = x << o; + return y; +} Marek