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