diff --git a/nativeunwind/elfunwindinfo/elfgopclntab.go b/nativeunwind/elfunwindinfo/elfgopclntab.go index a0ac5b9fe..ab8715950 100644 --- a/nativeunwind/elfunwindinfo/elfgopclntab.go +++ b/nativeunwind/elfunwindinfo/elfgopclntab.go @@ -11,9 +11,9 @@ import ( "bytes" "debug/elf" "fmt" + "go/version" "unsafe" - log "github.com/sirupsen/logrus" "go.opentelemetry.io/ebpf-profiler/libpf/pfelf" sdtypes "go.opentelemetry.io/ebpf-profiler/nativeunwind/stackdeltatypes" ) @@ -229,11 +229,13 @@ func getString(data []byte, offset int) []byte { return data[offset : offset+zeroIdx] } +type strategy int + const ( - strategyUnknown = iota + strategyUnknown strategy = iota strategyFramePointer - strategyDeltasWithoutRBP - strategyDeltasWithRBP + strategyDeltasWithFrame + strategyDeltasWithoutFrame ) // noFPSourceSuffixes lists the go runtime source files that call assembly code @@ -251,30 +253,24 @@ var noFPSourceSuffixes = [][]byte{ } // getSourceFileStrategy categorizes sourceFile's unwinding strategy based on its name -func getSourceFileStrategy(arch elf.Machine, sourceFile []byte) int { +func getSourceFileStrategy(arch elf.Machine, sourceFile []byte, defaultStrategy strategy) strategy { switch arch { case elf.EM_X86_64: // Most of the assembly code needs explicit SP delta as they do not // create stack frame. Do not recover RBP as it is not modified. if bytes.HasSuffix(sourceFile, []byte(".s")) { - return strategyDeltasWithoutRBP + return strategyDeltasWithoutFrame } - // Check for the Go source files needing SP delta unwinding to recover RBP for _, suffix := range noFPSourceSuffixes { if bytes.HasSuffix(sourceFile, suffix) { - return strategyDeltasWithRBP + return strategyDeltasWithFrame } } - case elf.EM_AARCH64: - // Assume all code has frame pointers as the code generated by Golang compiler - // for ARM64 supports frame pointers even for asm code. Frame pointers - // get omitted only for leaf, no arg functions. - return strategyFramePointer + return defaultStrategy + default: + return defaultStrategy } - - // Use frame pointer for others - return strategyFramePointer } // SearchGoPclntab uses heuristic to find the gopclntab from RO data. @@ -474,10 +470,37 @@ func (ee *elfExtractor) parseGoPclntab() error { // would fill up our precious kernel delta maps fast, the strategy is to // create deltastack maps for non-Go source files only, and otherwise // cover the vast majority with "use frame pointer" stack delta. - sourceStrategy := make(map[int]int) + sourceStrategy := make(map[int]strategy) // Get target machine architecture for the ELF file arch := ef.Machine + defaultStrategy := strategyFramePointer + var parsePclntab func(*sdtypes.StackDeltaArray, *pclntabFunc, uintptr, []byte, + strategy, uint64, uint8) error + + switch arch { + case elf.EM_X86_64: + parsePclntab = parseX86pclntabFunc + case elf.EM_AARCH64: + parsePclntab = parseArm64pclntabFunc + // Go 1.20 and earlier did not maintain frame pointers properly on arm64. + // This was fixed for Go 1.21 and later in: + // https://github.com/golang/go/commit/a41a29ad19c25c3475a65b7265fcad870d954c2a + switch hdr.magic { + case magicGo1_2, magicGo1_16, magicGo1_18: + // Magic indicates old Go with broken arm64 frame pointers + defaultStrategy = strategyDeltasWithFrame + case magicGo1_20: + // Ambiguous regarding if frame pointer is kept correctly. + // Take the slow path of resolving Go version. + goVer, err := ee.file.GoVersion() + if err != nil || version.Compare(goVer, "go1.21rc1") < 0 { + defaultStrategy = strategyDeltasWithFrame + } + } + default: + return fmt.Errorf("unsupported ELF architecture (%x)", arch) + } fmap := &pclntabFuncMap{} fun := &pclntabFunc{} @@ -526,7 +549,7 @@ func (ee *elfExtractor) parseGoPclntab() error { // Use source file to determine strategy if possible, and default // to using frame pointers in the unlikely case of no file info - strategy := strategyFramePointer + fileStrategy := defaultStrategy if fun.pcfileOff != 0 { p := newPcval(pctab[fun.pcfileOff:], uint(fun.startPc), hdr.quantum) fileIndex := int(p.val) @@ -535,25 +558,25 @@ func (ee *elfExtractor) parseGoPclntab() error { } // Determine strategy - strategy = sourceStrategy[fileIndex] - if strategy == strategyUnknown { + fileStrategy = sourceStrategy[fileIndex] + if fileStrategy == strategyUnknown { sourceFile := getString(filetab, getInt32(cutab, 4*fileIndex)) - strategy = getSourceFileStrategy(arch, sourceFile) - sourceStrategy[fileIndex] = strategy + fileStrategy = getSourceFileStrategy(arch, sourceFile, defaultStrategy) + sourceStrategy[fileIndex] = fileStrategy } } - switch arch { - case elf.EM_X86_64: - if err := parseX86pclntabFunc(ee.deltas, fun, dataLen, pctab, strategy, i, - hdr.quantum); err != nil { - return err - } - case elf.EM_AARCH64: - if err := parseArm64pclntabFunc(ee.deltas, fun, dataLen, pctab, i, - hdr.quantum); err != nil { - return err - } + if fileStrategy == strategyFramePointer { + // Use stack frame-pointer delta + ee.deltas.Add(sdtypes.StackDelta{ + Address: fun.startPc, + Info: sdtypes.UnwindInfoFramePointer, + }) + continue + } + if err := parsePclntab(ee.deltas, fun, dataLen, pctab, fileStrategy, i, + hdr.quantum); err != nil { + return err } } @@ -587,48 +610,41 @@ func (ee *elfExtractor) parseGoPclntab() error { // parseX86pclntabFunc extracts interval information from x86_64 based pclntabFunc. func parseX86pclntabFunc(deltas *sdtypes.StackDeltaArray, fun *pclntabFunc, dataLen uintptr, - pctab []byte, strategy int, i uint64, quantum uint8) error { - switch { - case strategy == strategyFramePointer: - // Use stack frame-pointer delta - deltas.Add(sdtypes.StackDelta{ - Address: fun.startPc, - Info: sdtypes.UnwindInfoFramePointerX64, - }) + pctab []byte, s strategy, i uint64, quantum uint8) error { + if fun.pcspOff == 0 { + // Some functions don't have PCSP info: skip them. return nil - case fun.pcspOff != 0: - // Generate stack deltas as the information is available - if dataLen < uintptr(fun.pcspOff) { - return fmt.Errorf(".gopclntab func %v pcscOff (%d) is invalid", - i, fun.pcspOff) - } + } + // Generate stack deltas as the information is available + if dataLen < uintptr(fun.pcspOff) { + return fmt.Errorf(".gopclntab func %v pcscOff (%d) is invalid", + i, fun.pcspOff) + } - p := newPcval(pctab[fun.pcspOff:], uint(fun.startPc), quantum) - hints := sdtypes.UnwindHintKeep - for ok := true; ok; ok = p.step() { - info := sdtypes.UnwindInfo{ - Opcode: sdtypes.UnwindOpcodeBaseSP, - Param: p.val + 8, - } - if strategy == strategyDeltasWithRBP && info.Param >= 16 { - info.FPOpcode = sdtypes.UnwindOpcodeBaseCFA - info.FPParam = -16 - } - deltas.Add(sdtypes.StackDelta{ - Address: uint64(p.pcStart), - Hints: hints, - Info: info, - }) - hints = sdtypes.UnwindHintNone + p := newPcval(pctab[fun.pcspOff:], uint(fun.startPc), quantum) + hints := sdtypes.UnwindHintKeep + for ok := true; ok; ok = p.step() { + info := sdtypes.UnwindInfo{ + Opcode: sdtypes.UnwindOpcodeBaseSP, + Param: p.val + 8, + } + if s == strategyDeltasWithFrame && info.Param >= 16 { + info.FPOpcode = sdtypes.UnwindOpcodeBaseCFA + info.FPParam = -16 } + deltas.Add(sdtypes.StackDelta{ + Address: uint64(p.pcStart), + Hints: hints, + Info: info, + }) + hints = sdtypes.UnwindHintNone } - log.Debugf("Unhandled .gopclntab func at %d", i) return nil } // parseArm64pclntabFunc extracts interval information from ARM64 based pclntabFunc. func parseArm64pclntabFunc(deltas *sdtypes.StackDeltaArray, fun *pclntabFunc, - dataLen uintptr, pctab []byte, i uint64, quantum uint8) error { + dataLen uintptr, pctab []byte, s strategy, i uint64, quantum uint8) error { if fun.pcspOff == 0 { // Some CGO functions don't have PCSP info: skip them. return nil @@ -637,13 +653,6 @@ func parseArm64pclntabFunc(deltas *sdtypes.StackDeltaArray, fun *pclntabFunc, return fmt.Errorf(".gopclntab func %v pcspOff = %d is invalid", i, fun.pcspOff) } - // On ARM64, frame pointers are not properly kept when the Go runtime copies the stack during - // `runtime.morestack` calls: all old frame pointers are set to 0. - // - // https://github.com/golang/go/blob/c318f191/src/runtime/stack.go#L676 - // - // We thus need to unwind with stack delta offsets. - hint := sdtypes.UnwindHintKeep p := newPcval(pctab[fun.pcspOff:], uint(fun.startPc), quantum) for ok := true; ok; ok = p.step() { @@ -657,9 +666,11 @@ func parseArm64pclntabFunc(deltas *sdtypes.StackDeltaArray, fun *pclntabFunc, // Unwind via SP offset. Opcode: sdtypes.UnwindOpcodeBaseSP, Param: p.val, + } + if s == strategyDeltasWithFrame { // On ARM64, the previous LR value is stored to top-of-stack. - FPOpcode: sdtypes.UnwindOpcodeBaseSP, - FPParam: 0, + info.FPOpcode = sdtypes.UnwindOpcodeBaseSP + info.FPParam = 0 } } diff --git a/nativeunwind/elfunwindinfo/elfgopclntab_test.go b/nativeunwind/elfunwindinfo/elfgopclntab_test.go index 308698265..7271db650 100644 --- a/nativeunwind/elfunwindinfo/elfgopclntab_test.go +++ b/nativeunwind/elfunwindinfo/elfgopclntab_test.go @@ -53,16 +53,16 @@ func TestPcvalInvalid(_ *testing.T) { // Some strategy tests func TestGoStrategy(t *testing.T) { res := []struct { - file string - strategy int + file string + result strategy }{ {"foo.go", strategyFramePointer}, - {"foo.s", strategyDeltasWithoutRBP}, - {"go/src/crypto/elliptic/p256_asm.go", strategyDeltasWithRBP}, + {"foo.s", strategyDeltasWithoutFrame}, + {"go/src/crypto/elliptic/p256_asm.go", strategyDeltasWithFrame}, } for _, x := range res { - s := getSourceFileStrategy(elf.EM_X86_64, []byte(x.file)) - assert.Equal(t, x.strategy, s) + s := getSourceFileStrategy(elf.EM_X86_64, []byte(x.file), strategyFramePointer) + assert.Equal(t, x.result, s) } } diff --git a/nativeunwind/stackdeltatypes/stackdeltatypes.go b/nativeunwind/stackdeltatypes/stackdeltatypes.go index d91e3d3dd..cfdf77d91 100644 --- a/nativeunwind/stackdeltatypes/stackdeltatypes.go +++ b/nativeunwind/stackdeltatypes/stackdeltatypes.go @@ -29,10 +29,11 @@ const ( UnwindOpcodeFlagDeref uint8 = C.UNWIND_OPCODEF_DEREF // UnwindCommands from the C header file - UnwindCommandInvalid int32 = C.UNWIND_COMMAND_INVALID - UnwindCommandStop int32 = C.UNWIND_COMMAND_STOP - UnwindCommandPLT int32 = C.UNWIND_COMMAND_PLT - UnwindCommandSignal int32 = C.UNWIND_COMMAND_SIGNAL + UnwindCommandInvalid int32 = C.UNWIND_COMMAND_INVALID + UnwindCommandStop int32 = C.UNWIND_COMMAND_STOP + UnwindCommandPLT int32 = C.UNWIND_COMMAND_PLT + UnwindCommandSignal int32 = C.UNWIND_COMMAND_SIGNAL + UnwindCommandFramePointer int32 = C.UNWIND_COMMAND_FRAME_POINTER // UnwindDeref handling from the C header file UnwindDerefMask int32 = C.UNWIND_DEREF_MASK @@ -63,12 +64,10 @@ var UnwindInfoStop = UnwindInfo{Opcode: UnwindOpcodeCommand, Param: UnwindComman // UnwindInfoSignal is the stack delta info indicating signal return frame. var UnwindInfoSignal = UnwindInfo{Opcode: UnwindOpcodeCommand, Param: UnwindCommandSignal} -// UnwindInfoFramePointerX64 contains the description to unwind a x86-64 frame pointer frame. -var UnwindInfoFramePointerX64 = UnwindInfo{ - Opcode: UnwindOpcodeBaseFP, - Param: 16, - FPOpcode: UnwindOpcodeBaseCFA, - FPParam: -16, +// UnwindInfoFramePointer contains the description to unwind a frame pointer frame. +var UnwindInfoFramePointer = UnwindInfo{ + Opcode: UnwindOpcodeCommand, + Param: UnwindCommandFramePointer, } // UnwindInfoLR contains the description to unwind ARM64 function without a frame (LR only) diff --git a/support/ebpf/native_stack_trace.ebpf.c b/support/ebpf/native_stack_trace.ebpf.c index 62c6d5c63..0b1e65476 100644 --- a/support/ebpf/native_stack_trace.ebpf.c +++ b/support/ebpf/native_stack_trace.ebpf.c @@ -403,6 +403,11 @@ static ErrorCode unwind_one_frame(u64 pid, u32 frame_idx, UnwindState *state, bo DEBUG_PRINT("signal frame"); goto frame_ok; case UNWIND_COMMAND_STOP: *stop = true; return ERR_OK; + case UNWIND_COMMAND_FRAME_POINTER: + if (!unwinder_unwind_frame_pointer(state)) { + goto err_native_pc_read; + } + goto frame_ok; default: return ERR_UNREACHABLE; } } else { @@ -485,6 +490,11 @@ static ErrorCode unwind_one_frame(u64 pid, u32 frame_idx, struct UnwindState *st DEBUG_PRINT("signal frame"); goto frame_ok; case UNWIND_COMMAND_STOP: *stop = true; return ERR_OK; + case UNWIND_COMMAND_FRAME_POINTER: + if (!unwinder_unwind_frame_pointer(state)) { + goto err_native_pc_read; + } + goto frame_ok; default: return ERR_UNREACHABLE; } } diff --git a/support/ebpf/stackdeltatypes.h b/support/ebpf/stackdeltatypes.h index c0509d86e..3c9a6a422 100644 --- a/support/ebpf/stackdeltatypes.h +++ b/support/ebpf/stackdeltatypes.h @@ -17,13 +17,15 @@ #define UNWIND_OPCODEF_DEREF 0x80 // Unsupported or no value for the register -#define UNWIND_COMMAND_INVALID 0 +#define UNWIND_COMMAND_INVALID 0 // For CFA: stop unwinding, this function is a stack root function -#define UNWIND_COMMAND_STOP 1 +#define UNWIND_COMMAND_STOP 1 // Unwind a PLT entry -#define UNWIND_COMMAND_PLT 2 +#define UNWIND_COMMAND_PLT 2 // Unwind a signal frame -#define UNWIND_COMMAND_SIGNAL 3 +#define UNWIND_COMMAND_SIGNAL 3 +// Unwind using standard frame pointer +#define UNWIND_COMMAND_FRAME_POINTER 4 // If opcode has UNWIND_OPCODEF_DEREF set, the lowest bits of 'param' are used // as second adder as post-deref operation. This contains the mask for that. diff --git a/support/ebpf/tracemgmt.h b/support/ebpf/tracemgmt.h index cc2816ac9..60258f908 100644 --- a/support/ebpf/tracemgmt.h +++ b/support/ebpf/tracemgmt.h @@ -280,6 +280,22 @@ unwinder_mark_nonleaf_frame(UnwindState *state) #endif } +// unwinder_unwind_frame_pointer unwinds using the Frame Pointer. +static inline __attribute__((__always_inline__)) bool +unwinder_unwind_frame_pointer(UnwindState *state) +{ + unsigned long regs[2]; + + if (bpf_probe_read_user(regs, sizeof(regs), (void *)state->fp)) { + return false; + } + state->sp = state->fp + sizeof(regs); + state->fp = regs[0]; + state->pc = regs[1]; + unwinder_mark_nonleaf_frame(state); + return true; +} + // Push the file ID, line number and frame type into FrameList with a user-defined // maximum stack size. // diff --git a/support/ebpf/tracer.ebpf.release.amd64 b/support/ebpf/tracer.ebpf.release.amd64 index c9fd60c3b..fc76f8977 100644 Binary files a/support/ebpf/tracer.ebpf.release.amd64 and b/support/ebpf/tracer.ebpf.release.amd64 differ diff --git a/support/ebpf/tracer.ebpf.release.arm64 b/support/ebpf/tracer.ebpf.release.arm64 index 27aa5b678..365871f08 100644 Binary files a/support/ebpf/tracer.ebpf.release.arm64 and b/support/ebpf/tracer.ebpf.release.arm64 differ diff --git a/support/ebpf/v8_tracer.ebpf.c b/support/ebpf/v8_tracer.ebpf.c index 10ca4d5f3..bb12dddb8 100644 --- a/support/ebpf/v8_tracer.ebpf.c +++ b/support/ebpf/v8_tracer.ebpf.c @@ -105,7 +105,7 @@ unwind_one_v8_frame(PerCPURecord *record, V8ProcInfo *vi, bool top) { UnwindState *state = &record->state; Trace *trace = &record->trace; - unsigned long regs[2], sp = state->sp, fp = state->fp, pc = state->pc; + unsigned long sp = state->sp, fp = state->fp, pc = state->pc; V8UnwindScratchSpace *scratch = &record->v8UnwindScratch; // All V8 frames have frame pointer. Check that the FP looks valid. @@ -275,20 +275,18 @@ unwind_one_v8_frame(PerCPURecord *record, V8ProcInfo *vi, bool top) u32 cookie = (code_size << 4) | code_kind; delta_or_marker = (pc - code_start) | ((uintptr_t)cookie << V8_LINE_COOKIE_SHIFT); -frame_done: - // Unwind with frame pointer - if (bpf_probe_read_user(regs, sizeof(regs), (void *)fp)) { - DEBUG_PRINT("v8: --> bad frame pointer"); - increment_metric(metricID_UnwindV8ErrBadFP); - return ERR_V8_BAD_FP; - } - +frame_done:; ErrorCode error = push_v8(trace, pointer_and_type, delta_or_marker, state->return_address); if (error) { return error; } - state->sp = fp + sizeof(regs); + // Unwind with frame pointer + if (!unwinder_unwind_frame_pointer(state)) { + DEBUG_PRINT("v8: --> bad frame pointer"); + increment_metric(metricID_UnwindV8ErrBadFP); + return ERR_V8_BAD_FP; + } // The JS Entry Frame's layout differs from other frames because some callee // saved registers might be pushed onto the stack before the [fp, lr] pair. @@ -297,10 +295,6 @@ unwind_one_v8_frame(PerCPURecord *record, V8ProcInfo *vi, bool top) if (pointer_and_type == V8_FILE_TYPE_MARKER && delta_or_marker == 1) state->sp += V8_ENTRYFRAME_CALLEE_SAVED_REGS_BEFORE_FP_LR_PAIR * sizeof(size_t); - state->fp = regs[0]; - state->pc = regs[1]; - unwinder_mark_nonleaf_frame(state); - DEBUG_PRINT( "v8: pc: %lx, sp: %lx, fp: %lx", (unsigned long)state->pc, diff --git a/tools/stackdeltas/stackdeltas.go b/tools/stackdeltas/stackdeltas.go index d9c16b44d..dbdea57ec 100644 --- a/tools/stackdeltas/stackdeltas.go +++ b/tools/stackdeltas/stackdeltas.go @@ -44,6 +44,8 @@ func getOpcode(opcode uint8, param int32) string { return "plt" case sdtypes.UnwindCommandSignal: return "signal" + case sdtypes.UnwindCommandFramePointer: + return "framepointer" default: return "?" }