[PATCH] D32202: [XRay][compiler-rt] Fix up CFI annotations and stack alignment

[Keith via Phabricator via llvm-commits]

kpw added a comment.

This may fix our alignment issues. It might be worth mentioning that the stack alignment bug we was orthogonal to the buggy stack alignment code, which I think is mostly used for debugging and exception purposes.

I still think that we're missing a big piece of the puzzle to have stack unwinding support code with exception handling rather than just produce a stacktrace.

It seems to me that the entry trampolines should have directives in the SAVE_REGISTERS macro that specify how an unwinder restores registers when carrying an exception up the stack.

An example directive, from https://sourceware.org/binutils/docs/as/CFI-directives.html

.cfi_offset register, offset ---  Previous value of register is saved at offset offset from CFA.

CFI is all a part of the ABI that I've never dug into before, and I'm still relatively poor at x86 comprehension, so I'd like to make sure more experienced eyes give this at least a spot check.



================
Comment at: lib/xray/xray_trampoline_x86_64.S:19
 .macro SAVE_REGISTERS
-	subq $200, %rsp
-	movupd	%xmm0, 184(%rsp)
-	movupd	%xmm1, 168(%rsp)
-	movupd	%xmm2, 152(%rsp)
-	movupd	%xmm3, 136(%rsp)
-	movupd	%xmm4, 120(%rsp)
-	movupd	%xmm5, 104(%rsp)
-	movupd	%xmm6, 88(%rsp)
-	movupd	%xmm7, 72(%rsp)
-	movq	%rdi, 64(%rsp)
-	movq	%rax, 56(%rsp)
-	movq	%rdx, 48(%rsp)
-	movq	%rsi, 40(%rsp)
-	movq	%rcx, 32(%rsp)
-	movq	%r8, 24(%rsp)
-	movq	%r9, 16(%rsp)
+	subq $184, %rsp
+	movupd	%xmm0, 168(%rsp)
----------------
We're trying to maintain 16 byte stack alignment IIUC. Are we  expecting the stack pointer to be unaligned by an 8 byte offset when this is invoked? Is this expectation due to a callq instruction in the entry sled?




================
Comment at: lib/xray/xray_trampoline_x86_64.S:70
 	SAVE_REGISTERS
+	.cfi_def_cfa_offset 200
 
----------------
I think you can offload your math onto the assembler with .cfi_adjust_cfa_offset


Repository:
  rL LLVM

https://reviews.llvm.org/D32202