[PATCH] D31941: [ELF] - Implemented --compress-debug-sections option.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Driver.cpp:560
+      return false;
+    if (S == "zlib" || S == "zlib-gabi")
+      return zlib::isAvailable();
----------------
ruiu wrote:
> What is `-gabi`?
I don't think `zlib` and `zlib-gabi` are the same. What you implemented in this patch is `zlib-gabi`, so don't handle `zlib` as a synonym for `zlib-gabi`.


================
Comment at: ELF/OutputSections.cpp:309-315
+  // If we have compressed output section here, we should write
+  // header and data that we already prepared in finilize().
+  // Sections compression header format is explaned here:
+  // https://docs.oracle.com/cd/E53394_01/html/E54813/section_compression.html
+  if (!CompressedData.empty()) {
+    writeCompressed(Buf, CompressedData, DecompressedSize, Alignment);
+    return;
----------------
You can do this in finalize(), right?


================
Comment at: test/ELF/compress-debug-sections.s:13-15
+# ZLIBCONTENT-NEXT:  0000 01000000 00000000 38000000 00000000
+# ZLIBCONTENT-NEXT:  0010 01000000 00000000 789c6360 200f0000
+# ZLIBCONTENT-NEXT:  0020 00380001
----------------
Don't check for the section contents because `zlib` doesn't really guarantee the output.


================
Comment at: test/ELF/compress-debug-sections.s:27
+# ZLIBFLAGS-NEXT:    Offset:
+# ZLIBFLAGS-NEXT:    Size: 36
+
----------------
Ditto


================
Comment at: test/ELF/compress-debug-sections.s:29-48
+# RUN: ld.lld %t.o -o %t3 --compress-debug-sections=none
+# RUN: llvm-objdump -s %t3 | FileCheck %s --check-prefix=CONTENT
+# RUN: llvm-readobj -s %t3 | FileCheck %s --check-prefix=FLAGS
+
+# CONTENT:      Contents of section .debug_str:
+# CONTENT-NEXT:  0000 41414141 41414141 41414141 41414141  AAAAAAAAAAAAAAAA
+# CONTENT-NEXT:  0010 41414141 41414141 41414100 42424242  AAAAAAAAAAA.BBBB
----------------
Why do you need this?


https://reviews.llvm.org/D31941