Skip to content

Commit 962ccbe

Browse files
randall77randall77
committed
cmd/compile: ensure pointer arithmetic happens after the nil check
Have nil checks return a pointer that is known non-nil. Users of that pointer can use the result, ensuring that they are ordered after the nil check itself. The order dependence goes away after scheduling, when we've fixed an order. At that point we move uses back to the original pointer so it doesn't change regalloc any. This prevents pointer arithmetic on nil from being spilled to the stack and then observed by a stack scan. Fixes #63657 Change-Id: I1a5fa4f2e6d9000d672792b4f90dfc1b7b67f6ea Reviewed-on: https://go-review.googlesource.com/c/go/+/537775 Reviewed-by: David Chase <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 43b57b8 commit 962ccbe

15 files changed

Lines changed: 179 additions & 81 deletions

File tree

‎src/cmd/compile/internal/ssa/_gen/generic.rules‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@
981981
(ConstNil <typ.Uintptr>)
982982
(ConstNil <typ.BytePtr>))
983983

984-
(NilCheck (GetG mem) mem) => mem
984+
(NilCheck ptr:(GetG mem) mem) => ptr
985985

986986
(If (Not cond) yes no) => (If cond no yes)
987987
(If (ConstBool [c]) yes no) && c => (First yes no)
@@ -2055,19 +2055,19 @@
20552055
&& isSameCall(call.Aux, "runtime.newobject")
20562056
=> mem
20572057

2058-
(NilCheck (SelectN [0] call:(StaticLECall _ _)) _)
2058+
(NilCheck ptr:(SelectN [0] call:(StaticLECall _ _)) _)
20592059
&& isSameCall(call.Aux, "runtime.newobject")
20602060
&& warnRule(fe.Debug_checknil(), v, "removed nil check")
2061-
=> (Invalid)
2061+
=> ptr
20622062

2063-
(NilCheck (OffPtr (SelectN [0] call:(StaticLECall _ _))) _)
2063+
(NilCheck ptr:(OffPtr (SelectN [0] call:(StaticLECall _ _))) _)
20642064
&& isSameCall(call.Aux, "runtime.newobject")
20652065
&& warnRule(fe.Debug_checknil(), v, "removed nil check")
2066-
=> (Invalid)
2066+
=> ptr
20672067

20682068
// Addresses of globals are always non-nil.
2069-
(NilCheck (Addr {_} (SB)) _) => (Invalid)
2070-
(NilCheck (Convert (Addr {_} (SB)) _) _) => (Invalid)
2069+
(NilCheck ptr:(Addr {_} (SB)) _) => ptr
2070+
(NilCheck ptr:(Convert (Addr {_} (SB)) _) _) => ptr
20712071

20722072
// for late-expanded calls, recognize memequal applied to a single constant byte
20732073
// Support is limited by 1, 2, 4, 8 byte sizes

