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 <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>

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)
+			}
+		}
+	}
+}

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
+}

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 {
+// If addPosition is true, add position info in the ident name.
-	var expr ast.Expr = ast.NewIdent(r.Name.Mangle) // default
+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)