From 6be1cb8c7a8771310e0cc36c3d8fa783d48d0cf9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 7 Nov 2013 15:24:51 -0500 Subject: [PATCH] cmd/cgo: fix handling of array of pointers when using clang Clang does not record the "size" field for pointer types, so we must insert the size ourselves. We were already doing this, but only for the case of pointer types. For an array of pointer types, the setting of the size for the nested pointer type was happening after the computation of the size of the array type, meaning that the array type was always computed as 0 bytes. Delay the size computation. This bug happens on all Clang systems, not just FreeBSD. Our test checked that cgo wrote something, not that it was correct. FreeBSD's default clang rejects array[0] as a C struct field, so it noticed the incorrect sizes. But the sizes were incorrect everywhere. Update testcdefs to check the output has the right semantics. Fixes #6292. R=golang-dev, iant CC=golang-dev https://golang.org/cl/22840043 --- misc/cgo/testcdefs/main.c | 48 ++++++++++++++++++++++++++++++++++++ misc/cgo/testcdefs/main.go | 11 ++++++--- misc/cgo/testcdefs/test.bash | 2 +- src/cmd/cgo/gcc.go | 34 ++++++++++++++++--------- 4 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 misc/cgo/testcdefs/main.c diff --git a/misc/cgo/testcdefs/main.c b/misc/cgo/testcdefs/main.c new file mode 100644 index 0000000000..2d3ee4dbea --- /dev/null +++ b/misc/cgo/testcdefs/main.c @@ -0,0 +1,48 @@ +// Copyright 2013 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. + +#include "runtime.h" +#include "cdefstest.h" + +void runtime·printf(int8*, ...); + +// From cdefstest.go. +typedef struct CdefsOrig CdefsOrig; +struct CdefsOrig { + int8 array1[20]; + int8 array2[20][20]; + int8 *array3[20]; + int8 *array4[20][20]; + int8 **array5[20][20]; +}; + +void +main·test(int32 ret) +{ + CdefsOrig o; + CdefsTest t; + + ret = 0; + if(sizeof(t.array1) != sizeof(o.array1) || offsetof(CdefsTest, array1[0]) != offsetof(CdefsOrig, array1[0])) { + runtime·printf("array1: size, offset = %d, %d, want %d, %d\n", sizeof(t.array1), offsetof(CdefsTest, array1[0]), sizeof(o.array1), offsetof(CdefsOrig, array1[0])); + ret = 1; + } + if(sizeof(t.array2) != sizeof(o.array2) || offsetof(CdefsTest, array2[0][0]) != offsetof(CdefsOrig, array2[0][0])) { + runtime·printf("array2: size, offset = %d, %d, want %d, %d\n", sizeof(t.array2), offsetof(CdefsTest, array2[0][0]), sizeof(o.array2), offsetof(CdefsOrig, array2[0][0])); + ret = 1; + } + if(sizeof(t.array3) != sizeof(o.array3) || offsetof(CdefsTest, array3[0]) != offsetof(CdefsOrig, array3[0])) { + runtime·printf("array3: size, offset = %d, %d, want %d, %d\n", sizeof(t.array3), offsetof(CdefsTest, array3[0]), sizeof(o.array3), offsetof(CdefsOrig, array3[0])); + ret = 1; + } + if(sizeof(t.array4) != sizeof(o.array4) || offsetof(CdefsTest, array4[0][0]) != offsetof(CdefsOrig, array4[0][0])) { + runtime·printf("array4: size, offset = %d, %d, want %d, %d\n", sizeof(t.array4), offsetof(CdefsTest, array4[0][0]), sizeof(o.array4), offsetof(CdefsOrig, array4[0][0])); + ret = 1; + } + if(sizeof(t.array5) != sizeof(o.array5) || offsetof(CdefsTest, array5[0][0]) != offsetof(CdefsOrig, array5[0][0])) { + runtime·printf("array5: size, offset = %d, %d, want %d, %d\n", sizeof(t.array5), offsetof(CdefsTest, array5[0][0]), sizeof(o.array5), offsetof(CdefsOrig, array5[0][0])); + ret = 1; + } + FLUSH(&ret); // flush return value +} diff --git a/misc/cgo/testcdefs/main.go b/misc/cgo/testcdefs/main.go index 864b4b2a7f..9231741ad0 100644 --- a/misc/cgo/testcdefs/main.go +++ b/misc/cgo/testcdefs/main.go @@ -2,7 +2,12 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package cgotest +package main -// This file only exists so we can run 'go build' and build our .c files -func test() {} +import "os" + +func test() int32 // in main.c + +func main() { + os.Exit(int(test())) +} diff --git a/misc/cgo/testcdefs/test.bash b/misc/cgo/testcdefs/test.bash index cbfa9b27d9..1a14ad35c6 100755 --- a/misc/cgo/testcdefs/test.bash +++ b/misc/cgo/testcdefs/test.bash @@ -10,7 +10,7 @@ do go tool cgo -cdefs ${FP}.go > ${FP}.h done -go build . +go build . && ./testcdefs EXIT=$? rm -rf _obj main *.h exit $EXIT diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index f2a109d343..3e1837ebf9 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -1046,21 +1046,11 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type { } t := new(Type) - t.Size = dtype.Size() + t.Size = dtype.Size() // note: wrong for array of pointers, corrected below t.Align = -1 t.C = &TypeRepr{Repr: dtype.Common().Name} c.m[dtype] = t - if t.Size < 0 { - // Unsized types are [0]byte - t.Size = 0 - t.Go = c.Opaque(0) - if t.C.Empty() { - t.C.Set("void") - } - return t - } - switch dt := dtype.(type) { default: fatalf("%s: unexpected type: %s", lineno(pos), dtype) @@ -1207,6 +1197,9 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type { return t case *dwarf.StructType: + if dt.ByteSize < 0 { // opaque struct + break + } // Convert to Go struct, being careful about alignment. // Have to give it a name to simulate C "struct foo" references. tag := dt.StructName @@ -1325,6 +1318,25 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type { } } + if t.Size <= 0 { + // Clang does not record the size of a pointer in its DWARF entry, + // so if dtype is an array, the call to dtype.Size at the top of the function + // computed the size as the array length * 0 = 0. + // The type switch called Type (this function) recursively on the pointer + // entry, and the code near the top of the function updated the size to + // be correct, so calling dtype.Size again will produce the correct value. + t.Size = dtype.Size() + if t.Size < 0 { + // Unsized types are [0]byte + t.Size = 0 + t.Go = c.Opaque(0) + if t.C.Empty() { + t.C.Set("void") + } + return t + } + } + if t.C.Empty() { fatalf("%s: internal error: did not create C name for %s", lineno(pos), dtype) } -- 2.50.0