cmd/compile: disallow CMOV optimization with ptr arithmetic as an arg

if q != nil {
        p = &q.f
    }

Which gets rewritten to a conditional move:

    tmp := &q.f
    p = Select q!=nil, tmp, p

Unfortunately, we can't compute &q.f before we've checked if q is nil,
because if it is nil, &q.f is an invalid pointer (if f's offset is
nonzero but small).

Normally this is not a problem because the tmp variable above
immediately dies, and is thus not live across any safepoint. However,
if later there is another &q.f computation, those two computations are
CSEd, causing tmp to be used at both use points. That will extend
tmp's lifetime, possibly across a call.

Fixes #56990

Change-Id: I3ea31be93feae04fbe3304cb11323194c5df3879
Reviewed-on: https://go-review.googlesource.com/c/go/+/454155
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>

src/cmd/compile/internal/ssa/branchelim.go

@@ -436,7 +436,7 @@ func canSpeculativelyExecute(b *Block) bool {
 	// don't fuse memory ops, Phi ops, divides (can panic),
 	// or anything else with side-effects
 	for _, v := range b.Values {
-		if v.Op == OpPhi || isDivMod(v.Op) || v.Type.IsMemory() ||
+		if v.Op == OpPhi || isDivMod(v.Op) || isPtrArithmetic(v.Op) || v.Type.IsMemory() ||
 			v.MemoryArg() != nil || opcodeTable[v.Op].hasSideEffects {
 			return false
 		}
@@ -456,3 +456,15 @@ func isDivMod(op Op) bool {
 		return false
 	}
 }
+
+func isPtrArithmetic(op Op) bool {
+	// Pointer arithmetic can't be speculatively executed because the result
+	// may be an invalid pointer (if, for example, the condition is that the
+	// base pointer is not nil). See issue 56990.
+	switch op {
+	case OpOffPtr, OpAddPtr, OpSubPtr:
+		return true
+	default:
+		return false
+	}
+}

test/fixedbugs/issue56990.go

@@ -0,0 +1,119 @@
+// 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
+
+import (
+	"fmt"
+	"path/filepath"
+	"testing"
+)
+
+var t *testing.T
+
+type TypeMeta struct {
+	Kind       string
+	APIVersion string
+}
+
+type ObjectMeta struct {
+	Name         string `json:"name,omitempty" protobuf:"bytes,1,opt,name=name"`
+	GenerateName string `json:"generateName,omitempty" protobuf:"bytes,2,opt,name=generateName"`
+	Namespace    string `json:"namespace,omitempty" protobuf:"bytes,3,opt,name=namespace"`
+	SelfLink     string `json:"selfLink,omitempty" protobuf:"bytes,4,opt,name=selfLink"`
+}
+
+type ConfigSpec struct {
+	Disks        []DiskSpec
+	StorageClass string
+}
+
+type DiskSpec struct {
+	Name         string
+	Size         string
+	StorageClass string
+	Annotations  map[string]string
+	VolumeName   string
+}
+
+// Config is the Schema for the configs API.
+type Config struct {
+	TypeMeta
+	ObjectMeta
+
+	Spec ConfigSpec
+}
+
+func findDiskSize(diskSpec *DiskSpec, configSpec *ConfigSpec) string {
+	t.Log(fmt.Sprintf("Hello World"))
+	return diskSpec.Size
+}
+
+func findStorageClassName(diskSpec *DiskSpec, configSpec *ConfigSpec) *string {
+	if diskSpec.StorageClass != "" {
+		return &diskSpec.StorageClass
+	}
+
+	if configSpec != nil {
+		for _, d := range configSpec.Disks {
+			if d.Name == diskSpec.Name {
+				if d.StorageClass != "" {
+					return &d.StorageClass
+				}
+				break
+			}
+		}
+
+		if configSpec.StorageClass != "" {
+			return &configSpec.StorageClass
+		}
+	}
+	return nil
+}
+
+func Bar(config *Config) *ConfigSpec {
+	var configSpec *ConfigSpec
+	if config != nil {
+		configSpec = &config.Spec
+	}
+	return configSpec
+}
+
+func Foo(diskSpec DiskSpec, config *Config) {
+	cs := Bar(config)
+	_ = findDiskSize(&diskSpec, cs)
+	cs = Bar(config)
+	_ = findStorageClassName(&diskSpec, cs)
+
+}
+
+func TestPanic(tt *testing.T) {
+	t = tt
+	myarray := []string{filepath.Join("..", "config", "crd", "bases")}
+
+	for i := 0; i < 1000; i++ {
+		Foo(DiskSpec{
+			Name: "DataDisk",
+			Size: "1Gi",
+		}, nil)
+	}
+
+	t.Log(myarray)
+}
+
+// Hack to run tests in a playground
+func matchString(a, b string) (bool, error) {
+	return a == b, nil
+}
+func main() {
+	testSuite := []testing.InternalTest{
+		{
+			Name: "TestPanic",
+			F:    TestPanic,
+		},
+	}
+	testing.Main(matchString, testSuite, nil, nil)
+}

test/fixedbugs/issue56990.out

@@ -0,0 +1 @@
+PASS