[PATCH 0/4] some vxworks crtstuff

[PATCH 0/4] some vxworks crtstuff

[Rasmus Villemoes]

From: Rasmus Villemoes <rv@rasmusvillemoes.dk>

A few things I hit when trying to upgrade our VxWorks5 toolchain. I
don't think these can break anything for VxWorks 6+, and patch 2
should be an improvement for all in that the current code doesn't get
compiled as it was clearly intended - though the real bug is likely in
gcc itself, it's easier to work around here by just removing the
declarations.

OK for master?


Rasmus Villemoes (4):
  libgcc: remove crt{begin,end}.o from powerpc-wrs-vxworks target
  libgcc: vxcrtstuff.c: remove ctor/dtor declarations
  libgcc: vxcrtstuff.c: make ctor/dtor functions static
  libgcc: vxcrtstuff.c: add a few undefs

 libgcc/config.host         |  1 -
 libgcc/config/vxcrtstuff.c | 21 +++++++++++++++------
 2 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.31.1

[PATCH 1/4] libgcc: remove crt{begin, end}.o from powerpc-wrs-vxworks target

[Rasmus Villemoes]

Since commit 78e49fb1bc (Introduce vxworks specific crtstuff support),
the generic crtbegin.o/crtend.o have been unnecessary to build. So
remove them from extra_parts.

This is effectively a revert of commit 9a5b8df70 (libgcc: add
crt{begin,end} for powerpc-wrs-vxworks target).

libgcc/
	* config.host (powerpc-wrs-vxworks): Do not add crtbegin.o and
	crtend.o to extra_parts.
---
 libgcc/config.host | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libgcc/config.host b/libgcc/config.host
index 85de83da766..651e63adb23 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -1243,7 +1243,6 @@ powerpc*-wrs-vxworks7*)
         ;;
 powerpc-wrs-vxworks*)
 	tmake_file="$tmake_file rs6000/t-ppccomm rs6000/t-savresfgpr t-fdpbit"
-	extra_parts="$extra_parts crtbegin.o crtend.o"
 	;;
 powerpc-*-lynxos*)
 	tmake_file="$tmake_file rs6000/t-lynx t-fdpbit"
-- 
2.31.1

Re: [PATCH 1/4] libgcc: remove crt{begin,end}.o from powerpc-wrs-vxworks target

[Olivier Hainque]

Hi Rasmus,

(making progress but not quite there on the stdint business)

> On 1 Nov 2021, at 10:34, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> 
> Since commit 78e49fb1bc (Introduce vxworks specific crtstuff support),
> the generic crtbegin.o/crtend.o have been unnecessary to build. So
> remove them from extra_parts.
> 
> This is effectively a revert of commit 9a5b8df70 (libgcc: add
> crt{begin,end} for powerpc-wrs-vxworks target).
> 
> libgcc/
> 	* config.host (powerpc-wrs-vxworks): Do not add crtbegin.o and
> 	crtend.o to extra_parts.

Yes, this one is ok, thanks!

[PATCH 2/4] libgcc: vxcrtstuff.c: remove ctor/dtor declarations

[Rasmus Villemoes]

These declarations prevent the priority given in the
constructor/destructor attributes from taking effect, thus emitting
the function pointers in the ordinary (lowest-priority)
.init_array/.fini_array sections.

libgcc/
	* config/vxcrtstuff.c: Remove constructor/destructor
	declarations.
---
 libgcc/config/vxcrtstuff.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libgcc/config/vxcrtstuff.c b/libgcc/config/vxcrtstuff.c
index 87fadda9ac5..c146e1be3be 100644
--- a/libgcc/config/vxcrtstuff.c
+++ b/libgcc/config/vxcrtstuff.c
@@ -88,9 +88,6 @@ __attribute__((section(__LIBGCC_EH_FRAME_SECTION_NAME__), aligned(4)))
 
 #endif /* USE_INITFINI_ARRAY  */
 
-void EH_CTOR_NAME (void);
-void EH_DTOR_NAME (void);
-
 EH_CTOR_ATTRIBUTE void EH_CTOR_NAME (void)
 {
   static struct object object;
-- 
2.31.1

Re: [PATCH 2/4] libgcc: vxcrtstuff.c: remove ctor/dtor declarations

[Olivier Hainque]

> On 1 Nov 2021, at 10:34, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> 
> These declarations prevent the priority given in the
> constructor/destructor attributes from taking effect, thus emitting
> the function pointers in the ordinary (lowest-priority)
> .init_array/.fini_array sections.
> 
> libgcc/
> 	* config/vxcrtstuff.c: Remove constructor/destructor
> 	declarations.

Ok, thanks.

Olivier

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

[Rasmus Villemoes]

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

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

[Olivier Hainque]

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
> 

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

[Olivier Hainque]

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

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

[Rasmus Villemoes]

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

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

[Olivier Hainque]

> 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 4/4] libgcc: vxcrtstuff.c: add a few undefs

[Rasmus Villemoes]

When vxcrtstuff.c was created, the set of #includes was copied from
crtstuff.c. But crtstuff.c also has a bunch of #undefs after the first
#include, because, as the comment says, including auto-host.h when
building objects that are meant for target is technically not
correct.

This manifests when I try do do a canadian cross, with build=linux,
host=windows and target=vxworks, in that we pick up a

  #define caddr_t char *

from auto-host.h, which then of course creates a problem when we later
include a target header that has

  typedef char * caddr_t;

I assume that the #undefs in crtstuff.c have been added for similar
reasons.

These potentially problematic #defines all seem to be guarded by
#ifndef USED_FOR_TARGET, which tconfig.h defines before including
auto-host.h. So at first, it seems that one could avoid the problem
by simply removing the initial include of auto-host.h. Unfortunately,
we do need some of the things defined in auto-host.h within such an
ifndef USED_FOR_TARGET, namely the define of
HAVE_INITFINI_ARRAY_SUPPORT, which is what later causes
initfini-array.h to define USE_INITFINI_ARRAY. So as the next best
fix, just copy the #undefs from crtstuff.c.

libgcc/
	* config/vxcrtstuff.c: Undefine caddr_t, pid_t, rlim_t,
	ssize_t and vfork after including auto-host.h.
---
 libgcc/config/vxcrtstuff.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/libgcc/config/vxcrtstuff.c b/libgcc/config/vxcrtstuff.c
index 652a65364b0..c15e15e54e9 100644
--- a/libgcc/config/vxcrtstuff.c
+++ b/libgcc/config/vxcrtstuff.c
@@ -26,7 +26,15 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #define IN_LIBGCC2
 
+/* FIXME: Including auto-host is incorrect, but until we have
+   identified the set of defines that need to go into auto-target.h,
+   this will have to do.  */
 #include "auto-host.h"
+#undef caddr_t
+#undef pid_t
+#undef rlim_t
+#undef ssize_t
+#undef vfork
 #include "tconfig.h"
 #include "tsystem.h"
 #include "coretypes.h"
-- 
2.31.1

Re: [PATCH 4/4] libgcc: vxcrtstuff.c: add a few undefs

[Olivier Hainque]

Hi Rasmus,

> On 1 Nov 2021, at 10:34, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> 
> libgcc/
> 	* config/vxcrtstuff.c: Undefine caddr_t, pid_t, rlim_t,
> 	ssize_t and vfork after including auto-host.h.

Ok, thanks;