[PATCH] D39207: [llvm-objcopy] Add support for dwarf fission

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: test/tools/llvm-objcopy/drawf-fission.test:6
+# RUN: llvm-readobj -sections %t2 | FileCheck %s -check-prefix=STRIP
+# RUN: diff -rs %t %t3 | FileCheck %s -check-prefix=DIFFEXTRACT
+# RUN: diff -rs %t2 %t4 | FileCheck %s -check-prefix=DIFFSTRIP
----------------
jhenderson wrote:
> I'm not sure you need the -rs here? Also, do you really need to check the output? Doesn't diff exit with a non-zero exit code if the files differ?
For some reason, until this comment, I was under the impression that llvm-lit and FileCheck had a special relationship and that if you wanted a test to fail the failure had to come from FileCheck but thinking about it that makes no sense. I prefer doing things this way much better.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:71
+    GSplitDwarf("gsplit-dwarf",
+                cl::desc("equivliant to strip-dwo and then only-keep-dwo on "
+                         "the gsplit-dwaf file"));
----------------
jhenderson wrote:
> What is "only-keep-dwo"? 
I had the option wrong to begin with and this is a remnant of that mistake. I meant "extract-dwo"


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:87-90
+  // We can't remove the note section because we need to match the .dwo file
+  // up with the binary.
+  if (Sec.Type == SHT_NOTE)
+    return false;
----------------
jhenderson wrote:
> Why do we need this? According to the DWARF 5 appendix, there are tags in the debug information that allow matching the two up. It certainly doesn't mention anything about the SHT_NOTE sections. Note that there are many different kinds of SHT_NOTE sections as well, and not just one single one.
Yeah I think I mislead myself on that one reading into this further. Nothing even seems to produce a SHT_NOTE section. I'll remove that.


Repository:
  rL LLVM

https://reviews.llvm.org/D39207