[PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Jason Merrill]

If a noexcept function calls a function that might throw, doing the tail
call optimization means that an exception thrown in the called function
will propagate out, breaking the noexcept specification.  So we need to
prevent the optimization in that case.

Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
regression, but it is a straightforward fix for a wrong-code bug.

	* tree-tailcall.c (find_tail_calls): Don't turn a call from a
	nothrow function to a might-throw function into a tail call.
---
 gcc/tree-tailcall.c                         |  7 +++++++
 gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C | 11 +++++++++++
 gcc/ChangeLog                               |  6 ++++++
 3 files changed, 24 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C

diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c
index afe8931b5f0..e0265b22dd5 100644
--- a/gcc/tree-tailcall.c
+++ b/gcc/tree-tailcall.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-into-ssa.h"
 #include "tree-dfa.h"
 #include "except.h"
+#include "tree-eh.h"
 #include "dbgcnt.h"
 #include "cfgloop.h"
 #include "common/common-target.h"
@@ -472,6 +473,12 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
       && !auto_var_in_fn_p (ass_var, cfun->decl))
     return;
 
+  /* If the call might throw an exception that wouldn't propagate out of
+     cfun, we can't transform to a tail or sibling call (82081).  */
+  if (stmt_could_throw_p (cfun, stmt)
+      && !stmt_can_throw_external (cfun, stmt))
+    return;
+
   /* We found the call, check whether it is suitable.  */
   tail_recursion = false;
   func = gimple_call_fndecl (call);
diff --git a/gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C b/gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C
new file mode 100644
index 00000000000..c67af6e41c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/tail-call-1.C
@@ -0,0 +1,11 @@
+// PR c++/82081
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-O2 -fdump-tree-optimized" }
+// { dg-final { scan-tree-dump-not "tail call" "optimized" } }
+
+int g(int) ;
+
+int f() noexcept {
+    int i = 42, j = 43;
+    return g(i+j);
+}
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 02d3d07d0e4..77ab20929ff 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2019-04-11  Jason Merrill  <jason@redhat.com>
+
+	PR c++/82081 - tail call optimization breaks noexcept
+	* tree-tailcall.c (find_tail_calls): Don't turn a call from a
+	nothrow function to a might-throw function into a tail call.
+
 2019-04-12  Jakub Jelinek  <jakub@redhat.com>
 	
 	PR rtl-optimization/89965

base-commit: b7a39acf193fd19e41502d5e2cf6b3fa8b44dbac
-- 
2.20.1

Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Jeff Law]

On 4/12/19 3:24 PM, Jason Merrill wrote:
> If a noexcept function calls a function that might throw, doing the tail
> call optimization means that an exception thrown in the called function
> will propagate out, breaking the noexcept specification.  So we need to
> prevent the optimization in that case.
> 
> Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
> regression, but it is a straightforward fix for a wrong-code bug.
> 
> 	* tree-tailcall.c (find_tail_calls): Don't turn a call from a
> 	nothrow function to a might-throw function into a tail call.
I'd go on the trunk.  It's a wrong-code issue, what we're doing is just
plain wrong.  One could even make a case for backporting to the branches.

jeff

ps.  I'm a bit surprised it hasn't been reported until now.

Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Richard Biener]

On Sat, Apr 13, 2019 at 12:34 AM Jeff Law <law@redhat.com> wrote:
>
> On 4/12/19 3:24 PM, Jason Merrill wrote:
> > If a noexcept function calls a function that might throw, doing the tail
> > call optimization means that an exception thrown in the called function
> > will propagate out, breaking the noexcept specification.  So we need to
> > prevent the optimization in that case.
> >
> > Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
> > regression, but it is a straightforward fix for a wrong-code bug.
> >
> >       * tree-tailcall.c (find_tail_calls): Don't turn a call from a
> >       nothrow function to a might-throw function into a tail call.
> I'd go on the trunk.  It's a wrong-code issue, what we're doing is just
> plain wrong.  One could even make a case for backporting to the branches.

Hmm, how's this different from adding another indirection?  That is,
I don't understand why the tailcall is the issue here, shouldn't unwind
still stop at the noexcept caller?  Thus, isn't this wrong CFI instead?

