On Wed, Sep 07, 2016 at 09:07:45AM +0200, Richard Biener wrote:
> > @@ -493,6 +504,8 @@ instrument_builtin_call (gimple_stmt_ite
> > if (!tree_fits_uhwi_p (last_arg)
> > || memmodel_base (tree_to_uhwi (last_arg)) >= MEMMODEL_LAST)
> > return;
> > + if (lookup_stmt_eh_lp (stmt))
> > + remove_stmt_from_eh_lp (stmt);
>
> These changes look bogus to me -- how can the tsan instrumentation
> function _not_ throw when the original function we replace it can?
The __tsan*atomic* functions are right now declared to be nothrow, so the
patch just matches how they are declared.
While the sync-builtins.def use
#define ATTR_NOTHROWCALL_LEAF_LIST (flag_non_call_exceptions ? \
ATTR_LEAF_LIST : ATTR_NOTHROW_LEAF_LIST)
I guess I could use the same for the tsan atomics, but wonder if it will work
properly when the libtsan is built with exceptions disabled and without
-fnon-call-exceptions. Perhaps it would, at least if it is built with
-fasynchronous-unwind-tables (which is the case for x86_64 and aarch64 and
tsan isn't supported elsewhere).
>
> It seems you are just avoiding the ICEs for now "wrong-code". (and
> how does this relate to -fnon-call-exceptions as both are calls?)
>
> The instrument_expr case seems to leave the original stmt in-place
> (and thus the EH).
Those are different. For loads and stores there is just added call that
logs in the load or store, but for atomics the atomics are done inside of
the library and the library tracks everything.
Jakub
