cmd/cgo: reject attempts to declare methods on C types

This change causes cgo to emit an error (with the same
message as the compiler) when it encounters a declaration
of a method whose receiver type is C.T or *C.T.

Conceptually, C is another package, but unfortunately
the desugaring of C.T is a type within the same package,
causing the previous behavior to accept invalid input.

It is likely that at least some users are intentionally
exploiting this behavior, so this may break their build.
We should mention it in the release notes.

Fixes #57926

Change-Id: I513cffb7e13bc93d08a07b7e61301ac1762fd42d
Reviewed-on: https://go-review.googlesource.com/c/go/+/490819
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>

Author: adonovan

src/cmd/cgo/ast.go

@@ -9,6 +9,7 @@ package main
 import (
 	"fmt"
 	"go/ast"
+	"go/format"
 	"go/parser"
 	"go/scanner"
 	"go/token"
@@ -62,29 +63,48 @@ func (f *File) ParseGo(abspath string, src []byte) {
 	// In ast1, find the import "C" line and get any extra C preamble.
 	sawC := false
 	for _, decl := range ast1.Decls {
-		d, ok := decl.(*ast.GenDecl)
-		if !ok {
-			continue
-		}
-		for _, spec := range d.Specs {
-			s, ok := spec.(*ast.ImportSpec)
-			if !ok || s.Path.Value != `"C"` {
-				continue
-			}
-			sawC = true
-			if s.Name != nil {
-				error_(s.Path.Pos(), `cannot rename import "C"`)
-			}
-			cg := s.Doc
-			if cg == nil && len(d.Specs) == 1 {
-				cg = d.Doc
+		switch decl := decl.(type) {
+		case *ast.GenDecl:
+			for _, spec := range decl.Specs {
+				s, ok := spec.(*ast.ImportSpec)
+				if !ok || s.Path.Value != `"C"` {
+					continue
+				}
+				sawC = true
+				if s.Name != nil {
+					error_(s.Path.Pos(), `cannot rename import "C"`)
+				}
+				cg := s.Doc
+				if cg == nil && len(decl.Specs) == 1 {
+					cg = decl.Doc
+				}
+				if cg != nil {
+					f.Preamble += fmt.Sprintf("#line %d %q\n", sourceLine(cg), abspath)
+					f.Preamble += commentText(cg) + "\n"
+					f.Preamble += "#line 1 \"cgo-generated-wrapper\"\n"
+				}
 			}
-			if cg != nil {
-				f.Preamble += fmt.Sprintf("#line %d %q\n", sourceLine(cg), abspath)
-				f.Preamble += commentText(cg) + "\n"
-				f.Preamble += "#line 1 \"cgo-generated-wrapper\"\n"
+
+		case *ast.FuncDecl:
+			// Also, reject attempts to declare methods on C.T or *C.T.
+			// (The generated code would otherwise accept this
+			// invalid input; see issue #57926.)
+			if decl.Recv != nil && len(decl.Recv.List) > 0 {
+				recvType := decl.Recv.List[0].Type
+				if recvType != nil {
+					t := recvType
+					if star, ok := unparen(t).(*ast.StarExpr); ok {
+						t = star.X
+					}
+					if sel, ok := unparen(t).(*ast.SelectorExpr); ok {
+						var buf strings.Builder
+						format.Node(&buf, fset, recvType)
+						error_(sel.Pos(), `cannot define new methods on non-local type %s`, &buf)
+					}
+				}
 			}
 		}
+
 	}
 	if !sawC {
 		error_(ast1.Package, `cannot find import "C"`)
@@ -542,3 +562,11 @@ func (f *File) walk(x interface{}, context astContext, visit func(*File, interfa
 		}
 	}
 }
+
+// If x is of the form (T), unparen returns unparen(T), otherwise it returns x.
+func unparen(x ast.Expr) ast.Expr {
+	if p, isParen := x.(*ast.ParenExpr); isParen {
+		x = unparen(p.X)
+	}
+	return x
+}

src/cmd/go/testdata/script/cgo_badmethod_issue57926.txt

@@ -0,0 +1,31 @@
+[short] skip
+[!cgo] skip
+
+# Test that cgo rejects attempts to declare methods
+# on the types C.T or *C.T; see issue #57926.
+
+! go build
+stderr 'cannot define new methods on non-local type C.T'
+stderr 'cannot define new methods on non-local type \*C.T'
+! stderr 'Alias'
+
+-- go.mod --
+module example.com
+go 1.12
+
+-- a.go --
+package a
+
+/*
+typedef int T;
+*/
+import "C"
+
+func (C.T) f() {}
+func (recv *C.T) g() {}
+
+// The check is more education than enforcement,
+// and is easily defeated using a type alias.
+type Alias = C.T
+func (Alias) h() {}
+func (*Alias) i() {}