[RISCV][Asan] Use dynamic shadow offset to make it work on different width of virtual-memory system.

[kito-cheng]

Summary

The original asan port was support Sv39 only, however Sv48 support has
landed to linux kernel[1] for a while, so we are trying to make it support
both Sv39 and Sv48, the key point is we need to use a dynamic offset for the
shadow region, this bring extra runtime overhead, but compare to other
overhead in ASan, this should be acceptable.

[1] https://www.phoronix.com/news/Linux-5.17-RISC-V-sv48

[kito-cheng]

Changes:

Fix marco condition.

[shabnam4b]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

310

Just out of curiosity, why can't we modify the code for SV57 here which I assume works for Sv48 and SV39? Maybe there is no way to test it?
I ran this patch on Qemu as well as Docker (Qemu user) and it worked.
The only thing on testing with docker was when the code does not have a bug, it prints the output but it generates a log for a fatal error:

root@74c2d5eadeed:/llvm-project/build# ./nobug
string is: Hello world!
==76543==LeakSanitizer has encountered a fatal error.
==76543==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
==76543==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)

[shabnam4b]

Folks, please let me know in case I can do anything to make this patch merged as soon as possible. I can run tests with Qemu, docker and an unmatched board.

[vitalybuka]

LGTM

[MaskRay]

The original asan port was support Sv39 only, however Sv48 support has landed to linux kernel[1] for a while, so we are trying to make it support both Sv39 and Sv48,

Supporting too many address space bits can introduce significant hidden maintenance burden and performance implication, e.g. using SizeClassAllocator32 for LeakSanitizer (standalone or asan/hwasan integrated) is extremely slow.
See D137666 that AArch64 msan dropped <48-bit VMA. A line needs to be drawn.

[vitalybuka]

In D139827#4038014, @MaskRay wrote:

The original asan port was support Sv39 only, however Sv48 support has landed to linux kernel[1] for a while, so we are trying to make it support both Sv39 and Sv48,

Supporting too many address space bits can introduce significant hidden maintenance burden and performance implication, e.g. using SizeClassAllocator32 for LeakSanitizer (standalone or asan/hwasan integrated) is extremely slow.
See D137666 that AArch64 msan dropped <48-bit VMA. A line needs to be drawn.

+1
Also overhead of dynamic shadow is quite significant, it dependents on hardware. So it would be nice if you can just drop smaller address spaces.

[kito-cheng]

Supporting too many address space bits can introduce significant hidden maintenance burden and performance implication, e.g. using SizeClassAllocator32 for LeakSanitizer (standalone or asan/hwasan integrated) is extremely slow.
See D137666 that AArch64 msan dropped <48-bit VMA. A line needs to be drawn.

As I know there is still many RISC-V core are implement with Sv39 for now, we might able to drop support in future but it's not good timing for now I think.

---

Will defer few more days to commit for continue the discussion.

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

310

Just out of curiosity, why can't we modify the code for SV57 here which I assume works for Sv48 and SV39? Maybe there is no way to test it?

In theory: yes, we can modify it to Sv57 and should work with Sv48 and Sv39, but we have no way to test due to the RISC-V linux didn't support that - and we don't have HW to test that too.

The only thing on testing with docker was when the code does not have a bug, it prints the output but it generates a log for a fatal error:

The message seems cause by docker.

[shabnam4b]

In D139827#4042468, @kito-cheng wrote:

Supporting too many address space bits can introduce significant hidden maintenance burden and performance implication, e.g. using SizeClassAllocator32 for LeakSanitizer (standalone or asan/hwasan integrated) is extremely slow.
See D137666 that AArch64 msan dropped <48-bit VMA. A line needs to be drawn.

As I know there is still many RISC-V core are implement with Sv39 for now, we might able to drop support in future but it's not good timing for now I think.

---

Will defer few more days to commit for continue the discussion.

Ok, thanks for checking! Could you please merge this PR if there is no issue left?

[luismarques]

With the current (unpatched) LLVM tree on an RV64 SV39 host I get this ASAN test failure (for check-asan):

• AddressSanitizer-riscv64-linux :: TestCases/memcmp_test.cpp

With this patch on an RV64 SV48 host I get these test failures:

• AddressSanitizer-riscv64-linux :: TestCases/Linux/syscalls.cpp
• AddressSanitizer-riscv64-linux :: TestCases/Posix/bcmp_test.cpp
• AddressSanitizer-riscv64-linux :: TestCases/strncpy-overflow.cpp

