diff mbox series

[sanitizer] Wrap rethrow_primary_exception (PR 87880).

Message ID 62358CB5-F721-4DA5-BE9F-7E9A34B2A78C@sandoe.co.uk
State New
Headers show
Series [sanitizer] Wrap rethrow_primary_exception (PR 87880). | expand

Commit Message

Iain Sandoe June 14, 2019, 2:38 p.m. UTC 
For some Darwin versions the absence of the rethrow_primary_exception
symbol causes almost all sanitizer tests to fail.

The following patch wraps it as suggested by Jakub in the PR trail, such that
if the gate is not defined, it’s assumed to be available.

OK for trunk?
Iain

libsanitizer/

2019-06-14  Iain Sandoe  <iain@sandoe.co.uk>

	PR libsanitizer/87880
	* asan/asan_interceptors.h:
	(ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION): New.
	* asan/Makefile.am (DEFS): Add 
	ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION, defined to 0.
	* asan/Makefile.in: Regenerated.

Comments

Jakub Jelinek June 14, 2019, 2:47 p.m. UTC | #1 
On Fri, Jun 14, 2019 at 03:38:05PM +0100, Iain Sandoe wrote:
> For some Darwin versions the absence of the rethrow_primary_exception
> symbol causes almost all sanitizer tests to fail.
> 
> The following patch wraps it as suggested by Jakub in the PR trail, such that
> if the gate is not defined, it’s assumed to be available.

I wonder if we shouldn't bump libasan soname because of this, as this change
is removing an exported symbol from it.

Otherwise LGTM (but bumping soname would mean it is not backportable).

> 2019-06-14  Iain Sandoe  <iain@sandoe.co.uk>
> 
> 	PR libsanitizer/87880
> 	* asan/asan_interceptors.h:
> 	(ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION): New.
> 	* asan/Makefile.am (DEFS): Add 
> 	ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION, defined to 0.
> 	* asan/Makefile.in: Regenerated.

	Jakub
Iain Sandoe June 16, 2019, 6:54 p.m. UTC | #2 
Hi Jakub,

> On 14 Jun 2019, at 15:47, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Fri, Jun 14, 2019 at 03:38:05PM +0100, Iain Sandoe wrote:
>> For some Darwin versions the absence of the rethrow_primary_exception
>> symbol causes almost all sanitizer tests to fail.
>> 
>> The following patch wraps it as suggested by Jakub in the PR trail, such that
>> if the gate is not defined, it’s assumed to be available.
> 
> I wonder if we shouldn't bump libasan soname because of this, as this change
> is removing an exported symbol from it.
> 
> Otherwise LGTM (but bumping soname would mean it is not backportable).

So, I guess, unless Jonathan has plans to add __cxa_rethrow_primary_exception
during the 10 time-frame, it’s correct to exclude the symbol anyway and we should
bump the so version and apply trunk.

Actually, because the way in which interposing works for Darwin is different, the only
symbol change in the library on Darwin is removing an "undefined dynamic lookup".
So, for back-ports, I can could up with some Darwin-specific Makefike change that
only adds the ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0 for Darwin.

So - OK for trunk with a bumped soname?
(and a TODO to figure a Darwin-only backport)

thanks
Iain


> 
>> 2019-06-14  Iain Sandoe  <iain@sandoe.co.uk>
>> 
>> 	PR libsanitizer/87880
>> 	* asan/asan_interceptors.h:
>> 	(ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION): New.
>> 	* asan/Makefile.am (DEFS): Add 
>> 	ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION, defined to 0.
>> 	* asan/Makefile.in: Regenerated.
> 
> 	Jakub
Jakub Jelinek June 16, 2019, 6:58 p.m. UTC | #3 
On Sun, Jun 16, 2019 at 07:54:42PM +0100, Iain Sandoe wrote:
> So, I guess, unless Jonathan has plans to add __cxa_rethrow_primary_exception
> during the 10 time-frame, it’s correct to exclude the symbol anyway and we should
> bump the so version and apply trunk.

I don't understand why they've added it, it should be called
std::rethrow_exception and that is how it is called in libstdc++.

> Actually, because the way in which interposing works for Darwin is different, the only
> symbol change in the library on Darwin is removing an "undefined dynamic lookup".
> So, for back-ports, I can could up with some Darwin-specific Makefike change that
> only adds the ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0 for Darwin.
> 
> So - OK for trunk with a bumped soname?

Yes.

> (and a TODO to figure a Darwin-only backport)

Yeah.

	Jakub
Jonathan Wakely June 17, 2019, 8:07 a.m. UTC | #4 
On 16/06/19 20:58 +0200, Jakub Jelinek wrote:
>On Sun, Jun 16, 2019 at 07:54:42PM +0100, Iain Sandoe wrote:
>> So, I guess, unless Jonathan has plans to add __cxa_rethrow_primary_exception
>> during the 10 time-frame, it’s correct to exclude the symbol anyway and we should
>> bump the so version and apply trunk.
>
>I don't understand why they've added it, it should be called
>std::rethrow_exception and that is how it is called in libstdc++.