‎src/cmd/compile/internal/ssa/_gen/genericOps.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ var genericOps = []opData{
477477
{name: "IsNonNil", argLength: 1, typ: "Bool"}, // arg0 != nil
478478
{name: "IsInBounds", argLength: 2, typ: "Bool"}, // 0 <= arg0 < arg1. arg1 is guaranteed >= 0.
479479
{name: "IsSliceInBounds", argLength: 2, typ: "Bool"}, // 0 <= arg0 <= arg1. arg1 is guaranteed >= 0.
480-
{name: "NilCheck", argLength: 2, typ: "Void"}, // arg0=ptr, arg1=mem. Panics if arg0 is nil. Returns void.
480+
{name: "NilCheck", argLength: 2, nilCheck: true}, // arg0=ptr, arg1=mem. Panics if arg0 is nil. Returns the ptr unmodified.
481481

482482
// Pseudo-ops
483483
{name: "GetG", argLength: 1, zeroWidth: true}, // runtime.getg() (read g pointer). arg0=mem

‎src/cmd/compile/internal/ssa/check.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,28 @@ func checkFunc(f *Func) {
317317
if !v.Aux.(*ir.Name).Type().HasPointers() {
318318
f.Fatalf("vardef must have pointer type %s", v.Aux.(*ir.Name).Type().String())
319319
}
320-
320+
case OpNilCheck:
321+
// nil checks have pointer type before scheduling, and
322+
// void type after scheduling.
323+
if f.scheduled {
324+
if v.Uses != 0 {
325+
f.Fatalf("nilcheck must have 0 uses %s", v.Uses)
326+
}
327+
if !v.Type.IsVoid() {
328+
f.Fatalf("nilcheck must have void type %s", v.Type.String())
329+
}
330+
} else {
331+
if !v.Type.IsPtrShaped() && !v.Type.IsUintptr() {
332+
f.Fatalf("nilcheck must have pointer type %s", v.Type.String())
333+
}
334+
}
335+
if !v.Args[0].Type.IsPtrShaped() && !v.Args[0].Type.IsUintptr() {
336+
f.Fatalf("nilcheck must have argument of pointer type %s", v.Args[0].Type.String())
337+
}
338+
if !v.Args[1].Type.IsMemory() {
339+
f.Fatalf("bad arg 1 type to %s: want mem, have %s",
340+
v.Op, v.Args[1].Type.String())
341+
}
321342
}
322343

323344
// TODO: check for cycles in values

‎src/cmd/compile/internal/ssa/deadcode.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,15 @@ func liveValues(f *Func, reachable []bool) (live []bool, liveOrderStmts []*Value
110110
}
111111
}
112112
for _, v := range b.Values {
113-
if (opcodeTable[v.Op].call || opcodeTable[v.Op].hasSideEffects) && !live[v.ID] {
113+
if (opcodeTable[v.Op].call || opcodeTable[v.Op].hasSideEffects || opcodeTable[v.Op].nilCheck) && !live[v.ID] {
114114
live[v.ID] = true
115115
q = append(q, v)
116116
if v.Pos.IsStmt() != src.PosNotStmt {
117117
liveOrderStmts = append(liveOrderStmts, v)
118118
}
119119
}
120-
if v.Type.IsVoid() && !live[v.ID] {
121-
// The only Void ops are nil checks and inline marks. We must keep these.
122-
if v.Op == OpInlMark && !liveInlIdx[int(v.AuxInt)] {
120+
if v.Op == OpInlMark {
121+
if !liveInlIdx[int(v.AuxInt)] {
123122
// We don't need marks for bodies that
124123
// have been completely optimized away.
125124
// TODO: save marks only for bodies which

‎src/cmd/compile/internal/ssa/deadstore.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func elimDeadAutosGeneric(f *Func) {
249249
}
250250

251251
if v.Uses == 0 && v.Op != OpNilCheck && !v.Op.IsCall() && !v.Op.HasSideEffects() || len(args) == 0 {
252-
// Nil check has no use, but we need to keep it.
252+
// We need to keep nil checks even if they have no use.
253253
// Also keep calls and values that have side effects.
254254
return
255255
}

‎src/cmd/compile/internal/ssa/fuse.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func fuseBlockIf(b *Block) bool {
169169
// There may be false positives.
170170
func isEmpty(b *Block) bool {
171171
for _, v := range b.Values {
172-
if v.Uses > 0 || v.Op.IsCall() || v.Op.HasSideEffects() || v.Type.IsVoid() {
172+
if v.Uses > 0 || v.Op.IsCall() || v.Op.HasSideEffects() || v.Type.IsVoid() || opcodeTable[v.Op].nilCheck {
173173
return false
174174
}
175175
}

‎src/cmd/compile/internal/ssa/fuse_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func TestFuseSideEffects(t *testing.T) {
254254
Valu("p", OpArg, c.config.Types.IntPtr, 0, nil),
255255
If("c1", "z0", "exit")),
256256
Bloc("z0",
257-
Valu("nilcheck", OpNilCheck, types.TypeVoid, 0, nil, "p", "mem"),
257+
Valu("nilcheck", OpNilCheck, c.config.Types.IntPtr, 0, nil, "p", "mem"),
258258
Goto("exit")),
259259
Bloc("exit",
260260
Exit("mem"),

‎src/cmd/compile/internal/ssa/nilcheck.go‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,14 @@ func nilcheckelim(f *Func) {
3838
work := make([]bp, 0, 256)
3939
work = append(work, bp{block: f.Entry})
4040

41-
// map from value ID to bool indicating if value is known to be non-nil
42-
// in the current dominator path being walked. This slice is updated by
41+
// map from value ID to known non-nil version of that value ID
42+
// (in the current dominator path being walked). This slice is updated by
4343
// walkStates to maintain the known non-nil values.
44-
nonNilValues := f.Cache.allocBoolSlice(f.NumValues())
45-
defer f.Cache.freeBoolSlice(nonNilValues)
44+
// If there is extrinsic information about non-nil-ness, this map
45+
// points a value to itself. If a value is known non-nil because we
46+
// already did a nil check on it, it points to the nil check operation.
47+
nonNilValues := f.Cache.allocValueSlice(f.NumValues())
48+
defer f.Cache.freeValueSlice(nonNilValues)
4649

4750
// make an initial pass identifying any non-nil values
4851
for _, b := range f.Blocks {
@@ -54,7 +57,7 @@ func nilcheckelim(f *Func) {
5457
// We assume that SlicePtr is non-nil because we do a bounds check
5558
// before the slice access (and all cap>0 slices have a non-nil ptr). See #30366.
5659
if v.Op == OpAddr || v.Op == OpLocalAddr || v.Op == OpAddPtr || v.Op == OpOffPtr || v.Op == OpAdd32 || v.Op == OpAdd64 || v.Op == OpSub32 || v.Op == OpSub64 || v.Op == OpSlicePtr {
57-
nonNilValues[v.ID] = true
60+
nonNilValues[v.ID] = v
5861
}
5962
}
6063
}
@@ -68,16 +71,16 @@ func nilcheckelim(f *Func) {
6871
if v.Op == OpPhi {
6972
argsNonNil := true
7073
for _, a := range v.Args {
71-
if !nonNilValues[a.ID] {
74+
if nonNilValues[a.ID] == nil {
7275
argsNonNil = false
7376
break
7477
}
7578
}
7679
if argsNonNil {
77-
if !nonNilValues[v.ID] {
80+
if nonNilValues[v.ID] == nil {
7881
changed = true
7982
}
80-
nonNilValues[v.ID] = true
83+
nonNilValues[v.ID] = v
8184
}
8285
}
8386
}
@@ -103,8 +106,8 @@ func nilcheckelim(f *Func) {
103106
if len(b.Preds) == 1 {
104107
p := b.Preds[0].b
105108
if p.Kind == BlockIf && p.Controls[0].Op == OpIsNonNil && p.Succs[0].b == b {
106-
if ptr := p.Controls[0].Args[0]; !nonNilValues[ptr.ID] {
107-
nonNilValues[ptr.ID] = true
109+
if ptr := p.Controls[0].Args[0]; nonNilValues[ptr.ID] == nil {
110+
nonNilValues[ptr.ID] = ptr
108111
work = append(work, bp{op: ClearPtr, ptr: ptr})
109112
}
110113
}
@@ -117,14 +120,11 @@ func nilcheckelim(f *Func) {
117120
pendingLines.clear()
118121

119122
// Next, process values in the block.
120-
i := 0
121123
for _, v := range b.Values {
122-
b.Values[i] = v
123-
i++
124124
switch v.Op {
125125
case OpIsNonNil:
126126
ptr := v.Args[0]
127-
if nonNilValues[ptr.ID] {
127+
if nonNilValues[ptr.ID] != nil {
128128
if v.Pos.IsStmt() == src.PosIsStmt { // Boolean true is a terrible statement boundary.
129129
pendingLines.add(v.Pos)
130130
v.Pos = v.Pos.WithNotStmt()
@@ -135,7 +135,7 @@ func nilcheckelim(f *Func) {
135135
}
136136
case OpNilCheck:
137137
ptr := v.Args[0]
138-
if nonNilValues[ptr.ID] {
138+
if nilCheck := nonNilValues[ptr.ID]; nilCheck != nil {
139139
// This is a redundant implicit nil check.
140140
// Logging in the style of the former compiler -- and omit line 1,
141141
// which is usually in generated code.
@@ -145,14 +145,13 @@ func nilcheckelim(f *Func) {
145145
if v.Pos.IsStmt() == src.PosIsStmt { // About to lose a statement boundary
146146
pendingLines.add(v.Pos)
147147
}
148-
v.reset(OpUnknown)
149-
f.freeValue(v)
150-
i--
148+
v.Op = OpCopy
149+
v.SetArgs1(nilCheck)
151150
continue
152151
}
153152
// Record the fact that we know ptr is non nil, and remember to
154153
// undo that information when this dominator subtree is done.
155-
nonNilValues[ptr.ID] = true
154+
nonNilValues[ptr.ID] = v
156155
work = append(work, bp{op: ClearPtr, ptr: ptr})
157156
fallthrough // a non-eliminated nil check might be a good place for a statement boundary.
158157
default:
@@ -163,7 +162,7 @@ func nilcheckelim(f *Func) {
163162
}
164163
}
165164
// This reduces the lost statement count in "go" by 5 (out of 500 total).
166-
for j := 0; j < i; j++ { // is this an ordering problem?
165+
for j := range b.Values { // is this an ordering problem?
167166
v := b.Values[j]
168167
if v.Pos.IsStmt() != src.PosNotStmt && !isPoorStatementOp(v.Op) && pendingLines.contains(v.Pos) {
169168
v.Pos = v.Pos.WithIsStmt()
@@ -174,15 +173,14 @@ func nilcheckelim(f *Func) {
174173
b.Pos = b.Pos.WithIsStmt()
175174
pendingLines.remove(b.Pos)
176175
}
177-
b.truncateValues(i)
178176

179177
// Add all dominated blocks to the work list.
180178
for w := sdom[node.block.ID].child; w != nil; w = sdom[w.ID].sibling {
181179
work = append(work, bp{op: Work, block: w})
182180
}
183181

184182
case ClearPtr:
185-
nonNilValues[node.ptr.ID] = false
183+
nonNilValues[node.ptr.ID] = nil
186184
continue
187185
}
188186
}

‎src/cmd/compile/internal/ssa/opGen.go‎

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/cmd/compile/internal/ssa/rewrite.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,9 @@ func disjoint(p1 *Value, n1 int64, p2 *Value, n2 int64) bool {
859859
offset += base.AuxInt
860860
base = base.Args[0]
861861
}
862+
if opcodeTable[base.Op].nilCheck {
863+
base = base.Args[0]
864+
}
862865
return base, offset
863866
}
864867
p1, off1 := baseAndOffset(p1)

0 commit comments

Comments
 (0)