Skip to content

Commit d9a6bdf

Browse files
committed
cmd/compile: don't allow go:notinheap on the heap or stack
Right now we just prevent such types from being on the heap. This CL makes it so they cannot appear on the stack either. The distinction between heap and stack is pretty vague at the language level (e.g. it is affected by -N), and we don't need the flexibility anyway. Once go:notinheap types cannot be in either place, we don't need to consider pointers to such types to be pointers, at least according to the garbage collector and stack copying. (This is the big win of this CL, in my opinion.) The distinction between HasPointers and HasHeapPointer no longer exists. There is only HasPointers. This CL is cleanup before possible use of go:notinheap to fix golang#40954. Update golang#13386 Change-Id: Ibd895aadf001c0385078a6d4809c3f374991231a Reviewed-on: https://go-review.googlesource.com/c/go/+/249917 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]> Reviewed-by: Cuong Manh Le <[email protected]>
1 parent 95df156 commit d9a6bdf

File tree

16 files changed

+54
-68
lines changed

16 files changed

+54
-68
lines changed

src/cmd/compile/internal/gc/escape.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,9 @@ func (e *Escape) newLoc(n *Node, transient bool) *EscLocation {
10291029
if e.curfn == nil {
10301030
Fatalf("e.curfn isn't set")
10311031
}
1032+
if n != nil && n.Type != nil && n.Type.NotInHeap() {
1033+
yyerrorl(n.Pos, "%v is go:notinheap; stack allocation disallowed", n.Type)
1034+
}
10321035

10331036
n = canonicalNode(n)
10341037
loc := &EscLocation{

src/cmd/compile/internal/gc/pgen_test.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func typeWithoutPointers() *types.Type {
2020

2121
func typeWithPointers() *types.Type {
2222
t := types.New(TSTRUCT)
23-
f := &types.Field{Type: types.New(TPTR)}
23+
f := &types.Field{Type: types.NewPtr(types.New(TINT))}
2424
t.SetFields([]*types.Field{f})
2525
return t
2626
}
@@ -181,14 +181,6 @@ func TestStackvarSort(t *testing.T) {
181181
nodeWithClass(Node{Type: &types.Type{}, Sym: &types.Sym{Name: "xyz"}}, PAUTO),
182182
nodeWithClass(Node{Type: typeWithoutPointers(), Sym: &types.Sym{}}, PAUTO),
183183
}
184-
// haspointers updates Type.Haspointers as a side effect, so
185-
// exercise this function on all inputs so that reflect.DeepEqual
186-
// doesn't produce false positives.
187-
for i := range want {
188-
want[i].Type.HasPointers()
189-
inp[i].Type.HasPointers()
190-
}
191-
192184
sort.Sort(byStackVar(inp))
193185
if !reflect.DeepEqual(want, inp) {
194186
t.Error("sort failed")

src/cmd/compile/internal/gc/plive.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ func (lv *Liveness) regEffects(v *ssa.Value) (uevar, kill liveRegMask) {
436436
case ssa.LocalSlot:
437437
return mask
438438
case *ssa.Register:
439-
if ptrOnly && !v.Type.HasHeapPointer() {
439+
if ptrOnly && !v.Type.HasPointers() {
440440
return mask
441441
}
442442
regs[0] = loc
@@ -451,7 +451,7 @@ func (lv *Liveness) regEffects(v *ssa.Value) (uevar, kill liveRegMask) {
451451
if loc1 == nil {
452452
continue
453453
}
454-
if ptrOnly && !v.Type.FieldType(i).HasHeapPointer() {
454+
if ptrOnly && !v.Type.FieldType(i).HasPointers() {
455455
continue
456456
}
457457
regs[nreg] = loc1.(*ssa.Register)
@@ -568,13 +568,13 @@ func onebitwalktype1(t *types.Type, off int64, bv bvec) {
568568
if t.Align > 0 && off&int64(t.Align-1) != 0 {
569569
Fatalf("onebitwalktype1: invalid initial alignment: type %v has alignment %d, but offset is %v", t, t.Align, off)
570570
}
571+
if !t.HasPointers() {
572+
// Note: this case ensures that pointers to go:notinheap types
573+
// are not considered pointers by garbage collection and stack copying.
574+
return
575+
}
571576

572577
switch t.Etype {
573-
case TINT8, TUINT8, TINT16, TUINT16,
574-
TINT32, TUINT32, TINT64, TUINT64,
575-
TINT, TUINT, TUINTPTR, TBOOL,
576-
TFLOAT32, TFLOAT64, TCOMPLEX64, TCOMPLEX128:
577-
578578
case TPTR, TUNSAFEPTR, TFUNC, TCHAN, TMAP:
579579
if off&int64(Widthptr-1) != 0 {
580580
Fatalf("onebitwalktype1: invalid alignment, %v", t)

src/cmd/compile/internal/gc/range.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ func arrayClear(n, v1, v2, a *Node) bool {
586586
n.Nbody.Append(nod(OAS, hn, tmp))
587587

588588
var fn *Node
589-
if a.Type.Elem().HasHeapPointer() {
589+
if a.Type.Elem().HasPointers() {
590590
// memclrHasPointers(hp, hn)
591591
Curfn.Func.setWBPos(stmt.Pos)
592592
fn = mkcall("memclrHasPointers", nil, nil, hp, hn)

src/cmd/compile/internal/gc/walk.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,9 @@ opswitch:
11561156
}
11571157

11581158
case ONEW:
1159+
if n.Type.Elem().NotInHeap() {
1160+
yyerror("%v is go:notinheap; heap allocation disallowed", n.Type.Elem())
1161+
}
11591162
if n.Esc == EscNone {
11601163
if n.Type.Elem().Width >= maxImplicitStackVarSize {
11611164
Fatalf("large ONEW with EscNone: %v", n)
@@ -1324,6 +1327,9 @@ opswitch:
13241327
l = r
13251328
}
13261329
t := n.Type
1330+
if t.Elem().NotInHeap() {
1331+
yyerror("%v is go:notinheap; heap allocation disallowed", t.Elem())
1332+
}
13271333
if n.Esc == EscNone {
13281334
if !isSmallMakeSlice(n) {
13291335
Fatalf("non-small OMAKESLICE with EscNone: %v", n)
@@ -1365,10 +1371,6 @@ opswitch:
13651371
// When len and cap can fit into int, use makeslice instead of
13661372
// makeslice64, which is faster and shorter on 32 bit platforms.
13671373

1368-
if t.Elem().NotInHeap() {
1369-
yyerror("%v is go:notinheap; heap allocation disallowed", t.Elem())
1370-
}
1371-
13721374
len, cap := l, r
13731375

13741376
fnname := "makeslice64"
@@ -1403,7 +1405,7 @@ opswitch:
14031405

14041406
t := n.Type
14051407
if t.Elem().NotInHeap() {
1406-
Fatalf("%v is go:notinheap; heap allocation disallowed", t.Elem())
1408+
yyerror("%v is go:notinheap; heap allocation disallowed", t.Elem())
14071409
}
14081410

14091411
length := conv(n.Left, types.Types[TINT])
@@ -2012,9 +2014,6 @@ func walkprint(nn *Node, init *Nodes) *Node {
20122014
}
20132015

20142016
func callnew(t *types.Type) *Node {
2015-
if t.NotInHeap() {
2016-
yyerror("%v is go:notinheap; heap allocation disallowed", t)
2017-
}
20182017
dowidth(t)
20192018
n := nod(ONEWOBJ, typename(t), nil)
20202019
n.Type = types.NewPtr(t)
@@ -2589,15 +2588,15 @@ func mapfast(t *types.Type) int {
25892588
}
25902589
switch algtype(t.Key()) {
25912590
case AMEM32:
2592-
if !t.Key().HasHeapPointer() {
2591+
if !t.Key().HasPointers() {
25932592
return mapfast32
25942593
}
25952594
if Widthptr == 4 {
25962595
return mapfast32ptr
25972596
}
25982597
Fatalf("small pointer %v", t.Key())
25992598
case AMEM64:
2600-
if !t.Key().HasHeapPointer() {
2599+
if !t.Key().HasPointers() {
26012600
return mapfast64
26022601
}
26032602
if Widthptr == 8 {
@@ -2744,7 +2743,7 @@ func appendslice(n *Node, init *Nodes) *Node {
27442743
nodes.Append(nod(OAS, s, nt))
27452744

27462745
var ncopy *Node
2747-
if elemtype.HasHeapPointer() {
2746+
if elemtype.HasPointers() {
27482747
// copy(s[len(l1):], l2)
27492748
nptr1 := nod(OSLICE, s, nil)
27502749
nptr1.Type = s.Type
@@ -3082,7 +3081,7 @@ func walkappend(n *Node, init *Nodes, dst *Node) *Node {
30823081
// Also works if b is a string.
30833082
//
30843083
func copyany(n *Node, init *Nodes, runtimecall bool) *Node {
3085-
if n.Left.Type.Elem().HasHeapPointer() {
3084+
if n.Left.Type.Elem().HasPointers() {
30863085
Curfn.Func.setWBPos(n.Pos)
30873086
fn := writebarrierfn("typedslicecopy", n.Left.Type.Elem(), n.Right.Type.Elem())
30883087
n.Left = cheapexpr(n.Left, init)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func decomposeStringPhi(v *Value) {
139139

140140
func decomposeSlicePhi(v *Value) {
141141
types := &v.Block.Func.Config.Types
142-
ptrType := types.BytePtr
142+
ptrType := v.Type.Elem().PtrTo()
143143
lenType := types.Int
144144

145145
ptr := v.Block.NewValue0(v.Pos, OpPhi, ptrType)

src/cmd/compile/internal/ssa/gen/dec.rules

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,14 @@
6666
(Load <typ.Int>
6767
(OffPtr <typ.IntPtr> [2*config.PtrSize] ptr)
6868
mem))
69-
(Store dst (SliceMake ptr len cap) mem) =>
69+
(Store {t} dst (SliceMake ptr len cap) mem) =>
7070
(Store {typ.Int}
7171
(OffPtr <typ.IntPtr> [2*config.PtrSize] dst)
7272
cap
7373
(Store {typ.Int}
7474
(OffPtr <typ.IntPtr> [config.PtrSize] dst)
7575
len
76-
(Store {typ.BytePtr} dst ptr mem)))
76+
(Store {t.Elem().PtrTo()} dst ptr mem)))
7777

7878
// interface ops
7979
(ITab (IMake itab _)) => itab

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func nilcheckelim2(f *Func) {
235235
continue
236236
}
237237
if v.Type.IsMemory() || v.Type.IsTuple() && v.Type.FieldType(1).IsMemory() {
238-
if v.Op == OpVarKill || v.Op == OpVarLive || (v.Op == OpVarDef && !v.Aux.(GCNode).Typ().HasHeapPointer()) {
238+
if v.Op == OpVarKill || v.Op == OpVarLive || (v.Op == OpVarDef && !v.Aux.(GCNode).Typ().HasPointers()) {
239239
// These ops don't really change memory.
240240
continue
241241
// Note: OpVarDef requires that the defined variable not have pointers.

src/cmd/compile/internal/ssa/rewritedec.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/writebarrier.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func needwb(v *Value, zeroes map[ID]ZeroRegion) bool {
3131
if !ok {
3232
v.Fatalf("store aux is not a type: %s", v.LongString())
3333
}
34-
if !t.HasHeapPointer() {
34+
if !t.HasPointers() {
3535
return false
3636
}
3737
if IsStackAddr(v.Args[0]) {

0 commit comments

Comments
 (0)