[PATCH] D13670: [sanitizer] Fix ptrace interceptor for aarch64

[Adhemerval Zanella via llvm-commits]

zatrazz added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:2464
@@ +2463,3 @@
+    else if (request == ptrace_setregset || request == ptrace_getregset) {
+      local_iovec = *(__sanitizer_iovec *)data;
+      if (request == ptrace_setregset)
----------------
eugenis wrote:
> Hm, we don't check that __sanitizer_iovec in data is accessible. Would you mind adding a READ_RANGE for that before copying it out?
> 
Right, I will do it.

================
Comment at: test/asan/TestCases/Linux/ptrace.cc:6
@@ -5,3 +5,3 @@
 // RUN: %clangxx_asan -DPOSITIVE -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s
-// REQUIRES: x86_64-supported-target,i386-supported-target
+// REQUIRES: x86_64-supported-target,i386-supported-target,aarch64-supported-target
 
----------------
eugenis wrote:
> This is impossible. You can not build compiler-rt to support all 3 of those at one time.
> UNSUPPORTED: AddressSanitizer-x86_64-linux :: TestCases/Linux/ptrace.cc
> UNSUPPORTED: AddressSanitizer-i386-linux :: TestCases/Linux/ptrace.cc
> 
> Not sure what's the best solution here. Maybe #ifdef-out the entire test on unsupported platforms?
Right, I misunderstood the idea of REQUIRES. If there is no way to conditionally select the requirement (x86_64 or i386 or aarch64) the way I see to enable this test is to remove the 'requires', enable on all the architectures and then XFAIL on the ones that do not have the instrumented ptrace (since the CHECK: will be still being evaluated even if we #ifdef-out the tests). Suggestions?

================
Comment at: test/asan/TestCases/Linux/ptrace.cc:18
@@ +17,3 @@
+#ifdef __aarch64__
+// GLIBC 2.20+ sys/user does not include asm/ptrace.h
+ #include <asm/ptrace.h>
----------------
eugenis wrote:
> Does it mean x86_64 run into the same problem soon?
The REQUIRES issue you noted above in fact showed this is not really necessary for the test itself. I will fix it and also another nits I found in this testcase.


http://reviews.llvm.org/D13670