diff --git a/AUTHORS b/AUTHORS index de23957..f04de5a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -14,3 +14,4 @@ Zachary Wasserman lu4p pagran shellhazard +xuannv diff --git a/reflect.go b/reflect.go index bca6fe2..ef1048f 100644 --- a/reflect.go +++ b/reflect.go @@ -406,32 +406,27 @@ func (ri *reflectInspector) recursivelyRecordUsedForReflect(t types.Type) { // TODO: consider caching recordedObjectString via a map, // if that shows an improvement in our benchmark func recordedObjectString(obj types.Object) objectString { + // For exported fields, "pkgpath.Field" is not unique, + // because two exported top-level types could share "Field". + // + // Moreover, note that not all fields belong to named struct types; + // an API could be exposing: + // + // var usedInReflection = struct{Field string} + // + // For now, a hack: assume that packages don't declare the same field + // more than once in the same line. This works in practice, but one + // could craft Go code to break this assumption. + // Also note that the compiler's object files include filenames and line + // numbers, but not column numbers nor byte offsets. + // TODO(mvdan): give this another think, and add tests involving anon types. + // Note that fields are never top-level. pkg := obj.Pkg() - if obj, ok := obj.(*types.Var); ok && obj.IsField() { - // For exported fields, "pkgpath.Field" is not unique, - // because two exported top-level types could share "Field". - // - // Moreover, note that not all fields belong to named struct types; - // an API could be exposing: - // - // var usedInReflection = struct{Field string} - // - // For now, a hack: assume that packages don't declare the same field - // more than once in the same line. This works in practice, but one - // could craft Go code to break this assumption. - // Also note that the compiler's object files include filenames and line - // numbers, but not column numbers nor byte offsets. - // TODO(mvdan): give this another think, and add tests involving anon types. + if pkg.Scope() != obj.Parent() { pos := fset.Position(obj.Pos()) return fmt.Sprintf("%s.%s - %s:%d", pkg.Path(), obj.Name(), filepath.Base(pos.Filename), pos.Line) } - // Names which are not at the top level cannot be imported, - // so we don't need to record them either. - // Note that this doesn't apply to fields, which are never top-level. - if pkg.Scope() != obj.Parent() { - return "" - } // For top-level exported names, "pkgpath.Name" is unique. return pkg.Path() + "." + obj.Name() } @@ -442,20 +437,44 @@ func (ri *reflectInspector) recordUsedForReflect(obj types.Object) { if obj.Pkg().Path() != ri.pkg.Path() { panic("called recordUsedForReflect with a foreign object") } - objStr := recordedObjectString(obj) - if objStr == "" { - // If the object can't be described via a qualified string, - // do we need to record it at all? + + if obj, ok := obj.(*types.Var); ok && obj.IsField() { + ri.result.ReflectObjects[recordedObjectString(obj)] = struct{}{} + + if !obj.Embedded() { + return + } + + embeddedType, ok := obj.Type().(*types.Named) + if !ok { + return + } + + embeddedObj := embeddedType.Obj() + if embeddedObj.Pkg().Scope() == embeddedObj.Parent() { + // not local type + return + } + + if embeddedObj.Pkg() == nil || embeddedObj.Pkg() != ri.pkg { + // not from the specified package + return + } + + ri.result.ReflectObjects[recordedObjectString(embeddedObj)] = struct{}{} + return } - ri.result.ReflectObjects[objStr] = struct{}{} + + // we don't need to record the local type names + if obj.Pkg().Scope() != obj.Parent() { + return + } + + ri.result.ReflectObjects[recordedObjectString(obj)] = struct{}{} } func usedForReflect(cache pkgCache, obj types.Object) bool { - objStr := recordedObjectString(obj) - if objStr == "" { - return false - } - _, ok := cache.ReflectObjects[objStr] + _, ok := cache.ReflectObjects[recordedObjectString(obj)] return ok } diff --git a/testdata/script/reflect.txtar b/testdata/script/reflect.txtar index e7ba4d9..a8f88c1 100644 --- a/testdata/script/reflect.txtar +++ b/testdata/script/reflect.txtar @@ -29,6 +29,7 @@ import ( "math/big" "os" "reflect" + "unsafe" "strings" "text/template" @@ -115,7 +116,7 @@ func main() { printfWithoutPackage("%#v\n", variadic) testx509() - + testGoSpew() // Very complex reflection used by gorm user := StatUser{} @@ -175,13 +176,54 @@ type FmtType struct { } // copied from github.com/davecgh/go-spew, which reaches into reflect's internals -var _ = func() uintptr { - field, ok := reflect.TypeOf(reflect.Value{}).FieldByName("flag") - if !ok { - panic("reflect.Value has no flag field") +func testGoSpew() { + flagValOffset := func() uintptr { + field, ok := reflect.TypeOf(reflect.Value{}).FieldByName("flag") + if !ok { + panic("reflect.Value has no flag field") + } + return field.Offset + }() + + type flag uintptr + flagField := func(v *reflect.Value) *flag { + return (*flag)(unsafe.Pointer(uintptr(unsafe.Pointer(v)) + flagValOffset)) + } + + type t0 int + var t struct { + A t0 + t0 + a t0 + } + vA := reflect.ValueOf(t).FieldByName("A") + va := reflect.ValueOf(t).FieldByName("a") + vt0 := reflect.ValueOf(t).FieldByName("t0") + flagvA := *flagField(&vA) + flagva := *flagField(&va) + flagvt0 := *flagField(&vt0) + + if flagvA&flagva&flagvt0 == 0 { + panic("reflect.Value read-only flag has changed semantics") } - return field.Offset -}() + + type T0 int + var T struct { + A T0 + T0 + a T0 + } + vA = reflect.ValueOf(T).FieldByName("A") + va = reflect.ValueOf(T).FieldByName("a") + vt0 = reflect.ValueOf(T).FieldByName("T0") + flagvA = *flagField(&vA) + flagva = *flagField(&va) + flagvt0 = *flagField(&vt0) + + if flagvA&flagva&flagvt0 == 0 { + panic("reflect.Value read-only flag has changed semantics") + } +} // encoding/x509 uses encoding/asn1, which uses reflect. // In one place it depends on field names; that used to be broken by garble.