[3/4] libgcc: vxcrtstuff.c: make ctor/dtor functions static

Message ID 20211101093456.2003236-4-rasmus.villemoes@prevas.dk
State Committed
Commit 365c7c6ac5400d88816e602f7f9aa12268ba53e3
Headers
Series some vxworks crtstuff |

Commit Message

Rasmus Villemoes Nov. 1, 2021, 9:34 a.m. UTC
  When the translation unit itself creates pointers to the ctors/dtors
in a specific section handled by the linker (whether .init_array or
.ctors.*), there's no reason for the functions to have external
linkage. That ends up polluting the symbol table in the running
kernel.

This makes vxcrtstuff.c on par with the generic crtstuff.c which also
defines e.g. frame_dummy and __do_global_dtors_aux static.

libgcc/
	* config/vxcrtstuff.c: Make constructor and destructor
	functions static when possible.
---
 libgcc/config/vxcrtstuff.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
  

Comments

Olivier Hainque Nov. 30, 2021, 10:14 a.m. UTC | #1
Hi Rasmus,

We had something close but slight different for
the support of shared libraries (for which I'm preparing
the patches). I think your version should work as well
but we have quite a few configurations and the devil is
in the details so I'm testing the effects in a few cases
before approving.

Olivier

> On 1 Nov 2021, at 10:34, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> 
> When the translation unit itself creates pointers to the ctors/dtors
> in a specific section handled by the linker (whether .init_array or
> .ctors.*), there's no reason for the functions to have external
> linkage. That ends up polluting the symbol table in the running
> kernel.
> 
> This makes vxcrtstuff.c on par with the generic crtstuff.c which also
> defines e.g. frame_dummy and __do_global_dtors_aux static.
> 
> libgcc/
> 	* config/vxcrtstuff.c: Make constructor and destructor
> 	functions static when possible.
> ---
> libgcc/config/vxcrtstuff.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libgcc/config/vxcrtstuff.c b/libgcc/config/vxcrtstuff.c
> index c146e1be3be..652a65364b0 100644
> --- a/libgcc/config/vxcrtstuff.c
> +++ b/libgcc/config/vxcrtstuff.c
> @@ -58,14 +58,18 @@ __attribute__((section(__LIBGCC_EH_FRAME_SECTION_NAME__), aligned(4)))
> 
> #define EH_CTOR_NAME _crtbe_register_frame
> #define EH_DTOR_NAME _ctrbe_deregister_frame
> +#define EH_LINKAGE static
> 
> #else
> 
> /* No specific sections for constructors or destructors: we thus use a
>    symbol naming convention so that the constructors are then recognized
> -   by munch or whatever tool is used for the final link phase.  */
> +   by munch or whatever tool is used for the final link phase.  Since the
> +   pointers to the constructor/destructor functions are not created in this
> +   translation unit, they must have external linkage.  */
> #define EH_CTOR_NAME _GLOBAL__I_00101_0__crtbe_register_frame
> #define EH_DTOR_NAME _GLOBAL__D_00101_1__crtbe_deregister_frame
> +#define EH_LINKAGE
> 
> #endif
> 
> @@ -88,13 +92,13 @@ __attribute__((section(__LIBGCC_EH_FRAME_SECTION_NAME__), aligned(4)))
> 
> #endif /* USE_INITFINI_ARRAY  */
> 
> -EH_CTOR_ATTRIBUTE void EH_CTOR_NAME (void)
> +EH_LINKAGE EH_CTOR_ATTRIBUTE void EH_CTOR_NAME (void)
> {
>   static struct object object;
>   __register_frame_info (__EH_FRAME_BEGIN__, &object);
> }
> 
> -EH_DTOR_ATTRIBUTE void EH_DTOR_NAME (void)
> +EH_LINKAGE EH_DTOR_ATTRIBUTE void EH_DTOR_NAME (void)
> {
>   __deregister_frame_info (__EH_FRAME_BEGIN__);
> }
> -- 
> 2.31.1
>
  
Olivier Hainque Dec. 10, 2021, 2:20 p.m. UTC | #2
Hi again Rasmus,

