[PATCH] D20470: [llvm-dwarfdump] - Teach dwarfdump to decompress debug sections in zlib style.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Sun May 22 11:20:55 PDT 2016


On Sat, May 21, 2016 at 2:16 AM, George Rimar via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> grimar added inline comments.
>
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:626
> @@ +625,3 @@
> +  if (Is64Bit)
> +    Extractor.getUnsigned(&Offset, sizeof(Elf64_Word));
> +
> ----------------
> dblaikie wrote:
> > You could just increment Offset (by sizeof(Elf64_Word)) here, I think?
> Right.
>
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:681-685
> @@ -639,10 +680,7 @@
> +    if (ZLibStyleCompressed || name.startswith("zdebug_")) {
> +      SmallString<32> Out;
> +      if (!tryDecompress(name, data, Out, ZLibStyleCompressed,
> IsLittleEndian,
> +                         AddressSize == 8))
>          continue;
> -      UncompressedSections.resize(UncompressedSections.size() + 1);
> -      if (zlib::uncompress(data, UncompressedSections.back(),
> OriginalSize) !=
> -          zlib::StatusOK) {
> -        UncompressedSections.pop_back();
> -        continue;
> -      }
> -      // Make data point to uncompressed section contents and save its
> contents.
> -      name = name.substr(1);
> +      UncompressedSections.emplace_back(Out);
>        data = UncompressedSections.back();
> ----------------
> dblaikie wrote:
> > This involves making extra copies of the data. Is there a reason you
> went with this over the original code that decompressed straight into the
> UncompressedSections?
> My point was to avoid possible calls of pop_back right after resize if
> decompression fails.
> That should be rare case (I think decompression fail is infrequent, except
> cases of disabled zlib (now check for that is inside tryDecompress)), but
> code itself looks bit overcomplicated.
>
> I just forgot to use std::move() to avoid extras when rewrote. Fixed.
> There should be no more extra heap allocations here.
>

Except this defeats/comes at a cost of the small vector optimization -
std::move on a small vector in small mode is the same as a copy.

Sorry, perhaps I should've commented the original code to explain/justify
it better.

- David


>
>
> http://reviews.llvm.org/D20470
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160522/ca4a0e11/attachment.html>


More information about the llvm-commits mailing list