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

[Rasmus Villemoes]

On 2018-06-19 18:45, Olivier Hainque wrote:
> Hi Rasmus,
> 
> 
> 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.

Yes, but doing regexp match on nm output feels wrong (though I am aware
that that is how things have been done for decades), and I really prefer
using the modern methods, which also allows e.g. use of constructor
priorities.

> 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.

Ah, of course. I'll add parens for the next roll.

> 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.

Hm, but if somebody built a compiler with --enable-initfini-array today
and passed -mrtp to it, they would already get .init_array instead of
.ctors, no? I.e., initfini-array.h would be included by tm.h after
vxworks.h, and the logic there would unconditionally override
vxworks.h's setting of TARGET_ASM_CONSTRUCTOR. So this "forcing" seems
to happen already for such a compiler, regardless of my patch?

> 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. 

OK.

> 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.

OK.

> 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).

See above, I don't see how that could happen. Patch 6/6 is essentially a
no-op, except that it makes the compiler binary a tiny bit smaller.

> 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.

Yes, I hope that building and installing crtbegin/end should be
harmless; it just requires that crtstuff.c actually compiles cleanly.

> 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).

Yes, these two are the most hackish ones, and I don't much like them
myself. So I'm happy to leave them out of mainline, and just use them
(or something equivalent) in-house.

> 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.

Using a custom spec file, custom linker script and something like the
vx_ctors_begin.o will probably be the long-term solution for us. We'd be
really happy to be able to use an upstream gcc out-of-the-box apart from
those few pieces of glue.

I'll fix and resend 1,2,3,6.

Once again, thank you for you time, Olivier!

Rasmus