Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/go: list -compiled fails to populate CompiledGoFiles when the resulting package cannot be linked #34229

Open
dominikh opened this issue Sep 11, 2019 · 4 comments
Open

cmd/go: list -compiled fails to populate CompiledGoFiles when the resulting package cannot be linked #34229

dominikh opened this issue Sep 11, 2019 · 4 comments
Labels
GoCommand cmd/go help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@dominikh
Copy link
Member

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

$ go version
go version go1.13 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

Linux, amd64

What did you do?

Create a package with the following contents:

package pkg

// #cgo LDFLAGS: -ldoesnotexist
import "C"

Run go list -json -x -compiled on the package

What did you expect to see?

No errors, no attempted linking of object files, complete JSON output (meaning CompiledGoFiles is populated).

What did you see instead?

$ go list -e -json -compiled -x
WORK=/tmp/go-build309865873
mkdir -p $WORK/b001/
cd /home/dominikh/prj/src/sandbox/bar
CGO_LDFLAGS='"-g" "-O2" "-ldoesnotexist"' /usr/lib/go/pkg/tool/linux_amd64/cgo -objdir $WORK/b001/ -importpath sandbox/bar -- -I $WORK/b001/ -g -O2 ./bar.go
cd $WORK
gcc -fno-caret-diagnostics -c -x c - -o /dev/null || true
gcc -Qunused-arguments -c -x c - -o /dev/null || true
gcc -fdebug-prefix-map=a=b -c -x c - -o /dev/null || true
gcc -gno-record-gcc-switches -c -x c - -o /dev/null || true
cd $WORK/b001
TERM='dumb' gcc -I /home/dominikh/prj/src/sandbox/bar -fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=$WORK/b001=/tmp/go-build -gno-record-gcc-switches -I ./ -g -O2 -o ./_x001.o -c _cgo_export.c
TERM='dumb' gcc -I /home/dominikh/prj/src/sandbox/bar -fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=$WORK/b001=/tmp/go-build -gno-record-gcc-switches -I ./ -g -O2 -o ./_x002.o -c bar.cgo2.c
TERM='dumb' gcc -I /home/dominikh/prj/src/sandbox/bar -fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=$WORK/b001=/tmp/go-build -gno-record-gcc-switches -I ./ -g -O2 -o ./_cgo_main.o -c _cgo_main.c
cd /home/dominikh/prj/src/sandbox/bar
TERM='dumb' gcc -I . -fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=$WORK/b001=/tmp/go-build -gno-record-gcc-switches -o $WORK/b001/_cgo_.o $WORK/b001/_cgo_main.o $WORK/b001/_x001.o $WORK/b001/_x002.o -g -O2 -ldoesnotexist
# sandbox/bar
/usr/bin/ld: cannot find -ldoesnotexist
collect2: error: ld returned 1 exit status
{
	"Dir": "/home/dominikh/prj/src/sandbox/bar",
	"ImportPath": "sandbox/bar",
	"Name": "pkg",
	"Target": "/home/dominikh/prj/pkg/linux_amd64/sandbox/bar.a",
	"Root": "/home/dominikh/prj/",
	"Match": [
		"."
	],
	"Stale": true,
	"StaleReason": "build ID mismatch",
	"CgoFiles": [
		"bar.go"
	],
	"CgoLDFLAGS": [
		"-ldoesnotexist"
	],
	"Imports": [
		"C",
		"unsafe",
		"runtime/cgo",
		"syscall"
	],
	"Deps": [
		"errors",
		"internal/bytealg",
		"internal/cpu",
		"internal/oserror",
		"internal/race",
		"internal/reflectlite",
		"runtime",
		"runtime/cgo",
		"runtime/internal/atomic",
		"runtime/internal/math",
		"runtime/internal/sys",
		"sync",
		"sync/atomic",
		"syscall",
		"unsafe"
	]
}

