Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/test/asan/TestCases/load_and_store_n.cpp | ||
---|---|---|
59 | Store is a void function. |
compiler-rt/test/asan/TestCases/load_and_store_n.cpp | ||
---|---|---|
30 | Could you tighten this check a little? Ex. you could print the expected "bad access" address in the test code, and then verify that it matches the address printed by asan. Also an interesting edge case to test would be when the access is entirely outside of the global (ex. (char*)&G + 9). |
LGTM w/ nits
compiler-rt/test/asan/TestCases/load_and_store_n.cpp | ||
---|---|---|
37 | Could you also extend this check? I imagine this would say "0 bytes to the right" vs "1 bytes to the right" depending on the case. |
android_compile script is confused by -emit-llvm it looks like. I'd just remove the IR checks from the test.
+ /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only --target=aarch64-linux-android24 --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -stdlib=libc++ -fuse-ld=lld -shared-libasan -O2 -fsanitize-address-outline-instrumentation /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/load_and_store_n.cpp -o - -S -emit-llvm + FileCheck /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/load_and_store_n.cpp --check-prefix=CHECK_REGULAR_LOAD_STORE clang-15: warning: argument unused during compilation: '-fuse-ld=lld' [-Wunused-command-line-argument] Traceback (most recent call last): File "/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py", line 35, in <module> os.rename(output, output + '.real') OSError: [Errno 2] No such file or directory
compiler-rt/test/asan/TestCases/load_and_store_n.cpp | ||
---|---|---|
30 | Should we have IR test on the clang side? |
Can we do this on the ASan error report side (i.e. compiler-rt/lib/asan/asan_errors.cpp) .
diff --git a/compiler-rt/lib/asan/asan_errors.cpp b/compiler-rt/lib/asan/asan_errors.cpp index a22bf130d823..2cdd13df6c2a 100644 --- a/compiler-rt/lib/asan/asan_errors.cpp +++ b/compiler-rt/lib/asan/asan_errors.cpp @@ -409,8 +409,10 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr, bug_descr = "unknown-crash"; if (AddrIsInMem(addr)) { u8 *shadow_addr = (u8 *)MemToShadow(addr); - // If we are accessing 16 bytes, look at the second shadow byte. - if (*shadow_addr == 0 && access_size > ASAN_SHADOW_GRANULARITY) + // If we are accessing 16 bytes or in unaligned accesses, look at the + // second shadow byte. + if (*shadow_addr == 0 && (access_size > ASAN_SHADOW_GRANULARITY || + (addr & (ASAN_SHADOW_GRANULARITY - 1)) != 0)) shadow_addr++; // If we are in the partial right redzone, look at the next shadow byte. if (*shadow_addr > 0 && *shadow_addr < 128) shadow_addr++;
In this way, the error report of the test load_and_store_n.cpp (e.g. %run %t A) will like:
==2884775==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000545bc7 at pc 0x00000050bf95 bp 0x7ffcdcbcb350 sp 0x7ffcdcbcb338 READ of size 2 at 0x000000545bc7 thread T0 ... 0x000000545bc8 is located 0 bytes to the right of global variable 'mem' defined in 'load_and_store_n.cpp:46:16' (0x545bc0) of size 8 SUMMARY: AddressSanitizer: global-buffer-overflow ./load_and_store_n.cpp:51:3 in UNALIGNED_LOAD(void const*) Shadow bytes around the buggy address: 0x0000800a0b20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0b30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0000800a0b70: 00 00 00 00 00 f9 f9 f9[00]f9 f9 f9 f9 f9 f9 f9 0x0000800a0b80: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 0x0000800a0b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800a0bc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
This way the access start address (0x000000545bc7) in this report is more readable.
Could you tighten this check a little? Ex. you could print the expected "bad access" address in the test code, and then verify that it matches the address printed by asan.
Also an interesting edge case to test would be when the access is entirely outside of the global (ex. (char*)&G + 9).