On Fri, Apr 07, 2017 at 04:26:50PM +0200, Martin Liška wrote: > Hello. > > Similar to what was done in Marek's r202113, when op1 is a SAVE_EXPR it must > be evaluated before condition, in order to be able to deliver the operand > to real shifting. And not just to a BB where ubsan report function is called. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > Apart from that make check RUNTESTFLAGS="ubsan.exp" works on x86_64-linux-gnu. > > Ready to be installed? > Martin
> >From 2ff2e17d82ee85b09cb5f83afbee70f8b1a84f4f Mon Sep 17 00:00:00 2001 > From: marxin <mli...@suse.cz> > Date: Fri, 7 Apr 2017 12:21:44 +0200 > Subject: [PATCH] Evaluate a SAVE_EXPR before an UBSAN check (PR > sanitizer/80350). > > gcc/c-family/ChangeLog: > > 2017-04-07 Martin Liska <mli...@suse.cz> > > PR sanitizer/80350 > * c-ubsan.c (ubsan_instrument_shift): Evaluate RHS before > doing an UBSAN check. > > gcc/testsuite/ChangeLog: > > 2017-04-07 Martin Liska <mli...@suse.cz> > > PR sanitizer/80350 > * c-c++-common/ubsan/pr80350.c: New test. > --- > gcc/c-family/c-ubsan.c | 4 +++- > gcc/testsuite/c-c++-common/ubsan/pr80350.c | 17 +++++++++++++++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr80350.c > > diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c > index 91bdef88320..ef45abdd19e 100644 > --- a/gcc/c-family/c-ubsan.c > +++ b/gcc/c-family/c-ubsan.c > @@ -171,7 +171,9 @@ ubsan_instrument_shift (location_t loc, enum tree_code > code, > > /* In case we have a SAVE_EXPR in a conditional context, we need to > make sure it gets evaluated before the condition. */ > - t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); > + t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), > + fold_build2 (COMPOUND_EXPR, TREE_TYPE (op1), > + unshare_expr (op0), unshare_expr (op1)), t); For consistency with ubsan_instrument_division and better readability, can't you: /* In case we have a SAVE_EXPR in a conditional context, we need to make sure it gets evaluated before the condition. */ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op1), t); Ok with that change. Jakub