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
Comments
CC @jayconrod Note that |
@bcmills I thought it was too, until I saw
That said, the commands I sent are not building a plugin, just a normal program that does |
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? |
@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? |
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. |
@cherrymui would you have pointers as to where this is done (hashing the export data) ? |
@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. |
There will likely be some rework of the object file in this cycle. I'll take this into consideration. |
Change https://golang.org/cl/236119 mentions this issue: |
Change https://golang.org/cl/236118 mentions this issue: |
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>
thank you ! |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
What did you expect to see?
A successful link.
What did you see instead?
Related issue
Misc
It is also not possible to use
buildmode=plugin
in linkobj builds.The text was updated successfully, but these errors were encountered: