[PATCH] D93678: [yaml2obj] - Support selecting the location of the section header table.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 22 20:48:10 PST 2020


MaskRay added a comment.

The placement was changed by D67221 <https://reviews.llvm.org/D67221>. This patch adds an option to change the placement which is definitely useful. A design with the syntax is requires so I think it'd be best to wait after @jhenderson comes back from the vacation. I haven't look hard whether the code can be simplified yet.



================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:882
+        *SecHdrTable.Location != ELFYAML::SectionHeaderTable::BeforeSec)
+      return ("Allowed values for Location are: " +
+              ELFYAML::SectionHeaderTable::BeforeSec + ", " +
----------------
Most binary utilities do not capitalize diagnostics.


================
Comment at: llvm/test/tools/yaml2obj/ELF/section-headers-location.yaml:1
+## Check how we can use the "Location" key of the "SectionHeaderTable" tag to
+## specify where the section header table should be placed.
----------------
Test that the "Location" key of "SectionHeaderTable" tag can change the placement of the section header table.


================
Comment at: llvm/test/tools/yaml2obj/ELF/section-headers-location.yaml:4
+
+## Check we can set the "Location" key to "AfterSecData" to place the section
+## header table after all sections data.
----------------
"AfterSecData" is the default which places the section header table after all section contents (i.e. at the end of the file)


================
Comment at: llvm/test/tools/yaml2obj/ELF/section-headers-location.yaml:39
+
+## Check we do not allow using the "Location" key when the
+## "NoHeaders" key is set to true.
----------------
"Location" cannot be used when "NoHeaders" is true

(Personally I think words like "Check" "we do not allow" can be removed)


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

https://reviews.llvm.org/D93678



More information about the llvm-commits mailing list