[PATCH v3 REVIEW 25/33] bfd, ld: add CTF section linking

[Nick Alcock]

On 9 Sep 2019, Alan Modra uttered the following:

> On Fri, Sep 06, 2019 at 11:55:12PM +0100, Nick Alcock wrote:
>>  - There are no SEC_* flags left, and we do not have a SHT_ type for CTF
>>    sections yet -- so we are reduced to doing string comparisons to
>>    determine whether we have to do special things for the CTF section.
>>    Nearly all of these things apply to any such "late-generated
>>    section", but in the absence of any remaining SEC_ flags I cannot say
>>    that.  Does anyone know if any of these flags are unused, or if we
>>    can make the flags word larger, or something?  I have defined a macro,
>>    SECTION_IS_CTF, to do the ugly string comparisons for now.
>
> Well if you don't have SHT_* or SHF_* for CTF sections then you'd be
> setting a SEC_* flag from the section name anyway. So I think you're
> stuck with name comparisons.

Oh well: I was hoping there was a better way :( At some point I guess we
should think about widening that flags word...

>                              Using a flag might be necessary if the
> CTF code impacts link time too much, but I doubt that is the case.

It *was* too slow, but after patch 30 in this series it isn't any more.
:)

(We shall see how slow the deduplicator is once I write it -- but I'm
hoping I can make it multithreaded to keep costs down.)

>>  - There is an arbitrary threshold value above which CTF sections are
>>    compressed (roughly: it is actually the threshold above which
>>    ctf_file_t's are compressed, and a CTF section may be comprised of
>>    many of these in a ctf_archive_t).  Right now I have arbitrarily
>>    hardwired this threshold at 4096 bytes.  Should it be configurable?
>>    Is a somewhat bigger value saner, on the grounds that wasting space
>>    on compression dictionaries for 4KiB files is just nuts? (64KiB is
>>    probably too big...)
>
> I think the 4k threshold is fine.  Maybe a #define?

Good idea, will do. (I doubt anyone will ever change it :) but if
someone does, I can add a new ctf_link_*() function to make it settable
at that layer, I suppose.)

>>  - We might well have memory leaks: we open the CTF sections for the
>>    input files and then never close them again.  Should we? ld does seem
>>    to operate on the basis that input files live forever so it doesn't
>>    matter if they are never freed...
>
> You might want to do something depending on the state of
> link_info.keep_memory.

Ooh, good idea. I'll see what other things do and do the same thing.

>>  - I'm fairly unhappy that I have to modify the default linker script,
>>    because that means that any project wanting CTF support and providing its
>>    own linker script will have to change.  But without doing this, we
>>    never get an input->output mapping and the CTF sections are simply never
>>    emitted.  This seems to be because non-loaded sections are simply thrown
>>    away in the elf32.em orphan-assignment code: but even if it might be
>
> That isn't true.  Non-alloc orphans are handled fine.  I suspect what
> you're running into is removal of zero size sections by
> strip_excluded_output_sections.

So... oh god this is disgusting... I could just set the size to 1, and
it would work? (oh wait, no, I should just teach
strip_excluded_output_sections() about CTF sections and it'll work. I
hope. I have vague memories of trying this before and it didn't work:
I'll try it again and see. Not before next week, though, after LPC.)

>>    desirable, we *cannot* make this a loaded section, since that would
>>    require its size and position to be computed before the strtab is laid
>>    out, while the strtab dedup, symtab shuffling, and compression requires us
>>    to compute it afterwards.  So we could avoid modifying the linker script
>>    by modifying the ELF orphan-assignment code, I suppose, but I have no idea
>>    which might be preferable.  The linker script modification is certainly
>>    simpler.
>
>> 	* ldlang.h (includes): Add elf-bfd.h, ctf-api.h.  Prevent NAME in
>> 	elf-bfd.h from wreaking havoc.
>
> No, elf-bfd.h should not be included by ldlang.h, and the fact that
> you needed to #undef NAME should have alerted you that this isn't a
> good idea.

Ya think? :) honestly I was assuming that this was a BFD bug: NAME is a
rather bad thing to name a macro.

>            This part of your patch needs rethinking.
>
> We probably should not have been lazy and allowed elf-bfd.h and
> coff-bfd.h in any of the generic linker code, but what we have could
> be tidied fairly easily.  With that in mind, ideally you'd write your
> new ldlang.c code in such a way that elf-bfd.h is not needed. I see
> you've discovered ldemul.[ch] so I don't need to point you at that as
> a way of putting the fancy CTF and ELF strtab interface code in
> elf-generic.em or even a new file.

Hmm. The problem here is specifically the examine_strtab callback, and
thus ldlang_ctf_apply_strsym(). Fundamentally we need to pass "the
format-dependent strtab syms" and "the format-dependent strtab hash"
from target-dependent code in bfd, through the generic ld layer, into
the emulation/target-dependent code in ld. Right now, this means the
callback is taking elf_sym_strtab * and elf_strtab_hash * struct
pointers, because that's what ELF is using -- and bfdlink.h already
defines these as opaque forwards since it's not ELF-specific. Now that I
am just passing them down through ldlang.c and not dereferencing them
there, and in fact only passing them (eventually) through elf-generic.em
and back into BFD... I think I can probably get away with leaving them
as forwards throughout ldlang.h and the emulation code. That seems
*much* neater. (And would have been possible even when all the work was
done in ldlang.c, I think, so I don't know why I didn't see that then.)

I'll give that a try next week, once my sore leg from kicking myself
heals. :P

(In the longer term it might be nicer to give them non-ELF-specific
names, but that would mean casting at every use point, or adding piles
of abstraction that would be redundant until the possibly-never-
happening future when CTF gains support for strtab sharing or symtab
shuffling with some non-ELF formats. I think that means that such
abstraction would just be obfuscation if we did it before then. But that
doesn't mean we need the *definitions* of those structs visible, nor
bfd-elf.h included in generic code!)

-- 
NULL && (void)