[PATCH] D89432: [llvm-elfabi] Emit ELF .dynsym and .dynamic sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:297
     }
-    std::vector<OutputSection<ELFT> *> Sections;
+    for (const auto &Lib : Stub.NeededLibs) {
+      StrTab.Content.add(Lib);
----------------
Remove braces
http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:320
+
+    // populate dynamic symbol table
+    for (const auto &Sym : Stub.Symbols) {
----------------
Capitalize the comment and end with a period.

https://llvm.org/docs/CodingStandards.html#commenting


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:330
+
+    // poplulate dynamic table
+    size_t DynSymIndex = DynTab.Content.addAddr(DT_SYMTAB, 0);
----------------
ditto


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:334
+    for (const auto &Lib : Stub.NeededLibs) {
+      DynTab.Content.addNeededLib(StrTab, Lib);
+    }
----------------
ditto


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:389
     writeShdr(Data, StrTab);
+    writeShdr(Data, DynSym);
+    writeShdr(Data, DynTab);
----------------
Usually linkers place .dynsym .dynstr before .strtab .shstrtab .symtab

The source order does not reflect the actual order and can cause confusion


================
Comment at: llvm/test/tools/llvm-elfabi/binary-write-sheaders.test:66
 # SECTIONS-NEXT: }
+# SECTIONS-NEXT:  Section {
+# SECTIONS-NEXT:    Index: 3
----------------
The section order should be changed. See the comment above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89432/new/

https://reviews.llvm.org/D89432