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

syscall: darwin/386 programs crash #7142

Closed
dvyukov opened this issue Jan 17, 2014 · 1 comment
Closed

syscall: darwin/386 programs crash #7142

dvyukov opened this issue Jan 17, 2014 · 1 comment
Assignees
Milestone

Comments

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Jan 17, 2014

After https://golang.org/cl/45930043
lots of programs crash on darwin/386 with:

panic: runtime error: call of nil func value
[signal 0xa code=0x2 addr=0x0 pc=0x0]

The crash is easily reproducible with:
$ go test -run=TestParseGlob text/template

The culprit is:

func ReadDirent(fd int, buf []byte) (n int, err error) {
    // Final argument is (basep *uintptr) and the syscall doesn't take nil.
    // TODO(rsc): Can we use a single global basep for all calls?
    return Getdirentries(fd, buf, new(uintptr))
}

The compiler seems to think that new(uintptr) is not alive during syscall. And combines
it... with return address? And then syscall writes 0 to return address.

With the following fix 'go test std' passes:

+var xx **uintptr
+
 func ReadDirent(fd int, buf []byte) (n int, err error) {
    // Final argument is (basep *uintptr) and the syscall doesn't take nil.
    // TODO(rsc): Can we use a single global basep for all calls?
-   return Getdirentries(fd, buf, new(uintptr))
+   x := new(uintptr)
+   if *x != 0 {
+       xx = &x
+   }
+   return Getdirentries(fd, buf, x)
 }

It seems wrong that compiler think that the var is not alive. And it probably could
break w/o the escape-related change, because liveness is orthogonal to escape-ness.

Russ, what do you think?
@rsc
Copy link
Contributor

@rsc rsc commented Apr 3, 2014

Comment 1:

This was fixed in CL https://golang.org/cl/53840043.
I don't believe you that the compiler thought new(uintptr) is not alive during the
system call. It was definitely alive, it was just passing a pointer to a stack value now
instead of a heap-allocated value. And on 32-bit systems uintptr is only 4 bytes but the
kernel wanted (and wrote) 8. It only worked before because if you asked the memory
allocator for 4 bytes you got at least 8 anyway.

Status changed to Fixed.

@dvyukov dvyukov added fixed labels Apr 3, 2014
@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.