mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

runtime: revert "traceback: include pc=0x%x for inline frames" 

This reverts commit 643d816c8b (CL 561635).

Reason for revert: This works for telemetry but broke various other
properties of the tracebacks as well as some programs that read
tracebacks. We should figure out a solution that works for all uses,
and in the interim we should not be making telemetry work at the
cost of breaking other, existing valid uses.

See #65761 for details.

Change-Id: I467993ae778887e5bd3cca4c0fb54e9d44802ee1
Reviewed-on: https://go-review.googlesource.com/c/go/+/571797
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This commit is contained in:
Russ Cox 2024-03-14 15:30:13 -04:00 committed by Gopher Robot
parent 966609ad9e
commit 386dcf4c93
3 changed files with 6 additions and 269 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/runtime/crash_test.go
View File

@ -14,7 +14,6 @@ import (
"internal/testenv"
tracev2 "internal/trace/v2"
"io"
"log"
"os"
"os/exec"
"path/filepath"
@ -29,19 +28,7 @@ import (
var toRemove []string
const entrypointVar = "RUNTIME_TEST_ENTRYPOINT"
func TestMain(m *testing.M) {
switch entrypoint := os.Getenv(entrypointVar); entrypoint {
case "crash":
crash()
panic("unreachable")
default:
log.Fatalf("invalid %s: %q", entrypointVar, entrypoint)
case "":
// fall through to normal behavior
}
_, coreErrBefore := os.Stat("core")
status := m.Run()

24
src/runtime/traceback.go
View File

@ -993,24 +993,12 @@ func traceback2(u *unwinder, showRuntime bool, skip, max int) (n, lastN int) {
}
print(")\n")
print("\t", file, ":", line)
// The contract between Callers and CallersFrames uses
// return addresses, which are +1 relative to the CALL
// instruction. Follow that convention.
pc := uf.pc + 1
if !iu.isInlined(uf) && pc > f.entry() {
// Func-relative PCs make no sense for inlined
// frames because there is no actual entry.
print(" +", hex(pc-f.entry()))
}
if gp.m != nil && gp.m.throwing >= throwTypeRuntime && gp == gp.m.curg || level >= 2 {
if !iu.isInlined(uf) {
// The stack information makes no sense for inline frames.
print(" fp=", hex(u.frame.fp), " sp=", hex(u.frame.sp), " pc=", hex(pc))
} else {
// The PC for an inlined frame is a special marker NOP,
// but crash monitoring tools may still parse the PCs
// and feed them to CallersFrames.
print(" pc=", hex(pc))
if !iu.isInlined(uf) {
if u.frame.pc > f.entry() {
print(" +", hex(u.frame.pc-f.entry()))
}
if gp.m != nil && gp.m.throwing >= throwTypeRuntime && gp == gp.m.curg || level >= 2 {
print(" fp=", hex(u.frame.fp), " sp=", hex(u.frame.sp), " pc=", hex(u.frame.pc))
}
}
print("\n")

238
src/runtime/traceback_system_test.go
View File

@ -1,238 +0,0 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package runtime_test
// This test of GOTRACEBACK=system has its own file,
// to minimize line-number perturbation.
import (
"bytes"
"fmt"
"internal/testenv"
"io"
"os"
"path/filepath"
"reflect"
"runtime"
"runtime/debug"
"strconv"
"strings"
"testing"
)
// This is the entrypoint of the child process used by
// TestTracebackSystem. It prints a crash report to stdout.
func crash() {
// Ensure that we get pc=0x%x values in the traceback.
debug.SetTraceback("system")
writeSentinel(os.Stdout)
debug.SetCrashOutput(os.Stdout)
go func() {
// This call is typically inlined.
child()
}()
select {}
}
func child() {
grandchild()
}
func grandchild() {
// Write runtime.Caller's view of the stack to stderr, for debugging.
var pcs [16]uintptr
n := runtime.Callers(1, pcs[:])
io.WriteString(os.Stderr, formatStack(pcs[:n]))
// Cause the crash report to be written to stdout.
panic("oops")
}
// TestTracebackSystem tests that the syntax of crash reports produced
// by GOTRACEBACK=system (see traceback2) contains a complete,
// parseable list of program counters for the running goroutine that
// can be parsed and fed to runtime.CallersFrames to obtain accurate
// information about the logical call stack, even in the presence of
// inlining.
//
// The test is a distillation of the crash monitor in
// golang.org/x/telemetry/crashmonitor.
func TestTracebackSystem(t *testing.T) {
testenv.MustHaveExec(t)
if runtime.GOOS == "android" {
t.Skip("Can't read source code for this file on Android")
}
// Fork+exec the crashing process.
exe, err := os.Executable()
if err != nil {
t.Fatal(err)
}
cmd := testenv.Command(t, exe)
cmd.Env = append(cmd.Environ(), entrypointVar+"=crash")
cmd.Stdout = new(strings.Builder)
// cmd.Stderr = os.Stderr // uncomment to debug, e.g. to see runtime.Caller's view
cmd.Run() // expected to crash
crash := cmd.Stdout.(*strings.Builder).String()
// If the only line is the sentinel, it wasn't a crash.
if strings.Count(crash, "\n") < 2 {
t.Fatalf("child process did not produce a crash report")
}
// Parse the PCs out of the child's crash report.
pcs, err := parseStackPCs(crash)
if err != nil {
t.Fatal(err)
}
// Unwind the stack using this executable's symbol table.
got := formatStack(pcs)
want := `redacted.go:0: runtime.gopanic
traceback_system_test.go:51: runtime_test.grandchild: panic("oops")
traceback_system_test.go:41: runtime_test.child: grandchild()
traceback_system_test.go:35: runtime_test.crash.func1: child()
redacted.go:0: runtime.goexit`
if strings.TrimSpace(got) != strings.TrimSpace(want) {
t.Errorf("got:\n%swant:\n%s", got, want)
}
}
// parseStackPCs parses the parent process's program counters for the
// first running goroutine out of a GOTRACEBACK=system traceback,
// adjusting them so that they are valid for the child process's text
// segment.
//
// This function returns only program counter values, ensuring that
// there is no possibility of strings from the crash report (which may
// contain PII) leaking into the telemetry system.
//
// (Copied from golang.org/x/telemetry/crashmonitor.parseStackPCs.)
func parseStackPCs(crash string) ([]uintptr, error) {
// getPC parses the PC out of a line of the form:
// \tFILE:LINE +0xRELPC sp=... fp=... pc=...
getPC := func(line string) (uint64, error) {
_, pcstr, ok := strings.Cut(line, " pc=") // e.g. pc=0x%x
if !ok {
return 0, fmt.Errorf("no pc= for stack frame: %s", line)
}
return strconv.ParseUint(pcstr, 0, 64) // 0 => allow 0x prefix
}
var (
pcs []uintptr
parentSentinel uint64
childSentinel = sentinel()
on = false // are we in the first running goroutine?
lines = strings.Split(crash, "\n")
)
for i := 0; i < len(lines); i++ {
line := lines[i]
// Read sentinel value.
if parentSentinel == 0 && strings.HasPrefix(line, "sentinel ") {
_, err := fmt.Sscanf(line, "sentinel %x", &parentSentinel)
if err != nil {
return nil, fmt.Errorf("can't read sentinel line")
}
continue
}
// Search for "goroutine GID [STATUS]"
if !on {
if strings.HasPrefix(line, "goroutine ") &&
strings.Contains(line, " [running]:") {
on = true
if parentSentinel == 0 {
return nil, fmt.Errorf("no sentinel value in crash report")
}
}
continue
}
// A blank line marks end of a goroutine stack.
if line == "" {
break
}
// Skip the final "created by SYMBOL in goroutine GID" part.
if strings.HasPrefix(line, "created by ") {
break
}
// Expect a pair of lines:
// SYMBOL(ARGS)
// \tFILE:LINE +0xRELPC sp=0x%x fp=0x%x pc=0x%x
// Note: SYMBOL may contain parens "pkg.(*T).method"
// The RELPC is sometimes missing.
// Skip the symbol(args) line.
i++
if i == len(lines) {
break
}
line = lines[i]
// Parse the PC, and correct for the parent and child's
// different mappings of the text section.
pc, err := getPC(line)
if err != nil {
// Inlined frame, perhaps; skip it.
continue
}
pcs = append(pcs, uintptr(pc-parentSentinel+childSentinel))
}
return pcs, nil
}
// The sentinel function returns its address. The difference between
// this value as observed by calls in two different processes of the
// same executable tells us the relative offset of their text segments.
//
// It would be nice if SetCrashOutput took care of this as it's fiddly
// and likely to confuse every user at first.
func sentinel() uint64 {
return uint64(reflect.ValueOf(sentinel).Pointer())
}
func writeSentinel(out io.Writer) {
fmt.Fprintf(out, "sentinel %x\n", sentinel())
}
// formatStack formats a stack of PC values using the symbol table,
// redacting information that cannot be relied upon in the test.
func formatStack(pcs []uintptr) string {
// When debugging, show file/line/content of files other than this one.
const debug = false
var buf strings.Builder
i := 0
frames := runtime.CallersFrames(pcs)
for {
fr, more := frames.Next()
if debug {
fmt.Fprintf(&buf, "pc=%x ", pcs[i])
i++
}
if base := filepath.Base(fr.File); base == "traceback_system_test.go" || debug {
content, err := os.ReadFile(fr.File)
if err != nil {
panic(err)
}
lines := bytes.Split(content, []byte("\n"))
fmt.Fprintf(&buf, "%s:%d: %s: %s\n", base, fr.Line, fr.Function, lines[fr.Line-1])
} else {
// For robustness, don't show file/line for functions from other files.
fmt.Fprintf(&buf, "redacted.go:0: %s\n", fr.Function)
}
if !more {
break
}
}
return buf.String()
}