Re: Patch RFA: With -fnon-call-exceptions sync builtins may throw

[Richard Biener <richard dot guenther at gmail dot com>]

Ian Lance Taylor <iant@google.com> wrote:
>On Mon, Nov 4, 2013 at 3:52 AM, Richard Biener
><richard.guenther@gmail.com> wrote:
>> On Mon, Nov 4, 2013 at 7:01 AM, Ian Lance Taylor <iant@google.com>
>wrote:
>>> The middle-end currently marks all the sync builtins as nothrow. 
>That
>>> is incorrect for most of them when using -fnon-call-exceptions.  The
>>> -fnon-call-exceptions option means that instructions that trap may
>throw
>>> exceptions.  Most of the sync builtins access memory via pointers
>passed
>>> by user code.  Therefore, they may trap.  (I noticed this because Go
>>> uses -fnon-call-exceptions.)
>>>
>>> This patch fixes the problem by introducing a new internal function
>>> attribute "nothrow call".  The new attribute is exactly like
>"nothrow",
>>> except that it is ignored when using -fnon-call-exceptions.
>>>
>>> It is an internal attribute because there is no reason for a user to
>use
>>> this.  The problem only arises when the middle-end sees a function
>call
>>> that the backend turns into an instruction sequence that directly
>>> references memory.  That can only happen for functions that are
>built in
>>> to the compiler.
>>>
>>> This requires changes to the C family, LTO, and Ada frontends.  I
>think
>>> I'm handling the option correctly for LTO, but it would be good if
>>> somebody could check the logic for me.
>>>
>>> Bootstrapped and ran tests on x86_64-unknown-linux-gnu.  OK for
>>> mainline?
>>
>> Hmm.  I think you can handle this in a simpler way by just
>> doing sth similar to
>>
>> #define ATTR_MATHFN_FPROUNDING_ERRNO (flag_errno_math ? \
>>         ATTR_NOTHROW_LEAF_LIST : ATTR_MATHFN_FPROUNDING)
>>
>> that is, conditionally drop the _NOTHROW part.
>
>Good point.
>
>> Btw, I also think
>> that if it can throw then it isn't LEAF (it may return exceptionally
>> into another function in the unit).
>
>According to the docs, a "leaf" function is permitted to return by
>throwing an exception.
>
>> Also this whole conditional-on-a-flag thing doesn't work well with
>> using -fnon-call-exceptions in the optimize attribute or with
>> different -fnon-call-exceptions settings in different TUs we LTO
>> together.  Because we only have a single builtin decl (so for
>> "proper" operation you'd have to clone builtin decls based on
>> the matrix of flags that generate different attributes and use the
>> correct one depending on context).
>
>Yes, but as you know this problem already exists with -fexceptions.
>I'm not really making it worse.
>
>> That said, I'd prefer a simpler approach for now, mimicing
>flag_errno_math
>> handling.
>
>Done.  Attached.  Bootstrapped on x86_64-unknown-linux-gnu, running
>tests now.  OK for mainline if the tests pass?

Ok

Thanks,
Richard

>Ian
>
>
>gcc/ChangeLog:
>
>2013-11-04  Ian Lance Taylor  <iant@google.com>
>
>	* builtins.def (ATTR_NOTHROWCALL_LEAF_LIST): Define.
>	* sync-builtins.def: Use ATTR_NOTHROWCALL_LEAF_LIST for all sync
>	builtins that take pointers.
>	* lto-opts.c (lto_write_options): Write -fnon-call-exceptions
>	if set.
>	* lto-wrapper.c (merge_and_complain): Collect
>	OPT_fnon_call_exceptions.
>	(run_gcc): Pass -fnon-call-exceptions.
>
>gcc/testsuite/ChangeLog:
>
>2013-11-03  Ian Lance Taylor  <iant@google.com>
>
>	* g++.dg/ext/sync-4.C: New test.