From 1a3e968b1fcb2082b1d99be563a7c9f8c61c66ba Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sun, 21 Feb 2021 22:09:03 +0700 Subject: [PATCH] cmd/compile: fix mishandling of unsafe-uintptr arguments with call method in go/defer In CL 253457, we did the same fix for direct function calls. But for method calls, the receiver argument also need to be passed through the wrapper function, which we are not doing so the compiler crashes with the code in #44415. As we already rewrite t.M(...) into T.M(t, ...) during walkCall1, to fix this, we can do the same trick in wrapCall, so the receiver argument will be treated as others. Fixes #44415 Change-Id: I396182983c85d9c5e4494657da79d25636e8a079 Reviewed-on: https://go-review.googlesource.com/c/go/+/294849 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le Reviewed-by: Matthew Dempsky TryBot-Result: Go Bot --- src/cmd/compile/internal/walk/expr.go | 32 +++++++++++++++------------ src/cmd/compile/internal/walk/stmt.go | 3 +++ test/fixedbugs/issue24491a.go | 30 +++++++++++++++++++++++++ test/fixedbugs/issue44415.go | 21 ++++++++++++++++++ 4 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 test/fixedbugs/issue44415.go diff --git a/src/cmd/compile/internal/walk/expr.go b/src/cmd/compile/internal/walk/expr.go index 7b65db5100..ce95fbc2b4 100644 --- a/src/cmd/compile/internal/walk/expr.go +++ b/src/cmd/compile/internal/walk/expr.go @@ -503,21 +503,8 @@ func walkCall1(n *ir.CallExpr, init *ir.Nodes) { } n.SetWalked(true) - // If this is a method call t.M(...), - // rewrite into a function call T.M(t, ...). // TODO(mdempsky): Do this right after type checking. - if n.Op() == ir.OCALLMETH { - withRecv := make([]ir.Node, len(n.Args)+1) - dot := n.X.(*ir.SelectorExpr) - withRecv[0] = dot.X - copy(withRecv[1:], n.Args) - n.Args = withRecv - - dot = ir.NewSelectorExpr(dot.Pos(), ir.OXDOT, ir.TypeNode(dot.X.Type()), dot.Selection.Sym) - - n.SetOp(ir.OCALLFUNC) - n.X = typecheck.Expr(dot) - } + rewriteMethodCall(n) args := n.Args params := n.X.Type().Params() @@ -547,6 +534,23 @@ func walkCall1(n *ir.CallExpr, init *ir.Nodes) { n.Args = args } +// rewriteMethodCall rewrites a method call t.M(...) into a function call T.M(t, ...). +func rewriteMethodCall(n *ir.CallExpr) { + if n.Op() != ir.OCALLMETH { + return + } + withRecv := make([]ir.Node, len(n.Args)+1) + dot := n.X.(*ir.SelectorExpr) + withRecv[0] = dot.X + copy(withRecv[1:], n.Args) + n.Args = withRecv + + dot = ir.NewSelectorExpr(dot.Pos(), ir.OXDOT, ir.TypeNode(dot.X.Type()), dot.Selection.Sym) + + n.SetOp(ir.OCALLFUNC) + n.X = typecheck.Expr(dot) +} + // walkDivMod walks an ODIV or OMOD node. func walkDivMod(n *ir.BinaryExpr, init *ir.Nodes) ir.Node { n.X = walkExpr(n.X, init) diff --git a/src/cmd/compile/internal/walk/stmt.go b/src/cmd/compile/internal/walk/stmt.go index 46a621c2ba..86f8819ec3 100644 --- a/src/cmd/compile/internal/walk/stmt.go +++ b/src/cmd/compile/internal/walk/stmt.go @@ -241,6 +241,9 @@ func wrapCall(n *ir.CallExpr, init *ir.Nodes) ir.Node { init.Append(ir.TakeInit(n)...) } + // TODO(mdempsky): Do this right after type checking. + rewriteMethodCall(n) + isBuiltinCall := n.Op() != ir.OCALLFUNC && n.Op() != ir.OCALLMETH && n.Op() != ir.OCALLINTER // Turn f(a, b, []T{c, d, e}...) back into f(a, b, c, d, e). diff --git a/test/fixedbugs/issue24491a.go b/test/fixedbugs/issue24491a.go index 8accf8c0a3..da734531a5 100644 --- a/test/fixedbugs/issue24491a.go +++ b/test/fixedbugs/issue24491a.go @@ -48,6 +48,30 @@ func f() int { return test("return", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup())) } +type S struct{} + +//go:noinline +//go:uintptrescapes +func (S) test(s string, p, q uintptr, rest ...uintptr) int { + runtime.GC() + runtime.GC() + + if *(*string)(unsafe.Pointer(p)) != "ok" { + panic(s + ": p failed") + } + if *(*string)(unsafe.Pointer(q)) != "ok" { + panic(s + ": q failed") + } + for _, r := range rest { + if *(*string)(unsafe.Pointer(r)) != "ok" { + panic(s + ": r[i] failed") + } + } + + done <- true + return 0 +} + func main() { test("normal", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup())) <-done @@ -60,6 +84,12 @@ func main() { }() <-done + func() { + s := &S{} + defer s.test("method call", uintptr(setup()), uintptr(setup())) + }() + <-done + f() <-done } diff --git a/test/fixedbugs/issue44415.go b/test/fixedbugs/issue44415.go new file mode 100644 index 0000000000..26820a9f09 --- /dev/null +++ b/test/fixedbugs/issue44415.go @@ -0,0 +1,21 @@ +// compile + +// 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. + +// +build windows + +package p + +import ( + "syscall" + "unsafe" +) + +var dllKernel = syscall.NewLazyDLL("Kernel32.dll") + +func Call() { + procLocalFree := dllKernel.NewProc("LocalFree") + defer procLocalFree.Call(uintptr(unsafe.Pointer(nil))) +} -- 2.50.0