[llvm] r312582 - Revert "[Decompression] Fail gracefully when out of memory"
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 8 10:40:42 PDT 2017
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.
>
>
>>
>>
>>
>>
>>> (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/a691d49e/attachment.html>
More information about the llvm-commits
mailing list