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/link: ABI hash of a shared library changes if any inlineable function changes #23405

Closed
bryanpkc opened this issue Jan 10, 2018 · 8 comments
Closed

cmd/link: ABI hash of a shared library changes if any inlineable function changes #23405

bryanpkc opened this issue Jan 10, 2018 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@bryanpkc
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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

Tip.

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?

  1. Install a package containing an exported inlineable function in shared mode.
  2. Build an executable that links against the shared library produced in step 1.
  3. Make a minor change to the package, without changing the ABI, and re-install the shared library.
  4. Run the executable produced in step 2.

You can reproduce this easily with the makefile in my example:

https://github.com/bryanpkc/pkgdef-example

What did you expect to see?

Correct execution of the main executable.

What did you see instead?

$./main
abi mismatch detected between the executable and libgithub.com-bryanpkc-pkgdef-example-arith.so
fatal error: abi mismatch
runtime: panic before malloc heap initialized

Proposed solution

The ABI hash of a shared library (or Go plugin) is computed as a SHA1 sum of the package hashes of all packages included in the shared library, which are in turn computed from the export data of the packages (i.e. the contents of __.PKGDEF in the corresponding package archives). Currently the export data of a package includes the ASTs of all inlineable exported functions in the package. Doing this allows cross-package inlining in the usual static compilation scenario, but also makes the ABI hashes of shared libraries unnecessarily unstable.

This can be fixed by:

  1. When computing the ABI hash for the package, exclude the list of inlineable functions from a package's export data.
  2. Do not allow cross-package inlining from a dynlink package archive into a dependant program. (This may already be the case today, but I am not 100% sure.)
@bryanpkc
Copy link
Contributor Author

/cc @mwhudson

@ianlancetaylor
Copy link
Contributor

We don't have any mechanism for preventing inlining of a package that is put into a shared library. The problem is that inlining is done by the compiler, but determination of which packages will be pulled from a shared library is determined by the linker. Any fix we make here would have to address that somehow.

@ianlancetaylor ianlancetaylor changed the title ABI hash of a shared library changes if any inlineable function changes cmd/link: ABI hash of a shared library changes if any inlineable function changes Jan 10, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Jan 10, 2018
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 10, 2018
@hirochachacha
Copy link
Contributor

Related to #21510.

@bryanpkc
Copy link
Contributor Author

As I said in point 1 above, when the compiler is invoked with -dynlink, it can omit the inlineable functions from a package's export data. This will ensure that any other package the imports it will not be able to inline its functions. I do realize that this will hurt the performance of multi-package shared libraries, but multi-package shared libraries are not common outside of libstd.so, and we could try to come up with a special case for the latter.

@ianlancetaylor
Copy link
Contributor

That doesn't sound precisely right. -dynlink is used when building with -linkshared, and we don't need to disable inlining in that case. But, otherwise, yes, that sounds good. I didn't understand that was what you were suggesting originally.

@bryanpkc
Copy link
Contributor Author

You're right, I made a mistake when I said -dynlink. -shared might be a more appropriate flag, but currently it is not passed to compile in "shared" mode.

@bryanpkc
Copy link
Contributor Author

Taking a look at this issue again, we found that merely moving code in a package and inserting blank lines would change the export data, because the export data records file names and line numbers for all exported functions (even ones that are not inlineable). This seems unnecessarily restrictive.

@seankhliao
Copy link
Member

Obsoleted by #47788

@rsc rsc mentioned this issue Oct 29, 2021
Closed
@golang golang locked and limited conversation to collaborators Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@hirochachacha @ianlancetaylor @bryanpkc @gopherbot @seankhliao