I'll need to double-check if those results are actually correct but...
@kito-cheng Does that match your test results? If not, what do you observe? If it matches, have you investigated the new test failures?

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

1119

The "default is 47-bit for RISCV64" is confusing, after listing "32, 39, 48 and 57". By that do you mean that Linux defaults to SV48? (if so fix 47->48). Or that the sanitizer defaults to a 1 << 47 userspace address range? Or both, or something else?

[kito-cheng]

In D139827#4080074, @luismarques wrote:

With the current (unpatched) LLVM tree on an RV64 SV39 host I get this ASAN test failure (for check-asan):

• AddressSanitizer-riscv64-linux :: TestCases/memcmp_test.cpp

With this patch on an RV64 SV48 host I get these test failures:

• AddressSanitizer-riscv64-linux :: TestCases/Linux/syscalls.cpp
• AddressSanitizer-riscv64-linux :: TestCases/Posix/bcmp_test.cpp
• AddressSanitizer-riscv64-linux :: TestCases/strncpy-overflow.cpp

I'll need to double-check if those results are actually correct but...
@kito-cheng Does that match your test results? If not, what do you observe? If it matches, have you investigated the new test failures?

Just back from Chinese new year vocation :P let me check with my local test environment again, thanks for testing and reporting this.

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

1119

That's my stupid copy-and-paste error from AArch64, and my intention is support multiple address space layout.

[kito-cheng]

Sorry for not updating for a while, we/SiFive gonna to seeking a ultimate solution to work on each different Sv* system, my exception is we still need dynamic shadow sentinel which is what we did for this patch, but it not work well as @luismarques report, so I would like to defer this patch landing for now, and it become high priority work to SiFive, we've allocated dedicated engineer resources to improve the ASan including the ASan for scalable vector stuff and RVV intrinsic, the both work are targeting end of Q2.

[hauhsu]

Hi all,
I just published 3 more reviews:
https://reviews.llvm.org/D152895
https://reviews.llvm.org/D152990
https://reviews.llvm.org/D152991

---

With the 3 new patches, the check-lsan passes in our sv39, sv48, sv57 QEMU system mode test environments.

---

The common check-asan failure cases in the 3 virtual memory modes are

AddressSanitizer-riscv64-linux :: TestCases/Linux/syscalls.cpp
AddressSanitizer-riscv64-linux :: TestCases/Posix/bcmp_test.cpp
AddressSanitizer-riscv64-linux :: TestCases/memcmp_test.cpp
AddressSanitizer-riscv64-linux :: TestCases/log-path_test.cpp

The first 3 failures are because the CHECK statements check specific function names in call stack, like

// CHECK: AddressSanitizer: stack-buffer-overflow  (v)
// CHECK: {{#1.*bcmp}}    <= frame1                            
// CHECK: {{#2.*main}}     <= frame2

But the function QuickCheckForUnpoisonedRegion() is not inlined, so the call stack looks a little bit different:

#0 0x2aac31a59c in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long) .../llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:804:7
#1 0x2aac31a9aa in QuickCheckForUnpoisonedRegion .../llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:850:10
#2 0x2aac31a9aa in bcmp .../llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:817:7
#3 0x2aac3bbca8 in main .../llvm-project/compiler-rt/test/asan/TestCases/Posix/bcmp_test.cpp:13:13
#4 0x3f886e36c0  (/lib/libc.so.6+0x266c0)
#5 0x3f886e376c in __libc_start_main (/lib/libc.so.6+0x2676c)
#6 0x2aac306d52 in _start (.../llvm-project/build/test/asan/RISCV64LinuxConfig/TestCases/Posix/Output/bcmp_test.cpp.tmp+0x26d52)

https://reviews.llvm.org/D152991 looses the check statement to make the tests pass.

For AddressSanitizer-riscv64-linux :: TestCases/log-path_test.cpp failure, one of the test tries to set the log path to /INVALID and expects error.
But since in our QEMU test environment uses root, the path is actually valid.

---

There are 3 additional check-asan failures for sv57:

AddressSanitizer-riscv64-linux :: TestCases/Linux/allocator_oom_test.cpp
AddressSanitizer-riscv64-linux :: TestCases/Linux/auto_memory_profile_test.cpp
AddressSanitizer-riscv64-linux :: TestCases/Linux/quarantine_size_mb.cpp

