pkg/proc: fix arm64 linux cgo stacktrace (#3192)

This patch introduces some changes, particularly to arm64SwitchStack
which fixes the test when running on linux/arm64. The changes causes the
same test to fail on darwin/m1 so temporarily keeping both versions.
Next step should be to refactor and unify the two so they both work with
the same function.

Fixes #2340

Documentation/backend_test_health.md

@@ -16,8 +16,6 @@ Tests skipped by each supported backend:
 	* 4 not implemented
 * linux/386/pie skipped = 1
 	* 1 broken
-* linux/arm64 skipped = 1
-	* 1 broken - cgo stacktraces
 * pie skipped = 2
 	* 2 upstream issue - https://github.com/golang/go/issues/29322
 * windows skipped = 5

pkg/proc/amd64_arch.go

@@ -179,7 +179,6 @@ func amd64SwitchStack(it *stackIterator, _ *op.DwarfRegisters) bool {
 		// switch from the system stack back into the goroutine stack
 		// Since we are going backwards on the stack here we see the transition
 		// as goroutine stack -> system stack.
-
 		if it.top || it.systemstack {
 			return false
 		}
@@ -198,6 +197,7 @@ func amd64SwitchStack(it *stackIterator, _ *op.DwarfRegisters) bool {
 		it.pc = frameOnSystemStack.Ret
 		it.regs = callFrameRegs
 		it.systemstack = true
+
 		return true
 
 	case "runtime.goexit", "runtime.rt0_go", "runtime.mcall":

pkg/proc/arm64_arch.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
+	"runtime"
 	"strings"
 
 	"github.com/go-delve/delve/pkg/dwarf/frame"
@@ -84,15 +85,15 @@ func arm64FixFrameUnwindContext(fctxt *frame.FrameContext, pc uint64, bi *Binary
 		return &frame.FrameContext{
 			RetAddrReg: regnum.ARM64_PC,
 			Regs: map[uint64]frame.DWRule{
-				regnum.ARM64_PC: frame.DWRule{
+				regnum.ARM64_PC: {
 					Rule:   frame.RuleOffset,
 					Offset: int64(-a.PtrSize()),
 				},
-				regnum.ARM64_BP: frame.DWRule{
+				regnum.ARM64_BP: {
 					Rule:   frame.RuleOffset,
 					Offset: int64(-2 * a.PtrSize()),
 				},
-				regnum.ARM64_SP: frame.DWRule{
+				regnum.ARM64_SP: {
 					Rule:   frame.RuleValOffset,
 					Offset: 0,
 				},
@@ -130,7 +131,7 @@ func arm64FixFrameUnwindContext(fctxt *frame.FrameContext, pc uint64, bi *Binary
 	}
 	if fctxt.Regs[regnum.ARM64_LR].Rule == frame.RuleUndefined {
 		fctxt.Regs[regnum.ARM64_LR] = frame.DWRule{
-			Rule:   frame.RuleFramePointer,
+			Rule:   frame.RuleRegister,
 			Reg:    regnum.ARM64_LR,
 			Offset: 0,
 		}
@@ -143,14 +144,80 @@ const arm64cgocallSPOffsetSaveSlot = 0x8
 const prevG0schedSPOffsetSaveSlot = 0x10
 
 func arm64SwitchStack(it *stackIterator, callFrameRegs *op.DwarfRegisters) bool {
-	if it.frame.Current.Fn == nil && it.systemstack && it.g != nil && it.top {
+	linux := runtime.GOOS == "linux"
+	if it.frame.Current.Fn == nil {
+		if it.systemstack && it.g != nil && it.top {
 			it.switchToGoroutineStack()
 			return true
 		}
-	if it.frame.Current.Fn != nil {
+		return false
+	}
 	switch it.frame.Current.Fn.Name {
-		case "runtime.asmcgocall", "runtime.cgocallback_gofunc", "runtime.sigpanic", "runtime.cgocallback":
+	case "runtime.cgocallback_gofunc", "runtime.cgocallback":
-			//do nothing
+		if linux {
+			// For a detailed description of how this works read the long comment at
+			// the start of $GOROOT/src/runtime/cgocall.go and the source code of
+			// runtime.cgocallback_gofunc in $GOROOT/src/runtime/asm_arm64.s
+			//
+			// When a C function calls back into go it will eventually call into
+			// runtime.cgocallback_gofunc which is the function that does the stack
+			// switch from the system stack back into the goroutine stack
+			// Since we are going backwards on the stack here we see the transition
+			// as goroutine stack -> system stack.
+			if it.top || it.systemstack {
+				return false
+			}
+
+			it.loadG0SchedSP()
+			if it.g0_sched_sp <= 0 {
+				return false
+			}
+			// Entering the system stack.
+			it.regs.Reg(callFrameRegs.SPRegNum).Uint64Val = it.g0_sched_sp
+			// Reads the previous value of g0.sched.sp that runtime.cgocallback_gofunc saved on the stack.
+			it.g0_sched_sp, _ = readUintRaw(it.mem, uint64(it.regs.SP()+prevG0schedSPOffsetSaveSlot), int64(it.bi.Arch.PtrSize()))
+			it.top = false
+			callFrameRegs, ret, retaddr := it.advanceRegs()
+			frameOnSystemStack := it.newStackframe(ret, retaddr)
+			it.pc = frameOnSystemStack.Ret
+			it.regs = callFrameRegs
+			it.systemstack = true
+
+			return true
+		}
+
+	case "runtime.asmcgocall":
+		if linux {
+			if it.top || !it.systemstack {
+				return false
+			}
+
+			// This function is called by a goroutine to execute a C function and
+			// switches from the goroutine stack to the system stack.
+			// Since we are unwinding the stack from callee to caller we have to switch
+			// from the system stack to the goroutine stack.
+			off, _ := readIntRaw(it.mem, uint64(it.regs.SP()+arm64cgocallSPOffsetSaveSlot),
+				int64(it.bi.Arch.PtrSize()))
+			oldsp := it.regs.SP()
+			newsp := uint64(int64(it.stackhi) - off)
+
+			it.regs.Reg(it.regs.SPRegNum).Uint64Val = uint64(int64(newsp))
+			// runtime.asmcgocall can also be called from inside the system stack,
+			// in that case no stack switch actually happens
+			if it.regs.SP() == oldsp {
+				return false
+			}
+
+			it.top = false
+			it.systemstack = false
+			// The return value is stored in the LR register which is saved at 24(SP).
+			it.frame.addrret = uint64(int64(it.regs.SP()) + int64(it.bi.Arch.PtrSize()*3))
+			it.frame.Ret, _ = readUintRaw(it.mem, it.frame.addrret, int64(it.bi.Arch.PtrSize()))
+			it.pc = it.frame.Ret
+
+			return true
+		}
+
 	case "runtime.goexit", "runtime.rt0_go", "runtime.mcall":
 		// Look for "top of stack" functions.
 		it.atend = true
@@ -174,9 +241,34 @@ func arm64SwitchStack(it *stackIterator, callFrameRegs *op.DwarfRegisters) bool
 			it.regs.AddReg(it.regs.BPRegNum, reg)
 		}
 		it.regs.Reg(it.regs.LRRegNum).Uint64Val = uint64(newlr)
+		if linux {
+			it.regs.Reg(it.regs.SPRegNum).Uint64Val = uint64(newbp)
+		} else {
 			it.regs.Reg(it.regs.SPRegNum).Uint64Val = uint64(newsp)
+		}
 		it.pc = newlr
 		return true
+	case "runtime.mstart":
+		if linux {
+			// Calls to runtime.systemstack will switch to the systemstack then:
+			// 1. alter the goroutine stack so that it looks like systemstack_switch
+			//    was called
+			// 2. alter the system stack so that it looks like the bottom-most frame
+			//    belongs to runtime.mstart
+			// If we find a runtime.mstart frame on the system stack of a goroutine
+			// parked on runtime.systemstack_switch we assume runtime.systemstack was
+			// called and continue tracing from the parked position.
+
+			if it.top || !it.systemstack || it.g == nil {
+				return false
+			}
+			if fn := it.bi.PCToFunc(it.g.PC); fn == nil || fn.Name != "runtime.systemstack_switch" {
+				return false
+			}
+
+			it.switchToGoroutineStack()
+			return true
+		}
 	default:
 		if it.systemstack && it.top && it.g != nil && strings.HasPrefix(it.frame.Current.Fn.Name, "runtime.") && it.frame.Current.Fn.Name != "runtime.throw" && it.frame.Current.Fn.Name != "runtime.fatalthrow" {
 			// The runtime switches to the system stack in multiple places.
@@ -190,7 +282,6 @@ func arm64SwitchStack(it *stackIterator, callFrameRegs *op.DwarfRegisters) bool
 			return true
 		}
 	}
-	}
 
 	fn := it.bi.PCToFunc(it.frame.Ret)
 	if fn == nil {

pkg/proc/proc_test.go

@@ -21,6 +21,7 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"text/tabwriter"
 	"time"
 
 	"github.com/go-delve/delve/pkg/dwarf/frame"
@@ -3312,6 +3313,8 @@ func TestIssue844(t *testing.T) {
 }
 
 func logStacktrace(t *testing.T, p *proc.Target, frames []proc.Stackframe) {
+	w := tabwriter.NewWriter(os.Stderr, 0, 0, 3, ' ', 0)
+	fmt.Fprintf(w, "\n%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t\n", "Call PC", "Frame Offset", "Frame Pointer Offset", "PC", "Return", "Function", "Location", "Top Defer", "Defers")
 	for j := range frames {
 		name := "?"
 		if frames[j].Current.Fn != nil {
@@ -3321,25 +3324,33 @@ func logStacktrace(t *testing.T, p *proc.Target, frames []proc.Stackframe) {
 			name = fmt.Sprintf("%s inlined in %s", frames[j].Call.Fn.Name, frames[j].Current.Fn.Name)
 		}
 
-		t.Logf("\t%#x %#x %#x %s at %s:%d\n", frames[j].Call.PC, frames[j].FrameOffset(), frames[j].FramePointerOffset(), name, filepath.Base(frames[j].Call.File), frames[j].Call.Line)
+		topmostdefer := ""
 		if frames[j].TopmostDefer != nil {
 			_, _, fn := frames[j].TopmostDefer.DeferredFunc(p)
 			fnname := ""
 			if fn != nil {
 				fnname = fn.Name
 			}
-			t.Logf("\t\ttopmost defer: %#x %s\n", frames[j].TopmostDefer.DwrapPC, fnname)
+			topmostdefer = fmt.Sprintf("%#x %s", frames[j].TopmostDefer.DwrapPC, fnname)
 		}
+
+		defers := ""
 		for deferIdx, _defer := range frames[j].Defers {
 			_, _, fn := _defer.DeferredFunc(p)
 			fnname := ""
 			if fn != nil {
 				fnname = fn.Name
 			}
-			t.Logf("\t\t%d defer: %#x %s\n", deferIdx, _defer.DwrapPC, fnname)
+			defers += fmt.Sprintf("%d %#x %s |", deferIdx, _defer.DwrapPC, fnname)
+		}
+
+		frame := frames[j]
+		fmt.Fprintf(w, "%#x\t%#x\t%#x\t%#x\t%#x\t%s\t%s:%d\t%s\t%s\t\n",
+			frame.Call.PC, frame.FrameOffset(), frame.FramePointerOffset(), frame.Current.PC, frame.Ret,
+			name, filepath.Base(frame.Call.File), frame.Call.Line, topmostdefer, defers)
 
 	}
-	}
+	w.Flush()
 }
 
 // stacktraceCheck checks that all the functions listed in tc appear in
@@ -3413,7 +3424,6 @@ func TestCgoStacktrace(t *testing.T) {
 	}
 
 	skipOn(t, "broken - cgo stacktraces", "386")
-	skipOn(t, "broken - cgo stacktraces", "linux", "arm64")
 	protest.MustHaveCgo(t)
 
 	// Tests that:
@@ -3440,6 +3450,8 @@ func TestCgoStacktrace(t *testing.T) {
 
 	withTestProcess("cgostacktest/", t, func(p *proc.Target, fixture protest.Fixture) {
 		for itidx, tc := range testCases {
+			t.Logf("iteration step %d", itidx)
+
 			assertNoError(p.Continue(), t, fmt.Sprintf("Continue at iteration step %d", itidx))
 
 			g, err := proc.GetG(p.CurrentThread())
@@ -3456,7 +3468,6 @@ func TestCgoStacktrace(t *testing.T) {
 			frames, err := g.Stacktrace(100, 0)
 			assertNoError(err, t, fmt.Sprintf("Stacktrace at iteration step %d", itidx))
 
-			t.Logf("iteration step %d", itidx)
 			logStacktrace(t, p, frames)
 
 			m := stacktraceCheck(t, tc, frames)
@@ -3475,7 +3486,7 @@ func TestCgoStacktrace(t *testing.T) {
 						t.Logf("frame %s offset mismatch", tc[i])
 					}
 					if framePointerOffs[tc[i]] != frames[j].FramePointerOffset() {
-						t.Logf("frame %s pointer offset mismatch", tc[i])
+						t.Logf("frame %s pointer offset mismatch, expected: %#v actual: %#v", tc[i], framePointerOffs[tc[i]], frames[j].FramePointerOffset())
 					}
 				} else {
 					frameOffs[tc[i]] = frames[j].FrameOffset()
@@ -3828,9 +3839,8 @@ func checkFrame(frame proc.Stackframe, fnname, file string, line int, inlined bo
 	if frame.Inlined != inlined {
 		if inlined {
 			return fmt.Errorf("not inlined")
-		} else {
-			return fmt.Errorf("inlined")
 		}
+		return fmt.Errorf("inlined")
 	}
 	return nil
 }

pkg/proc/stack.go

@@ -407,7 +407,14 @@ func (it *stackIterator) advanceRegs() (callFrameRegs op.DwarfRegisters, ret uin
 
 	callimage := it.bi.PCToImage(it.pc)
 
-	callFrameRegs = op.DwarfRegisters{StaticBase: callimage.StaticBase, ByteOrder: it.regs.ByteOrder, PCRegNum: it.regs.PCRegNum, SPRegNum: it.regs.SPRegNum, BPRegNum: it.regs.BPRegNum, LRRegNum: it.regs.LRRegNum}
+	callFrameRegs = op.DwarfRegisters{
+		StaticBase: callimage.StaticBase,
+		ByteOrder:  it.regs.ByteOrder,
+		PCRegNum:   it.regs.PCRegNum,
+		SPRegNum:   it.regs.SPRegNum,
+		BPRegNum:   it.regs.BPRegNum,
+		LRRegNum:   it.regs.LRRegNum,
+	}
 
 	// According to the standard the compiler should be responsible for emitting
 	// rules for the RSP register so that it can then be used to calculate CFA,

pkg/proc/variables.go

@@ -874,8 +874,8 @@ func (v *Variable) parseG() (*G, error) {
 	if bpvar := schedVar.fieldVariable("bp"); /* +rtype -opt uintptr */ bpvar != nil && bpvar.Value != nil {
 		bp, _ = constant.Int64Val(bpvar.Value)
 	}
-	if bpvar := schedVar.fieldVariable("lr"); /* +rtype -opt uintptr */ bpvar != nil && bpvar.Value != nil {
+	if lrvar := schedVar.fieldVariable("lr"); /* +rtype -opt uintptr */ lrvar != nil && lrvar.Value != nil {
-		lr, _ = constant.Int64Val(bpvar.Value)
+		lr, _ = constant.Int64Val(lrvar.Value)
 	}
 
 	unreadable := false