| Message ID | 20130720184112.GA14359@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 20 Jul 2013, Marek Polacek wrote:
> Ok for ubsan branch?
I explicitly have no comments on this branch patch. When there's
something for trunk I'll review that patch as a whole.
--- gcc/c/c-typeck.c.mp3 2013-07-20 20:28:36.841214356 +0200 +++ gcc/c/c-typeck.c 2013-07-20 20:29:36.646445839 +0200 @@ -10493,6 +10493,8 @@ build_binary_op (location_t location, en /* OP0 and/or OP1 might have side-effects. */ op0 = c_save_expr (op0); op1 = c_save_expr (op1); + op0 = c_fully_fold (op0, false, NULL); + op1 = c_fully_fold (op1, false, NULL); if (doing_div_or_mod) instrument_expr = ubsan_instrument_division (location, op0, op1); else if (doing_shift)
With ubsan, we ICEd in gimplify_expr e.g. on the following testcase typedef volatile int VI; int foo (void) { VI vi = 100; return 2 << ++vi; } because c_save_expr created c_maybe_const_expr which then leaked into gimplification, oops. Fixed by calling c_fully_fold on second argument; I'm calling it on the first one too, because it seems safer, but from my testing it almost seemed that it isn't actually needed, since we didn't ICEd e.g. with return ++vi << 2; Ok for ubsan branch? 2013-07-20 Marek Polacek <polacek@redhat.com> * c-typeck.c (build_binary_op): Call c_fully_fold on both SAVE_EXPRs. Marek