Skip to content

runtime: Stack captures incorrect line numbers #7690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
gopherbot opened this issue Apr 2, 2014 · 13 comments
Closed

runtime: Stack captures incorrect line numbers #7690

gopherbot opened this issue Apr 2, 2014 · 13 comments
Labels
FrozenDueToAge
Milestone
Go1.4

Comments

@gopherbot
Copy link
Contributor

by glyn.normington:

What does 'go version' print?

go version go1.2.1 darwin/amd64

Also fails on play.golang.org with go1.2rc3.

What steps reproduce the problem?

Run this: http://play.golang.org/p/PLLLhGYKry

What happened?

runtime.Stack is called on line 13 of the program, but the resultant stack trace shows
line 14.

What should have happened instead?

It should have shown line 13.

Please provide any additional information below.

runtime.Caller(0) gives the correct line number.
@gopherbot
Copy link
Contributor Author

Comment 1 by glyn.normington:

This seems only to affect the caller of runtime.Stack. Callers further up the stack get
the correct line numbers. See: http://play.golang.org/p/YM7plCbqnW

@ianlancetaylor
Copy link
Member

Comment 2:

Labels changed: added repo-main, release-go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2014

Comment 3:

84100043

Owner changed to @rsc.

Status changed to Started.

@gopherbot
Copy link
Contributor Author

Comment 4:

CL https://golang.org/cl/84100043 references this issue.

@rsc
Copy link
Contributor

rsc commented May 12, 2014

Comment 5:

This seems like such a trivial thing to fix, but it's actually a very deep problem in
the code: we just don't track whether a particular PC starting the stack was obtained
from a stack return address or a signal. The former should be backed up when translating
to a line number but the latter should not. I tried a few things but they are incomplete
and I think make the situation more confusing than it is now. I think we'll need to
postpone this to Go 1.4.

Labels changed: added release-go1.4, removed release-go1.3maybe.

@gopherbot
Copy link
Contributor Author

Comment 6 by glyn.normington:

Too bad, but thanks for investigating. I'll look forward to a robust fix in Go 1.4.

@ChrisHines
Copy link
Contributor

Comment 7:

I have also encountered this issue when using runtime.Callers and
runtime.FuncForPC(pc).FileLine(pc). In my experience, it does affect more than just the
immediate calling frame. Furthermore, the errors can be bigger than one (or negative)
when the analyzed call site is on the last line of a control structure block.
See http://play.golang.org/p/WZJ6yLD6NC for several additional test cases.

@gopherbot
Copy link
Contributor Author

Comment 8:

CL https://golang.org/cl/167810044 mentions this issue.

@gopherbot
Copy link
Contributor Author

Comment 9:

CL https://golang.org/cl/163550043 mentions this issue.

@rsc
Copy link
Contributor

rsc commented Oct 29, 2014

Comment 10:

This issue was closed by revision 8db71d4.

Status changed to Fixed.

@rsc
Copy link
Contributor

rsc commented Oct 29, 2014

Comment 11:

This issue was closed by revision a22c11b.

@gopherbot
Copy link
Contributor Author

Comment 12:

CL https://golang.org/cl/170720043 mentions this issue.

@rsc
Copy link
Contributor

rsc commented Oct 30, 2014

Comment 13:

This issue was closed by revision a5a0733.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@rsc @wheatman
runtime: update comment for Callers
57327e3 
Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes golang#7690.

LGTM=r, chris.cs.guy
R=r, chris.cs.guy
CC=golang-codereviews
https://golang.org/cl/163550043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@rsc @wheatman
runtime: fix line number in first stack frame in printed stack trace
0748c5b 
Originally traceback was only used for printing the stack
when an unexpected signal came in. In that case, the
initial PC is taken from the signal and should be used
unaltered. For the callers, the PC is the return address,
which might be on the line after the call; we subtract 1
to get to the CALL instruction.

Traceback is now used for a variety of things, and for
almost all of those the initial PC is a return address,
whether from getcallerpc, or gp->sched.pc, or gp->syscallpc.
In those cases, we need to subtract 1 from this initial PC,
but the traceback code had a hard rule "never subtract 1
from the initial PC", left over from the signal handling days.

