[PATCH] D51841: [llvm-objcopy] Dwarf decompression support.

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: test/tools/llvm-objcopy/compress-debug-sections-zlib-gnu.test:10
 # RUN: llvm-readobj -relocations -s %t-compressed.o | FileCheck %s --check-prefix=CHECK-FLAGS
+# RUN: llvm-objdump -s %t-decompressed.o -section=.debug_foo | FileCheck %s
 
----------------
jhenderson wrote:
> This still isn't a good enough test. All it does is show that we have a section named .debug_foo after decompression. It shows nothing about the properties of the section header or that the section has been decompressed. You need to check both (e.g. by comparing the contents of the section/section header before and after decompression). If you bring across the comparisons in compress-debug-sections.test, then that at least will cover the contents aspect.
Added checks for flags and content. Lemme know if these are sufficient. 


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:1023
+  if (Config.DecompressDebugSections && !zlib::isAvailable())
+    error("LLVM was not compiled with LLVM_ENABLE_ZLIB: can not decompress.");
+
----------------
jhenderson wrote:
> "can not" -> cannot
> 
> It feels to me like it would make sense to check both decompress and compress here, rather than in two completely different places, since they are closely related. I'd rather not rely on the library providing error messages, when we can error early and up-front.
I could do that in an NFC commit after this one. 


Repository:
  rL LLVM

https://reviews.llvm.org/D51841