Skip to content

Commit 11f7ea8

Browse files
andreybokhankogopherbot
authored andcommitted
cmd/compile: add type-based alias analysis
Make ssa.disjoint call ssa.disjointTypes to disambiguate Values based on their types. Only one type-based rule is employed: a Type can't alias with a pointer (https://pkg.go.dev/unsafe#Pointer). Fixes #70488 Change-Id: I5a7e75292c2b6b5a01fb9048e3e2360e31dbcdd9 Reviewed-on: https://go-review.googlesource.com/c/go/+/632176 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]> Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 2299a42 commit 11f7ea8

File tree

3 files changed

+144
-22
lines changed

3 files changed

+144
-22
lines changed

src/cmd/compile/internal/rttype/rttype.go

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,26 +50,26 @@ func Init() {
5050
// Note: this has to be called explicitly instead of being
5151
// an init function so it runs after the types package has
5252
// been properly initialized.
53-
Type = fromReflect(reflect.TypeOf(abi.Type{}))
54-
ArrayType = fromReflect(reflect.TypeOf(abi.ArrayType{}))
55-
ChanType = fromReflect(reflect.TypeOf(abi.ChanType{}))
56-
FuncType = fromReflect(reflect.TypeOf(abi.FuncType{}))
57-
InterfaceType = fromReflect(reflect.TypeOf(abi.InterfaceType{}))
58-
OldMapType = fromReflect(reflect.TypeOf(abi.OldMapType{}))
59-
SwissMapType = fromReflect(reflect.TypeOf(abi.SwissMapType{}))
60-
PtrType = fromReflect(reflect.TypeOf(abi.PtrType{}))
61-
SliceType = fromReflect(reflect.TypeOf(abi.SliceType{}))
62-
StructType = fromReflect(reflect.TypeOf(abi.StructType{}))
53+
Type = FromReflect(reflect.TypeOf(abi.Type{}))
54+
ArrayType = FromReflect(reflect.TypeOf(abi.ArrayType{}))
55+
ChanType = FromReflect(reflect.TypeOf(abi.ChanType{}))
56+
FuncType = FromReflect(reflect.TypeOf(abi.FuncType{}))
57+
InterfaceType = FromReflect(reflect.TypeOf(abi.InterfaceType{}))
58+
OldMapType = FromReflect(reflect.TypeOf(abi.OldMapType{}))
59+
SwissMapType = FromReflect(reflect.TypeOf(abi.SwissMapType{}))
60+
PtrType = FromReflect(reflect.TypeOf(abi.PtrType{}))
61+
SliceType = FromReflect(reflect.TypeOf(abi.SliceType{}))
62+
StructType = FromReflect(reflect.TypeOf(abi.StructType{}))
6363

64-
IMethod = fromReflect(reflect.TypeOf(abi.Imethod{}))
65-
Method = fromReflect(reflect.TypeOf(abi.Method{}))
66-
StructField = fromReflect(reflect.TypeOf(abi.StructField{}))
67-
UncommonType = fromReflect(reflect.TypeOf(abi.UncommonType{}))
64+
IMethod = FromReflect(reflect.TypeOf(abi.Imethod{}))
65+
Method = FromReflect(reflect.TypeOf(abi.Method{}))
66+
StructField = FromReflect(reflect.TypeOf(abi.StructField{}))
67+
UncommonType = FromReflect(reflect.TypeOf(abi.UncommonType{}))
6868

69-
InterfaceSwitch = fromReflect(reflect.TypeOf(abi.InterfaceSwitch{}))
70-
TypeAssert = fromReflect(reflect.TypeOf(abi.TypeAssert{}))
69+
InterfaceSwitch = FromReflect(reflect.TypeOf(abi.InterfaceSwitch{}))
70+
TypeAssert = FromReflect(reflect.TypeOf(abi.TypeAssert{}))
7171

72-
ITab = fromReflect(reflect.TypeOf(abi.ITab{}))
72+
ITab = FromReflect(reflect.TypeOf(abi.ITab{}))
7373

7474
// Make sure abi functions are correct. These functions are used
7575
// by the linker which doesn't have the ability to do type layout,
@@ -92,8 +92,8 @@ func Init() {
9292
}
9393
}
9494

