[PATCH] D52917: Avoid unnecessary buffer allocation and memcpy for compressed sections.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/InputSection.cpp:268
+  RawData = RawData.slice(sizeof(*Hdr));
 }
 
----------------
grimar wrote:
> What do you think about still using the `Decompressor` here for parsing the
> headers and checking `zlib` availability?
> Like below.
> 
> ```
>   Decompressor Dec = check(Decompressor::create(Name, toStringRef(RawData),
>                                                 Config->IsLE, Config->Is64));
>   UncompressedSize = Dec.getDecompressedSize();
>   if (Name.startswith(".zdebug")) {
>     RawData = RawData.slice(12);
>     Name = Saver.save("." + Name.substr(2));
>     return;
>   }
>   assert(Flags & SHF_COMPRESSED);
>   Flags &= ~(uint64_t)SHF_COMPRESSED;
> 
>   // New-style 64-bit header
>   if (Config->Is64)
>     RawData = RawData.slice(sizeof(Elf64_Chdr));
>   else
>     RawData = RawData.slice(sizeof(Elf32_Chdr));
> ```
> 
> or, if we add `getCompressedData` method to `Decompressor` class, it can be:
> 
> ```
>   Decompressor Dec = check(Decompressor::create(Name, toStringRef(RawData),
>                                                 Config->IsLE, Config->Is64));
>   UncompressedSize = Dec.getDecompressedSize();
>   RawData = Dec.getCompressedData();
> 
>   if (Name.startswith(".zdebug")) {
>     Name = Saver.save("." + Name.substr(2));
>     return;
>   }
> 
>   assert(Flags & SHF_COMPRESSED);
>   Flags &= ~(uint64_t)SHF_COMPRESSED;
> ```
I found that Decompressor hides too many details and is not easy to use. I feel more comfortable to do this explicitly in this file.


================
Comment at: lld/test/ELF/strip-debug.s:3
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: ld.lld %t -o %t2 --strip-debug
 # RUN: llvm-readobj -sections %t2 | FileCheck %s
----------------
grimar wrote:
> If you remove `--strip-debug`, this will crash the linker because of
> ` llvm_unreachable("zlib::uncompress is unavailable");` on windows, where zlib is not available.
> 
> You may want to check `zlib::isAvailable()` or use `Decompressor`.
Done.


https://reviews.llvm.org/D52917