[PATCH] D58960: [llvm-objcopy] - Fix --compress-debug-sections when there are relocations.

Tue Mar 5 06:01:52 PST 2019

grimar added inline comments.

================
Comment at: test/tools/llvm-objcopy/ELF/compress-debug-sections-relocations.test:1
+# REQUIRES: zlib
+
----------------
jhenderson wrote:
> Could you reuse the Inputs/compress-debug-sections.yaml input file? Does that cover enough here, if updated with an actual symbol (see the update to that file in D58510)? I vaguely remember requesting that the relocation existed, so that it would test relocation behaviour, but it turns out the yaml was not quite correct.
Done.

================
Comment at: tools/llvm-objcopy/ELF/Object.h:599
   void markSymbols() override;
+  void replaceSections(const DenseMap<SectionBase *, SectionBase *> &FromTo);

----------------
jhenderson wrote:
> A more in-keeping with the llvm-objcopy Object design might be to provide a do-nothing virtual function for all sections "replaceSectionReferences" or similar, and then override it for section types that actually need some work, like relocation sections.
> 
> In particular, I could imagine some issues with things like group sections referencing debug data or similar.
Reasonable. Done.

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

https://reviews.llvm.org/D58960