diff mbox

std::rethrow_exception is broken

Message ID yddppl2pnv7.fsf@lokon.CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth March 31, 2014, 12:13 p.m. UTC
Jonathan Wakely <jwakely@redhat.com> writes:

> On 25/03/14 17:25 +0000, Jonathan Wakely wrote:
>>Tested x86_64-linux, I plan to commit this to trunk soon.
>>
>
>>commit 06a845f80204947afd6866109db58cc85dc87117
>>Author: Jonathan Wakely <jwakely@redhat.com>
>>Date:   Tue Mar 25 14:42:45 2014 +0000
>>
>>    	PR libstdc++/60612
>>    	* libsupc++/eh_ptr.cc: Assert __cxa_dependent_exception layout is
>>    	compatible with __cxa_exception.
>>    	* libsupc++/unwind-cxx.h (__cxa_dependent_exception): Add padding.
>>    	Fix typos in comments.
>>    	* testsuite/18_support/exception_ptr/60612-terminate.cc: New.
>>    	* testsuite/18_support/exception_ptr/60612-unexpected.cc: New.
>
> Committed to trunk.

Unfortunately, the new tests FAIL on non-C99 targets (i386-pc-solaris2.9
in my case):

FAIL: 18_support/exception_ptr/60612-terminate.cc (test for excess errors)
Excess errors:
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/18_support/exception_ptr/6061
2-terminate.cc:26:27: error: '_Exit' was not declared in this scope

WARNING: 18_support/exception_ptr/60612-terminate.cc compilation failed to produ
ce executable

The following patch fixes this, following the idiom often used in the
libstdc++ testsuite.

Tested with the appropriate runtest invocation on
i386-pc-solaris2.{9,11}.  Ok for mainline?

When looking into this, I noticed that this idiom is very widespread in
the v3 testsuite, but IMNSHO a *very bad* idea since it hides the fact
that many tests are acutually UNSUPPORTED, but appear to PASS, while the
only thing that passes is an empty main.  I had something similar
recently when all C99 functionality in libstdc++ got disabled, but
testsuite results barely changed.

Unless someone strongly objects, I expect to change those tests to use
corresponding dg-require-* keywords to make mark them appropriately once
4.9 branches.

	Rainer


2014-03-31  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* testsuite/18_support/exception_ptr/60612-terminate.cc
	(terminate, f): Wrap in _GLIBCXX_USE_C99.
	* testsuite/18_support/exception_ptr/60612-unexpected.cc: Likewise.

Comments

Jonathan Wakely March 31, 2014, 12:46 p.m. UTC | #1
On 31/03/14 14:13 +0200, Rainer Orth wrote:
>
>Unfortunately, the new tests FAIL on non-C99 targets (i386-pc-solaris2.9
>in my case):
>
>FAIL: 18_support/exception_ptr/60612-terminate.cc (test for excess errors)
>Excess errors:
>/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/18_support/exception_ptr/6061
>2-terminate.cc:26:27: error: '_Exit' was not declared in this scope
>
>WARNING: 18_support/exception_ptr/60612-terminate.cc compilation failed to produ
>ce executable

Sorry about that.
Does Solaris 9 not support C99 at all, or does it only define _Exit
when __STDC_VERSION__ has the right value?

>The following patch fixes this, following the idiom often used in the
>libstdc++ testsuite.
>
>Tested with the appropriate runtest invocation on
>i386-pc-solaris2.{9,11}.  Ok for mainline?

The patch is OK, thanks.

>When looking into this, I noticed that this idiom is very widespread in
>the v3 testsuite, but IMNSHO a *very bad* idea since it hides the fact
>that many tests are acutually UNSUPPORTED, but appear to PASS, while the
>only thing that passes is an empty main.  I had something similar
>recently when all C99 functionality in libstdc++ got disabled, but
>testsuite results barely changed.

I completely agree it's a bad idea.

>Unless someone strongly objects, I expect to change those tests to use
>corresponding dg-require-* keywords to make mark them appropriately once
>4.9 branches.

I certainly don't object, but you should be aware that once we're in
stage 1 I want to review all our uses of _GLIBCXX_USE_C99 and replace
it with more fine-grained checks. We should at least have separate
macros for:

* "we can use the C99 library in C++11 mode"
(true for a modern libc that checks for __cplusplus >= 201103L)

* "we can use the C99 library in C++98 mode"
(true for glibc, because we define _GNU_SOURCE)
Rainer Orth March 31, 2014, 1:22 p.m. UTC | #2
Hi Jonathan,

> On 31/03/14 14:13 +0200, Rainer Orth wrote:
>>
>>Unfortunately, the new tests FAIL on non-C99 targets (i386-pc-solaris2.9
>>in my case):
>>
>>FAIL: 18_support/exception_ptr/60612-terminate.cc (test for excess errors)
>>Excess errors:
>>/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/18_support/exception_ptr/6061
>>2-terminate.cc:26:27: error: '_Exit' was not declared in this scope
>>
>>WARNING: 18_support/exception_ptr/60612-terminate.cc compilation failed to
>> produ
>>ce executable
>
> Sorry about that.
> Does Solaris 9 not support C99 at all, or does it only define _Exit
> when __STDC_VERSION__ has the right value?

no C99 support at all.  That only appeared in Solaris 10.

>>When looking into this, I noticed that this idiom is very widespread in
>>the v3 testsuite, but IMNSHO a *very bad* idea since it hides the fact
>>that many tests are acutually UNSUPPORTED, but appear to PASS, while the
>>only thing that passes is an empty main.  I had something similar
>>recently when all C99 functionality in libstdc++ got disabled, but
>>testsuite results barely changed.
>
> I completely agree it's a bad idea.
>
>>Unless someone strongly objects, I expect to change those tests to use
>>corresponding dg-require-* keywords to make mark them appropriately once
>>4.9 branches.
>
> I certainly don't object, but you should be aware that once we're in
> stage 1 I want to review all our uses of _GLIBCXX_USE_C99 and replace
> it with more fine-grained checks. We should at least have separate
> macros for:
>
> * "we can use the C99 library in C++11 mode"
> (true for a modern libc that checks for __cplusplus >= 201103L)
>
> * "we can use the C99 library in C++98 mode"
> (true for glibc, because we define _GNU_SOURCE)

Good to know, thanks for the heads-up.

	Rainer
diff mbox

Patch

# HG changeset patch
# Parent 23f930c5334d34dff20abb2647f50b4a095481e4
Fix 18_support/exception_ptr/60612-*.cc on non-C99 targets

diff --git a/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc
--- a/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc
+++ b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc
@@ -23,6 +23,7 @@ 
 #include <exception>
 #include <stdlib.h>
 
+#ifdef _GLIBCXX_USE_C99
 void terminate() { _Exit(0); }
 
 void f() noexcept
@@ -34,8 +35,12 @@  void f() noexcept
     std::rethrow_exception(std::current_exception());
   }
 }
+#endif
 
 int main()
 {
+#ifdef _GLIBCXX_USE_C99
   f();
+#endif
+  return 0;
 }
diff --git a/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc
--- a/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc
+++ b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc
@@ -23,6 +23,7 @@ 
 #include <exception>
 #include <stdlib.h>
 
+#ifdef _GLIBCXX_USE_C99
 void unexpected() { _Exit(0); }
 
 void f() throw()
@@ -34,8 +35,11 @@  void f() throw()
     std::rethrow_exception(std::current_exception());
   }
 }
+#endif
 
 int main()
 {
+#ifdef _GLIBCXX_USE_C99
   f();
+#endif
 }