Re: [RFC PATCH 0/6] some vxworks/powerpc patches

[Olivier Hainque <hainque at adacore dot com>]

Hi Rasmus,

> On 4 Jun 2018, at 16:46, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
> These are some of the patches I'm currently applying to get a full C
> and C++ compiler to work for powerpc-wrs-vxworks (for vxworks
> 5.5). With these, and a few more, I have ZCX, static
> constuctors/destructors and throwing across modules working. There are
> many vxworks variants out there, so I'm not sure how to avoid breaking
> things for others. I don't really like the crti hack, but I don't have
> any better ideas (I can probably patch the loader to recognize even
> symbols of type unknown, but that won't benefit anybody else, so I'm
> trying to fix problems in gcc whenever possible).
> 
> Rasmus Villemoes (6):

1/6:
>  vxworks: add target/h/wrn/coreip to the set of system include paths

As we discussed on the message dedicated to this patch, this
one is ok.

For the rest, I still have concerns for part of the patches. Some
would be ok modulo minor changes.

Before we get there: are you familiar with the "munch" facility
of VxWorks ?

That can offer alternatives to some of the things you were after.

3/6:
>  vxworks: enable use of .init_array/.fini_array for cdtors

Assuming we agree on the general approach, this part would
be fine modulo adding parens around the expression. It doesn't
do what you think it does in the current form.

I'm a bit worried that forcing .init_array for constructors
might not work for RTPs, at least on some versions of VxWorks.

We probably would need to fallback to ".ctors" as the section
base name for not so recent versions. I'm not clear on all the
ramifications there.

As long as we keep the patchset logic simple, that should not
be an issue, as we'll have only few things to adjust.

By keeping the logic simple, I mean restricting the effects
of --enable-initfini-array to a strict minimum in the toolchain
per se.

A bit like "issue ctors/dtors and .init_array/.fini_array
sections and assume the environment has the necessary support
to trigger the calls in priority order." and that's it.

Some of the crt related patches are too potentially damaging
for other modes of possible uses of a VxWorks toolchain, with
possibly other versions of VxWorks. 

I understand it works for you, that --enable-initfini-array is
a new mode of use and that users would have the choice of not
configuring with this.

I still think that we don't want to bind that configuration choice
to something too specific.

The more we enforce from that config choice, the trickier it will
get to be able to understand/explain what it means. The least we enforce,
the easier it will be to maintain and understand, and the more flexible
it will be for users.

I'll propose alternatives to the crt parts that I'd rather not get
in (below).

6/6:
>  vxworks: don't define vxworks_asm_out_constructor when using .init_array

Would be ok together with the previous one, and as long as we don't
uncover fallouts for RTPs (meaning, we might have to revisit this in the
future).

2/6:
>  libgcc: add crt{begin,end} for powerpc-wrs-vxworks target

That one would be fine as well. Even if we don't immediately add them
to link closures from the config files (see below), I see how handy it
can be to be able to reference them from user specs for instance.

4 and 5/6
>  powerpc/vxworks: add {START,END}FILE_SPEC for HAVE_INITFINI_ARRAY_SUPPORT
>  powerpc/vxworks: [hack] add crti.o file

I really think those two should stay out. As I mentioned in a previous message,
there are many possible uses of a toolchain and this (unconditional addition
of crtfiles and references to _ctors) would undoubtedly cause trouble in one
case or another.

Also, the reference to _ctors tie to strong assumptions about how the
environment arranges for .init_array to work, so really blurs the responsibility
assignments.

We might be able to come up with a proper set of options controlling what
crt file to include in such or such circumstance but we're not quite there yet.

In the meantime, I think you could get things to work with a custom
userland spec and local crti.o (or alike, on your end so you can name and
locate it as you wish).

You could have, for example a spec file like this (sketch, to give an idea):

*startfile:
+ vx_ctors-begin.o crtbegin.o%s

where vx_ctors-begin.c would hold something like:

  void (* volatile _ctors[])()
    __attribute__((section (".init_array"))) = {};

with possible adjustments to priority sections and synchronization
with your linker script.

You can handle the sentinel addition likewise, with a '*endfile'
spec, or leave this to your linker script.

Regards,

Olivier