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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added a comment.

I've checked this on Android, the test passes.
Indeed, you'd need Android 10, or an AOSP master branch build from 2019 to run this.

Compiler-rt tests can be difficult to setup on Android. LLVM has an unofficial GN build system where this works out of the box, check llvm/utils/gn/README.rst if you feel like it; setting android_ndk_path and android_serial_for_testing should be enough to get the tests running, but you would also need to update the build file under llvm/utils/gn/secondary to add the new source file to a list.



================
Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:243
+typedef struct __jmp_buf_tag jmp_buf[1];
+typedef struct __jmp_buf_tag sigjmp_buf[1];
+
----------------
To make sure that we do not overflow the platform's jmp_buf, could you move this definition to ex. hwasan.h, then add a compile-time check that  its sizeof is <= than the system's? See CHECK_TYPE_SIZE macro in sanitizer_platform_limits_posix.h.

We don't allow system includes in this file because then the signatures of the interceptor wrappers would need to match system headers exactly, and that's not always possible across multiple libc versions.



================
Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:258
+static void __attribute__((always_inline))
+__hwasan_internal_longjmp(__jmp_buf env, int retval) {
+  // Clear all memory tags on the stack between here and where we're going.
----------------
We use CamelCase w/o underscores for internal functions.


================
Comment at: compiler-rt/lib/hwasan/hwasan_setjmp.S:99
+// We do not need executable stack.
+NO_EXEC_STACK_DIRECTIVE
+#endif
----------------
NO_EXEC_STACK_DIRECTIVE should go below #ifdef - otherwise you might get an empty source file that suppressed noexecstack.

Clang assembler defaults to non-executable stack when targeting Android, but other platforms may not be so lucky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69045