[hwasan] Show sp in register dump.

[fmayer]

Format test properly.

[fmayer]

Require aarch64 rather than XFAIL x86.

[eugenis]

compiler-rt/lib/hwasan/hwasan_report.cpp

657

I'd rather put it in the last line of the dump, after x30.

compiler-rt/test/hwasan/TestCases/stack-pointer-report.c

13 ↗

(On Diff #353989)

I'd rather copy sp into a local variable and print it right here. It is not impossible that x11 would be in use at this point. SP does not normally change within a function (unless you do alloca or other weird things), so the copy does not have to be immediately before the fault.

14 ↗

(On Diff #353989)

this volatile does not do anything AFAIK - you'd need "z" itself to be volatile to force the memory access. This does not really matter at -O0 but still.

[eugenis]

compiler-rt/test/hwasan/TestCases/stack-pointer-report.c

13 ↗

(On Diff #353989)

In fact, update register-dump-read.c test instead of adding a new one.

[hctim]

compiler-rt/test/hwasan/TestCases/stack-pointer-report.c

13 ↗

(On Diff #353989)

-ffixed-x11 is another option but I like Evgenii's suggestion better.

[fmayer]

Change format, use existing test.

[fmayer]

Thanks!

compiler-rt/test/hwasan/TestCases/stack-pointer-report.c

13 ↗

(On Diff #353989)

I added it to register-dump-read.c, but used a register (and fixed-x11), because that is how the whole test is structured and it seemed simpler this way.

[hctim]

LGTM w/ nit.

compiler-rt/lib/hwasan/hwasan_report.cpp

678

I think (and agree with) @eugenis meant to be on the same line as x30.

[fmayer]

Move sp to last line rather than its own.

[eugenis]

LGTM

compiler-rt/lib/hwasan/hwasan_report.cpp

void ReportTagMismatch(StackTrace *stack, uptr tagged_addr, uptr access_size,
   ReportErrorSummary(bug_type, stack);
 }
 
 // See the frame breakdown defined in __hwasan_tag_mismatch (from
 // hwasan_tag_mismatch_aarch64.S).
 void ReportRegisters(uptr *frame, uptr pc) {
   Printf("Registers where the failure occurred (pc %p):\n", pc);
 
   // We explicitly print a single line (4 registers/line) each iteration to
   // reduce the amount of logcat error messages printed. Each Printf() will
   // result in a new logcat line, irrespective of whether a newline is present,
   // and so we wish to reduce the number of Printf() calls we have to make.
   Printf("    x0  %016llx  x1  %016llx  x2  %016llx  x3  %016llx\n",
        frame[0], frame[1], frame[2], frame[3]);
   Printf("    x4  %016llx  x5  %016llx  x6  %016llx  x7  %016llx\n",
        frame[4], frame[5], frame[6], frame[7]);
   Printf("    x8  %016llx  x9  %016llx  x10 %016llx  x11 %016llx\n",
        frame[8], frame[9], frame[10], frame[11]);
   Printf("    x12 %016llx  x13 %016llx  x14 %016llx  x15 %016llx\n",
        frame[12], frame[13], frame[14], frame[15]);
   Printf("    x16 %016llx  x17 %016llx  x18 %016llx  x19 %016llx\n",
        frame[16], frame[17], frame[18], frame[19]);
   Printf("    x20 %016llx  x21 %016llx  x22 %016llx  x23 %016llx\n",
        frame[20], frame[21], frame[22], frame[23]);
   Printf("    x24 %016llx  x25 %016llx  x26 %016llx  x27 %016llx\n",
        frame[24], frame[25], frame[26], frame[27]);
-  Printf("    x28 %016llx  x29 %016llx  x30 %016llx\n",
-       frame[28], frame[29], frame[30]);
+  // hwasan_check* reduces the stack pointer by 256, then __hwasan_tag_mismatch
+  // passes it to this function.
+  Printf("    x28 %016llx  x29 %016llx  x30 %016llx   sp %016llx\n", frame[28],
+         frame[29], frame[30], reinterpret_cast<u8 *>(frame) + 256);
 }
 
 }  // namespace __hwasan
 
 void __hwasan_set_error_report_callback(void (*callback)(const char *)) {
   __hwasan::ScopedReport::SetErrorReportCallback(callback);
 }

compiler-rt/test/hwasan/TestCases/register-dump-read.c

-// RUN: %clang_hwasan -ffixed-x10 -ffixed-x23 -ffixed-x27 -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: %clang_hwasan -ffixed-x10 -ffixed-x23 -ffixed-x27 -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: %clang_hwasan -ffixed-x10 -ffixed-x23 -ffixed-x27 -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: %clang_hwasan -ffixed-x10 -ffixed-x23 -ffixed-x27 -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_hwasan -ffixed-x10 -ffixed-x11 -ffixed-x23 -ffixed-x27 -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_hwasan -ffixed-x10 -ffixed-x11 -ffixed-x23 -ffixed-x27 -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_hwasan -ffixed-x10 -ffixed-x11 -ffixed-x23 -ffixed-x27 -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_hwasan -ffixed-x10 -ffixed-x11 -ffixed-x23 -ffixed-x27 -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
 // REQUIRES: aarch64-target-arch
 
-// RUN: %clang_hwasan -ffixed-x10 -ffixed-x23 -ffixed-x27 -O2 %s -o %t && not %env_hwasan_opts=fast_unwind_on_fatal=true %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: %clang_hwasan -ffixed-x10 -ffixed-x23 -ffixed-x27 -O2 %s -o %t && not %env_hwasan_opts=fast_unwind_on_fatal=false %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_hwasan -ffixed-x10 -ffixed-x11 -ffixed-x23 -ffixed-x27 -O2 %s -o %t && not %env_hwasan_opts=fast_unwind_on_fatal=true %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_hwasan -ffixed-x10 -ffixed-x11 -ffixed-x23 -ffixed-x27 -O2 %s -o %t && not %env_hwasan_opts=fast_unwind_on_fatal=false %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
 
 #include <stdlib.h>
 #include <stdio.h>
 #include <sanitizer/hwasan_interface.h>
 
 int main() {
   __hwasan_enable_allocator_tagging();
   char * volatile x = (char*) malloc(10);
   asm volatile("mov x10, #0x2222\n"
                "mov x23, #0x3333\n"
-               "mov x27, #0x4444\n");
+               "mov x27, #0x4444\n"
+               "mov x11, sp\n");
   return x[16];
 
   // CHECK: ERROR: HWAddressSanitizer:
   // CHECK: #0 {{.*}} in main {{.*}}register-dump-read.c:[[@LINE-3]]
 
   // Developer note: FileCheck really doesn't like when you have a regex that
   // ends with a '}' character, e.g. the regex "[0-9]{10}" will fail, because
   // the closing '}' fails as an "unbalanced regex". We work around this by
   // encasing the trailing space after a register, or the end-of-line specifier.
 
   // CHECK: Registers where the failure occurred
   // CHECK-NEXT: x0{{[ ]+[0-9a-f]{16}[ ]}}x1{{[ ]+[0-9a-f]{16}[ ]}}x2{{[ ]+[0-9a-f]{16}[ ]}}x3{{[ ]+[0-9a-f]{16}$}}
   // CHECK-NEXT: x4{{[ ]+[0-9a-f]{16}[ ]}}x5{{[ ]+[0-9a-f]{16}[ ]}}x6{{[ ]+[0-9a-f]{16}[ ]}}x7{{[ ]+[0-9a-f]{16}$}}
   // CHECK-NEXT: x8{{[ ]+[0-9a-f]{16}[ ]}}x9{{[ ]+[0-9a-f]{16}[ ]}}
   // CHECK-SAME: x10 0000000000002222
-  // CHECK-SAME: x11{{[ ]+[0-9a-f]{16}$}}
+  // CHECK-SAME: x11[[STACK:[ ]+[0-9a-f]{16}$]]
   // CHECK-NEXT: x12{{[ ]+[0-9a-f]{16}[ ]}}x13{{[ ]+[0-9a-f]{16}[ ]}}x14{{[ ]+[0-9a-f]{16}[ ]}}x15{{[ ]+[0-9a-f]{16}$}}
   // CHECK-NEXT: x16{{[ ]+[0-9a-f]{16}[ ]}}x17{{[ ]+[0-9a-f]{16}[ ]}}x18{{[ ]+[0-9a-f]{16}[ ]}}x19{{[ ]+[0-9a-f]{16}$}}
   // CHECK-NEXT: x20{{[ ]+[0-9a-f]{16}[ ]}}x21{{[ ]+[0-9a-f]{16}[ ]}}x22{{[ ]+[0-9a-f]{16}[ ]}}
   // CHECK-SAME: x23 0000000000003333{{$}}
   // CHECK-NEXT: x24{{[ ]+[0-9a-f]{16}[ ]}}x25{{[ ]+[0-9a-f]{16}[ ]}}x26{{[ ]+[0-9a-f]{16}[ ]}}
   // CHECK-SAME: x27 0000000000004444
-  // CHECK-NEXT: x28{{[ ]+[0-9a-f]{16}[ ]}}x29{{[ ]+[0-9a-f]{16}[ ]}}x30{{[ ]+[0-9a-f]{16}$}}
+  // CHECK-NEXT: x28{{[ ]+[0-9a-f]{16}[ ]}}x29{{[ ]+[0-9a-f]{16}[ ]}}x30{{[ ]+[0-9a-f]{16}[ ]}}
+  // CHECK-SAME: sp{{.*}}[[STACK]]
 }