summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKeith Randall <khr@golang.org>2023-03-28 10:19:21 -0700
committerMatthew Dempsky <mdempsky@google.com>2023-03-29 19:07:23 +0000
commit8dce4ca8dfd4892e2f5bdf0a1224b9efe0dca89f (patch)
tree88fe27bf38433c05e290d3cfbdbd370aa0086973
parent94c02a3cc481e4b73ce275ba6e50c286d4c492ff (diff)
downloadgo-git-8dce4ca8dfd4892e2f5bdf0a1224b9efe0dca89f.tar.gz
[release-branch.go1.20] cmd/compile: don't assume pointer of a slice is non-nil
unsafe.SliceData can return pointers which are nil. That function gets lowered to the SSA OpSlicePtr, which the compiler assumes is non-nil. This used to be the case as OpSlicePtr was only used in situations where the bounds check already passed. But with unsafe.SliceData that is no longer the case. There are situations where we know it is nil. Use Bounded() to indicate that. I looked through all the uses of OSPTR and added SetBounded where it made sense. Most OSPTR results are passed directly to runtime calls (e.g. memmove), so even if we know they are non-nil that info isn't helpful. Fixes #59296 Change-Id: I437a15330db48e0082acfb1f89caf8c56723fc51 Reviewed-on: https://go-review.googlesource.com/c/go/+/479896 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Keith Randall <khr@golang.org> (cherry picked from commit b899641ecea7d07c997282e985beb295c31d1097) Reviewed-on: https://go-review.googlesource.com/c/go/+/479899 Run-TryBot: Keith Randall <khr@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
-rw-r--r--src/cmd/compile/internal/ir/node.go2
-rw-r--r--src/cmd/compile/internal/ssagen/ssa.go5
-rw-r--r--src/cmd/compile/internal/walk/convert.go4
-rw-r--r--src/cmd/compile/internal/walk/range.go1
-rw-r--r--test/fixedbugs/issue59293.go28
5 files changed, 37 insertions, 3 deletions
diff --git a/src/cmd/compile/internal/ir/node.go b/src/cmd/compile/internal/ir/node.go
index b42f914aad..547dd48a14 100644
--- a/src/cmd/compile/internal/ir/node.go
+++ b/src/cmd/compile/internal/ir/node.go
@@ -291,7 +291,7 @@ const (
OEFACE // itable and data words of an empty-interface value.
OITAB // itable word of an interface value.
OIDATA // data word of an interface value in X
- OSPTR // base pointer of a slice or string.
+ OSPTR // base pointer of a slice or string. Bounded==1 means known non-nil.
OCFUNC // reference to c function pointer (not go func value)
OCHECKNIL // emit code to ensure pointer/interface not nil
ORESULT // result of a function call; Xoffset is stack offset
diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index 52f94030df..526332294c 100644
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -3203,7 +3203,10 @@ func (s *state) exprCheckPtr(n ir.Node, checkPtrOK bool) *ssa.Value {
n := n.(*ir.UnaryExpr)
a := s.expr(n.X)
if n.X.Type().IsSlice() {
- return s.newValue1(ssa.OpSlicePtr, n.Type(), a)
+ if n.Bounded() {
+ return s.newValue1(ssa.OpSlicePtr, n.Type(), a)
+ }
+ return s.newValue1(ssa.OpSlicePtrUnchecked, n.Type(), a)
} else {
return s.newValue1(ssa.OpStringPtr, n.Type(), a)
}
diff --git a/src/cmd/compile/internal/walk/convert.go b/src/cmd/compile/internal/walk/convert.go
index bf06ed6f46..ca0c806a1e 100644
--- a/src/cmd/compile/internal/walk/convert.go
+++ b/src/cmd/compile/internal/walk/convert.go
@@ -281,7 +281,9 @@ func walkStringToBytes(n *ir.ConvExpr, init *ir.Nodes) ir.Node {
// Copy from the static string data to the [n]byte.
if len(sc) > 0 {
- as := ir.NewAssignStmt(base.Pos, ir.NewStarExpr(base.Pos, p), ir.NewStarExpr(base.Pos, typecheck.ConvNop(ir.NewUnaryExpr(base.Pos, ir.OSPTR, s), t.PtrTo())))
+ sptr := ir.NewUnaryExpr(base.Pos, ir.OSPTR, s)
+ sptr.SetBounded(true)
+ as := ir.NewAssignStmt(base.Pos, ir.NewStarExpr(base.Pos, p), ir.NewStarExpr(base.Pos, typecheck.ConvNop(sptr, t.PtrTo())))
appendWalkStmt(init, as)
}
diff --git a/src/cmd/compile/internal/walk/range.go b/src/cmd/compile/internal/walk/range.go
index 64af26bf29..82e7d22cfa 100644
--- a/src/cmd/compile/internal/walk/range.go
+++ b/src/cmd/compile/internal/walk/range.go
@@ -191,6 +191,7 @@ func walkRange(nrange *ir.RangeStmt) ir.Node {
// Pointer to current iteration position. Start on entry to the loop
// with the pointer in hu.
ptr := ir.NewUnaryExpr(base.Pos, ir.OSPTR, hs)
+ ptr.SetBounded(true)
huVal := ir.NewConvExpr(base.Pos, ir.OCONVNOP, types.Types[types.TUNSAFEPTR], ptr)
huVal = ir.NewConvExpr(base.Pos, ir.OCONVNOP, types.Types[types.TUINTPTR], huVal)
hu := typecheck.Temp(types.Types[types.TUINTPTR])
diff --git a/test/fixedbugs/issue59293.go b/test/fixedbugs/issue59293.go
new file mode 100644
index 0000000000..1f05fe9a7a
--- /dev/null
+++ b/test/fixedbugs/issue59293.go
@@ -0,0 +1,28 @@
+// run
+
+// Copyright 2023 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 "unsafe"
+
+//go:noinline
+func f(x []byte) bool {
+ return unsafe.SliceData(x) != nil
+}
+
+//go:noinline
+func g(x string) bool {
+ return unsafe.StringData(x) != nil
+}
+
+func main() {
+ if f(nil) {
+ panic("bad f")
+ }
+ if g("") {
+ panic("bad g")
+ }
+}