[scudo] Add specific die functions for linux specific failures. #68650
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
compiler-rt
compiler-rt:scudo
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Christopher Ferris (cferris1000) ChangesWhile running into failures on unmap calls, it becomes difficult to figure out what's wrong. Break the dieOnMapUnmapError into specific versions for map, unmap, and then one for mprotect. Also, put these in a common linux space so that all linux derived code can reuse this code. Full diff: https://github.com/llvm/llvm-project/pull/68650.diff 8 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/CMakeLists.txt
index c4d3ea1e4f05ba8..9738fb17d7f1241 100644
--- a/compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ b/compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -108,6 +108,7 @@ set(SCUDO_SOURCES
flags.cpp
fuchsia.cpp
linux.cpp
+ linux_common.cpp
mem_map.cpp
mem_map_fuchsia.cpp
mem_map_linux.cpp
diff --git a/compiler-rt/lib/scudo/standalone/common.cpp b/compiler-rt/lib/scudo/standalone/common.cpp
index 666f95400c7e7aa..06e930638f6f971 100644
--- a/compiler-rt/lib/scudo/standalone/common.cpp
+++ b/compiler-rt/lib/scudo/standalone/common.cpp
@@ -21,18 +21,4 @@ uptr getPageSizeSlow() {
return PageSizeCached;
}
-// Fatal internal map() or unmap() error (potentially OOM related).
-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();
-}
-
} // namespace scudo
diff --git a/compiler-rt/lib/scudo/standalone/common.h b/compiler-rt/lib/scudo/standalone/common.h
index d0f429cfcb7a08e..3581c946d1608fc 100644
--- a/compiler-rt/lib/scudo/standalone/common.h
+++ b/compiler-rt/lib/scudo/standalone/common.h
@@ -175,10 +175,6 @@ void setMemoryPermission(uptr Addr, uptr Size, uptr Flags,
void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size,
MapPlatformData *Data = nullptr);
-// 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.
void setAbortMessage(const char *Message);
diff --git a/compiler-rt/lib/scudo/standalone/linux.cpp b/compiler-rt/lib/scudo/standalone/linux.cpp
index c31c3d2483a97ad..3e3b2147b3c77c1 100644
--- a/compiler-rt/lib/scudo/standalone/linux.cpp
+++ b/compiler-rt/lib/scudo/standalone/linux.cpp
@@ -13,6 +13,7 @@
#include "common.h"
#include "internal_defs.h"
#include "linux.h"
+#include "linux_common.h"
#include "mutex.h"
#include "string_utils.h"
@@ -41,8 +42,6 @@ namespace scudo {
uptr getPageSize() { return static_cast<uptr>(sysconf(_SC_PAGESIZE)); }
-void NORETURN die() { abort(); }
-
// TODO: Will be deprecated. Use the interfaces in MemMapLinux instead.
void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags,
UNUSED MapPlatformData *Data) {
@@ -66,7 +65,7 @@ void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags,
void *P = mmap(Addr, Size, MmapProt, MmapFlags, -1, 0);
if (P == MAP_FAILED) {
if (!(Flags & MAP_ALLOWNOMEM) || errno != ENOMEM)
- dieOnMapUnmapError(errno == ENOMEM ? Size : 0);
+ dieOnMapError(errno == ENOMEM ? Size : 0);
return nullptr;
}
#if SCUDO_ANDROID
@@ -80,7 +79,7 @@ void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags,
void unmap(void *Addr, uptr Size, UNUSED uptr Flags,
UNUSED MapPlatformData *Data) {
if (munmap(Addr, Size) != 0)
- dieOnMapUnmapError();
+ dieOnUnmapError(reinterpret_cast<uptr>(Addr), Size);
}
// TODO: Will be deprecated. Use the interfaces in MemMapLinux instead.
@@ -88,7 +87,7 @@ void setMemoryPermission(uptr Addr, uptr Size, uptr Flags,
UNUSED MapPlatformData *Data) {
int Prot = (Flags & MAP_NOACCESS) ? PROT_NONE : (PROT_READ | PROT_WRITE);
if (mprotect(reinterpret_cast<void *>(Addr), Size, Prot) != 0)
- dieOnMapUnmapError();
+ dieOnProtectError(Addr, Size, Prot);
}
// TODO: Will be deprecated. Use the interfaces in MemMapLinux instead.
diff --git a/compiler-rt/lib/scudo/standalone/linux_common.cpp b/compiler-rt/lib/scudo/standalone/linux_common.cpp
new file mode 100644
index 000000000000000..d0e39fb0ced87e3
--- /dev/null
+++ b/compiler-rt/lib/scudo/standalone/linux_common.cpp
@@ -0,0 +1,65 @@
+//===-- linux_common.cpp ----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "platform.h"
+
+#if SCUDO_LINUX
+
+#include "common.h"
+#include "internal_defs.h"
+#include "linux_common.h"
+#include "string_utils.h"
+
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+
+namespace scudo {
+
+void NORETURN die() { abort(); }
+
+// Fatal internal map() error (potentially OOM related).
+void NORETURN dieOnMapError(uptr SizeIfOOM) {
+ char Error[128] = "Scudo ERROR: internal map 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();
+}
+
+void NORETURN dieOnUnmapError(uptr Addr, uptr Size) {
+ char Error[128];
+ formatString(Error, sizeof(Error),
+ "Scudo ERROR: internal unmap failure (error desc=%s) Addr 0x%zx "
+ "Size %zu\n",
+ strerror(errno), Addr, Size);
+ outputRaw(Error);
+ setAbortMessage(Error);
+ die();
+}
+
+void NORETURN dieOnProtectError(uptr Addr, uptr Size, int Prot) {
+ char Error[128];
+ formatString(
+ Error, sizeof(Error),
+ "Scudo ERROR: internal protect failure (error desc=%s) Addr 0x%zx "
+ "Size %zu Prot %x\n",
+ strerror(errno), Addr, Size, Prot);
+ outputRaw(Error);
+ setAbortMessage(Error);
+ die();
+}
+
+} // namespace scudo
+
+#endif // SCUDO_LINUX
diff --git a/compiler-rt/lib/scudo/standalone/linux_common.h b/compiler-rt/lib/scudo/standalone/linux_common.h
new file mode 100644
index 000000000000000..6a5c528a040168b
--- /dev/null
+++ b/compiler-rt/lib/scudo/standalone/linux_common.h
@@ -0,0 +1,34 @@
+//===-- linux_common.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SCUDO_LINUX_COMMON_H_
+#define SCUDO_LINUX_COMMON_H_
+
+#include "platform.h"
+
+#if SCUDO_LINUX
+
+#include "internal_defs.h"
+
+namespace scudo {
+
+// Internal map fatal error. This must not call map(). SizeIfOOM shall
+// hold the requested size on an out-of-memory error, 0 otherwise.
+void NORETURN dieOnMapError(uptr SizeIfOOM = 0);
+
+// Internal unmap fatal error. This must not call map().
+void NORETURN dieOnUnmapError(uptr Addr, uptr Size);
+
+// Internal protect fatal error. This must not call map().
+void NORETURN dieOnProtectError(uptr Addr, uptr Size, int Prot);
+
+} // namespace scudo
+
+#endif // SCUDO_LINUX
+
+#endif // SCUDO_LINUX_COMMON_H_
diff --git a/compiler-rt/lib/scudo/standalone/mem_map_linux.cpp b/compiler-rt/lib/scudo/standalone/mem_map_linux.cpp
index f377d105894db7d..8e55599398b956b 100644
--- a/compiler-rt/lib/scudo/standalone/mem_map_linux.cpp
+++ b/compiler-rt/lib/scudo/standalone/mem_map_linux.cpp
@@ -15,6 +15,7 @@
#include "common.h"
#include "internal_defs.h"
#include "linux.h"
+#include "linux_common.h"
#include "mutex.h"
#include "string_utils.h"
@@ -64,7 +65,7 @@ static void *mmapWrapper(uptr Addr, uptr Size, const char *Name, uptr Flags) {
mmap(reinterpret_cast<void *>(Addr), Size, MmapProt, MmapFlags, -1, 0);
if (P == MAP_FAILED) {
if (!(Flags & MAP_ALLOWNOMEM) || errno != ENOMEM)
- dieOnMapUnmapError(errno == ENOMEM ? Size : 0);
+ dieOnMapError(errno == ENOMEM ? Size : 0);
return nullptr;
}
#if SCUDO_ANDROID
@@ -101,21 +102,21 @@ void MemMapLinux::unmapImpl(uptr Addr, uptr Size) {
}
if (munmap(reinterpret_cast<void *>(Addr), Size) != 0)
- dieOnMapUnmapError();
+ dieOnUnmapError(Addr, Size);
}
bool MemMapLinux::remapImpl(uptr Addr, uptr Size, const char *Name,
uptr Flags) {
void *P = mmapWrapper(Addr, Size, Name, Flags);
if (reinterpret_cast<uptr>(P) != Addr)
- dieOnMapUnmapError();
+ dieOnMapError();
return true;
}
void MemMapLinux::setMemoryPermissionImpl(uptr Addr, uptr Size, uptr Flags) {
int Prot = (Flags & MAP_NOACCESS) ? PROT_NONE : (PROT_READ | PROT_WRITE);
if (mprotect(reinterpret_cast<void *>(Addr), Size, Prot) != 0)
- dieOnMapUnmapError();
+ dieOnProtectError(Addr, Size, Prot);
}
void MemMapLinux::releaseAndZeroPagesToOSImpl(uptr From, uptr Size) {
@@ -139,7 +140,7 @@ bool ReservedMemoryLinux::createImpl(uptr Addr, uptr Size, const char *Name,
void ReservedMemoryLinux::releaseImpl() {
if (munmap(reinterpret_cast<void *>(getBase()), getCapacity()) != 0)
- dieOnMapUnmapError();
+ dieOnUnmapError(getBase(), getCapacity());
}
ReservedMemoryLinux::MemMapT ReservedMemoryLinux::dispatchImpl(uptr Addr,
diff --git a/compiler-rt/lib/scudo/standalone/trusty.cpp b/compiler-rt/lib/scudo/standalone/trusty.cpp
index 5f72b1cb3e54b05..daa72ba0c4bf3b1 100644
--- a/compiler-rt/lib/scudo/standalone/trusty.cpp
+++ b/compiler-rt/lib/scudo/standalone/trusty.cpp
@@ -11,6 +11,7 @@
#if SCUDO_TRUSTY
#include "common.h"
+#include "linux_common.h"
#include "mutex.h"
#include "trusty.h"
@@ -28,8 +29,6 @@ namespace scudo {
uptr getPageSize() { return getauxval(AT_PAGESZ); }
-void NORETURN die() { abort(); }
-
void *map(void *Addr, uptr Size, const char *Name, uptr Flags,
UNUSED MapPlatformData *Data) {
uint32_t MmapFlags =
@@ -51,7 +50,7 @@ void *map(void *Addr, uptr Size, const char *Name, uptr Flags,
if (IS_ERR(P)) {
errno = lk_err_to_errno(PTR_ERR(P));
if (!(Flags & MAP_ALLOWNOMEM) || errno != ENOMEM)
- dieOnMapUnmapError(Size);
+ dieOnMapError(Size);
return nullptr;
}
@@ -61,7 +60,7 @@ void *map(void *Addr, uptr Size, const char *Name, uptr Flags,
void unmap(UNUSED void *Addr, UNUSED uptr Size, UNUSED uptr Flags,
UNUSED MapPlatformData *Data) {
if (_trusty_munmap(Addr, Size) != 0)
- dieOnMapUnmapError();
+ dieOnUnmapError(Addr, Size);
}
void setMemoryPermission(UNUSED uptr Addr, UNUSED uptr Size, UNUSED uptr Flags,
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
fabio-d
reviewed
Oct 11, 2023
fabio-d
reviewed
Oct 11, 2023
While running into failures on unmap calls, it becomes difficult to figure out what's wrong. Break the dieOnMapUnmapError into specific Report versions for map, unmap, and then one for mprotect. Also, put these in a common report linux file so that all linux derived code can reuse this code. Finally, add a ReportRawError that is used by all of the code so that it's all in the same space.
Move the dieOnXXX routines to reportXXX. Mv linux_common to report_linux. Add a new reportRawError function that doesn't use ScopedString.
Move the die definitions in trusty.cpp and linux.cpp to their original location to avoid unnecessary churn in the files.
cferris1000
force-pushed
the
die_cleanup
branch
from
2f39907
to
c72edbb
Compare
|
Fixed the comment on the first message to actually reflect what was done. |
|
Still LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While running into failures on unmap calls, it becomes difficult to figure out what's wrong. Break the dieOnMapUnmapError into specific versions for map, unmap, and then one for mprotect.
Also, put these in a common linux space so that all linux derived code can reuse this code.