[PATCH] D69045: libhwasan interceptor ABI intercept longjmp/setjmp

[Matthew Malcomson via Phabricator via llvm-commits]

mmalcomson added inline comments.


================
Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:224
+
+#if HWASAN_WITH_INTERCEPTORS && defined(__aarch64__)
+/*
----------------
eugenis wrote:
> This won't work on Android, please add && !SANITIZER_ANDROID.
> 
> Just FYI, this is what we do there:
> https://android.googlesource.com/platform/bionic/+/919dc05d66b129ad6f34fad95322efb6de245754/libc/arch-arm64/bionic/setjmp.S#214
> 
Can do, but I don't fully understand -- could you explain why a bit more?

I was thinking that the Android "platform ABI" would handle longjmp, but the Android "interceptor ABI" would have the same missing feature as any other interceptor ABI.

I would have guessed that the feature check in the bionic sources would be essentially checking for the platform ABI -- since if using the interceptor ABI bionic wouldn't know that hwasan is being used.


================
Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:271
+  register void *env_address asm ("x0") = &env[0];
+  asm volatile (
+	"ldp	x19, x20, [%0, #0<<3];"
----------------
eugenis wrote:
> Why can't this asm be replaced with REAL(longjmp)(env, retval) ?
As I understand it, the order that registers are stored in the `jmp_buf` structure is undefined by any architecture ABI.
Hence different longjmp/setjmp implementations are likely to have a different order of registers  (and a quick comparison of the bionic and glibc setjmps seem to confirm this).

Because of this, if using REAL(longjmp), we'd need to also use REAL(setjmp), and when using REAL(setjmp) we wouldn't know where the stack pointer is in the jmp_buf.

Also, some libc's (like glibc) mangle their stack pointer to improve security, and we wouldn't be able to read it even if we did know where it was (e.g. if hard-coded offset different for each platform we compile for or only targeting one libc).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69045/new/

https://reviews.llvm.org/D69045