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: cgo allows creating values of incomplete struct/union type #40507

Open
mdempsky opened this issue Jul 31, 2020 · 4 comments
Open

cmd/cgo: cgo allows creating values of incomplete struct/union type #40507

mdempsky opened this issue Jul 31, 2020 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Jul 31, 2020

This program builds, but fails at runtime:

// x.go
package main

// struct s;
// union u;
// extern void f(struct s *, union u *);
import "C"

func main() {
	C.f(new(C.struct_s), new(C.union_u))
}
// x.c
#include <assert.h>

struct s { int x; };
union u { long x; };

void f(struct s *sp, union u *up) {
  if (sp && up) {
    assert((void *)sp != (void *)up);
  }
}

This is because cgo defines incomplete struct/union types like C.struct_s and C.union_u as struct{}, rather than giving an error. So the new(C.struct_s) and new(C.union_u) end up both allocating as zero-size objects.

I think this program shouldn't build at all.

One reasonable fix would be to disallow incomplete C.struct_* or C.union_* references outside of a pointer type. This might have false positives for things like:

package p

// struct s;
import "C"

type s C.struct_s
var _ *s

A more invasive but more robust fix would be a //go:cgo_incomplete directive that tells the compiler it shouldn't allow values of that type.

/cc @ianlancetaylor

(Noted while working on fix for #40494.)

@mdempsky mdempsky added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 31, 2020
@beoran
Copy link

beoran commented Aug 4, 2020

Incomplete structs are often used for abstraction in C, cgo really needs to support them. The problem is only with new, that could give a go get diagnostic.

@mdempsky
Copy link
Contributor Author

mdempsky commented Aug 4, 2020

I agree cgo should support incomplete types. It shouldn't allow the user to instantiate them though. That's what this issue is about.

@beoran
Copy link

beoran commented Aug 4, 2020

Thanks for clarifying that. I see now that we are in violent agreement. 🙂

@mdempsky
Copy link
Contributor Author

This is similar to #19487.

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. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Status: Triage Backlog
Development

No branches or pull requests

4 participants