Change gentraceback to take a flag that specifies whether
we are tracing a trap.

Change traceback to default to "starting with a return PC",
which is the overwhelmingly common case.

Add tracebacktrap, like traceback but starting with a trap PC.

Use tracebacktrap in signal handlers.

Fixes golang#7690.

LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/167810044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@rsc @wheatman
runtime: change top-most return PC from goexit to goexit+PCQuantum
d759e14 
If you get a stack of PCs from Callers, it would be expected
that every PC is immediately after a call instruction, so to find
the line of the call, you look up the line for PC-1.
CL 163550043 now explicitly documents that.

The most common exception to this is the top-most return PC
on the stack, which is the entry address of the runtime.goexit
function. Subtracting 1 from that PC will end up in a different
function entirely.

To remove this special case, make the top-most return PC
goexit+PCQuantum and then implement goexit in assembly
so that the first instruction can be skipped.

Fixes golang#7690.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/170720043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@rsc @wheatman
runtime: update comment for Callers
6fccd7f 
Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes golang#7690.

LGTM=r, chris.cs.guy
R=r, chris.cs.guy
CC=golang-codereviews
https://golang.org/cl/163550043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@rsc @wheatman
runtime: fix line number in first stack frame in printed stack trace
41ac060 
Originally traceback was only used for printing the stack
when an unexpected signal came in. In that case, the
initial PC is taken from the signal and should be used
unaltered. For the callers, the PC is the return address,
which might be on the line after the call; we subtract 1
to get to the CALL instruction.

Traceback is now used for a variety of things, and for
almost all of those the initial PC is a return address,
whether from getcallerpc, or gp->sched.pc, or gp->syscallpc.
In those cases, we need to subtract 1 from this initial PC,
but the traceback code had a hard rule "never subtract 1
from the initial PC", left over from the signal handling days.

Change gentraceback to take a flag that specifies whether
we are tracing a trap.

Change traceback to default to "starting with a return PC",
which is the overwhelmingly common case.

Add tracebacktrap, like traceback but starting with a trap PC.

Use tracebacktrap in signal handlers.

Fixes golang#7690.

LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/167810044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@rsc @wheatman
runtime: change top-most return PC from goexit to goexit+PCQuantum
a1ef344 
If you get a stack of PCs from Callers, it would be expected
that every PC is immediately after a call instruction, so to find
the line of the call, you look up the line for PC-1.
CL 163550043 now explicitly documents that.

The most common exception to this is the top-most return PC
on the stack, which is the entry address of the runtime.goexit
function. Subtracting 1 from that PC will end up in a different
function entirely.

To remove this special case, make the top-most return PC
goexit+PCQuantum and then implement goexit in assembly
so that the first instruction can be skipped.

Fixes golang#7690.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/170720043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@rsc @wheatman
runtime: update comment for Callers
e654450 
Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes golang#7690.

LGTM=r, chris.cs.guy
R=r, chris.cs.guy
CC=golang-codereviews
https://golang.org/cl/163550043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@rsc @wheatman
runtime: fix line number in first stack frame in printed stack trace
aee6bca 
Originally traceback was only used for printing the stack
when an unexpected signal came in. In that case, the
initial PC is taken from the signal and should be used
unaltered. For the callers, the PC is the return address,
which might be on the line after the call; we subtract 1
to get to the CALL instruction.

Traceback is now used for a variety of things, and for
almost all of those the initial PC is a return address,
whether from getcallerpc, or gp->sched.pc, or gp->syscallpc.
In those cases, we need to subtract 1 from this initial PC,
but the traceback code had a hard rule "never subtract 1
from the initial PC", left over from the signal handling days.

Change gentraceback to take a flag that specifies whether
we are tracing a trap.

Change traceback to default to "starting with a return PC",
which is the overwhelmingly common case.

Add tracebacktrap, like traceback but starting with a trap PC.

Use tracebacktrap in signal handlers.

Fixes golang#7690.

LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/167810044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@rsc @wheatman
runtime: change top-most return PC from goexit to goexit+PCQuantum
e1df3cb 
If you get a stack of PCs from Callers, it would be expected
that every PC is immediately after a call instruction, so to find
the line of the call, you look up the line for PC-1.
CL 163550043 now explicitly documents that.

