[llvm] r312582 - Revert "[Decompression] Fail gracefully when out of memory"

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 7 09:10:24 PDT 2017


On Thu, Sep 7, 2017 at 3:17 AM, George Rimar <grimar at accesssoftek.com>
wrote:

> Compressing/decompressing can be used for any sections in theory,
>
> but in practice used for .debug_* ones only I think (not sure though).
>
> Afaik LLVM does not support DWARF64 now.
>
> Given above we probably can limit decompressed size to 4gb and it be
> reasonable limit ?
>
>
> It might be reasonable as the default limit, but for fuzzing 4Gb is a bit
too expensive.
The default total rss limit in libFuzzer is 2Gb, and the same limit is
enforced on oss-fuzz.
So, there should be a way to enforce another limit (1Gb sounds best to me)


> Another point (already mentioned in different thread) is that
> install_bad_alloc_error_handler API looks completely dead then ?
> Since it looks not possible to use it currently without breaking ASan​.
>
> Best regards,
> George | Developer | Access Softek, Inc
> ------------------------------
> *От:* Kostya Serebryany <kcc at google.com>
> *Отправлено:* 7 сентября 2017 г. 3:26
> *Кому:* Jonas Devlieghere
> *Копия:* Vedant Kumar; LLVM Commits; George Rimar
> *Тема:* Re: [llvm] r312582 - Revert "[Decompression] Fail gracefully when
> out of memory"
>
>
>
> On Wed, Sep 6, 2017 at 2:38 PM, Jonas Devlieghere <jdevlieghere at apple.com>
> wrote:
>
>> Hi Kostya,
>>
>> I completely agree with you that this is far from an optimal solution!
>> However, I don’t really see an alternative here without a complete rewrite
>> of how we currently do decompression. For this particular case, how would
>> you decide whether the allocated chuck is too big?
>>
>
> It could be an arbitrary limit which can be redefined at compile-time or,
> better, with a run-time flag (and the default set to e.g. 1Gb?)
>
>
>> Any limit would be arbitrary before doing the actual extraction. This is
>> a pretty common problem with extraction
>>
>
> Oh yea. zip bombs they call it.
>
>
>> and I *think* libz has some kind of work around for it, though I’d have
>> to check to refresh my memory. We could probably do something similar in
>> the decompressor, but that’d be a pretty big change and this seemed like a
>> reasonable first step as opposed to just crashing.
>>
>
> This change doesn't really fix the problem though.
> Yes, it might help if the requested allocation is e.g. 42Tb.
> But what if the requested allocation is 3Gb? LibFuzzer will still die with
> OOM (default rss_limit_mb=2048).
> What if the requested allocation is 1.99Gb? libFuzzer will dies due to OOM
> again, just because the process will consume 2Gb
>
>
>>
>> Here's the original differential https://reviews.llvm.org/D37447
>>
>> Jonas
>>
>>
>> On Sep 6, 2017, at 9:37 PM, Kostya Serebryany <kcc at google.com> wrote:
>>
>> I missed the original commit but I'm glad it's reverted.
>> I think it's a really bad practice to allocate huge chunks of RAM
>> (petabytes?) and rely on the new handler to bark.
>> Instead, we should simply not try to allocate large chunks. In most cases
>> if the compiler-related too wants to allocate many Gb of RAM at once it's
>> an indication of a logical bug.
>>
>> On Tue, Sep 5, 2017 at 3:04 PM, Vedant Kumar via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: vedantk
>>> Date: Tue Sep  5 15:04:00 2017
>>> New Revision: 312582
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=312582&view=rev
>>> Log:
>>> Revert "[Decompression] Fail gracefully when out of memory"
>>>
>>> This reverts commit r312526.
>>>
>>> Revert "Fix test/DebugInfo/dwarfdump-decompression-invalid-size.test"
>>>
>>> This reverts commit r312527.
>>>
>>> It causes an ASan failure:
>>> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/4150
>>>
>>> Removed:
>>>     llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-inv
>>> alid-size.elf-x86-64
>>>     llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test
>>> Modified:
>>>     llvm/trunk/include/llvm/Object/Decompressor.h
>>>     llvm/trunk/lib/Object/Decompressor.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/Object/Decompressor.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>>> Object/Decompressor.h?rev=312582&r1=312581&r2=312582&view=diff
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/include/llvm/Object/Decompressor.h (original)
>>> +++ llvm/trunk/include/llvm/Object/Decompressor.h Tue Sep  5 15:04:00
>>> 2017
>>> @@ -13,7 +13,6 @@
>>>  #include "llvm/ADT/SmallString.h"
>>>  #include "llvm/ADT/StringRef.h"
>>>  #include "llvm/Object/ObjectFile.h"
>>> -#include "llvm/Support/ErrorHandling.h"
>>>
>>>  namespace llvm {
>>>  namespace object {
>>> @@ -32,9 +31,7 @@ public:
>>>    /// @brief Resize the buffer and uncompress section data into it.
>>>    /// @param Out         Destination buffer.
>>>    template <class T> Error resizeAndDecompress(T &Out) {
>>> -    install_bad_alloc_error_handler(outOfMemoryHandler, this);
>>>      Out.resize(DecompressedSize);
>>> -    remove_bad_alloc_error_handler();
>>>      return decompress({Out.data(), (size_t)DecompressedSize});
>>>    }
>>>
>>> @@ -55,14 +52,11 @@ public:
>>>    static bool isGnuStyle(StringRef Name);
>>>
>>>  private:
>>> -  static void outOfMemoryHandler(void *Data, const std::string
>>> &Message, bool);
>>> -
>>> -  Decompressor(StringRef Name, StringRef Data);
>>> +  Decompressor(StringRef Data);
>>>
>>>    Error consumeCompressedGnuHeader();
>>>    Error consumeCompressedZLibHeader(bool Is64Bit, bool IsLittleEndian);
>>>
>>> -  StringRef SectionName;
>>>    StringRef SectionData;
>>>    uint64_t DecompressedSize;
>>>  };
>>>
>>> Modified: llvm/trunk/lib/Object/Decompressor.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/De
>>> compressor.cpp?rev=312582&r1=312581&r2=312582&view=diff
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/lib/Object/Decompressor.cpp (original)
>>> +++ llvm/trunk/lib/Object/Decompressor.cpp Tue Sep  5 15:04:00 2017
>>> @@ -23,7 +23,7 @@ Expected<Decompressor> Decompressor::cre
>>>    if (!zlib::isAvailable())
>>>      return createError("zlib is not available");
>>>
>>> -  Decompressor D(Name, Data);
>>> +  Decompressor D(Data);
>>>    Error Err = isGnuStyle(Name) ? D.consumeCompressedGnuHeader()
>>>                                 : D.consumeCompressedZLibHeader(Is64Bit,
>>> IsLE);
>>>    if (Err)
>>> @@ -31,8 +31,8 @@ Expected<Decompressor> Decompressor::cre
>>>    return D;
>>>  }
>>>
>>> -Decompressor::Decompressor(StringRef Name, StringRef Data)
>>> -    : SectionName(Name), SectionData(Data), DecompressedSize(0) {}
>>> +Decompressor::Decompressor(StringRef Data)
>>> +    : SectionData(Data), DecompressedSize(0) {}
>>>
>>>  Error Decompressor::consumeCompressedGnuHeader() {
>>>    if (!SectionData.startswith("ZLIB"))
>>> @@ -92,11 +92,3 @@ Error Decompressor::decompress(MutableAr
>>>    size_t Size = Buffer.size();
>>>    return zlib::uncompress(SectionData, Buffer.data(), Size);
>>>  }
>>> -
>>> -void Decompressor::outOfMemoryHandler(void *Data, const std::string
>>> &Message,
>>> -                                      bool) {
>>> -  const auto *D = static_cast<const Decompressor *>(Data);
>>> -  report_fatal_error("decompression of '" + Twine(D->SectionName) +
>>> -                     "' failed: unable to allocate " +
>>> -                     Twine(D->DecompressedSize) + " bytes.");
>>> -}
>>>
>>> Removed: llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-inv
>>> alid-size.elf-x86-64
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInf
>>> o/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64?rev
>>> =312581&view=auto
>>> ============================================================
>>> ==================
>>> Binary files llvm/trunk/test/DebugInfo/Inpu
>>> ts/dwarfdump-decompression-invalid-size.elf-x86-64 (original) and
>>> llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64
>>> (removed) differ
>>>
>>> Removed: llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-si
>>> ze.test
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInf
>>> o/dwarfdump-decompression-invalid-size.test?rev=312581&view=auto
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test
>>> (original)
>>> +++ llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test
>>> (removed)
>>> @@ -1,15 +0,0 @@
>>> -REQUIRES: zlib
>>> -
>>> -// dwarfdump-decompression-invalid-size.elf-x86-64 is prepared using
>>> following
>>> -// source code and invocation:
>>> -// test.cpp:
>>> -// int main() { return 0; }
>>> -//
>>> -// gcc test.cpp -o out -g -Wl,--compress-debug-sections,zlib
>>> -//
>>> -// After that result object was modified manually. Decompressed size of
>>> -// .debug_frame section was changed to 0xffffffffffffffff in compression
>>> -// header.
>>> -RUN: not llvm-dwarfdump %p/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64
>>> 2>&1 | FileCheck %s
>>> -
>>> -CHECK: decompression of '.debug_frame' failed: unable to allocate
>>> 18446744073709551615 bytes.
>>>
>>>
>>> _______________________________________________
>>> 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/20170907/4597a029/attachment.html>


More information about the llvm-commits mailing list