Skip to content

Commit 99d5622

Browse files
committed
chore: self-review misc fixes
1 parent 10d5e17 commit 99d5622

3 files changed

Lines changed: 13 additions & 13 deletions

File tree

libdd-otel-thread-ctx-ffi/README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,16 @@ inline the C wrapper at link time. The requirements are:
2121
- `clang` is available to compile the C shim to LLVM IR (version requirements
2222
aren't clear -- tested with clang18 and clang20, but ideally the version
2323
should be the same or close to the LLVM version shipped with `rustc`)
24-
- Either the Rust toolchain ships lld or there's a system-wide lld install
24+
- Either the Rust toolchain ships `lld` or there's a system-wide `lld` install
2525
(Rust ships `rust-lld` for a long time, something like since 1.53+, however
26-
some musl-based distro like Alpine might have Rust packages without LLD)
27-
- lld version is at least 19 (TLSDESC support)
26+
some musl-based distro like Alpine might have the Rust toolchain without LLD)
27+
- `lld` version is at least 19 (TLSDESC support)
2828

2929
If those requirements are met, you can use the small wrapper script provided in
3030
this directory to build an optimized release version where the C shim is
3131
inlined. A wrapper script is needed because cross-language LTO requires two
3232
`rustc` codegen flags (`-Clinker-plugin-lto` and `-Clinker=clang`) that cannot
33-
be set from a Cargo build script they must come from `RUSTFLAGS` or
33+
be set from a Cargo build script: they must come from `RUSTFLAGS` or
3434
`.cargo/config.toml`. The script sets them via the target-scoped
3535
`CARGO_TARGET_<TRIPLE>_RUSTFLAGS` env var so they don't leak to build scripts
3636
or proc-macros.

libdd-otel-thread-ctx-ffi/build-optimized.sh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# on every TLS access.
88
#
99
# Requirements: clang, lld (rust-lld from the toolchain is used automatically).
10+
# The requirements are checked by the build.rs script.
1011
#
1112
# Usage:
1213
# ./build-optimized.sh # auto-detect host triple
@@ -44,9 +45,9 @@ cargo build --release \
4445
-p libdd-otel-thread-ctx-ffi \
4546
"${EXTRA_ARGS[@]}"
4647

47-
# Sanity-check that the C shim was actually inlined.
48+
# Sanity-check that the C shim was actually inlined, if `nm` is available.
4849
if ! command -v nm &>/dev/null; then
49-
echo >&2 "WARNING: nm not found — skipping sanity check that the C TLS shim was inlined."
50+
echo >&2 "WARNING: skipping sanity check that the C TLS shim was inlined (\`nm\` not found)"
5051
else
5152
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
5253
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"

libdd-otel-thread-ctx-ffi/build.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
extern crate build_common;
44

55
use build_common::generate_and_configure_header;
6-
use std::env;
7-
use std::path::PathBuf;
8-
use std::process::Command;
6+
use std::{env, path::PathBuf, process::Command};
97

108
/// Locate the `gcc-ld/` shim directory shipped with the Rust toolchain.
119
///
@@ -86,22 +84,23 @@ fn main() {
8684

8785
println!("cargo:rerun-if-env-changed=LIBDD_OTEL_THREAD_CTX_INLINE");
8886

89-
let inline_mode = env::var_os("LIBDD_OTEL_THREAD_CTX_INLINE").is_some();
87+
let inline_mode = env::var("LIBDD_OTEL_THREAD_CTX_INLINE").unwrap();
9088
let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
9189
let target_arch = env::var("CARGO_CFG_TARGET_ARCH").unwrap();
9290

93-
if inline_mode {
91+
if &inline_mode == "1" {
9492
let rust_lld_dir = require_lld_for_inline(&target_arch);
9593

9694
// Emit link args for ALL link types (not just cdylib) so that test
97-
// binaries also link correctly when RUSTFLAGS sets clang as the linker.
95+
// binaries also link correctly when RUSTFLAGS sets clang as the linker (although we should
96+
// only build the shared object file in inline mode).
9897
if let Some(dir) = rust_lld_dir {
9998
println!("cargo:rustc-link-arg=-B{}", dir.display());
10099
}
101100
println!("cargo:rustc-link-arg=-fuse-ld=lld");
102101

103102
// On x86-64, tell the LLVM backend to use TLSDESC during LTO codegen.
104-
// On aarch64 TLSDESC is the only model, so no flag is needed.
103+
// On aarch64 TLSDESC is the default and the only model.
105104
if target_arch == "x86_64" {
106105
println!("cargo:rustc-link-arg=-Wl,-mllvm,-enable-tlsdesc");
107106
}

0 commit comments

Comments
 (0)