diff mbox

Fix PR libitm/55693

Message ID 20130206212738.GA6682@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Feb. 6, 2013, 9:27 p.m. UTC 
On Wed, Feb 06, 2013 at 09:15:20PM +0100, Patrick Marlier wrote:
> On Wed, Feb 6, 2013 at 6:28 PM, Iain Sandoe <iain@codesourcery.com> wrote:
> >
> > On 6 Feb 2013, at 17:20, Jack Howarth wrote:
> >
> >> On Wed, Feb 06, 2013 at 05:37:12PM +0100, Patrick Marlier wrote:
> >>> Hi Jack,
> >>>
> >>> Thanks for having a look at this.
> >>>
> >>> However I don't understand why you need this:
> >>>
> >>> Index: gcc/config/i386/darwin.h
> >>> ===================================================================
> >>> --- gcc/config/i386/darwin.h (revision 195764)
> >>> +++ gcc/config/i386/darwin.h (working copy)
> >>> @@ -131,8 +131,7 @@ extern int darwin_emit_branch_islands;
> >>>   "%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
> >>>    %{mpc32:crtprec32.o%s} \
> >>>    %{mpc64:crtprec64.o%s} \
> >>> -   %{mpc80:crtprec80.o%s} \
> >>> -   %{fgnu-tm: -lcrttme.o}"
> >>> +   %{mpc80:crtprec80.o%s}" TM_DESTRUCTOR
> >>>
> >>> #undef SUBTARGET_EXTRA_SPECS
> >>> #define SUBTARGET_EXTRA_SPECS                                   \
> >>> Index: gcc/config/darwin.h
> >>> ===================================================================
> >>> --- gcc/config/darwin.h (revision 195764)
> >>> +++ gcc/config/darwin.h (working copy)
> >>> @@ -363,7 +363,8 @@ extern GTY(()) int darwin_ms_struct;
> >>>   %{shared-libgcc:%:version-compare(< 10.5 mmacosx-version-min= crt3.o%s)}"
> >>>
> >>> /* We want a destructor last in the list.  */
> >>> -#define ENDFILE_SPEC "%{fgnu-tm: -lcrttme.o}"
> >>> +#define TM_DESTRUCTOR "%{fgnu-tm: -lcrttme.o}"
> >>> +#define ENDFILE_SPEC TM_DESTRUCTOR
> >>>
> >>> #define DARWIN_EXTRA_SPECS \
> >>>   { "darwin_crt1", DARWIN_CRT1_SPEC }, \
> >>>
> >>>
> >>> It seems you just add a macro TM_DESTRUCTOR which is the same as
> >>> ENDFILE_SPEC. Maybe I missed something (I am updating my svn)...
> >>
> >> Patrick,
> >>   This was the patch Iain proposed off-list and I just cleaned up the comments.
> >> I believe that he added the additional definition of TM_DESTRUCTOR so that
> >> it could be used to replace the explicit instance of %{fgnu-tm: -lcrttme.o}
> >> in the definition of ENDFILE_SPEC in gcc/config/i386/darwin.h.
> >
> > Correct - it was non-obvious to have a second instance embedded in the sub-dir.
> > (but I'm not going to complain if that change is removed).
> 
> I had a look at Iain's patch from the PR but some changes are missing
> in the proposed patch to give TM_DESTRUCTOR a sense.
> So either you add changes for files: gcc/config/darwin10.h
> libgcc/config.host libgcc/config/t-darwin to this patch or you create
> another patch with changes to gcc/config/darwin10.h libgcc/config.host
> libgcc/config/t-darwin gcc/config/darwin.h gcc/config/i386/darwin.h.

Patrick,
   I think you are making this much more complex than it really is.
The ENDFILE_SPEC on ppc is obtained from gcc/config/darwin.h and is
only...

#define TM_DESTRUCTOR "%{fgnu-tm: -lcrttme.o}"

whereas for intel darwin, it is defined in gcc/config/i386/darwin.h as

#undef ENDFILE_SPEC
#define ENDFILE_SPEC \
  "%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
   %{mpc32:crtprec32.o%s} \
   %{mpc64:crtprec64.o%s} \
   %{mpc80:crtprec80.o%s} \
   %{fgnu-tm: -lcrttme.o}"

Iain only added TM_DESTRUCTOR to remove the explicit instance of
%{fgnu-tm: -lcrttme.o} in the definition of ENDFILE_SPEC in 
gcc/config/i386/darwin.h (which meant he needed a new definition, TM_DESTRUCTOR).
That change is non-essential to the fix and completely tangential to the
problem. The actual fix is only the change...


which correctly eliminates the definition of the dummy functions on darwin
when c++ fast weak-symbol coalescing is present (which was introduced in
dyld at 10.6). The problem and solution was identified by the darwin linker
developer. The fast weak-symbol coalescing now assumes that weak symbol
duplicates are rare and only looks for them if the weak symbol appears
in a shared library. In our case, the weak dummy functions are considered
to reside in an object file, libgcc/config/darwin-crt-tm.o, and this
doesn't trigger the weak-symbol coalescing to identify the duplication so
that the non-weak symbols for __cxa* in libstdc++ aren't used for c++
programs using libitm. This works prior to 10.6, because the weak-symbol
coalescing was exhaustive and always checked every image regardless of
where the weak symbol originated.
            Jack

> 
> About this old patch
> http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00851.html, I though that
> "&& !defined (__MACH__)" should be changed but it seems the best
> way...
> 
> Thanks for the precision about XCode.
> 
> Note that I cannot approve anything. So the patch must be approved by
> a GCC master ;)
> --
> Patrick

