[PATCH] D118840: [ELF] Support (TYPE=<value>) to customize the output section type
Peter Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 8 05:41:56 PST 2022
peter.smith added a comment.
Thanks for the update. The intent is clearer now. I've left some small suggestions.
================
Comment at: lld/ELF/ScriptParser.cpp:793
+constexpr std::pair<const char *, unsigned> typeMap[] = {
+ ECase(SHT_PROGBITS), ECase(SHT_STRTAB), ECase(SHT_NOTE),
+ ECase(SHT_NOBITS), ECase(SHT_INIT_ARRAY), ECase(SHT_FINI_ARRAY),
----------------
I'm interested in why `SHT_STRTAB` that is usually only accessed from linker generated symbol tables. While in theory it could be constructed by a user created section, what symbol table would reference it with the sh_link field.
================
Comment at: lld/ELF/ScriptParser.cpp:797
+};
+
// Tries to read the special directive for an output section definition which
----------------
Worth undefining ECase after its last use?
================
Comment at: lld/ELF/ScriptParser.cpp:799
// Tries to read the special directive for an output section definition which
// can be one of following: "(NOLOAD)", "(COPY)", "(INFO)" or "(OVERLAY)".
// Tok1 and Tok2 are next 2 tokens peeked. See comment for readSectionAddressType below.
----------------
Comment will need updating to include (TYPE)
================
Comment at: lld/ELF/ScriptParser.cpp:822
+ // Otherwise, read an expression.
+ cmd->type = readExpr()().getValue();
+ }
----------------
It will be worth checking that there is a `SHT_` prefix here so we can give a better error message if someone has used an unsupported type like `SHT_DYNAMIC`.
================
Comment at: lld/docs/ELF/linker_script.rst:109
+
+When ``sh_type`` is specified, it is an error if an input section has a
+different type.
----------------
Suggest `if an input section in *S* has a different type.
================
Comment at: lld/test/ELF/linkerscript/custom-section-type.s:62
+ nobits ( TYPE=SHT_NOBITS) : { BYTE(8) }
+ init_array (TYPE=SHT_INIT_ARRAY ) : { QUAD(14) }
+ fini_array (TYPE=SHT_FINI_ARRAY) : { QUAD(15) }
----------------
For `SHT_INIT_ARRAY` et-al to be practically useful I guess we're looking at something like
```
.init_array { QUAD(function_name) }
```
As the address of the function to execute won't usually be known when writing the script. Is it worth making the test reflect that?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118840/new/
https://reviews.llvm.org/D118840
More information about the llvm-commits
mailing list