> On 1 Nov 2021, at 10:34, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> 
> When the translation unit itself creates pointers to the ctors/dtors
> in a specific section handled by the linker (whether .init_array or
> .ctors.*), there's no reason for the functions to have external
> linkage. That ends up polluting the symbol table in the running
> kernel.
> 
> This makes vxcrtstuff.c on par with the generic crtstuff.c which also
> defines e.g. frame_dummy and __do_global_dtors_aux static.
> 
> libgcc/
> 	* config/vxcrtstuff.c: Make constructor and destructor
> 	functions static when possible.


This is OK, can you please commit?

Thanks!

Cheers,

Olivier
  
Rasmus Villemoes Dec. 10, 2021, 3:07 p.m. UTC | #3
On 10/12/2021 15.20, Olivier Hainque wrote:
> Hi again Rasmus,
> 
>> On 1 Nov 2021, at 10:34, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
>>
>> When the translation unit itself creates pointers to the ctors/dtors
>> in a specific section handled by the linker (whether .init_array or
>> .ctors.*), there's no reason for the functions to have external
>> linkage. That ends up polluting the symbol table in the running
>> kernel.
>>
>> This makes vxcrtstuff.c on par with the generic crtstuff.c which also
>> defines e.g. frame_dummy and __do_global_dtors_aux static.
>>
>> libgcc/
>> 	* config/vxcrtstuff.c: Make constructor and destructor
>> 	functions static when possible.
> 
> 
> This is OK, can you please commit?

Well, yes, but it depends contextually (but not functionally) on patch
2/4. Should I redo this one, or can I get you to take a look at 2/4 first?

You've already ok'ed 1/4 and 4/4 (which were anyway independent of each
other and 2,3) and I've committed those

Rasmus
  
Olivier Hainque Dec. 10, 2021, 3:12 p.m. UTC | #4
> On 10 Dec 2021, at 16:07, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
>> 
>> This is OK, can you please commit?
> 
> Well, yes, but it depends contextually (but not functionally) on patch
> 2/4. Should I redo this one, or can I get you to take a look at 2/4 first?
> 
> You've already ok'ed 1/4 and 4/4 (which were anyway independent of each
> other and 2,3) and I've committed those


Ah, sure. Yes, 2/4 is ok as well. I have been testing with
a combined version of the two, actually.

I'll send a separate approval for 2/4 momentarily. Thanks.

Olivier
  

Patch

diff --git a/libgcc/config/vxcrtstuff.c b/libgcc/config/vxcrtstuff.c
index c146e1be3be..652a65364b0 100644
--- a/libgcc/config/vxcrtstuff.c
+++ b/libgcc/config/vxcrtstuff.c
@@ -58,14 +58,18 @@  __attribute__((section(__LIBGCC_EH_FRAME_SECTION_NAME__), aligned(4)))
 
 #define EH_CTOR_NAME _crtbe_register_frame
 #define EH_DTOR_NAME _ctrbe_deregister_frame
+#define EH_LINKAGE static
 
 #else
 
 /* No specific sections for constructors or destructors: we thus use a
    symbol naming convention so that the constructors are then recognized
-   by munch or whatever tool is used for the final link phase.  */
+   by munch or whatever tool is used for the final link phase.  Since the
+   pointers to the constructor/destructor functions are not created in this
+   translation unit, they must have external linkage.  */
 #define EH_CTOR_NAME _GLOBAL__I_00101_0__crtbe_register_frame
 #define EH_DTOR_NAME _GLOBAL__D_00101_1__crtbe_deregister_frame
+#define EH_LINKAGE
 
 #endif
 
@@ -88,13 +92,13 @@  __attribute__((section(__LIBGCC_EH_FRAME_SECTION_NAME__), aligned(4)))
 
 #endif /* USE_INITFINI_ARRAY  */
 
-EH_CTOR_ATTRIBUTE void EH_CTOR_NAME (void)
+EH_LINKAGE EH_CTOR_ATTRIBUTE void EH_CTOR_NAME (void)
 {
   static struct object object;
   __register_frame_info (__EH_FRAME_BEGIN__, &object);
 }
 
-EH_DTOR_ATTRIBUTE void EH_DTOR_NAME (void)
+EH_LINKAGE EH_DTOR_ATTRIBUTE void EH_DTOR_NAME (void)
 {
   __deregister_frame_info (__EH_FRAME_BEGIN__);
 }