cmd/go: missing entry in ImportMap for `runtime/cgo [runtime.test]` when -compiled is set

[dominikh]

What version of Go are you using (go version)?

$ go version
go version devel go1.17-3b770f2ccb Sun May 30 17:47:50 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

No

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/dominikh/.cache/go-build"
GOENV="/home/dominikh/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/dominikh/prj/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/dominikh/prj"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/dominikh/prj/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/dominikh/prj/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.17-3b770f2ccb Sun May 30 17:47:50 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/dominikh/prj/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1117114117=/tmp/go-build -gno-record-gcc-switches"

What did you do?

go list -json -compiled -test -deps runtime

What did you see?

The package with the ImportPath net [runtime.test] has an entry in Imports that reads runtime/cgo [runtime.test]. However, there is no corresponding entry in ImportMap.

It is my understanding that all non-identity imports should have a corresponding entry in ImportMap, and go/packages has matching logic for populating go/packages.Imports, a mapping from import paths to packages.

Note that this does not reproduce without the -compiled flag. It also does not reproduce in Go 1.16.4, but in that version, there is no package runtime/cgo [runtime.test] in the first place.

I am tentatively marking this as a release blocker because it is a regression and affects third party tools, but I may be overstepping here.

/cc @bcmills

[dominikh]

972e883 seems to be the commit that introduces the runtime/cgo [runtime.test] package to the package graph. At this point, it's already missing from the ImportMap.

Edit: to clarify, the commit causes the problem to become visible, but it's probably not the underlying reason for the incomplete ImportMap.

[jayconrod]

cc @matloob as well.

This might be working as intended, but I'm not completely sure. Would be good for others to confirm. Also I'm not at all sure if go/packages needs to be more aware of this.

It is my understanding that all non-identity imports should have a corresponding entry in ImportMap, and go/packages has matching logic for populating go/packages.Imports, a mapping from import paths to packages.

I think this is only true for mapping import strings from source files to packages passed to the linker (for example, after vendor resolution). It's not true for implicitly imported packages like runtime/cgo.

The runtime/cgo dependency is added to CompiledImports in Package.load. That addImport function makes no attempt to update ImportMap.

I think this only shows up with the -compiled flag because of this branch.

[mdempsky]

[Adding to the Go 1.17 milestone, at least until we can confirm whether this is working as intended.]

[gopherbot]

Change https://golang.org/cl/324573 mentions this issue: [dev.typeparams] cmd/compile: unified IR construction

[bcmills]

The addImport function is a fairly fragile hack to expand import "C" to a set of concrete packages; it wouldn't surprise me if the fact that it makes no attempt to update ImportMap is, in fact, the cause of a bug here.

(IMO a more robust alternative to addImport would synthesize an actual C package with its own unique import path and dependencies, and would and add an ImportMap entry for that synthetic package. But that's way more refactoring than we can afford to do right now.)

[bcmills]

I think this is only true for mapping import strings from source files to packages passed to the linker (for example, after vendor resolution). It's not true for implicitly imported packages like runtime/cgo.

I agree with Jay's assessment. The ImportMap reported by go list should map strings from import statements to the corresponding entities produced by go list. Since the source code does not itself contain the string import "runtime/cgo", the ImportMap does not need an entry for the path "runtime/cgo".

IMO the real bug here is the fact that there is no ImportMap entry for the import path "C". Ideally, instead of adding synthetic imports of the various cgo dependencies, we should add a synthetic C package that itself depends on the cgo dependencies, and add an ImportPath entry mapping "C" to that synthetic package. But that's more refactoring than we can afford to do at this point for Go 1.17.

[bcmills]

@dominikh, can you give some more detail on why the entry in Imports without a corresponding entry in ImportMap causes a problem for you? Perhaps there is some other way we can address that problem.

[dominikh]

@bcmills Concretely I ran into this issue because of the logic in https://github.com/golang/tools/blob/7ac129f24ace03f9aa0fa6097083e825536363e9/go/packages/golist.go#L635-L654 which populates a mapping from import paths as they appear in source code to the Package being imported. In Staticcheck I use this mapping when type-checking a package to resolve imports.

Concretely, I am currently getting the following error in Staticcheck when checking std with Go tip:

$ ./staticcheck -checks=none std
/home/dominikh/.cache/go-build/44/44c013685155a782dca631cad53734c5e4bdb7c6aa82272a1f1ceb8e2925e6fe-d:9:10: could not import runtime/cgo (trying to import "runtime/cgo" in the context of "net [runtime.test]" returned nil PackageSpec) (compile)

The file in question is generated by cmd/cgo and starts like this:

//go:cgo_ldflag "-g"
//go:cgo_ldflag "-O2"
// Code generated by cmd/cgo; DO NOT EDIT.

package net

import "unsafe"

import _ "runtime/cgo"

Because of the logic in go/packages, there is no mapping from runtime/cgo to any package.

Now, this might well be a bug in go/packages instead and someone might have to come up with different logic. But I disagree with the notion that "Since the source code does not itself contain the string". No, the code as written by the user does not in fact import runtime/cgo. However, that is not the input to the compiler or linker, either. First, cmd/cgo preprocesses the user's code, and the result of that very much imports runtime/cgo. That code is reflected in CompiledGoFiles. IMO it would make sense for ImportMap to match CompiledGoFiles, not GoFiles.

[bcmills]

Ahh, I see what you mean. The -compiled flag causes the source code (reported in CompiledGoFiles) to actually contain the literal string for the import, in which case the ImportMap entry is relevant.