Skip to content

Commit

Permalink
cmd/compile: fix order-of-assignment issue w/ defers
Browse files Browse the repository at this point in the history
CL 261677 fixed a logic issue in walk's alias detection, where it was
checking the RHS expression instead of the LHS expression when trying
to determine the kind of assignment. However, correcting this exposed
a latent issue with assigning to result parameters in functions with
defers, where an assignment could become visible earlier than intended
if a later expression could panic.

Fixes #43835.

Change-Id: I061ced125e3896e26d65f45b28c99db2c8a74a8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/285633
Run-TryBot: Matthew Dempsky <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Trust: Matthew Dempsky <[email protected]>
  • Loading branch information
mdempsky committed Jan 25, 2021
1 parent ad2ca26 commit deaf29a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/cmd/compile/internal/gc/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func walkstmt(n *Node) *Node {
if n.List.Len() == 0 {
break
}
if (Curfn.Type.FuncType().Outnamed && n.List.Len() > 1) || paramoutheap(Curfn) {
if (Curfn.Type.FuncType().Outnamed && n.List.Len() > 1) || paramoutheap(Curfn) || Curfn.Func.HasDefer() {
// assign to the function out parameters,
// so that reorder3 can fix up conflicts
var rl []*Node
Expand Down Expand Up @@ -2233,7 +2233,15 @@ func aliased(r *Node, all []*Node) bool {
memwrite = true
continue

case PAUTO, PPARAM, PPARAMOUT:
case PPARAMOUT:
// Assignments to a result parameter in a function with defers
// becomes visible early if evaluation of any later expression
// panics (#43835).
if Curfn.Func.HasDefer() {
return true
}
fallthrough
case PAUTO, PPARAM:
if l.Name.Addrtaken() {
memwrite = true
continue
Expand Down
33 changes: 33 additions & 0 deletions test/fixedbugs/issue43835.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// run

// 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.

package main

func main() {
if f() {
panic("FAIL")
}
if bad, _ := g(); bad {
panic("FAIL")
}
}

func f() (bad bool) {
defer func() {
recover()
}()
var p *int
bad, _ = true, *p
return
}

func g() (bool, int) {
defer func() {
recover()
}()
var p *int
return true, *p
}

0 comments on commit deaf29a

Please sign in to comment.