On Wed, Oct 05, 2016 at 07:57:12PM +0200, Bernd Schmidt wrote:
> On 10/05/2016 06:51 PM, Marek Polacek wrote:
> >On Wed, Oct 05, 2016 at 05:29:49PM +0200, Jakub Jelinek wrote:
> >>When writing test for this PR, I've noticed ICE if the header is compiled
> >>without -fsanitize=undefined, but source is compiled with it.
> >>
> >>We had various issues like this in the past, and we handle it by calling
> >>initialize_sanitizer_builtins, which does nothing if the sanitizer bultins
> >>are already created, but after loading PCH (which can kill them) can fix
> >>stuff
> >>up again. I found various spots where the call has been missing in the
> >>ubsan instrumentation, but common feature of all those spots is first
> >>calling ubsan_create_data and only then using builtin_decl_explicit
> >>for the ubsan builtins. So, this patch puts the initialization call into
> >>that routine, which fixes all uses, and removes the two calls that are now
> >>unnecessary because of that.
> >>
> >>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> >LGTM, but can't approve neither.
>
> Ok. Although I wonder, pass_ubsan::execute calls this function: why isn't
> this enough? Maybe we just need to add it to other pass execute functions as
> well?
-fsanitize=undefined is actually just a collection of various runtime
checks that are added at various times. The call at the start of
pass_ubsan::execute is needed for any builtin_decl_explicit uses in that
pass. But some sanitization happens already in the FEs. That is sometimes
done by creating internal calls and lowered only later on during the ubsan
pass (which also instruments e.g. arithmetics overflows etc.), and sometimes
by creating the builtin calls right away. The call I've added to
ubsan_create_data is for all those uses. An alternative would be to add
IFN_UBSAN_* calls for all the stuff that we instrument in the FEs and lower
it in the ubsan pass to what we emit right away.
Jakub
