On Mon, 9 Dec 2019 13:05:22 +0000
Jozef Lawrynowicz <joze...@mittosystems.com> wrote:

> On Sat, 07 Dec 2019 11:27:54 -0700
> Jeff Law <l...@redhat.com> wrote:
> 
> > On Wed, 2019-11-06 at 16:19 +0000, Jozef Lawrynowicz wrote:  
> > > From 7bc0971d2936ebe71e7b7d3d805cf1bbf9f9f5af Mon Sep 17 00:00:00 2001
> > > From: Jozef Lawrynowicz <joze...@mittosystems.com>
> > > Date: Mon, 4 Nov 2019 17:38:13 +0000
> > > Subject: [PATCH 3/3] libgcc: Implement TARGET_LIBGCC_REMOVE_DSO_HANDLE
> > > 
> > > gcc/ChangeLog:
> > > 
> > > 2019-11-06  Jozef Lawrynowicz  <joze...@mittosystems.com>
> > > 
> > >   * doc/tm.texi: Regenerate.
> > >   * doc/tm.texi.in: Define TARGET_LIBGCC_REMOVE_DSO_HANDLE.
> > > 
> > > libgcc/ChangeLog:
> > > 
> > > 2019-11-06  Jozef Lawrynowicz  <joze...@mittosystems.com>
> > > 
> > >   * crtstuff.c: Don't declare __dso_handle if
> > >   TARGET_LIBGCC_REMOVE_DSO_HANDLE is defined.    
> > Presumably you'll switch this on for your bare elf target
> > configuration?  
> 
> Yep that's the plan. I originally didn't include the target changes in
> this patch since other target changes (disabling __cxa_atexit) were required 
> for
> the removal of __dso_handle to be OK.
> 
> > 
> > Are there other things, particularly related to shared library support,
> > that we wouldn't need to use as well?  The reason I ask is I'm trying
> > to figure out if REMOVE_DSO_HANDLE is the right name or if we should
> > generalize it to a name that indicates shared libraries in general
> > aren't supported on the target.  
> 
> CRTSTUFFS_O is defined when compiling shared versions of crt{begin,end} and
> handles an extra case in crtstuff.c where there's some shared library related
> functionality we don't need on MSP430.
> 
> But when CRTSTUFFS_O is undefined __dso_handle is still declared - to 0. The
> comment gives some additional insight:
> 
> /* Declare the __dso_handle variable.  It should have a unique value  
>    in every shared-object; in a main program its value is zero.  The  
>    object should in any case be protected.  This means the instance   
>    in one DSO or the main program is not used in another object.  The 
>    dynamic linker takes care of this.  */                             
> 
> I haven't noticed any further shared library-related bloat coming from libgcc.
> 
> I think a better way of solving this problem is just to check
> DEFAULT_USE_CXA_ATEXIT rather than adding this new macro. If __cxa_atexit is
> not enabled then as far as I understand __dso_handle serves no purpose.
> DEFAULT_USE_CXA_ATEXIT is defined at configure time for any targets that want
> __cxa_atexit support.
> 
> A quick bootstrap and test of dg.exp on x86_64-pc-linux-gnu shows no issues
> with the following:
> 
> > diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
> > index ae6328d317d..349f8191e61 100644
> > --- a/libgcc/crtstuff.c
> > +++ b/libgcc/crtstuff.c
> > @@ -340,8 +340,10 @@ extern void *__dso_handle __attribute__ 
> > ((__visibility__ ("hidden")));
> >  #ifdef CRTSTUFFS_O
> >  void *__dso_handle = &__dso_handle;
> >  #else
> > +#if DEFAULT_USE_CXA_ATEXIT
> >  void *__dso_handle = 0;
> >  #endif
> > +#endif
> >  
> >  /* The __cxa_finalize function may not be available so we use only a
> >     weak declaration.  */  
> 
> I'll put that patch through some more rigorous testing.

Successfully bootstrapped and regtested the attached patch for
x86_64-pc-linux-gnu (where DEFAULT_USE_CXA_ATEXIT is set to 1) and the proposed
msp430-elfbare target (where DEFAULT_USE_CXA_ATEXIT is set to 0).

Ok to apply?
> 
> Thanks,
> Jozef
> > 
> > Jeff  

>From fc2564803c33229184926a5ac6db62ae36ea8d77 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <joze...@mittosystems.com>
Date: Mon, 9 Dec 2019 15:20:23 +0000
Subject: [PATCH] libgcc: Only define __dso_handle if DEFAULT_USE_CXA_ATEXIT is
 true

libgcc/ChangeLog:

2019-12-11  Jozef Lawrynowicz  <joze...@mittosystems.com>

	* crtstuff.c: Declare __dso_handle only if DEFAULT_USE_CXA_ATEXIT is
	true.

---
 libgcc/crtstuff.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
index 9346cc5ca54..e282cb1aabb 100644
--- a/libgcc/crtstuff.c
+++ b/libgcc/crtstuff.c
@@ -325,11 +325,14 @@ register_tm_clones (void)
 
 #ifdef OBJECT_FORMAT_ELF
 
+#if DEFAULT_USE_CXA_ATEXIT
 /* Declare the __dso_handle variable.  It should have a unique value
    in every shared-object; in a main program its value is zero.  The
    object should in any case be protected.  This means the instance
    in one DSO or the main program is not used in another object.  The
-   dynamic linker takes care of this.  */
+   dynamic linker takes care of this.
+   If __cxa_atexit is not being used, __dso_handle will not be used and
+   doesn't need to be defined.  */
 
 #ifdef TARGET_LIBGCC_SDATA_SECTION
 extern void *__dso_handle __attribute__ ((__section__ (TARGET_LIBGCC_SDATA_SECTION)));
@@ -342,6 +345,7 @@ void *__dso_handle = &__dso_handle;
 #else
 void *__dso_handle = 0;
 #endif
+#endif /* DEFAULT_USE_CXA_ATEXIT */
 
 /* The __cxa_finalize function may not be available so we use only a
    weak declaration.  */
-- 
2.17.1

Reply via email to