On Thu, Apr 3, 2025 at 11:56 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> r8-3988-g356fcc67fba52b added code to turn return statements into
> __builtin_unreachable
> calls inside noreturn functions but only while optimizing. Since
> -funreachable-traps
> was added (r13-1204-gd68d3664253696), it is a good idea to move over to using
> __builtin_unreachable (and the trap version with this option which defaults
> at -O0 and -0g)
> instead of just a follow through even at -O0.
>
> This also fixes a regression when inlining a noreturn function that returns
> at -O0 (due to always_inline)
> as we would get an empty bb which has no successor edge instead of one with a
> call to __builtin_unreachable.
>
> I also noticed there was no testcase testing the warning about
> __builtin_return inside a noreturn function
> so I added a testcase there.
>
> Bootstrapped and tested on x86_64-linux-gnu.
OK.
Thanks,
Richard.
> PR ipa/119599
>
> gcc/ChangeLog:
>
> * tree-cfg.cc (pass_warn_function_return::execute): Turn return
> statements always
> into __builtin_unreachable calls.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/pr119599-1.c: New test.
> * gcc.dg/builtin-apply5.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/testsuite/gcc.dg/builtin-apply5.c | 23 +++++++++++++++++++
> gcc/testsuite/gcc.dg/torture/pr119599-1.c | 27 +++++++++++++++++++++++
> gcc/tree-cfg.cc | 20 +++++++++--------
> 3 files changed, 61 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/builtin-apply5.c
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr119599-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-apply5.c
> b/gcc/testsuite/gcc.dg/builtin-apply5.c
> new file mode 100644
> index 00000000000..16892f76a8a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-apply5.c
> @@ -0,0 +1,23 @@
> +/* { dg-options "-O2 -Wmissing-noreturn -fgnu89-inline" } */
> +/* { dg-additional-options "-mno-mmx" { target { { i?86-*-* x86_64-*-* } &&
> ia32 } } } */
> +/* { dg-do compile } */
> +
> +extern void abort (void);
> +
> +double
> +foo (int arg)
> +{
> + if (arg != 116)
> + abort();
> + return arg + 1;
> +}
> +
> +__attribute__((noreturn))
> +double
> +bar (int arg)
> +{
> + foo (arg);
> + __builtin_return (__builtin_apply ((void (*) ()) foo, /* { dg-warning
> "'noreturn' function does return" } */
> + __builtin_apply_args (), 16));
> +}
> +
> diff --git a/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> new file mode 100644
> index 00000000000..4fbd228a359
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-einline" } */
> +
> +/* PR ipa/119599 */
> +/* inlining a noreturn function which returns
> + can cause an ICE when dealing finding an unreachable block.
> + We should get a __builtin_unreachable after the inliing. */
> +
> +
> +void baz (void);
> +
> +static inline __attribute__((always_inline, noreturn)) void
> +bar (void)
> +{
> + static volatile int t = 0;
> + if (t == 0)
> + baz ();
> +} /* { dg-warning "function does return" } */
> +
> +void
> +foo (void)
> +{
> + bar ();
> +}
> +
> +/* After inlining, we should have call to __builtin_unreachable now. */
> +/* { dg-final { scan-tree-dump "__builtin_unreachable " "einline" } } */
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index f25480cf6fc..6a29a56ca9a 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -9808,18 +9808,20 @@ pass_warn_function_return::execute (function *fun)
> (e = ei_safe_edge (ei)); )
> {
> last = *gsi_last_bb (e->src);
> - if ((gimple_code (last) == GIMPLE_RETURN
> - || gimple_call_builtin_p (last, BUILT_IN_RETURN))
> - && location == UNKNOWN_LOCATION
> - && ((location = LOCATION_LOCUS (gimple_location (last)))
> - != UNKNOWN_LOCATION)
> - && !optimize)
> - break;
> - /* When optimizing, replace return stmts in noreturn functions
> + /* Warn about __builtin_return .*/
> + if (gimple_call_builtin_p (last, BUILT_IN_RETURN)
> + && location == UNKNOWN_LOCATION)
> + {
> + location = LOCATION_LOCUS (gimple_location (last));
> + ei_next (&ei);
> + }
> + /* Replace return stmts in noreturn functions
> with __builtin_unreachable () call. */
> - if (optimize && gimple_code (last) == GIMPLE_RETURN)
> + else if (gimple_code (last) == GIMPLE_RETURN)
> {
> location_t loc = gimple_location (last);
> + if (location == UNKNOWN_LOCATION)
> + location = LOCATION_LOCUS (loc);
> gimple *new_stmt = gimple_build_builtin_unreachable (loc);
> gimple_stmt_iterator gsi = gsi_for_stmt (last);
> gsi_replace (&gsi, new_stmt, true);
> --
> 2.43.0
>
