[compiler-rt][Fuchsia] Change GetMaxUserVirtualAddress to invoke syscall

[PiJoules]

LSan was recently refactored to call GetMaxUserVirtualAddress for diagnostic purposes. This leads to failures for some of our downstream tests which only run with lsan. This occurs because GetMaxUserVirtualAddress depends on setting up shadow via a call to __sanitizer_shadow_bounds, but shadow bounds aren't set for standalone lsan because it doesn't use shadow. This updates the function to invoke the same syscall used by __sanitizer_shadow_bounds calls for getting the memory limit. Ideally this function would only be called once since we only need to get the bounds once.

More context in https://fxbug.dev/437346226.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (PiJoules)

Changes

LSan was recently refactored to call GetMaxUserVirtualAddress for diagnostic purposes. This leads to failures for some of our downstream tests which only run with lsan. This occurs because GetMaxUserVirtualAddress depends on setting up shadow via a call to __sanitizer_shadow_bounds, but shadow bounds aren't set for standalone lsan because it doesn't use shadow. This updates the function to invoke the same syscall used by __sanitizer_shadow_bounds calls for getting the memory limit. Ideally this function would only be called once since we only need to get the bounds once.

More context in https://fxbug.dev/437346226.

---

Full diff: https://github.com/llvm/llvm-project/pull/153309.diff

2 Files Affected:

• (modified) compiler-rt/lib/asan/asan_fuchsia.cpp (+2-4)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp (+13-2)

diff --git a/compiler-rt/lib/asan/asan_fuchsia.cpp b/compiler-rt/lib/asan/asan_fuchsia.cpp
index 6876be1dca535..5bdf144723bc7 100644
--- a/compiler-rt/lib/asan/asan_fuchsia.cpp
+++ b/compiler-rt/lib/asan/asan_fuchsia.cpp
@@ -32,11 +32,9 @@ void EarlySanitizerInit() {}
 
 namespace __asan {
 
-// The system already set up the shadow memory for us.
-// __sanitizer::GetMaxUserVirtualAddress has already been called by
-// AsanInitInternal->InitializeHighMemEnd (asan_rtl.cpp).
-// Just do some additional sanity checks here.
 void InitializeShadowMemory() {
+  InitShadowBounds();
+
   if (Verbosity())
     PrintAddressSpaceLayout();
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
index 625f30ceca1c2..c8250b8135f28 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
@@ -119,9 +119,20 @@ sanitizer_shadow_bounds_t ShadowBounds;
 
 void InitShadowBounds() { ShadowBounds = __sanitizer_shadow_bounds(); }
 
+// TODO(leonardchan): It's not immediately clear from a user perspective if
+// `GetMaxUserVirtualAddress` should be called exatly once on runtime startup
+// or can be called multiple times. Currently it looks like most instances of
+// `GetMaxUserVirtualAddress` are meant to be called once, but if someone
+// decides to call this multiple times in the future, we should have a separate
+// function that's ok to call multiple times. Ideally we would just invoke this
+// syscall once. Also for Fuchsia, this syscall technically gets invoked twice
+// since `__sanitizer_shadow_bounds` also invokes this syscall under the hood.
 uptr GetMaxUserVirtualAddress() {
-  InitShadowBounds();
-  return ShadowBounds.memory_limit - 1;
+  zx_info_vmar_t info;
+  zx_status_t status = _zx_object_get_info(
+      __zircon_vmar_root_self, ZX_INFO_VMAR, &info, sizeof(info), NULL, NULL);
+  CHECK_EQ(status, ZX_OK);
+  return info.base + info.len - 1;
 }
 
 uptr GetMaxVirtualAddress() { return GetMaxUserVirtualAddress(); }

[frobtech]

Now what calls InitShadowBounds?

[frobtech]

Ah, I see it added above now for asan. As long as that is certainly early enough, this should be OK. It's far from obvious that this is the only place that needs to call this, however. Comments are appropriate.

[PiJoules]

Added some comments. This and hwasan are the only places that need this rn, but hwasan already calls this during its init path.

[thurstond]

Please clarify in title that change is only for Fuchsia.

(P.S. sorry for writing the culprit patch #152428!)

[frobtech]

Where is this symbol declared?
It's not part of the Fuchsia libc API.

[PiJoules]

Updated. This should be _zx_vmar_root_self().

[PiJoules]

Please clarify in title that change is only for Fuchsia.

(P.S. sorry for writing the culprit patch #152428!)

No worries. Updated title

[thurstond]

I don't know any of the Zircon details but LGTM from my perspective on the sanitizers team since this is a Fuchsia-specific change.

[PiJoules]

@frobtech how does this patch look to you?