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

sync/atomic: arm CompareAndSwap backtraces fail #4422

Closed
remyoudompheng opened this issue Nov 21, 2012 · 12 comments
Closed

sync/atomic: arm CompareAndSwap backtraces fail #4422

remyoudompheng opened this issue Nov 21, 2012 · 12 comments
Labels
FrozenDueToAge
Milestone
Go1.1

Comments

@remyoudompheng
Copy link
Contributor 

What steps will reproduce the problem?
1. go run the following program

package main

import (
      "sync"
)

func main() {
      var lock *sync.Mutex
      lock.Lock()
}

What is the expected output? What do you see instead?

Expected: something looking roughly like the x86 case

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x4d900d]

goroutine 1 [running]:
sync/atomic.CompareAndSwapUint32(0x0, 0x100000000, 0xc200079870, 0x4, 0x4d6d95, ...)
    /opt/remy/go/src/pkg/sync/atomic/asm_amd64.s:14 +0xd
sync.(*Mutex).Lock(0x0, 0x76eda8)
    /opt/remy/go/src/pkg/sync/mutex.go:43 +0x35
sync.(*RWMutex).Lock(0x0, 0x0)
    /opt/remy/go/src/pkg/sync/rwmutex.go:75 +0x25
net/http.(*ServeMux).Handle(0x0, 0x6247a0, 0x4, 0xc200079870, 0x400c50, ...)
    /opt/remy/go/src/pkg/net/http/server.go:998 +0x30
net/http.(*ServeMux).HandleFunc(0x0, 0x6247a0, 0x4, 0x400c50, 0x400c84, ...)
    /opt/remy/go/src/pkg/net/http/server.go:1036 +0x58
main.Register(0x0, 0x400f5a)
    /home/remy/travail/go/src/teststack/irc.go:8 +0x43
main.init·1()
    /home/remy/travail/go/src/teststack/webtoys.go:15 +0x24
main.init()
    /home/remy/travail/go/src/teststack/webtoys.go:33 +0xaa

goroutine 2 [syscall]:
created by runtime.main
    /opt/remy/go/src/pkg/runtime/proc.c:225

Got: empty stack trace

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0xffff0fc4]

goroutine 1 [running]:

goroutine 2 [syscall]:
created by runtime.main
    /storage/remy/go/src/pkg/runtime/proc.c:225
exit status 2

Also gdb shows corrupted output:

(gdb) run
Starting program: /home/remy/go/src/teststack/lock 
[New LWP 6399]

Program received signal SIGSEGV, Segmentation fault.
[Switching to LWP 6399]
0xffff0fc4 in ?? ()
(gdb) bt
#0  0xffff0fc4 in ?? ()
#1  0x00025150 in sync/atomic.CompareAndSwapUint32 (addr=void, old=void, new=void,
ret=void)
    at /storage/remy/go/src/pkg/sync/atomic/asm_linux_arm.s:36
#2  0x00025150 in sync/atomic.CompareAndSwapUint32 (addr=void, old=void, new=void,
ret=void)
    at /storage/remy/go/src/pkg/sync/atomic/asm_linux_arm.s:36
#3  0x00025150 in sync/atomic.CompareAndSwapUint32 (addr=void, old=void, new=void,
ret=void)
    at /storage/remy/go/src/pkg/sync/atomic/asm_linux_arm.s:36
#4  0x00025150 in sync/atomic.CompareAndSwapUint32 (addr=void, old=void, new=void,
ret=void)
    at /storage/remy/go/src/pkg/sync/atomic/asm_linux_arm.s:36
#5  0x00025150 in sync/atomic.CompareAndSwapUint32 (addr=void, old=void, new=void,
ret=void)
    at /storage/remy/go/src/pkg/sync/atomic/asm_linux_arm.s:36


Please use labels and text to provide additional information.
@remyoudompheng
Copy link
Contributor Author

Comment 1:

The issue initially arose when calling HandleFunc on a nil *http.ServeMux.

@minux
Copy link
Member

minux commented Nov 25, 2012

Comment 2:

the sigsegv happened in kernel provided cas primitive and it doesn't use our standard
stack frame so it confused the back tracer.
perhaps we can force a read in sync/atomic CompareAndSwapUint32 so the the signal
is received earlier.
diff -r bbc0024af4a4 src/pkg/sync/atomic/asm_linux_arm.s
--- a/src/pkg/sync/atomic/asm_linux_arm.s       Sun Nov 18 15:31:26 2012 +1100
+++ b/src/pkg/sync/atomic/asm_linux_arm.s       Sun Nov 25 00:20:48 2012 +0000
@@ -30,6 +30,7 @@
 // Implement using kernel cas for portability.
 TEXT ·CompareAndSwapUint32(SB),7,$0
        MOVW    addr+0(FP), R2
+       MOVW    (R2), R1 // force a read here
        MOVW    old+4(FP), R0
 casagain:
        MOVW    new+8(FP), R1

Labels changed: added priority-later, go1.1, arch-arm, removed priority-triage, go1.1maybe.

Status changed to Accepted.

@remyoudompheng
Copy link
Contributor Author

Comment 3:

Yes, that's what I did locally and it's much better.

@minux
Copy link
Member

minux commented Nov 25, 2012

Comment 4:

as an alternative, we can fix the backtracer to recognize this case.
which solution do you think is better?

@remyoudompheng
Copy link
Contributor Author

Comment 5:

You would need to fix both Go's backtracer and GDB's backtracer then. Do you know how to
do that?
If it gets too complicated, I would say a dummy instruction in the ASM would be useful.
I don't know what the performance impact would be.

@davecheney
Copy link
Contributor

Comment 6:

I think we should to Remy's solution, a single MOVW won't hurt performance, the call
stack for Arm's CAS is quite long.

@rsc
Copy link
Contributor

rsc commented Dec 10, 2012

Comment 7:

Rémy, want to send this 1-line fix as a CL? LGTM.

@rsc
Copy link
Contributor

rsc commented Dec 10, 2012

Comment 9:

Labels changed: added size-s.

@davecheney
Copy link
Contributor

Comment 10:

https://golang.org/cl/6939056

Owner changed to @davecheney.

Status changed to Started.

@davecheney
Copy link
Contributor

Comment 11:

I've not been able to solve the problem for the CAS64 case, if others want to pick this
up, please consider https://golang.org/cl/6939056/ as a starting point.

Owner changed to ---.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Feb 14, 2013

Comment 12:

This was fixed by 
changeset:   15628:8f547c432c68
user:        Shenghou Ma <minux.ma@gmail.com>
date:        Wed Feb 06 01:18:37 2013 +0800
summary:     runtime: save LR to stack when panicking to handle leaf function traceback
right?

@minux
Copy link
Member

minux commented Feb 14, 2013

Comment 13:

right.

Status changed to Fixed.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 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.1
Development

No branches or pull requests
5 participants
@davecheney @rsc @minux @remyoudompheng @gopherbot