-
Notifications
You must be signed in to change notification settings - Fork 46
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
Update cargo dependencies, fix key engine LTO #499
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
varunpuranik
approved these changes
Jan 17, 2023
onalante-msft
approved these changes
Jan 19, 2023
onalante-msft
force-pushed
the
main
branch
5 times, most recently
from
January 19, 2023 21:01
4aba0a6
to
589091a
Compare
LTO is enabled by default for .deb packages starting in Ubuntu 21.04[^1], causing build failures on Ubuntu 22.04. Previously, we hacked around this by compiling outside of the `dpkg-buildpackage` environment[^2]. However, this hack stopped working when the `cc` crate began emitting `rerun-if-env-changed` in version `1.0.74`[^3]. Hence, would need to set `emit_rerun_if_env_changed(false)` for all `cc::Build` instances in `aziot-key-openssl-engine-shared` and its dependency graph to restore our hack for `cc > 1.0.73`. This is not remotely practical. LTO is incompatible with `__asm__(".symver ..")` because it uses the linker-script-provided exported symbols to determine what is safe to optimize out, and the linker script is naturally unaware of manually-exported symbols. We can work around this by adding `__attribute__((used))` to the functions we want to keep in the final object. Another option would be to use GCC's `__attribute__((symver("..@")))`[^4] directive, but this relies on too new of a toolchain version (GCC 10). Addendum 1: The fundamental reason for why LTO is a problem for `aziot-key-openssl-engine-shared` in the first place is that this crate uses what is, in effect, a "symbol stub" to hook Rust code into OpenSSL's engine macros. First, `build/engine.c` declares the engine function signature and uses OpenSSL's macros to expand the dynamic engine binding. This file is then compiled (but not linked) into an object that will become the dynamic library's public interface. The way this is accomplished is by linking the whole object into the `cdylib` (as opposed to only linking referenced functions). LTO requires us to go one step further by preventing the linker from optimizing out symbols not declared as globally-exported in `rustc`'s linker script, which does not know of the symbol declaration in the stub object. There is an open RFC request for allowing re-export of C symbols from `cdylib` crates: rust-lang/rfcs#2771. Addendum 2: When our supported platforms start shipping with GNU as >= 2.35 and/or Clang 13, we may want to add `,remove` to the `.symver` directive arguments to lift the restriction that `aziot-key-openssl-engine-shared` cannot be included in tests[^5]. [^1]: https://wiki.ubuntu.com/ToolChain/LTO [^2]: Azure@f66c155 [^3]: https://github.com/rust-lang/cc-rs/releases/tag/1.0.74 [^4]: `..@` instead of `..@@` since we do not define a version node name, meaning `..@@` leads to symbol duplication. [^5]: https://maskray.me/blog/2020-11-26-all-about-symbol-versioning
onalante-msft
changed the title
Update cargo dependencies
Update cargo dependencies, fix key engine LTO
Jan 19, 2023
damonbarry
pushed a commit
to damonbarry/iot-identity-service
that referenced
this pull request
Feb 9, 2023
LTO is enabled by default for .deb packages starting in Ubuntu 21.04[^1], causing build failures on Ubuntu 22.04. Previously, we hacked around this by compiling outside of the `dpkg-buildpackage` environment[^2]. However, this hack stopped working when the `cc` crate began emitting `rerun-if-env-changed` in version `1.0.74`[^3]. Hence, would need to set `emit_rerun_if_env_changed(false)` for all `cc::Build` instances in `aziot-key-openssl-engine-shared` and its dependency graph to restore our hack for `cc > 1.0.73`. This is not remotely practical. LTO is incompatible with `__asm__(".symver ..")` because it uses the linker-script-provided exported symbols to determine what is safe to optimize out, and the linker script is naturally unaware of manually-exported symbols. We can work around this by adding `__attribute__((used))` to the functions we want to keep in the final object. Another option would be to use GCC's `__attribute__((symver("..@")))`[^4] directive, but this relies on too new of a toolchain version (GCC 10). Addendum 1: The fundamental reason for why LTO is a problem for `aziot-key-openssl-engine-shared` in the first place is that this crate uses what is, in effect, a "symbol stub" to hook Rust code into OpenSSL's engine macros. First, `build/engine.c` declares the engine function signature and uses OpenSSL's macros to expand the dynamic engine binding. This file is then compiled (but not linked) into an object that will become the dynamic library's public interface. The way this is accomplished is by linking the whole object into the `cdylib` (as opposed to only linking referenced functions). LTO requires us to go one step further by preventing the linker from optimizing out symbols not declared as globally-exported in `rustc`'s linker script, which does not know of the symbol declaration in the stub object. There is an open RFC request for allowing re-export of C symbols from `cdylib` crates: rust-lang/rfcs#2771. Addendum 2: When our supported platforms start shipping with GNU as >= 2.35 and/or Clang 13, we may want to add `,remove` to the `.symver` directive arguments to lift the restriction that `aziot-key-openssl-engine-shared` cannot be included in tests[^5]. [^1]: https://wiki.ubuntu.com/ToolChain/LTO [^2]: Azure@f66c155 [^3]: https://github.com/rust-lang/cc-rs/releases/tag/1.0.74 [^4]: `..@` instead of `..@@` since we do not define a version node name, meaning `..@@` leads to symbol duplication. [^5]: https://maskray.me/blog/2020-11-26-all-about-symbol-versioning
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LTO is enabled by default for .deb packages starting in Ubuntu
21.041, causing build failures on Ubuntu 22.04. Previously, we
hacked around this by compiling outside of the
dpkg-buildpackage
environment2. However, this hack stopped working when the
cc
cratebegan emitting
rerun-if-env-changed
in version1.0.74
3. Hence,would need to set
emit_rerun_if_env_changed(false)
for allcc::Build
instances in
aziot-key-openssl-engine-shared
and its dependency graphto restore our hack for
cc > 1.0.73
. This is not remotely practical.LTO is incompatible with
__asm__(".symver ..")
because it uses thelinker-script-provided exported symbols to determine what is safe to
optimize out, and the linker script is naturally unaware of
manually-exported symbols. We can work around this by adding
__attribute__((used))
to the functions we want to keep in the finalobject.
Another option would be to use GCC's
__attribute__((symver("..@")))
4 directive, but this relies on toonew of a toolchain version (GCC 10).
Addendum 1: The fundamental reason for why LTO is a problem for
aziot-key-openssl-engine-shared
in the first place is that this crateuses what is, in effect, a "symbol stub" to hook Rust code into
OpenSSL's engine macros. First,
build/engine.c
declares the enginefunction signature and uses OpenSSL's macros to expand the dynamic
engine binding. This file is then compiled (but not linked) into an
object that will become the dynamic library's public interface. The way
this is accomplished is by linking the whole object into the
cdylib
(as opposed to only linking referenced functions). LTO requires us to
go one step further by preventing the linker from optimizing out symbols
not declared as globally-exported in
rustc
's linker script, which doesnot know of the symbol declaration in the stub object. There is an open
RFC request for allowing re-export of C symbols from
cdylib
crates:rust-lang/rfcs#2771.
Addendum 2: When our supported platforms start shipping with GNU as >=
2.35 and/or Clang 13, we may want to add
,remove
to the.symver
directive arguments to lift the restriction that
aziot-key-openssl-engine-shared
cannot be included in tests5.Footnotes
https://wiki.ubuntu.com/ToolChain/LTO ↩
https://github.com/Azure/iot-identity-service/commit/f66c155113cce5089d398376d16b206f7973d6f6 ↩
https://github.com/rust-lang/cc-rs/releases/tag/1.0.74 ↩
..@
instead of..@@
since we do not define a versionnode name, meaning
..@@
leads to symbol duplication. ↩https://maskray.me/blog/2020-11-26-all-about-symbol-versioning ↩