std::rethrow_exception was new in C++11, so maybe they wanted to be
able to use it in C++03 code as well? (Just a guess).

>> Actually, because the way in which interposing works for Darwin is different, the only
>> symbol change in the library on Darwin is removing an "undefined dynamic lookup".
>> So, for back-ports, I can could up with some Darwin-specific Makefike change that
>> only adds the ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0 for Darwin.
>>
>> So - OK for trunk with a bumped soname?
>
>Yes.
>
>> (and a TODO to figure a Darwin-only backport)
>
>Yeah.
>
>	Jakub
Iain Sandoe June 18, 2019, 8:21 a.m. UTC | #5 
> On 17 Jun 2019, at 09:07, Jonathan Wakely <jwakely@redhat.com> wrote:
> 
> On 16/06/19 20:58 +0200, Jakub Jelinek wrote:
>> On Sun, Jun 16, 2019 at 07:54:42PM +0100, Iain Sandoe wrote:
>>> So, I guess, unless Jonathan has plans to add __cxa_rethrow_primary_exception
>>> during the 10 time-frame, it’s correct to exclude the symbol anyway and we should
>>> bump the so version and apply trunk.
>> 
>> I don't understand why they've added it, it should be called
>> std::rethrow_exception and that is how it is called in libstdc++.
> 
> std::rethrow_exception was new in C++11, so maybe they wanted to be
> able to use it in C++03 code as well? (Just a guess).
> 
>>> Actually, because the way in which interposing works for Darwin is different, the only
>>> symbol change in the library on Darwin is removing an "undefined dynamic lookup".
>>> So, for back-ports, I can could up with some Darwin-specific Makefike change that
>>> only adds the ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0 for Darwin.
>>> 
>>> So - OK for trunk with a bumped soname?
>> 
>> Yes.

this is what I applied.
Iain

2019-06-18  Iain Sandoe  <iain@sandoe.co.uk>

	PR libsanitizer/87880
	* asan/asan_interceptors.h:
	(ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION): New.
	* asan/Makefile.am (DEFS): Add 
	ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION, defined to 0.
	* asan/Makefile.in: Regenerated.
	* asan/libtool-version: Bump version.


diff --git a/libsanitizer/asan/Makefile.am b/libsanitizer/asan/Makefile.am
index 867240d..b18ab2a 100644
--- a/libsanitizer/asan/Makefile.am
+++ b/libsanitizer/asan/Makefile.am
@@ -3,7 +3,7 @@ AM_CPPFLAGS = -I $(top_srcdir)/include -I $(top_srcdir)
# May be used by toolexeclibdir.
gcc_version := $(shell @get_gcc_base_ver@ $(top_srcdir)/../gcc/BASE-VER)

-DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 -DASAN_NEEDS_SEGV=1 -DCAN_SANITIZE_UB=0
+DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 -DASAN_NEEDS_SEGV=1 -DCAN_SANITIZE_UB=0 -DASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0
if USING_MAC_INTERPOSE
DEFS += -DMAC_INTERPOSE_FUNCTIONS -DMISSING_BLOCKS_SUPPORT
endif
diff --git a/libsanitizer/asan/asan_interceptors.h b/libsanitizer/asan/asan_interceptors.h
index b599ebb..beb1dc9 100644
--- a/libsanitizer/asan/asan_interceptors.h
+++ b/libsanitizer/asan/asan_interceptors.h
@@ -79,7 +79,12 @@ void InitializePlatformInterceptors();
#if ASAN_HAS_EXCEPTIONS && !SANITIZER_WINDOWS && !SANITIZER_SOLARIS && \
    !SANITIZER_NETBSD
# define ASAN_INTERCEPT___CXA_THROW 1
-# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# if ! defined(ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION) \
+     || ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION
+#   define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# else
+#   define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 0
+# endif
# if defined(_GLIBCXX_SJLJ_EXCEPTIONS) || (SANITIZER_IOS && defined(__arm__))
#  define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 1
# else
diff --git a/libsanitizer/asan/libtool-version b/libsanitizer/asan/libtool-version
index e3138f3..c509757 100644
--- a/libsanitizer/asan/libtool-version
+++ b/libsanitizer/asan/libtool-version
@@ -3,4 +3,4 @@
# a separate file so that version updates don't involve re-running
# automake.
# CURRENT:REVISION:AGE
-5:0:0
+6:0:0
Iain Sandoe Aug. 18, 2019, 7:19 p.m. UTC | #6 
> On 16 Jun 2019, at 19:58, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Sun, Jun 16, 2019 at 07:54:42PM +0100, Iain Sandoe wrote:

