[scudo] Rework dieOnMapUnmapError
Said function had a few shortfalls:
- didn't set an abort message on Android
- was logged on several lines
- didn't provide extra information like the size requested if OOM'ing
This improves the function to address those points.
Differential Revision: https://reviews.llvm.org/D103034
GitOrigin-RevId: 1872283457fc1617fa750a11abdfd44e881dfcdb
diff --git a/common.cpp b/common.cpp
index d93bfc5..666f954 100644
--- a/common.cpp
+++ b/common.cpp
@@ -8,6 +8,7 @@
#include "common.h"
#include "atomic_helpers.h"
+#include "string_utils.h"
namespace scudo {
@@ -21,11 +22,16 @@
}
// Fatal internal map() or unmap() error (potentially OOM related).
-void NORETURN dieOnMapUnmapError(bool OutOfMemory) {
- outputRaw("Scudo ERROR: internal map or unmap failure");
- if (OutOfMemory)
- outputRaw(" (OOM)");
- outputRaw("\n");
+void NORETURN dieOnMapUnmapError(uptr SizeIfOOM) {
+ char Error[128] = "Scudo ERROR: internal map or unmap failure\n";
+ if (SizeIfOOM) {
+ formatString(
+ Error, sizeof(Error),
+ "Scudo ERROR: internal map failure (NO MEMORY) requesting %zuKB\n",
+ SizeIfOOM >> 10);
+ }
+ outputRaw(Error);
+ setAbortMessage(Error);
die();
}
diff --git a/common.h b/common.h
index d9884df..3f27a3d 100644
--- a/common.h
+++ b/common.h
@@ -171,8 +171,9 @@
void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size,
MapPlatformData *Data = nullptr);
-// Internal map & unmap fatal error. This must not call map().
-void NORETURN dieOnMapUnmapError(bool OutOfMemory = false);
+// Internal map & unmap fatal error. This must not call map(). SizeIfOOM shall
+// hold the requested size on an out-of-memory error, 0 otherwise.
+void NORETURN dieOnMapUnmapError(uptr SizeIfOOM = 0);
// Logging related functions.
diff --git a/fuchsia.cpp b/fuchsia.cpp
index 11c1de7..3b473bc 100644
--- a/fuchsia.cpp
+++ b/fuchsia.cpp
@@ -41,7 +41,7 @@
Size, &Data->Vmar, &Data->VmarBase);
if (UNLIKELY(Status != ZX_OK)) {
if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
- dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY);
+ dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0);
return nullptr;
}
return reinterpret_cast<void *>(Data->VmarBase);
@@ -71,7 +71,7 @@
Status = _zx_vmo_set_size(Vmo, VmoSize + Size);
if (Status != ZX_OK) {
if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
- dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY);
+ dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0);
return nullptr;
}
} else {
@@ -79,7 +79,7 @@
Status = _zx_vmo_create(Size, ZX_VMO_RESIZABLE, &Vmo);
if (UNLIKELY(Status != ZX_OK)) {
if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
- dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY);
+ dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0);
return nullptr;
}
_zx_object_set_property(Vmo, ZX_PROP_NAME, Name, strlen(Name));
@@ -105,7 +105,7 @@
}
if (UNLIKELY(Status != ZX_OK)) {
if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
- dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY);
+ dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0);
return nullptr;
}
if (Data)
diff --git a/linux.cpp b/linux.cpp
index 7ec6987..301bdcd 100644
--- a/linux.cpp
+++ b/linux.cpp
@@ -67,7 +67,7 @@
void *P = mmap(Addr, Size, MmapProt, MmapFlags, -1, 0);
if (P == MAP_FAILED) {
if (!(Flags & MAP_ALLOWNOMEM) || errno != ENOMEM)
- dieOnMapUnmapError(errno == ENOMEM);
+ dieOnMapUnmapError(errno == ENOMEM ? Size : 0);
return nullptr;
}
#if SCUDO_ANDROID
@@ -91,15 +91,15 @@
}
static bool madviseNeedsMemset() {
- uptr Size = getPageSizeCached();
- char *P = (char *)mmap(0, Size, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ const uptr Size = getPageSizeCached();
+ char *P = reinterpret_cast<char *>(mmap(0, Size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0));
if (!P)
- dieOnMapUnmapError(errno == ENOMEM);
+ dieOnMapUnmapError(errno == ENOMEM ? Size : 0);
*P = 1;
while (madvise(P, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) {
}
- bool R = (*P != 0);
+ const bool R = (*P != 0);
if (munmap(P, Size) != 0)
dieOnMapUnmapError();
return R;
diff --git a/string_utils.cpp b/string_utils.cpp
index f304491..25bddbc 100644
--- a/string_utils.cpp
+++ b/string_utils.cpp
@@ -115,8 +115,8 @@
return Res;
}
-int formatString(char *Buffer, uptr BufferLength, const char *Format,
- va_list Args) {
+static int formatString(char *Buffer, uptr BufferLength, const char *Format,
+ va_list Args) {
static const char *PrintfFormatsHelp =
"Supported formatString formats: %([0-9]*)?(z|ll)?{d,u,x,X}; %p; "
"%[-]([0-9]*)?(\\.\\*)?s; %c\n";
@@ -210,6 +210,14 @@
return Res;
}
+int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) {
+ va_list Args;
+ va_start(Args, Format);
+ int Res = formatString(Buffer, BufferLength, Format, Args);
+ va_end(Args);
+ return Res;
+}
+
void ScopedString::append(const char *Format, va_list Args) {
DCHECK_LT(Length, String.size());
va_list ArgsCopy;
diff --git a/string_utils.h b/string_utils.h
index acd60bd..4880fa1 100644
--- a/string_utils.h
+++ b/string_utils.h
@@ -36,6 +36,7 @@
uptr Length;
};
+int formatString(char *Buffer, uptr BufferLength, const char *Format, ...);
void Printf(const char *Format, ...);
} // namespace scudo
