Re: [PATCH] Fix -fsanitize=thread with -fnon-call-exceptions (PR sanitizer/80110)

[Richard Biener <rguenther at suse dot de>]

On Wed, 22 Mar 2017, Jakub Jelinek wrote:

> On Tue, Mar 21, 2017 at 09:26:49AM +0100, Richard Biener wrote:
> > On Tue, 21 Mar 2017, Jakub Jelinek wrote:
> > 
> > > On Tue, Mar 21, 2017 at 09:12:51AM +0100, Richard Biener wrote:
> > > > > libtsan atomics aren't throwing, so if we transform atomics which
> > > > > are throwing with -fnon-call-exceptions, we need to clean up EH stuff.
> > > > > 
> > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > > > 
> > > > Huh, but this means with TSAN we create wrong-code with 
> > > > -fnon-call-exceptions and programs will crash instead of properly
> > > > catching things like null-pointer accesses?
> > > 
> > > True.  I think it is only about atomics, normal loads/stores are done
> > > inline with extra calls before/after that just tell the library about
> > > those loads/stores (though not sure what the library code will do with
> > > invalid or NULL pointers, if it won't crash on them).
> > > The question is what we could do about it and if it is worth bothering,
> > > I guess we'd need to build tsan_interface_atomics.cc with
> > > -fnon-call-exceptions and the question would be if it doesn't slow it
> > > down for the common case where -fnon-call-exceptions isn't used.
> > > Another option would be to add another set of __tsan_atomic* entrypoints
> > > for -fnon-call-exceptions.
> > > 
> > > > We should at least document this or reject sanitize=thread with
> > > > -fnon-call-exceptions.
> > > 
> > > Rejecting would mean that even code that doesn't use the atomics or doesn't
> > > use atomics on invalid pointers would not be allowed.
> > 
> > Indeed.  So maybe just document it then.  I suppose sanitizing in
> > general has the issue that its events are not properly raising
> > exceptions (of whatever kind), so documenting that programs relying
> > on async EH to be reliably delivered may not work as expected with
> > sanitization would be good.
> 
> So like this in addition to the posted patch?

Looks good.

Thanks,
Richard.

> 2017-03-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/80110
> 	* doc/invoke.texi (-fsanitize=thread): Document that with
> 	-fnon-call-exceptions atomics are not able to throw
> 	exceptions.
> 
> --- gcc/doc/invoke.texi.jj	2017-03-19 11:57:08.000000000 +0100
> +++ gcc/doc/invoke.texi	2017-03-22 14:22:40.737505684 +0100
> @@ -10761,6 +10761,10 @@ supported options.
>  The option can't be combined with @option{-fsanitize=address},
>  @option{-fsanitize=leak} and/or @option{-fcheck-pointer-bounds}.
>  
> +Note that sanitized atomic builtins cannot throw exceptions when
> +operating on invalid memory addresses with non-call exceptions
> +(@option{-fnon-call-exceptions}).
> +
>  @item -fsanitize=leak
>  @opindex fsanitize=leak
>  Enable LeakSanitizer, a memory leak detector.
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)