[compiler-rt] [ASan] Honor `allocator_may_return_null` when set through user-function and fix large alloc edge case (PR #117929)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 27 13:27:02 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: David Justo (davidmrdavid)
<details>
<summary>Changes</summary>
**Related: ** #<!-- -->117925
**About this PR:**
This PR performs 3 small but related fixes for ASan users on Windows:
1. It ensures that the `allocator_may_return_null` flag is honored when set through the user function `__asan_default_options`. For more details, please see: #<!-- -->117925
2. It adds a missing `AllocatorMayReturnNull()` check inside `InternalAlloc` that's needed to avoid error'ing out when the allocator _correctly_ returns `null` when `allocator_may_return_null` is set.
3. In `sanitizer_win`'s `ReturnNullptrOnOOMOrDie`, it allows returning `null` when the last error is set to `ERROR_INVALID_PARAMETER` which may be set by `VirtualAlloc` on WIndows when attempting to allocate exceedingly large memory.
I've added test cases that should cover these new behaviors. Happy to take on any feedback as well. Thank you :-)
---
Full diff: https://github.com/llvm/llvm-project/pull/117929.diff
5 Files Affected:
- (modified) compiler-rt/lib/asan/asan_flags.cpp (+7)
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp (+5-1)
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_win.cpp (+17-1)
- (added) compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_set_via_user_function.cpp (+26)
- (added) compiler-rt/test/asan/TestCases/allocator_may_return_null_limits.cpp (+22)
``````````diff
diff --git a/compiler-rt/lib/asan/asan_flags.cpp b/compiler-rt/lib/asan/asan_flags.cpp
index 56deb1b0d082b8..1499c63fd19014 100644
--- a/compiler-rt/lib/asan/asan_flags.cpp
+++ b/compiler-rt/lib/asan/asan_flags.cpp
@@ -240,6 +240,13 @@ void InitializeFlags() {
DisplayHelpMessages(&asan_parser);
ProcessFlags();
+
+ // TODO: Update other globals and data structures that may change after
+ // initialization due to these flags potentially changing.
+ // These flags can be identified by looking at the work done in
+ // `AsanInitInternal` inside of `asan_rtl.cpp`.
+ // See GH issue 'https://github.com/llvm/llvm-project/issues/117925' for details.
+ SetAllocatorMayReturnNull(common_flags()->allocator_may_return_null);
});
# if CAN_SANITIZE_UB
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp
index 9d899371c2dd90..b8c772d7df6c68 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp
@@ -85,8 +85,12 @@ static void NORETURN ReportInternalAllocatorOutOfMemory(uptr requested_size) {
void *InternalAlloc(uptr size, InternalAllocatorCache *cache, uptr alignment) {
void *p = RawInternalAlloc(size, cache, alignment);
- if (UNLIKELY(!p))
+ if (UNLIKELY(!p)) {
+ if (AllocatorMayReturnNull()){
+ return nullptr;
+ }
ReportInternalAllocatorOutOfMemory(size);
+ }
return p;
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
index ea513d5f263fe2..045a9a9d7b0b2d 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
@@ -164,7 +164,23 @@ void UnmapOrDie(void *addr, uptr size, bool raw_report) {
static void *ReturnNullptrOnOOMOrDie(uptr size, const char *mem_type,
const char *mmap_type) {
error_t last_error = GetLastError();
- if (last_error == ERROR_NOT_ENOUGH_MEMORY)
+
+ // Assumption: VirtualAlloc is the last system call that was invoked before
+ // this method.
+ // VirtualAlloc emits one of 2 error codes when running out of memory
+ // 1. ERROR_NOT_ENOUGH_MEMORY:
+ // There's not enough memory to execute the command
+ // 2. ERROR_INVALID_PARAMETER:
+ // VirtualAlloc will return this if the request would allocate memory at an
+ // address exceeding or being very close to the maximum application address
+ // (the `lpMaximumApplicationAddress` field within the `SystemInfo` struct).
+ // This does not seem to be officially documented, but is corroborated here:
+ // https://stackoverflow.com/questions/45833674/why-does-virtualalloc-fail-for-lpaddress-greater-than-0x6ffffffffff
+
+ // Note - It's possible that 'ERROR_COMMITMENT_LIMIT' needs to be handled here as well.
+ // It is currently not handled due to the lack of a reproducer that induces the error code.
+ if (last_error == ERROR_NOT_ENOUGH_MEMORY ||
+ last_error == ERROR_INVALID_PARAMETER)
return nullptr;
ReportMmapFailureAndDie(size, mem_type, mmap_type, last_error);
}
diff --git a/compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_set_via_user_function.cpp b/compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_set_via_user_function.cpp
new file mode 100644
index 00000000000000..23af2224ec4f08
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_set_via_user_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %run %t 2>&1
+// CHECK: Success
+
+#include <cstdint>
+#include <cstdlib>
+#include <limits>
+
+// On Windows, flags configured through the user-defined function `__asan_default_options`
+// are suspected to not always be honored according to this GH issue:
+// https://github.com/llvm/llvm-project/issues/117925
+// This issue is resolved for the `allocator_may_return_null` flag, but not for all others.
+// This test ensures we do not regress on `allocator_may_return_null` specifically.
+extern "C" __declspec(dllexport) extern const char *__asan_default_options() {
+ return "allocator_may_return_null=1";
+}
+
+int main() {
+ // Attempt to allocate an excessive amount of memory, which should
+ // terminate the program unless `allocator_may_return_null` is set.
+ size_t max = std::numeric_limits<size_t>::max();
+
+ free(malloc(max));
+ printf("Success");
+ return 0;
+}
diff --git a/compiler-rt/test/asan/TestCases/allocator_may_return_null_limits.cpp b/compiler-rt/test/asan/TestCases/allocator_may_return_null_limits.cpp
new file mode 100644
index 00000000000000..515ac2643e161a
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/allocator_may_return_null_limits.cpp
@@ -0,0 +1,22 @@
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %env_asan_opts=allocator_may_return_null=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1
+// RUN: %env_asan_opts=allocator_may_return_null=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2
+
+// CHECK1: exceeds maximum supported size
+// CHECK1: ABORT
+
+// CHECK2: Success
+
+#include <cstdint>
+#include <cstdlib>
+#include <limits>
+
+int main() {
+ // Attempt to allocate an excessive amount of memory, which should
+ // terminate the program unless `allocator_may_return_null` is set.
+ size_t max = std::numeric_limits<size_t>::max();
+
+ free(malloc(max));
+ printf("Success");
+ return 0;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/117929
More information about the llvm-commits
mailing list