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

plugin: can't use plugin package in linkobj builds #33820

Closed
steeve opened this issue Aug 24, 2019 · 12 comments
Closed

plugin: can't use plugin package in linkobj builds #33820

steeve opened this issue Aug 24, 2019 · 12 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@steeve
Copy link
Contributor

steeve commented Aug 24, 2019

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

$ go1.13rc1 version
go version go1.13rc1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/steeve/Library/Caches/go-build"
GOENV="/Users/steeve/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/steeve/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/steeve/sdk/go1.13rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/steeve/sdk/go1.13rc1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/steeve/hdrgenerator/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bs/51dlb_nn5k35xq9qfsxv9wc00000gr/T/go-build596961202=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

$ cat test.go
package main

import _ "plugin"

func main() {}
$ go1.13rc1 tool compile -pack -o test.a -linkobj test.link.a test.go

$ go1.13rc1 tool link -o test test.link.a
test.link.a: missing package data entry

# this couldn't work, but I tested it anyway
$ go1.13rc1 tool link -o test test.a
/Users/steeve/sdk/go1.13rc1/pkg/tool/darwin_amd64/link: test.a: not package main

What did you expect to see?

A successful link.

What did you see instead?

test.link.a: missing package data entry

Related issue

Misc

It is also not possible to use buildmode=plugin in linkobj builds.

@smasher164 smasher164 changed the title Can't use plugin package in linkobj builds plugin: can't use plugin package in linkobj builds Aug 24, 2019
@smasher164
Copy link
Member

/cc @ianlancetaylor @cherrymui

@bcmills
Copy link
Contributor

bcmills commented Aug 26, 2019

CC @jayconrod

Note that -buildmode is a cmd/go flag, while -linkobj is a cmd/compile flag. It's no surprise that they don't get along...

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 26, 2019
@bcmills bcmills added this to the Unplanned milestone Aug 26, 2019
@steeve
Copy link
Contributor Author

steeve commented Aug 26, 2019

@bcmills I thought it was too, until I saw go build invoke it

$ go1.13rc1 tool link
usage: link [options] main.o
...
  -buildmode mode
    	set build mode
...

That said, the commands I sent are not building a plugin, just a normal program that does import "plugin"

@cherrymui
Copy link
Member

When building a plugin or a program that uses plugin, the linker reads the export data to find out the toolchain version, and uses this to build a hash to check version mismatch. In theory we could implement this in a different way.

However, the compiler's -linkobj option is used for some specific build tools, not for general uses. In particular, it is not used by "go build". @steeve is there a specific reason that you use this flag?

@steeve
Copy link
Contributor Author

steeve commented Aug 27, 2019

@cherrymui yes that makes sense. That said, it helps a lot to know linkobj is supposed to be handled with care.

I was asking because I'm adding linkobj to the Go bazel rules, which is pretty impressive at speeding up the build and reducing actions. I managed to disable it when building plug-ins, but unfortunately can't detect when "plugin" is imported. Also, it seems it needs that data for all dependant packages, or am I mistaken? I'm asking because I tried linking against a full archive for main, but if I remember correctly had the same problem. Should that work?

I heard the internal Blaze rules do it too, is it opt-in?

@cherrymui
Copy link
Member

I'm not familiar enough with Bazel to be sure, but I think you're probably right that it is needed for all the dependencies.

One possibility is that we could implement the version checking in a different way. For example, as it appears that the linker reads the export data solely for hashing, maybe the compiler could just hash it and put only the hash in the linkobj.

@steeve
Copy link
Contributor Author

steeve commented Sep 2, 2019

@cherrymui would you have pointers as to where this is done (hashing the export data) ?

@jayconrod
Copy link
Contributor

@steeve The hashing is done in genhash in src/cmd/link/internal/ld/lib.go.

@cherrymui Having the compiler compute this hash instead of requiring the linker to read export data seems like a good option. I think we can workaround this in Bazel rules_go, and we'll want to support older versions of Go, so this isn't super high priority, but it would be nice to have.

@cherrymui
Copy link
Member

There will likely be some rework of the object file in this cycle. I'll take this into consideration.

@gopherbot
Copy link

Change https://golang.org/cl/236119 mentions this issue: [dev.link] cmd/link: use fingerprint as package hash

@gopherbot
Copy link

Change https://golang.org/cl/236118 mentions this issue: [dev.link] cmd/compile: use hash of export data as fingerprint

gopherbot pushed a commit that referenced this issue Jun 4, 2020
Currently, the compiler generates a fingerprint for each package,
which is used by the linker for index consistency check.

When building plugin or shared object, currently the linker also
generates a hash, by hashing the export data. At run time, when
a package is referenced by multiple DSOs, this hash is compared
to ensure consistency.

It would be good if we can unify this two hashes. This way, the
linker doesn't need to read the export data (which is intended
for the compiler only, and is not always available for the
linker). The export data hash is sufficient for both purposes.
It is consistent with the current hash geneated by the linker.
And the export data includes indices for exported symbols, so its
hash can be used to catch index mismatches.

Updates #33820.

Change-Id: I2bc0d74930746f54c683a10dfd695d50ea3f5a38
Reviewed-on: https://go-review.googlesource.com/c/go/+/236118
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
@steeve
Copy link
Contributor Author

steeve commented Aug 12, 2020

thank you !

@golang golang locked and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants