Skip to content
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.

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

runtime: Stack captures incorrect line numbers #7690

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
Milestone

Comments

@gopherbot
Copy link

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
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
Contributor

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
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
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
Author

Comment 8:

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

@gopherbot
Copy link
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
Projects
None yet
Development

No branches or pull requests

4 participants