This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Don't add __builtin_unreachable for -Wreturn-type places at -O0
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Jason Merrill <jason at redhat dot com>,Nathan Sidwell <nathan at acm dot org>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 05 Dec 2017 07:29:58 +0100
- Subject: Re: [PATCH] Don't add __builtin_unreachable for -Wreturn-type places at -O0
- Authentication-results: sourceware.org; auth=none
- References: <20171204234802.GQ2353@tucnak>
On December 5, 2017 12:48:02 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>When debugging what turned out to be just a -Wreturn-type reported
>failure now being a fatal problem even at -O0, it occurs to me that
>because we are not optimizing at -O0, adding the __builtin_unreachable
>doesn't bring many optimization advantages (practically just optimize
>away epilogue) and so I think I'd prefer to only add those
>when optimizing.
>
>Similarly, for UBSan, we instrument these with -fsanitize=return,
>part of -fsanitize=unreachable, where we provide nice runtime errors,
>but when somebody intentionally turns that off (e.g. to quickly get
>past such issues to look for something else), say with
>-fsanitize=undefined -fno-sanitize=return, the same error will be
>reported as __builtin_unreachable with a cryptic <built-in> location.
>I think it is better to just leave it out if -fsanitize=unreachable.
>
>Changed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
>ok
>for trunk?
OK.
Richard.
>2017-12-05 Jakub Jelinek <jakub@redhat.com>
>
> * cp-gimplify.c (cp_maybe_instrument_return): Don't add
> __builtin_unreachable if -O0 or if -fsanitize=unreachable.
>
> * g++.dg/missing-return.C: Add -O to dg-options.
>
>--- gcc/cp/cp-gimplify.c.jj 2017-12-04 22:29:04.759741988 +0100
>+++ gcc/cp/cp-gimplify.c 2017-12-04 22:37:07.283784470 +0100
>@@ -1554,6 +1554,18 @@ cp_maybe_instrument_return (tree fndecl)
> || !targetm.warn_func_return (fndecl))
> return;
>
>+ if (!sanitize_flags_p (SANITIZE_RETURN, fndecl)
>+ /* Don't add __builtin_unreachable () if not optimizing, it will
>not
>+ improve any optimizations in that case, just break UB code.
>+ Don't add it if -fsanitize=unreachable -fno-sanitize=return either,
>+ UBSan covers this with ubsan_instrument_return above where
>sufficient
>+ information is provided, while the __builtin_unreachable () below
>+ if return sanitization is disabled will just result in hard to
>+ understand runtime error without location. */
>+ && (!optimize
>+ || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl)))
>+ return;
>+
> tree t = DECL_SAVED_TREE (fndecl);
> while (t)
> {
>--- gcc/testsuite/g++.dg/missing-return.C.jj 2017-11-06
>17:24:11.000000000 +0100
>+++ gcc/testsuite/g++.dg/missing-return.C 2017-12-05 00:25:13.028800127
>+0100
>@@ -1,5 +1,5 @@
> /* { dg-do compile } */
>-/* { dg-options "-Wreturn-type -fdump-tree-optimized" } */
>+/* { dg-options "-Wreturn-type -fdump-tree-optimized -O" } */
>
> int foo(int a)
> {
>
> Jakub