Skip to content

Commit

Permalink
cmd/compile: consistent unified IR handling of package unsafe
Browse files Browse the repository at this point in the history
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 <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
mdempsky authored and gopherbot committed Apr 29, 2022
1 parent e7c56fe commit 8c5917c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 19 deletions.
12 changes: 7 additions & 5 deletions src/cmd/compile/internal/importer/ureader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/noder/irgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 6 additions & 4 deletions src/cmd/compile/internal/noder/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
14 changes: 11 additions & 3 deletions src/cmd/compile/internal/noder/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
11 changes: 5 additions & 6 deletions src/go/internal/gcimporter/ureader.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,14 @@ func (pr *pkgReader) pkgIdx(idx int) *types.Package {

func (r *reader) doPkg() *types.Package {
path := r.String()
if path == "builtin" {
switch path {
case "":
path = r.p.PkgPath()
case "builtin":
return nil // universe
}
if path == "unsafe" {
case "unsafe":
return types.Unsafe
}
if path == "" {
path = r.p.PkgPath()
}

if pkg := r.p.imports[path]; pkg != nil {
return pkg
Expand Down

0 comments on commit 8c5917c

Please sign in to comment.