From d5b9dc1317e3d898650fcb6e417f03b00e69270b Mon Sep 17 00:00:00 2001 From: wolf1996 Date: Fri, 13 Nov 2020 14:30:15 +0300 Subject: [PATCH] cmd/cgo: pass end position info for C function arguments. Pass information about original end position for c function arguments processed in pointer checking generated code. Fixes #42580 Change-Id: Ic8a578168362f0ca6055064dbbea092ad37477a6 Reviewed-on: https://go-review.googlesource.com/c/go/+/269760 Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Trust: Dmitri Shuralyov --- misc/cgo/errors/argposition_test.go | 134 +++++++++++++++++++++++++ misc/cgo/errors/testdata/issue42580.go | 44 ++++++++ src/cmd/cgo/gcc.go | 34 +++++-- 3 files changed, 203 insertions(+), 9 deletions(-) create mode 100644 misc/cgo/errors/argposition_test.go create mode 100644 misc/cgo/errors/testdata/issue42580.go diff --git a/misc/cgo/errors/argposition_test.go b/misc/cgo/errors/argposition_test.go new file mode 100644 index 0000000000..331095f747 --- /dev/null +++ b/misc/cgo/errors/argposition_test.go @@ -0,0 +1,134 @@ +// Copyright 2021 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 42580: cmd/cgo: shifting identifier position in ast + +package errorstest + +import ( + "bytes" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +type ShortPosition struct { + Line int + Column int + Visited bool +} + +type IdentPositionInfo map[string][]ShortPosition + +type Visitor struct { + identPosInfo IdentPositionInfo + fset *token.FileSet + t *testing.T +} + +func (v *Visitor) Visit(node ast.Node) ast.Visitor { + if ident, ok := node.(*ast.Ident); ok { + if expectedPositions, ok := v.identPosInfo[ident.Name]; ok { + gotMatch := false + var errorMessage strings.Builder + for caseIndex, expectedPos := range expectedPositions { + actualPosition := v.fset.PositionFor(ident.Pos(), true) + errorOccured := false + if expectedPos.Line != actualPosition.Line { + fmt.Fprintf(&errorMessage, "wrong line number for ident %s: expected: %d got: %d\n", ident.Name, expectedPos.Line, actualPosition.Line) + errorOccured = true + } + if expectedPos.Column != actualPosition.Column { + fmt.Fprintf(&errorMessage, "wrong column number for ident %s: expected: %d got: %d\n", ident.Name, expectedPos.Column, actualPosition.Column) + errorOccured = true + } + if errorOccured { + continue + } + gotMatch = true + expectedPositions[caseIndex].Visited = true + } + + if !gotMatch { + v.t.Errorf(errorMessage.String()) + } + } + } + return v +} + +func TestArgumentsPositions(t *testing.T) { + testdata, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + + tmpPath := t.TempDir() + + dir := filepath.Join(tmpPath, "src", "testpositions") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + + cmd := exec.Command("go", "tool", "cgo", + "-srcdir", testdata, + "-objdir", dir, + "issue42580.go") + cmd.Stderr = new(bytes.Buffer) + + err = cmd.Run() + if err != nil { + t.Fatalf("%s: %v\n%s", cmd, err, cmd.Stderr) + } + mainProcessed, err := ioutil.ReadFile(filepath.Join(dir, "issue42580.cgo1.go")) + if err != nil { + t.Fatal(err) + } + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "", mainProcessed, parser.AllErrors) + if err != nil { + fmt.Println(err) + return + } + + expectation := IdentPositionInfo{ + "checkedPointer": []ShortPosition{ + ShortPosition{ + Line: 32, + Column: 56, + }, + }, + "singleInnerPointerChecked": []ShortPosition{ + ShortPosition{ + Line: 37, + Column: 91, + }, + }, + "doublePointerChecked": []ShortPosition{ + ShortPosition{ + Line: 42, + Column: 91, + }, + }, + } + for _, decl := range f.Decls { + if fdecl, ok := decl.(*ast.FuncDecl); ok { + ast.Walk(&Visitor{expectation, fset, t}, fdecl.Body) + } + } + for ident, positions := range expectation { + for _, position := range positions { + if !position.Visited { + t.Errorf("Position %d:%d missed for %s ident", position.Line, position.Column, ident) + } + } + } +} diff --git a/misc/cgo/errors/testdata/issue42580.go b/misc/cgo/errors/testdata/issue42580.go new file mode 100644 index 0000000000..aba80dfeba --- /dev/null +++ b/misc/cgo/errors/testdata/issue42580.go @@ -0,0 +1,44 @@ +// Copyright 2021 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 42580: cmd/cgo: shifting identifier position in ast + +package cgotest + +// typedef int (*intFunc) (); +// +// char* strarg = ""; +// +// int func_with_char(char* arg, void* dummy) +// {return 5;} +// +// int* get_arr(char* arg, void* dummy) +// {return NULL;} +import "C" +import "unsafe" + +// Test variables +var ( + checkedPointer = []byte{1} + doublePointerChecked = []byte{1} + singleInnerPointerChecked = []byte{1} +) + +// This test checks the positions of variable identifiers. +// Changing the positions of the test variables idents after this point will break the test. + +func TestSingleArgumentCast() C.int { + retcode := C.func_with_char((*C.char)(unsafe.Pointer(&checkedPointer[0])), unsafe.Pointer(C.strarg)) + return retcode +} + +func TestSingleArgumentCastRecFuncAsSimpleArg() C.int { + retcode := C.func_with_char((*C.char)(unsafe.Pointer(C.get_arr((*C.char)(unsafe.Pointer(&singleInnerPointerChecked[0])), unsafe.Pointer(C.strarg)))), nil) + return retcode +} + +func TestSingleArgumentCastRecFunc() C.int { + retcode := C.func_with_char((*C.char)(unsafe.Pointer(C.get_arr((*C.char)(unsafe.Pointer(&doublePointerChecked[0])), unsafe.Pointer(C.strarg)))), unsafe.Pointer(C.strarg)) + return retcode +} diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index 775f20b09f..ae61725bc7 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -909,7 +909,7 @@ func (p *Package) rewriteCall(f *File, call *Call) (string, bool) { var sbCheck bytes.Buffer for i, param := range params { origArg := args[i] - arg, nu := p.mangle(f, &args[i]) + arg, nu := p.mangle(f, &args[i], true) if nu { needsUnsafe = true } @@ -952,7 +952,7 @@ func (p *Package) rewriteCall(f *File, call *Call) (string, bool) { sb.WriteString("return ") } - m, nu := p.mangle(f, &call.Call.Fun) + m, nu := p.mangle(f, &call.Call.Fun, false) if nu { needsUnsafe = true } @@ -1086,7 +1086,8 @@ func (p *Package) hasPointer(f *File, t ast.Expr, top bool) bool { // rewriting calls when it finds them. // It removes the corresponding references in f.Ref and f.Calls, so that we // don't try to do the replacement again in rewriteRef or rewriteCall. -func (p *Package) mangle(f *File, arg *ast.Expr) (ast.Expr, bool) { +// If addPosition is true, add position info to the idents of C names in arg. +func (p *Package) mangle(f *File, arg *ast.Expr, addPosition bool) (ast.Expr, bool) { needsUnsafe := false f.walk(arg, ctxExpr, func(f *File, arg interface{}, context astContext) { px, ok := arg.(*ast.Expr) @@ -1101,7 +1102,7 @@ func (p *Package) mangle(f *File, arg *ast.Expr) (ast.Expr, bool) { for _, r := range f.Ref { if r.Expr == px { - *px = p.rewriteName(f, r) + *px = p.rewriteName(f, r, addPosition) r.Done = true break } @@ -1361,7 +1362,7 @@ func (p *Package) rewriteRef(f *File) { } } - expr := p.rewriteName(f, r) + expr := p.rewriteName(f, r, false) if *godefs { // Substitute definition for mangled type name. @@ -1424,8 +1425,23 @@ func (p *Package) rewriteRef(f *File) { } // rewriteName returns the expression used to rewrite a reference. -func (p *Package) rewriteName(f *File, r *Ref) ast.Expr { - var expr ast.Expr = ast.NewIdent(r.Name.Mangle) // default +// If addPosition is true, add position info in the ident name. +func (p *Package) rewriteName(f *File, r *Ref, addPosition bool) ast.Expr { + getNewIdent := ast.NewIdent + if addPosition { + getNewIdent = func(newName string) *ast.Ident { + mangledIdent := ast.NewIdent(newName) + if len(newName) == len(r.Name.Go) { + return mangledIdent + } + p := fset.Position((*r.Expr).End()) + if p.Column == 0 { + return mangledIdent + } + return ast.NewIdent(fmt.Sprintf("%s /*line :%d:%d*/", newName, p.Line, p.Column)) + } + } + var expr ast.Expr = getNewIdent(r.Name.Mangle) // default switch r.Context { case ctxCall, ctxCall2: if r.Name.Kind != "func" { @@ -1453,7 +1469,7 @@ func (p *Package) rewriteName(f *File, r *Ref) ast.Expr { n.Mangle = "_C2func_" + n.Go f.Name["2"+r.Name.Go] = n } - expr = ast.NewIdent(n.Mangle) + expr = getNewIdent(n.Mangle) r.Name = n break } @@ -1484,7 +1500,7 @@ func (p *Package) rewriteName(f *File, r *Ref) ast.Expr { // issue 7757. expr = &ast.CallExpr{ Fun: &ast.Ident{NamePos: (*r.Expr).Pos(), Name: "_Cgo_ptr"}, - Args: []ast.Expr{ast.NewIdent(name.Mangle)}, + Args: []ast.Expr{getNewIdent(name.Mangle)}, } case "type": // Okay - might be new(T) -- 2.48.1