>> Actually, because the way in which interposing works for Darwin is different, the only
>> symbol change in the library on Darwin is removing an "undefined dynamic lookup".
>> So, for back-ports, I can could up with some Darwin-specific Makefike change that
>> only adds the ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0 for Darwin.
>> 
>> So - OK for trunk with a bumped soname?
> 
> Yes.
> 
>> (and a TODO to figure a Darwin-only backport)
> 
> Yeah.

This turned out to be easier than I’d expected, since there was already a Darwin-specific
guard in the Makefile.

Tested on x86_64-darwin{11,13,14,16,17,18}, x86_64,powerpc64-linux-gnu
(that the symbol is not removed for the Linux cases).

Applied to the 9 branch for 9.3
Iain

libsanitizer/

2019-08-18  Iain Sandoe  <iain@sandoe.co.uk>

	Backport from mainline
	2019-06-18  Iain Sandoe  <iain@sandoe.co.uk>

	PR libsanitizer/87880
	* asan/asan_interceptors.h:
	(ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION): New.
	* asan/Makefile.am (DEFS): Add (for Darwin only)
	ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION, defined to 0.
	* asan/Makefile.in: Regenerated.


diff --git a/libsanitizer/asan/Makefile.am b/libsanitizer/asan/Makefile.am
index 867240d244..6efbc1df7f 100644
--- a/libsanitizer/asan/Makefile.am
+++ b/libsanitizer/asan/Makefile.am
@@ -5,7 +5,7 @@ gcc_version := $(shell @get_gcc_base_ver@ $(top_srcdir)/../gcc/BASE-VER)
 
 DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 -DASAN_NEEDS_SEGV=1 -DCAN_SANITIZE_UB=0
 if USING_MAC_INTERPOSE
-DEFS += -DMAC_INTERPOSE_FUNCTIONS -DMISSING_BLOCKS_SUPPORT
+DEFS += -DMAC_INTERPOSE_FUNCTIONS -DMISSING_BLOCKS_SUPPORT -DASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0
 endif
 AM_CXXFLAGS = -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long  -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -fno-ipa-icf
 AM_CXXFLAGS += $(LIBSTDCXX_RAW_CXX_CXXFLAGS)

diff --git a/libsanitizer/asan/asan_interceptors.h b/libsanitizer/asan/asan_interceptors.h
index b599ebb0ba..beb1dc9532 100644
--- a/libsanitizer/asan/asan_interceptors.h
+++ b/libsanitizer/asan/asan_interceptors.h
@@ -79,7 +79,12 @@ void InitializePlatformInterceptors();
 #if ASAN_HAS_EXCEPTIONS && !SANITIZER_WINDOWS && !SANITIZER_SOLARIS && \
     !SANITIZER_NETBSD
 # define ASAN_INTERCEPT___CXA_THROW 1
-# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# if ! defined(ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION) \
+     || ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION
+#   define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# else
+#   define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 0
+# endif
 # if defined(_GLIBCXX_SJLJ_EXCEPTIONS) || (SANITIZER_IOS && defined(__arm__))
 #  define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 1
 # else
diff mbox series

Patch

diff --git a/libsanitizer/asan/Makefile.am b/libsanitizer/asan/Makefile.am
index 867240d..b18ab2a 100644
--- a/libsanitizer/asan/Makefile.am
+++ b/libsanitizer/asan/Makefile.am
@@ -3,7 +3,7 @@  AM_CPPFLAGS = -I $(top_srcdir)/include -I $(top_srcdir)
 # May be used by toolexeclibdir.
 gcc_version := $(shell @get_gcc_base_ver@ $(top_srcdir)/../gcc/BASE-VER)
 
-DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 -DASAN_NEEDS_SEGV=1 -DCAN_SANITIZE_UB=0
+DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 -DASAN_NEEDS_SEGV=1 -DCAN_SANITIZE_UB=0 -DASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION=0
 if USING_MAC_INTERPOSE
 DEFS += -DMAC_INTERPOSE_FUNCTIONS -DMISSING_BLOCKS_SUPPORT
 endif
diff --git a/libsanitizer/asan/asan_interceptors.h b/libsanitizer/asan/asan_interceptors.h
index b599ebb..beb1dc9 100644
--- a/libsanitizer/asan/asan_interceptors.h
+++ b/libsanitizer/asan/asan_interceptors.h
@@ -79,7 +79,12 @@  void InitializePlatformInterceptors();
 #if ASAN_HAS_EXCEPTIONS && !SANITIZER_WINDOWS && !SANITIZER_SOLARIS && \
     !SANITIZER_NETBSD
 # define ASAN_INTERCEPT___CXA_THROW 1
-# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# if ! defined(ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION) \
+     || ASAN_HAS_CXA_RETHROW_PRIMARY_EXCEPTION
+#   define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# else
+#   define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 0
+# endif
 # if defined(_GLIBCXX_SJLJ_EXCEPTIONS) || (SANITIZER_IOS && defined(__arm__))
 #  define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 1
 # else