From 43c97eab79f88b544078ac777d9a4514429294cd Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 30 May 2014 16:27:51 -0400 Subject: [PATCH] go.tools/go/pointer: fix solver nontermination bug due to reflective type construction cycles. Programs such as this cause the PtrTo solver to attempt to enumerate an infinite set of types {T, *T, ..., *******T, etc}. t := reflect.TypeOf(T{}) for { t = reflect.PtrTo(t) } The fix is to bound the depth of reflectively created types at about 4 map/chan/slice/pointer constructors. + test. LGTM=gri R=gri CC=crawshaw, golang-codereviews https://golang.org/cl/102030044 --- go/pointer/reflect.go | 92 ++++++++++++++++++++++++++-------- go/pointer/testdata/reflect.go | 28 +++++++++++ 2 files changed, 100 insertions(+), 20 deletions(-) diff --git a/go/pointer/reflect.go b/go/pointer/reflect.go index 26ddaf21..c9b3e24a 100644 --- a/go/pointer/reflect.go +++ b/go/pointer/reflect.go @@ -11,8 +11,6 @@ package pointer // memoize as much as possible, like TypeOf and Zero do for their // tagged objects. // -// TODO(adonovan): all {} functions are TODO. -// // TODO(adonovan): this file is rather subtle. Explain how we derive // the implementation of each reflect operator from its spec, // including the subtleties of reflect.flag{Addr,RO,Indir}. @@ -151,7 +149,7 @@ func init() { // -------------------- (reflect.Value) -------------------- -func ext۰reflect۰Value۰Addr(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Value۰Addr(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (Value).Bytes() Value ---------- @@ -352,7 +350,7 @@ func ext۰reflect۰Value۰CallSlice(a *analysis, cgn *cgnode) { } } -func ext۰reflect۰Value۰Convert(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Value۰Convert(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (Value).Elem() Value ---------- @@ -411,10 +409,10 @@ func ext۰reflect۰Value۰Elem(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰Value۰Field(a *analysis, cgn *cgnode) {} -func ext۰reflect۰Value۰FieldByIndex(a *analysis, cgn *cgnode) {} -func ext۰reflect۰Value۰FieldByName(a *analysis, cgn *cgnode) {} -func ext۰reflect۰Value۰FieldByNameFunc(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Value۰Field(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰Value۰FieldByIndex(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰Value۰FieldByName(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰Value۰FieldByNameFunc(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (Value).Index() Value ---------- @@ -642,8 +640,8 @@ func ext۰reflect۰Value۰MapKeys(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰Value۰Method(a *analysis, cgn *cgnode) {} -func ext۰reflect۰Value۰MethodByName(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Value۰Method(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰Value۰MethodByName(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (Value).Recv(Value) ---------- @@ -749,7 +747,7 @@ func ext۰reflect۰Value۰Send(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰Value۰Set(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Value۰Set(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (Value).SetBytes(x []byte) ---------- @@ -857,7 +855,7 @@ func ext۰reflect۰Value۰SetMapIndex(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰Value۰SetPointer(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Value۰SetPointer(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (Value).Slice(v Value, i, j int) ---------- @@ -932,9 +930,9 @@ func ext۰reflect۰Value۰Slice(a *analysis, cgn *cgnode) { // -------------------- Standalone reflect functions -------------------- -func ext۰reflect۰Append(a *analysis, cgn *cgnode) {} -func ext۰reflect۰AppendSlice(a *analysis, cgn *cgnode) {} -func ext۰reflect۰Copy(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Append(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰AppendSlice(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰Copy(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func ChanOf(ChanDir, Type) Type ---------- @@ -962,6 +960,10 @@ func (c *reflectChanOfConstraint) solve(a *analysis, _ *node, delta nodeset) { for tObj := range delta { T := a.rtypeTaggedValue(tObj) + if typeTooHigh(T) { + continue + } + for _, dir := range c.dirs { if a.addLabel(c.result, a.makeRtype(types.NewChan(dir, T))) { changed = true @@ -1110,7 +1112,7 @@ func ext۰reflect۰MakeChan(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰MakeFunc(a *analysis, cgn *cgnode) {} +func ext۰reflect۰MakeFunc(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func MakeMap(Type) Value ---------- @@ -1222,7 +1224,7 @@ func ext۰reflect۰MakeSlice(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰MapOf(a *analysis, cgn *cgnode) {} +func ext۰reflect۰MapOf(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func New(Type) Value ---------- @@ -1310,6 +1312,10 @@ func (c *reflectPtrToConstraint) solve(a *analysis, _ *node, delta nodeset) { for tObj := range delta { T := a.rtypeTaggedValue(tObj) + if typeTooHigh(T) { + continue + } + if a.addLabel(c.result, a.makeRtype(types.NewPointer(T))) { changed = true } @@ -1327,7 +1333,7 @@ func ext۰reflect۰PtrTo(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰Select(a *analysis, cgn *cgnode) {} +func ext۰reflect۰Select(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func SliceOf(Type) Type ---------- @@ -1354,6 +1360,10 @@ func (c *reflectSliceOfConstraint) solve(a *analysis, _ *node, delta nodeset) { for tObj := range delta { T := a.rtypeTaggedValue(tObj) + if typeTooHigh(T) { + continue + } + if a.addLabel(c.result, a.makeRtype(types.NewSlice(T))) { changed = true } @@ -1613,8 +1623,8 @@ func ext۰reflect۰rtype۰Field(a *analysis, cgn *cgnode) { }) } -func ext۰reflect۰rtype۰FieldByIndex(a *analysis, cgn *cgnode) {} -func ext۰reflect۰rtype۰FieldByNameFunc(a *analysis, cgn *cgnode) {} +func ext۰reflect۰rtype۰FieldByIndex(a *analysis, cgn *cgnode) {} // TODO(adonovan) +func ext۰reflect۰rtype۰FieldByNameFunc(a *analysis, cgn *cgnode) {} // TODO(adonovan) // ---------- func (*rtype) In/Out(i int) Type ---------- @@ -1844,3 +1854,45 @@ func ext۰reflect۰rtype۰Method(a *analysis, cgn *cgnode) { result: a.funcResults(cgn.obj), }) } + +// typeHeight returns the "height" of the type, which is roughly +// speaking the number of chan, map, pointer and slice type constructors +// at the root of T; these are the four type kinds that can be created +// via reflection. Chan and map constructors are counted as double the +// height of slice and pointer constructors since they are less often +// deeply nested. +// +// The solver rules for type constructors must somehow bound the set of +// types they create to ensure termination of the algorithm in cases +// where the output of a type constructor flows to its input, e.g. +// +// func f(t reflect.Type) { +// f(reflect.PtrTo(t)) +// } +// +// It does this by limiting the type height to k, but this still leaves +// a potentially exponential (4^k) number of of types that may be +// enumerated in pathological cases. +// +func typeHeight(T types.Type) int { + switch T := T.(type) { + case *types.Chan: + return 2 + typeHeight(T.Elem()) + case *types.Map: + k := typeHeight(T.Key()) + v := typeHeight(T.Elem()) + if v > k { + k = v // max(k, v) + } + return 2 + k + case *types.Slice: + return 1 + typeHeight(T.Elem()) + case *types.Pointer: + return 1 + typeHeight(T.Elem()) + } + return 0 +} + +func typeTooHigh(T types.Type) bool { + return typeHeight(T) > 3 +} diff --git a/go/pointer/testdata/reflect.go b/go/pointer/testdata/reflect.go index ec254db0..6b8d0f22 100644 --- a/go/pointer/testdata/reflect.go +++ b/go/pointer/testdata/reflect.go @@ -78,10 +78,38 @@ func metareflection() { print(x0a.Interface()) // @types int } +type T struct{} + +// When the output of a type constructor flows to its input, we must +// bound the set of types created to ensure termination of the algorithm. +func typeCycle() { + t := reflect.TypeOf(0) + u := reflect.TypeOf("") + v := reflect.TypeOf(T{}) + for unknown { + t = reflect.PtrTo(t) + t = reflect.SliceOf(t) + + u = reflect.SliceOf(u) + + if unknown { + v = reflect.ChanOf(reflect.BothDir, v) + } else { + v = reflect.PtrTo(v) + } + } + + // Type height is bounded to about 4 map/slice/chan/pointer constructors. + print(reflect.Zero(t).Interface()) // @types int | []*int | []*[]*int + print(reflect.Zero(u).Interface()) // @types string | []string | [][]string | [][][]string | [][][][]string + print(reflect.Zero(v).Interface()) // @types T | *T | **T | ***T | ****T | chan T | *chan T | **chan T | chan *T | *chan *T | chan **T | chan ***T | chan chan T | chan *chan T | chan chan *T +} + func main() { reflectIndirect() reflectNewAt() reflectTypeOf() reflectTypeElem() metareflection() + typeCycle() }