[PATCH] D58625: [llvm-objcopy] - Check for invalidated relocations when removing a section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/strip-section-err.test:7
+
+# CHECK: error: Section .data cannot be removed because it is referenced by the relocation section .rela.text.
+ 
----------------
The relocations don't really reference the section, but rather a symbol in that section. I think the error message should reflect that somehow. It would also be great if the index of the relocation could be mentioned in the error message.


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1375
+  // For each section that remains alive, we want to remove the dead references.
+  // This either might update the content of the section (e.g.remove symbols
+  // from symbol table that belongs to removed section) or trigger an error if
----------------
Nit: add a space between "e.g." and "remove".


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1377
+  // from symbol table that belongs to removed section) or trigger an error if
+  // alive section critically depends on a section being removed somehow
+  // (e.g.removed section is referenced by a relocation).
----------------
alive -> a live (here only, not in the first line).


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1378
+  // alive section critically depends on a section being removed somehow
+  // (e.g.removed section is referenced by a relocation).
+  for (auto &KeepSec : make_range(std::begin(Sections), Iter)) {
----------------
Same as above comment on e.g.


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

https://reviews.llvm.org/D58625