Fix uses of DW_CFA_restore_state in assembly code.
The specification does not define how DW_CFA_restore_state affects the CFA.
There is no de-facto consensus on the expected behaviour between tools and
sometimes even the same tool might differ for different architectures.
Therefore we must always explicitly set the CFA after the state restore.
Bug: 31975598
Bug: 140407634
Test: "test.py -r --gcstress --optimizing --host" + extra backtrace checks.
Test: "objdump -Wf" and manually check that all restores match saves.
Change-Id: Ic1547149f2159e77887d48dedd4afb7e000066e8
diff --git a/runtime/arch/x86/asm_support_x86.S b/runtime/arch/x86/asm_support_x86.S
index cd5ebd7..8f43cc8 100644
--- a/runtime/arch/x86/asm_support_x86.S
+++ b/runtime/arch/x86/asm_support_x86.S
@@ -77,8 +77,12 @@
#define CFI_DEF_CFA_REGISTER(reg) .cfi_def_cfa_register reg
#define CFI_RESTORE(reg) .cfi_restore reg
#define CFI_REL_OFFSET(reg,size) .cfi_rel_offset reg,size
- #define CFI_RESTORE_STATE .cfi_restore_state
#define CFI_REMEMBER_STATE .cfi_remember_state
+ // The spec is not clear whether the CFA is part of the saved state and tools
+ // differ in the behaviour, so explicitly set the CFA to avoid any ambiguity.
+ // The restored CFA state should match the CFA state during CFI_REMEMBER_STATE.
+ // `objdump -Wf libart.so | egrep "_cfa|_state"` is useful to audit the opcodes.
+ #define CFI_RESTORE_STATE_AND_DEF_CFA(reg,off) .cfi_restore_state .cfi_def_cfa reg,off
#define CFI_ESCAPE(...) .cfi_escape __VA_ARGS__
#else
// Mac OS' doesn't like cfi_* directives.
@@ -89,8 +93,8 @@
#define CFI_DEF_CFA_REGISTER(reg)
#define CFI_RESTORE(reg)
#define CFI_REL_OFFSET(reg,size)
- #define CFI_RESTORE_STATE
#define CFI_REMEMBER_STATE
+ #define CFI_RESTORE_STATE_AND_DEF_CFA(off)
#define CFI_ESCAPE(...)
#endif
diff --git a/runtime/arch/x86/memcmp16_x86.S b/runtime/arch/x86/memcmp16_x86.S
index a315a37..bd33a62 100644
--- a/runtime/arch/x86/memcmp16_x86.S
+++ b/runtime/arch/x86/memcmp16_x86.S
@@ -40,7 +40,7 @@
#define BLK2 BLK1+4
#define LEN BLK2+4
#define RETURN_END POP (%edi); POP (%esi); POP (%ebx); ret
-#define RETURN RETURN_END; CFI_RESTORE_STATE; CFI_REMEMBER_STATE
+#define RETURN RETURN_END; CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16); CFI_REMEMBER_STATE
DEFINE_FUNCTION MEMCMP
movl LEN(%esp), %ecx
@@ -131,7 +131,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_0_gobble):
@@ -177,7 +177,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_2):
@@ -207,7 +207,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_2_gobble):
@@ -260,7 +260,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_4):
@@ -290,7 +290,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_4_gobble):
@@ -343,7 +343,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_6):
@@ -373,7 +373,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_6_gobble):
@@ -426,7 +426,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_8):
@@ -456,7 +456,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_8_gobble):
@@ -509,7 +509,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_10):
@@ -539,7 +539,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_10_gobble):
@@ -592,7 +592,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_12):
@@ -622,7 +622,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_12_gobble):
@@ -675,7 +675,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_14):
@@ -705,7 +705,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(shr_14_gobble):
@@ -758,7 +758,7 @@
POP (%esi)
jmp L(less48bytes)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
CFI_REMEMBER_STATE
.p2align 4
L(exit):
diff --git a/runtime/arch/x86/quick_entrypoints_x86.S b/runtime/arch/x86/quick_entrypoints_x86.S
index b8ccd8e..1819f57 100644
--- a/runtime/arch/x86/quick_entrypoints_x86.S
+++ b/runtime/arch/x86/quick_entrypoints_x86.S
@@ -944,8 +944,7 @@
CFI_REMEMBER_STATE
RESTORE_SAVE_EVERYTHING_FRAME_KEEP_EAX // restore frame up to return address
ret // return
- CFI_RESTORE_STATE
- CFI_DEF_CFA(esp, FRAME_SIZE_SAVE_EVERYTHING) // workaround for clang bug: 31975598
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, FRAME_SIZE_SAVE_EVERYTHING)
1:
DELIVER_PENDING_EXCEPTION_FRAME_READY
END_FUNCTION VAR(c_name)
@@ -1836,7 +1835,7 @@
POP ESI
POP EDI
jmp *ART_METHOD_QUICK_CODE_OFFSET_32(%eax)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
.Limt_table_next_entry:
// If the entry is null, the interface method is not in the ImtConflictTable.
cmpl LITERAL(0), 0(%edi)
@@ -1852,7 +1851,7 @@
POP ESI
POP EDI
INVOKE_TRAMPOLINE_BODY artInvokeInterfaceTrampoline
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(esp, 16)
.Limt_conflict_trampoline_dex_cache_miss:
// We're not creating a proper runtime method frame here,
// artLookupResolvedMethod() is not allowed to walk the stack.
@@ -2445,8 +2444,7 @@
movss %xmm0, (%ecx) // Store the floating point result
ret
.Losr_entry:
- CFI_RESTORE_STATE
- CFI_DEF_CFA(ebp, SAVE_SIZE) // CFA = ebp + SAVE_SIZE
+ CFI_RESTORE_STATE_AND_DEF_CFA(ebp, SAVE_SIZE) // CFA = ebp + SAVE_SIZE
subl LITERAL(4), %ecx // Given stack size contains pushed frame pointer, substract it.
subl %ecx, %esp
mov %esp, %edi // EDI = beginning of stack
diff --git a/runtime/arch/x86_64/asm_support_x86_64.S b/runtime/arch/x86_64/asm_support_x86_64.S
index 6b09a6e..b6a64b6 100644
--- a/runtime/arch/x86_64/asm_support_x86_64.S
+++ b/runtime/arch/x86_64/asm_support_x86_64.S
@@ -76,8 +76,12 @@
#define CFI_DEF_CFA_REGISTER(reg) .cfi_def_cfa_register reg
#define CFI_RESTORE(reg) .cfi_restore reg
#define CFI_REL_OFFSET(reg,size) .cfi_rel_offset reg,size
- #define CFI_RESTORE_STATE .cfi_restore_state
#define CFI_REMEMBER_STATE .cfi_remember_state
+ // The spec is not clear whether the CFA is part of the saved state and tools
+ // differ in the behaviour, so explicitly set the CFA to avoid any ambiguity.
+ // The restored CFA state should match the CFA state during CFI_REMEMBER_STATE.
+ // `objdump -Wf libart.so | egrep "_cfa|_state"` is useful to audit the opcodes.
+ #define CFI_RESTORE_STATE_AND_DEF_CFA(reg,off) .cfi_restore_state .cfi_def_cfa reg,off
#else
// Mac OS' doesn't like cfi_* directives.
#define CFI_STARTPROC
@@ -87,8 +91,8 @@
#define CFI_DEF_CFA_REGISTER(reg)
#define CFI_RESTORE(reg)
#define CFI_REL_OFFSET(reg,size)
- #define CFI_RESTORE_STATE
#define CFI_REMEMBER_STATE
+ #define CFI_RESTORE_STATE_AND_DEF_CFA(off)
#endif
// Symbols.
diff --git a/runtime/arch/x86_64/quick_entrypoints_x86_64.S b/runtime/arch/x86_64/quick_entrypoints_x86_64.S
index 1177477..e1e9dcc 100644
--- a/runtime/arch/x86_64/quick_entrypoints_x86_64.S
+++ b/runtime/arch/x86_64/quick_entrypoints_x86_64.S
@@ -967,8 +967,7 @@
CFI_REMEMBER_STATE
RESTORE_SAVE_EVERYTHING_FRAME_KEEP_RAX // restore frame up to return address
ret
- CFI_RESTORE_STATE
- CFI_DEF_CFA(rsp, FRAME_SIZE_SAVE_EVERYTHING) // workaround for clang bug: 31975598
+ CFI_RESTORE_STATE_AND_DEF_CFA(rsp, FRAME_SIZE_SAVE_EVERYTHING)
1:
DELIVER_PENDING_EXCEPTION_FRAME_READY
END_FUNCTION VAR(c_name)
@@ -1428,7 +1427,7 @@
addq LITERAL(24), %rsp // pop arguments
CFI_ADJUST_CFA_OFFSET(-24)
ret
- CFI_RESTORE_STATE // Reset unwind info so following code unwinds.
+ CFI_RESTORE_STATE_AND_DEF_CFA(rsp, 64) // Reset unwind info so following code unwinds.
.Lthrow_class_cast_exception:
RESTORE_FP_CALLEE_SAVE_FRAME
@@ -1690,7 +1689,7 @@
CFI_REMEMBER_STATE
POP rdx
jmp *ART_METHOD_QUICK_CODE_OFFSET_64(%rdi)
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(rsp, 16)
.Limt_table_next_entry:
// If the entry is null, the interface method is not in the ImtConflictTable.
cmpq LITERAL(0), 0(%rdi)
@@ -1705,7 +1704,7 @@
POP rdx
movq %rax, %rdi // Load interface method
INVOKE_TRAMPOLINE_BODY artInvokeInterfaceTrampoline
- CFI_RESTORE_STATE
+ CFI_RESTORE_STATE_AND_DEF_CFA(rsp, 16)
.Limt_conflict_trampoline_dex_cache_miss:
// We're not creating a proper runtime method frame here,
// artLookupResolvedMethod() is not allowed to walk the stack.
@@ -2419,8 +2418,9 @@
movss %xmm0, (%rcx) // Store the floating point result.
ret
.Losr_entry:
- CFI_RESTORE_STATE // Restore CFI state; however, since the call has pushed the
- CFI_DEF_CFA_REGISTER(rbp) // return address we need to switch the CFA register to RBP.
+ CFI_RESTORE_STATE_AND_DEF_CFA(rsp, 80)
+ // Since the call has pushed the return address we need to switch the CFA register to RBP.
+ CFI_DEF_CFA_REGISTER(rbp)
subl LITERAL(8), %ecx // Given stack size contains pushed frame pointer, substract it.
subq %rcx, %rsp