From 8c5917cd76905b1ab16d41eadc8786e190eeecce Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 29 Apr 2022 10:47:57 -0700 Subject: [PATCH] cmd/compile: consistent unified IR handling of package unsafe Within the unified IR export format, I was treating package unsafe as a normal package, but expecting importers to correctly handle deduplicating it against their respective representation of package unsafe. However, the surrounding importer logic differs slightly between cmd/compile/internal/noder (which unified IR was originally implemented against) and go/importer (which it was more recently ported to). In particular, noder initializes its packages map as `map[string]*types2.Package{"unsafe": types2.Unsafe}`, whereas go/importer initializes it as just `make(map[string]*types.Package)`. This CL makes them all consistent. In particular, it: 1. changes noder to initialize packages to an empty map to prevent further latent issues from the discrepency, 2. adds the same special handling of package unsafe already present in go/internal/gcimporter's unified IR reader to both of cmd/compile's implementations, and 3. changes the unified IR writer to treat package unsafe as a builtin package, to force that readers similarly handle it correctly. Fixes #52623. Change-Id: Ibbab9b0a1d2a52d4cc91b56c5df49deedf81295a Reviewed-on: https://go-review.googlesource.com/c/go/+/403196 Auto-Submit: Matthew Dempsky Run-TryBot: Matthew Dempsky Reviewed-by: Russ Cox TryBot-Result: Gopher Robot --- src/cmd/compile/internal/importer/ureader.go | 12 +++++++----- src/cmd/compile/internal/noder/irgen.go | 2 +- src/cmd/compile/internal/noder/reader.go | 10 ++++++---- src/cmd/compile/internal/noder/writer.go | 14 +++++++++++--- src/go/internal/gcimporter/ureader.go | 13 ++++++------- 5 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/cmd/compile/internal/importer/ureader.go b/src/cmd/compile/internal/importer/ureader.go index a22cd2bb531..b8938cd2d67 100644 --- a/src/cmd/compile/internal/importer/ureader.go +++ b/src/cmd/compile/internal/importer/ureader.go @@ -148,11 +148,13 @@ func (pr *pkgReader) pkgIdx(idx int) *types2.Package { func (r *reader) doPkg() *types2.Package { path := r.String() - if path == "builtin" { - return nil // universe - } - if path == "" { + switch path { + case "": path = r.p.PkgPath() + case "builtin": + return nil // universe + case "unsafe": + return types2.Unsafe } if pkg := r.p.imports[path]; pkg != nil { @@ -371,7 +373,7 @@ func (pr *pkgReader) objIdx(idx int) (*types2.Package, string) { tag := pkgbits.CodeObj(rname.Code(pkgbits.SyncCodeObj)) if tag == pkgbits.ObjStub { - assert(objPkg == nil || objPkg == types2.Unsafe) + base.Assertf(objPkg == nil || objPkg == types2.Unsafe, "unexpected stub package: %v", objPkg) return objPkg, objName } diff --git a/src/cmd/compile/internal/noder/irgen.go b/src/cmd/compile/internal/noder/irgen.go index 5499ccd4051..628c0f54fc7 100644 --- a/src/cmd/compile/internal/noder/irgen.go +++ b/src/cmd/compile/internal/noder/irgen.go @@ -36,7 +36,7 @@ func checkFiles(noders []*noder) (posMap, *types2.Package, *types2.Info) { ctxt := types2.NewContext() importer := gcimports{ ctxt: ctxt, - packages: map[string]*types2.Package{"unsafe": types2.Unsafe}, + packages: make(map[string]*types2.Package), } conf := types2.Config{ Context: ctxt, diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index 1350c22467f..83ebe247792 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -282,11 +282,13 @@ func (pr *pkgReader) pkgIdx(idx int) *types.Pkg { func (r *reader) doPkg() *types.Pkg { path := r.String() - if path == "builtin" { - return types.BuiltinPkg - } - if path == "" { + switch path { + case "": path = r.p.PkgPath() + case "builtin": + return types.BuiltinPkg + case "unsafe": + return types.UnsafePkg } name := r.String() diff --git a/src/cmd/compile/internal/noder/writer.go b/src/cmd/compile/internal/noder/writer.go index 0fb162d381b..39f0ad794f0 100644 --- a/src/cmd/compile/internal/noder/writer.go +++ b/src/cmd/compile/internal/noder/writer.go @@ -214,13 +214,21 @@ func (pw *pkgWriter) pkgIdx(pkg *types2.Package) int { w := pw.newWriter(pkgbits.RelocPkg, pkgbits.SyncPkgDef) pw.pkgsIdx[pkg] = w.Idx - if pkg == nil { - w.String("builtin") - } else { + // The universe and package unsafe need to be handled specially by + // importers anyway, so we serialize them using just their package + // path. This ensures that readers don't confuse them for + // user-defined packages. + switch pkg { + case nil: // universe + w.String("builtin") // same package path used by godoc + case types2.Unsafe: + w.String("unsafe") + default: var path string if pkg != w.p.curpkg { path = pkg.Path() } + base.Assertf(path != "builtin" && path != "unsafe", "unexpected path for user-defined package: %q", path) w.String(path) w.String(pkg.Name()) w.Len(pkg.Height()) diff --git a/src/go/internal/gcimporter/ureader.go b/src/go/internal/gcimporter/ureader.go index 5260759c4fc..e27d3e0b4d2 100644 --- a/src/go/internal/gcimporter/ureader.go +++ b/src/go/internal/gcimporter/ureader.go @@ -184,14 +184,13 @@ func (pr *pkgReader) pkgIdx(idx int) *types.Package { func (r *reader) doPkg() *types.Package { path := r.String() - if path == "builtin" { - return nil // universe - } - if path == "unsafe" { - return types.Unsafe - } - if path == "" { + switch path { + case "": path = r.p.PkgPath() + case "builtin": + return nil // universe + case "unsafe": + return types.Unsafe } if pkg := r.p.imports[path]; pkg != nil {