Of course I know to little about this.

Btw, doesn't your check also prevent tail/sibling calls when
the caller wraps it into a try { } catch (...) {}?  Or does unwind
not work in that case either?

Btw, I'd like to see a runtime testcase that fails.

Richard.

> jeff
>
> ps.  I'm a bit surprised it hasn't been reported until now.

Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Andrew Pinski]

On Sun, Apr 14, 2019 at 11:50 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Sat, Apr 13, 2019 at 12:34 AM Jeff Law <law@redhat.com> wrote:
> >
> > On 4/12/19 3:24 PM, Jason Merrill wrote:
> > > If a noexcept function calls a function that might throw, doing the tail
> > > call optimization means that an exception thrown in the called function
> > > will propagate out, breaking the noexcept specification.  So we need to
> > > prevent the optimization in that case.
> > >
> > > Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
> > > regression, but it is a straightforward fix for a wrong-code bug.
> > >
> > >       * tree-tailcall.c (find_tail_calls): Don't turn a call from a
> > >       nothrow function to a might-throw function into a tail call.
> > I'd go on the trunk.  It's a wrong-code issue, what we're doing is just
> > plain wrong.  One could even make a case for backporting to the branches.
>
> Hmm, how's this different from adding another indirection?  That is,
> I don't understand why the tailcall is the issue here, shouldn't unwind
> still stop at the noexcept caller?  Thus, isn't this wrong CFI instead?

noexcept caller is no longer on the stack so the unwinder does not see it.
It is not the tail call from a normal function to a noexcept that is
an issue but rather inside a noexcept caller to a normal function.

>
> Of course I know to little about this.
>
> Btw, doesn't your check also prevent tail/sibling calls when
> the caller wraps it into a try { } catch (...) {}?  Or does unwind
> not work in that case either?
>
> Btw, I'd like to see a runtime testcase that fails.

There is one in the bug report.  Though it would not work for the
testsuite.  It should not be hard to change it to be one that works
for the testsuite.

Thanks,
Andrew Pinski

>
> Richard.
>
> > jeff
> >
> > ps.  I'm a bit surprised it hasn't been reported until now.

Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Richard Biener]

On Mon, Apr 15, 2019 at 7:09 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Sun, Apr 14, 2019 at 11:50 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Sat, Apr 13, 2019 at 12:34 AM Jeff Law <law@redhat.com> wrote:
> > >
> > > On 4/12/19 3:24 PM, Jason Merrill wrote:
> > > > If a noexcept function calls a function that might throw, doing the tail
> > > > call optimization means that an exception thrown in the called function
> > > > will propagate out, breaking the noexcept specification.  So we need to
> > > > prevent the optimization in that case.
> > > >
> > > > Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
> > > > regression, but it is a straightforward fix for a wrong-code bug.
> > > >
> > > >       * tree-tailcall.c (find_tail_calls): Don't turn a call from a
> > > >       nothrow function to a might-throw function into a tail call.
> > > I'd go on the trunk.  It's a wrong-code issue, what we're doing is just
> > > plain wrong.  One could even make a case for backporting to the branches.
> >
> > Hmm, how's this different from adding another indirection?  That is,
> > I don't understand why the tailcall is the issue here, shouldn't unwind
> > still stop at the noexcept caller?  Thus, isn't this wrong CFI instead?
>
> noexcept caller is no longer on the stack so the unwinder does not see it.
> It is not the tail call from a normal function to a noexcept that is
> an issue but rather inside a noexcept caller to a normal function.

Hmm, OK, so essentially a tail-call cannot be represented in the CFI
program.

> >
> > Of course I know to little about this.
> >
> > Btw, doesn't your check also prevent tail/sibling calls when
> > the caller wraps it into a try { } catch (...) {}?  Or does unwind
> > not work in that case either?
> >
> > Btw, I'd like to see a runtime testcase that fails.
>
> There is one in the bug report.  Though it would not work for the
> testsuite.  It should not be hard to change it to be one that works
> for the testsuite.

With dg-additional-sources and registering a custom std::terminate
it should work I guess (or by catching SIGABRT).

