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: don't accept mangled C names in Cgo programs #28721

Closed
wzshiming opened this issue Nov 11, 2018 · 10 comments
Closed

cmd/cgo: don't accept mangled C names in Cgo programs #28721

wzshiming opened this issue Nov 11, 2018 · 10 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@wzshiming
Copy link

What version of Go are you using (go version)?

go version go1.11.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
GOARCH="amd64"
GOBIN="/Users/zsm/go/bin"
GOCACHE="/Users/zsm/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/zsm/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/6v/7stmg2756wlfk9c_qnv1hnbm0000gn/T/go-build905706456=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package test

/*
typedef struct a {
	int i;
} a;

a* A() {
	return NULL;
};
*/
import "C"

type a struct {
	a *_Ctype_struct_a
}

func A() *a {
	i := C.A()
	return &a{i}
}

$ gomobile bind test

What did you expect to see?

Building succeeds.

What did you see instead?

gomobile: /Users/zsm/go/bin/gobind -lang=go,java -outdir=/var/folders/6v/7stmg2756wlfk9c_qnv1hnbm0000gn/T/gomobile-work-058021600 test failed: exit status 1
type-checking package "test" failed (/Users/zsm/go/src/test/main.go:15:5: undeclared name: _Ctype_struct_a)

@gopherbot gopherbot added this to the Unreleased milestone Nov 11, 2018
@gopherbot gopherbot added the mobile Android, iOS, and x/mobile label Nov 11, 2018
@wzshiming
Copy link
Author

#24941
And this is the same kind of problem.

@AlexRouSg
Copy link
Contributor

What happens when you do this instead?

type a struct {
	a *C.a
}

@wzshiming
Copy link
Author

@AlexRouSg Building succeeds.

@AlexRouSg
Copy link
Contributor

Any particular reason you're using a *_Ctype_struct_a?

@wzshiming
Copy link
Author

wzshiming commented Nov 11, 2018

A third-party package I used is like this.
There was no problem with previous use but gomobile-bind does.

Debugging finds this.
_Ctype_struct_a not in Go-struct field is no problem.
Why can't put a Go-struct field.

@AlexRouSg
Copy link
Contributor

cc @eliasnaur

@eliasnaur
Copy link
Contributor

I believe the bug is that Go accepts the program, but shouldn't, while go/types correctly rejects the program. _Ctype_struct_a is not a defined type, and the program only builds because of the way Cgo mangles C type names. @ianlancetaylor do you agree? If so, should Go reject such programs?

I believe the correct fix is to change to program to use C.a instead, as suggested by AlexRouSg.

@eliasnaur eliasnaur changed the title x/mobile: gomobile bind fails, have C-struct in the Go-struct field cmd/cgo: don't accept mangled C names in Cgo programs Nov 21, 2018
@eliasnaur eliasnaur removed the mobile Android, iOS, and x/mobile label Nov 21, 2018
@eliasnaur
Copy link
Contributor

Ian, I changed the title to match what I believe is the root cause. Feel free to throw it back to me if I'm mistaken.

@ianlancetaylor
Copy link
Member

Sorry, somehow I missed this earlier. I agree: cgo should reject the attempt to use _Ctype_struct_a.

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Nov 21, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Unreleased, Unplanned Nov 21, 2018
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/152657 mentions this issue: cmd/cgo: reject names that are likely to be mangled C name

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted 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