From 1bed192de059e05bda2d024dae39adbb43ba335f Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Fri, 12 Mar 2021 17:47:06 +0100 Subject: [PATCH] cgo: add support for CFLAGS in .c files This patch adds support for passing CFLAGS added in #cgo lines of the CGo preprocessing phase to the compiler when compiling C files inside packages. This is expected and convenient but didn't work before. --- builder/build.go | 5 +++-- builder/cc.go | 4 ++-- cgo/cgo.go | 31 +++++++++++++++++-------------- cgo/cgo_test.go | 2 +- loader/loader.go | 12 ++++++------ testdata/cgo/main.c | 2 ++ testdata/cgo/main.go | 4 ++++ testdata/cgo/main.h | 2 ++ testdata/cgo/out.txt | 1 + 9 files changed, 38 insertions(+), 25 deletions(-) diff --git a/builder/build.go b/builder/build.go index a5fd84b4..1a9ad9c1 100644 --- a/builder/build.go +++ b/builder/build.go @@ -430,7 +430,7 @@ func Build(pkgName, outpath string, config *compileopts.Config, action func(Buil job := &compileJob{ description: "compile extra file " + path, run: func(job *compileJob) error { - result, err := compileAndCacheCFile(abspath, dir, config) + result, err := compileAndCacheCFile(abspath, dir, config.CFlags(), config) job.result = result return err }, @@ -443,12 +443,13 @@ func Build(pkgName, outpath string, config *compileopts.Config, action func(Buil // TODO: do this as part of building the package to be able to link the // bitcode files together. for _, pkg := range lprogram.Sorted() { + pkg := pkg for _, filename := range pkg.CFiles { abspath := filepath.Join(pkg.Dir, filename) job := &compileJob{ description: "compile CGo file " + abspath, run: func(job *compileJob) error { - result, err := compileAndCacheCFile(abspath, dir, config) + result, err := compileAndCacheCFile(abspath, dir, pkg.CFlags, config) job.result = result return err }, diff --git a/builder/cc.go b/builder/cc.go index 3f78ce00..0261fdb4 100644 --- a/builder/cc.go +++ b/builder/cc.go @@ -57,7 +57,7 @@ import ( // depfile but without invalidating its name. For this reason, the depfile is // written on each new compilation (even when it seems unnecessary). However, it // could in rare cases lead to a stale file fetched from the cache. -func compileAndCacheCFile(abspath, tmpdir string, config *compileopts.Config) (string, error) { +func compileAndCacheCFile(abspath, tmpdir string, cflags []string, config *compileopts.Config) (string, error) { // Hash input file. fileHash, err := hashFile(abspath) if err != nil { @@ -121,7 +121,7 @@ func compileAndCacheCFile(abspath, tmpdir string, config *compileopts.Config) (s return "", err } depTmpFile.Close() - flags := config.CFlags() + flags := append([]string{}, cflags...) // copy cflags flags = append(flags, "-MD", "-MV", "-MTdeps", "-MF", depTmpFile.Name()) // autogenerate dependencies flags = append(flags, "-c", "-o", objTmpFile.Name(), abspath) if config.Options.PrintCommands { diff --git a/cgo/cgo.go b/cgo/cgo.go index c496cfef..8581807f 100644 --- a/cgo/cgo.go +++ b/cgo/cgo.go @@ -41,7 +41,8 @@ type cgoPackage struct { elaboratedTypes map[string]*elaboratedTypeInfo enums map[string]enumInfo anonStructNum int - ldflags []string + cflags []string // CFlags from #cgo lines + ldflags []string // LDFlags from #cgo lines visitedFiles map[string][]byte } @@ -157,10 +158,10 @@ 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 LDFLAGS for this package, 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, map[string][]byte, []error) { +// 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) { p := &cgoPackage{ dir: dir, fset: fset, @@ -175,11 +176,6 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string visitedFiles: map[string][]byte{}, } - // Disable _FORTIFY_SOURCE as it causes problems on macOS. - // Note that it is only disabled for memcpy (etc) calls made from Go, which - // have better alternatives anyway. - cflags = append(cflags, "-D_FORTIFY_SOURCE=0") - // Add a new location for the following file. generatedTokenPos := p.fset.AddFile(dir+"/!cgo.go", -1, 0) generatedTokenPos.SetLines([]int{0}) @@ -188,7 +184,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, []error{ + return 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 @@ -363,7 +359,7 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string continue } makePathsAbsolute(flags, packagePath) - cflags = append(cflags, flags...) + p.cflags = append(p.cflags, flags...) case "LDFLAGS": flags, err := shlex.Split(value) if err != nil { @@ -386,6 +382,13 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string } } + // Define CFlags that will be used while parsing the package. + // Disable _FORTIFY_SOURCE as it causes problems on macOS. + // Note that it is only disabled for memcpy (etc) calls made from Go, which + // have better alternatives anyway. + cflagsForCGo := append([]string{"-D_FORTIFY_SOURCE=0"}, cflags...) + cflagsForCGo = append(cflagsForCGo, p.cflags...) + // Process all CGo imports. for _, genDecl := range statements { cgoComment := genDecl.Doc.Text() @@ -395,7 +398,7 @@ func Process(files []*ast.File, dir string, fset *token.FileSet, cflags []string pos = genDecl.Doc.Pos() } position := fset.PositionFor(pos, true) - p.parseFragment(cgoComment+cgoTypes, cflags, position.Filename, position.Line) + p.parseFragment(cgoComment+cgoTypes, cflagsForCGo, position.Filename, position.Line) } // Declare functions found by libclang. @@ -430,7 +433,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.ldflags, p.visitedFiles, p.errors + return p.generated, 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 33a49b96..625a11f3 100644 --- a/cgo/cgo_test.go +++ b/cgo/cgo_test.go @@ -65,7 +65,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/loader/loader.go b/loader/loader.go index 1eed4aae..4672f406 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -382,14 +382,14 @@ func (p *Package) parseFiles() ([]*ast.File, error) { // Do CGo processing. if len(p.CgoFiles) != 0 { - var cflags []string - cflags = append(cflags, p.program.config.CFlags()...) - cflags = append(cflags, "-I"+p.Dir) + var initialCFlags []string + initialCFlags = append(initialCFlags, p.program.config.CFlags()...) + initialCFlags = append(initialCFlags, "-I"+p.Dir) if p.program.clangHeaders != "" { - cflags = append(cflags, "-Xclang", "-internal-isystem", "-Xclang", p.program.clangHeaders) + initialCFlags = append(initialCFlags, "-Xclang", "-internal-isystem", "-Xclang", p.program.clangHeaders) } - p.CFlags = cflags - generated, ldflags, accessedFiles, errs := cgo.Process(files, p.program.workingDir, p.program.fset, cflags) + generated, cflags, ldflags, accessedFiles, errs := cgo.Process(files, p.program.workingDir, p.program.fset, initialCFlags) + p.CFlags = append(initialCFlags, cflags...) for path, hash := range accessedFiles { p.FileHashes[path] = hash } diff --git a/testdata/cgo/main.c b/testdata/cgo/main.c index 3a2e9c57..7954b91f 100644 --- a/testdata/cgo/main.c +++ b/testdata/cgo/main.c @@ -20,6 +20,8 @@ int globalUnionSize = sizeof(globalUnion); option_t globalOption = optionG; bitfield_t globalBitfield = {244, 15, 1, 2, 47, 5}; +int cflagsConstant = SOME_CONSTANT; + int smallEnumWidth = sizeof(option2_t); int fortytwo() { diff --git a/testdata/cgo/main.go b/testdata/cgo/main.go index d9e2d729..55f9f4c4 100644 --- a/testdata/cgo/main.go +++ b/testdata/cgo/main.go @@ -5,6 +5,7 @@ int fortytwo(void); #include "main.h" int mul(int, int); #include +#cgo CFLAGS: -DSOME_CONSTANT=17 */ import "C" @@ -118,6 +119,9 @@ func main() { // Check that enums are considered the same width in C and CGo. println("enum width matches:", unsafe.Sizeof(C.option2_t(0)) == uintptr(C.smallEnumWidth)) + // Check whether CFLAGS are correctly passed on to compiled C files. + println("CFLAGS value:", C.cflagsConstant) + // libc: test whether C functions work at all. buf1 := []byte("foobar\x00") buf2 := make([]byte, len(buf1)) diff --git a/testdata/cgo/main.h b/testdata/cgo/main.h index 857a796e..ddd07efa 100644 --- a/testdata/cgo/main.h +++ b/testdata/cgo/main.h @@ -139,6 +139,8 @@ extern bitfield_t globalBitfield; extern int smallEnumWidth; +extern int cflagsConstant; + // test duplicate definitions int add(int a, int b); extern int global; diff --git a/testdata/cgo/out.txt b/testdata/cgo/out.txt index 31fd4d33..2eb4f354 100644 --- a/testdata/cgo/out.txt +++ b/testdata/cgo/out.txt @@ -58,4 +58,5 @@ option G: 12 option 2A: 20 option 3A: 21 enum width matches: true +CFLAGS value: 17 copied string: foobar