Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 19/07/16 10:32 +0100, Jonathan Wakely wrote: On 18/07/16 12:49 -0400, Jason Merrill wrote: Perhaps the right answer is to drop support for catching nullptr as a pointers to member from the language. Yes, I've been drafting a ballot comment along those lines. On the CWG reflector Richard Smith suggested using static objects as the result for pointer to member handlers. I had tried that unsuccessfully, but must have done something wrong because it works fine, and avoids any races. Tested x86_64-linux. I'll commit this to trunk later today. commit 6cc1a2bca8dddb8ff5994849fcd3ee22de8776ed Author: Jonathan WakelyDate: Wed Jul 20 12:49:50 2016 +0100 Use static pointer to member when catching nullptr libstdc++-v3: * libsupc++/pbase_type_info.cc (__pbase_type_info::__do_catch): Use static objects for catching nullptr as pointer to member types. gcc/testsuite: * g++.dg/cpp0x/nullptr35.C: Change expected result for catching as pointer to member function and also test catching by reference. diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C index c84966f..d932114 100644 --- a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C +++ b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C @@ -39,7 +39,7 @@ int main() caught(4); throw; } -} catch (int (A::*pmf)()) { // FIXME: currently unsupported +} catch (int (A::*pmf)()) { if (pmf == nullptr) caught(8); throw; @@ -47,6 +47,35 @@ int main() } catch (nullptr_t) { } - if (result != 7) // should be 15 + try { +try { + try { +try { + try { +throw nullptr; + } catch (void* const& p) { +if (p == nullptr) + caught(16); +throw; + } +} catch (void(* const& pf)()) { + if (pf == nullptr) +caught(32); + throw; +} + } catch (int A::* const& pm) { +if (pm == nullptr) + caught(64); +throw; + } +} catch (int (A::* const& pmf)()) { + if (pmf == nullptr) +caught(128); + throw; +} + } catch (nullptr_t) { + } + + if (result != 255) abort (); } diff --git a/libstdc++-v3/libsupc++/pbase_type_info.cc b/libstdc++-v3/libsupc++/pbase_type_info.cc index a2993e4..ff6b756 100644 --- a/libstdc++-v3/libsupc++/pbase_type_info.cc +++ b/libstdc++-v3/libsupc++/pbase_type_info.cc @@ -50,14 +50,16 @@ __do_catch (const type_info *thr_type, { if (__pointee->__is_function_p ()) { - // A pointer-to-member-function is two words but the - // nullptr_t exception object at *(nullptr_t*)*thr_obj is only - // one word, so we can't safely return it as a PMF. FIXME. - return false; + using pmf_type = void (__pbase_type_info::*)(); + static const pmf_type pmf = nullptr; + *thr_obj = const_cast (); + return true; } else { - *(ptrdiff_t*)*thr_obj = -1; // null pointer to data member + using pm_type = int __pbase_type_info::*; + static const pm_type pm = nullptr; + *thr_obj = const_cast (); return true; } }
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 18/07/16 12:49 -0400, Jason Merrill wrote: Perhaps the right answer is to drop support for catching nullptr as a pointers to member from the language. Yes, I've been drafting a ballot comment along those lines.
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
Perhaps the right answer is to drop support for catching nullptr as a pointers to member from the language. Jason
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 18/07/16 11:51 +0100, Jonathan Wakely wrote: say anything about the identity of the caught objects when nullptr is thrown). Not sure what happened to this sentence, it was supposed to say: (The standard doesn't say anything about the identity of the caught objects when nullptr is thrown).
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 16/07/16 11:11 +0200, Jakub Jelinek wrote: On Fri, Jul 15, 2016 at 11:14:03PM +0100, Jonathan Wakely wrote: On 15/07/16 22:53 +0100, Jonathan Wakely wrote: >On 15/07/16 23:36 +0200, Jakub Jelinek wrote: >>On Fri, Jul 15, 2016 at 10:07:03PM +0100, Jonathan Wakely wrote: + if (typeid (*this) == typeid(__pointer_type_info)) +{ + *thr_obj = nullptr; + return true; >> >>But you have the above store too. > >That doesn't write to the exception object, it only does a single >dereference (compared to the double dereference of the racy write), so >it writes to the local variable in the PERSONALITY_FUNCTION in >eh_personality.cc > >So that shouldn't race with other threads. I think. > TSan agrees. When I compile my test and yours (and include libsupc++/pbase_type_info.cc in the executable, so the writes are also instrumented by tsan) then I see races for the *(ptrdiff_t*)*thr_obj writes but not the *thr_obj ones. It's only the ptr-to-data-member case that scribbles in the actual exception object. In: struct A { int a; }; int a; int main () { try { throw nullptr; } catch (int * const ) { __builtin_printf ("%p %p\n", p, ); } try { throw nullptr; } catch (int A::* const ) { __builtin_printf ("%p\n", ); asm volatile ("" : : "r" ()); } try { throw } catch (int * const ) { __builtin_printf ("%p %p\n", p, ); } } I see being the address passed to __cxa_throw only in the second case, where does the reference point to in the other two cases? Some temporary in main? I'm not sure what the difference is there. There's some difference in how pointers and non-pointers are handled. Does that mean if you rethrow the exception multiple times in different threads you get references to the same object for PMF and to different objects for pointers? No, for the throw case you'll get the same exception object in every thread. say anything about the identity of the caught objects when nullptr is thrown).
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
* g++.dg/cpp0x/nullptr35.C (caught): Fix typo. diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C index 2f93ce1..c84966f 100644 --- a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C +++ b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C @@ -11,7 +11,7 @@ int result = 0; void __attribute__((noinline)) caught(int bit) { - result &= bit; + result |= bit; } struct A { }; -- 2.9.1 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On Fri, Jul 15, 2016 at 11:14:03PM +0100, Jonathan Wakely wrote: > On 15/07/16 22:53 +0100, Jonathan Wakely wrote: > >On 15/07/16 23:36 +0200, Jakub Jelinek wrote: > >>On Fri, Jul 15, 2016 at 10:07:03PM +0100, Jonathan Wakely wrote: > + if (typeid (*this) == typeid(__pointer_type_info)) > +{ > + *thr_obj = nullptr; > + return true; > >> > >>But you have the above store too. > > > >That doesn't write to the exception object, it only does a single > >dereference (compared to the double dereference of the racy write), so > >it writes to the local variable in the PERSONALITY_FUNCTION in > >eh_personality.cc > > > >So that shouldn't race with other threads. I think. > > > > TSan agrees. When I compile my test and yours (and include > libsupc++/pbase_type_info.cc in the executable, so the writes are also > instrumented by tsan) then I see races for the *(ptrdiff_t*)*thr_obj > writes but not the *thr_obj ones. > > It's only the ptr-to-data-member case that scribbles in the actual > exception object. In: struct A { int a; }; int a; int main () { try { throw nullptr; } catch (int * const ) { __builtin_printf ("%p %p\n", p, ); } try { throw nullptr; } catch (int A::* const ) { __builtin_printf ("%p\n", ); asm volatile ("" : : "r" ()); } try { throw } catch (int * const ) { __builtin_printf ("%p %p\n", p, ); } } I see being the address passed to __cxa_throw only in the second case, where does the reference point to in the other two cases? Some temporary in main? Does that mean if you rethrow the exception multiple times in different threads you get references to the same object for PMF and to different objects for pointers? Jakub
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 15/07/16 22:53 +0100, Jonathan Wakely wrote: On 15/07/16 23:36 +0200, Jakub Jelinek wrote: On Fri, Jul 15, 2016 at 10:07:03PM +0100, Jonathan Wakely wrote: + if (typeid (*this) == typeid(__pointer_type_info)) +{ + *thr_obj = nullptr; + return true; But you have the above store too. That doesn't write to the exception object, it only does a single dereference (compared to the double dereference of the racy write), so it writes to the local variable in the PERSONALITY_FUNCTION in eh_personality.cc So that shouldn't race with other threads. I think. TSan agrees. When I compile my test and yours (and include libsupc++/pbase_type_info.cc in the executable, so the writes are also instrumented by tsan) then I see races for the *(ptrdiff_t*)*thr_obj writes but not the *thr_obj ones. It's only the ptr-to-data-member case that scribbles in the actual exception object.
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 15/07/16 23:36 +0200, Jakub Jelinek wrote: On Fri, Jul 15, 2016 at 10:07:03PM +0100, Jonathan Wakely wrote: >+ if (typeid (*this) == typeid(__pointer_type_info)) >+{ >+ *thr_obj = nullptr; >+ return true; But you have the above store too. That doesn't write to the exception object, it only does a single dereference (compared to the double dereference of the racy write), so it writes to the local variable in the PERSONALITY_FUNCTION in eh_personality.cc So that shouldn't race with other threads. I think.
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On Fri, Jul 15, 2016 at 10:07:03PM +0100, Jonathan Wakely wrote: > >+ if (typeid (*this) == typeid(__pointer_type_info)) > >+{ > >+ *thr_obj = nullptr; > >+ return true; But you have the above store too. Can't you rethrow_exception in certain threads trying to catch it as pointer to data member and in other threads as a pointer? #include #include #include std::exception_ptr p; struct A { }; void t() { try { std::rethrow_exception(p); } catch (int A::* const& e) { assert( e == nullptr ); } } void h() { try { std::rethrow_exception(p); } catch (int *& e) { assert( e == nullptr ); } } int main() { try { throw nullptr; } catch (...) { p = std::current_exception(); } std::thread t1(t); std::thread t2(h); std::thread t3(t); std::thread t4(h); t1.join(); t2.join(); t3.join(); t4.join(); } Jakub
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On 15/07/16 12:47 +0100, Jonathan Wakely wrote: diff --git a/libstdc++-v3/libsupc++/pbase_type_info.cc b/libstdc++-v3/libsupc++/pbase_type_info.cc index d47fb23..a2993e4 100644 --- a/libstdc++-v3/libsupc++/pbase_type_info.cc +++ b/libstdc++-v3/libsupc++/pbase_type_info.cc @@ -38,6 +38,31 @@ __do_catch (const type_info *thr_type, return true; // same type #if __cpp_rtti + if (*thr_type == typeid (nullptr)) +{ + // A catch handler for any pointer type matches nullptr_t. + if (typeid (*this) == typeid(__pointer_type_info)) +{ + *thr_obj = nullptr; + return true; +} + else if (typeid (*this) == typeid(__pointer_to_member_type_info)) +{ + if (__pointee->__is_function_p ()) +{ + // A pointer-to-member-function is two wordsbut the + // nullptr_t exception object at *(nullptr_t*)*thr_obj is only + // one word, so we can't safely return it as a PMF. FIXME. + return false; +} + else +{ + *(ptrdiff_t*)*thr_obj = -1; // null pointer to data member This store could race with accesses in other threads, if the same exception object gets thrown in multiple threads and the value of the pointer-to-data-member is read concurrently with the write: #include #include #include std::exception_ptr p; struct A { }; void t() { try { std::rethrow_exception(p); } catch (int A::* const& e) { assert( e == nullptr ); } } int main() { try { throw nullptr; } catch (...) { p = std::current_exception(); } std::thread t1(t); std::thread t2(t); t1.join(); t2.join(); } We could use __atomic_store to do the write, but the reads would still be non-atomic. The attached patch makes __cxa_throw do that write on the initial throw, so that a thrown nullptr_t always has the right content to act as a null pointer-to-member. However that adds overhead for every throw. A more efficient solution might be for the front-end to do that write when it sees a nullptr_t being thrown. diff --git a/libstdc++-v3/libsupc++/eh_throw.cc b/libstdc++-v3/libsupc++/eh_throw.cc index 9aac218..e6b155f 100644 --- a/libstdc++-v3/libsupc++/eh_throw.cc +++ b/libstdc++-v3/libsupc++/eh_throw.cc @@ -62,6 +62,11 @@ __cxxabiv1::__cxa_throw (void *obj, std::type_info *tinfo, { PROBE2 (throw, obj, tinfo); +#if __cpp_rtti + if (*tinfo == typeid(nullptr)) +*(ptrdiff_t*)obj = -1; // make it act as a null pointer-to-data-member +#endif + __cxa_eh_globals *globals = __cxa_get_globals (); globals->uncaughtExceptions += 1; diff --git a/libstdc++-v3/libsupc++/pbase_type_info.cc b/libstdc++-v3/libsupc++/pbase_type_info.cc index a2993e4..4cb626c 100644 --- a/libstdc++-v3/libsupc++/pbase_type_info.cc +++ b/libstdc++-v3/libsupc++/pbase_type_info.cc @@ -57,7 +57,7 @@ __do_catch (const type_info *thr_type, } else { - *(ptrdiff_t*)*thr_obj = -1; // null pointer to data member + // __cxa_throw already wrote (ptrdiff_t)-1 to the object. return true; } }