[llvm] 848b0e2 - Improve error handling for SmallVector programming errors
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 2 15:01:17 PDT 2020
Author: Geoffrey Martin-Noble
Date: 2020-09-02T15:00:26-07:00
New Revision: 848b0e244c9ff5413c2eee6357d5faab1402d619
URL: https://github.com/llvm/llvm-project/commit/848b0e244c9ff5413c2eee6357d5faab1402d619
DIFF: https://github.com/llvm/llvm-project/commit/848b0e244c9ff5413c2eee6357d5faab1402d619.diff
LOG: Improve error handling for SmallVector programming errors
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
Added:
Modified:
llvm/include/llvm/ADT/SmallVector.h
llvm/include/llvm/Support/ErrorHandling.h
llvm/lib/Support/ErrorHandling.cpp
llvm/lib/Support/SmallVector.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index 3ccee3d21d48..5d8658f61271 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -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; }
@@ -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
@@ -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());
diff --git a/llvm/include/llvm/Support/ErrorHandling.h b/llvm/include/llvm/Support/ErrorHandling.h
index 7cbc668b3a0e..0ec0242d569d 100644
--- a/llvm/include/llvm/Support/ErrorHandling.h
+++ b/llvm/include/llvm/Support/ErrorHandling.h
@@ -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);
diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp
index e962657730fe..23b9f962422e 100644
--- a/llvm/lib/Support/ErrorHandling.cpp
+++ b/llvm/lib/Support/ErrorHandling.cpp
@@ -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
}
diff --git a/llvm/lib/Support/SmallVector.cpp b/llvm/lib/Support/SmallVector.cpp
index 6d5fe7165f63..73137640536c 100644
--- a/llvm/lib/Support/SmallVector.cpp
+++ b/llvm/lib/Support/SmallVector.cpp
@@ -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) {
More information about the llvm-commits
mailing list