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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

What I was saying was that you can compress multiple sections concurrently, right? I didn't say you should try to compress single stream of data using multiple threads. I meant, since we have multiple .debug_ sections, you could compress them separately.



================
Comment at: ELF/Driver.cpp:560
+      return false;
+    if (S == "zlib" || S == "zlib-gabi")
+      return zlib::isAvailable();
----------------
grimar wrote:
> ruiu wrote:
> > 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`.
> http://man7.org/linux/man-pages/man1/ld.1.html says:
> //--compress-debug-sections=zlib and
>  --compress-debug-sections=zlib-gabi compress DWARF debug sections with SHF_COMPRESSED from the ELF ABI.  The default behaviour varies depending upon the target involved and the configure options used to build the toolchain.//
> 
> https://docs.oracle.com/cd/E53394_01/html/E54813/man-cds.html says that there are 3 options: none, zlib, zlib-gnu.
> It does not mention what is zlib-gabi.
> 
> The only compression flag available now in generic ABI is ELFCOMPRESS_ZLIB:
> http://www.sco.com/developers/gabi/latest/ch4.sheader.html
> 
> So my conclusion from above:
> 1) Since oracle does not mention zlib-gabi, I implemented zlib and not zlib-gabi.
> 2) Since ld manual says both zlib/zlib-gabi compresses sections with SHF_COMPRESSED from ABI and only available type is ELFCOMPRESS_ZLIB, then zlib and zlib-gabi is the same.
When I give -compress-debug-sections=zlib and -compress-debug-sections=zlib-gabi, outputs from gold were actually different. But interestingly enough the difference wasn't meaningful. Gold seems to set garbage to ch_reserved members of compressed section headers. (Probably they forgot to initialize the header with zero before setting values to the members.)

So my conclusion is zlib-gabi and zlib are the same. But I don't see a reason to add two identical options at the moment. Please add just "zlib".


================
Comment at: ELF/OutputSections.cpp:72-76
+  if (!Config->CompressDebugSections || !Name.startswith(".debug"))
+    return;
+  // SHF_COMPRESSED can not be used in conjunction with SHF_ALLOC flag.
+  if (!(Flags & SHF_ALLOC))
+    this->Flags |= SHF_COMPRESSED;
----------------
The way this code is written is confusing. What you should state is that we want to compress .debug sections (which needless to say non-alloc sections).

  // If -compress-debug-sections is specified, we compress output debug sections.
  if (Config->CompressDebugSections && Name.startswith(".debug_") && !(Flags & SHF_ALLOC))
    this->Flags |= SHF_COMPRESSED;

However, this piece of code should probably be moved to finalize().


https://reviews.llvm.org/D31941