[llvm] r312582 - Revert "[Decompression] Fail gracefully when out of memory"
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 8 13:11:49 PDT 2017
On Fri, Sep 8, 2017 at 10:45 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Fri, Sep 8, 2017 at 10:40 AM Kostya Serebryany <kcc at google.com> wrote:
>
>> On Fri, Sep 8, 2017 at 10:11 AM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Fri, Sep 8, 2017 at 10:06 AM Kostya Serebryany <kcc at google.com>
>>> wrote:
>>>
>>>> On Fri, Sep 8, 2017 at 9:36 AM, David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>> On Thu, Sep 7, 2017 at 6:38 PM Kostya Serebryany <kcc at google.com>
>>>>> wrote:
>>>>>
>>>>>> On Thu, Sep 7, 2017 at 9:14 AM, David Blaikie <dblaikie at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> maybe this is unreasonable, but could libFuzzer/asan/something mock
>>>>>>> out malloc & have it fail for large allocations
>>>>>>>
>>>>>>
>>>>>> Depending on your definition of 'fail'.
>>>>>> libFuzzer has a flag -rss_limit_mb (default: 2048) that will case the
>>>>>> process to crash (i.e. report a bug) on
>>>>>> a) any single allocation exceeding the limit (yes, libFuzzer
>>>>>> intercepts malloc) or
>>>>>> b) if the total rss exceeds the limit.
>>>>>>
>>>>>> asan has another flag (allocator_may_return_null, off by default)
>>>>>> that will cause malloc to return 0 on large allocations instead of
>>>>>> crashing.
>>>>>>
>>>>>
>>>>> That ^ seems like a good thing/approach.
>>>>>
>>>>
>>>>
>>>> Not sure I understand you.
>>>> Is allocator_may_return_null=0 a good approach or
>>>> allocator_may_return_null=1?
>>>>
>>>
>>> allocator_may_return_null=1, I think. But I could be wrong/no doubt
>>> other folks have other ideas.
>>>
>>
>> The default for asan is =0 (i.e. crash on large allocs), but on
>> ClusterFuzz for Chrome
>> we use =1 because it allows to find some extra bugs (when malloc returns
>> 0, it goes unchecked and the code dereferences the null pointer using a
>> large index => oops).
>> However when combined with libFuzzer =1 does not work because of
>> -rss_limit_mb=2048 which does not allow malloc >= 2048Mb.
>> The problem is not when we try to allocate 1Tb, the problem is when we
>> try to allocate 3Gb or 6 times by 512Mb.
>>
>
> Not sure I'm following - it sounds like (originally) the issue is that the
> fuzzer might produce inputs that cause the code under test to allocate a
> lot of memory (either in one allocation, or several). So it seems to me
> like it'd be helpful/good if the fuzzer/asan/something (if it's easy)
> placed an artificial bound on total amount of memory that can be allocated
> (having the malloc return 0 beyond that). Maybe that's not practical, I'm
> not sure.
>
It may or may not be practical depending on the project specifics.
I would like to be in the world where this is never practical.
>
>
>>
>>
>>
>>>
>>>
>>>>
>>>>
>>>>
>>>>
>>>>> (though we're sort of trained that checking for allocation failure is
>>>>> almost pointless because modern systems don't actually allocate at the time
>>>>> & wait to page fault the memory in (I'm sure I'm using the terminology
>>>>> incorrectly, but hopefully makes enough sense) - but seems like a possible
>>>>> necessary/useful tradeoff for fuzzing rather than every program needing to
>>>>> implement/enforce a fixed memory bound)
>>>>>
>>>>>
>>>>>> By default asan crashes on HUGE allocations (e.g. 1Tb) but doesn't
>>>>>> fail on anything it can actually allocate.
>>>>>>
>>>>>> --kcc
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> rather than having applications need to support such a bound
>>>>>>> themselves?
>>>>>>>
>>>>>>> On Thu, Sep 7, 2017 at 9:10 AM Kostya Serebryany <kcc at google.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> 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-invalid-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/
>>>>>>>>>>> Decompressor.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-invalid-size.elf-x86-64
>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>>>>>>>>>>> DebugInfo/Inputs/dwarfdump-decompression-invalid-size.
>>>>>>>>>>> elf-x86-64?rev=312581&view=auto
>>>>>>>>>>> ============================================================
>>>>>>>>>>> ==================
>>>>>>>>>>> Binary files llvm/trunk/test/DebugInfo/Inputs/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-size.test
>>>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>>>>>>>>>>> DebugInfo/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/20170908/24c4e656/attachment-0001.html>
More information about the llvm-commits
mailing list