From c82304b7129d7e675ff010d54f58d039235e4802 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Tue, 6 Sep 2022 12:54:54 +0700 Subject: [PATCH] cmd/compile: do not devirtualize defer/go calls 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 Auto-Submit: Cuong Manh Le Reviewed-by: Matthew Dempsky TryBot-Result: Gopher Robot Run-TryBot: Cuong Manh Le --- .../internal/devirtualize/devirtualize.go | 20 ++++++++++-- test/fixedbugs/issue52072.go | 32 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue52072.go diff --git a/src/cmd/compile/internal/devirtualize/devirtualize.go b/src/cmd/compile/internal/devirtualize/devirtualize.go index b620470b0e..7350a6f171 100644 --- a/src/cmd/compile/internal/devirtualize/devirtualize.go +++ b/src/cmd/compile/internal/devirtualize/devirtualize.go @@ -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 index 0000000000..f372696d34 --- /dev/null +++ b/test/fixedbugs/issue52072.go @@ -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) + } +}