Commit 90df3776 authored by Ian Lance Taylor's avatar Ian Lance Taylor

cmd/cgo: rewrite pointer checking to use more function literals

Fixes #14210
Fixes #25941

Change-Id: Idde2d032290da3edb742b5b4f6ffeb625f05b494
Reviewed-on: https://go-review.googlesource.com/c/142884Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
parent 02aa1aee
...@@ -357,6 +357,55 @@ var ptrTests = []ptrTest{ ...@@ -357,6 +357,55 @@ var ptrTests = []ptrTest{
body: `r, _, _ := os.Pipe(); r.SetDeadline(time.Now().Add(C.US * time.Microsecond))`, body: `r, _, _ := os.Pipe(); r.SetDeadline(time.Now().Add(C.US * time.Microsecond))`,
fail: false, fail: false,
}, },
{
// Test for double evaluation of channel receive.
name: "chan-recv",
c: `void f(char** p) {}`,
imports: []string{"time"},
body: `c := make(chan []*C.char, 2); c <- make([]*C.char, 1); go func() { time.Sleep(10 * time.Second); panic("received twice from chan") }(); C.f(&(<-c)[0]);`,
fail: false,
},
{
// Test that converting the address of a struct field
// to unsafe.Pointer still just checks that field.
// Issue #25941.
name: "struct-field",
c: `void f(void* p) {}`,
imports: []string{"unsafe"},
support: `type S struct { p *int; a [8]byte; u uintptr }`,
body: `s := &S{p: new(int)}; C.f(unsafe.Pointer(&s.a))`,
fail: false,
},
{
// Test that converting multiple struct field
// addresses to unsafe.Pointer still just checks those
// fields. Issue #25941.
name: "struct-field-2",
c: `void f(void* p, int r, void* s) {}`,
imports: []string{"unsafe"},
support: `type S struct { a [8]byte; p *int; b int64; }`,
body: `s := &S{p: new(int)}; C.f(unsafe.Pointer(&s.a), 32, unsafe.Pointer(&s.b))`,
fail: false,
},
{
// Test that second argument to cgoCheckPointer is
// evaluated when a deferred function is deferred, not
// when it is run.
name: "defer2",
c: `void f(char **pc) {}`,
support: `type S1 struct { s []*C.char }; type S2 struct { ps *S1 }`,
body: `p := &S2{&S1{[]*C.char{nil}}}; defer C.f(&p.ps.s[0]); p.ps = nil`,
fail: false,
},
{
// Test that indexing into a function call still
// examines only the slice being indexed.
name: "buffer",
c: `void f(void *p) {}`,
imports: []string{"bytes", "unsafe"},
body: `var b bytes.Buffer; b.WriteString("a"); C.f(unsafe.Pointer(&b.Bytes()[0]))`,
fail: false,
},
} }
func TestPointerChecks(t *testing.T) { func TestPointerChecks(t *testing.T) {
......
...@@ -721,7 +721,9 @@ func (p *Package) mangleName(n *Name) { ...@@ -721,7 +721,9 @@ func (p *Package) mangleName(n *Name) {
// This returns whether the package needs to import unsafe as _cgo_unsafe. // This returns whether the package needs to import unsafe as _cgo_unsafe.
func (p *Package) rewriteCalls(f *File) bool { func (p *Package) rewriteCalls(f *File) bool {
needsUnsafe := false needsUnsafe := false
for _, call := range f.Calls { // Walk backward so that in C.f1(C.f2()) we rewrite C.f2 first.
for i := len(f.Calls) - 1; i >= 0; i-- {
call := f.Calls[i]
// This is a call to C.xxx; set goname to "xxx". // This is a call to C.xxx; set goname to "xxx".
goname := call.Call.Fun.(*ast.SelectorExpr).Sel.Name goname := call.Call.Fun.(*ast.SelectorExpr).Sel.Name
if goname == "malloc" { if goname == "malloc" {
...@@ -768,103 +770,132 @@ func (p *Package) rewriteCall(f *File, call *Call, name *Name) bool { ...@@ -768,103 +770,132 @@ func (p *Package) rewriteCall(f *File, call *Call, name *Name) bool {
// We need to rewrite this call. // We need to rewrite this call.
// //
// We are going to rewrite C.f(p) to // Rewrite C.f(p) to
// func (_cgo0 ptype) { // func() {
// _cgo0 := p
// _cgoCheckPointer(_cgo0) // _cgoCheckPointer(_cgo0)
// C.f(_cgo0) // C.f(_cgo0)
// }(p) // }()
// Using a function literal like this lets us do correct // Using a function literal like this lets us evaluate the
// argument type checking, and works correctly if the call is // function arguments only once while doing pointer checks.
// deferred. // This is particularly useful when passing additional arguments
var sb bytes.Buffer // to _cgoCheckPointer, as done in checkIndex and checkAddr.
sb.WriteString("func(") //
// When the function argument is a conversion to unsafe.Pointer,
needsUnsafe := false // we unwrap the conversion before checking the pointer,
// and then wrap again when calling C.f. This lets us check
for i, param := range params { // the real type of the pointer in some cases. See issue #25941.
if i > 0 { //
sb.WriteString(", ") // When the call to C.f is deferred, we use an additional function
} // literal to evaluate the arguments at the right time.
// defer func() func() {
fmt.Fprintf(&sb, "_cgo%d ", i) // _cgo0 := p
// return func() {
// _cgoCheckPointer(_cgo0)
// C.f(_cgo0)
// }
// }()()
// This works because the defer statement evaluates the first
// function literal in order to get the function to call.
ptype := p.rewriteUnsafe(param.Go) var sb bytes.Buffer
if ptype != param.Go { sb.WriteString("func() ")
needsUnsafe = true if call.Deferred {
} sb.WriteString("func() ")
sb.WriteString(gofmtLine(ptype))
} }
sb.WriteString(")") needsUnsafe := false
result := false result := false
twoResults := false twoResults := false
if !call.Deferred {
// Check whether this call expects two results. // Check whether this call expects two results.
for _, ref := range f.Ref { for _, ref := range f.Ref {
if ref.Expr != &call.Call.Fun { if ref.Expr != &call.Call.Fun {
continue continue
} }
if ref.Context == ctxCall2 { if ref.Context == ctxCall2 {
sb.WriteString(" (") sb.WriteString("(")
result = true result = true
twoResults = true twoResults = true
}
break
} }
break
}
// Add the result type, if any. // Add the result type, if any.
if name.FuncType.Result != nil { if name.FuncType.Result != nil {
rtype := p.rewriteUnsafe(name.FuncType.Result.Go) rtype := p.rewriteUnsafe(name.FuncType.Result.Go)
if rtype != name.FuncType.Result.Go { if rtype != name.FuncType.Result.Go {
needsUnsafe = true needsUnsafe = true
} }
if !twoResults { sb.WriteString(gofmtLine(rtype))
sb.WriteString(" ") result = true
} }
sb.WriteString(gofmtLine(rtype))
result = true
}
// Add the second result type, if any. // Add the second result type, if any.
if twoResults { if twoResults {
if name.FuncType.Result == nil { if name.FuncType.Result == nil {
// An explicit void result looks odd but it // An explicit void result looks odd but it
// seems to be how cgo has worked historically. // seems to be how cgo has worked historically.
sb.WriteString("_Ctype_void") sb.WriteString("_Ctype_void")
}
sb.WriteString(", error)")
} }
sb.WriteString(", error)")
} }
sb.WriteString(" { ") sb.WriteString("{ ")
// Define _cgoN for each argument value.
// Write _cgoCheckPointer calls to sbCheck.
var sbCheck bytes.Buffer
for i, param := range params { for i, param := range params {
arg := args[i] arg := p.mangle(f, &args[i])
if !p.needsPointerCheck(f, param.Go, arg) {
// Explicitly convert untyped constants to the
// parameter type, to avoid a type mismatch.
if p.isConst(f, arg) {
ptype := p.rewriteUnsafe(param.Go)
if ptype != param.Go {
needsUnsafe = true
}
arg = &ast.CallExpr{
Fun: ptype,
Args: []ast.Expr{arg},
}
}
if !p.needsPointerCheck(f, param.Go, args[i]) {
fmt.Fprintf(&sb, "_cgo%d := %s; ", i, gofmtLine(arg))
continue continue
} }
// Check for &a[i]. // Check for &a[i].
if p.checkIndex(&sb, f, arg, i) { if p.checkIndex(&sb, &sbCheck, arg, i) {
continue continue
} }
// Check for &x. // Check for &x.
if p.checkAddr(&sb, arg, i) { if p.checkAddr(&sb, &sbCheck, arg, i) {
continue continue
} }
fmt.Fprintf(&sb, "_cgoCheckPointer(_cgo%d); ", i) fmt.Fprintf(&sb, "_cgo%d := %s; ", i, gofmtLine(arg))
fmt.Fprintf(&sbCheck, "_cgoCheckPointer(_cgo%d); ", i)
}
if call.Deferred {
sb.WriteString("return func() { ")
} }
// Write out the calls to _cgoCheckPointer.
sb.WriteString(sbCheck.String())
if result { if result {
sb.WriteString("return ") sb.WriteString("return ")
} }
// Now we are ready to call the C function. // Now we are ready to call the C function.
// To work smoothly with rewriteRef we leave the call in place // To work smoothly with rewriteRef we leave the call in place
// and just insert our new arguments between the function // and just replace the old arguments with our new ones.
// and the old arguments.
f.Edit.Insert(f.offset(call.Call.Fun.Pos()), sb.String()) f.Edit.Insert(f.offset(call.Call.Fun.Pos()), sb.String())
sb.Reset() sb.Reset()
...@@ -875,9 +906,17 @@ func (p *Package) rewriteCall(f *File, call *Call, name *Name) bool { ...@@ -875,9 +906,17 @@ func (p *Package) rewriteCall(f *File, call *Call, name *Name) bool {
} }
fmt.Fprintf(&sb, "_cgo%d", i) fmt.Fprintf(&sb, "_cgo%d", i)
} }
sb.WriteString("); }") sb.WriteString("); ")
if call.Deferred {
sb.WriteString("}")
}
sb.WriteString("}")
if call.Deferred {
sb.WriteString("()")
}
sb.WriteString("()")
f.Edit.Insert(f.offset(call.Call.Lparen), sb.String()) f.Edit.Replace(f.offset(call.Call.Lparen), f.offset(call.Call.Rparen)+1, sb.String())
return needsUnsafe return needsUnsafe
} }
...@@ -986,11 +1025,44 @@ func (p *Package) hasPointer(f *File, t ast.Expr, top bool) bool { ...@@ -986,11 +1025,44 @@ func (p *Package) hasPointer(f *File, t ast.Expr, top bool) bool {
} }
} }
// mangle replaces references to C names in arg with the mangled names.
// It removes the corresponding references in f.Ref, so that we don't
// try to do the replacement again in rewriteRef.
func (p *Package) mangle(f *File, arg *ast.Expr) ast.Expr {
f.walk(arg, ctxExpr, func(f *File, arg interface{}, context astContext) {
px, ok := arg.(*ast.Expr)
if !ok {
return
}
sel, ok := (*px).(*ast.SelectorExpr)
if !ok {
return
}
if l, ok := sel.X.(*ast.Ident); !ok || l.Name != "C" {
return
}
for _, r := range f.Ref {
if r.Expr == px {
*px = p.rewriteName(f, r)
r.Done = true
break
}
}
})
return *arg
}
// checkIndex checks whether arg the form &a[i], possibly inside type // checkIndex checks whether arg the form &a[i], possibly inside type
// conversions. If so, and if a has no side effects, it writes // conversions. If so, it writes
// _cgoCheckPointer(_cgoNN, a) to sb and returns true. This tells // _cgoIndexNN := a
// _cgoCheckPointer to check the complete contents of the slice. // _cgoNN := &cgoIndexNN[i] // with type conversions, if any
func (p *Package) checkIndex(sb *bytes.Buffer, f *File, arg ast.Expr, i int) bool { // to sb, and writes
// _cgoCheckPointer(_cgoNN, _cgoIndexNN)
// to sbCheck, and returns true. This tells _cgoCheckPointer to check
// the complete contents of the slice or array being indexed, but no
// other part of the memory allocation.
func (p *Package) checkIndex(sb, sbCheck *bytes.Buffer, arg ast.Expr, i int) bool {
// Strip type conversions. // Strip type conversions.
x := arg x := arg
for { for {
...@@ -1008,22 +1080,29 @@ func (p *Package) checkIndex(sb *bytes.Buffer, f *File, arg ast.Expr, i int) boo ...@@ -1008,22 +1080,29 @@ func (p *Package) checkIndex(sb *bytes.Buffer, f *File, arg ast.Expr, i int) boo
if !ok { if !ok {
return false return false
} }
if p.hasSideEffects(f, index.X) {
return false
}
fmt.Fprintf(sb, "_cgoCheckPointer(_cgo%d, %s); ", i, gofmtLine(index.X)) fmt.Fprintf(sb, "_cgoIndex%d := %s; ", i, gofmtLine(index.X))
origX := index.X
index.X = ast.NewIdent(fmt.Sprintf("_cgoIndex%d", i))
fmt.Fprintf(sb, "_cgo%d := %s; ", i, gofmtLine(arg))
index.X = origX
fmt.Fprintf(sbCheck, "_cgoCheckPointer(_cgo%d, _cgoIndex%d); ", i, i)
return true return true
} }
// checkAddr checks whether arg has the form &x, possibly inside type // checkAddr checks whether arg has the form &x, possibly inside type
// conversions. If so it writes _cgoCheckPointer(_cgoNN, true) to sb // conversions. If so it writes
// and returns true. This tells _cgoCheckPointer to check just the // _cgoBaseNN := &x
// contents of the pointer being passed, not any other part of the // _cgoNN := _cgoBaseNN // with type conversions, if any
// memory allocation. This is run after checkIndex, which looks for // to sb, and writes
// the special case of &a[i], which requires different checks. // _cgoCheckPointer(_cgoBaseNN, true)
func (p *Package) checkAddr(sb *bytes.Buffer, arg ast.Expr, i int) bool { // to sbCheck, and returns true. This tells _cgoCheckPointer to check
// just the contents of the pointer being passed, not any other part
// of the memory allocation. This is run after checkIndex, which looks
// for the special case of &a[i], which requires different checks.
func (p *Package) checkAddr(sb, sbCheck *bytes.Buffer, arg ast.Expr, i int) bool {
// Strip type conversions. // Strip type conversions.
px := &arg px := &arg
for { for {
...@@ -1037,27 +1116,20 @@ func (p *Package) checkAddr(sb *bytes.Buffer, arg ast.Expr, i int) bool { ...@@ -1037,27 +1116,20 @@ func (p *Package) checkAddr(sb *bytes.Buffer, arg ast.Expr, i int) bool {
return false return false
} }
fmt.Fprintf(sb, "_cgoBase%d := %s; ", i, gofmtLine(*px))
origX := *px
*px = ast.NewIdent(fmt.Sprintf("_cgoBase%d", i))
fmt.Fprintf(sb, "_cgo%d := %s; ", i, gofmtLine(arg))
*px = origX
// Use "0 == 0" to do the right thing in the unlikely event // Use "0 == 0" to do the right thing in the unlikely event
// that "true" is shadowed. // that "true" is shadowed.
fmt.Fprintf(sb, "_cgoCheckPointer(_cgo%d, 0 == 0); ", i) fmt.Fprintf(sbCheck, "_cgoCheckPointer(_cgoBase%d, 0 == 0); ", i)
return true return true
} }
// hasSideEffects returns whether the expression x has any side
// effects. x is an expression, not a statement, so the only side
// effect is a function call.
func (p *Package) hasSideEffects(f *File, x ast.Expr) bool {
found := false
f.walk(x, ctxExpr,
func(f *File, x interface{}, context astContext) {
if _, ok := x.(*ast.CallExpr); ok {
found = true
}
})
return found
}
// isType returns whether the expression is definitely a type. // isType returns whether the expression is definitely a type.
// This is conservative--it returns false for an unknown identifier. // This is conservative--it returns false for an unknown identifier.
func (p *Package) isType(t ast.Expr) bool { func (p *Package) isType(t ast.Expr) bool {
...@@ -1087,6 +1159,9 @@ func (p *Package) isType(t ast.Expr) bool { ...@@ -1087,6 +1159,9 @@ func (p *Package) isType(t ast.Expr) bool {
return true return true
} }
if strings.HasPrefix(t.Name, "_Ctype_") {
return true
}
case *ast.StarExpr: case *ast.StarExpr:
return p.isType(t.X) return p.isType(t.X)
case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType,
...@@ -1097,6 +1172,29 @@ func (p *Package) isType(t ast.Expr) bool { ...@@ -1097,6 +1172,29 @@ func (p *Package) isType(t ast.Expr) bool {
return false return false
} }
// isConst returns whether x is an untyped constant.
func (p *Package) isConst(f *File, x ast.Expr) bool {
switch x := x.(type) {
case *ast.BasicLit:
return true
case *ast.SelectorExpr:
id, ok := x.X.(*ast.Ident)
if !ok || id.Name != "C" {
return false
}
name := f.Name[x.Sel.Name]
if name != nil {
return name.IsConst()
}
case *ast.Ident:
return x.Name == "nil" ||
strings.HasPrefix(x.Name, "_Ciconst_") ||
strings.HasPrefix(x.Name, "_Cfconst_") ||
strings.HasPrefix(x.Name, "_Csconst_")
}
return false
}
// rewriteUnsafe returns a version of t with references to unsafe.Pointer // rewriteUnsafe returns a version of t with references to unsafe.Pointer
// rewritten to use _cgo_unsafe.Pointer instead. // rewritten to use _cgo_unsafe.Pointer instead.
func (p *Package) rewriteUnsafe(t ast.Expr) ast.Expr { func (p *Package) rewriteUnsafe(t ast.Expr) ast.Expr {
...@@ -1205,11 +1303,13 @@ func (p *Package) rewriteRef(f *File) { ...@@ -1205,11 +1303,13 @@ func (p *Package) rewriteRef(f *File) {
*r.Expr = expr *r.Expr = expr
// Record source-level edit for cgo output. // Record source-level edit for cgo output.
repl := gofmt(expr) if !r.Done {
if r.Name.Kind != "type" { repl := gofmt(expr)
repl = "(" + repl + ")" if r.Name.Kind != "type" {
repl = "(" + repl + ")"
}
f.Edit.Replace(f.offset(old.Pos()), f.offset(old.End()), repl)
} }
f.Edit.Replace(f.offset(old.Pos()), f.offset(old.End()), repl)
} }
// Remove functions only used as expressions, so their respective // Remove functions only used as expressions, so their respective
......
...@@ -88,6 +88,7 @@ type Ref struct { ...@@ -88,6 +88,7 @@ type Ref struct {
Name *Name Name *Name
Expr *ast.Expr Expr *ast.Expr
Context astContext Context astContext
Done bool
} }
func (r *Ref) Pos() token.Pos { func (r *Ref) Pos() token.Pos {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment