Skip to content

Commit 7252a1b

Browse files
committed
Prevent LTO from optimizing out OpenSSL engine symbols
LTO is enabled by default for .deb packages starting in Ubuntu 21.04[1], causing build failures on Ubuntu 22.04 when we use GCC ranlib (used by default with `cc` crate version >= 1.0.74[2]). 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("..@")))`[3] 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[4]. [1]: https://wiki.ubuntu.com/ToolChain/LTO [2]: Binutils ranlib was used before, see discussion in rust-lang/cc-rs#735 for full details. [3]: `..@` instead of `..@@` since we do not define a version node name, meaning double-at leads to symbol duplication. [4]: https://maskray.me/blog/2020-11-26-all-about-symbol-versioning
1 parent 3d24d26 commit 7252a1b

File tree

4 files changed

+13
-19
lines changed

4 files changed

+13
-19
lines changed

Makefile

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,14 +310,7 @@ deb: dist
310310
cp -R contrib/debian /tmp/aziot-identity-service-$(PACKAGE_VERSION)/
311311
sed -i -e 's/@version@/$(PACKAGE_VERSION)/g; s/@release@/$(PACKAGE_RELEASE)/g' /tmp/aziot-identity-service-$(PACKAGE_VERSION)/debian/changelog
312312

313-
# Build package
314-
# Note: This builds the `default` target before the normal Debian packaging (instead
315-
# of as part of it) to workaround linker errors on Ubuntu 22.04 when building
316-
# aziot-key-openssl-engine-shared. The extra flags to dpkg-buildpackage are to
317-
# circumvent the default clean and to ignore build artifacts that will already exist.
318-
cd /tmp/aziot-identity-service-$(PACKAGE_VERSION) && \
319-
make RELEASE=1 V=1 ARCH=$(ARCH) && \
320-
dpkg-buildpackage -us -uc $(DPKG_ARCH_FLAGS) -nc -tc -F -I=vendor -I=target --source-option=--extend-diff-ignore="target|vendor|third-party|keys\.generated\.rs"
313+
cd /tmp/aziot-identity-service-$(PACKAGE_VERSION) && dpkg-buildpackage -us -uc $(DPKG_ARCH_FLAGS)
321314

322315
# rpm
323316
#

ci/install-build-deps.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ case "$OS:$ARCH" in
1616

1717
yum install -y centos-release-scl epel-release
1818
yum install -y \
19-
autoconf autoconf-archive automake clang curl devtoolset-9-gcc devtoolset-9-gcc-c++ \
19+
autoconf autoconf-archive automake curl devtoolset-9-gcc devtoolset-9-gcc-c++ \
2020
git jq libcurl-devel libtool llvm-toolset-7-clang llvm-toolset-7-llvm-devel \
2121
make openssl openssl-devel pkgconfig
2222

key/aziot-key-openssl-engine-shared/build/engine.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,19 @@ int aziot_key_openssl_engine_shared_bind(ENGINE* e, const char* id);
1414
* So the bind_engine and v_check functions generated by these two macros will not actually be exported by the final cdylib. It's not possible
1515
* to provide a custom version script via `cargo:rustc-cdylib-link-arg` because it conflicts with the one generated by rustc.
1616
*
17-
* So we have to use the other way mentioned by `ld`'s docs, `__asm__(".symver")`
17+
* So we have to use the other way mentioned by `ld`'s docs[1], `__asm__(".symver")`. We also need to add `__attribute__((used))` since otherwise
18+
* the versioned symbol is optimized out by LTO.
1819
*
1920
* Unfortunately this trick means this crate cannot be compiled for tests since the linker will see duplicate symbols, so every invocation of
20-
* `cargo test --all` or `cargo clippy --tests --all` has to also exclude this crate with `--exclude`.
21+
* `cargo test --all` or `cargo clippy --tests --all` has to also exclude this crate with `--exclude`. When all of our platforms provide
22+
* GNU as >= 2.35 and/or Clang >= 13, we can lift this restriction by appending `,remove` to the `.symver` directive arguments[2].
23+
*
24+
* [1]: https://sourceware.org/binutils/docs/ld/VERSION.html
25+
* [2]: https://maskray.me/blog/2020-11-26-all-about-symbol-versioning
2126
*/
27+
__attribute__((used))
2228
IMPLEMENT_DYNAMIC_BIND_FN(aziot_key_openssl_engine_shared_bind);
2329
__asm__(".symver bind_engine,bind_engine@@");
30+
__attribute__((used))
2431
IMPLEMENT_DYNAMIC_CHECK_FN();
2532
__asm__(".symver v_check,v_check@@");

key/aziot-key-openssl-engine-shared/build/main.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#![warn(clippy::all, clippy::pedantic)]
55

66
fn main() {
7-
println!("cargo:rerun-if-changed=build/engine.c");
7+
println!("cargo:rerun-if-changed=build/");
88

99
openssl_build::define_version_number_cfg();
1010

@@ -13,13 +13,7 @@ fn main() {
1313
// Since we are going to use the generated archive in a shared
1414
// library, we need +whole-archive to be set. See:
1515
// https://github.com/rust-lang/rust/blob/1.61.0/RELEASES.md#compatibility-notes
16-
.cargo_metadata(false)
16+
.link_lib_modifier("+whole-archive")
1717
.file("build/engine.c")
1818
.compile("aziot_key_openssl_shared_engine_wrapper");
19-
20-
println!("cargo:rustc-link-lib=static:+whole-archive=aziot_key_openssl_shared_engine_wrapper");
21-
println!(
22-
"cargo:rustc-link-search=native={}",
23-
std::env::var("OUT_DIR").unwrap()
24-
);
2519
}

0 commit comments

Comments
 (0)