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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 29 01:37:32 PDT 2017


jhenderson added a comment.

I think we could use one more test, if it is possible to construct: a section between two segments that is then removed, allowing the second segment to move in the file (but not in memory):

  | Section 1 | Section 2 | Section 3 |
  | Segment 1 |-----------| Segment 2 |

Don't worry about it, if it's not possible to sensibly, as I think this an edge case that is unlikely to be seen in most situations.



================
Comment at: test/tools/llvm-objcopy/segment-test-remove-section.test:4
+# section inside that segment maintains the relative positioning it had in the
+# segment. Note worthy is that .text3 keeps it's offset despite it being
+# possible to place it after .text when .text2 is removed.
----------------
it's -> its


================
Comment at: test/tools/llvm-objcopy/symtab-error-on-remove-strtab.test:11
+
+# CHECK: String table .strtab cannot be removed because it is referenced by the symbol table
----------------
by the symbol table .symtab


================
Comment at: tools/llvm-objcopy/Object.h:200
+// All relocation sections denote relocations to apply to another section.
+// However some relocation sections use a dynamic symbol table and others use
+// a regular symbol table. Because the types of the two symbol tables differ in
----------------
However some... -> However, some


================
Comment at: tools/llvm-objcopy/Object.h:206
+// different classes, one which handles the section the relocation is applied to
+// and the other handles the symbol table type. The symbol table type is taken
+// as a type parameter to the class (see RelocSectionWithSymtabBase).
----------------
the other handles... -> another which handles


================
Comment at: tools/llvm-objcopy/Object.h:221
+
+// Takes the symbol table type to use as a parameter so that we can deduplicate.
+// that code between the two relocation types.
----------------
Too many full stops this time!


================
Comment at: tools/llvm-objcopy/Object.h:222
+// Takes the symbol table type to use as a parameter so that we can deduplicate.
+// that code between the two relocation types.
+template <class SymTabType>
----------------
two relocation types -> two symbol table types

Two relocation types implies to me "Rel" and "Rela" relocations, which isn't what is relevant here. What's relevant is the symbol table being referred to.


Repository:
  rL LLVM

https://reviews.llvm.org/D38260





More information about the llvm-commits mailing list