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.

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

runtime: "runtime: unexpected return pc" when doing CPU profiling on ARM #6681

Closed
josharian opened this issue Oct 28, 2013 · 14 comments
Closed

runtime: "runtime: unexpected return pc" when doing CPU profiling on ARM #6681

josharian opened this issue Oct 28, 2013 · 14 comments
Labels
FrozenDueToAge
Milestone
Go1.2

Comments

@josharian
Copy link
Contributor 

Run http://play.golang.org/p/nORLXBUuEL on an ARM machine.

This yields a slew of "unexpected return pc"s. Here's a sampling of them:

runtime: unexpected return pc for runtime.makeslice called from 0xca6d0         
runtime: unexpected return pc for makeslice1 called from 0xca6d0                
runtime: unexpected return pc for math/big.(*Int).Div called from 0x1054ace0    
runtime: unexpected return pc for math/big.(*Int).QuoRem called from 0x0        
runtime: unexpected return pc for runtime.makeslice called from 0xbb10cb77      
runtime: unexpected return pc for runtime.new called from 0x10577220            
runtime: unexpected return pc for runtime.main called from 0x10500130           
runtime: unexpected return pc for runtime.cnewarray called from 0x40101ecc      
runtime: unexpected return pc for _divu called from 0xbb10cb77                 


Which version are you using?  (run 'go version')

go version devel +f9af8b83c78c Fri Oct 25 23:00:22 2013 +0300 darwin/amd64


Please provide any additional information below.

Reproduces on two linux/arm5 systems, one of which is a stock Raspberry Pi. Doesn't
reproduce on OS X or linux/amd64.
@ianlancetaylor
Copy link
Member

Comment 1:

It's pretty late for 1.2 but it would be nice if somebody could at least look into this.

Labels changed: added go1.2maybe.

@davecheney
Copy link
Contributor

Comment 2:

Is this a regression? https://golang.org/issue/6015

@josharian
Copy link
Contributor Author

Comment 3:

> Is this a regression? https://golang.org/issue/6015
Doesn't look like it. It reproduces with all of:
5ffdb9cc0bfe Tue Oct 29 -- tip
6112d74a00fa Fri Sep 13 -- commit that fixed issue #6015
a5eb738ff649 Fri Aug 02 -- earliest commit that will cross-compile on recent OS X
I can do a hg bisect further into the past tomorrow, if that'd be useful.

@ianlancetaylor
Copy link
Member

Comment 4:

Thanks for looking into it.  Since it's not a regression, switching target to 1.3.

Labels changed: added go1.3maybe, removed go1.2maybe.

@dvyukov
Copy link
Member

dvyukov commented Oct 29, 2013

Comment 5:

I believe this is a regression against Go1.1.
Go1.1. does not have that "unexpected return pc for" message at all.

@josharian
Copy link
Contributor Author

Comment 6:

Sorry, Dave, I misinterpreted your question as asking whether it was a regression since
issue #6015.
Dmitry is correct. I checked, and this is a regression from Go 1.1.2. (1.1.2 needs a
runtime.Gosched added -- http://play.golang.org/p/q8CxPa5WBa)

@rsc
Copy link
Contributor

rsc commented Oct 30, 2013

Comment 7:

Labels changed: added priority-later, go1.2, removed priority-triage, go1.3maybe.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Oct 30, 2013

Comment 8:

This is an excellent test program. Thank you.

@rsc
Copy link
Contributor

rsc commented Oct 30, 2013

Comment 9:

This issue was closed by revision b0db472.

Status changed to Fixed.

@rsc
Copy link
Contributor

rsc commented Oct 31, 2013

Comment 10:

This issue was closed by revision b88148b.

@rsc
Copy link
Contributor

rsc commented Oct 31, 2013

Comment 11:

The issue was actually reopened by the revision in #10, which rolled back the fix from
#9.
I have a new fix on the way.
The original fix is still a good idea, just not for Go 1.2. See issue #6699.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Oct 31, 2013

Comment 12:

This issue was closed by revision 2c98a3b.

Status changed to Fixed.

@adg
Copy link
Contributor

adg commented Nov 1, 2013

Comment 13:

This issue was closed by revision 29d07b807b75.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
@josharian josharian mentioned this issue Apr 17, 2015
Closed
adg added a commit that referenced this issue May 11, 2015
@adg
[release-branch.go1.2] cmd/5l, runtime: fix divide for profiling trac…
4708b76 
…ebacks on ARM

««« CL 19910044 / 9eb64f5ef3a6
cmd/5l, runtime: fix divide for profiling tracebacks on ARM

Two bugs:
1. The first iteration of the traceback always uses LR when provided,
which it is (only) during a profiling signal, but in fact LR is correct
only if the stack frame has not been allocated yet. Otherwise an
intervening call may have changed LR, and the saved copy in the stack
frame should be used. Fix in traceback_arm.c.

2. The division runtime call adds 8 bytes to the stack. In order to
keep the traceback routines happy, it must copy the saved LR into
the new 0(SP). Change

        SUB $8, SP

into

        MOVW    0(SP), R11 // r11 is temporary, for use by linker
        MOVW.W  R11, -8(SP)

to update SP and 0(SP) atomically, so that the traceback always
sees a saved LR at 0(SP).

Fixes #6681.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/19910044
»»»

R=golang-dev
CC=golang-dev
https://golang.org/cl/20170048
@gopherbot
Copy link
Contributor

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

rsc added a commit that referenced this issue Jul 30, 2015
@rsc
runtime: change arm software div/mod call sequence not to modify stack
b2dfacf 
Instead of pushing the denominator argument on the stack,
the denominator is now passed in m.

This fixes a variety of bugs related to trying to take stack traces
backwards from the middle of the software div/mod routines.
Some of those bugs have been kludged around in the past,
but others have not. Instead of trying to patch up after breaking
the stack, this CL stops breaking the stack.

This is an update of https://golang.org/cl/19810043,
which was rolled back in https://golang.org/cl/20350043.

The problem in the original CL was that there were divisions
at bad times, when m was not available. These were divisions
by constant denominators, either in C code or in assembly.
The Go compiler knows how to generate division by multiplication
for constant denominators, but the C compiler did not.
There is no longer any C code, so that's taken care of.
There was one problematic DIV in runtime.usleep (assembly)
but https://golang.org/cl/12898 took care of that one.
So now this approach is safe.

Reject DIV/MOD in NOSPLIT functions to keep them from
coming back.

Fixes #6681.
Fixes #6699.
Fixes #10486.

Change-Id: I09a13c76ad08ba75b3bd5d46a3eb78e66a84ab38
Reviewed-on: https://go-review.googlesource.com/12899
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Aug 5, 2016
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.2
Development

No branches or pull requests
7 participants
@davecheney @josharian @rsc @dvyukov @ianlancetaylor @adg @gopherbot