]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/cgo: bug fixes
authorRuss Cox <rsc@golang.org>
Sun, 19 Feb 2012 18:32:55 +0000 (13:32 -0500)
committerRuss Cox <rsc@golang.org>
Sun, 19 Feb 2012 18:32:55 +0000 (13:32 -0500)
* disallow embedding of C type (Fixes issue 2552)
* detect 0-length array (Fixes issue 2806)
* use typedefs when possible, to avoid attribute((unavailable)) (Fixes issue 2888)
* print Go types constructed from C types using original C types (Fixes issue 2612)

This fix changes _cgo_export.h to repeat the preamble from import "C".
Otherwise the fix to issue 2612 is impossible, since it cannot refer to
types that have not been defined.  If people are using //export and
putting non-header information in the preamble, they will need to
refactor their code.

R=golang-dev, r, r
CC=golang-dev
https://golang.org/cl/5672080

14 files changed:
doc/go1.html
doc/go1.tmpl
src/cmd/cgo/ast.go
src/cmd/cgo/doc.go
src/cmd/cgo/gcc.go
src/cmd/cgo/godefs.go
src/cmd/cgo/main.go
src/cmd/cgo/out.go
src/cmd/cgo/util.go
src/pkg/debug/dwarf/testdata/typedef.c
src/pkg/debug/dwarf/testdata/typedef.elf
src/pkg/debug/dwarf/testdata/typedef.macho
src/pkg/debug/dwarf/type.go
src/pkg/debug/dwarf/type_test.go

index 05d3eb50317427dcbf0832fc52a43cd851767090..f4a4623db61e7861a6c1e38e579a167a1225421d 100644 (file)
@@ -1853,7 +1853,24 @@ Code that uses the old fields will fail to compile and must be updated by hand.
 The semantic changes make it difficult for the fix tool to update automatically.
 </p>
 
-<h2 id="go_command">The go command</h2>
+<h2 id="cmd_go">The go command</h2>
+
+<p>
+TODO: Write this.
+</p>
+
+<h2 id="cmd_cgo">The cgo command</h2>
+
+<p>
+In Go 1, the <a href="/cmd/cgo">cgo command</a>
+uses a different <code>_cgo_export.h</code>
+file, which is generated for packages containing <code>//export</code> lines.
+The <code>_cgo_export.h</code> file now begins with the C preamble comment,
+so that exported function definitions can use types defined there.
+This has the effect of compiling the preamble multiple times, so a
+package using <code>//export</code> must not put function definitions
+or variable initializations in the C preamble.
+</p
 
 <h2 id="releases">Packaged releases</h2>
 
index 7a28be3c3af8f07b22747053eb494f09826080e6..314a6de93d3f326ac100643b0d9669823cb29364 100644 (file)
@@ -1725,7 +1725,24 @@ Code that uses the old fields will fail to compile and must be updated by hand.
 The semantic changes make it difficult for the fix tool to update automatically.
 </p>
 
-<h2 id="go_command">The go command</h2>
+<h2 id="cmd_go">The go command</h2>
+
+<p>
+TODO: Write this.
+</p>
+
+<h2 id="cmd_cgo">The cgo command</h2>
+
+<p>
+In Go 1, the <a href="/cmd/cgo">cgo command</a>
+uses a different <code>_cgo_export.h</code>
+file, which is generated for packages containing <code>//export</code> lines.
+The <code>_cgo_export.h</code> file now begins with the C preamble comment,
+so that exported function definitions can use types defined there.
+This has the effect of compiling the preamble multiple times, so a
+package using <code>//export</code> must not put function definitions
+or variable initializations in the C preamble.
+</p
 
 <h2 id="releases">Packaged releases</h2>
 
