From 50a677e9b791bbf34eaa9effca231a036a284ce8 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Fri, 17 Jul 2020 00:18:50 +0200 Subject: [PATCH] arm: use CFI directives for stack usage Call Frame Information is stored in the .debug_frame section and is used by debuggers for unwinding. For assembly, this information is not known. Debuggers will normally use heuristics to figure out the parent function in the absence of call frame information. This usually works fine, but is not enough for determining stack sizes. Instead, I hardcoded the stack size information in stacksize/stacksize.go, which is somewhat fragile. This change uses CFI assembly directives to store this information instead of hardcoding it. This change also fixes the following error message that would appear in GDB: Backtrace stopped: previous frame identical to this frame (corrupt stack?) More information on CFI: * https://sourceware.org/binutils/docs/as/CFI-directives.html * https://www.imperialviolet.org/2017/01/18/cfi.html --- src/runtime/scheduler_cortexm.S | 30 ++++++++++++++++++++++++++++++ stacksize/dwarf.go | 13 +++++++++++-- stacksize/stacksize.go | 8 -------- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/runtime/scheduler_cortexm.S b/src/runtime/scheduler_cortexm.S index 9b852714..a0485b38 100644 --- a/src/runtime/scheduler_cortexm.S +++ b/src/runtime/scheduler_cortexm.S @@ -1,13 +1,22 @@ +// Only generate .debug_frame, don't generate .eh_frame. +.cfi_sections .debug_frame + .section .text.tinygo_startTask .global tinygo_startTask .type tinygo_startTask, %function tinygo_startTask: + .cfi_startproc // Small assembly stub for starting a goroutine. This is already run on the // new stack, with the callee-saved registers already loaded. // Most importantly, r4 contains the pc of the to-be-started function and r5 // contains the only argument it is given. Multiple arguments are packed // into one by storing them in a new allocation. + // Indicate to the unwinder that there is nothing to unwind, this is the + // root frame. It avoids the following (bogus) error message in GDB: + // Backtrace stopped: previous frame identical to this frame (corrupt stack?) + .cfi_undefined lr + // Set the first argument of the goroutine start wrapper, which contains all // the arguments. mov r0, r5 @@ -18,15 +27,19 @@ tinygo_startTask: // After return, exit this goroutine. This is a tail call. bl tinygo_pause + .cfi_endproc .size tinygo_startTask, .-tinygo_startTask .section .text.tinygo_getSystemStackPointer .global tinygo_getSystemStackPointer .type tinygo_getSystemStackPointer, %function tinygo_getSystemStackPointer: + .cfi_startproc // The system stack pointer is always stored in the MSP register. mrs r0, MSP bx lr + .cfi_endproc +.size tinygo_getSystemStackPointer, .-tinygo_getSystemStackPointer // switchToScheduler and switchToTask are also in the same section, to make sure @@ -36,6 +49,7 @@ tinygo_getSystemStackPointer: .global tinygo_switchToScheduler .type tinygo_switchToScheduler, %function tinygo_switchToScheduler: + .cfi_startproc // r0 = sp *uintptr // Currently on the task stack (SP=PSP). We need to store the position on @@ -45,11 +59,13 @@ tinygo_switchToScheduler: str r1, [r0] b tinygo_swapTask + .cfi_endproc .size tinygo_switchToScheduler, .-tinygo_switchToScheduler .global tinygo_switchToTask .type tinygo_switchToTask, %function tinygo_switchToTask: + .cfi_startproc // r0 = sp uintptr // Currently on the scheduler stack (SP=MSP). We'll have to update the PSP, @@ -58,11 +74,13 @@ tinygo_switchToTask: // Continue executing in the swapTask function, which swaps the stack // pointer. + .cfi_endproc .size tinygo_switchToTask, .-tinygo_switchToTask .global tinygo_swapTask .type tinygo_swapTask, %function tinygo_swapTask: + .cfi_startproc // This function stores the current register state to the stack, switches to // the other stack (MSP/PSP), and loads the register state from the other // stack. Apart from saving and restoring all relevant callee-saved @@ -84,13 +102,16 @@ tinygo_swapTask: // invocation of swapTask). #if defined(__thumb2__) push {r4-r11, lr} + .cfi_def_cfa_offset 9*4 #else mov r0, r8 mov r1, r9 mov r2, r10 mov r3, r11 push {r0-r3, lr} + .cfi_def_cfa_offset 5*4 push {r4-r7} + .cfi_def_cfa_offset 9*4 #endif // Switch the stack. This could either switch from PSP to MSP, or from MSP @@ -107,29 +128,36 @@ tinygo_swapTask: pop {r4-r11, pc} #else pop {r4-r7} + .cfi_def_cfa_offset 5*9 pop {r0-r3} + .cfi_def_cfa_offset 1*9 mov r8, r0 mov r9, r1 mov r10, r2 mov r11, r3 pop {pc} #endif + .cfi_endproc .size tinygo_swapTask, .-tinygo_swapTask .section .text.tinygo_scanCurrentStack .global tinygo_scanCurrentStack .type tinygo_scanCurrentStack, %function tinygo_scanCurrentStack: + .cfi_startproc // Save callee-saved registers onto the stack. #if defined(__thumb2__) push {r4-r11, lr} + .cfi_def_cfa_offset 9*4 #else mov r0, r8 mov r1, r9 mov r2, r10 mov r3, r11 push {r0-r3, lr} + .cfi_def_cfa_offset 5*4 push {r4-r7} + .cfi_def_cfa_offset 4*4 #endif // Scan the stack. @@ -138,5 +166,7 @@ tinygo_scanCurrentStack: // Restore stack state and return. add sp, #32 + .cfi_def_cfa_offset 1*4 pop {pc} + .cfi_endproc .size tinygo_scanCurrentStack, .-tinygo_scanCurrentStack diff --git a/stacksize/dwarf.go b/stacksize/dwarf.go index d913186f..6186a8c2 100644 --- a/stacksize/dwarf.go +++ b/stacksize/dwarf.go @@ -207,6 +207,15 @@ func (fi *frameInfo) exec(bytecode []byte) ([]frameInfoLine, error) { switch lowBits { case 0: // DW_CFA_nop // no operation + case 0x07: // DW_CFA_undefined + // Marks a single register as undefined. This is used to stop + // unwinding in tinygo_startTask using: + // .cfi_undefined lr + // Ignore this directive. + _, err := readULEB128(r) + if err != nil { + return nil, err + } case 0x0c: // DW_CFA_def_cfa register, err := readULEB128(r) if err != nil { @@ -225,10 +234,10 @@ func (fi *frameInfo) exec(bytecode []byte) ([]frameInfoLine, error) { } fi.cfaOffset = offset default: - return nil, fmt.Errorf("could not decode .debug_frame bytecode op 0x%x", op) + return nil, fmt.Errorf("could not decode .debug_frame bytecode op 0x%x (for address 0x%x)", op, fi.loc) } default: - return nil, fmt.Errorf("could not decode .debug_frame bytecode op 0x%x", op) + return nil, fmt.Errorf("could not decode .debug_frame bytecode op 0x%x (for address 0x%x)", op, fi.loc) } } } diff --git a/stacksize/stacksize.go b/stacksize/stacksize.go index da9143b2..4161dc2f 100644 --- a/stacksize/stacksize.go +++ b/stacksize/stacksize.go @@ -181,14 +181,6 @@ func CallGraph(f *elf.File, callsIndirectFunction []string) (map[string][]*CallN switch f.Machine { case elf.EM_ARM: knownFrameSizes := map[string]uint64{ - // implemented in assembly in TinyGo - "tinygo_startTask": 0, // thunk - "tinygo_getSystemStackPointer": 0, // getter - "tinygo_switchToScheduler": 0, // calls tinygo_swapTask - "tinygo_switchToTask": 0, // calls tinygo_swapTask - "tinygo_swapTask": 9 * 4, // 9 registers saved - "tinygo_scanCurrentStack": 9 * 4, // 9 registers saved - // implemented with assembly in compiler-rt "__aeabi_uidivmod": 3 * 4, // 3 registers on thumb1 but 1 register on thumb2 }