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

cmd/cgo: add test for passing arguments with typedefs compatible in C #23720

Closed
liggitt opened this issue Feb 6, 2018 · 5 comments
Closed

cmd/cgo: add test for passing arguments with typedefs compatible in C #23720

liggitt opened this issue Feb 6, 2018 · 5 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@liggitt
Copy link
Contributor

liggitt commented Feb 6, 2018 •
edited

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10rc1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jliggitt/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/jliggitt/go"
GORACE=""
GOROOT="/Users/jliggitt/.gvm/gos/go1.10rc1"
GOTMPDIR=""
GOTOOLDIR="/Users/jliggitt/.gvm/gos/go1.10rc1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
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/lw/0m4r59zx3_b56vbmr2j0zkbr0000gn/T/go-build814899031=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Tried to build/test a cgo package that compiled successfully under previous versions of go

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

Seen in github.com/apcera/gssapi, but reproduceable with similar typedefs in this program:

package main

/*
typedef int *A;

typedef const int *B;

int testfunc(B f) {
	return 1;
}
*/
import "C"

func main() {
	var x C.A
	C.testfunc(x)
}

What did you expect to see?

with go1.9.2:

go version
go version go1.9.2 darwin/amd64

cd $GOPATH/src/github.com/apcera/gssapi

go test ./
ok  	github.com/apcera/gssapi	0.049s

What did you see instead?

with go1.10rc1:

go version
go version go1.10rc1 darwin/amd64

cd $GOPATH/src/github.com/apcera/gssapi

go test ./
# _/Users/jliggitt/go/src/github.com/apcera/gssapi
./name.go:214: cannot use n.C_gss_name_t (type _Ctype_gss_name_t) as type _Ctype_gss_const_name_t in argument to func literal
FAIL	_/Users/jliggitt/go/src/github.com/apcera/gssapi [build failed]

the coercion of a gss_name_t (typedef struct gss_name_struct) to gss_const_name_t (typedef const struct gss_name_struct) previously worked as expected, but go1.10rc1 now complains about it and requires an explicit cast.

gss_name_t: http://www.gnu.org/software/gss/reference/gss-api.html#gss-name-t

gss_const_name_t: http://www.gnu.org/software/gss/reference/gss-api.html#gss-const-name-t

xref https://github.com/apcera/gssapi/issues/54
@ianlancetaylor
Copy link
Contributor

I believe this is due to the fix for #19832.

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Feb 6, 2018
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 6, 2018
@gopherbot
Copy link

Change https://golang.org/cl/92455 mentions this issue: cmd/cgo: revert CL 49490 "fix for function taking pointer typedef"

@liggitt
Copy link
Contributor Author

liggitt commented Feb 7, 2018 •
edited

@ianlancetaylor many thanks for the quick response/revert. Is it possible to add a test for the broken case to ensure it doesn't slip?

@ianlancetaylor
Copy link
Contributor

@liggitt In 1.11, sure.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Go1.11 Feb 7, 2018
@ianlancetaylor ianlancetaylor changed the title cgo function argument coercion broken in 1.10rc1 cmd/cgo: add test for passing arguments with typedefs compatible in C Feb 7, 2018
@ianlancetaylor ianlancetaylor mentioned this issue Feb 7, 2018
Open
@ianlancetaylor ianlancetaylor reopened this Feb 7, 2018
@gopherbot
Copy link

Change https://golang.org/cl/118737 mentions this issue: misc/cgo/test: add test for passing compatible typedefs

dna2github pushed a commit to dna2fork/go that referenced this issue Jun 14, 2018
@ianlancetaylor @dna2github
misc/cgo/test: add test for passing compatible typedefs
a5d320f 
Fixes golang#23720

Change-Id: I28e0c16503bc043f793e0dab19668f7a66313312
Reviewed-on: https://go-review.googlesource.com/118737
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
3 participants
@liggitt @ianlancetaylor @gopherbot