[PATCH] D20211: [ELF] - Support of compressed debug sections creation(draft).

[H.J. Lu via llvm-commits]

On Fri, May 13, 2016 at 4:35 AM, George Rimar via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> grimar added inline comments.
>
> ================
> Comment at: ELF/OutputSections.cpp:842
> @@ +841,3 @@
> +    // renamed to start with.zdebug to identify the use of compression.
> +    StringSaver Saver(ScriptConfig->Alloc);
> +    this->Name = Saver.save(Twine(".z") + Name.drop_front(1));
> ----------------
> ruiu wrote:
>> Please don't use an unrelated object's members.
> Yep, I know, that is just because it was a draft patch.
> I was thinking about it when did that. What is the good way ?
> Actually I do not like how it is done now: we have lots of StringSaver instances in many places in lld.
> Are there reasons for that instead of having one StringSaver for all with single Alloc and just global reference to it ?
>
> ================
> Comment at: ELF/OutputSections.cpp:869-874
> @@ +868,8 @@
> +
> +  // If not successfully compressed or total size is greater than
> +  // size before applying compression, then cancel compressed output.
> +  size_t TotalSize = Compressed.size() + getCompressionHeaderSize<ELFT>();
> +  Compress = Success == zlib::Status::StatusOK && TotalSize < getSize();
> +  if (!Compress)
> +    return;
> +
> ----------------
> ruiu wrote:
>> I think we don't have to care about this edge case. Unless you are compressing random bits, it usually shrink.
> I would leave it as is. That is mentioned in docs: "The .debug_line and .debug_pubnames sections would be larger compressed than in their original uncompressed form, and have therefore been left uncompressed." (https://docs.oracle.com/cd/E53394_01/html/E54813/section_compression.html#scrolltoc) and done both by gold/bfd and very transparent in implementation.
> In addition I think that if feature is used then motivation and expected result is to consume less space, I do not think it is acceptable to consume more than without it, it is confusing and opposite to its description.
>
> ================
> Comment at: ELF/OutputSections.cpp:1014-1025
> @@ +1013,14 @@
> +  const endianness E = ELFT::TargetEndianness;
> +  if (Config->CompressDebugSections == "zlib-gnu") {
> +    // 4 bytes magic + size of uncompressed data in big endian.
> +    memcpy(Buf, "ZLIB", 4);
> +    write64be(Buf + 4, UncompressedSize);
> +  } else {
> +    // "zlib-gabi" or "zlib".
> +    write32<E>(Buf, ELFCOMPRESS_ZLIB);
> +    if (ELFT::Is64Bits)
> +      write64<E>(Buf + 2 * sizeof(Elf64_Word), UncompressedSize);
> +    else
> +      write32<E>(Buf + sizeof(Elf32_Word), UncompressedSize);
> +  }
> +
> ----------------
> ruiu wrote:
>> Why we have two types of headers? If one is obsolete, we probably don't want to implement it.
> There are 2 different approaches used (https://docs.oracle.com/cd/E53394_01/html/E54813/section_compression.html):
> 1) gnu-style renames sections to ".z*[NAME]" and do not uses SHF_COMPRESSED.
> 2) zlib style. Sections have SHF_COMPRESSED flag and are not renamed.
>
> "Unless there is a specific requirement to use the zlib-gnu style, the more general default zlib style is recommended." says https://docs.oracle.com/cd/E53394_01/html/E54813/man-cds.html, but it is not mentioned if that depricated or something.
> Not supporting of zlib-gnu style would make implementation a bit cleaner.
>
> It is not hard to support both though.
>

Support both for input is nice.  I don't think ldd needs to support
zlib-gnu for output, which has been deprecated in ld and gold.

-- 
H.J.