Comments

Patrick Marlier Feb. 7, 2013, 7:50 a.m. UTC | #1 
Jack,

On Wed, Feb 6, 2013 at 10:27 PM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
>    I think you are making this much more complex than it really is.
> The ENDFILE_SPEC on ppc is obtained from gcc/config/darwin.h and is
> only...
>
> #define TM_DESTRUCTOR "%{fgnu-tm: -lcrttme.o}"
>
> whereas for intel darwin, it is defined in gcc/config/i386/darwin.h as
>
> #undef ENDFILE_SPEC
> #define ENDFILE_SPEC \
>   "%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
>    %{mpc32:crtprec32.o%s} \
>    %{mpc64:crtprec64.o%s} \
>    %{mpc80:crtprec80.o%s} \
>    %{fgnu-tm: -lcrttme.o}"
>
> Iain only added TM_DESTRUCTOR to remove the explicit instance of
> %{fgnu-tm: -lcrttme.o} in the definition of ENDFILE_SPEC in
> gcc/config/i386/darwin.h (which meant he needed a new definition, TM_DESTRUCTOR).
> That change is non-essential to the fix and completely tangential to the
> problem. The actual fix is only the change...

Ok thanks. I am fine with this then. It was just I did not understand
the purpose of this from your first email on gcc-patches.
--
Patrick
diff mbox

Patch

Index: libgcc/config/darwin-crt-tm.c
===================================================================
--- libgcc/config/darwin-crt-tm.c	(revision 195764)
+++ libgcc/config/darwin-crt-tm.c	(working copy)
@@ -103,9 +103,12 @@  void __doTMdeRegistrations (void)
     _ITM_deregisterTMCloneTable (tmct);
 }
 
+#if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && \
+            __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__  < 1060
+
 /* Provide dummy functions to satisfy linkage for versions of the Darwin 
    tool-chain that that can't handle undefined weak refs at the link stage.
-   ??? Define these dummy functions only when !HAVE_ELF_STYLE_WEAKREF. */
+   Don't define for 10.6 or later with faster weak-symbol coalescing. */
 
 extern void *__cxa_allocate_exception (size_t) WEAK;
 extern void __cxa_throw (void *, void *, void *) WEAK;
@@ -144,5 +147,5 @@  void _ZdlPvRKSt9nothrow_t (void * a UNUS
 void *_ZnaXRKSt9nothrow_t (size_t s UNUSED, c_nothrow_p b UNUSED)
   { return NULL; }
 void _ZdaPvRKSt9nothrow_t (void * a UNUSED, c_nothrow_p b UNUSED) { return; }
-
+#endif
 #endif