The most common exception to this is the top-most return PC
on the stack, which is the entry address of the runtime.goexit
function. Subtracting 1 from that PC will end up in a different
function entirely.

To remove this special case, make the top-most return PC
goexit+PCQuantum and then implement goexit in assembly
so that the first instruction can be skipped.

Fixes golang#7690.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/170720043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
@rsc @wheatman
runtime: update comment for Callers
7cf70de 
Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes golang#7690.

LGTM=r, chris.cs.guy
R=r, chris.cs.guy
CC=golang-codereviews
https://golang.org/cl/163550043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
@rsc @wheatman
runtime: fix line number in first stack frame in printed stack trace
afa0399 
Originally traceback was only used for printing the stack
when an unexpected signal came in. In that case, the
initial PC is taken from the signal and should be used
unaltered. For the callers, the PC is the return address,
which might be on the line after the call; we subtract 1
to get to the CALL instruction.

Traceback is now used for a variety of things, and for
almost all of those the initial PC is a return address,
whether from getcallerpc, or gp->sched.pc, or gp->syscallpc.
In those cases, we need to subtract 1 from this initial PC,
but the traceback code had a hard rule "never subtract 1
from the initial PC", left over from the signal handling days.

Change gentraceback to take a flag that specifies whether
we are tracing a trap.

Change traceback to default to "starting with a return PC",
which is the overwhelmingly common case.

Add tracebacktrap, like traceback but starting with a trap PC.

Use tracebacktrap in signal handlers.

Fixes golang#7690.

LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/167810044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
@rsc @wheatman
runtime: change top-most return PC from goexit to goexit+PCQuantum
8ef0e38 
If you get a stack of PCs from Callers, it would be expected
that every PC is immediately after a call instruction, so to find
the line of the call, you look up the line for PC-1.
CL 163550043 now explicitly documents that.

The most common exception to this is the top-most return PC
on the stack, which is the entry address of the runtime.goexit
function. Subtracting 1 from that PC will end up in a different
function entirely.

To remove this special case, make the top-most return PC
goexit+PCQuantum and then implement goexit in assembly
so that the first instruction can be skipped.

Fixes golang#7690.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/170720043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@rsc @wheatman
runtime: update comment for Callers
917020e 
Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes golang#7690.

LGTM=r, chris.cs.guy
R=r, chris.cs.guy
CC=golang-codereviews
https://golang.org/cl/163550043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@rsc @wheatman
runtime: fix line number in first stack frame in printed stack trace
5942e94 
Originally traceback was only used for printing the stack
when an unexpected signal came in. In that case, the
initial PC is taken from the signal and should be used
unaltered. For the callers, the PC is the return address,
which might be on the line after the call; we subtract 1
to get to the CALL instruction.

Traceback is now used for a variety of things, and for
almost all of those the initial PC is a return address,
whether from getcallerpc, or gp->sched.pc, or gp->syscallpc.
In those cases, we need to subtract 1 from this initial PC,
but the traceback code had a hard rule "never subtract 1
from the initial PC", left over from the signal handling days.

Change gentraceback to take a flag that specifies whether
we are tracing a trap.

Change traceback to default to "starting with a return PC",
which is the overwhelmingly common case.

Add tracebacktrap, like traceback but starting with a trap PC.

Use tracebacktrap in signal handlers.

Fixes golang#7690.

LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/167810044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@rsc @wheatman
runtime: change top-most return PC from goexit to goexit+PCQuantum
8e1ea6f 
If you get a stack of PCs from Callers, it would be expected
that every PC is immediately after a call instruction, so to find
the line of the call, you look up the line for PC-1.
CL 163550043 now explicitly documents that.

The most common exception to this is the top-most return PC
on the stack, which is the entry address of the runtime.goexit
function. Subtracting 1 from that PC will end up in a different
function entirely.

To remove this special case, make the top-most return PC
goexit+PCQuantum and then implement goexit in assembly
so that the first instruction can be skipped.

Fixes golang#7690.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/170720043
@rsc rsc removed their assignment Jun 23, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4
Development

No branches or pull requests
4 participants
@rsc @ChrisHines @ianlancetaylor @gopherbot