Skip to content

Commit 1ca4aa2

Browse files
committed
internal/gcimporter: refine golang/go#76222 crash
This CL asserts that pkg is non-nil in exportWriter.pkg, which will help us distinguish a nil pointer from an invalid (i.e. memory corrupted) value in future telemetry field reports. Also, eliminate the mutable currPkg field of importReader by plumbing it down as a parameter through the obj() and signature() methods. Updates golang/go#76222 Change-Id: I702219bc0a3bd7fecaaf379bef80e759ba05a9bf Reviewed-on: https://go-review.googlesource.com/c/tools/+/719260 Reviewed-by: Robert Findley <rfindley@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 73ff4bd commit 1ca4aa2

File tree

2 files changed

+47
-37
lines changed

2 files changed

+47
-37
lines changed

internal/gcimporter/iexport.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,13 @@ func (w *exportWriter) posV0(pos token.Pos) {
943943
}
944944

945945
func (w *exportWriter) pkg(pkg *types.Package) {
946+
if pkg == nil {
947+
// [exportWriter.typ] accepts a nil pkg only for types
948+
// of constants, which cannot contain named objects
949+
// such as fields or methods and thus should never
950+
// reach this method (#76222).
951+
panic("nil package")
952+
}
946953
// Ensure any referenced packages are declared in the main index.
947954
w.p.allPkgs[pkg] = true
948955

@@ -958,9 +965,11 @@ func (w *exportWriter) qualifiedType(obj *types.TypeName) {
958965
w.pkg(obj.Pkg())
959966
}
960967

961-
// TODO(rfindley): what does 'pkg' even mean here? It would be better to pass
962-
// it in explicitly into signatures and structs that may use it for
963-
// constructing fields.
968+
// typ emits the specified type.
969+
//
970+
// Objects within the type (struct fields and interface methods) are
971+
// qualified by pkg. It may be nil if the type cannot contain objects,
972+
// such as the type of a constant.
964973
func (w *exportWriter) typ(t types.Type, pkg *types.Package) {
965974
w.data.uint64(w.p.typOff(t, pkg))
966975
}
@@ -990,6 +999,7 @@ func (w *exportWriter) startType(k itag) {
990999
w.data.uint64(uint64(k))
9911000
}
9921001

1002+
// doTyp is the implementation of [exportWriter.typ].
9931003
func (w *exportWriter) doTyp(t types.Type, pkg *types.Package) {
9941004
if trace {
9951005
w.p.trace("exporting type %s (%T)", t, t)
@@ -1063,7 +1073,7 @@ func (w *exportWriter) doTyp(t types.Type, pkg *types.Package) {
10631073

10641074
case *types.Signature:
10651075
w.startType(signatureType)
1066-
w.pkg(pkg)
1076+
w.pkg(pkg) // qualifies param/result vars
10671077
w.signature(t)
10681078

10691079
case *types.Struct:
@@ -1109,19 +1119,19 @@ func (w *exportWriter) doTyp(t types.Type, pkg *types.Package) {
11091119

11101120
case *types.Interface:
11111121
w.startType(interfaceType)
1112-
w.pkg(pkg)
1122+
w.pkg(pkg) // qualifies unexported method funcs
11131123

11141124
n := t.NumEmbeddeds()
11151125
w.uint64(uint64(n))
11161126
for i := 0; i < n; i++ {
11171127
ft := t.EmbeddedType(i)
1118-
tPkg := pkg
11191128
if named, _ := types.Unalias(ft).(*types.Named); named != nil {
11201129
w.pos(named.Obj().Pos())
11211130
} else {
1131+
// e.g. ~int
11221132
w.pos(token.NoPos)
11231133
}
1124-
w.typ(ft, tPkg)
1134+
w.typ(ft, pkg)
11251135
}
11261136

11271137
// See comment for struct fields. In shallow mode we change the encoding

internal/gcimporter/iimport.go

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,10 @@ func (p *iimporter) doDecl(pkg *types.Package, name string) {
432432
errorf("%v.%v not in index", pkg, name)
433433
}
434434

435-
r := &importReader{p: p, currPkg: pkg}
435+
r := &importReader{p: p}
436436
r.declReader.Reset(p.declData[off:])
437437

438-
r.obj(name)
438+
r.obj(pkg, name)
439439
}
440440

441441
func (p *iimporter) stringAt(off uint64) string {
@@ -551,7 +551,6 @@ func canReuse(def *types.Named, rhs types.Type) bool {
551551
type importReader struct {
552552
p *iimporter
553553
declReader bytes.Reader
554-
currPkg *types.Package
555554
prevFile string
556555
prevLine int64
557556
prevColumn int64
@@ -565,7 +564,8 @@ type importReader struct {
565564
// for 1.24, but the fix was not worth back-porting).
566565
var markBlack = func(name *types.TypeName) {}
567566

568-
func (r *importReader) obj(name string) {
567+
// obj decodes and declares the package-level object denoted by (pkg, name).
568+
func (r *importReader) obj(pkg *types.Package, name string) {
569569
tag := r.byte()
570570
pos := r.pos()
571571

@@ -576,27 +576,27 @@ func (r *importReader) obj(name string) {
576576
tparams = r.tparamList()
577577
}
578578
typ := r.typ()
579-
obj := aliases.NewAlias(r.p.aliases, pos, r.currPkg, name, typ, tparams)
579+
obj := aliases.NewAlias(r.p.aliases, pos, pkg, name, typ, tparams)
580580
markBlack(obj) // workaround for golang/go#69912
581581
r.declare(obj)
582582

583583
case constTag:
584584
typ, val := r.value()
585585

586-
r.declare(types.NewConst(pos, r.currPkg, name, typ, val))
586+
r.declare(types.NewConst(pos, pkg, name, typ, val))
587587

588588
case funcTag, genericFuncTag:
589589
var tparams []*types.TypeParam
590590
if tag == genericFuncTag {
591591
tparams = r.tparamList()
592592
}
593-
sig := r.signature(nil, nil, tparams)
594-
r.declare(types.NewFunc(pos, r.currPkg, name, sig))
593+
sig := r.signature(pkg, nil, nil, tparams)
594+
r.declare(types.NewFunc(pos, pkg, name, sig))
595595

596596
case typeTag, genericTypeTag:
597597
// Types can be recursive. We need to setup a stub
598598
// declaration before recursing.
599-
obj := types.NewTypeName(pos, r.currPkg, name, nil)
599+
obj := types.NewTypeName(pos, pkg, name, nil)
600600
named := types.NewNamed(obj, nil, nil)
601601

602602
markBlack(obj) // workaround for golang/go#69912
@@ -616,7 +616,7 @@ func (r *importReader) obj(name string) {
616616
for n := r.uint64(); n > 0; n-- {
617617
mpos := r.pos()
618618
mname := r.ident()
619-
recv := r.param()
619+
recv := r.param(pkg)
620620

621621
// If the receiver has any targs, set those as the
622622
// rparams of the method (since those are the
@@ -630,9 +630,9 @@ func (r *importReader) obj(name string) {
630630
rparams[i] = types.Unalias(targs.At(i)).(*types.TypeParam)
631631
}
632632
}
633-
msig := r.signature(recv, rparams, nil)
633+
msig := r.signature(pkg, recv, rparams, nil)
634634

635-
named.AddMethod(types.NewFunc(mpos, r.currPkg, mname, msig))
635+
named.AddMethod(types.NewFunc(mpos, pkg, mname, msig))
636636
}
637637
}
638638

@@ -644,12 +644,12 @@ func (r *importReader) obj(name string) {
644644
errorf("unexpected type param type")
645645
}
646646
name0 := tparamName(name)
647-
tn := types.NewTypeName(pos, r.currPkg, name0, nil)
647+
tn := types.NewTypeName(pos, pkg, name0, nil)
648648
t := types.NewTypeParam(tn, nil)
649649

650650
// To handle recursive references to the typeparam within its
651651
// bound, save the partial type in tparamIndex before reading the bounds.
652-
id := ident{r.currPkg, name}
652+
id := ident{pkg, name}
653653
r.p.tparamIndex[id] = t
654654
var implicit bool
655655
if r.p.version >= iexportVersionGo1_18 {
@@ -672,7 +672,7 @@ func (r *importReader) obj(name string) {
672672
case varTag:
673673
typ := r.typ()
674674

675-
v := types.NewVar(pos, r.currPkg, name, typ)
675+
v := types.NewVar(pos, pkg, name, typ)
676676
typesinternal.SetVarKind(v, typesinternal.PackageVar)
677677
r.declare(v)
678678

@@ -905,11 +905,11 @@ func (r *importReader) doType(base *types.Named) (res types.Type) {
905905
case mapType:
906906
return types.NewMap(r.typ(), r.typ())
907907
case signatureType:
908-
r.currPkg = r.pkg()
909-
return r.signature(nil, nil, nil)
908+
paramPkg := r.pkg()
909+
return r.signature(paramPkg, nil, nil, nil)
910910

911911
case structType:
912-
r.currPkg = r.pkg()
912+
fieldPkg := r.pkg()
913913

914914
fields := make([]*types.Var, r.uint64())
915915
tags := make([]string, len(fields))
@@ -932,7 +932,7 @@ func (r *importReader) doType(base *types.Named) (res types.Type) {
932932
// discussed in iexport.go, this is not correct, but mostly works and is
933933
// preferable to failing (for now at least).
934934
if field == nil {
935-
field = types.NewField(fpos, r.currPkg, fname, ftyp, emb)
935+
field = types.NewField(fpos, fieldPkg, fname, ftyp, emb)
936936
}
937937

938938
fields[i] = field
@@ -941,7 +941,7 @@ func (r *importReader) doType(base *types.Named) (res types.Type) {
941941
return types.NewStruct(fields, tags)
942942

943943
case interfaceType:
944-
r.currPkg = r.pkg()
944+
methodPkg := r.pkg() // qualifies methods and their param/result vars
945945

946946
embeddeds := make([]types.Type, r.uint64())
947947
for i := range embeddeds {
@@ -963,12 +963,12 @@ func (r *importReader) doType(base *types.Named) (res types.Type) {
963963
// don't agree with this.
964964
var recv *types.Var
965965
if base != nil {
966-
recv = types.NewVar(token.NoPos, r.currPkg, "", base)
966+
recv = types.NewVar(token.NoPos, methodPkg, "", base)
967967
}
968-
msig := r.signature(recv, nil, nil)
968+
msig := r.signature(methodPkg, recv, nil, nil)
969969

970970
if method == nil {
971-
method = types.NewFunc(mpos, r.currPkg, mname, msig)
971+
method = types.NewFunc(mpos, methodPkg, mname, msig)
972972
}
973973
methods[i] = method
974974
}
@@ -1049,9 +1049,9 @@ func (r *importReader) objectPathObject() types.Object {
10491049
return obj
10501050
}
10511051

1052-
func (r *importReader) signature(recv *types.Var, rparams []*types.TypeParam, tparams []*types.TypeParam) *types.Signature {
1053-
params := r.paramList()
1054-
results := r.paramList()
1052+
func (r *importReader) signature(paramPkg *types.Package, recv *types.Var, rparams []*types.TypeParam, tparams []*types.TypeParam) *types.Signature {
1053+
params := r.paramList(paramPkg)
1054+
results := r.paramList(paramPkg)
10551055
variadic := params.Len() > 0 && r.bool()
10561056
return types.NewSignatureType(recv, rparams, tparams, params, results, variadic)
10571057
}
@@ -1070,19 +1070,19 @@ func (r *importReader) tparamList() []*types.TypeParam {
10701070
return xs
10711071
}
10721072

1073-
func (r *importReader) paramList() *types.Tuple {
1073+
func (r *importReader) paramList(pkg *types.Package) *types.Tuple {
10741074
xs := make([]*types.Var, r.uint64())
10751075
for i := range xs {
1076-
xs[i] = r.param()
1076+
xs[i] = r.param(pkg)
10771077
}
10781078
return types.NewTuple(xs...)
10791079
}
10801080

1081-
func (r *importReader) param() *types.Var {
1081+
func (r *importReader) param(pkg *types.Package) *types.Var {
10821082
pos := r.pos()
10831083
name := r.ident()
10841084
typ := r.typ()
1085-
return types.NewParam(pos, r.currPkg, name, typ)
1085+
return types.NewParam(pos, pkg, name, typ)
10861086
}
10871087

10881088
func (r *importReader) bool() bool {

0 commit comments

Comments
 (0)