[llvm] r270466 - llvm-dwp: More error handling around invalid compressed sections

[David Blaikie via llvm-commits]

On Wed, May 25, 2016 at 11:59 AM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On Mon, May 23, 2016 at 10:59 AM, David Blaikie via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: dblaikie
>> Date: Mon May 23 12:59:17 2016
>> New Revision: 270466
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=270466&view=rev
>> Log:
>> llvm-dwp: More error handling around invalid compressed sections
>>
>> Added:
>>     llvm/trunk/test/tools/llvm-dwp/Inputs/empty_compressed_section.dwo
>>     llvm/trunk/test/tools/llvm-dwp/Inputs/invalid_compressed.dwo
>> Modified:
>>     llvm/trunk/test/tools/llvm-dwp/X86/compressfail.test
>>     llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
>>
>> Added: llvm/trunk/test/tools/llvm-dwp/Inputs/empty_compressed_section.dwo
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-dwp/Inputs/empty_compressed_section.dwo?rev=270466&view=auto
>>
>> ==============================================================================
>> Binary files
>> llvm/trunk/test/tools/llvm-dwp/Inputs/empty_compressed_section.dwo (added)
>> and llvm/trunk/test/tools/llvm-dwp/Inputs/empty_compressed_section.dwo Mon
>> May 23 12:59:17 2016 differ
>>
>> Added: llvm/trunk/test/tools/llvm-dwp/Inputs/invalid_compressed.dwo
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-dwp/Inputs/invalid_compressed.dwo?rev=270466&view=auto
>>
>> ==============================================================================
>> Binary files llvm/trunk/test/tools/llvm-dwp/Inputs/invalid_compressed.dwo
>> (added) and llvm/trunk/test/tools/llvm-dwp/Inputs/invalid_compressed.dwo
>> Mon May 23 12:59:17 2016 differ
>>
>> Modified: llvm/trunk/test/tools/llvm-dwp/X86/compressfail.test
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-dwp/X86/compressfail.test?rev=270466&r1=270465&r2=270466&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/tools/llvm-dwp/X86/compressfail.test (original)
>> +++ llvm/trunk/test/tools/llvm-dwp/X86/compressfail.test Mon May 23
>> 12:59:17 2016
>> @@ -1,5 +1,7 @@
>>  RUN: not llvm-dwp %p/../Inputs/compressfail/a.dwo -o %t 2>&1 | FileCheck
>> %s
>> +RUN: not llvm-dwp %p/../Inputs/empty_compressed_section.dwo -o %t 2>&1 |
>> FileCheck %s
>> +RUN: not llvm-dwp %p/../Inputs/invalid_compressed.dwo -o %t 2>&1 |
>> FileCheck %s
>>
>>  REQUIRES: zlib
>>
>> -CHECK: error: failure while decompressing compressed section:
>> 'zdebug_info.dwo'
>> +CHECK: error: failure while decompressing compressed section:
>> 'zdebug_{{.*}}.dwo'
>>
>> Modified: llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp?rev=270466&r1=270465&r2=270466&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp (original)
>> +++ llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp Mon May 23 12:59:17 2016
>> @@ -432,20 +432,17 @@ static Error write(MCStreamer &Out, Arra
>>          return errorCodeToError(Err);
>>
>>        if (Name.startswith("zdebug_")) {
>> +        UncompressedSections.emplace_back();
>>          uint64_t OriginalSize;
>>          if (!zlib::isAvailable())
>>            return make_error<DWPError>("zlib not available");
>> -        if (!consumeCompressedDebugSectionHeader(Contents, OriginalSize))
>> +        if (!consumeCompressedDebugSectionHeader(Contents, OriginalSize)
>> ||
>> +            zlib::uncompress(Contents, UncompressedSections.back(),
>> +                             OriginalSize) != zlib::StatusOK)
>>            return make_error<DWPError>(
>>                ("failure while decompressing compressed section: '" +
>> Name +
>>                 "\'")
>>                    .str());
>> -        UncompressedSections.resize(UncompressedSections.size() + 1);
>> -        if (zlib::uncompress(Contents, UncompressedSections.back(),
>> -                             OriginalSize) != zlib::StatusOK) {
>> -          UncompressedSections.pop_back();
>> -          continue;
>> -        }
>>          Name = Name.substr(1);
>>          Contents = UncompressedSections.back();
>
>
> This use of UncompressedSections looks wrong. Either:
>
> 1) you only need storage for a single string, and can reuse that storage
> for each section you decompress, or
> 2) you are holding onto a StringRef referring to a SmallString vector
> element across that vector being resized
>
> From code inspection, it looks like option (1): you can replace the
> UncompressedSections vector with storage for a single uncompressed section.
>

Actually it's (2) - DWPStringPool.h holds const char* for any string in the
entire run for deduplication. So a const char* could point into an
uncompressed string in the first input file and still need to be there for
comparison/lookup when observing a string in the second/nth input file.

Committed a fix to use std::deque of SmallStrings (have to use SmallStrings
because that's what our zlib wrapper API requires) in r270797.

Thanks for the catch!

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160525/852cc6f3/attachment.html>