Skip to content

cmd/gc: Camlistore crash after recent gc change #8961

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
bradfitz opened this issue Oct 19, 2014 · 10 comments
Closed

cmd/gc: Camlistore crash after recent gc change #8961

bradfitz opened this issue Oct 19, 2014 · 10 comments
Labels
FrozenDueToAge
Milestone
Go1.4

Comments

@bradfitz
Copy link
Contributor 

The change https://golang.org/cl/152700045/ ("cmd/gc: don't use static
init to initialize small structs, fields") broke Camlistore.

Caught by our buildbot, but verified by hand before (okay) & at that commit, failing
with:

$ go version
go version devel +28de6f41b1c7 Fri Oct 17 13:10:42 2014 -0400 darwin/amd64

$ go test camlistore.org/cmd/camput
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x4065a1c]

goroutine 39 [running]:
camlistore.org/cmd/camput.(*blobCmd).RunCommand(0x496bb20, 0xc208031730, 0x1, 0x1, 0x0,
0x0)
    /Users/bradfitz/src/camlistore.org/cmd/camput/blobs.go:76 +0x25c
camlistore.org/pkg/cmdmain.Main()
    /Users/bradfitz/src/camlistore.org/pkg/cmdmain/cmdmain.go:273 +0xb27
camlistore.org/cmd/camput.func·029()
    /Users/bradfitz/src/camlistore.org/cmd/camput/camput_test.go:67 +0x1f
created by camlistore.org/cmd/camput.(*env).Run
    /Users/bradfitz/src/camlistore.org/cmd/camput/camput_test.go:69 +0x450

goroutine 1 [chan receive]:
testing.RunTests(0x4763548, 0x4968300, 0x5, 0x5, 0xc208030b01)
    /Users/bradfitz/go/src/testing/testing.go:556 +0xad6
testing.(*M).Run(0xc20802e5a0, 0x496a980)
    /Users/bradfitz/go/src/testing/testing.go:485 +0x6c
main.main()
    camlistore.org/cmd/camput/_test/_testmain.go:60 +0x1d5

goroutine 38 [select]:
camlistore.org/cmd/camput.(*env).Run(0xc2080e81a0, 0xc208157d98, 0x5, 0x5, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, ...)
    /Users/bradfitz/src/camlistore.org/cmd/camput/camput_test.go:70 +0x63d
camlistore.org/cmd/camput.func·033(0xc208162480, 0x3a)
    /Users/bradfitz/src/camlistore.org/cmd/camput/camput_test.go:224 +0x2e3
camlistore.org/cmd/camput.testWithTempDir(0xc2081053b0, 0xc208157f28)
    /Users/bradfitz/src/camlistore.org/cmd/camput/camput_test.go:140 +0x534
camlistore.org/cmd/camput.TestCamputBlob(0xc2081053b0)
    /Users/bradfitz/src/camlistore.org/cmd/camput/camput_test.go:231 +0x136
testing.tRunner(0xc2081053b0, 0x4968360)
    /Users/bradfitz/go/src/testing/testing.go:447 +0xbf
created by testing.RunTests
    /Users/bradfitz/go/src/testing/testing.go:555 +0xa8b

goroutine 33 [syscall, locked to thread]:
runtime.goexit()
    /Users/bradfitz/go/src/runtime/proc.c:1651
FAIL    camlistore.org/cmd/camput   0.166s
@DanielMorsing
Copy link
Contributor

Comment 2:

Minimal reproducer. Amazed that nothing else caught this.
package main
import (
    "fmt"
    "sync"
)
var once sync.Once
func main() {
    once.Do(foo)
    once = sync.Once{}
    once.Do(foo)
}
func foo() {
    fmt.Println("twice")
}

@gopherbot
Copy link
Contributor

Comment 3:

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

@DanielMorsing
Copy link
Contributor

Comment 4:

This issue was closed by revision 0edafef.

Status changed to Fixed.

@rsc
Copy link
Contributor

rsc commented Oct 28, 2014

Comment 5:

Issue #8911 has been merged into this issue.

@alexbrainman
Copy link
Member

Comment 6:

Issue #8911 has been merged into this issue.

@alexbrainman
Copy link
Member

Comment 7:

I don't think this is fixed by revision 98477b56eb36. We should reopen this issue.
Alex

@rsc
Copy link
Contributor

rsc commented Oct 29, 2014

Comment 8:

Brad, is it fixed?

@bradfitz
Copy link
Contributor Author

Comment 9:

Yes, http://build.camlistore.org/ went "ok" right that change.

@alexbrainman
Copy link
Member

Comment 10:

I take it back. I really don't know if it has been fixed by revision 98477b56eb36. I was
confused by https://golang.org/issue/8911?c=4 comment on a dup
issue. The comment is (I can see it now) unrelated to this issue. Sorry.
Alex

@alexbrainman
Copy link
Member

Comment 11:

Issue #8911 has been merged into this issue.

@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
@DanielMorsing @wheatman
cmd/gc: emit code for extern = <N>
0f9019d 
https://golang.org/cl/152700045/ made it possible for struct literals assigned to globals to use <N> as the RHS. Normally, this is to zero out variables on first use. Because globals are already zero (or their linker initialized value), we just ignored this.

Now that <N> can occur from non-initialization code, we need to emit this code. We don't use <N> for initialization of globals any more, so this shouldn't cause any excessive zeroing.

Fixes golang#8961.

LGTM=rsc
R=golang-codereviews, rsc
CC=bradfitz, golang-codereviews
https://golang.org/cl/154540044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@DanielMorsing @wheatman
cmd/gc: emit code for extern = <N>
5bc9f22 
https://golang.org/cl/152700045/ made it possible for struct literals assigned to globals to use <N> as the RHS. Normally, this is to zero out variables on first use. Because globals are already zero (or their linker initialized value), we just ignored this.

Now that <N> can occur from non-initialization code, we need to emit this code. We don't use <N> for initialization of globals any more, so this shouldn't cause any excessive zeroing.

Fixes golang#8961.

LGTM=rsc
R=golang-codereviews, rsc
CC=bradfitz, golang-codereviews
https://golang.org/cl/154540044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@DanielMorsing @wheatman
cmd/gc: emit code for extern = <N>
4021efa 
https://golang.org/cl/152700045/ made it possible for struct literals assigned to globals to use <N> as the RHS. Normally, this is to zero out variables on first use. Because globals are already zero (or their linker initialized value), we just ignored this.

Now that <N> can occur from non-initialization code, we need to emit this code. We don't use <N> for initialization of globals any more, so this shouldn't cause any excessive zeroing.

Fixes golang#8961.

LGTM=rsc
R=golang-codereviews, rsc
CC=bradfitz, golang-codereviews
https://golang.org/cl/154540044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@DanielMorsing @wheatman
cmd/gc: emit code for extern = <N>
c89f881 
https://golang.org/cl/152700045/ made it possible for struct literals assigned to globals to use <N> as the RHS. Normally, this is to zero out variables on first use. Because globals are already zero (or their linker initialized value), we just ignored this.

Now that <N> can occur from non-initialization code, we need to emit this code. We don't use <N> for initialization of globals any more, so this shouldn't cause any excessive zeroing.

Fixes golang#8961.

LGTM=rsc
R=golang-codereviews, rsc
CC=bradfitz, golang-codereviews
https://golang.org/cl/154540044
@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
5 participants
@bradfitz @rsc @DanielMorsing @gopherbot @alexbrainman