Revert "[hwasan] Add fixed_shadow_base flag"

[fmayer]

Reverts #73980

This broke static hwasan binaries in Android, for some reason the fixed_shadow_base branch gets taken

[llvmbot]

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

Author: Florian Mayer (fmayer)

Changes

Reverts llvm/llvm-project#73980

This broke static hwasan binaries in Android, for some reason the fixed_shadow_base branch gets taken

---

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

3 Files Affected:

• (modified) compiler-rt/lib/hwasan/hwasan_flags.inc (-7)
• (modified) compiler-rt/lib/hwasan/hwasan_linux.cpp (+2-6)
• (removed) compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c (-76)

diff --git a/compiler-rt/lib/hwasan/hwasan_flags.inc b/compiler-rt/lib/hwasan/hwasan_flags.inc
index 058a0457b9e7f..978fa46b705cb 100644
--- a/compiler-rt/lib/hwasan/hwasan_flags.inc
+++ b/compiler-rt/lib/hwasan/hwasan_flags.inc
@@ -84,10 +84,3 @@ HWASAN_FLAG(bool, malloc_bisect_dump, false,
 // are untagged before the call.
 HWASAN_FLAG(bool, fail_without_syscall_abi, true,
             "Exit if fail to request relaxed syscall ABI.")
-
-HWASAN_FLAG(
-    uptr, fixed_shadow_base, -1,
-    "If not -1, HWASan will attempt to allocate the shadow at this address, "
-    "instead of choosing one dynamically."
-    "Tip: this can be combined with the compiler option, "
-    "-hwasan-mapping-offset, to optimize the instrumentation.")
diff --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp
index e6aa60b324fa7..c254670ee2d48 100644
--- a/compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -106,12 +106,8 @@ static uptr GetHighMemEnd() {
 }
 
 static void InitializeShadowBaseAddress(uptr shadow_size_bytes) {
-  if (flags()->fixed_shadow_base != (uptr)-1) {
-    __hwasan_shadow_memory_dynamic_address = flags()->fixed_shadow_base;
-  } else {
-    __hwasan_shadow_memory_dynamic_address =
-        FindDynamicShadowStart(shadow_size_bytes);
-  }
+  __hwasan_shadow_memory_dynamic_address =
+      FindDynamicShadowStart(shadow_size_bytes);
 }
 
 static void MaybeDieIfNoTaggingAbi(const char *message) {
diff --git a/compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c b/compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c
deleted file mode 100644
index 4ff1d3e64c1d0..0000000000000
--- a/compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c
+++ /dev/null
@@ -1,76 +0,0 @@
-// Test fixed shadow base functionality.
-//
-// Default compiler instrumentation works with any shadow base (dynamic or fixed).
-// RUN: %clang_hwasan %s -o %t && %run %t
-// RUN: %clang_hwasan %s -o %t && HWASAN_OPTIONS=fixed_shadow_base=263878495698944 %run %t
-// RUN: %clang_hwasan %s -o %t && HWASAN_OPTIONS=fixed_shadow_base=4398046511104 %run %t
-//
-// If -hwasan-mapping-offset is set, then the fixed_shadow_base needs to match.
-// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=263878495698944 -o %t && HWASAN_OPTIONS=fixed_shadow_base=263878495698944 %run %t
-// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=4398046511104 -o %t && HWASAN_OPTIONS=fixed_shadow_base=4398046511104 %run %t
-// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=263878495698944 -o %t && HWASAN_OPTIONS=fixed_shadow_base=4398046511104 not %run %t
-// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=4398046511104 -o %t && HWASAN_OPTIONS=fixed_shadow_base=263878495698944 not %run %t
-//
-// Note: if fixed_shadow_base is not set, compiler-rt will dynamically choose a
-// shadow base, which has a tiny but non-zero probability of matching the
-// compiler instrumentation. To avoid test flake, we do not test this case.
-//
-// Assume 48-bit VMA
-// REQUIRES: aarch64-target-arch
-//
-// REQUIRES: Clang
-//
-// UNSUPPORTED: android
-
-#include <assert.h>
-#include <sanitizer/allocator_interface.h>
-#include <sanitizer/hwasan_interface.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/mman.h>
-
-int main() {
-  __hwasan_enable_allocator_tagging();
-
-  // We test that the compiler instrumentation is able to access shadow memory
-  // for many different addresses. If we only test a small number of addresses,
-  // it might work by chance even if the shadow base does not match between the
-  // compiler instrumentation and compiler-rt.
-  void **mmaps[256];
-  // 48-bit VMA
-  for (int i = 0; i < 256; i++) {
-    unsigned long long addr = (i * (1ULL << 40));
-
-    void *p = mmap((void *)addr, 4096, PROT_READ | PROT_WRITE,
-                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-    // We don't use MAP_FIXED, to avoid overwriting critical memory.
-    // However, if we don't get allocated the requested address, it
-    // isn't a useful test.
-    if ((unsigned long long)p != addr) {
-      munmap(p, 4096);
-      mmaps[i] = MAP_FAILED;
-    } else {
-      mmaps[i] = p;
-    }
-  }
-
-  int failures = 0;
-  for (int i = 0; i < 256; i++) {
-    if (mmaps[i] == MAP_FAILED) {
-      failures++;
-    } else {
-      printf("%d %p\n", i, mmaps[i]);
-      munmap(mmaps[i], 4096);
-    }
-  }
-
-  // We expect roughly 17 failures:
-  // - the page at address zero
-  // - 16 failures because the shadow memory takes up 1/16th of the address space
-  // We could also get unlucky e.g., if libraries or binaries are loaded into the
-  // exact addresses where we tried to map.
-  // To avoid test flake, we allow some margin of error.
-  printf("Failed: %d\n", failures);
-  assert(failures < 48);
-  return 0;
-}

[vitalybuka]

@fmayer can you please debug this case. I don't see how that's its possible?

[vitalybuka]

My guess __hwasan_init_static() called before initializing flags

[vitalybuka]

@thurstond
I guess it's the issue for stuff which does not use .preinit_array, like Android
Short fix, please reland with #if NOT ANDROID
Then, we need to fix flag initialization for android, it should happen for __hwasan_init_static() as well.

[vitalybuka]

better #if SANITIZER_CAN_USE_PREINIT_ARRAY

[fmayer]

I think that's incorrect:

#ifndef SANITIZER_CAN_USE_PREINIT_ARRAY
#if (SANITIZER_LINUX || SANITIZER_FUCHSIA || SANITIZER_NETBSD) && !defined(PIC)
#define SANITIZER_CAN_USE_PREINIT_ARRAY 1
// Before Solaris 11.4, .preinit_array is fully supported only with GNU ld.
// FIXME: Check for those conditions.

ANDROID is Linux and does in fact support preinit array, we just don't use it for HWASan.

[vitalybuka]

I think that's incorrect:

#ifndef SANITIZER_CAN_USE_PREINIT_ARRAY
#if (SANITIZER_LINUX || SANITIZER_FUCHSIA || SANITIZER_NETBSD) && !defined(PIC)
#define SANITIZER_CAN_USE_PREINIT_ARRAY 1
// Before Solaris 11.4, .preinit_array is fully supported only with GNU ld.
// FIXME: Check for those conditions.

ANDROID is Linnux and does in fact support preinit array, we just don't use it for HWASan.

Is hwasan dynamic lib? if so .preinit_array can not be used

[fmayer]

I think that's incorrect:

#ifndef SANITIZER_CAN_USE_PREINIT_ARRAY
#if (SANITIZER_LINUX || SANITIZER_FUCHSIA || SANITIZER_NETBSD) && !defined(PIC)
#define SANITIZER_CAN_USE_PREINIT_ARRAY 1
// Before Solaris 11.4, .preinit_array is fully supported only with GNU ld.
// FIXME: Check for those conditions.

ANDROID is Linnux and does in fact support preinit array, we just don't use it for HWASan.

Is hwasan dynamic lib? if so .preinit_array can not be used

In this case we are compiling a static binary, so HWASan is not a dynamic lib