Skip to content

Commit 1bdf1c3

Browse files
cmd/cgo: fix use of unsafe argument in new deferred function
The combination of https://golang.org/cl/23650 and https://golang.org/cl/23675 did not work--they were tested separately but not together. The problem was that 23650 introduced deferred argument checking, and the deferred function loses the type that 23675 started requiring. The fix is to go back to using an empty interface type in a deferred argument check. No new test required--fixes broken build. Change-Id: I5ea023c5aed71d70e57b11c4551242d3ef25986d Reviewed-on: https://go-review.googlesource.com/23961 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent 837984f commit 1bdf1c3

File tree

3 files changed

+32
-9
lines changed

3 files changed

+32
-9
lines changed

src/cmd/cgo/gcc.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ func (p *Package) rewriteCall(f *File, call *Call, name *Name) {
657657
// Instead we use a local variant of _cgoCheckPointer.
658658

659659
var arg ast.Expr
660-
if n := p.unsafeCheckPointerName(param.Go); n != "" {
660+
if n := p.unsafeCheckPointerName(param.Go, call.Deferred); n != "" {
661661
c.Fun = ast.NewIdent(n)
662662
arg = c
663663
} else {
@@ -939,20 +939,31 @@ func (p *Package) isType(t ast.Expr) bool {
939939
// assertion to unsafe.Pointer in our copy of user code. We return
940940
// the name of the _cgoCheckPointer function we are going to build, or
941941
// the empty string if the type does not use unsafe.Pointer.
942-
func (p *Package) unsafeCheckPointerName(t ast.Expr) string {
942+
//
943+
// The deferred parameter is true if this check is for the argument of
944+
// a deferred function. In that case we need to use an empty interface
945+
// as the argument type, because the deferred function we introduce in
946+
// rewriteCall will use an empty interface type, and we can't add a
947+
// type assertion. This is handled by keeping a separate list, and
948+
// writing out the lists separately in writeDefs.
949+
func (p *Package) unsafeCheckPointerName(t ast.Expr, deferred bool) string {
943950
if !p.hasUnsafePointer(t) {
944951
return ""
945952
}
946953
var buf bytes.Buffer
947954
conf.Fprint(&buf, fset, t)
948955
s := buf.String()
949-
for i, t := range p.CgoChecks {
956+
checks := &p.CgoChecks
957+
if deferred {
958+
checks = &p.DeferredCgoChecks
959+
}
960+
for i, t := range *checks {
950961
if s == t {
951-
return p.unsafeCheckPointerNameIndex(i)
962+
return p.unsafeCheckPointerNameIndex(i, deferred)
952963
}
953964
}
954-
p.CgoChecks = append(p.CgoChecks, s)
955-
return p.unsafeCheckPointerNameIndex(len(p.CgoChecks) - 1)
965+
*checks = append(*checks, s)
966+
return p.unsafeCheckPointerNameIndex(len(*checks)-1, deferred)
956967
}
957968

958969
// hasUnsafePointer returns whether the Go type t uses unsafe.Pointer.
@@ -980,7 +991,10 @@ func (p *Package) hasUnsafePointer(t ast.Expr) bool {
980991

981992
// unsafeCheckPointerNameIndex returns the name to use for a
982993
// _cgoCheckPointer variant based on the index in the CgoChecks slice.
983-
func (p *Package) unsafeCheckPointerNameIndex(i int) string {
994+
func (p *Package) unsafeCheckPointerNameIndex(i int, deferred bool) string {
995+
if deferred {
996+
return fmt.Sprintf("_cgoCheckPointerInDefer%d", i)
997+
}
984998
return fmt.Sprintf("_cgoCheckPointer%d", i)
985999
}
9861000

src/cmd/cgo/main.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ type Package struct {
4242
GoFiles []string // list of Go files
4343
GccFiles []string // list of gcc output files
4444
Preamble string // collected preamble for _cgo_export.h
45-
CgoChecks []string // see unsafeCheckPointerName
45+
46+
// See unsafeCheckPointerName.
47+
CgoChecks []string
48+
DeferredCgoChecks []string
4649
}
4750

4851
// A File collects information about a single Go input file.

src/cmd/cgo/out.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,17 @@ func (p *Package) writeDefs() {
112112
}
113113

114114
for i, t := range p.CgoChecks {
115-
n := p.unsafeCheckPointerNameIndex(i)
115+
n := p.unsafeCheckPointerNameIndex(i, false)
116116
fmt.Fprintf(fgo2, "\nfunc %s(p %s, args ...interface{}) %s {\n", n, t, t)
117117
fmt.Fprintf(fgo2, "\treturn _cgoCheckPointer(p, args...).(%s)\n", t)
118118
fmt.Fprintf(fgo2, "}\n")
119119
}
120+
for i, t := range p.DeferredCgoChecks {
121+
n := p.unsafeCheckPointerNameIndex(i, true)
122+
fmt.Fprintf(fgo2, "\nfunc %s(p interface{}, args ...interface{}) %s {\n", n, t)
123+
fmt.Fprintf(fgo2, "\treturn _cgoCheckPointer(p, args...).(%s)\n", t)
124+
fmt.Fprintf(fgo2, "}\n")
125+
}
120126

121127
gccgoSymbolPrefix := p.gccgoSymbolPrefix()
122128

0 commit comments

Comments
 (0)