[v3] libstdc++: fix pointer type exception catch [PR105387]
Commit Message
Hello,
two weeks ago I submitted the second version of the patch PR105387 for the bug 105387. Now I added a pointer-to-member exception test just to make sure that it doesn't break in case RTTI is enabled. The test is disabled if RTTI is disabled. I didn't receive any feedback so far regarding the second version of the patch. Is there any issue preventing acceptance?
I ran the conformance tests on libstdc++v3 by running
make -j 18 check RUNTESTFLAGS=conformance.exp
Results for the current version (only difference is the added pointer-to-member test):
Without RTTI before applying patch:
=== libstdc++ Summary ===
# of expected passes 14560
# of unexpected failures 5
# of expected failures 95
# of unsupported tests 702
Without RTTI after applying patch:
=== libstdc++ Summary ===
# of expected passes 14562
# of unexpected failures 5
# of expected failures 95
# of unsupported tests 703
With RTTI before applying patch:
=== libstdc++ Summary ===
# of expected passes 14598
# of unexpected failures 2
# of expected failures 95
# of unsupported tests 683
With RTTI after applying patch:
=== libstdc++ Summary ===
# of expected passes 14602
# of unexpected failures 2
# of expected failures 95
# of unsupported tests 683
Given that the pointer-to-member test is disabled when RTTI is disabled, the results look logical to me.
Comments
On Wed, 25 May 2022 at 03:30, Jakob Hasse via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Hello,
>
> two weeks ago I submitted the second version of the patch PR105387 for the bug 105387. Now I added a pointer-to-member exception test just to make sure that it doesn't break in case RTTI is enabled. The test is disabled if RTTI is disabled. I didn't receive any feedback so far regarding the second version of the patch. Is there any issue preventing acceptance?
Just a lack of time to review it properly.
It's on my list.
>
> I ran the conformance tests on libstdc++v3 by running
> make -j 18 check RUNTESTFLAGS=conformance.exp
>
> Results for the current version (only difference is the added pointer-to-member test):
>
> Without RTTI before applying patch:
> === libstdc++ Summary ===
>
> # of expected passes 14560
> # of unexpected failures 5
> # of expected failures 95
> # of unsupported tests 702
>
> Without RTTI after applying patch:
> === libstdc++ Summary ===
>
> # of expected passes 14562
> # of unexpected failures 5
> # of expected failures 95
> # of unsupported tests 703
>
> With RTTI before applying patch:
> === libstdc++ Summary ===
>
> # of expected passes 14598
> # of unexpected failures 2
> # of expected failures 95
> # of unsupported tests 683
>
> With RTTI after applying patch:
> === libstdc++ Summary ===
>
> # of expected passes 14602
> # of unexpected failures 2
> # of expected failures 95
> # of unsupported tests 683
>
> Given that the pointer-to-member test is disabled when RTTI is disabled, the results look logical to me.
>
Hello, is there any update regarding the patch PR105387 for bug 105387? We've been waiting for some time now, but the bugzilla bug is still open: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105387. If there is any issue with the patch (besides the ones we discussed before), please let us know. If there's no chance to integrate that patch, we would also like to know, to make decisions on how to handle the patch internally.
Thanks, and All the Best,
Jakob Hasse
On Tue, 11 Oct 2022 at 16:05, Jakob Hasse via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Hello, is there any update regarding the patch PR105387 for bug 105387? We've been waiting for some time now, but the bugzilla bug is still open: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105387. If there is any issue with the patch (besides the ones we discussed before), please let us know. If there's no chance to integrate that patch, we would also like to know, to make decisions on how to handle the patch internally.
Thanks for your patience. I'm going to push this patch.
I've had to adjust the tests slightly, it should use 0 not nullptr so
it can be tested with -std=c++98, and 18_support/execption_ptr is
about std::exception_ptr not catching pointers, so is the wrong place
for the new test. But I can take care of those and push it.
>
> Thanks, and All the Best,
> Jakob Hasse
> ________________________________
> From: Jonathan Wakely <jwakely.gcc@gmail.com>
> Sent: Wednesday, May 25, 2022 5:18 PM
> To: Jakob Hasse <jakob.hasse@espressif.com>
> Cc: libstdc++@gcc.gnu.org <libstdc++@gcc.gnu.org>; gcc-patches <gcc-patches@gcc.gnu.org>; Anton Maklakov <anton.maklakov@espressif.com>; Ivan Grokhotkov <ivan@espressif.com>
> Subject: Re: [PATCH v3] libstdc++: fix pointer type exception catch [PR105387]
>
> [External: This email originated outside Espressif]
>
> On Wed, 25 May 2022 at 03:30, Jakob Hasse via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > Hello,
> >
> > two weeks ago I submitted the second version of the patch PR105387 for the bug 105387. Now I added a pointer-to-member exception test just to make sure that it doesn't break in case RTTI is enabled. The test is disabled if RTTI is disabled. I didn't receive any feedback so far regarding the second version of the patch. Is there any issue preventing acceptance?
>
> Just a lack of time to review it properly.
>
> It's on my list.
>
>
> >
> > I ran the conformance tests on libstdc++v3 by running
> > make -j 18 check RUNTESTFLAGS=conformance.exp
> >
> > Results for the current version (only difference is the added pointer-to-member test):
> >
> > Without RTTI before applying patch:
> > === libstdc++ Summary ===
> >
> > # of expected passes 14560
> > # of unexpected failures 5
> > # of expected failures 95
> > # of unsupported tests 702
> >
> > Without RTTI after applying patch:
> > === libstdc++ Summary ===
> >
> > # of expected passes 14562
> > # of unexpected failures 5
> > # of expected failures 95
> > # of unsupported tests 703
> >
> > With RTTI before applying patch:
> > === libstdc++ Summary ===
> >
> > # of expected passes 14598
> > # of unexpected failures 2
> > # of expected failures 95
> > # of unsupported tests 683
> >
> > With RTTI after applying patch:
> > === libstdc++ Summary ===
> >
> > # of expected passes 14602
> > # of unexpected failures 2
> > # of expected failures 95
> > # of unsupported tests 683
> >
> > Given that the pointer-to-member test is disabled when RTTI is disabled, the results look logical to me.
> >
>
From 26004c6f26f4b2f3e664184767d861c7291f3a16 Mon Sep 17 00:00:00 2001
From: Jakob Hasse <0xjakob@users.noreply.github.com>
Date: Tue, 26 Apr 2022 12:03:47 +0800
Subject: [PATCH] [PATCH] libstdc++: fix pointer type exception catch (no RTTI)
[PR105387]
PR libstdc++/105387
__pbase_type_info::__do_catch(), used to catch pointer type exceptions,
did not check if the type info object to compare against is a pointer
type info object before doing a static down-cast to a pointer type info
object. If RTTI is disabled, this leads to the following situation:
Since a pointer type info object has additional fields, they would
end up being undefined if the actual type info object was not a pointer
type info object.
A simple check has been added before the down-cast happens.
Note that a consequence of this check is that exceptions of type
pointer-to-member cannot be caught anymore.
In case RTTI is enabled, this does not seem to be a problem because
RTTI-based checks would run before and prevent running into the bad
down-cast. Hence, the fix is disabled if RTTI is enabled and exceptions
of type pointer-to-member can still be caught.
libstdc++-v3/ChangeLog:
* libsupc++/pbase_type_info.cc (__do_catch):
* testsuite/18_support/105387.cc: New test.
Signed-off-by: Jakob Hasse <jakob.hasse@espressif.com>
---
libstdc++-v3/libsupc++/pbase_type_info.cc | 7 ++-
libstdc++-v3/testsuite/18_support/105387.cc | 61 +++++++++++++++++++
.../18_support/exception_ptr/ptr_to_member.cc | 25 ++++++++
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 libstdc++-v3/testsuite/18_support/105387.cc
create mode 100644 libstdc++-v3/testsuite/18_support/exception_ptr/ptr_to_member.cc
@@ -74,7 +74,12 @@ __do_catch (const type_info *thr_type,
// Therefore there must at least be a qualification conversion involved
// But for that to be valid, our outer pointers must be const qualified.
return false;
-
+
+#if !__cpp_rtti
+ if (!thr_type->__is_pointer_p ())
+ return false;
+#endif
+
const __pbase_type_info *thrown_type =
static_cast <const __pbase_type_info *> (thr_type);
new file mode 100644
@@ -0,0 +1,61 @@
+#include <stdexcept>
+#include <cxxabi.h>
+#include <testsuite_hooks.h>
+
+// Test cases for PR libstdc++/105387
+
+// This test is to trigger undefined behavior if the bug 105387 is present
+// in the code. Note, however, given that the bug is present, this test runs
+// into undefined behavior which can also mean that it passes.
+// It has been observed to fail quite reliably on x86_64-linux-gnu but only
+// fail sporadically on Xtensa, depending on the code placement.
+void portable_test()
+{
+ bool exception_thrown = false;
+ try {
+ throw std::runtime_error("test");
+ } catch (const char *e) {
+ VERIFY(false);
+ } catch (const std::exception &e) {
+ exception_thrown = true;
+ }
+ VERIFY(exception_thrown);
+}
+
+// This test relies on the types defined in the files typeinfo and cxxabi.h
+// It is therefore less portable then the test case above but should be
+// guaranteed to fail if the implementation has the bug 105387.
+//
+// This test case checks that __pbase_type_info::__do_catch() behaves
+// correctly when called with a non-pointer type info object as argument.
+// In particular, __pbase_type_info::__do_catch() should not cast
+// the given type object into a pointer type and try to access the
+// extended fields.
+
+void non_portable_test()
+{
+ // Create a zero-initialized buffer for allocation of the type object
+ unsigned char buffer [sizeof(__cxxabiv1::__fundamental_type_info) * 2] = {};
+
+ // Use placement-new to create the fundamental type info object in the
+ // first half of the buffer. Whenever that type info object will be
+ // casted to a pointer type info object, the extended fields of the
+ // pointer type info object will be in the second half of the buffer
+ // and hence be guaranteed zero.
+ __cxxabiv1::__fundamental_type_info *p_fund_info =
+ new(buffer) __cxxabiv1::__fundamental_type_info("fund_type");
+
+ __cxxabiv1::__pointer_type_info ptr_info("ptr_type", 0, p_fund_info);
+
+ // __do_catch is declared protected in __pointer_type_info, but public in
+ // type_info, so we upcast it here
+ std::type_info *abstract_ptr_info = static_cast<std::type_info*>(&ptr_info);
+ VERIFY(abstract_ptr_info->__do_catch(p_fund_info, nullptr, 1) == false);
+}
+
+int main()
+{
+ portable_test();
+ non_portable_test();
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,25 @@
+#include <testsuite_hooks.h>
+
+// Test related to PR libstdc++/105387
+// Check that pointer-to-member type exceptions can still be caught with -frtti.
+// { dg-require-effective-target rtti }
+
+void test_catch_ptr_to_member()
+{
+ bool exception_thrown = false;
+ struct X { int i; };
+ try {
+ throw &X::i;
+ }
+ catch (const int X::*) {
+ exception_thrown = true;
+ }
+
+ VERIFY(exception_thrown);
+}
+
+int main()
+{
+ test_catch_ptr_to_member();
+ return 0;
+}
--
2.25.1