[PATCH] D45303: shadowcallstack: Make runtime tests compatible with aarch64.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/test/shadowcallstack/libc_support.h:24
+
+__attribute__((noinline)) void scs_fputs_stdout(const char *p) {
+  __asm__ __volatile__(
----------------
vlad.tsyrklevich wrote:
> Attribute also belongs on the definition below
Probably doesn't matter much in this case because we won't be inlining fputs. But I guess there's no harm in doing it.


================
Comment at: compiler-rt/test/shadowcallstack/minimal_runtime.h:39
+
+  // We can't simply return scs_main() because scs_main might have corrupted our
+  // return address for testing purposes (see overflow.c), so we need to exit
----------------
vlad.tsyrklevich wrote:
> Not sure I follow, if scs_main corrupted the RA don't we want to allow it to crash to satisfy the test? print_and_exit() also calls exit() directly so it shouldn't be required in that case.
For aarch64 we don't crash on an RA mismatch, we just return to the address on the SCS. That means that when testing SCS + iterations=12 scs_main will return to main even though the RA on the stack is corrupted. Then when main, a non-SCS function, returns it will read the RA from the corrupted stack and return to it. We need to prevent that from happening because we want to simulate the behaviour when the entire system is built with SCS.


https://reviews.llvm.org/D45303