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