[compiler-rt] 2dc2261 - [ASan] Honor `allocator_may_return_null` when set through user-function and fix large alloc edge case (#117929)

[via llvm-commits]

Author: David Justo
Date: 2024-12-11T10:21:35+01:00
New Revision: 2dc22615fd46ab2566d0f26d5ba234ab12dc4bf8

URL: https://github.com/llvm/llvm-project/commit/2dc22615fd46ab2566d0f26d5ba234ab12dc4bf8
DIFF: https://github.com/llvm/llvm-project/commit/2dc22615fd46ab2566d0f26d5ba234ab12dc4bf8.diff

LOG: [ASan] Honor `allocator_may_return_null` when set through user-function and fix large alloc edge case (#117929)

**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 :-)

---------

Co-authored-by: David Justo <dajusto at microsoft.com>

Added: 
    compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_limits.cpp

Modified: 
    compiler-rt/lib/asan/asan_flags.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_flags.cpp b/compiler-rt/lib/asan/asan_flags.cpp
index 56deb1b0d082b8..9cfb70bd00c786 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 need to change
+        // after initialization due to new flags potentially being set changing after
+        // `__asan_default_options` is registered.
+        // 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_win.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
index ea513d5f263fe2..1eef16fbde3e7d 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
@@ -164,7 +164,24 @@ 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_limits.cpp b/compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_limits.cpp
new file mode 100644
index 00000000000000..90db0d45af2d78
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Windows/allocator_may_return_null_limits.cpp
@@ -0,0 +1,34 @@
+// 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=CHECK-ABORT
+// RUN: %env_asan_opts=allocator_may_return_null=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-RETURN-NULL
+
+// RUN: %clangxx_asan -O0 %s -o %t -DUSER_FUNCTION
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-RETURN-NULL
+
+#if USER_FUNCTION
+// On Windows, flags configured through the user-defined function `__asan_default_options`
+// are suspected to not always be honored according to GitHub bug:
+// https://github.com/llvm/llvm-project/issues/117925
+// 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";
+}
+#endif
+
+#include <cstdint>
+#include <cstdio>
+#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();
+
+  // CHECK-ABORT: exceeds maximum supported size
+  // CHECK-ABORT: ABORT
+  free(malloc(max));
+
+  printf("Success"); // CHECK-RETURN-NULL: Success
+  return 0;
+}