[PATCH] D38260: [llvm-objcopy] Add support for removing sections

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Not sure if this got missed from my earlier comments, but I think you need tests for removing sections when there are segments present, to show that the addresses are unchanged, even if the offset changes, and to show the restriction on moving sections within segments etc. I've suggested changing the remaining yaml2obj-based tests to use ET_REL, to crudely indicate these are missing segments (feel free to ignore those comments), and then any new tests involving segments would use ET_EXEC.



================
Comment at: test/tools/llvm-objcopy/remove-section-with-symbol.test:9
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
----------------
ET_EXEC -> ET_REL


================
Comment at: test/tools/llvm-objcopy/remove-section.test:9
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
----------------
ET_EXEC -> ET_REL


================
Comment at: tools/llvm-objcopy/Object.h:213
+template <class SymTabType>
+class RelocSectionWithSymtabBase : public RelocationSectionBase {
 private:
----------------
Could we add a comment to explain why we need separate RelocationSectionBase and RelocationSectionWtihSymtabBase classes, please.

Also, I think we need to prevent users directly instantiating RelocationSectionBase somehow, maybe by giving it a protected constructor?


Repository:
  rL LLVM

https://reviews.llvm.org/D38260