From 1d05591fbb1017db20e369e54a3a43a49eb56a57 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Fri, 13 Nov 2020 19:07:20 +0000 Subject: [PATCH] [vm] Produce clearer error messages when operator new fails. TEST=bots Bug: https://github.com/dart-lang/sdk/issues/43642 Bug: https://github.com/dart-lang/sdk/issues/43862 Change-Id: I598a26a56762c141a1aef972f7d32f2c9594b244 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171802 Commit-Queue: Ryan Macnak Reviewed-by: Alexander Aprelev Reviewed-by: Martin Kustermann --- runtime/platform/allocation.h | 31 +++++++++++++++++++++++++++++++ runtime/platform/growable_array.h | 9 ++++----- runtime/platform/hashmap.cc | 3 --- runtime/vm/allocation.h | 4 ++-- runtime/vm/deopt_instructions.h | 2 +- runtime/vm/handles.h | 2 +- runtime/vm/handles_impl.h | 6 ------ runtime/vm/hash_map.h | 8 +++++--- runtime/vm/heap/pointer_block.h | 2 +- runtime/vm/port_set.h | 3 ++- runtime/vm/timeline.h | 4 ++-- 11 files changed, 49 insertions(+), 25 deletions(-) diff --git a/runtime/platform/allocation.h b/runtime/platform/allocation.h index 2e3b76b172e..e2d7dede8c4 100644 --- a/runtime/platform/allocation.h +++ b/runtime/platform/allocation.h @@ -5,6 +5,7 @@ #ifndef RUNTIME_PLATFORM_ALLOCATION_H_ #define RUNTIME_PLATFORM_ALLOCATION_H_ +#include "platform/address_sanitizer.h" #include "platform/assert.h" namespace dart { @@ -31,6 +32,36 @@ class AllStatic { DISALLOW_IMPLICIT_CONSTRUCTORS(AllStatic); }; +class MallocAllocated { + public: + MallocAllocated() {} + + // Intercept operator new to produce clearer error messages when we run out + // of memory. Don't do this when running under ASAN so it can continue to + // check malloc/new/new[] are paired with free/delete/delete[] respectively. +#if !defined(USING_ADDRESS_SANITIZER) + void* operator new(size_t size) { + void* result = ::malloc(size); + if (result == nullptr) { + OUT_OF_MEMORY(); + } + return result; + } + + void* operator new[](size_t size) { + void* result = ::malloc(size); + if (result == nullptr) { + OUT_OF_MEMORY(); + } + return result; + } + + void operator delete(void* pointer) { ::free(pointer); } + + void operator delete[](void* pointer) { ::free(pointer); } +#endif +}; + } // namespace dart #endif // RUNTIME_PLATFORM_ALLOCATION_H_ diff --git a/runtime/platform/growable_array.h b/runtime/platform/growable_array.h index b604ea471b5..20d5f1f135e 100644 --- a/runtime/platform/growable_array.h +++ b/runtime/platform/growable_array.h @@ -254,14 +254,13 @@ class Malloc : public AllStatic { } }; -class EmptyBase {}; - template -class MallocGrowableArray : public BaseGrowableArray { +class MallocGrowableArray + : public BaseGrowableArray { public: explicit MallocGrowableArray(intptr_t initial_capacity) - : BaseGrowableArray(initial_capacity, NULL) {} - MallocGrowableArray() : BaseGrowableArray(NULL) {} + : BaseGrowableArray(initial_capacity, NULL) {} + MallocGrowableArray() : BaseGrowableArray(NULL) {} }; } // namespace dart diff --git a/runtime/platform/hashmap.cc b/runtime/platform/hashmap.cc index 88c521e13ba..730b94ed375 100644 --- a/runtime/platform/hashmap.cc +++ b/runtime/platform/hashmap.cc @@ -157,9 +157,6 @@ SimpleHashMap::Entry* SimpleHashMap::Probe(void* key, uint32_t hash) { void SimpleHashMap::Initialize(uint32_t capacity) { ASSERT(dart::Utils::IsPowerOfTwo(capacity)); map_ = new Entry[capacity]; - if (map_ == NULL) { - OUT_OF_MEMORY(); - } capacity_ = capacity; occupancy_ = 0; } diff --git a/runtime/vm/allocation.h b/runtime/vm/allocation.h index 834b28bb9dd..fc4be724ad7 100644 --- a/runtime/vm/allocation.h +++ b/runtime/vm/allocation.h @@ -54,10 +54,10 @@ class ZoneAllocated { ZoneAllocated() {} // Implicitly allocate the object in the current zone. - void* operator new(uword size); + void* operator new(size_t size); // Allocate the object in the given zone, which must be the current zone. - void* operator new(uword size, Zone* zone); + void* operator new(size_t size, Zone* zone); // Ideally, the delete operator should be protected instead of // public, but unfortunately the compiler sometimes synthesizes diff --git a/runtime/vm/deopt_instructions.h b/runtime/vm/deopt_instructions.h index 0c40fa905e7..dd896ccd077 100644 --- a/runtime/vm/deopt_instructions.h +++ b/runtime/vm/deopt_instructions.h @@ -27,7 +27,7 @@ class TimelineEvent; // Holds all data relevant for execution of deoptimization instructions. // Structure is allocated in C-heap. -class DeoptContext { +class DeoptContext : public MallocAllocated { public: enum DestFrameOptions { kDestIsOriginalFrame, // Replace the original frame with deopt frame. diff --git a/runtime/vm/handles.h b/runtime/vm/handles.h index 28ec9cc6d0d..ca6b5f4c147 100644 --- a/runtime/vm/handles.h +++ b/runtime/vm/handles.h @@ -116,7 +116,7 @@ class Handles { // is allocated from the chunk until we run out space in the chunk, // at this point another chunk is allocated. These chunks are chained // together. - class HandlesBlock { + class HandlesBlock : public MallocAllocated { public: explicit HandlesBlock(HandlesBlock* next) : next_handle_slot_(0), next_block_(next) {} diff --git a/runtime/vm/handles_impl.h b/runtime/vm/handles_impl.h index f4033dc91b8..0697a138191 100644 --- a/runtime/vm/handles_impl.h +++ b/runtime/vm/handles_impl.h @@ -157,9 +157,6 @@ void Handles:: } if (scoped_blocks_->next_block() == NULL) { HandlesBlock* block = new HandlesBlock(NULL); - if (block == NULL) { - OUT_OF_MEMORY(); - } scoped_blocks_->set_next_block(block); } scoped_blocks_ = scoped_blocks_->next_block(); @@ -207,9 +204,6 @@ void Handles:: CountScopedHandles()); } zone_blocks_ = new HandlesBlock(zone_blocks_); - if (zone_blocks_ == NULL) { - OUT_OF_MEMORY(); - } } #if defined(DEBUG) diff --git a/runtime/vm/hash_map.h b/runtime/vm/hash_map.h index 409ab896f9e..88d58ba66e3 100644 --- a/runtime/vm/hash_map.h +++ b/runtime/vm/hash_map.h @@ -423,15 +423,17 @@ class DirectChainedHashMap template class MallocDirectChainedHashMap - : public BaseDirectChainedHashMap { + : public BaseDirectChainedHashMap { public: MallocDirectChainedHashMap() - : BaseDirectChainedHashMap(NULL) {} + : BaseDirectChainedHashMap(NULL) { + } // The only use of the copy constructor seems to be in hash_map_test.cc. // Not disallowing it for now just in case there are other users. MallocDirectChainedHashMap(const MallocDirectChainedHashMap& other) - : BaseDirectChainedHashMap(other) {} + : BaseDirectChainedHashMap( + other) {} private: void operator=(const MallocDirectChainedHashMap& other) = delete; diff --git a/runtime/vm/heap/pointer_block.h b/runtime/vm/heap/pointer_block.h index 4a40cd1ad70..5e7dde89d25 100644 --- a/runtime/vm/heap/pointer_block.h +++ b/runtime/vm/heap/pointer_block.h @@ -18,7 +18,7 @@ class ObjectPointerVisitor; // A set of ObjectPtr. Must be emptied before destruction (using Pop/Reset). template -class PointerBlock { +class PointerBlock : public MallocAllocated { public: enum { kSize = Size }; diff --git a/runtime/vm/port_set.h b/runtime/vm/port_set.h index 97a2c9a8276..a632f50c5d3 100644 --- a/runtime/vm/port_set.h +++ b/runtime/vm/port_set.h @@ -7,6 +7,7 @@ #include "include/dart_api.h" +#include "platform/allocation.h" #include "platform/globals.h" #include "platform/utils.h" @@ -18,7 +19,7 @@ class PortSet { static constexpr Dart_Port kFreePort = static_cast(0); static constexpr Dart_Port kDeletedPort = static_cast(3); - struct Entry { + struct Entry : public MallocAllocated { Entry() : port(kFreePort) {} // Free entries have set this to 0. diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h index d5be399182a..38e739ae6f4 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -574,7 +574,7 @@ class TimelineBeginEndScope : public TimelineEventScope { }; // A block of |TimelineEvent|s. Not thread safe. -class TimelineEventBlock { +class TimelineEventBlock : public MallocAllocated { public: static const intptr_t kBlockSize = 64; @@ -707,7 +707,7 @@ class IsolateTimelineEventFilter : public TimelineEventFilter { }; // Recorder of |TimelineEvent|s. -class TimelineEventRecorder { +class TimelineEventRecorder : public MallocAllocated { public: TimelineEventRecorder(); virtual ~TimelineEventRecorder() {}