The patch and the bug also suggests that an internally
throwing function cannot be tail-called either (can't find a testcase
we'd mark as tail-call here)

Richard.

> Thanks,
> Andrew Pinski
>
> >
> > Richard.
> >
> > > jeff
> > >
> > > ps.  I'm a bit surprised it hasn't been reported until now.

Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Jason Merrill]

On Tue, Apr 16, 2019 at 1:24 AM Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Apr 15, 2019 at 7:09 PM Andrew Pinski <pinskia@gmail.com> wrote:
> > On Sun, Apr 14, 2019 at 11:50 PM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Sat, Apr 13, 2019 at 12:34 AM Jeff Law <law@redhat.com> wrote:
> > > >
> > > > On 4/12/19 3:24 PM, Jason Merrill wrote:
> > > > > If a noexcept function calls a function that might throw, doing the tail
> > > > > call optimization means that an exception thrown in the called function
> > > > > will propagate out, breaking the noexcept specification.  So we need to
> > > > > prevent the optimization in that case.
> > > > >
> > > > > Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
> > > > > regression, but it is a straightforward fix for a wrong-code bug.
> > > > >
> > > > >       * tree-tailcall.c (find_tail_calls): Don't turn a call from a
> > > > >       nothrow function to a might-throw function into a tail call.
> > > > I'd go on the trunk.  It's a wrong-code issue, what we're doing is just
> > > > plain wrong.  One could even make a case for backporting to the branches.
> > >
> > > Hmm, how's this different from adding another indirection?  That is,
> > > I don't understand why the tailcall is the issue here, shouldn't unwind
> > > still stop at the noexcept caller?  Thus, isn't this wrong CFI instead?
> >
> > noexcept caller is no longer on the stack so the unwinder does not see it.
> > It is not the tail call from a normal function to a noexcept that is
> > an issue but rather inside a noexcept caller to a normal function.
>
> Hmm, OK, so essentially a tail-call cannot be represented in the CFI
> program.

Right.  Because the "caller" frame no longer exists.

> > > Of course I know to little about this.
> > >
> > > Btw, doesn't your check also prevent tail/sibling calls when
> > > the caller wraps it into a try { } catch (...) {}?  Or does unwind
> > > not work in that case either?
> > >
> > > Btw, I'd like to see a runtime testcase that fails.
> >
> > There is one in the bug report.  Though it would not work for the
> > testsuite.  It should not be hard to change it to be one that works
> > for the testsuite.
>
> With dg-additional-sources and registering a custom std::terminate
> it should work I guess (or by catching SIGABRT).
>
> The patch and the bug also suggests that an internally
> throwing function cannot be tail-called either (can't find a testcase
> we'd mark as tail-call here)

If you mean a call wrapped in try/catch, that is correct.  The
tail-call optimization breaks all exception handlers, so the patch
prevents it if the call can throw and is in an active exception
region.

Jason

Re: [PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Jakub Jelinek]

On Fri, Apr 12, 2019 at 05:24:26PM -0400, Jason Merrill wrote:
> If a noexcept function calls a function that might throw, doing the tail
> call optimization means that an exception thrown in the called function
> will propagate out, breaking the noexcept specification.  So we need to
> prevent the optimization in that case.
> 
> Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
> regression, but it is a straightforward fix for a wrong-code bug.
> 
> 	* tree-tailcall.c (find_tail_calls): Don't turn a call from a
> 	nothrow function to a might-throw function into a tail call.

Ok for trunk (i.e. GCC 10) and we can backport later to 9.2 if it works fine
on the trunk.

	Jakub

[PATCH] (RFA tree-tailcall) PR c++/82081 - tail call optimization breaks noexcept

[Ville Voutilainen]

Jason wrote:

>If a noexcept function calls a function that might throw, doing the tail
call optimization means that an exception thrown in the called function
will propagate out, breaking the noexcept specification.  So we need to
prevent the optimization in that case.

>Tested x86_64-pc-linux-gnu.  OK for trunk or hold for GCC 10?  This isn't a
regression, but it is a straightforward fix for a wrong-code bug.

It is indeed not a regression, but exceptions escaping through a noexcept
are really unfortunate.