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: never generate Go fields for zero-sized C fields #20275

Open
zagrodzki opened this issue May 7, 2017 · 7 comments
Open

cmd/cgo: never generate Go fields for zero-sized C fields #20275

zagrodzki opened this issue May 7, 2017 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@zagrodzki
Copy link

Please answer these questions before submitting your issue. Thanks!

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

tested with 1.7.5 and 1.8.1

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

linux/amd64

What did you do?

package main

/*
struct foo {
	int x;
	char padding;
	char y[];
};
*/
import "C"
import "fmt"

var Y C.struct_foo

func main() {
	fmt.Println(Y.y)
}

What did you expect to see?

Y.y undefined (type C.struct_foo has no field or method y)

(because Y.y is a zero-length field and Go code cannot refer to these fields)

What did you see instead?

Compiles and prints:

[]

I've checked that it's dependent on memory alignment. If the padding field is removed from struct foo, generated cgo type doesn't have the y field. With the padding, the field is included.

@IanTayler
Copy link

IanTayler commented May 7, 2017

I was able to reproduce with go version go1.8.1 linux/amd64 on Solus, using the code @zagrodzki provided. Both the case where Y.y exists and the case where it doesn't (i.e. without padding).

If we change the array to a pointer we get <nil> with or without padding (which, I assume, is the intended behaviour).

@ianlancetaylor ianlancetaylor added this to the Go1.9Maybe milestone May 8, 2017
@ianlancetaylor
Copy link
Contributor

This is happening because the implicit padding added by these lines (in typeconv.Struct in cmd/cgo/gcc.go)

	if off < dt.ByteSize {
		fld, sizes = c.pad(fld, sizes, dt.ByteSize-off)
		off = dt.ByteSize
	}

is adding a new trailing field and thus breaking the check of whether the actual trailing field has zero size. This does mean that the trailing zero-sized field is going to work in this case, but it would minimize confusion to drop the trailing field in this case as well. Actually, we don't want to drop it, we want to rename it to _ or something along those lines.

@ianlancetaylor ianlancetaylor changed the title cgo: zero-sized fields sometimes visible to Go code cmd/cgo: zero-sized fields sometimes visible to Go code May 8, 2017
@bcmills
Copy link
Contributor

bcmills commented Jun 14, 2017

I'm not sure that we should even rename it. We should convert it to a zero-sized Go array so that it can still be addressed.

There's nothing obviously wrong with this program, for example, assuming that foo_create always returns a pointer with at least one character in y:

package main

/*
struct foo {
	int x;
	char padding;
	char y[];
};
extern struct foo* foo_create();
extern void foo_destroy(struct foo*);
*/
import "C"

import (
    "fmt"
    "unsafe"
)

func main() {
	y := C.foo_create()
	defer C.foo_destroy(y)
	c := (*C.char)((unsafe.Pointer)(&y.y))
	fmt.Println(*c)
}

@ianlancetaylor
Copy link
Contributor

@bcmills Offhand I don't see how to do that while still preserving the size of the struct as seen in Go as being the same as the size as seen in C. That seems like a difficult property to lose. See https://golang.org/cl/12864 .

@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Go1.9Maybe Jun 14, 2017
@bcmills
Copy link
Contributor

bcmills commented Jun 15, 2017

I can see how to solve it with a fairly minimal language change, but not without one.

The language change would be to add a distinguished zero-size type (unsafe.Void?) that does not induce padding and cannot be instantiated in Go. That would prevent an unsafe.Void from ever occurring in the Go heap (except by casting through unsafe.Pointer, which is of course unsafe anyway). Then we could map the C struct as

type struct_foo struct{
  x C.int
  padding C.char
  y unsafe.Void
}

and the y field in C-allocated structs could still be accessed via unsafe.Pointer as in the sketch above.

@rsc
Copy link
Contributor

rsc commented Nov 22, 2017

I don't want to change cgo this late in the cycle but I would be OK for Go 1.11 with never making that field accessible. That will end the inconsistency. We should not make language changes to Go just for this awful detail of C.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@rsc rsc changed the title cmd/cgo: zero-sized fields sometimes visible to Go code cmd/cgo: never generate Go fields for zero-sized C fields Nov 22, 2017
@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 22, 2017
@gopherbot gopherbot modified the milestones: Go1.11, Unplanned May 23, 2018
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@randall77
Copy link
Contributor

Similar to @bcmills 's idea, we can use go:notinheap to mark structures with one of these weird fields.

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. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants