| Message ID | CAEwic4Zrmnq5mshti1a3k2aNaXy3tOjugVvE87v3AkyetCptVA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi,
> So updated patch is:
Looks good to me. I guess that in principle we could try to have a macro
which is the typedef itself, but what you tested seems good enough to
resolve the PR.
Thanks,
Paolo.
PS: remember to adjust the Copyright years of eh_throw.cc and unwind-cxx.h. I would also mention the PR in the os_* files. Paolo.
2011/12/12 Paolo Carlini <paolo.carlini@oracle.com>: > PS: remember to adjust the Copyright years of eh_throw.cc and unwind-cxx.h. > I would also mention the PR in the os_* files. > > Paolo. Thanks for the reminder about copyright. Added comment about pr and added copyright year to files not mentioning 2011. Applied at revision 182237. Kai
On Mon, 12 Dec 2011, Kai Tietz wrote: > Index: gcc/libstdc++-v3/libsupc++/cxxabi.h > =================================================================== > --- gcc.orig/libstdc++-v3/libsupc++/cxxabi.h > +++ gcc/libstdc++-v3/libsupc++/cxxabi.h > @@ -51,6 +51,10 @@ > #include <bits/cxxabi_tweaks.h> > #include <bits/cxxabi_forced.h> > > +#ifndef _GLIBCXX_USE_THISCALL_ON_DTOR > +typedef void (*__cxa_dtor_type) (void *); > +#endif > + This changes the type from a function with "C" linkage to one with "C++" linkage, is that on purpose? There is a type __cxa_cdtor_type a couple lines below, which also seems used for destructors, but that one doesn't get __thiscall, that's confusing (but then there's probably a reason why it wasn't used in __cxa_throw). (Note: feel free to ignore, those are questions not comments, I don't know this code)
Hi, > On Mon, 12 Dec 2011, Kai Tietz wrote: > >> Index: gcc/libstdc++-v3/libsupc++/cxxabi.h >> =================================================================== >> --- gcc.orig/libstdc++-v3/libsupc++/cxxabi.h >> +++ gcc/libstdc++-v3/libsupc++/cxxabi.h >> @@ -51,6 +51,10 @@ >> #include <bits/cxxabi_tweaks.h> >> #include <bits/cxxabi_forced.h> >> >> +#ifndef _GLIBCXX_USE_THISCALL_ON_DTOR >> +typedef void (*__cxa_dtor_type) (void *); >> +#endif >> + > > This changes the type from a function with "C" linkage to one with > "C++" linkage, is that on purpose? Humm, thanks, I didn't really spend time on what was going on *below* the define, only to the right way to implement the mingw specific bits. I guess moving the #ifndef a few lines down, close to the other typedef should be the safe thing to do. That also requires adjustment in the config files, the typedef there must be also wrapped in #ifdef __cplusplus, etc. Please do the Change Kai. > There is a type __cxa_cdtor_type a couple lines below, which also > seems used for destructors, but that one doesn't get __thiscall, > that's confusing (but then there's probably a reason why it wasn't > used in __cxa_throw). No idea if it's right for mingw. Paolo.
2011/12/12 Paolo Carlini <paolo.carlini@oracle.com>: > Hi, > >> On Mon, 12 Dec 2011, Kai Tietz wrote: >> >>> Index: gcc/libstdc++-v3/libsupc++/cxxabi.h >>> =================================================================== >>> --- gcc.orig/libstdc++-v3/libsupc++/cxxabi.h >>> +++ gcc/libstdc++-v3/libsupc++/cxxabi.h >>> @@ -51,6 +51,10 @@ >>> #include <bits/cxxabi_tweaks.h> >>> #include <bits/cxxabi_forced.h> >>> >>> +#ifndef _GLIBCXX_USE_THISCALL_ON_DTOR >>> +typedef void (*__cxa_dtor_type) (void *); >>> +#endif >>> + >> >> >> This changes the type from a function with "C" linkage to one with "C++" >> linkage, is that on purpose? > > Humm, thanks, I didn't really spend time on what was going on *below* the > define, only to the right way to implement the mingw specific bits. I guess > moving the #ifndef a few lines down, close to the other typedef should be > the safe thing to do. That also requires adjustment in the config files, the > typedef there must be also wrapped in #ifdef __cplusplus, etc. > > Please do the Change Kai. Ok. By looking at this, it might be better to use here a define - as you mentioned. As I would need to copy here namespace too. >> There is a type __cxa_cdtor_type a couple lines below, which also seems >> used for destructors, but that one doesn't get __thiscall, that's confusing >> (but then there's probably a reason why it wasn't used in __cxa_throw). > > No idea if it's right for mingw. Well, not sure too. Logically, if those function in cdtor list (handled in vec.cc) are constructors/destructors, then it would require thiscall for IA-32 mingw, too. By tests I see that those function stored within that list have cdecl-calling convention. Therefore I didn't touched them by this patch. Kai Kai
On 12 December 2011 12:42, Kai Tietz wrote: > > PR libstdc++/511135 > * libsupc++/cxxabi.h (__cxxabi_dtor_type): New type. ChangeLog needs to be updated for the new type name (whether it ends up being __cxa_dtor_type or something else).
Hi, > Ok. By looking at this, it might be better to use here a define - as > you mentioned. As I would need to copy here namespace too. Ok, thanks. Let's make sure nothing can possibly change for != mingw, we don't want to take risks at this time. Paolo.
On 12/12/2011 04:28 PM, Paolo Carlini wrote: > Hi, >> Ok. By looking at this, it might be better to use here a define - as >> you mentioned. As I would need to copy here namespace too. > Ok, thanks. Let's make sure nothing can possibly change for != mingw, > we don't want to take risks at this time. For the time being I reverted the whole thing, the unwind-cxx.h bits in particular made me very nervous, because we have "C++" linkage in that case. Please make sure to handle it correctly in the next try. Paolo.
On Mon, 12 Dec 2011, Paolo Carlini wrote: > On 12/12/2011 04:28 PM, Paolo Carlini wrote: >> Hi, >>> Ok. By looking at this, it might be better to use here a define - as you >>> mentioned. As I would need to copy here namespace too. >> Ok, thanks. Let's make sure nothing can possibly change for != mingw, we >> don't want to take risks at this time. > For the time being I reverted the whole thing, the unwind-cxx.h bits in > particular made me very nervous, because we have "C++" linkage in that case. > Please make sure to handle it correctly in the next try. Actually, g++ currently simply ignores the linkage as part of function types, so this shouldn't have any effect. But it won't hurt to keep it inside extern "C" ;-)
On 12/12/2011 06:45 PM, Marc Glisse wrote: > Actually, g++ currently simply ignores the linkage as part of function > types, so this shouldn't have any effect. Also note that other systems have been using libsupc++ (without using the whole C++ lib), and I don't think we should play dirty games here and make the life unnecessarily hard to compilers which in this specific respect are more conforming than GCC. Paolo.
2011/12/12 Paolo Carlini <paolo.carlini@oracle.com>: > On 12/12/2011 06:45 PM, Marc Glisse wrote: >> >> Actually, g++ currently simply ignores the linkage as part of function >> types, so this shouldn't have any effect. > > Also note that other systems have been using libsupc++ (without using the > whole C++ lib), and I don't think we should play dirty games here and make > the life unnecessarily hard to compilers which in this specific respect are > more conforming than GCC. > > Paolo. Well, could you be please more specific, what in the unwind-cxx.h is causing especially your questioning here? As exactly the unwinding requires that dtor is called for C++ with proper calling-convention. Only thing I see, if we need instead of introducing here specific _cxa_dtor_type marking existing type __cxa_cdtor_type optional by this calling-abi attribute and replacing in libsupc++ the uses of 'void (*)(void *)' by __cxa_cdtor_type. Kai
Index: gcc/libstdc++-v3/libsupc++/cxxabi.h =================================================================== --- gcc.orig/libstdc++-v3/libsupc++/cxxabi.h +++ gcc/libstdc++-v3/libsupc++/cxxabi.h @@ -51,6 +51,10 @@ #include <bits/cxxabi_tweaks.h> #include <bits/cxxabi_forced.h> +#ifndef _GLIBCXX_USE_THISCALL_ON_DTOR +typedef void (*__cxa_dtor_type) (void *); +#endif + #ifdef __cplusplus namespace __cxxabiv1 { @@ -596,7 +600,7 @@ namespace __cxxabiv1 // Throw the exception. void - __cxa_throw(void*, std::type_info*, void (*) (void *)) + __cxa_throw(void*, std::type_info*, __cxa_dtor_type) __attribute__((__noreturn__)); // Used to implement exception handlers. Index: gcc/libstdc++-v3/libsupc++/eh_throw.cc =================================================================== --- gcc.orig/libstdc++-v3/libsupc++/eh_throw.cc +++ gcc/libstdc++-v3/libsupc++/eh_throw.cc @@ -58,8 +58,8 @@ __gxx_exception_cleanup (_Unwind_Reason_ extern "C" void -__cxxabiv1::__cxa_throw (void *obj, std::type_info *tinfo, - void (*dest) (void *)) +__cxxabiv1::__cxa_throw (void *obj, std::type_info *tinfo, + __cxa_dtor_type dest) { // Definitely a primary. __cxa_refcounted_exception *header Index: gcc/libstdc++-v3/libsupc++/unwind-cxx.h =================================================================== --- gcc.orig/libstdc++-v3/libsupc++/unwind-cxx.h +++ gcc/libstdc++-v3/libsupc++/unwind-cxx.h @@ -51,7 +51,7 @@ struct __cxa_exception { // Manage the exception object itself. std::type_info *exceptionType; - void (*exceptionDestructor)(void *); + __cxa_dtor_type exceptionDestructor; // The C++ standard has entertaining rules wrt calling set_terminate // and set_unexpected in the middle of the exception cleanup process. Index: gcc/libstdc++-v3/config/os/mingw32-w64/os_defines.h =================================================================== --- gcc.orig/libstdc++-v3/config/os/mingw32-w64/os_defines.h +++ gcc/libstdc++-v3/config/os/mingw32-w64/os_defines.h @@ -65,4 +65,9 @@ // ioctlsocket function doesn't work for normal file-descriptors. #define _GLIBCXX_NO_IOCTL 1 +#if defined (__i386__) +#define _GLIBCXX_USE_THISCALL_ON_DTOR 1 +typedef void (__thiscall *__cxa_dtor_type) (void *); +#endif + #endif Index: gcc/libstdc++-v3/config/os/mingw32/os_defines.h =================================================================== --- gcc.orig/libstdc++-v3/config/os/mingw32/os_defines.h +++ gcc/libstdc++-v3/config/os/mingw32/os_defines.h @@ -65,4 +65,9 @@ // ioctlsocket function doesn't work for normal file-descriptors. #define _GLIBCXX_NO_IOCTL 1 +#if defined (__i386__) +#define _GLIBCXX_USE_THISCALL_ON_DTOR 1 +typedef void (__thiscall *__cxa_dtor_type) (void *); +#endif + #endif