That is, Go actually attempts to build the package and fails due to the missing library we're trying to link against. It also doesn't populate CompiledGoFiles, even though cgo preprocessing succeeded.

As I understand it, this is trying to do more work than is necessary. It also unnecessarily fails.

The -compiled flag is documented as follows:

The -compiled flag causes list to set CompiledGoFiles to the Go source
files presented to the compiler. Typically this means that it repeats
the files listed in GoFiles and then also adds the Go code generated
by processing CgoFiles and SwigFiles. The Imports list contains the
union of all imports from both GoFiles and CompiledGoFiles.

It does not imply actual compilation. Also note that we're not passing the -exported flag. Furthermore, in order to do cgo processing, we do not need the linker. For that, looking at headers suffices, and go tool cgo foo.go does succeed.

After a cursory look at cmd/go/internal/work/exec.go, (*Builder).build, it seems that if we require CompiledGoFiles, and they can't be found in the cache, we execute the full cgo pipeline used for building. In theory, it should be possible to optimize this.

The current behavior is problematic for two reasons, from the point of view of tools that analyze Go code:

  1. it is slower than necessary. Analyzing a large cgo-using program will be significantly faster if we can skip the linking phase.
  2. it breaks certain setups of running static analysis. For example, a test environment may have the necessary headers available, but no object files for the libraries. We should be able to do cgo preprocessing on the code, and thus be able to do our static analysis. Requiring the linker to run successfully prevents this from happening.

/cc @matloob @ianthehat @bcmills
@dominikh dominikh added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Sep 11, 2019
@dominikh dominikh mentioned this issue Sep 11, 2019
Open
@bcmills
Copy link
Contributor

bcmills commented Sep 12, 2019 •
edited

No errors,

That is #26755.

no attempted linking of object files,

That's a subset of #29666, I think? But we can leave this issue open as a specific instance to address.

(CC @jayconrod)

@bcmills
Copy link
Contributor

bcmills commented Sep 12, 2019

We should certainly prioritize a fix for #26755, which is what breaks static analysis.

Let's make this issue about the “slower than necessary” part — I agree that it would be nice to avoid the unnecessary work, but I don't think that's likely to happen for 1.14.

@bcmills bcmills added this to the Unplanned milestone Sep 12, 2019
@dominikh
Copy link
Member Author

These issues are certainly related, but even if both #26755 and #29666 were fixed, this issue wouldn't be fixed.

#26755 is about -e not being honored in all cases, which means go list will exit non-zero for packages that fail to compile. However, when that issue gets fixed, its only effect on this issue is that the linker error will be moved into the per-package structure. It would still fail to populate CompiledGoFiles because go list would still try to compile and link.

#29666 is related only insofar as it, too, wants to improve the performance of go list. However, #29666 is about only computing those fields that a template actually uses, while this issue is about doing more work than is necessary to compute a field that is being used.

I regret bringing up performance in the first place, because this issue is also one of correctness, in its own right. Currently, to compute CompiledGoFiles, go list invokes the full cgo pipeline, including compilation and linking. This does not work on systems that don't have the necessary libraries for linking available. And strictly speaking, computation of CompiledGoFiles shouldn't need libraries. Computing it should only require the cgo preprocessing step, i.e. the processing of headers and rewriting of Go code. In other words, we fail to populate CompiledGoFiles even when we should be able to populate it.

@bcmills
Copy link
Contributor

bcmills commented Sep 12, 2019

Ah, I see. Will update and retag accordingly.

@bcmills bcmills changed the title cmd/go: list -compiled does too much work when using cgo cmd/go: list -compiled fails to populate CompiledGoFiles when the resulting package cannot be linked Sep 12, 2019
@bcmills bcmills removed the ToolSpeed label Sep 12, 2019
@bcmills bcmills modified the milestones: Unplanned, Go1.14 Sep 12, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@dominikh dominikh mentioned this issue May 11, 2020
Closed
@dzbarsky dzbarsky mentioned this issue Sep 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@dominikh @rsc @bcmills