95-
// fromReflect translates from a host type to the equivalent target type.
96-
func fromReflect(rt reflect.Type) *types.Type {
95+
// FromReflect translates from a host type to the equivalent target type.
96+
func FromReflect(rt reflect.Type) *types.Type {
9797
t := reflectToType(rt)
9898
types.CalcSize(t)
9999
return t
@@ -108,6 +108,10 @@ func reflectToType(rt reflect.Type) *types.Type {
108108
return types.Types[types.TBOOL]
109109
case reflect.Int:
110110
return types.Types[types.TINT]
111+
case reflect.Int8:
112+
return types.Types[types.TINT8]
113+
case reflect.Int16:
114+
return types.Types[types.TINT16]
111115
case reflect.Int32:
112116
return types.Types[types.TINT32]
113117
case reflect.Uint8:
@@ -116,9 +120,15 @@ func reflectToType(rt reflect.Type) *types.Type {
116120
return types.Types[types.TUINT16]
117121
case reflect.Uint32:
118122
return types.Types[types.TUINT32]
123+
case reflect.Float32:
124+
return types.Types[types.TFLOAT32]
125+
case reflect.Float64:
126+
return types.Types[types.TFLOAT64]
119127
case reflect.Uintptr:
120128
return types.Types[types.TUINTPTR]
121-
case reflect.Ptr, reflect.Func, reflect.UnsafePointer:
129+
case reflect.Ptr:
130+
return types.NewPtr(reflectToType(rt.Elem()))
131+
case reflect.Func, reflect.UnsafePointer:
122132
// TODO: there's no mechanism to distinguish different pointer types,
123133
// so we treat them all as unsafe.Pointer.
124134
return types.Types[types.TUNSAFEPTR]
@@ -134,6 +144,12 @@ func reflectToType(rt reflect.Type) *types.Type {
134144
fields[i] = &types.Field{Sym: &types.Sym{Name: f.Name}, Type: ft}
135145
}
136146
return types.NewStruct(fields)
147+
case reflect.Chan:
148+
return types.NewChan(reflectToType(rt.Elem()), types.ChanDir(rt.ChanDir()))
149+
case reflect.String:
150+
return types.Types[types.TSTRING]
151+
case reflect.Complex128:
152+
return types.Types[types.TCOMPLEX128]
137153
default:
138154
base.Fatalf("unhandled kind %s", rt.Kind())
139155
return nil
@@ -155,7 +171,7 @@ func NewCursor(lsym *obj.LSym, off int64, t *types.Type) Cursor {
155171

156172
// WritePtr writes a pointer "target" to the component at the location specified by c.
157173
func (c Cursor) WritePtr(target *obj.LSym) {
158-
if c.typ.Kind() != types.TUNSAFEPTR {
174+
if c.typ.Kind() != types.TUNSAFEPTR && c.typ.Kind() != types.TPTR {
159175
base.Fatalf("can't write ptr, it has kind %s", c.typ.Kind())
160176
}
161177
if target == nil {

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,12 @@ func disjoint(p1 *Value, n1 int64, p2 *Value, n2 int64) bool {
863863
}
864864
return base, offset
865865
}
866+
867+
// Run types-based analysis
868+
if disjointTypes(p1.Type, p2.Type) {
869+
return true
870+
}
871+
866872
p1, off1 := baseAndOffset(p1)
867873
p2, off2 := baseAndOffset(p2)
868874
if isSamePtr(p1, p2) {
@@ -888,6 +894,39 @@ func disjoint(p1 *Value, n1 int64, p2 *Value, n2 int64) bool {
888894
return false
889895
}
890896

897+
// disjointTypes reports whether a memory region pointed to by a pointer of type
898+
// t1 does not overlap with a memory region pointed to by a pointer of type t2 --
899+
// based on type aliasing rules.
900+
func disjointTypes(t1 *types.Type, t2 *types.Type) bool {
901+
// Unsafe pointer can alias with anything.
902+
if t1.IsUnsafePtr() || t2.IsUnsafePtr() {
903+
return false
904+
}
905+
906+
if !t1.IsPtr() || !t2.IsPtr() {
907+
panic("disjointTypes: one of arguments is not a pointer")
908+
}
909+
910+
t1 = t1.Elem()
911+
t2 = t2.Elem()
912+
913+
// Not-in-heap types are not supported -- they are rare and non-important; also,
914+
// type.HasPointers check doesn't work for them correctly.
915+
if t1.NotInHeap() || t2.NotInHeap() {
916+
return false
917+
}
918+
919+
isPtrShaped := func(t *types.Type) bool { return int(t.Size()) == types.PtrSize && t.HasPointers() }
920+
921+
// Pointers and non-pointers are disjoint (https://pkg.go.dev/unsafe#Pointer).
922+
if (isPtrShaped(t1) && !t2.HasPointers()) ||
923+
(isPtrShaped(t2) && !t1.HasPointers()) {
924+
return true
925+
}
926+
927+
return false
928+
}
929+
891930
// moveSize returns the number of bytes an aligned MOV instruction moves.
892931
func moveSize(align int64, c *Config) int64 {
893932
switch {

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

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44

55
package ssa
66

7-
import "testing"
7+
import (
8+
"cmd/compile/internal/rttype"
9+
"reflect"
10+
"testing"
11+
"unsafe"
12+
)
813

914
// We generate memmove for copy(x[1:], x[:]), however we may change it to OpMove,
1015
// because size is known. Check that OpMove is alias-safe, or we did call memmove.
@@ -218,3 +223,65 @@ func TestMergePPC64AndSrwi(t *testing.T) {
218223
}
219224
}
220225
}
226+
227+
func TestDisjointTypes(t *testing.T) {
228+
tests := []struct {
229+
v1, v2 any // two pointers to some types
230+
expected bool
231+
}{
232+
{new(int8), new(int8), false},
233+
{new(int8), new(float32), false},
234+
{new(int8), new(*int8), true},
235+
{new(*int8), new(*float32), false},
236+
{new(*int8), new(chan<- int8), false},
237+
{new(**int8), new(*int8), false},
238+
{new(***int8), new(**int8), false},
239+
{new(int8), new(chan<- int8), true},
240+
{new(int), unsafe.Pointer(nil), false},
241+
{new(byte), new(string), false},
242+
{new(int), new(string), false},
243+
{new(*int8), new(struct{ a, b int }), true},
244+
{new(*int8), new(struct {
245+
a *int
246+
b int
247+
}), false},
248+
{new(*int8), new(struct {
249+
a int
250+
b *int
251+
}), false}, // with more precise analysis it should be true
252+
{new(*byte), new(string), false},
253+
{new(int), new(struct {
254+
a int
255+
b *int
256+
}), false},
257+
{new(float64), new(complex128), false},
258+
{new(*byte), new([]byte), false},
259+
{new(int), new([]byte), false},
260+
{new(int), new([2]*byte), false}, // with more recise analysis it should be true
261+
{new([2]int), new(*byte), true},
262+
}
263+
for _, tst := range tests {
264+
t1 := rttype.FromReflect(reflect.TypeOf(tst.v1))
265+
t2 := rttype.FromReflect(reflect.TypeOf(tst.v2))
266+
result := disjointTypes(t1, t2)
267+
if result != tst.expected {
268+
t.Errorf("disjointTypes(%s, %s) got %t expected %t", t1.String(), t2.String(), result, tst.expected)
269+
}
270+
}
271+
}
272+
273+
//go:noinline
274+
func foo(p1 *int64, p2 *float64) int64 {
275+
*p1 = 10
276+
*p2 = 0 // disjointTypes shouldn't consider this and preceding stores as non-aliasing
277+
return *p1
278+
}
279+
280+
func TestDisjointTypesRun(t *testing.T) {
281+
f := float64(0)
282+
i := (*int64)(unsafe.Pointer(&f))
283+
r := foo(i, &f)
284+
if r != 0 {
285+
t.Errorf("disjointTypes gives an incorrect answer that leads to an incorrect optimization.")
286+
}
287+
}

0 commit comments

Comments
 (0)