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

cmd/cgo: link error when binding C.malloc to a Go variable #18889

Closed
bcmills opened this issue Feb 1, 2017 · 6 comments
Closed

cmd/cgo: link error when binding C.malloc to a Go variable #18889

bcmills opened this issue Feb 1, 2017 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 1, 2017

This program fails to build at the linking step:

src/mallocvar/mallocvar.go:

package main

/*
#include <stdlib.h>
*/
import "C"
import "fmt"

var malloc = C.malloc

func main() {
        fmt.Println("%T", malloc)
}
bcmills:~$ go version
go version devel +c47df7ae17 Tue Jan 31 13:01:31 2017 -0500 linux/amd64
bcmills:~$ go build mallocvar
# mallocvar
/tmp/go-build119669921/mallocvar/_obj/_cgo_main.o:(.data.rel+0x0): undefined reference to `_CMalloc'
collect2: error: ld returned 1 exit status

If the program is invalid, it should be rejected earlier and produce a more helpful error message. (For example, changing C.malloc to C.monkeypants produces the message could not determine kind of name for C.monkeypants, which is arguably more helpful.)

If the program is valid, it should link successfully.

(attn: @ianlancetaylor)

@ianlancetaylor
Copy link
Contributor

Note that it works if you don't use the magic function malloc, and if you use a function that is actually defined.

@bcmills
Copy link
Contributor Author

bcmills commented Feb 2, 2017

Hmm, interesting! Didn't occur to me that malloc would be a magic function, since I included stdlib.h explicitly...

@bcmills bcmills changed the title cgo: link error when binding C functions to Go variables cgo: link error when binding C.malloc to a Go variable Feb 2, 2017
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 27, 2017
@quentinmit quentinmit added this to the Go1.9Maybe milestone Feb 27, 2017
@amenzhinsky
Copy link
Contributor

amenzhinsky commented Apr 15, 2017

cgo converts malloc to the _CMalloc built-in and none of built-ins can be used not in a function call now because technically they're go functions, opposed to C's unsafe.Pointer.

Should we support using their references or an error message would be enough?

@bcmills
Copy link
Contributor Author

bcmills commented Apr 16, 2017

An error message would probably suffice.

IMO it's odd that we wrap malloc at all, given that it doesn't refer to any Go types. From #3403 (comment) it appears that the original reason was to work around the function having a non-standard signature on OS X, but it seems like a simpler fix would have been to hard-code the standard signature instead of shimming in a function, or only shim in the function on the platform(s) that have a nonstandard malloc, or name the shim with something other than the name of the standard function. But I'm guessing it's probably too late to change any of that in Go 1.

@ianlancetaylor
Copy link
Contributor

At this point, I know of two reasons for the continued special wrapping of C.malloc:

  1. So that it works even if the magic C comment doesn't #include <stdlib.h>.
  2. So that it never returns nil.

@gopherbot
Copy link

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

@ianlancetaylor ianlancetaylor changed the title cgo: link error when binding C.malloc to a Go variable cmd/cgo: link error when binding C.malloc to a Go variable Apr 18, 2017
@golang golang locked and limited conversation to collaborators Apr 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants