Message ID | a347e239-3fc6-6d34-185a-9162c1d665e0@acm.org |
---|---|
State | New |
Headers | show |
Series | [libitm] eh specifications are lax | expand |
ping? On 5/5/20 4:08 PM, Nathan Sidwell wrote: > I discovered that libitm: > (a) declares __cxa_allocate_exception and friends directly, > (b) doesn't mark them as 'throw()' > (c) doesn't mark the replacment fns _ITM_$foo as nothrow either > > We happen to get away with it because of code in the compiler that, > although it checks the parameter types, doesn't check the exception > specification. (One reason being they used to not be part of the > language's type system, but now they are.) I suspect this can lead us > to generate pessimal code later, if we've seen one of these decls > earlier. Anyway, with modules it becomes trickier[*], so I'm trying to > clean it up and not be a problem. I see Jakub fixed part of the problem > (https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513302.html) > AFAICT, he did fix libitm's decls, but left the lax parm-type checking > in the compiler. > > libitm.h is not very informative about specification: > in version 1 of http://www.intel.com/some/path/here.pdf. */ > > Anyway, it was too fiddly to have libitm pick up the declarations from > libsupc++. Besides it makes them weak declarations, and then provides > definitions for non-elf systems. So this patch adds the expected 'throw()' > > While I can't be sure, I suspect the _ITM entry points are supposed to > have the same exception specification as the original entry points. So > those are also made 'throw ()'. libstdc++'s _GLIBCXX_NOTHROW didn't > seem available, so I make use of a new _ITM_NOTHROW macro, suitably > defined. > > Because of the lax checking in the compiler, and old compiler with a > patched libitm.h will be ok... Until I change the compiler :) > > booted & tested on x86_64-linux, ok? > > nathan > > [*] modules make it harder to have ODR violations, that's why it finds > ODR violations in existing code. >
On Tue, 2020-05-05 at 16:08 -0400, Nathan Sidwell wrote: > I discovered that libitm: > (a) declares __cxa_allocate_exception and friends directly, > (b) doesn't mark them as 'throw()' > (c) doesn't mark the replacment fns _ITM_$foo as nothrow either > > We happen to get away with it because of code in the compiler that, > although it checks the parameter types, doesn't check the exception > specification. (One reason being they used to not be part of the > language's type system, but now they are.) I suspect this can lead us > to generate pessimal code later, if we've seen one of these decls > earlier. Anyway, with modules it becomes trickier[*], so I'm trying to > clean it up and not be a problem. I see Jakub fixed part of the problem > (https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513302.html) > AFAICT, he did fix libitm's decls, but left the lax parm-type checking > in the compiler. > > libitm.h is not very informative about specification: > in version 1 of http://www.intel.com/some/path/here.pdf. */ > > Anyway, it was too fiddly to have libitm pick up the declarations from > libsupc++. Besides it makes them weak declarations, and then provides > definitions for non-elf systems. So this patch adds the expected 'throw()' > > While I can't be sure, I suspect the _ITM entry points are supposed to > have the same exception specification as the original entry points. So > those are also made 'throw ()'. libstdc++'s _GLIBCXX_NOTHROW didn't > seem available, so I make use of a new _ITM_NOTHROW macro, suitably defined. > > Because of the lax checking in the compiler, and old compiler with a > patched libitm.h will be ok... Until I change the compiler :) > > booted & tested on x86_64-linux, ok? > > nathan > > [*] modules make it harder to have ODR violations, that's why it finds > ODR violations in existing code. > > > 2020-05-05 Nathan Sidwell <nathan@acm.org> > > > > Fix throw specifiers on interface. > > * libitm/libitm.h (_ITM_NOTHROW): Define. > > (_ITM_cxa_allocate_exception, _ITM_cxa_free_exception) > > (_ITM_cxa_begin_catch): Use it. > > * eh_cpp.cc: Add throw() to __cxa_allocate_exception, > > __cxa_free_exception, __cxa_begin_catch, __cxa_tm_cleanup, > > __cxa_get_globals. > > (_ITM_cxa_allocate_exception, _ITM_cxa_free_exception) > > (_ITM_cxa_begin_catch): Likewise. > > OK jeff
2020-05-05 Nathan Sidwell <nathan@acm.org> Fix throw specifiers on interface. * libitm/libitm.h (_ITM_NOTHROW): Define. (_ITM_cxa_allocate_exception, _ITM_cxa_free_exception) (_ITM_cxa_begin_catch): Use it. * eh_cpp.cc: Add throw() to __cxa_allocate_exception, __cxa_free_exception, __cxa_begin_catch, __cxa_tm_cleanup, __cxa_get_globals. (_ITM_cxa_allocate_exception, _ITM_cxa_free_exception) (_ITM_cxa_begin_catch): Likewise. diff --git a/libitm/eh_cpp.cc b/libitm/eh_cpp.cc index 7d570e74fc2..6c9da2d06e4 100644 --- a/libitm/eh_cpp.cc +++ b/libitm/eh_cpp.cc @@ -87,23 +87,23 @@ struct __cxa_eh_globals unsigned int uncaughtExceptions; }; -extern void *__cxa_allocate_exception (size_t) WEAK; -extern void __cxa_free_exception (void *) WEAK; +extern void *__cxa_allocate_exception (size_t) _ITM_NOTHROW WEAK; +extern void __cxa_free_exception (void *) _ITM_NOTHROW WEAK; extern void __cxa_throw (void *, void *, void (*) (void *)) WEAK; -extern void *__cxa_begin_catch (void *) WEAK; +extern void *__cxa_begin_catch (void *) _ITM_NOTHROW WEAK; extern void __cxa_end_catch (void) WEAK; -extern void __cxa_tm_cleanup (void *, void *, unsigned int) WEAK; -extern __cxa_eh_globals *__cxa_get_globals (void) WEAK; +extern void __cxa_tm_cleanup (void *, void *, unsigned int) throw () WEAK; +extern __cxa_eh_globals *__cxa_get_globals (void) _ITM_NOTHROW WEAK; #if !defined (HAVE_ELF_STYLE_WEAKREF) -void *__cxa_allocate_exception (size_t) { return NULL; } -void __cxa_free_exception (void *) { return; } +void *__cxa_allocate_exception (size_t) _ITM_NOTHROW { return NULL; } +void __cxa_free_exception (void *) _ITM_NOTHROW { return; } void __cxa_throw (void *, void *, void (*) (void *)) { return; } -void *__cxa_begin_catch (void *) { return NULL; } +void *__cxa_begin_catch (void *) _ITM_NOTHROW { return NULL; } void __cxa_end_catch (void) { return; } -void __cxa_tm_cleanup (void *, void *, unsigned int) { return; } +void __cxa_tm_cleanup (void *, void *, unsigned int) throw () { return; } void _Unwind_DeleteException (_Unwind_Exception *) { return; } -__cxa_eh_globals *__cxa_get_globals (void) { return NULL; } +__cxa_eh_globals *__cxa_get_globals (void) _ITM_NOTHROW { return NULL; } #endif /* HAVE_ELF_STYLE_WEAKREF */ } @@ -117,7 +117,7 @@ free_any_exception (void *exc_ptr) } void * -_ITM_cxa_allocate_exception (size_t size) +_ITM_cxa_allocate_exception (size_t size) _ITM_NOTHROW { void *r = __cxa_allocate_exception (size); gtm_thr()->record_allocation (r, free_any_exception); @@ -125,7 +125,7 @@ _ITM_cxa_allocate_exception (size_t size) } void -_ITM_cxa_free_exception (void *exc_ptr) +_ITM_cxa_free_exception (void *exc_ptr) _ITM_NOTHROW { // __cxa_free_exception can be called from user code directly if // construction of an exception object throws another exception, in which @@ -143,7 +143,7 @@ _ITM_cxa_throw (void *obj, void *tinfo, void (*dest) (void *)) } void * -_ITM_cxa_begin_catch (void *exc_ptr) +_ITM_cxa_begin_catch (void *exc_ptr) _ITM_NOTHROW { // If this exception object has been allocated by this transaction, we // discard the undo log entry for the allocation; we are entering phase (3) diff --git a/libitm/libitm.h b/libitm/libitm.h index 69f551e2675..4f8051bdfb7 100644 --- a/libitm/libitm.h +++ b/libitm/libitm.h @@ -45,6 +45,15 @@ extern "C" { #define ITM_NORETURN __attribute__((noreturn)) #define ITM_PURE __attribute__((transaction_pure)) +#ifdef _GLIBCXX_NOTHROW +# define _ITM_NOTHROW _GLIBCXX_NOTHROW +#elif !defined (__cplusplus) +# define _ITM_NOTHROW __attribute__((__nothrow__)) +#elif __cplusplus < 201103L +# define _ITM_NOTHROW throw () +#else +# define _ITM_NOTHROW noexcept +#endif /* The following are externally visible definitions and functions, though only very few of these should be called by user code. */ @@ -282,11 +291,11 @@ extern void *_ITM_getTMCloneSafe (void *) ITM_REGPARM; extern void _ITM_registerTMCloneTable (void *, size_t); extern void _ITM_deregisterTMCloneTable (void *); -extern void *_ITM_cxa_allocate_exception (size_t); -extern void _ITM_cxa_free_exception (void *exc_ptr); +extern void *_ITM_cxa_allocate_exception (size_t) _ITM_NOTHROW; +extern void _ITM_cxa_free_exception (void *exc_ptr) _ITM_NOTHROW; extern void _ITM_cxa_throw (void *obj, void *tinfo, void (*dest) (void *)); -extern void *_ITM_cxa_begin_catch (void *exc_ptr); -extern void _ITM_cxa_end_catch (void); +extern void *_ITM_cxa_begin_catch (void *exc_ptr) _ITM_NOTHROW; +extern void _ITM_cxa_end_catch (void); /* This can throw. */ extern void _ITM_commitTransactionEH(void *exc_ptr) ITM_REGPARM; #ifdef __cplusplus