We are still investigating them. But since the tests seems not so critical and sv57 is rather little users, we decide to propose the reviews for now.

[vitalybuka]

What is the status of this patch?

[kito-cheng]

@vitalybuka this patch is waiting D152895 for make sure LSan is work fine on both Sv39 and Sv48, and D152990 could make it work on Sv57.

This patch has tested ASan on Sv39 and Sv48 by @hauhsu, and all failures found by @luismarques has been addressed by D152991 + D152895.

[palmer-dabbelt]

If I'm reading this right, anything built with ASAN will fail on sv48 or sv57? We're defaulting the mmap() hints to sv48 because we didn't have any failing workloads there (Hotspot and Go fail sv57), but if this fails we should probably default to sv37. Is there a reproducer bouncing around we can put in kselftests?

[kito-cheng]

In D139827#4614214, @palmer-dabbelt wrote:

If I'm reading this right, anything built with ASAN will fail on sv48 or sv57? We're defaulting the mmap() hints to sv48 because we didn't have any failing workloads there (Hotspot and Go fail sv57), but if this fails we should probably default to sv37. Is there a reproducer bouncing around we can put in kselftests?

I guess you might confused by SANITIZER_MMAP_RANGE_SIZE, it a parameter for internal memory allocator, set for larger Sv is always works on smaller Sv but less memory efficiency.

And the original reason fail on the Sv48/Sv57 is because most setting is Sv sensitive (kRISCV64_ShadowOffset64), and we try make it into Sv insensitive by trading few more runtime performance.

compiler-rt/lib/asan/asan_mapping.h

 //
 // Default Linux/MIPS64 mapping:
 // || `[0x4000000000, 0xffffffffff]` || HighMem    ||
 // || `[0x2800000000, 0x3fffffffff]` || HighShadow ||
 // || `[0x2400000000, 0x27ffffffff]` || ShadowGap  ||
 // || `[0x2000000000, 0x23ffffffff]` || LowShadow  ||
 // || `[0x0000000000, 0x1fffffffff]` || LowMem     ||
 //
-// Default Linux/RISCV64 Sv39 mapping:
+// Default Linux/RISCV64 Sv39 mapping with SHADOW_OFFSET == 0xd55550000;
+// (the exact location of SHADOW_OFFSET may vary depending the dynamic probing
+//  by FindDynamicShadowStart).
+//
 // || `[0x1555550000, 0x3fffffffff]` || HighMem    ||
 // || `[0x0fffffa000, 0x1555555fff]` || HighShadow ||
 // || `[0x0effffa000, 0x0fffff9fff]` || ShadowGap  ||
 // || `[0x0d55550000, 0x0effff9fff]` || LowShadow  ||
 // || `[0x0000000000, 0x0d5554ffff]` || LowMem     ||
 //
 // Default Linux/AArch64 (39-bit VMA) mapping:
 // || `[0x2000000000, 0x7fffffffff]` || highmem    ||

 #else
 #  if SANITIZER_IOS
 #    define ASAN_SHADOW_OFFSET_DYNAMIC
 #  elif SANITIZER_APPLE && defined(__aarch64__)
 #    define ASAN_SHADOW_OFFSET_DYNAMIC
 #  elif SANITIZER_FREEBSD && defined(__aarch64__)
 #    define ASAN_SHADOW_OFFSET_CONST 0x0000800000000000
 #  elif SANITIZER_RISCV64
