On 07/11/2009 05:59 AM, Jan Hubicka wrote:
Re: http://gcc.gnu.org/ml/gcc-patches/2009-03/msg01404.html
Do you have test cases for this?
Changing can_throw_internal/external to depend on whether or not future
inlining is possible looks *very* wrong to me. Surely the only thing
that matters for new code that might appear "below" this position in the
tree is whether or not it might throw, and the only thing that changes
with inlining is increased knowledge of whether and how it throws.
The problem here is fact that MUST_NOT_THROW region reachable only via
runtime is handled completely via runtime, however MUST_NOT_THROW region
reachable via RESX is eventually going to be handled by direct
std::terminate call, since RESX will eventually get translated as direct
goto to the MIST_NOT_THROW reciever.
I'm committing the following test case that displays the bug. It does
in fact pass with mainline, and does in fact fail with gcc 4.4.0.
I spent two days trying to come up with some cleaner way to fix this bug
than the inlinable flag you pass around, but to no avail. The only
thing better I could think of is some global flag (or state variable)
that indicates whether or not inlining is complete. At least then we
would not have to pass around that flag. But I wouldn't want to
introduce yet another boolean state variable; I'd much prefer all of the
existing state variables we have be consolidated, and I can't justify
spending the time on that just now.
Well, we can either teach inlinable_call_p to handle your new indirect
calls as "for sure uninlinable"
This is the approach I'll take. I've already hacked on an extra bit in
the gimple call subcode to indicate whether an indirect call is nothrow;
I might as well add another bit to say an indirect call is noinline.
r~
--- testsuite/g++.dg/opt/eh4.C (revision 149703)
+++ testsuite/g++.dg/opt/eh4.C (local)
@@ -0,0 +1,59 @@
+// { dg-do run }
+// { dg-options "-O3" }
+
+// Make sure that the call to terminate within F2 is not eliminated
+// by incorrect MUST_NOT_THROW optimization. Note that we expect F1
+// to be inlined into F2 in order to expose this case.
+
+#include <cstdlib>
+#include <exception>
+
+static volatile int zero = 0;
+
+// Note that we need F0 to not be marked nothrow, though we don't actually
+// want a throw to happen at runtime here. The noinline tag is merely to
+// make sure the assembly in F0 is not unnecessarily complex.
+static void __attribute__((noinline)) f0()
+{
+ if (zero != 0)
+ throw 0;
+}
+
+struct S1
+{
+ S1() { }
+ ~S1() { f0(); }
+};
+
+static void f1()
+{
+ S1 s1;
+ throw 1;
+}
+
+struct S2
+{
+ S2() { }
+ ~S2() { f1(); }
+};
+
+static void __attribute__((noinline)) f2()
+{
+ S2 s2;
+ throw 2;
+}
+
+static void pass()
+{
+ exit (0);
+}
+
+int main()
+{
+ std::set_terminate (pass);
+ try {
+ f2();
+ } catch (...) {
+ }
+ abort ();
+}
