Home Original page

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

Alan Modra amodra@gmail.com
Mon Sep 9 00:42:00 GMT 2019 
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.  Using a flag might be necessary if the
CTF code impacts link time too much, but I doubt that is the case.

>  - The naming of functions in ldlang.c is opaque: some are ldlang_* and
>    some are lang_*.  I've split the difference so things are named
>    similarly to the functions called near them in lang_process, but this
>    feels... wrong.  Is there any consistency here?

Heh, no.  (But the naming of static functions hardly matters.)

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

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

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

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

-- 
Alan Modra
Australia Development Lab, IBM
More information about the Binutils mailing list