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: _cgopackage.Incomplete when accessing C function starting with "union_" #68682

Closed
facingfrost opened this issue Jul 31, 2024 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@facingfrost
Copy link

Go version

go version go1.22.4 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/wanglinhan/Library/Caches/go-build'
GOENV='/Users/wanglinhan/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/wanglinhan/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/wanglinhan/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/r_/l0_2w8jx07xg37vpsf72k0pw0000gn/T/go-build3228678536=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

My colleagues(@mschoema, @Davichet-e) and I are trying to implement Go wrapper for MEOS C library, using CGO to access C function by CGO. However, we found that when a C function starts with "union_", cgo is not able to access it. Below is a minimal example:

//example.go
package main

/*
#include <stdio.h>
void union_test(){
	printf("union_test success!");
}
void test_union(){
	printf("test_union success!");
}
*/
import "C"


func main() {
	C.test_union()
	C.union_test()
}

This is two very simple C functions that print out strings, so in theory they should work. However, we see errors.

What did you see happen?

When running the go file, I got the following error:

./example.go:16:2: missing argument in conversion to cgo.Incomplete

However, when I comment the C.union_test() and run the following:

//example.go
package main

/*
#include <stdio.h>
void union_test(){
	printf("union_test success!");
}
void test_union(){
	printf("test_union success!");
}
*/
import "C"


func main() {
	C.test_union()
	//C.union_test()
}

The output is test_union success!. The only difference between this two function is union_test() starts with union_.

I think this is a bug because it means any C function starts with union_ can not be accessed by cgo. The function naming like this works in both C and Go, but not accepted by CGO. So it's very hard to make wrappers for existing geospatial C library like MEOS by CGO.

I'm not very familiar with go codebase, but the code here may be relavant.

What did you expect to see?

I expect both C.test_union() and C.union_test() works properly and we can see the output:

test_union success!
union_test success!
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 31, 2024
@mknyszek
Copy link
Contributor

CC @golang/compiler

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 31, 2024
@mknyszek mknyszek added this to the Backlog milestone Jul 31, 2024
@ianlancetaylor
Copy link
Member

Unfortunately, in cgo, C.union_U is a reference to the C type union U. See https://pkg.go.dev/cmd/cgo#hdr-Go_references_to_C. We can't reasonably change that today; it would surely break existing working programs. So unfortunately I think that you will need to use a different name for this case.

Closing because I don't think there is anything we can do. Please comment if you disagree.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2024
@Davichet-e
Copy link

Davichet-e commented Jul 31, 2024

First of all state that I'm completely new to the Go ecosystem so I may be missing something here, but isn't weird that a core library to a language so widely used just doesn't support functions starting with specific prefixes? Naively, any function that you can create in C, you should be able to use it in Go, as it's the ultimate purpose of CGo AFAIK. Also this limitation doesn't exist in other languages such as Rust (bindgen)

Also I don't fully understand the issue, I understand that in CGo, you can access an union using the syntax C.union_UNION_NAME, but why would that disallow a function that starts with union_ to exist in the first place? I don't see how that would affect already working code. Either way, even if that would indeed break existing code, wouldn't it be ways around it? For example in Rust, the CGo equivalent, bindgen, is configurable and you can choose how are things translated into Rust. Wouldn't it be possible to do the same in CGo (if not possible already)? What I mean by this is adding a flag or configuration primitive that changes this behavior of CGo regarding unions that wouldn't affect at all to existing code.

As already stated, it feels odd that there is no workaround, if it's a design decision to not support some functions just based on the name, it should be then clearly documented. I saw this other related issue being fixed, so I just wonder why a and r deserve to be valid identifiers in CGo but not any function starting with union_

Finally, as the real use case behind this is to create a Go wrapper over a C library (meos), if in the end it's deemed as not planned, what way forward would you recommend? Since changing the C library functions names it's not a possibility.

@ianlancetaylor
Copy link
Member

I agree that it was a bad choice, made many years ago.

Given that choice, Go code that looks like C.union_X is translated to refer to a C union type. We can't both do that and translate it to a C function name. We could try to guess based on the C code, but that introduces more complexity and a greater likelihood of unusual bugs.

There is a straightforward workaround: in the cgo comment, write a line #define go_union_test union_test. Then in your Go code, refer to C.go_union_test.

Issue #28721 is a different case. That was a case where code that was never intended to be valid behaved oddly, and the solution was to ban such code. C.union_X is different in that its behavior is documented and intended.

The simple way forward for your case is to use #define in the cgo comment to give you new names that you can use in your Go code.

@Davichet-e
Copy link

Thank you for providing a workaround! Maybe as a minor suggestion, somewhere in the documentation it should be stated that functions starting with some specific prefixes won't work as expected, that would avoid some debugging to some users in the future.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/602215 mentions this issue: cmd/cgo: document workaround for C struct_stat function

gopherbot pushed a commit that referenced this issue Jul 31, 2024
For #68682

Change-Id: I13b61f915925a9ee510e0a42e95da7a83678b3b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/602215
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants