Skip to content

Commit

Permalink
cmd/cgo: discard trailing zero-sized fields in a non-empty C struct
Browse files Browse the repository at this point in the history
In order to fix issue golang#9401 the compiler was changed to add a padding
byte to any non-empty Go struct that ends in a zero-sized field.  That
causes the Go version of such a C struct to have a different size than
the C struct, which can considerable confusion.  Change cgo so that it
discards any such zero-sized fields, so that the Go and C structs are
the same size.

This is a change from previous releases, in that it used to be
possible to refer to a zero-sized trailing field (by taking its
address), and with this change it no longer is.  That is unfortunate,
but something has to change.  It seems better to visibly break
programs that do this rather than to silently break programs that rely
on the struct sizes being the same.

Update golang#9401.
Fixes golang#11925.

Change-Id: I3fba3f02f11265b3c41d68616f79dedb05b81225
Reviewed-on: https://go-review.googlesource.com/12864
Reviewed-by: Russ Cox <[email protected]>
  • Loading branch information
ianlancetaylor committed Jul 30, 2015
1 parent c9d2c7f commit 7904946
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 6 deletions.
1 change: 1 addition & 0 deletions misc/cgo/test/cgo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,6 @@ func TestReturnAfterGrowFromGo(t *testing.T) { testReturnAfterGrowFromGo(t) }
func Test9026(t *testing.T) { test9026(t) }
func Test9557(t *testing.T) { test9557(t) }
func Test10303(t *testing.T) { test10303(t, 10) }
func Test11925(t *testing.T) { test11925(t) }

func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }
37 changes: 37 additions & 0 deletions misc/cgo/test/issue11925.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2015 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Issue 11925. Structs with zero-length trailing fields are now
// padded by the Go compiler.

package cgotest

/*
struct a11925 {
int i;
char a[0];
char b[0];
};
struct b11925 {
int i;
char a[0];
char b[];
};
*/
import "C"

import (
"testing"
"unsafe"
)

func test11925(t *testing.T) {
if C.sizeof_struct_a11925 != unsafe.Sizeof(C.struct_a11925{}) {
t.Errorf("size of a changed: C %d, Go %d", C.sizeof_struct_a11925, unsafe.Sizeof(C.struct_a11925{}))
}
if C.sizeof_struct_b11925 != unsafe.Sizeof(C.struct_b11925{}) {
t.Errorf("size of b changed: C %d, Go %d", C.sizeof_struct_b11925, unsafe.Sizeof(C.struct_b11925{}))
}
}
7 changes: 5 additions & 2 deletions misc/cgo/test/issue8428.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ struct issue8428two {
void *p;
char b;
char rest[0];
char pad;
};
struct issue8428three {
Expand All @@ -34,8 +35,10 @@ import "C"
import "unsafe"

var _ = C.struct_issue8428one{
b: C.char(0),
rest: [0]C.char{},
b: C.char(0),
// The trailing rest field is not available in cgo.
// See issue 11925.
// rest: [0]C.char{},
}

var _ = C.struct_issue8428two{
Expand Down
26 changes: 22 additions & 4 deletions src/cmd/cgo/gcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1544,11 +1544,13 @@ func (c *typeConv) intExpr(n int64) ast.Expr {
}

// Add padding of given size to fld.
func (c *typeConv) pad(fld []*ast.Field, size int64) []*ast.Field {
func (c *typeConv) pad(fld []*ast.Field, sizes []int64, size int64) ([]*ast.Field, []int64) {
n := len(fld)
fld = fld[0 : n+1]
fld[n] = &ast.Field{Names: []*ast.Ident{c.Ident("_")}, Type: c.Opaque(size)}
return fld
sizes = sizes[0 : n+1]
sizes[n] = size
return fld, sizes
}

// Struct conversion: return Go and (gc) C syntax for type.
Expand All @@ -1559,6 +1561,7 @@ func (c *typeConv) Struct(dt *dwarf.StructType, pos token.Pos) (expr *ast.Struct
var buf bytes.Buffer
buf.WriteString("struct {")
fld := make([]*ast.Field, 0, 2*len(dt.Field)+1) // enough for padding around every field
sizes := make([]int64, 0, 2*len(dt.Field)+1)
off := int64(0)

// Rename struct fields that happen to be named Go keywords into
Expand Down Expand Up @@ -1594,7 +1597,7 @@ func (c *typeConv) Struct(dt *dwarf.StructType, pos token.Pos) (expr *ast.Struct
anon := 0
for _, f := range dt.Field {
if f.ByteOffset > off {
fld = c.pad(fld, f.ByteOffset-off)
fld, sizes = c.pad(fld, sizes, f.ByteOffset-off)
off = f.ByteOffset
}

Expand Down Expand Up @@ -1652,6 +1655,8 @@ func (c *typeConv) Struct(dt *dwarf.StructType, pos token.Pos) (expr *ast.Struct
ident[name] = name
}
fld[n] = &ast.Field{Names: []*ast.Ident{c.Ident(ident[name])}, Type: tgo}
sizes = sizes[0 : n+1]
sizes[n] = size
off += size
buf.WriteString(t.C.String())
buf.WriteString(" ")
Expand All @@ -1662,9 +1667,22 @@ func (c *typeConv) Struct(dt *dwarf.StructType, pos token.Pos) (expr *ast.Struct
}
}
if off < dt.ByteSize {
fld = c.pad(fld, dt.ByteSize-off)
fld, sizes = c.pad(fld, sizes, dt.ByteSize-off)
off = dt.ByteSize
}

// If the last field in a non-zero-sized struct is zero-sized
// the compiler is going to pad it by one (see issue 9401).
// We can't permit that, because then the size of the Go
// struct will not be the same as the size of the C struct.
// Our only option in such a case is to remove the field,
// which means that it can not be referenced from Go.
for off > 0 && sizes[len(sizes)-1] == 0 {
n := len(sizes)
fld = fld[0 : n-1]
sizes = sizes[0 : n-1]
}

if off != dt.ByteSize {
fatalf("%s: struct size calculation error off=%d bytesize=%d", lineno(pos), off, dt.ByteSize)
}
Expand Down

0 comments on commit 7904946

Please sign in to comment.