Skip to content

Commit

Permalink
Improve error handling for SmallVector programming errors
Browse files Browse the repository at this point in the history
This patch changes errors in `SmallVector::grow` that are independent of
memory capacity to be reported using report_fatal_error or
std::length_error instead of report_bad_alloc_error, which falsely signals
an OOM.

It also cleans up a few related things:
- makes report_bad_alloc_error to print the failure reason passed
  to it.
- fixes the documentation to indicate that report_bad_alloc_error
  calls `abort()` not "an assertion"
- uses a consistent name for the size/capacity argument to `grow`
  and `grow_pod`

Reviewed By: mehdi_amini, MaskRay

Differential Revision: https://reviews.llvm.org/D86892
  • Loading branch information
GMNGeoffrey authored and MaskRay committed Sep 2, 2020
1 parent 1284dc3 commit 848b0e2
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 22 deletions.
34 changes: 25 additions & 9 deletions llvm/include/llvm/ADT/SmallVector.h
Expand Up @@ -60,7 +60,7 @@ template <class Size_T> class SmallVectorBase {
/// This is an implementation of the grow() method which only works
/// on POD-like data types and is out of line to reduce code duplication.
/// This function will report a fatal error if it cannot increase capacity.
void grow_pod(void *FirstEl, size_t MinCapacity, size_t TSize);
void grow_pod(void *FirstEl, size_t MinSize, size_t TSize);

public:
size_t size() const { return Size; }
Expand Down Expand Up @@ -115,8 +115,8 @@ class SmallVectorTemplateCommon
protected:
SmallVectorTemplateCommon(size_t Size) : Base(getFirstEl(), Size) {}

void grow_pod(size_t MinCapacity, size_t TSize) {
Base::grow_pod(getFirstEl(), MinCapacity, TSize);
void grow_pod(size_t MinSize, size_t TSize) {
Base::grow_pod(getFirstEl(), MinSize, TSize);
}

/// Return true if this is a smallvector which has not had dynamic
Expand Down Expand Up @@ -268,16 +268,32 @@ template <typename T, bool TriviallyCopyable>
void SmallVectorTemplateBase<T, TriviallyCopyable>::grow(size_t MinSize) {
// Ensure we can fit the new capacity.
// This is only going to be applicable when the capacity is 32 bit.
if (MinSize > this->SizeTypeMax())
report_bad_alloc_error("SmallVector capacity overflow during allocation");
if (MinSize > this->SizeTypeMax()) {
std::string Reason = "SmallVector unable to grow. Requested capacity (" +
std::to_string(MinSize) +
") is larger than maximum value for size type (" +
std::to_string(this->SizeTypeMax()) + ")";
#ifdef LLVM_ENABLE_EXCEPTIONS
throw std::length_error(Reason);
#else
report_fatal_error(Reason);
#endif
}

// Ensure we can meet the guarantee of space for at least one more element.
// The above check alone will not catch the case where grow is called with a
// default MinCapacity of 0, but the current capacity cannot be increased.
// default MinSize of 0, but the current capacity cannot be increased.
// This is only going to be applicable when the capacity is 32 bit.
if (this->capacity() == this->SizeTypeMax())
report_bad_alloc_error("SmallVector capacity unable to grow");

if (this->capacity() == this->SizeTypeMax()) {
std::string Reason =
"SmallVector capacity unable to grow. Already at maximum size " +
std::to_string(this->SizeTypeMax());
#ifdef LLVM_ENABLE_EXCEPTIONS
throw std::length_error(Reason);
#else
report_fatal_error(Reason);
#endif
}
// Always grow, even from zero.
size_t NewCapacity = size_t(NextPowerOf2(this->capacity() + 2));
NewCapacity = std::min(std::max(NewCapacity, MinSize), this->SizeTypeMax());
Expand Down
6 changes: 3 additions & 3 deletions llvm/include/llvm/Support/ErrorHandling.h
Expand Up @@ -110,9 +110,9 @@ void install_out_of_memory_new_handler();
/// the following unwind succeeds, e.g. do not trigger additional allocations
/// in the unwind chain.
///
/// If no error handler is installed (default), then a bad_alloc exception
/// is thrown, if LLVM is compiled with exception support, otherwise an
/// assertion is called.
/// If no error handler is installed (default), throws a bad_alloc exception
/// if LLVM is compiled with exception support. Otherwise prints the error
/// to standard error and calls abort().
LLVM_ATTRIBUTE_NORETURN void report_bad_alloc_error(const char *Reason,
bool GenCrashDiag = true);

Expand Down
8 changes: 5 additions & 3 deletions llvm/lib/Support/ErrorHandling.cpp
Expand Up @@ -168,9 +168,11 @@ void llvm::report_bad_alloc_error(const char *Reason, bool GenCrashDiag) {
#else
// Don't call the normal error handler. It may allocate memory. Directly write
// an OOM to stderr and abort.
char OOMMessage[] = "LLVM ERROR: out of memory\n";
ssize_t written = ::write(2, OOMMessage, strlen(OOMMessage));
(void)written;
const char *OOMMessage = "LLVM ERROR: out of memory\n";
const char *Newline = "\n";
(void)::write(2, OOMMessage, strlen(OOMMessage));
(void)::write(2, Reason, strlen(Reason));
(void)::write(2, Newline, strlen(Newline));
abort();
#endif
}
Expand Down
30 changes: 23 additions & 7 deletions llvm/lib/Support/SmallVector.cpp
Expand Up @@ -44,24 +44,40 @@ static_assert(sizeof(SmallVector<char, 0>) ==

// Note: Moving this function into the header may cause performance regression.
template <class Size_T>
void SmallVectorBase<Size_T>::grow_pod(void *FirstEl, size_t MinCapacity,
void SmallVectorBase<Size_T>::grow_pod(void *FirstEl, size_t MinSize,
size_t TSize) {
// Ensure we can fit the new capacity.
// This is only going to be applicable when the capacity is 32 bit.
if (MinCapacity > SizeTypeMax())
report_bad_alloc_error("SmallVector capacity overflow during allocation");
if (MinSize > SizeTypeMax()) {
std::string Reason = "SmallVector unable to grow. Requested capacity (" +
std::to_string(MinSize) +
") is larger than maximum value for size type (" +
std::to_string(SizeTypeMax()) + ")";
#ifdef LLVM_ENABLE_EXCEPTIONS
throw std::length_error(Reason);
#else
report_fatal_error(Reason);
#endif
}

// Ensure we can meet the guarantee of space for at least one more element.
// The above check alone will not catch the case where grow is called with a
// default MinCapacity of 0, but the current capacity cannot be increased.
// default MinSize of 0, but the current capacity cannot be increased.
// This is only going to be applicable when the capacity is 32 bit.
if (capacity() == SizeTypeMax())
report_bad_alloc_error("SmallVector capacity unable to grow");
if (capacity() == SizeTypeMax()) {
std::string Reason =
"SmallVector capacity unable to grow. Already at maximum size " +
std::to_string(SizeTypeMax());
#ifdef LLVM_ENABLE_EXCEPTIONS
throw std::length_error(Reason);
#endif
report_fatal_error(Reason);
}

// In theory 2*capacity can overflow if the capacity is 64 bit, but the
// original capacity would never be large enough for this to be a problem.
size_t NewCapacity = 2 * capacity() + 1; // Always grow.
NewCapacity = std::min(std::max(NewCapacity, MinCapacity), SizeTypeMax());
NewCapacity = std::min(std::max(NewCapacity, MinSize), SizeTypeMax());

void *NewElts;
if (BeginX == FirstEl) {
Expand Down

0 comments on commit 848b0e2

Please sign in to comment.