From 06c71d8a6f98f645ce370ad5c24546a3ce7d9848 Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Tue, 20 Feb 2018 05:41:26 +0000 Subject: [PATCH] Report fatal error in the case of out of memory This is the second part of recommit of r325224. The previous part was committed in r325426, which deals with C++ memory allocation. Solution for C memory allocation involved functions `llvm::malloc` and similar. This was a fragile solution because it caused ambiguity errors in some cases. In this commit the new functions have names like `llvm::safe_malloc`. The relevant part of original comment is below, updated for new function names. Analysis of fails in the case of out of memory errors can be tricky on Windows. Such error emerges at the point where memory allocation function fails, but manifests itself when null pointer is used. These two points may be distant from each other. Besides, next runs may not exhibit allocation error. In some cases memory is allocated by a call to some of C allocation functions, malloc, calloc and realloc. They are used for interoperability with C code, when allocated object has variable size and when it is necessary to avoid call of constructors. In many calls the result is not checked for null pointer. To simplify checks, new functions are defined in the namespace 'llvm': `safe_malloc`, `safe_calloc` and `safe_realloc`. They behave as corresponding standard functions but produce fatal error if allocation fails. This change replaces the standard functions like 'malloc' in the cases when the result of the allocation function is not checked for null pointer. Finally, there are plain C code, that uses malloc and similar functions. If the result is not checked, assert statement is added. Differential Revision: https://reviews.llvm.org/D43010 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@325551 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ADT/BitVector.h | 7 +++-- include/llvm/ADT/SparseMultiSet.h | 2 +- include/llvm/ADT/SparseSet.h | 3 +- include/llvm/Support/Allocator.h | 28 +++++++++++++++++++ include/llvm/Support/OnDiskHashTable.h | 5 ++-- lib/CodeGen/InterferenceCache.cpp | 4 +-- lib/CodeGen/LiveIntervalUnion.cpp | 2 +- lib/CodeGen/RegisterPressure.cpp | 2 +- lib/ExecutionEngine/Interpreter/Execution.cpp | 2 +- lib/Object/Object.cpp | 2 +- lib/Support/RWMutex.cpp | 3 +- lib/Support/StringMap.cpp | 13 ++++----- lib/Support/Unix/Signals.inc | 2 +- lib/Support/Windows/RWMutex.inc | 4 +-- tools/llvm-c-test/attributes.c | 3 ++ tools/llvm-c-test/echo.cpp | 3 +- unittests/Support/AllocatorTest.cpp | 2 +- unittests/Support/ManagedStatic.cpp | 6 ++-- 18 files changed, 64 insertions(+), 29 deletions(-) diff --git a/include/llvm/ADT/BitVector.h b/include/llvm/ADT/BitVector.h index 99147fec4d4..124c2a8c86d 100644 --- a/include/llvm/ADT/BitVector.h +++ b/include/llvm/ADT/BitVector.h @@ -828,7 +828,8 @@ private: } MutableArrayRef allocate(size_t NumWords) { - BitWord *RawBits = (BitWord *)std::malloc(NumWords * sizeof(BitWord)); + BitWord *RawBits = static_cast( + safe_malloc(NumWords * sizeof(BitWord))); return MutableArrayRef(RawBits, NumWords); } @@ -867,8 +868,8 @@ private: void grow(unsigned NewSize) { size_t NewCapacity = std::max(NumBitWords(NewSize), Bits.size() * 2); assert(NewCapacity > 0 && "realloc-ing zero space"); - BitWord *NewBits = - (BitWord *)std::realloc(Bits.data(), NewCapacity * sizeof(BitWord)); + BitWord *NewBits = static_cast( + safe_realloc(Bits.data(), NewCapacity * sizeof(BitWord))); Bits = MutableArrayRef(NewBits, NewCapacity); clear_unused_bits(); } diff --git a/include/llvm/ADT/SparseMultiSet.h b/include/llvm/ADT/SparseMultiSet.h index c91e0d70f65..3c863762151 100644 --- a/include/llvm/ADT/SparseMultiSet.h +++ b/include/llvm/ADT/SparseMultiSet.h @@ -211,7 +211,7 @@ public: // The Sparse array doesn't actually need to be initialized, so malloc // would be enough here, but that will cause tools like valgrind to // complain about branching on uninitialized data. - Sparse = reinterpret_cast(calloc(U, sizeof(SparseT))); + Sparse = static_cast(safe_calloc(U, sizeof(SparseT))); Universe = U; } diff --git a/include/llvm/ADT/SparseSet.h b/include/llvm/ADT/SparseSet.h index 25ade883192..74cc6dab8c7 100644 --- a/include/llvm/ADT/SparseSet.h +++ b/include/llvm/ADT/SparseSet.h @@ -22,6 +22,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/Support/Allocator.h" #include #include #include @@ -163,7 +164,7 @@ public: // The Sparse array doesn't actually need to be initialized, so malloc // would be enough here, but that will cause tools like valgrind to // complain about branching on uninitialized data. - Sparse = reinterpret_cast(calloc(U, sizeof(SparseT))); + Sparse = static_cast(safe_calloc(U, sizeof(SparseT))); Universe = U; } diff --git a/include/llvm/Support/Allocator.h b/include/llvm/Support/Allocator.h index 7f9c39345b4..8ed4109c6fa 100644 --- a/include/llvm/Support/Allocator.h +++ b/include/llvm/Support/Allocator.h @@ -439,6 +439,34 @@ public: T *Allocate(size_t num = 1) { return Allocator.Allocate(num); } }; +/// \{ +/// Counterparts of allocation functions defined in namespace 'std', which crash +/// on allocation failure instead of returning null pointer. + +LLVM_ATTRIBUTE_RETURNS_NONNULL inline void *safe_malloc(size_t Sz) { + void *Result = std::malloc(Sz); + if (Result == nullptr) + report_bad_alloc_error("Allocation failed."); + return Result; +} + +LLVM_ATTRIBUTE_RETURNS_NONNULL inline void *safe_calloc(size_t Count, + size_t Sz) { + void *Result = std::calloc(Count, Sz); + if (Result == nullptr) + report_bad_alloc_error("Allocation failed."); + return Result; +} + +LLVM_ATTRIBUTE_RETURNS_NONNULL inline void *safe_realloc(void *Ptr, size_t Sz) { + void *Result = std::realloc(Ptr, Sz); + if (Result == nullptr) + report_bad_alloc_error("Allocation failed."); + return Result; +} + +/// \} + } // end namespace llvm template diff --git a/include/llvm/Support/OnDiskHashTable.h b/include/llvm/Support/OnDiskHashTable.h index e9c28daf03b..3ef004b9c7b 100644 --- a/include/llvm/Support/OnDiskHashTable.h +++ b/include/llvm/Support/OnDiskHashTable.h @@ -95,7 +95,8 @@ private: /// \brief Resize the hash table, moving the old entries into the new buckets. void resize(size_t NewSize) { - Bucket *NewBuckets = (Bucket *)std::calloc(NewSize, sizeof(Bucket)); + Bucket *NewBuckets = static_cast( + safe_calloc(NewSize, sizeof(Bucket))); // Populate NewBuckets with the old entries. for (size_t I = 0; I < NumBuckets; ++I) for (Item *E = Buckets[I].Head; E;) { @@ -226,7 +227,7 @@ public: NumBuckets = 64; // Note that we do not need to run the constructors of the individual // Bucket objects since 'calloc' returns bytes that are all 0. - Buckets = (Bucket *)std::calloc(NumBuckets, sizeof(Bucket)); + Buckets = static_cast(safe_calloc(NumBuckets, sizeof(Bucket))); } ~OnDiskChainedHashTableGenerator() { std::free(Buckets); } diff --git a/lib/CodeGen/InterferenceCache.cpp b/lib/CodeGen/InterferenceCache.cpp index 72227cc7bba..82f6e8d8e23 100644 --- a/lib/CodeGen/InterferenceCache.cpp +++ b/lib/CodeGen/InterferenceCache.cpp @@ -48,8 +48,8 @@ void InterferenceCache::reinitPhysRegEntries() { if (PhysRegEntriesCount == TRI->getNumRegs()) return; free(PhysRegEntries); PhysRegEntriesCount = TRI->getNumRegs(); - PhysRegEntries = (unsigned char*) - calloc(PhysRegEntriesCount, sizeof(unsigned char)); + PhysRegEntries = static_cast( + safe_calloc(PhysRegEntriesCount, sizeof(unsigned char))); } void InterferenceCache::init(MachineFunction *mf, diff --git a/lib/CodeGen/LiveIntervalUnion.cpp b/lib/CodeGen/LiveIntervalUnion.cpp index 3e742a6c2f2..36428e0335f 100644 --- a/lib/CodeGen/LiveIntervalUnion.cpp +++ b/lib/CodeGen/LiveIntervalUnion.cpp @@ -187,7 +187,7 @@ void LiveIntervalUnion::Array::init(LiveIntervalUnion::Allocator &Alloc, clear(); Size = NSize; LIUs = static_cast( - malloc(sizeof(LiveIntervalUnion)*NSize)); + safe_malloc(sizeof(LiveIntervalUnion)*NSize)); for (unsigned i = 0; i != Size; ++i) new(LIUs + i) LiveIntervalUnion(Alloc); } diff --git a/lib/CodeGen/RegisterPressure.cpp b/lib/CodeGen/RegisterPressure.cpp index bc1af1594c2..97e5851e025 100644 --- a/lib/CodeGen/RegisterPressure.cpp +++ b/lib/CodeGen/RegisterPressure.cpp @@ -635,7 +635,7 @@ void PressureDiffs::init(unsigned N) { } Max = Size; free(PDiffArray); - PDiffArray = reinterpret_cast(calloc(N, sizeof(PressureDiff))); + PDiffArray = static_cast(safe_calloc(N, sizeof(PressureDiff))); } void PressureDiffs::addInstruction(unsigned Idx, diff --git a/lib/ExecutionEngine/Interpreter/Execution.cpp b/lib/ExecutionEngine/Interpreter/Execution.cpp index 96844439e72..358366c765f 100644 --- a/lib/ExecutionEngine/Interpreter/Execution.cpp +++ b/lib/ExecutionEngine/Interpreter/Execution.cpp @@ -974,7 +974,7 @@ void Interpreter::visitAllocaInst(AllocaInst &I) { unsigned MemToAlloc = std::max(1U, NumElements * TypeSize); // Allocate enough memory to hold the type... - void *Memory = malloc(MemToAlloc); + void *Memory = safe_malloc(MemToAlloc); DEBUG(dbgs() << "Allocated Type: " << *Ty << " (" << TypeSize << " bytes) x " << NumElements << " (Total: " << MemToAlloc << ") at " diff --git a/lib/Object/Object.cpp b/lib/Object/Object.cpp index 1d2859cfbe9..5fd823e0117 100644 --- a/lib/Object/Object.cpp +++ b/lib/Object/Object.cpp @@ -228,7 +228,7 @@ uint64_t LLVMGetRelocationType(LLVMRelocationIteratorRef RI) { const char *LLVMGetRelocationTypeName(LLVMRelocationIteratorRef RI) { SmallVector ret; (*unwrap(RI))->getTypeName(ret); - char *str = static_cast(malloc(ret.size())); + char *str = static_cast(safe_malloc(ret.size())); std::copy(ret.begin(), ret.end(), str); return str; } diff --git a/lib/Support/RWMutex.cpp b/lib/Support/RWMutex.cpp index 83c6d1d52b4..8182319541e 100644 --- a/lib/Support/RWMutex.cpp +++ b/lib/Support/RWMutex.cpp @@ -11,6 +11,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/Support/Allocator.h" #include "llvm/Support/RWMutex.h" #include "llvm/Config/config.h" @@ -49,7 +50,7 @@ RWMutexImpl::RWMutexImpl() { // Declare the pthread_rwlock data structures pthread_rwlock_t* rwlock = - static_cast(malloc(sizeof(pthread_rwlock_t))); + static_cast(safe_malloc(sizeof(pthread_rwlock_t))); #ifdef __APPLE__ // Workaround a bug/mis-feature in Darwin's pthread_rwlock_init. diff --git a/lib/Support/StringMap.cpp b/lib/Support/StringMap.cpp index 4341da2d97b..9382c3ce29e 100644 --- a/lib/Support/StringMap.cpp +++ b/lib/Support/StringMap.cpp @@ -57,10 +57,9 @@ void StringMapImpl::init(unsigned InitSize) { NumItems = 0; NumTombstones = 0; - TheTable = (StringMapEntryBase **)calloc(NewNumBuckets+1, - sizeof(StringMapEntryBase **) + - sizeof(unsigned)); - + TheTable = static_cast( + std::calloc(NewNumBuckets+1, + sizeof(StringMapEntryBase **) + sizeof(unsigned))); if (TheTable == nullptr) report_bad_alloc_error("Allocation of StringMap table failed."); @@ -219,10 +218,8 @@ unsigned StringMapImpl::RehashTable(unsigned BucketNo) { unsigned NewBucketNo = BucketNo; // Allocate one extra bucket which will always be non-empty. This allows the // iterators to stop at end. - StringMapEntryBase **NewTableArray = - (StringMapEntryBase **)calloc(NewSize+1, sizeof(StringMapEntryBase *) + - sizeof(unsigned)); - + auto NewTableArray = static_cast( + std::calloc(NewSize+1, sizeof(StringMapEntryBase *) + sizeof(unsigned))); if (NewTableArray == nullptr) report_bad_alloc_error("Allocation of StringMap hash table failed."); diff --git a/lib/Support/Unix/Signals.inc b/lib/Support/Unix/Signals.inc index aaf760c5b61..a626b251ccd 100644 --- a/lib/Support/Unix/Signals.inc +++ b/lib/Support/Unix/Signals.inc @@ -138,7 +138,7 @@ static void CreateSigAltStack() { return; stack_t AltStack = {}; - AltStack.ss_sp = reinterpret_cast(malloc(AltStackSize)); + AltStack.ss_sp = static_cast(safe_malloc(AltStackSize)); NewAltStackPointer = AltStack.ss_sp; // Save to avoid reporting a leak. AltStack.ss_size = AltStackSize; if (sigaltstack(&AltStack, &OldAltStack) != 0) diff --git a/lib/Support/Windows/RWMutex.inc b/lib/Support/Windows/RWMutex.inc index ac60c2fc05b..5eb9351eee5 100644 --- a/lib/Support/Windows/RWMutex.inc +++ b/lib/Support/Windows/RWMutex.inc @@ -74,10 +74,10 @@ static bool loadSRW() { sys::RWMutexImpl::RWMutexImpl() { if (loadSRW()) { - data_ = calloc(1, sizeof(SRWLOCK)); + data_ = safe_calloc(1, sizeof(SRWLOCK)); fpInitializeSRWLock(static_cast(data_)); } else { - data_ = calloc(1, sizeof(CRITICAL_SECTION)); + data_ = safe_calloc(1, sizeof(CRITICAL_SECTION)); InitializeCriticalSection(static_cast(data_)); } } diff --git a/tools/llvm-c-test/attributes.c b/tools/llvm-c-test/attributes.c index c6beab1d31b..c7bc0d3c87b 100644 --- a/tools/llvm-c-test/attributes.c +++ b/tools/llvm-c-test/attributes.c @@ -14,6 +14,7 @@ #include "llvm-c-test.h" +#include #include int llvm_test_function_attributes(void) { @@ -30,6 +31,7 @@ int llvm_test_function_attributes(void) { int AttrCount = LLVMGetAttributeCountAtIndex(F, Idx); LLVMAttributeRef *Attrs = (LLVMAttributeRef *)malloc(AttrCount * sizeof(LLVMAttributeRef)); + assert(Attrs); LLVMGetAttributesAtIndex(F, Idx, Attrs); free(Attrs); } @@ -61,6 +63,7 @@ int llvm_test_callsite_attributes(void) { int AttrCount = LLVMGetCallSiteAttributeCount(I, Idx); LLVMAttributeRef *Attrs = (LLVMAttributeRef *)malloc( AttrCount * sizeof(LLVMAttributeRef)); + assert(Attrs); LLVMGetCallSiteAttributes(I, Idx, Attrs); free(Attrs); } diff --git a/tools/llvm-c-test/echo.cpp b/tools/llvm-c-test/echo.cpp index 05d80363162..db7211a3eab 100644 --- a/tools/llvm-c-test/echo.cpp +++ b/tools/llvm-c-test/echo.cpp @@ -90,7 +90,8 @@ struct TypeCloner { unsigned ParamCount = LLVMCountParamTypes(Src); LLVMTypeRef* Params = nullptr; if (ParamCount > 0) { - Params = (LLVMTypeRef*) malloc(ParamCount * sizeof(LLVMTypeRef)); + Params = static_cast( + safe_malloc(ParamCount * sizeof(LLVMTypeRef))); LLVMGetParamTypes(Src, Params); for (unsigned i = 0; i < ParamCount; i++) Params[i] = Clone(Params[i]); diff --git a/unittests/Support/AllocatorTest.cpp b/unittests/Support/AllocatorTest.cpp index 4897c47eb28..74b394f1b17 100644 --- a/unittests/Support/AllocatorTest.cpp +++ b/unittests/Support/AllocatorTest.cpp @@ -147,7 +147,7 @@ public: // Allocate space for the alignment, the slab, and a void* that goes right // before the slab. size_t Alignment = 4096; - void *MemBase = malloc(Size + Alignment - 1 + sizeof(void*)); + void *MemBase = safe_malloc(Size + Alignment - 1 + sizeof(void*)); // Find the slab start. void *Slab = (void *)alignAddr((char*)MemBase + sizeof(void *), Alignment); diff --git a/unittests/Support/ManagedStatic.cpp b/unittests/Support/ManagedStatic.cpp index 07e324cdfb6..d3cc80cf5e9 100644 --- a/unittests/Support/ManagedStatic.cpp +++ b/unittests/Support/ManagedStatic.cpp @@ -6,6 +6,8 @@ // License. See LICENSE.TXT for details. // //===----------------------------------------------------------------------===// + +#include "llvm/Support/Allocator.h" #include "llvm/Support/ManagedStatic.h" #include "llvm/Config/config.h" #ifdef HAVE_PTHREAD_H @@ -30,7 +32,7 @@ namespace test1 { // Valgrind's leak checker complains glibc's stack allocation. // To appease valgrind, we provide our own stack for each thread. void *allocate_stack(pthread_attr_t &a, size_t n = 65536) { - void *stack = malloc(n); + void *stack = safe_malloc(n); pthread_attr_init(&a); #if defined(__linux__) pthread_attr_setstack(&a, stack, n); @@ -83,7 +85,7 @@ TEST(ManagedStaticTest, NestedStatics) { namespace CustomCreatorDeletor { struct CustomCreate { static void *call() { - void *Mem = std::malloc(sizeof(int)); + void *Mem = safe_malloc(sizeof(int)); *((int *)Mem) = 42; return Mem; }