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

[Olivier Hainque <hainque at adacore dot com>]

Hello Rasmus,

> On 26 Jun 2018, at 14:45, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
> On 2018-06-19 18:45, Olivier Hainque wrote:
>> 
>> Before we get there: are you familiar with the "munch" facility
>> of VxWorks ?
> 
> 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.

I agree the perspective of having an alternate mechanism is
attractive :)

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

Right. It's just that there is no trace of init-fini support
before the patch, so it is not really expected to work. The
presence of conditions on HAVE_INITFINI_ARRAY here and there sends
a pretty strong signal that --enable-initfini-array is expected
to do something sensible, and obviously breaking RTPs would be
annoying.

Now the patches we're discussing (without 4 and 5) are non-intrusive
enough to allow a quick understanding and simple definition of the effects,
and it's not clear that RTPs would indeed be broken to this is not
blocking at this stage.

This can be seen as a step forward which maybe will call for adjustments
for RTPs.

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

In the current state, I agree. Experiments on RTPs might show a need
for adjustments but we're not there and it's fine to defer this part.

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

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

Ok, glad we agree on this.

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

Works for me. I won't be able to review for 3+ weeks.

> Once again, thank you for you time, Olivier!

Sure. Thanks for your proposing the approach and input
on how it works for you. This is an interesting way to address
a pretty hairy issue.

Regards,

Olivier