-#    define ASAN_SHADOW_OFFSET_CONST 0x0000000d55550000
+#    define ASAN_SHADOW_OFFSET_DYNAMIC
 #  elif defined(__aarch64__)
 #    define ASAN_SHADOW_OFFSET_CONST 0x0000001000000000
 #  elif defined(__powerpc64__)
 #    define ASAN_SHADOW_OFFSET_CONST 0x0000100000000000
 #  elif defined(__s390x__)
 #    define ASAN_SHADOW_OFFSET_CONST 0x0010000000000000
 #  elif SANITIZER_FREEBSD
 #    define ASAN_SHADOW_OFFSET_CONST 0x0000400000000000

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

 #endif  // SANITIZER_LINUX && !SANITIZER_X32
 }
 #endif  // SANITIZER_WORDSIZE == 32
 
 uptr GetMaxVirtualAddress() {
 #if SANITIZER_NETBSD && defined(__x86_64__)
   return 0x7f7ffffff000ULL;  // (0x00007f8000000000 - PAGE_SIZE)
 #elif SANITIZER_WORDSIZE == 64
-# if defined(__powerpc64__) || defined(__aarch64__) || defined(__loongarch__)
+#    if defined(__powerpc64__) || defined(__aarch64__) || \
+        defined(__loongarch__) || SANITIZER_RISCV64
   // On PowerPC64 we have two different address space layouts: 44- and 46-bit.
   // We somehow need to figure out which one we are using now and choose
   // one of 0x00000fffffffffffUL and 0x00003fffffffffffUL.
   // Note that with 'ulimit -s unlimited' the stack is moved away from the top
   // of the address space, so simply checking the stack address is not enough.
   // This should (does) work for both PowerPC64 Endian modes.
   // Similarly, aarch64 has multiple address space layouts: 39, 42 and 47-bit.
   // loongarch64 also has multiple address space layouts: default is 47-bit.
+  // RISC-V also has multiple address space layouts: 32, 39, 48 and 57,
+  // default is 47-bit for RISCV64.
   return (1ULL << (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1)) - 1;
-#elif SANITIZER_RISCV64
-  return (1ULL << 38) - 1;
-# elif SANITIZER_MIPS64
+#    elif SANITIZER_MIPS64
   return (1ULL << 40) - 1;  // 0x000000ffffffffffUL;
-# elif defined(__s390x__)
+#    elif defined(__s390x__)
   return (1ULL << 53) - 1;  // 0x001fffffffffffffUL;
-#elif defined(__sparc__)
+#    elif defined(__sparc__)
   return ~(uptr)0;
-# else
+#    else
   return (1ULL << 47) - 1;  // 0x00007fffffffffffUL;
-# endif
+#    endif
 #else  // SANITIZER_WORDSIZE == 32
 # if defined(__s390__)
   return (1ULL << 31) - 1;  // 0x7fffffff;
 # else
   return (1ULL << 32) - 1;  // 0xffffffff;
 # endif
 #endif  // SANITIZER_WORDSIZE
 }

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

 // will still work but will consume more memory for TwoLevelByteMap.
 #if defined(__mips__)
 #  if SANITIZER_GO && defined(__mips64)
 #    define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
 #  else
 #    define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 40)
 #  endif
 #elif SANITIZER_RISCV64
-#  define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 38)
+// FIXME: Set to 47 to ensure it works with both Sv39 and Sv48; however,
+// this will not work correctly on Sv57.
+#  define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
 #elif defined(__aarch64__)
 #  if SANITIZER_APPLE
 #    if SANITIZER_OSX || SANITIZER_IOSSIM
 #      define SANITIZER_MMAP_RANGE_SIZE \
         FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
 #    else
 // Darwin iOS/ARM64 has a 36-bit VMA, 64GiB VM
 #      define SANITIZER_MMAP_RANGE_SIZE \

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

 static const uint64_t kLinuxKasan_ShadowOffset64 = 0xdffffc0000000000;
 static const uint64_t kPPC64_ShadowOffset64 = 1ULL << 44;
 static const uint64_t kSystemZ_ShadowOffset64 = 1ULL << 52;
 static const uint64_t kMIPS_ShadowOffsetN32 = 1ULL << 29;
 static const uint64_t kMIPS32_ShadowOffset32 = 0x0aaa0000;
 static const uint64_t kMIPS64_ShadowOffset64 = 1ULL << 37;
 static const uint64_t kAArch64_ShadowOffset64 = 1ULL << 36;
 static const uint64_t kLoongArch64_ShadowOffset64 = 1ULL << 46;
-static const uint64_t kRISCV64_ShadowOffset64 = 0xd55550000;
+static const uint64_t kRISCV64_ShadowOffset64 = kDynamicShadowSentinel;
 static const uint64_t kFreeBSD_ShadowOffset32 = 1ULL << 30;
 static const uint64_t kFreeBSD_ShadowOffset64 = 1ULL << 46;
 static const uint64_t kFreeBSDAArch64_ShadowOffset64 = 1ULL << 47;
 static const uint64_t kFreeBSDKasan_ShadowOffset64 = 0xdffff7c000000000;
 static const uint64_t kNetBSD_ShadowOffset32 = 1ULL << 30;
 static const uint64_t kNetBSD_ShadowOffset64 = 1ULL << 46;
 static const uint64_t kNetBSDKasan_ShadowOffset64 = 0xdfff900000000000;
 static const uint64_t kPS_ShadowOffset64 = 1ULL << 40;