[PATCH] D58168: [yaml2obj] - Do not ignore explicit addresses for .dynsym and .dynstr

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 18 06:01:26 PST 2019


jhenderson added inline comments.


================
Comment at: test/tools/yaml2obj/dynsym-dynstr-addr.yaml:5
+## Check yaml2obj soes not ignore the address of the
+## explicitly listed .dynstr and .dynstr sections.
+
----------------
Nit: .dynstr -> .dynsym in one case


================
Comment at: tools/yaml2obj/yaml2elf.cpp:324-325
 
+  // If .dynsym section is explicitly described in the YAML
+  // then we want to use its section address.
+  if (!IsStatic) {
----------------
Nit: Let clang-format reflow your comments across multiple lines, rather than doing it by hand.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:377
+    if (SecNdx < Doc.Sections.size())
+      SHeader.sh_addr = Doc.Sections[SecNdx]->Address;
+  }
----------------
Do you think this should have a sanity assertion to show that Doc.Sections[SecNdx] is the right section? If I'm right, this only works because an auto-generated .dynstr happens to be added to the end of the sections, but it would not be unreasonable to change that behaviour.


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

https://reviews.llvm.org/D58168





More information about the llvm-commits mailing list