import "C" not working after staticcheck upgrade. New loader regression?

[dzbarsky]

• The output of 'staticcheck -version' staticcheck_1.12_bin 2019.2.1
• The output of 'staticcheck -debug.version' (it is fine if this command fails)

staticcheck_1.12_bin 2019.2.1

Compiled with Go version: go1.12.9

• The output of 'go version' go version go1.12.9 linux/amd64
• The output of 'go env'

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/zbarsky/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/zbarsky/src/server/go"
GOPROXY=""
GORACE=""
GOROOT="/home/zbarsky/src/server/bazel-server/external/go1_12_9_linux_amd64_tar_gz/go"
GOTMPDIR=""
GOTOOLDIR="/home/zbarsky/src/server/bazel-server/external/go1_12_9_linux_amd64_tar_gz/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS="-I/home/zbarsky/src/server -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/leveldb/include/ -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/org_linuxcontainers_lxc/src/ -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/percona_server/include/ -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/rdkafka/ -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/rocksdb/include/ -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/zookeeper_3_4_6/include/ -I/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/com_github_dropbox_libseccomp/include/"
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-build280212957=/tmp/go-build -gno-record-gcc-switches"

• Exactly which command you ran ['/mnt/cache/home/zbarsky/.cache/bazel/_bazel_zbarsky/9beb5023491c0352a77da06871417523/execroot/__main__/bazel-out/k8-fastbuild/bin/tools/go-staticcheck.runfiles/__main__/go/src/honnef.co/go/tools/cmd/staticcheck/staticcheck_1.12', 'afs/afsd/...']
• Output of the command and what's wrong with the output

-: could not analyze dependency afs/afsd [afs/afsd.test] of afs/afsd.test (compile)
-: could not analyze dependency util/store/rocksdb of afs/afsd [afs/afsd.test]:
	could not analyze dependency util/store/rocksdb/levigo_rocksdb of util/store/rocksdb:
	/home/zbarsky/src/server/go/src/util/store/rocksdb/levigo_rocksdb/batch.go:4:8: could not import C (Config.Importer.Import(C) returned nil but no error) (compile)

• Where we can read the code you're running staticcheck on (GitHub repo, link to playground, code embedded in the issue, ...) Batch.go file from above:

package levigo_rocksdb

// #include "rocksdb/c.h"
import "C"

import (
        "unsafe"
)

type WriteBatch struct {
        wbatch *C.rocksdb_writebatch_t
}
<more code, not relevant>

Our setup is a bit weird due to use of bazel, so I'm hoping you can provide me some guidance to track this down but understand you may not be able to repro. How is "import C"/cgo supposed to work? staticcheck is using a custom importer:

importer := func(path string) (*types.Package, error) {
                if path == "unsafe" {
                        return types.Unsafe, nil
                }
                imp := pkg.Imports[path]
                if imp == nil {
                        return nil, nil
                }
                if len(imp.Errors) > 0 {
                        return nil, imp.Errors[0]
                }
                return imp.Types, nil
        }

With this code, pkg.Imports does not contain "C" so we return nil, is something else supposed to handle cgo? I believe the loading mechanism has changed, since this does work correctly in staticcheck from revision f68e85e63525b16caea5b5e587dad88f1646ddda. Any pointers?

[ainar-g]

Just a guess, but have your tried with 2019.2.2? It is the latest release.

[dominikh]

You're correct that the loading mechanism has changed. In the past, we used the go/loader package, which implemented package loading and cgo processing. Nowadays we're using the go/packages package, which defers to the underlying build system to resolve import paths, do cgo processing etc.

I don't have any experience with Bazel, but I can explain how cgo works with go build: go/packages calls out to go list with the right set of flags, which causes go list to apply cgo preprocessing. By the time we get a list of Go source files, all instances of import "C" will be gone, and references to the C package will have been replaced with the appropriate cgo magic.

To see this in action, create a trivial cgo-using package, such as

package foo

// #include <stdio.h>
import "C"

func fn() {
	C.puts()
}

and run go list -json -compiled on it. You will see a field named CompiledGoFiles – this is the set of Go files of the package that we will type-check. For example, our original file will have been transformed to the following:

/ Code generated by cmd/cgo; DO NOT EDIT.

//line /home/dominikh/prj/src/sandbox/foo/foo.go:1:1
package foo

// #include <stdio.h>
import _ "unsafe"

func fn() {
	( /*line :7:2*/_Cfunc_puts /*line :7:7*/)()
}

and will be accompanied by other, newly generated files that define functions and variables necessary for cgo, such as the definition of _Cfunc_puts.

As far as I am aware, go/packages doesn't have support for Bazel yet, so it should still be calling out to go list, which means I don't know why this isn't working for you.

If you export GOPACKAGESDEBUG=1, go/packages will emit debug information to stderr, showing all the processes it invokes, plus their arguments. Maybe it contains some useful information.

I'm afraid trying with 2019.2.2 won't help, that release did not fix anything related to package loading or cgo.

One thing to make sure: the system you're running staticcheck on, does it have a C compiler installed?

[dzbarsky]

Thanks for the pointers, GOPACKAGESDEBUG=1 enabled me to make some progress and confirm that compilation of cgo was failing. It seems previously we only needed access to c headers in a limited form, but now it seems we are performing a more full compilation of the c code (i.e. I had to include additional headers that weren't necessary previously). Is there a way to hint to the loader that we don't actually care about compiling the cgo code and to just trust the exported API? That would solve my issues

[dzbarsky]

i.e. I see errors like the following:

collect2: error: ld returned 1 exit status
# github.com/valyala/gozstd
/usr/bin/ld: cannot find -lzstd
collect2: error: ld returned 1 exit status
# github.com/jmhodges/levigo
/usr/bin/ld: cannot find -lleveldb

Whereas previously I don't think the loader was performing linking.

[dominikh]

I'll put it on my TODO list to investigate why we're linking. In the meantime, maybe @matloob has the answer?

[matloob]

Hm, maybe it's https://go-review.googlesource.com/c/tools/+/190339 ? That's the only recent cgo related change we've made, but I'm not sure how it could be connected.

[dominikh]

Note that it's not necessarily a recent change. I haven't bisected it yet, so the possible range of commits begins at "the creation of go/packages". OP is comparing to a version of staticcheck that predates the use of go/packages altogether.

I predict that this is go list doing more cgo work than is necessary.

[matloob]

Ah, got it.

[dominikh]

I've filed https://github.com/golang/go/issues/34229 to address the unnecessary use of the linker. In the meantime, I should improve my loader to emit a better error message for when cgo failed.