Skip to content
Permalink
Browse files

runtime: fix line number for faulting instructions

Unlike function calls, when processing instructions that directly
fault we must not subtract 1 from the pc before looking up the
file/line information.

Since the file/line lookup unconditionally subtracts 1, add 1 to
the faulting instruction PCs to compensate.

Fixes #34123

Change-Id: Ie7361e3d2f84a0d4f48d97e5a9e74f6291ba7a8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/196962
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
  • Loading branch information...
randall77 committed Sep 23, 2019
1 parent 9e914f5 commit 9ee6ba089dc9dd2402bccd9ed28b07140f76de15
Showing with 60 additions and 10 deletions.
  1. +0 −6 src/runtime/pprof/proto.go
  2. +3 −3 src/runtime/pprof/proto_test.go
  3. +14 −1 src/runtime/traceback.go
  4. +43 −0 test/fixedbugs/issue34123.go
@@ -359,13 +359,7 @@ func (b *profileBuilder) build() {
}
}

// Addresses from stack traces point to the next instruction after each call,
// except for the leaf, which points to where the signal occurred.
// appendLocsForStack expects return PCs so increment the leaf address to
// look like a return PC.
e.stk[0] += 1
locs = b.appendLocsForStack(locs[:0], e.stk)
e.stk[0] -= 1 // undo the adjustment on the leaf.

b.pbSample(values, locs, labels)
}
@@ -116,9 +116,9 @@ func TestConvertCPUProfile(t *testing.T) {

b := []uint64{
3, 0, 500, // hz = 500
5, 0, 10, uint64(addr1), uint64(addr1 + 2), // 10 samples in addr1
5, 0, 40, uint64(addr2), uint64(addr2 + 2), // 40 samples in addr2
5, 0, 10, uint64(addr1), uint64(addr1 + 2), // 10 samples in addr1
5, 0, 10, uint64(addr1 + 1), uint64(addr1 + 2), // 10 samples in addr1
5, 0, 40, uint64(addr2 + 1), uint64(addr2 + 2), // 40 samples in addr2
5, 0, 10, uint64(addr1 + 1), uint64(addr1 + 2), // 10 samples in addr1
}
p, err := translateCPUProfile(b)
if err != nil {
@@ -340,7 +340,20 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
pc := frame.pc
// backup to CALL instruction to read inlining info (same logic as below)
tracepc := pc
if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic {
// Normally, pc is a return address. In that case, we want to look up
// file/line information using pc-1, because that is the pc of the
// call instruction (more precisely, the last byte of the call instruction).
// Callers expect the pc buffer to contain return addresses and do the
// same -1 themselves, so we keep pc unchanged.
// When the pc is from a signal (e.g. profiler or segv) then we want
// to look up file/line information using pc, and we store pc+1 in the
// pc buffer so callers can unconditionally subtract 1 before looking up.
// See issue 34123.
// The pc can be at function entry when the frame is initialized without
// actually running code, like runtime.mstart.
if (n == 0 && flags&_TraceTrap != 0) || waspanic || pc == f.entry {
pc++
} else {
tracepc--
}

@@ -0,0 +1,43 @@
// run

// Copyright 2019 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.

// Make sure that the line number is reported correctly
// for faulting instructions.

package main

import (
"fmt"
"runtime"
)

var x byte
var p *byte

//go:noinline
func f() {
q := p
x = 11 // line 23
*q = 12 // line 24
}
func main() {
defer func() {
recover()
var pcs [10]uintptr
n := runtime.Callers(1, pcs[:])
frames := runtime.CallersFrames(pcs[:n])
for {
f, more := frames.Next()
if f.Function == "main.f" && f.Line != 24 {
panic(fmt.Errorf("expected line 24, got line %d", f.Line))
}
if !more {
break
}
}
}()
f()
}

0 comments on commit 9ee6ba0

Please sign in to comment.
You can’t perform that action at this time.