Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type

[Jonathan Wakely]

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 Wakely 
Date:   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

[Jonathan Wakely]

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

[Jason Merrill]

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

[Jonathan Wakely]

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

[Jonathan Wakely]

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

[Andreas Schwab]

* 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

[Jakub Jelinek]

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

[Jonathan Wakely]

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

[Jonathan Wakely]

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

[Jakub Jelinek]

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

[Jonathan Wakely]

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 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;
+}
+  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;
 }
 }

[PATCH] c++/58796 Make nullptr match exception handlers of pointer type

[Jonathan Wakely]

This allows exception handlers of pointer and pointer to data members
to match exceptions of type std::nullptr_t.

Pointers to member functions don't work yet, because a nullptr_t
exception object is only a single word, and we need two words for a
pointer to member function, and we don't have anywhere to put those
two words in the exception object. I'm going to raise this with the
C++ ABI group to coordinate with other implementations.

Tested x86_64-linux, I plan to commit this to trunk soon.


libstdc++-v3:

PR c++/58796
* libsupc++/pbase_type_info.cc (__pbase_type_info::__do_catch): Make
nullptr match handlers of pointer type.

gcc/testsuite:

PR c++/58796
* g++.dg/cpp0x/nullptr21.C: Remove void* handlers.
* g++.dg/cpp0x/nullptr35.C: New test.



commit 2e6bb9fbc1d47915bdfe487efd7e870600b4b442
Author: Jonathan Wakely 
Date:   Thu Jul 14 14:33:24 2016 +0100

c++/58796 Make nullptr match exception handlers of pointer type

libstdc++-v3:

	PR c++/58796
	* libsupc++/pbase_type_info.cc (__pbase_type_info::__do_catch): Make
	nullptr match handlers of pointer type.

gcc/testsuite:

	PR c++/58796
	* g++.dg/cpp0x/nullptr21.C: Remove void* handlers.
	* g++.dg/cpp0x/nullptr35.C: New test.

diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr21.C b/gcc/testsuite/g++.dg/cpp0x/nullptr21.C
index 89884b9..c6f560e 100644
--- a/gcc/testsuite/g++.dg/cpp0x/nullptr21.C
+++ b/gcc/testsuite/g++.dg/cpp0x/nullptr21.C
@@ -18,8 +18,6 @@ int main()
 {
   try {
 throw nullptr;
-  } catch (void*) {
-foo (0, 1);
   } catch (bool) {
 foo (0, 2);
   } catch (int) {
@@ -35,8 +33,6 @@ int main()
   nullptr_t mynull = 0;
   try {
 throw mynull;
-  } catch (void*) {
-foo (1, 1);
   } catch (bool) {
 foo (1, 2);
   } catch (int) {
diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C
new file mode 100644
index 000..2f93ce1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C
@@ -0,0 +1,52 @@
+// { dg-do run { target c++11 } }
+
+// Test catching as pointer and pointer to member types, [except.handle] p3.
+
+extern "C" void abort (void);
+
+typedef decltype(nullptr) nullptr_t;
+
+int result = 0;
+
+void __attribute__((noinline))
+caught(int bit)
+{
+  result &= bit;
+}
+
+struct A { };
+
+int main()
+{
+  try {
+try {
+  try {
+try {
+  try {
+throw nullptr;
+  } catch (void* p) {
+if (p == nullptr)
+  caught(1);
+throw;
+  }
+} catch (void(*pf)()) {
+  if (pf == nullptr)
+caught(2);
+  throw;
+}
+  } catch (int A::*pm) {
+if (pm == nullptr)
+  caught(4);
+throw;
+  }
+} catch (int (A::*pmf)()) {  // FIXME: currently unsupported
+  if (pmf == nullptr)
+caught(8);
+  throw;
+}
+  } catch (nullptr_t) {
+  }
+
+  if (result != 7) // should be 15
+abort ();
+}
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 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;
+}
+  else
+{
+  *(ptrdiff_t*)*thr_obj = -1; // null pointer to data member
+  return true;
+}
+}
+}
+
   if (typeid (*this) != typeid (*thr_type))
 return false; // not both same kind of pointers
 #endif