diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 94ebd9b2e..8b4e2c511 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -264,6 +264,25 @@ jobs: command: sdist args: --out dist + # Audit each Linux wheel's dynamic symbol references against its + # declared manylinux glibc floor. Catches the bug class from + # issue #355 (#386): a manylinux_2_28 wheel that ends up + # referencing `__isoc23_strtoll` (glibc 2.38+) because a + # statically-linked C++ dep was compiled with a recent toolchain. + # The build host has glibc 2.38 so the link succeeds and CI + # passes — but every customer with libc < 2.38 sees + # `ImportError: undefined symbol: __isoc23_strtoll` at + # `import headroom._core`. The script compares every UND symbol + # against the wheel's manylinux tag and fails the release if + # any symbol exceeds the floor. + - name: Audit wheel glibc symbols (Linux only) + if: startsWith(matrix.target, 'x86_64-unknown-linux') || startsWith(matrix.target, 'aarch64-unknown-linux') + run: | + set -e + for whl in dist/*.whl; do + python3 scripts/audit_wheel_glibc_symbols.py "$whl" + done + - name: Upload wheels artifact uses: actions/upload-artifact@v4 with: diff --git a/.gitignore b/.gitignore index 42a73b1c7..09075e8b5 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ scripts/* !scripts/install-git-hooks.sh !scripts/smoke_issue_327.py !scripts/refresh_model_limits.sh +!scripts/audit_wheel_glibc_symbols.py # Rust / Cargo build artifacts /target/ diff --git a/Cargo.lock b/Cargo.lock index ae1d75c38..d871bf240 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1904,6 +1904,7 @@ dependencies = [ name = "headroom-py" version = "0.1.0" dependencies = [ + "cc", "headroom-core", "pyo3", "serde_json", diff --git a/crates/headroom-py/Cargo.toml b/crates/headroom-py/Cargo.toml index f509213ad..d23eeb7c8 100644 --- a/crates/headroom-py/Cargo.toml +++ b/crates/headroom-py/Cargo.toml @@ -6,6 +6,16 @@ rust-version.workspace = true license.workspace = true repository.workspace = true description = "PyO3 bindings exposing headroom-core to Python as headroom._core." +# Issue #355: build.rs compiles a glibc-2.38-compat shim +# (glibc_compat.c) on Linux/glibc only. Without `build = "build.rs"` +# Cargo skips it. See the script's header comment for rationale. +build = "build.rs" + +[build-dependencies] +# Compile the C shim into a static library that links into _core.so. +# `cc` is already transitively in our build graph (used by aws-lc-rs, +# zstd-sys, ring, etc) so this is not a new cold dep. +cc = "1" [lib] name = "_core" diff --git a/crates/headroom-py/build.rs b/crates/headroom-py/build.rs new file mode 100644 index 000000000..1b6ee860e --- /dev/null +++ b/crates/headroom-py/build.rs @@ -0,0 +1,40 @@ +// Compile a tiny C shim that provides weak aliases for the C23 +// strtol family (`__isoc23_strtol`, `__isoc23_strtoll`, etc.). +// +// glibc < 2.38 doesn't ship these symbols. Static dependencies in +// `_core.so` (notably the prebuilt ONNX Runtime artifacts compiled +// with gcc 14.x) reference them, so without this shim the wheel +// fails to import on Ubuntu 22.04, Conda envs with libc 2.35, etc. +// +// See `glibc_compat.c` for the full background. The shim is +// Linux-only — macOS and Windows don't ship glibc. +// +// Issue: #355 (https://github.com/chopratejas/headroom/issues/355) + +fn main() { + println!("cargo:rerun-if-changed=glibc_compat.c"); + println!("cargo:rerun-if-changed=build.rs"); + + // The shim is glibc-specific. Skip on every other target: macOS + // uses Darwin libc, Windows has MSVCRT, musl handles strtoll + // identically and never emits __isoc23_*. + let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default(); + let target_env = std::env::var("CARGO_CFG_TARGET_ENV").unwrap_or_default(); + if target_os != "linux" || target_env != "gnu" { + return; + } + + cc::Build::new() + .file("glibc_compat.c") + // -fPIC because we link into a cdylib. -O2 for size — the + // file is ~10 lines but every byte counts in a wheel that's + // already 35 MiB. + .flag_if_supported("-fPIC") + .opt_level(2) + // Suppress GCC's "weak attribute, alias to symbol that may + // not exist" warning. `strtoll` etc. always exist; the + // weak attribute is for OUR `__isoc23_*` definitions, not + // for the alias targets. + .flag_if_supported("-Wno-attribute-alias") + .compile("headroom_glibc_compat"); +} diff --git a/crates/headroom-py/glibc_compat.c b/crates/headroom-py/glibc_compat.c new file mode 100644 index 000000000..eb7075478 --- /dev/null +++ b/crates/headroom-py/glibc_compat.c @@ -0,0 +1,68 @@ +/* + * glibc < 2.38 compatibility shim for the C23 strtol* family. + * + * Why this file exists + * -------------------- + * + * glibc 2.38 (Aug 2023) added `__isoc23_strtol`, `__isoc23_strtoll`, + * `__isoc23_strtoul`, and `__isoc23_strtoull` as canonical C23 + * implementations of strtol*. When you compile C/C++ code with a + * recent toolchain (gcc >= 13) and the headers see C23/C++23 mode + * (or `_GNU_SOURCE`), `` redirects every call to + * `strtoll(...)` to `__isoc23_strtoll(...)` via a transparent inline. + * + * The ONNX Runtime prebuilt artifacts that we statically link + * (downloaded by `ort-download-binaries-rustls-tls` via fastembed) + * are compiled with gcc-14.2.1 on a glibc >= 2.38 toolchain. They + * therefore reference `__isoc23_*` symbols. Our wheel build runs + * in `manylinux_2_28` (glibc 2.28 baseline) but the linker doesn't + * complain because these are deferred (DT_NEEDED-style) symbols + * resolved at runtime — and the manylinux_2_28 host's glibc has + * them, so the link-time check passes. + * + * On the END USER's runtime, however, glibc < 2.38 has none of + * these symbols, and `import headroom._core` fails with: + * + * ImportError: undefined symbol: __isoc23_strtoll + * + * (Reported in issue #355; first hit by users on Ubuntu 22.04 + + * Conda Python 3.12 environments where libc.so.6 is glibc 2.35.) + * + * The fix + * ------- + * + * Define the four `__isoc23_*` symbols as weak aliases for the + * older `strtoll` family. The dynamic linker resolves symbols in + * library load order: when glibc has the strong symbol (>=2.38) + * its definition wins; when it doesn't (<2.38) our weak symbol + * is used. Either way, `_core.so` loads. + * + * The C23 vs pre-C23 behavioural difference is binary-literal + * support: `__isoc23_strtoll` accepts strings like "0b1010" + * whereas `strtoll` returns 0 for those. Our static-link callsites + * (deep inside ORT's protobuf parsers) only pass decimal / + * hexadecimal numerics, so the fallback is functionally identical. + * + * References: + * - https://sourceware.org/glibc/wiki/Release/2.38 + * - https://github.com/pypa/manylinux/issues/1725 + * - issue #355: tests/test_rust_core_smoke.py was the canary that + * surfaced this on user installs. + * + * This file is compiled and linked into `_core.so` only on Linux + * (gated in build.rs). macOS and Windows have neither glibc nor + * this symbol family. + */ + +#include + +#define ALIAS_TO(target) \ + __attribute__((weak, alias(#target))) + +long __isoc23_strtol(const char *nptr, char **endptr, int base) ALIAS_TO(strtol); + +long long __isoc23_strtoll(const char *nptr, char **endptr, int base) ALIAS_TO(strtoll); + +unsigned long __isoc23_strtoul(const char *nptr, char **endptr, int base) ALIAS_TO(strtoul); + +unsigned long long __isoc23_strtoull(const char *nptr, char **endptr, int base) ALIAS_TO(strtoull); diff --git a/scripts/audit_wheel_glibc_symbols.py b/scripts/audit_wheel_glibc_symbols.py new file mode 100755 index 000000000..5b6c27ec6 --- /dev/null +++ b/scripts/audit_wheel_glibc_symbols.py @@ -0,0 +1,202 @@ +#!/usr/bin/env python3 +"""Audit a manylinux Python wheel for glibc symbol references that exceed +the wheel's declared manylinux ABI floor. + +This catches the bug class from issue #355: a wheel tagged +`manylinux_2_28_x86_64` whose `_core.so` references `__isoc23_strtoll` +(introduced in glibc 2.38) — the link succeeds because the manylinux +build host has 2.38+, but every end user with glibc < 2.38 sees an +`ImportError: undefined symbol` at `import headroom._core`. + +How it works +------------ + +1. Parse the wheel filename to extract the manylinux tag + (`manylinux_2_28` → glibc floor 2.28). +2. Run `objdump -T` (or `nm -D`) on every `.so` inside the wheel. +3. For every `UND` symbol, check whether it's allowed at the glibc + floor. Allowed: + - Symbols with a `GLIBC_x.y` version tag where `x.y <= floor`. + - Symbols defined by us locally (i.e. NOT marked `UND`). +4. Reject any `UND` symbol that's: + - Tagged with a GLIBC version > floor. + - Or in the `__isoc23_*` family (no version tag, but introduced + in glibc 2.38 — special-cased). + - Or in any other known "introduced after floor" symbol list + (extensible). + +Exit 0 = wheel is portable. Exit 1 = wheel will break on some users. + +Usage +----- + + scripts/audit_wheel_glibc_symbols.py path/to/headroom_ai-*.whl + +Run on Linux only — `objdump` from binutils is the audit tool. macOS's +default `objdump` is llvm-objdump, also works on ELF. +""" + +from __future__ import annotations + +import argparse +import re +import shutil +import subprocess +import sys +import tempfile +import zipfile +from pathlib import Path + +# Symbol families introduced after specific glibc versions, beyond what +# `auditwheel` already checks. Add here as new bug classes surface. +# +# Each entry is `(symbol_name_prefix, min_glibc_version_introduced, justification_url)`. +POST_FLOOR_SYMBOLS = [ + ( + "__isoc23_", + (2, 38), + "https://sourceware.org/glibc/wiki/Release/2.38", + ), +] + + +def parse_manylinux_floor(wheel_filename: str) -> tuple[int, int] | None: + """Extract glibc floor from a manylinux wheel filename. + + `headroom_ai-0.20.26-cp312-cp312-manylinux_2_28_x86_64.whl` → (2, 28). + Returns None for non-manylinux wheels (macOS, Windows, sdist). + """ + m = re.search(r"manylinux_(\d+)_(\d+)_", wheel_filename) + if m: + return (int(m.group(1)), int(m.group(2))) + # `manylinux2014_x86_64` is the legacy alias for manylinux_2_17. + if "manylinux2014_" in wheel_filename: + return (2, 17) + if "manylinux1_" in wheel_filename: + return (2, 5) + return None + + +def list_undef_symbols(so_path: Path) -> list[tuple[str, str]]: + """Return [(symbol_name, glibc_version_or_empty), ...] for every + UND (undefined) dynamic symbol in `so_path`. + """ + objdump = shutil.which("objdump") or shutil.which("llvm-objdump") + if not objdump: + raise RuntimeError( + "neither `objdump` nor `llvm-objdump` is on PATH; " + "install binutils (Linux) or LLVM (macOS) to run this audit" + ) + out = subprocess.run( + [objdump, "-T", str(so_path)], + check=True, + capture_output=True, + text=True, + ).stdout + found = [] + for line in out.splitlines(): + # `objdump -T` lines: `address SECTION ... NAME` where SECTION + # contains `*UND*` for undefined references and a versioned + # symbol name like `__isoc23_strtoll@GLIBC_2.38` or unversioned. + if "*UND*" not in line: + continue + # The last whitespace-separated token is the (versioned) symbol. + token = line.split()[-1] + if "@" in token: + name, _, ver = token.partition("@") + # `@@` indicates the default version; strip the second `@`. + ver = ver.lstrip("@") + else: + name, ver = token, "" + found.append((name, ver)) + return found + + +def glibc_version_from_token(ver: str) -> tuple[int, int] | None: + """`GLIBC_2.28` → (2, 28). Returns None for non-GLIBC tokens.""" + m = re.match(r"^GLIBC_(\d+)\.(\d+)", ver) + if m: + return (int(m.group(1)), int(m.group(2))) + return None + + +def audit_so(so_path: Path, floor: tuple[int, int]) -> list[str]: + """Return a list of human-readable violation strings.""" + violations: list[str] = [] + for name, ver in list_undef_symbols(so_path): + v = glibc_version_from_token(ver) + if v is not None and v > floor: + violations.append( + f" {name}@{ver} requires glibc {v[0]}.{v[1]} > floor {floor[0]}.{floor[1]}" + ) + continue + # Versionless symbols: cross-check the post-floor list. + for prefix, introduced, url in POST_FLOOR_SYMBOLS: + if name.startswith(prefix) and introduced > floor: + violations.append( + f" {name} (no version tag, introduced in glibc " + f"{introduced[0]}.{introduced[1]} > floor " + f"{floor[0]}.{floor[1]} — see {url})" + ) + break + return violations + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("wheel", type=Path, help="path to the .whl to audit") + args = parser.parse_args() + + wheel: Path = args.wheel + if not wheel.exists(): + print(f"ERROR: wheel not found: {wheel}", file=sys.stderr) + return 1 + + floor = parse_manylinux_floor(wheel.name) + if floor is None: + print( + f"OK: {wheel.name} is not a manylinux wheel; nothing to audit " + "(macOS / Windows / sdist run on a different runtime ABI)." + ) + return 0 + + print(f"Auditing {wheel.name} (glibc floor: {floor[0]}.{floor[1]})") + with tempfile.TemporaryDirectory() as td: + td_path = Path(td) + with zipfile.ZipFile(wheel) as zf: + so_members = [m for m in zf.namelist() if m.endswith(".so")] + if not so_members: + print( + f"WARN: {wheel.name} contains no .so files; " + "nothing to audit (pure-Python wheel?)" + ) + return 0 + zf.extractall(td_path, members=so_members) + + all_violations: list[tuple[str, list[str]]] = [] + for so_rel in so_members: + so_path = td_path / so_rel + violations = audit_so(so_path, floor) + if violations: + all_violations.append((so_rel, violations)) + + if not all_violations: + print(f"OK: all .so files in {wheel.name} are within glibc {floor[0]}.{floor[1]}.") + return 0 + + print(f"\nFAIL: {wheel.name} references symbols above its glibc floor:") + for so_name, viols in all_violations: + print(f"\n {so_name}:") + for v in viols: + print(f" {v}") + print( + "\nThis wheel will fail to import on end-user systems with the " + "older glibc. Fix the build (or add a compat shim) before " + "publishing to PyPI. See issue #355 for the canonical example " + "and `crates/headroom-py/glibc_compat.c` for the shim pattern." + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_release_workflows.py b/tests/test_release_workflows.py index f6080f482..91230ab25 100644 --- a/tests/test_release_workflows.py +++ b/tests/test_release_workflows.py @@ -487,6 +487,86 @@ def test_sdist_build_conditional_keyed_on_target_not_os() -> None: ) +def test_glibc_compat_shim_present_in_headroom_py() -> None: + """STRUCTURAL INVARIANT: the headroom-py crate ships a glibc-2.38 + compatibility shim that defines weak `__isoc23_*` aliases. + + Issue #355 (https://github.com/chopratejas/headroom/issues/355) — + the published wheel's `_core.so` references `__isoc23_strtoll` + (glibc 2.38+) because we statically link prebuilt ONNX Runtime + artifacts compiled with gcc 14. Users with libc < 2.38 (Ubuntu + 22.04, most Conda envs, Debian 11/12) hit: + + ImportError: undefined symbol: __isoc23_strtoll + + The fix is `crates/headroom-py/glibc_compat.c` which provides + weak-alias definitions for the four `__isoc23_*` symbols, + delegating to the older `strtol*` family. `build.rs` compiles + the shim into `_core.so` on Linux/glibc only. + + A future "let me drop this weird C file, surely it's dead code" + refactor would silently re-introduce the import failure for + every user on glibc < 2.38. This test pins all three load-bearing + pieces (the .c file, the build.rs trigger, the [build-dependencies] + cc dep). + """ + headroom_py_dir = ROOT / "crates" / "headroom-py" + + shim = headroom_py_dir / "glibc_compat.c" + assert shim.exists(), ( + "crates/headroom-py/glibc_compat.c is missing — without it, " + "`_core.so` fails to import on every glibc < 2.38 host. See " + "issue #355 for the full bug class. NEVER delete this file " + "without confirming via `scripts/audit_wheel_glibc_symbols.py` " + "that the wheel no longer references __isoc23_* symbols." + ) + shim_content = shim.read_text(encoding="utf-8") + for sym in ("__isoc23_strtol", "__isoc23_strtoll", "__isoc23_strtoul", "__isoc23_strtoull"): + assert sym in shim_content, f"shim missing alias for {sym}" + + build_rs = headroom_py_dir / "build.rs" + assert build_rs.exists(), "crates/headroom-py/build.rs is missing" + build_rs_content = build_rs.read_text(encoding="utf-8") + assert "glibc_compat.c" in build_rs_content, ( + "build.rs must reference glibc_compat.c — otherwise Cargo " + "skips the shim and the wheel's `_core.so` ships without it." + ) + + cargo_toml = (headroom_py_dir / "Cargo.toml").read_text(encoding="utf-8") + assert 'build = "build.rs"' in cargo_toml, ( + 'headroom-py/Cargo.toml must declare `build = "build.rs"` — ' + "Cargo only auto-detects build.rs when this is set; without " + "it, the shim never compiles." + ) + assert "[build-dependencies]" in cargo_toml and 'cc = "1"' in cargo_toml, ( + 'headroom-py/Cargo.toml must declare `cc = "1"` in ' + "[build-dependencies] for build.rs to compile the C shim." + ) + + +def test_release_workflow_audits_wheel_glibc_symbols() -> None: + """STRUCTURAL INVARIANT: the release workflow audits each Linux + wheel for symbol references that exceed its manylinux glibc floor. + + Companion to `test_glibc_compat_shim_present_in_headroom_py` — + the shim is the FIX, this audit is the GATE. Without the audit, + a future toolchain bump in the prebuilt ORT artifacts (or any + other statically-linked C/C++ dep) could re-introduce a + post-floor symbol that our current shim doesn't cover. The audit + catches that at release time, before publish-pypi. + """ + content = (ROOT / ".github" / "workflows" / "release.yml").read_text(encoding="utf-8") + + assert "audit_wheel_glibc_symbols.py" in content, ( + "release.yml must invoke `scripts/audit_wheel_glibc_symbols.py` " + "on every Linux wheel before publish. Without it, regressions " + "of issue #355's bug class ship to PyPI silently." + ) + assert "Audit wheel glibc symbols (Linux only)" in content, ( + "audit step name has been renamed; update both this test and the workflow" + ) + + def test_npm_publish_jobs_do_not_download_dist_artifact() -> None: """`publish-npm` and `publish-github-packages` `npm pack`+`npm publish` directly from the checked-out source tree; they never read the