[PATCH] D67090: [llvm-objcopy][llvm-strip] Support --only-keep-debug

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/only-keep-debug.test:28
+
+## Contents of SHT_NOTE and .debug* are kept.
+
----------------
grimar wrote:
> Perhaps not '.debug_*', but all non-alloc sections (looking at the logic implemented)?
> 
> What about adding non-alloc section that is not '.debug_*' to the test?
Thanks. I added a `.comment` to test it is not stripped.


================
Comment at: test/tools/llvm-objcopy/ELF/only-keep-debug.test:79
+Symbols:
+  - Name: foo
+ProgramHeaders:
----------------
grimar wrote:
> Do you need symbol?
This is to test --only-keep-debug does not strip .symtab

I guess it may be used by debuggers to help symbolize addresses, i.e. lib/DebugInfo/Symbolize* has logic to override DWARF results with .symtab. gdb and lldb may have similar behaviors but I'm not sure.

Added a comment.


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1935
+  uint32_t Index = 1;
+  for (auto &Sec : Obj.sections()) {
+    Sec.Index = Index++;
----------------
grimar wrote:
> Don't use auto?
Thanks. If I did D67143 first, I should have not written code like this..


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67090