From e02727679fc9cc24e59e6d5378eabd313605cfc6 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Fri, 17 Sep 2021 02:41:45 +0200 Subject: [PATCH] builder, cgo: support function definitions in CGo headers For example, the following did not work before but does work with this change: // int add(int a, int b) { // return a + b; // } import "C" func main() { println("add:", C.add(3, 5)) } Even better, the functions in the header are compiled together with the rest of the Go code and so they can be optimized together! Currently, inlining is not yet allowed but const-propagation across functions works. This should be improved in the future. --- builder/build.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ cgo/cgo.go | 37 +++++++++++++++++++++++++++++++------ cgo/cgo_test.go | 2 +- compiler/compiler.go | 4 ++-- loader/loader.go | 4 +++- testdata/cgo/main.go | 4 ++++ testdata/cgo/out.txt | 1 + 7 files changed, 86 insertions(+), 10 deletions(-) diff --git a/builder/build.go b/builder/build.go index 1bfc3a41..12ea76f1 100644 --- a/builder/build.go +++ b/builder/build.go @@ -207,6 +207,50 @@ func Build(pkgName, outpath string, config *compileopts.Config, action func(Buil return errors.New("verification error after compiling package " + pkg.ImportPath) } + // Load bitcode of CGo headers and join the modules together. + // This may seem vulnerable to cache problems, but this is not + // the case: the Go code that was just compiled already tracks + // all C files that are read and hashes them. + // These headers could be compiled in parallel but the benefit + // is so small that it's probably not worth parallelizing. + // Packages are compiled independently anyway. + for _, cgoHeader := range pkg.CGoHeaders { + // Store the header text in a temporary file. + f, err := ioutil.TempFile(dir, "cgosnippet-*.c") + if err != nil { + return err + } + _, err = f.Write([]byte(cgoHeader)) + if err != nil { + return err + } + f.Close() + + // Compile the code (if there is any) to bitcode. + flags := append([]string{"-c", "-emit-llvm", "-o", f.Name() + ".bc", f.Name()}, pkg.CFlags...) + if config.Options.PrintCommands != nil { + config.Options.PrintCommands("clang", flags...) + } + err = runCCompiler(flags...) + if err != nil { + return &commandError{"failed to build CGo header", "", err} + } + + // Load and link the bitcode. + // This makes it possible to optimize the functions defined + // in the header together with the Go code. In particular, + // this allows inlining. It also ensures there is only one + // file per package to cache. + headerMod, err := mod.Context().ParseBitcodeFile(f.Name() + ".bc") + if err != nil { + return fmt.Errorf("failed to load bitcode file: %w", err) + } + err = llvm.LinkModules(mod, headerMod) + if err != nil { + return fmt.Errorf("failed to link module: %w", err) + } + } + // Erase all globals that are part of the undefinedGlobals list. // This list comes from the -ldflags="-X pkg.foo=val" option. // Instead of setting the value directly in the AST (which would diff --git a/cgo/cgo.go b/cgo/cgo.go index a509e4b9..8a47eb9c 100644 --- a/cgo/cgo.go +++ b/cgo/cgo.go @@ -29,6 +29,7 @@ import ( type cgoPackage struct { generated *ast.File generatedPos token.Pos + cgoHeaders []string errors []error dir string fset *token.FileSet @@ -158,10 +159,11 @@ typedef unsigned long long _Cgo_ulonglong; // Process extracts `import "C"` statements from the AST, parses the comment // with libclang, and modifies the AST to use this information. It returns a // newly created *ast.File that should be added to the list of to-be-parsed -// files, the CFLAGS and LDFLAGS found in #cgo lines, and a map of file hashes -// of the accessed C header files. If there is one or more error, it returns -// these in the []error slice but still modifies the AST. -func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string) (*ast.File, []string, []string, map[string][]byte, []error) { +// files, the CGo header snippets that should be compiled (for inline +// functions), the CFLAGS and LDFLAGS found in #cgo lines, and a map of file +// hashes of the accessed C header files. If there is one or more error, it +// returns these in the []error slice but still modifies the AST. +func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string) (*ast.File, []string, []string, []string, map[string][]byte, []error) { p := &cgoPackage{ dir: dir, fset: fset, @@ -184,7 +186,7 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string // Find the absolute path for this package. packagePath, err := filepath.Abs(fset.File(files[0].Pos()).Name()) if err != nil { - return nil, nil, nil, nil, []error{ + return nil, nil, nil, nil, nil, []error{ scanner.Error{ Pos: fset.Position(files[0].Pos()), Msg: "cgo: cannot find absolute path: " + err.Error(), // TODO: wrap this error @@ -258,6 +260,7 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string // Find `import "C"` statements in the file. var statements []*ast.GenDecl for _, f := range files { + var cgoHeader string for i := 0; i < len(f.Decls); i++ { decl := f.Decls[i] genDecl, ok := decl.(*ast.GenDecl) @@ -284,11 +287,33 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string // Found a CGo statement. statements = append(statements, genDecl) + // Store the text of the CGo fragment so it can be compiled. This is + // for cases like these, where functions are defined directly in the + // header: + // // int add(int a, int b) { + // // return a + b; + // // } + // import "C" + if genDecl.Doc != nil { + position := fset.Position(genDecl.Doc.Pos()) + lines := []string{fmt.Sprintf("# %d %#v\n", position.Line, position.Filename)} + for _, line := range strings.Split(getCommentText(genDecl.Doc), "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "#cgo") { + line = "" + } + lines = append(lines, line) + } + fragment := strings.Join(lines, "\n") + cgoHeader += fragment + } + // Remove this import declaration. f.Decls = append(f.Decls[:i], f.Decls[i+1:]...) i-- } + p.cgoHeaders = append(p.cgoHeaders, cgoHeader) + // Print the AST, for debugging. //ast.Print(fset, f) } @@ -436,7 +461,7 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string // Print the newly generated in-memory AST, for debugging. //ast.Print(fset, p.generated) - return p.generated, p.cflags, p.ldflags, p.visitedFiles, p.errors + return p.generated, p.cgoHeaders, p.cflags, p.ldflags, p.visitedFiles, p.errors } // makePathsAbsolute converts some common path compiler flags (-I, -L) from diff --git a/cgo/cgo_test.go b/cgo/cgo_test.go index 88c506d4..a5043712 100644 --- a/cgo/cgo_test.go +++ b/cgo/cgo_test.go @@ -56,7 +56,7 @@ func TestCGo(t *testing.T) { } // Process the AST with CGo. - cgoAST, _, _, _, cgoErrors := Process([]*ast.File{f}, "testdata", fset, cflags) + cgoAST, _, _, _, _, cgoErrors := Process([]*ast.File{f}, "testdata", fset, cflags) // Check the AST for type errors. var typecheckErrors []error diff --git a/compiler/compiler.go b/compiler/compiler.go index 2b1ab019..cd2585f9 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -283,14 +283,14 @@ func CompilePackage(moduleName string, pkg *loader.Package, ssaPkg *ssa.Package, if c.Debug { c.mod.AddNamedMetadataOperand("llvm.module.flags", c.ctx.MDNode([]llvm.Metadata{ - llvm.ConstInt(c.ctx.Int32Type(), 1, false).ConstantAsMetadata(), // Error on mismatch + llvm.ConstInt(c.ctx.Int32Type(), 2, false).ConstantAsMetadata(), // Warning on mismatch c.ctx.MDString("Debug Info Version"), llvm.ConstInt(c.ctx.Int32Type(), 3, false).ConstantAsMetadata(), // DWARF version }), ) c.mod.AddNamedMetadataOperand("llvm.module.flags", c.ctx.MDNode([]llvm.Metadata{ - llvm.ConstInt(c.ctx.Int32Type(), 1, false).ConstantAsMetadata(), + llvm.ConstInt(c.ctx.Int32Type(), 7, false).ConstantAsMetadata(), // Max on mismatch c.ctx.MDString("Dwarf Version"), llvm.ConstInt(c.ctx.Int32Type(), 4, false).ConstantAsMetadata(), }), diff --git a/loader/loader.go b/loader/loader.go index a201b152..3991fdcf 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -72,6 +72,7 @@ type Package struct { Files []*ast.File FileHashes map[string][]byte CFlags []string // CFlags used during CGo preprocessing (only set if CGo is used) + CGoHeaders []string // text above 'import "C"' lines Pkg *types.Package info types.Info } @@ -397,8 +398,9 @@ func (p *Package) parseFiles() ([]*ast.File, error) { if p.program.clangHeaders != "" { initialCFlags = append(initialCFlags, "-Xclang", "-internal-isystem", "-Xclang", p.program.clangHeaders) } - generated, cflags, ldflags, accessedFiles, errs := cgo.Process(files, p.program.workingDir, p.program.fset, initialCFlags) + generated, headerCode, cflags, ldflags, accessedFiles, errs := cgo.Process(files, p.program.workingDir, p.program.fset, initialCFlags) p.CFlags = append(initialCFlags, cflags...) + p.CGoHeaders = headerCode for path, hash := range accessedFiles { p.FileHashes[path] = hash } diff --git a/testdata/cgo/main.go b/testdata/cgo/main.go index 36ef2905..113176df 100644 --- a/testdata/cgo/main.go +++ b/testdata/cgo/main.go @@ -10,6 +10,7 @@ int mul(int, int); */ import "C" +// int headerfunc(int a) { return a + 1; } import "C" import "unsafe" @@ -43,6 +44,9 @@ func main() { println("variadic0:", C.variadic0()) println("variadic2:", C.variadic2(3, 5)) + // functions in the header C snippet + println("headerfunc:", C.headerfunc(5)) + // equivalent types var goInt8 int8 = 5 var _ C.int8_t = goInt8 diff --git a/testdata/cgo/out.txt b/testdata/cgo/out.txt index 5f4b5f41..00444cfa 100644 --- a/testdata/cgo/out.txt +++ b/testdata/cgo/out.txt @@ -15,6 +15,7 @@ callback 1: 50 callback 2: 600 variadic0: 1 variadic2: 15 +headerfunc: 6 bool: true true float: +3.100000e+000 double: +3.200000e+000