[3/4] libgcc: vxcrtstuff.c: make ctor/dtor functions static
Commit Message
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
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
>
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
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
> 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
@@ -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__);
}