From: Richard Biener <richard.guenther@gmail.com>
To: Ian Lance Taylor <iant@google.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Patch RFA: With -fnon-call-exceptions sync builtins may throw
Date: Mon, 04 Nov 2013 11:58:00 -0000 [thread overview]
Message-ID: <CAFiYyc0oD-LRDrczegyWdZVBOD3+CUqYf2tQrFYLHGK87ksWDA@mail.gmail.com> (raw)
In-Reply-To: <mcrd2mgiu3k.fsf@iant-glaptop.roam.corp.google.com>
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. Btw, I also think
that if it can throw then it isn't LEAF (it may return exceptionally
into another function in the unit).
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).
That said, I'd prefer a simpler approach for now, mimicing flag_errno_math
handling.
Your lto-wrapper parts are clearly "wrong" (in a conservative way
though). They follow -fexceptions so the lto-wrapper.c and
lto-opts.c parts are ok.
Fixing the LTO pieces would require to really "stream" most
builtins instead of generating them in the LTO frontend and streaming
them via DECL_FUNCTION_CODE. tree merging should get
rid of the duplicates. I suppose I should try that again ;)
Thanks,
Richard.
> Ian
>
>
> gcc/ChangeLog:
>
> 2013-11-03 Ian Lance Taylor <iant@google.com>
>
> * builtin-attrs.def (ATTR_NOTHROWCALL): Define.
> (ATTR_NOTHROWCALL_LIST, 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/c-family/ChangeLog:
>
> 2013-11-03 Ian Lance Taylor <iant@google.com>
>
> * c-common.c (c_common_attribute_table): Add "nothrow call".
> (handle_nothrowcall_attribute): New static function.
>
> gcc/lto/ChangeLog:
>
> 2013-11-03 Ian Lance Taylor <iant@google.com>
>
> * lto-lang.c (lto_attribute_table): Add "nothrow call"
> (handle_nothrowcall_attribute): New static function.
>
> gcc/ada/ChangeLog:
>
> 2013-11-03 Ian Lance Taylor <iant@google.com>
>
> * gcc-interface/utils.c (gnat_internal_attribute_table): Add
> "nothrow call".
> (handle_nothrowcall_attribute): New static function.
>
> gcc/testsuite/ChangeLog:
>
> 2013-11-03 Ian Lance Taylor <iant@google.com>
>
> * g++.dg/ext/sync-4.C: New test.
>
>
next prev parent reply other threads:[~2013-11-04 11:52 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-04 6:01 Ian Lance Taylor
2013-11-04 11:58 ` Richard Biener [this message]
2013-11-04 16:45 ` Ian Lance Taylor
2013-11-04 18:13 ` Richard Biener
2013-11-04 21:28 ` Marc Glisse
2013-11-04 22:25 ` Ian Lance Taylor
2013-11-05 10:06 ` Andreas Schwab
2013-11-04 21:44 Dominique Dhumieres
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFiYyc0oD-LRDrczegyWdZVBOD3+CUqYf2tQrFYLHGK87ksWDA@mail.gmail.com \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=iant@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).