index da6ae4176dab92389ceed830dffe4ea694bd8d55..381e606ef406832e499435c892b6733ba9355162 100644 (file)
@@ -147,6 +147,9 @@ func (f *File) saveRef(x interface{}, context string) {
                        if context == "as2" {
                                context = "expr"
                        }
+                       if context == "embed-type" {
+                               error_(sel.Pos(), "cannot embed C type")
+                       }
                        goname := sel.Sel.Name
                        if goname == "errno" {
                                error_(sel.Pos(), "cannot refer to errno directly; see documentation")
@@ -232,7 +235,11 @@ func (f *File) walk(x interface{}, context string, visit func(*File, interface{}
 
        // These are ordered and grouped to match ../../pkg/go/ast/ast.go
        case *ast.Field:
-               f.walk(&n.Type, "type", visit)
+               if len(n.Names) == 0 && context == "field" {
+                       f.walk(&n.Type, "embed-type", visit)
+               } else {
+                       f.walk(&n.Type, "type", visit)
+               }
        case *ast.FieldList:
                for _, field := range n.List {
                        f.walk(field, context, visit)
@@ -289,9 +296,9 @@ func (f *File) walk(x interface{}, context string, visit func(*File, interface{}
        case *ast.StructType:
                f.walk(n.Fields, "field", visit)
        case *ast.FuncType:
-               f.walk(n.Params, "field", visit)
+               f.walk(n.Params, "param", visit)
                if n.Results != nil {
-                       f.walk(n.Results, "field", visit)
+                       f.walk(n.Results, "param", visit)
                }
        case *ast.InterfaceType:
                f.walk(n.Methods, "field", visit)
@@ -379,7 +386,7 @@ func (f *File) walk(x interface{}, context string, visit func(*File, interface{}
                f.walk(n.Specs, "spec", visit)
        case *ast.FuncDecl:
                if n.Recv != nil {
-                       f.walk(n.Recv, "field", visit)
+                       f.walk(n.Recv, "param", visit)
                }
                f.walk(n.Type, "type", visit)
                if n.Body != nil {
index 1d64c75ada9f6b07c25135f6d7fd4ca1215588ca..83f1ba46c099d6589291e9fb630add66e6253fd6 100644 (file)
@@ -16,8 +16,8 @@ the pseudo-package "C" and then refers to types such as C.size_t,
 variables such as C.stdout, or functions such as C.putchar.
 
 If the import of "C" is immediately preceded by a comment, that
-comment is used as a header when compiling the C parts of
-the package.  For example:
+comment, called the preamble, is used as a header when compiling
+the C parts of the package.  For example:
 
        // #include <stdio.h>
        // #include <errno.h>
@@ -57,6 +57,8 @@ The C type void* is represented by Go's unsafe.Pointer.
 To access a struct, union, or enum type directly, prefix it with
 struct_, union_, or enum_, as in C.struct_stat.
 
+Go structs cannot embed fields with C types.
+
 Any C function that returns a value may be called in a multiple
 assignment context to retrieve both the return value and the
 C errno variable as an error.  For example:
@@ -100,7 +102,8 @@ They will be available in the C code as:
        extern int64 MyFunction(int arg1, int arg2, GoString arg3);
        extern struct MyFunction2_return MyFunction2(int arg1, int arg2, GoString arg3);
 
-found in _cgo_export.h generated header. Functions with multiple
+found in _cgo_export.h generated header, after any preambles
+copied from the cgo input files. Functions with multiple
 return values are mapped to functions returning a struct.
 Not all Go types can be mapped to C types in a useful way.
 
index 71a9457f5f750dd7105575233882d3c60a5333d2..342a8a530d143186c1908cf941b696dfc83c741e 100644 (file)
@@ -709,7 +709,7 @@ func (p *Package) rewriteRef(f *File) {
                        // Substitute definition for mangled type name.
                        if id, ok := expr.(*ast.Ident); ok {
                                if t := typedef[id.Name]; t != nil {
-                                       expr = t
+                                       expr = t.Go
                                }
                                if id.Name == r.Name.Mangle && r.Name.Const != "" {
                                        expr = ast.NewIdent(r.Name.Const)
@@ -894,7 +894,7 @@ type typeConv struct {
 }
 
 var tagGen int
-var typedef = make(map[string]ast.Expr)
+var typedef = make(map[string]*Type)
 var goIdent = make(map[string]*ast.Ident)
 
 func (c *typeConv) Init(ptrSize int64) {
@@ -1164,17 +1164,22 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type {
                goIdent[name.Name] = name
                switch dt.Kind {
                case "union", "class":
-                       typedef[name.Name] = c.Opaque(t.Size)
                        if t.C.Empty() {
                                t.C.Set("typeof(unsigned char[%d])", t.Size)
                        }
+                       typedef[name.Name] = t
                case "struct":
                        g, csyntax, align := c.Struct(dt, pos)
                        if t.C.Empty() {
                                t.C.Set(csyntax)
                        }
                        t.Align = align
-                       typedef[name.Name] = g
+                       tt := *t
+                       if tag != "" {
+                               tt.C = &TypeRepr{"struct %s", []interface{}{tag}}
+                       }
+                       tt.Go = g
+                       typedef[name.Name] = &tt
                }
 
        case *dwarf.TypedefType:
@@ -1203,7 +1208,9 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type {
                t.Size = sub.Size
                t.Align = sub.Align
                if _, ok := typedef[name.Name]; !ok {
-                       typedef[name.Name] = sub.Go
+                       tt := *t
+                       tt.Go = sub.Go
+                       typedef[name.Name] = &tt
                }
                if *godefs || *cdefs {
                        t.Go = sub.Go
@@ -1250,7 +1257,8 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type {
                        }
                        s = strings.Join(strings.Split(s, " "), "") // strip spaces
                        name := c.Ident("_Ctype_" + s)
-                       typedef[name.Name] = t.Go
+                       tt := *t
+                       typedef[name.Name] = &tt
                        if !*godefs && !*cdefs {
                                t.Go = name
                        }
@@ -1288,9 +1296,18 @@ func (c *typeConv) FuncArg(dtype dwarf.Type, pos token.Pos) *Type {
                if ptr, ok := base(dt.Type).(*dwarf.PtrType); ok {
                        // Unless the typedef happens to point to void* since
                        // Go has special rules around using unsafe.Pointer.
-                       if _, void := base(ptr.Type).(*dwarf.VoidType); !void {
-                               return c.Type(ptr, pos)
+                       if _, void := base(ptr.Type).(*dwarf.VoidType); void {
+                               break
                        }
+
+                       t = c.Type(ptr, pos)
+                       if t == nil {
+                               return nil
+                       }
+
+                       // Remember the C spelling, in case the struct
+                       // has __attribute__((unavailable)) on it.  See issue 2888.
+                       t.Typedef = dt.Name
                }
        }
        return t
@@ -1443,7 +1460,7 @@ func (c *typeConv) Struct(dt *dwarf.StructType, pos token.Pos) (expr *ast.Struct
                off = dt.ByteSize
        }
        if off != dt.ByteSize {
-               fatalf("%s: struct size calculation error", lineno(pos))
+               fatalf("%s: struct size calculation error off=%d bytesize=%d", lineno(pos), off, dt.ByteSize)
        }
        buf.WriteString("}")
        csyntax = buf.String()
index 478ed261cb72b1fd04805362103c57b325efee35..fec70a334b03b7fa008db83c21419d79c4160b54 100644 (file)
@@ -80,7 +80,7 @@ func (p *Package) godefs(f *File, srcfile string) string {
        // and xxx is a typedef for yyy, make C.yyy format as T.
        for typ, def := range typedef {
                if new := override[typ]; new != "" {
-                       if id, ok := def.(*ast.Ident); ok {
+                       if id, ok := def.Go.(*ast.Ident); ok {
                                override[id.Name] = new
                        }
                }
index fb5074e8140c76e5781e8adf41fe936bf4fd84f3..a8be7be7d938f0fbcd5b977d03e7c1304710aa85 100644 (file)
@@ -39,6 +39,7 @@ type Package struct {
        Decl        []ast.Decl
        GoFiles     []string // list of Go files
        GccFiles    []string // list of gcc output files
+       Preamble    string   // collected preamble for _cgo_export.h
 }
 
 // A File collects information about a single Go input file.
@@ -98,6 +99,7 @@ type Type struct {
        C          *TypeRepr
        Go         ast.Expr
        EnumValues map[string]int64
+       Typedef    string
 }
 
 // A FuncType collects information about a function type in both the C and Go worlds.
@@ -312,6 +314,9 @@ func (p *Package) Record(f *File) {
                }
        }
 
-       p.ExpFunc = append(p.ExpFunc, f.ExpFunc...)
+       if f.ExpFunc != nil {
+               p.ExpFunc = append(p.ExpFunc, f.ExpFunc...)
+               p.Preamble += "\n" + f.Preamble
+       }
        p.Decl = append(p.Decl, f.AST.Decls...)
 }
index 2a012177b36fb8ef72d72c5d63301fcd7428351b..4dc0f845496a4173a8efad35b817ab786d4695e1 100644 (file)
@@ -59,7 +59,7 @@ func (p *Package) writeDefs() {
 
        for name, def := range typedef {
                fmt.Fprintf(fgo2, "type %s ", name)
-               conf.Fprint(fgo2, fset, def)
+               conf.Fprint(fgo2, fset, def.Go)
                fmt.Fprintf(fgo2, "\n\n")
        }
        fmt.Fprintf(fgo2, "type _Ctype_void [0]byte\n")
@@ -196,7 +196,11 @@ func (p *Package) structType(n *Name) (string, int64) {
                        fmt.Fprintf(&buf, "\t\tchar __pad%d[%d];\n", off, pad)
                        off += pad
                }
-               fmt.Fprintf(&buf, "\t\t%s p%d;\n", t.C, i)
+               c := t.Typedef
+               if c == "" {
+                       c = t.C.String()
+               }
+               fmt.Fprintf(&buf, "\t\t%s p%d;\n", c, i)
                off += t.Size
        }
        if off%p.PtrSize != 0 {
@@ -428,6 +432,7 @@ func (p *Package) writeExports(fgo2, fc, fm *os.File) {
        fgcch := creat(*objDir + "_cgo_export.h")
 
        fmt.Fprintf(fgcch, "/* Created by cgo - DO NOT EDIT. */\n")
+       fmt.Fprintf(fgcch, "%s\n", p.Preamble)
        fmt.Fprintf(fgcch, "%s\n", gccExportHeaderProlog)
 
        fmt.Fprintf(fgcc, "/* Created by cgo - DO NOT EDIT. */\n")
@@ -693,10 +698,8 @@ func (p *Package) cgoType(e ast.Expr) *Type {
                                }
                        }
                }
-               for name, def := range typedef {
-                       if name == t.Name {
-                               return p.cgoType(def)
-                       }
+               if def := typedef[t.Name]; def != nil {
+                       return def
                }
                if t.Name == "uintptr" {
                        return &Type{Size: p.PtrSize, Align: p.PtrSize, C: c("uintptr")}
@@ -721,7 +724,7 @@ func (p *Package) cgoType(e ast.Expr) *Type {
                        return &Type{Size: p.PtrSize, Align: p.PtrSize, C: c("void*")}
                }
        }
-       error_(e.Pos(), "unrecognized Go type %T", e)
+       error_(e.Pos(), "Go type not supported in export: %s", gofmt(e))
        return &Type{Size: 4, Align: 4, C: c("int")}
 }
 
index 1bf665ff4757dbb8a1bcb423c79f2d1fb26714d2..d6b6a7abb61b714f99ed65f0412b618cdb975ae1 100644 (file)
@@ -70,7 +70,11 @@ func lineno(pos token.Pos) string {
 
 // Die with an error message.
 func fatalf(msg string, args ...interface{}) {
-       fmt.Fprintf(os.Stderr, msg+"\n", args...)
+       // If we've already printed other errors, they might have
+       // caused the fatal condition.  Assume they're enough.
+       if nerrors == 0 {
+               fmt.Fprintf(os.Stderr, msg+"\n", args...)
+       }
        os.Exit(2)
 }
 
index 664d021ced5e6b884a2b5e7cbda08cfe7f5afcf0..f05f01564ff8c5f090b8c1bad1ce638bf2f475ef 100644 (file)
@@ -28,8 +28,13 @@ typedef struct my_struct {
        volatile int vi;
        char x : 1;
        int y : 4;
+       int z[0];
        long long array[40];
+       int zz[0];
 } t_my_struct;
+typedef struct my_struct1 {
+       int zz [1];
+} t_my_struct1;
 typedef union my_union {
        volatile int vi;
        char x : 1;
@@ -65,7 +70,8 @@ t_func_void_of_char *a9;
 t_func_void_of_void *a10;
 t_func_void_of_ptr_char_dots *a11;
 t_my_struct *a12;
-t_my_union *a12a;
+t_my_struct1 *a12a;
+t_my_union *a12b;
 t_my_enum *a13;
 t_my_list *a14;
 t_my_tree *a15;
index 44df8da9bc7c4683d557a34bee8715ab754e2d34..b2062d2c4bb828dcb229f12869f77fd5d9522278 100755 (executable)
Binary files a/src/pkg/debug/dwarf/testdata/typedef.elf and b/src/pkg/debug/dwarf/testdata/typedef.elf differ
index 41019c1e1461536738788a750b49abb0a4fd626b..f75afcccbfc852d284a33c01a49eb1035b6dae9c 100644 (file)
Binary files a/src/pkg/debug/dwarf/testdata/typedef.macho and b/src/pkg/debug/dwarf/testdata/typedef.macho differ
index 9be66658fe9fa0242e69fca787284bbd6c255f14..4502355022d60aafa499b1ac9ee6a1fd7b090cf1 100644 (file)
@@ -426,6 +426,8 @@ func (d *Data) Type(off Offset) (Type, error) {
                t.StructName, _ = e.Val(AttrName).(string)
                t.Incomplete = e.Val(AttrDeclaration) != nil
                t.Field = make([]*StructField, 0, 8)
+               var lastFieldType Type
+               var lastFieldBitOffset int64
                for kid := next(); kid != nil; kid = next() {
                        if kid.Tag == TagMember {
                                f := new(StructField)
@@ -444,11 +446,32 @@ func (d *Data) Type(off Offset) (Type, error) {
                                                goto Error
                                        }
                                }
+
+                               haveBitOffset := false
                                f.Name, _ = kid.Val(AttrName).(string)
                                f.ByteSize, _ = kid.Val(AttrByteSize).(int64)
-                               f.BitOffset, _ = kid.Val(AttrBitOffset).(int64)
+                               f.BitOffset, haveBitOffset = kid.Val(AttrBitOffset).(int64)
                                f.BitSize, _ = kid.Val(AttrBitSize).(int64)
                                t.Field = append(t.Field, f)
+
+                               bito := f.BitOffset
+                               if !haveBitOffset {
+                                       bito = f.ByteOffset * 8
+                               }
+                               if bito == lastFieldBitOffset && t.Kind != "union" {
+                                       // Last field was zero width.  Fix array length.
+                                       // (DWARF writes out 0-length arrays as if they were 1-length arrays.)
+                                       zeroArray(lastFieldType)
+                               }
+                               lastFieldType = f.Type
+                               lastFieldBitOffset = bito
+                       }
+               }
+               if t.Kind != "union" {
+                       b, ok := e.Val(AttrByteSize).(int64)
+                       if ok && b*8 == lastFieldBitOffset {
+                               // Final field must be zero width.  Fix array length.
+                               zeroArray(lastFieldType)
                        }
                }
 
@@ -579,3 +602,14 @@ Error:
        delete(d.typeCache, off)
        return nil, err
 }
+
+func zeroArray(t Type) {
+       for {
+               at, ok := t.(*ArrayType)
+               if !ok {
+                       break
+               }
+               at.Count = 0
+               t = at.Type
+       }
+}
index b9470a4fcb4dad4dc27421906aa1ffd10f8ba923..b5b255f6f4a600846b0e44c2ff452982df544fb3 100644 (file)
@@ -25,13 +25,22 @@ var typedefTests = map[string]string{
        "t_func_void_of_char":                   "func(char) void",
        "t_func_void_of_void":                   "func() void",
        "t_func_void_of_ptr_char_dots":          "func(*char, ...) void",
-       "t_my_struct":                           "struct my_struct {vi volatile int@0; x char@4 : 1@7; y int@4 : 4@27; array [40]long long int@8}",
+       "t_my_struct":                           "struct my_struct {vi volatile int@0; x char@4 : 1@7; y int@4 : 4@27; z [0]int@8; array [40]long long int@8; zz [0]int@328}",
+       "t_my_struct1":                          "struct my_struct1 {zz [1]int@0}",
        "t_my_union":                            "union my_union {vi volatile int@0; x char@0 : 1@7; y int@0 : 4@28; array [40]long long int@0}",
        "t_my_enum":                             "enum my_enum {e1=1; e2=2; e3=-5; e4=1000000000000000}",
        "t_my_list":                             "struct list {val short int@0; next *t_my_list@8}",
        "t_my_tree":                             "struct tree {left *struct tree@0; right *struct tree@8; val long long unsigned int@16}",
 }
 
+// As Apple converts gcc to a clang-based front end
+// they keep breaking the DWARF output.  This map lists the
+// conversion from real answer to Apple answer.
+var machoBug = map[string]string{
+       "func(*char, ...) void":                                 "func(*char) void",
+       "enum my_enum {e1=1; e2=2; e3=-5; e4=1000000000000000}": "enum my_enum {e1=1; e2=2; e3=-5; e4=-1530494976}",
+}
+
 func elfData(t *testing.T, name string) *Data {
        f, err := elf.Open(name)
        if err != nil {
@@ -58,13 +67,13 @@ func machoData(t *testing.T, name string) *Data {
        return d
 }
 
-func TestTypedefsELF(t *testing.T) { testTypedefs(t, elfData(t, "testdata/typedef.elf")) }
+func TestTypedefsELF(t *testing.T) { testTypedefs(t, elfData(t, "testdata/typedef.elf"), "elf") }
 
 func TestTypedefsMachO(t *testing.T) {
-       testTypedefs(t, machoData(t, "testdata/typedef.macho"))
+       testTypedefs(t, machoData(t, "testdata/typedef.macho"), "macho")
 }
 
-func testTypedefs(t *testing.T, d *Data) {
+func testTypedefs(t *testing.T, d *Data, kind string) {
        r := d.Reader()
        seen := make(map[string]bool)
        for {
@@ -93,7 +102,7 @@ func testTypedefs(t *testing.T, d *Data) {
                                        t.Errorf("multiple definitions for %s", t1.Name)
                                }
                                seen[t1.Name] = true
-                               if typstr != want {
+                               if typstr != want && (kind != "macho" || typstr != machoBug[want]) {
                                        t.Errorf("%s:\n\thave %s\n\twant %s", t1.Name, typstr, want)
                                }
                        }