]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: do not devirtualize defer/go calls
authorCuong Manh Le <cuong.manhle.vn@gmail.com>
Tue, 6 Sep 2022 05:54:54 +0000 (12:54 +0700)
committerGopher Robot <gobot@golang.org>
Tue, 6 Sep 2022 22:14:56 +0000 (22:14 +0000)
For defer/go calls, the function/method value are evaluated immediately.
So after devirtualizing, it may trigger a panic when implicitly deref
a nil pointer receiver, causing the program behaves unexpectedly.

It's safer to not devirtualizing defer/go calls at all.

Fixes #52072

Change-Id: I562c2860e3e577b36387dc0a12ae5077bc0766bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/428495
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>

src/cmd/compile/internal/devirtualize/devirtualize.go
test/fixedbugs/issue52072.go [new file with mode: 0644]

index b620470b0e83a61a12ad77cd0b4f1b02e95bd74c..7350a6f1714417df2392bff87eae0408b260b352 100644 (file)
@@ -17,9 +17,25 @@ import (
 // Func devirtualizes calls within fn where possible.
 func Func(fn *ir.Func) {
        ir.CurFunc = fn
+
+       // For promoted methods (including value-receiver methods promoted to pointer-receivers),
+       // the interface method wrapper may contain expressions that can panic (e.g., ODEREF, ODOTPTR, ODOTINTER).
+       // Devirtualization involves inlining these expressions (and possible panics) to the call site.
+       // This normally isn't a problem, but for go/defer statements it can move the panic from when/where
+       // the call executes to the go/defer statement itself, which is a visible change in semantics (e.g., #52072).
+       // To prevent this, we skip devirtualizing calls within go/defer statements altogether.
+       goDeferCall := make(map[*ir.CallExpr]bool)
        ir.VisitList(fn.Body, func(n ir.Node) {
-               if call, ok := n.(*ir.CallExpr); ok {
-                       Call(call)
+               switch n := n.(type) {
+               case *ir.GoDeferStmt:
+                       if call, ok := n.Call.(*ir.CallExpr); ok {
+                               goDeferCall[call] = true
+                       }
+                       return
+               case *ir.CallExpr:
+                       if !goDeferCall[n] {
+                               Call(n)
+                       }
                }
        })
 }
diff --git a/test/fixedbugs/issue52072.go b/test/fixedbugs/issue52072.go
new file mode 100644 (file)
index 0000000..f372696
--- /dev/null
@@ -0,0 +1,32 @@
+// run
+
+// Copyright 2022 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.
+
+package main
+
+type I interface{ M() }
+
+type T struct {
+       x int
+}
+
+func (T) M() {}
+
+var pt *T
+
+func f() (r int) {
+       defer func() { recover() }()
+
+       var i I = pt
+       defer i.M()
+       r = 1
+       return
+}
+
+func main() {
+       if got := f(); got != 1 {
+               panic(got)
+       }
+}