From a9ef9c7830f4d6efacb59aa1e6ae326c8ea72e8e Mon Sep 17 00:00:00 2001
From: Dominik Honnef <dominik@honnef.co>
Date: Tue, 21 May 2024 02:54:39 +0200
Subject: [PATCH] go/ir: don't panic trying to replace phi in unreachable exit
 node

When the exit node is unreachable, for example because of a function
that ends with an infinite loop, and we lift an alloc for a named return
value, we may end up creating a phi node with no edges. Don't do that.

Closes: gh-1533
---
 go/ir/builder_test.go | 36 ++++++++++++++++++++++++++++++++++++
 go/ir/lift.go         | 16 ++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/go/ir/builder_test.go b/go/ir/builder_test.go
index 62820852..2a585fa2 100644
--- a/go/ir/builder_test.go
+++ b/go/ir/builder_test.go
@@ -633,3 +633,39 @@ func TestLabels(t *testing.T) {
 		pkg.Build()
 	}
 }
+
+func TestUnreachableExit(t *testing.T) {
+	tests := []string{
+		`
+		package pkg
+
+		func foo() (err error) {
+			if true {
+				println()
+			}
+
+			println()
+
+			for {
+				err = nil
+			}
+		}`,
+	}
+	for _, test := range tests {
+		conf := loader.Config{Fset: token.NewFileSet()}
+		f, err := parser.ParseFile(conf.Fset, "<input>", test, 0)
+		if err != nil {
+			t.Errorf("parse error: %s", err)
+			return
+		}
+		conf.CreateFromFiles("pkg", f)
+		iprog, err := conf.Load()
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		prog := irutil.CreateProgram(iprog, ir.BuilderMode(0))
+		pkg := prog.Package(iprog.Created[0].Pkg)
+		pkg.Build()
+	}
+}
diff --git a/go/ir/lift.go b/go/ir/lift.go
index a207c23a..08be0d37 100644
--- a/go/ir/lift.go
+++ b/go/ir/lift.go
@@ -1243,6 +1243,22 @@ func liftAlloc(closure *closure, df domFrontier, rdf postDomFrontier, alloc *All
 
 						// Create φ-node.
 						// It will be prepended to v.Instrs later, if needed.
+						if len(y.Preds) == 0 {
+							// The exit block may be unreachable if the function doesn't
+							// return, e.g. due to an infinite loop. In that case we
+							// should not replace loads in the exit block with ϕ node that
+							// have no edges. Such loads exist when the function has named
+							// return parameters, as the exit block loads them to turn
+							// them into a Return instruction. By not replacing the loads
+							// with ϕ nodes, they will later be replaced by zero
+							// constants. This is arguably more correct, and more
+							// importantly, it doesn't break code that assumes that phis
+							// have at least one edge.
+							//
+							// For one instance of breakage see
+							// https://staticcheck.dev/issues/1533
+							continue
+						}
 						phi := &Phi{
 							Edges: make([]Value, len(y.Preds)),
 						}