[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