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: size of struct with 0 length trailing field has changed #11925

Closed
ebfe opened this issue Jul 29, 2015 · 15 comments

Comments

@ebfe
Copy link
Contributor

commented Jul 29, 2015

package main

import (
        "fmt"
        "unsafe"
)

/*
struct st {
        int a;
        char d[0];
};
*/
import "C"

func main() {
        var s C.struct_st
        fmt.Println(unsafe.Sizeof(s))
}
$ go version
go version devel +c1ccbab Wed Jul 29 21:44:27 2015 +0000 linux/amd64
$ go run t.go
8

With go 1.4

$ go version
go version go1.4.2 linux/amd64
$ go run t.go
4

Is this expected? This potentially breaks a lot of c-bindings.

@ebfe

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2015

This behaviour was introduced with 6f07ac2

@ianlancetaylor ianlancetaylor changed the title cgo: C.struct sizes changed in tip cmd/compile: size of struct with 0 length field has changed Jul 29, 2015

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2015

Right, this doesn't have anything to do with cgo as such. It's a change in the way the Go compiler handles trailing zero length fields.

I think for clarity it would help to see an example of the kind of code that will break.

@ianlancetaylor ianlancetaylor changed the title cmd/compile: size of struct with 0 length field has changed cmd/cgo: size of struct with 0 length trailing field has changed Jul 30, 2015

@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jul 30, 2015

@ianlancetaylor ianlancetaylor self-assigned this Jul 30, 2015

@ebfe

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2015

I ran into something like this:

import (
        "fmt"
        "unsafe"
)

/*
struct msg_header {
        int flags;
        int len;
        char data[0];
};
*/
import "C"

func marshalMsg(payload []byte, flags int) []byte {
        var hdr C.struct_msg_header
        hdr.flags = C.int(flags)
        hdr.len = C.int(len(payload))
        msg := C.GoBytes(unsafe.Pointer(&hdr), C.int(unsafe.Sizeof(hdr)))
        return append(msg, payload...)
}
func main() {
        fmt.Printf("%x\n", marshalMsg([]byte("hello"), 0))
}

go tip: 00000000050000000000000068656c6c6f
go 1.4.2: 000000000500000068656c6c6f

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2015

We can fix this by dropping the trailing zero byte field in the cgo translation, but that breaks code like that in issue #8428. You can fix your code by using C.sizeof_struct_msg_header, but I agree that it would be nice if your existing code continued to work.

I'm not seeing any good choices here.

@ebfe

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2015

Yes, I already discovered C.sizeof_struct_.... though it doesn't seem to be documented anywhere (?).

C.sizeof_struct_a != unsafe.Sizeof(C.struct_a) is pretty surprising.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2015

I think there is definitely a bug, we just need to decide who to break.

@gopherbot

This comment has been minimized.

Copy link

commented Jul 30, 2015

CL https://golang.org/cl/12864 mentions this issue.

@gopherbot

This comment has been minimized.

Copy link

commented Jul 30, 2015

CL https://golang.org/cl/12866 mentions this issue.

ianlancetaylor added a commit that referenced this issue Jul 30, 2015
doc: add go1.5 note about change to zero-sized fields in cgo
This documents the change made in https://golang.org/cl/12864 for
https://golang.org/issue/11925.

Update #11925.

Change-Id: Id09f2a489ea947a725ed12c9cf793e5daef07a06
Reviewed-on: https://go-review.googlesource.com/12866
Reviewed-by: David Crawshaw <crawshaw@golang.org>
@LK4D4

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2015

@ianlancetaylor Could you help me? What is recommended way now to get those fields like char[0]?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2015

It depends on why you want to access the field.

If you want to take its address, you can write code like
(*ELEMENTTYPE)unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(STRUCTYPE{}))

I agree that this is not ideal but I don't see how to avoid it.

@LK4D4

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2015

@ianlancetaylor We had code which received such struct from C, struct looks like:

struct dm_deps {
    uint32_t count;
    uint32_t filler;
    uint64_t device[0];
};

Before in go we just had cool struct with field device, which was sorta slice, at least range worked with it, like for _, d := range deps.device. Now there is no field at all. So only way is to get pointer and cast it to slice, right?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2015

I'm not sure I understand. Given a C struct like the above, you used to get a Go struct with a zero-sized array (not a slice). Writing
for _, d := range deps.device
would give you a loop that would execute zero times. That doesn't seem useful.

You will now have to get a pointer, convert it to an array pointer, and slice the array to the desired length.

@tiborvass

This comment has been minimized.

Copy link

commented Aug 6, 2015

@ianlancetaylor maybe this could help

package main

/*
#include <inttypes.h>
struct dm_deps {
    uint32_t count;
    uint32_t filler;
    uint64_t device[0];
};
*/
import "C"
import "fmt"

func main() {
    deps := &C.struct_dm_deps{}
    fmt.Printf("%#v\n", deps)
    // go version go1.4.2 linux/amd64
    // &main._Ctype_struct_dm_deps{count:0x0, filler:0x0, device:[0]main._Ctype_uint64_t{}}

    // go version devel +d3ffc97 Wed Jul 29 23:50:20 2015 +0000 linux/amd64
    // &main._Ctype_struct_dm_deps{count:0x0, filler:0x0}
}

(courtesy of @vbatts)

@LK4D4

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2015

@ianlancetaylor Yes, you're right I think, this code was weird. Thanks for answers!

@gopherbot

This comment has been minimized.

Copy link

commented Jan 12, 2016

CL https://golang.org/cl/18542 mentions this issue.

gopherbot pushed a commit that referenced this issue Jan 12, 2016
cmd/cgo: document C.sizeof_T and zero-sized field restriction
Update #9401.
Update #11925.
Update #13919.

Change-Id: I52c679353693e8165b2972d4d3974ee8bb1207ef
Reviewed-on: https://go-review.googlesource.com/18542
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
csabahenk added a commit to gluster/parsefuse that referenced this issue Sep 13, 2016
Fix Dirent size calculation
Accounting for zero size arrays fileds in structs has
changend in golang/go@6f07ac2 (go1.5beta1~2396),
which made the size of fuse_dirent (C name) / parsefuse.Dirent (Go name)
different when calculated from C (= serialized size. "real size") and Go.

Go code using cgo can get the real size through the C.sizeof_struct_...
construct (cf.
golang/go#11925 (comment)).
However, we don't use cgo, but parse C through Ruby, so this is not
available for us. Instead we take the offset of the zero size array
field, which does return the desired value.

Fixes #1

@golang golang locked and limited conversation to collaborators Jan 13, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.