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: detect incompatible C declarations #67699

Closed
randall77 opened this issue May 29, 2024 · 1 comment
Closed

cmd/cgo: detect incompatible C declarations #67699

randall77 opened this issue May 29, 2024 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@randall77
Copy link
Contributor

We already detect incompatible C declarations if both declarations are in the same file. For instance,

/*
void f();
int f();
*/
import "C"

Gives an error conflicting types for 'f'.

When the conflicting definitions are in different files, however, we don't detect it. That can lead to very tricky runtime failures. See #67670 for an example.

Here's a small reproducer:
test1.go:

package main

/*
int f(int x, int y, int z, int w);
*/
import "C"
import "fmt"

func main() {
	var x [4]uint64
	pre(&x)
	C.f(5, 6, 7, 8)
	post(&x)
	main2()
}

//go:noinline
func pre(p *[4]uint64) {
	for i := range p {
		p[i] = 0xaaaaaaaaaaaaaaaa
	}
}

//go:noinline
func post(p *[4]uint64) {
	for i := range p {
		if p[i] != 0xaaaaaaaaaaaaaaaa {
			fmt.Printf("%d %x\n", i, p[i])
			panic("bad")
		}
	}

}

test2.go:

package main

/*
void f(int x, int y, int z, int w) { }
*/
import "C"

func main2() {
	var x [4]uint64
	pre(&x)
	C.f(5, 6, 7, 8)
	post(&x)
}

When run with go run test1.go test2.go, we get (darwin/arm64):

0 aaaaaaaa00000005
panic: bad

goroutine 1 [running]:
main.post(0x14000080f18)
	/Users/khr/gowork/test1.go:29 +0xe4
main.main()
	/Users/khr/gowork/test1.go:13 +0x4c
exit status 2

Which means that a stack variable got partially overwritten by a cgo call.

I'm pretty sure what is happening is that at the callsite the compiler assumes C.f has no return value, so does not allocate space for it. But the cgo wrapper assumes C.f does have a return value, and generates code to move f's return value to the stack frame. That write lands on the local variable x.

I'm not sure how the compiler and/or cgo decide which version of f to use. Perhaps if they just chose consistently things would work. But probably better to detect the conflicting definitions and error out.

@ianlancetaylor

@randall77 randall77 added the FeatureRequest Issues asking for a new feature that does not need a proposal. label May 29, 2024
@randall77 randall77 added this to the Unplanned milestone May 29, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 29, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/588977 mentions this issue: cmd/cgo: error on multiple incompatible function declarations

@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 3, 2024
@dmitshur dmitshur modified the milestones: Unplanned, Go1.24 Nov 26, 2024
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. FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants