From 11ee376258651316909bb1610a1826c00e573d0b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:51:56 +0100 Subject: [PATCH 1/7] Outline assert failure handling --- libbzip2-rs-sys/src/lib.rs | 47 ++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/libbzip2-rs-sys/src/lib.rs b/libbzip2-rs-sys/src/lib.rs index d56a4d077..eaef371b3 100644 --- a/libbzip2-rs-sys/src/lib.rs +++ b/libbzip2-rs-sys/src/lib.rs @@ -84,37 +84,40 @@ pub(crate) use libbzip2_rs_sys_version; // --- assert failure logic macro_rules! assert_h { - ($condition:expr, $errcode:expr) => {{ - #[cfg(feature = "stdio")] + ($condition:expr, $errcode:expr) => { if !$condition { - #[cfg(feature = "std")] - std::eprint!("{}", $crate::AssertFail($errcode)); - #[cfg(feature = "std")] - std::process::exit(3); - - #[cfg(not(feature = "std"))] - panic!("{}", $crate::AssertFail($errcode)); - } - - #[cfg(not(feature = "stdio"))] - if !$condition { - $crate::no_stdio_internal_error($errcode) + $crate::handle_assert_failure($errcode) } - }}; + }; } -#[cfg(not(feature = "stdio"))] -pub(crate) fn no_stdio_internal_error(errcode: c_int) { - extern "C" { - fn bz_internal_error(errcode: c_int); +#[cold] +fn handle_assert_failure(errcode: c_int) -> ! { + #[cfg(feature = "stdio")] + { + #[cfg(feature = "std")] + std::eprint!("{}", AssertFail(errcode)); + #[cfg(feature = "std")] + std::process::exit(3); + + #[cfg(not(feature = "std"))] + panic!("{}", AssertFail(errcode)); } - unsafe { bz_internal_error(errcode) } + #[cfg(not(feature = "stdio"))] + { + extern "C" { + fn bz_internal_error(errcode: c_int); + } + + unsafe { bz_internal_error(errcode) } + loop {} + } } -pub(crate) use assert_h; +use assert_h; -pub(crate) struct AssertFail(i32); +struct AssertFail(i32); impl core::fmt::Display for AssertFail { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { From ffec039fe23cfb54442659241a04807cc1af4dbc Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:55:53 +0100 Subject: [PATCH 2/7] Remove useless default feature from libbzip2-rs-sys The default features includes both c-allocator and (through std) rust-allocator. Rust-allocator wins when both are given, so remove c-allocator from the default features. --- libbzip2-rs-sys/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libbzip2-rs-sys/Cargo.toml b/libbzip2-rs-sys/Cargo.toml index ad15cd317..b4d4229a7 100644 --- a/libbzip2-rs-sys/Cargo.toml +++ b/libbzip2-rs-sys/Cargo.toml @@ -4,7 +4,7 @@ readme = "README.md" edition.workspace = true [features] -default = ["std", "c-allocator", "stdio"] +default = ["std", "stdio"] c-allocator = ["dep:libc"] # use a malloc-based C allocator (rust is picked over c if both are configured) rust-allocator = [] # use the rust global allocator (rust is picked over c if both are configured) std = ["rust-allocator"] From e7d4706f28615cfc19f7909612f3473ec5f17987 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:00:16 +0100 Subject: [PATCH 3/7] Remove a commented out CI job that exists as duplicate of an existing job --- .github/workflows/checks.yaml | 55 ----------------------------------- 1 file changed, 55 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 6316512ec..ac017bd40 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -255,61 +255,6 @@ jobs: # fi # RUST_BACKTRACE=1 cargo fuzz run --no-default-features --features="$features" $target -- -max_total_time=10 # done - # - # link-c-dynamic-library: - # name: dynamic library - # strategy: - # matrix: - # include: - # - target: x86_64-unknown-linux-gnu - # features: - # - '' - # runs-on: ubuntu-latest - # steps: - # - name: Checkout sources - # uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - # with: - # persist-credentials: false - # submodules: true - # - name: Install rust toolchain - # uses: dtolnay/rust-toolchain@be73d7920c329f220ce78e0234b8f96b7ae60248 - # with: - # toolchain: stable - # targets: ${{matrix.target}} - # - name: "cdylib: default settings" - # working-directory: libz-rs-sys-cdylib - # env: - # LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" - # run: | - # cargo build --release --target ${{matrix.target}} - # cc -o zpipe zpipe.c target/${{matrix.target}}/release/deps/libz_rs.so - # ./zpipe < Cargo.toml | ./zpipe -d > out.txt - # cmp -s Cargo.toml out.txt - # - name: "cdylib: rust-allocator" - # env: - # LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" - # working-directory: libz-rs-sys-cdylib - # run: | - # cargo build --release --target ${{matrix.target}} --no-default-features --features="rust-allocator" - # cc -o zpipe zpipe.c target/${{matrix.target}}/release/deps/libz_rs.so - # ./zpipe < Cargo.toml | ./zpipe -d > out.txt - # cmp -s Cargo.toml out.txt - # - name: "cdylib: no_std" - # env: - # LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" - # working-directory: libz-rs-sys-cdylib - # run: | - # cargo build --release --target ${{matrix.target}} --no-default-features - # cc -o zpipe_no_std zpipe_no_std.c target/${{matrix.target}}/release/deps/libz_rs.so - # ./zpipe_no_std < Cargo.toml | ./zpipe_no_std -d > out.txt - # cmp -s Cargo.toml out.txt - # - name: "cdylib: custom-prefix" - # working-directory: libz-rs-sys-cdylib - # env: - # LIBZ_RS_SYS_PREFIX: "MY_CUSTOM_PREFIX_" - # run: | - # cargo build --release --target ${{matrix.target}} --features=custom-prefix - # objdump -tT target/${{matrix.target}}/release/deps/libz_rs.so | grep -q "MY_CUSTOM_PREFIX_uncompress" || (echo "symbol not found!" && exit 1) wasm32: name: "wasm32" From a94677fe41666b88cff74e1acdcc4e7daf4fdcab Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:57:37 +0100 Subject: [PATCH 4/7] Make libbz2-rs-sys-cdylib unconditionally no_std This reduces the size of libbz2_rs.so by 408KB in release mode. --- .github/workflows/checks.yaml | 18 ------------------ libbzip2-rs-sys-cdylib/Cargo.toml | 10 ++++++++-- libbzip2-rs-sys-cdylib/src/lib.rs | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index ac017bd40..197150b0a 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -346,24 +346,6 @@ jobs: cc -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ ./bzpipe < Cargo.toml | ./bzpipe -d > out.txt cmp -s Cargo.toml out.txt - - name: "cdylib: rust-allocator" - env: - LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" - working-directory: libbzip2-rs-sys-cdylib - run: | - cargo build --release --target ${{matrix.target}} --no-default-features --features="rust-allocator" - cc -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ - ./bzpipe < Cargo.toml | ./bzpipe -d > out.txt - cmp -s Cargo.toml out.txt - - name: "cdylib: no_std" - env: - LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" - working-directory: libbzip2-rs-sys-cdylib - run: | - cargo build --release --target ${{matrix.target}} --no-default-features - cc -DNO_STD -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ - ./bzpipe < Cargo.toml | ./bzpipe -d > out.txt - cmp -s Cargo.toml out.txt - name: "staticlib: no stdio" env: LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" diff --git a/libbzip2-rs-sys-cdylib/Cargo.toml b/libbzip2-rs-sys-cdylib/Cargo.toml index af115967e..821b442a0 100644 --- a/libbzip2-rs-sys-cdylib/Cargo.toml +++ b/libbzip2-rs-sys-cdylib/Cargo.toml @@ -15,9 +15,8 @@ name = "bz2_rs" # turns into e.g. `libbz2_rs.so` crate-type=["cdylib"] [features] -default = ["c-allocator", "libbzip2-rs-sys/stdio", "libbzip2-rs-sys/std"] # when used as a cdylib crate, use the c allocator +default = ["c-allocator", "libbzip2-rs-sys/stdio"] # when used as a cdylib crate, use the c allocator c-allocator = ["libbzip2-rs-sys/c-allocator"] # by default, use malloc/free for memory allocation -rust-allocator = ["libbzip2-rs-sys/rust-allocator", "libbzip2-rs-sys/std"] # by default, use the rust global alloctor for memory allocation custom-prefix = ["libbzip2-rs-sys/custom-prefix"] # use the LIBBZIP2_RS_SYS_PREFIX to prefix all exported symbols capi = [] @@ -34,3 +33,10 @@ enabled = false [package.metadata.capi.pkg_config] name = "libbz2_rs" filename = "libbz2_rs" + +# no_std requires panic=abort +[profile.dev] +panic = "abort" + +[profile.release] +panic = "abort" diff --git a/libbzip2-rs-sys-cdylib/src/lib.rs b/libbzip2-rs-sys-cdylib/src/lib.rs index 4c833f413..ea85968f4 100644 --- a/libbzip2-rs-sys-cdylib/src/lib.rs +++ b/libbzip2-rs-sys-cdylib/src/lib.rs @@ -1,3 +1,17 @@ +#![no_std] + extern crate libbzip2_rs_sys; +use core::ffi::c_int; +use core::panic::PanicInfo; pub use libbzip2_rs_sys::*; + +#[panic_handler] +fn panic_handler(_info: &PanicInfo) -> ! { + extern "C" { + fn bz_internal_error(errcode: c_int); + } + + unsafe { bz_internal_error(-1) } + loop {} +} From 1e928483247e4394412281628970947f458b1e24 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:25:10 +0100 Subject: [PATCH 5/7] Better handling of panics and assertions --- libbzip2-rs-sys-cdylib/Cargo.lock | 1 + libbzip2-rs-sys-cdylib/Cargo.toml | 7 ++++-- libbzip2-rs-sys-cdylib/bzpipe.c | 1 + libbzip2-rs-sys-cdylib/src/lib.rs | 41 +++++++++++++++++++++++++++---- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/libbzip2-rs-sys-cdylib/Cargo.lock b/libbzip2-rs-sys-cdylib/Cargo.lock index 2bf3797f9..94b82a2f2 100644 --- a/libbzip2-rs-sys-cdylib/Cargo.lock +++ b/libbzip2-rs-sys-cdylib/Cargo.lock @@ -7,6 +7,7 @@ name = "libbz2-rs-sys-cdylib" version = "0.0.0" dependencies = [ "libbzip2-rs-sys", + "libc", ] [[package]] diff --git a/libbzip2-rs-sys-cdylib/Cargo.toml b/libbzip2-rs-sys-cdylib/Cargo.toml index 821b442a0..9b7f9085f 100644 --- a/libbzip2-rs-sys-cdylib/Cargo.toml +++ b/libbzip2-rs-sys-cdylib/Cargo.toml @@ -12,16 +12,19 @@ rust-version = "1.82" # MSRV [lib] name = "bz2_rs" # turns into e.g. `libbz2_rs.so` -crate-type=["cdylib"] +crate-type = ["cdylib"] +test = false [features] -default = ["c-allocator", "libbzip2-rs-sys/stdio"] # when used as a cdylib crate, use the c allocator +default = ["c-allocator", "stdio"] # when used as a cdylib crate, use the c allocator +stdio = ["libbzip2-rs-sys/stdio"] c-allocator = ["libbzip2-rs-sys/c-allocator"] # by default, use malloc/free for memory allocation custom-prefix = ["libbzip2-rs-sys/custom-prefix"] # use the LIBBZIP2_RS_SYS_PREFIX to prefix all exported symbols capi = [] [dependencies] libbzip2-rs-sys = { version = "0.0.0", path = "../libbzip2-rs-sys", default-features = false } +libc = "0.2" [package.metadata.capi.library] version = "1.0.9" # the bzip2 api version we match diff --git a/libbzip2-rs-sys-cdylib/bzpipe.c b/libbzip2-rs-sys-cdylib/bzpipe.c index 33103904d..54f14b968 100644 --- a/libbzip2-rs-sys-cdylib/bzpipe.c +++ b/libbzip2-rs-sys-cdylib/bzpipe.c @@ -6,6 +6,7 @@ extern void bz_internal_error(int errcode) { fprintf(stderr, "bzip2 hit internal error code: %d\n", errcode); + exit(1); } #ifdef NO_STD diff --git a/libbzip2-rs-sys-cdylib/src/lib.rs b/libbzip2-rs-sys-cdylib/src/lib.rs index ea85968f4..b6c7b0953 100644 --- a/libbzip2-rs-sys-cdylib/src/lib.rs +++ b/libbzip2-rs-sys-cdylib/src/lib.rs @@ -2,16 +2,47 @@ extern crate libbzip2_rs_sys; -use core::ffi::c_int; use core::panic::PanicInfo; pub use libbzip2_rs_sys::*; +#[cfg(feature = "stdio")] +struct StderrWritter; + +#[cfg(feature = "stdio")] +impl core::fmt::Write for StderrWritter { + fn write_str(&mut self, s: &str) -> core::fmt::Result { + use core::ffi::c_void; + use libc::write; + + unsafe { + write(2, s.as_ptr() as *const c_void, s.len()); + } + Ok(()) + } +} + #[panic_handler] fn panic_handler(_info: &PanicInfo) -> ! { - extern "C" { - fn bz_internal_error(errcode: c_int); + #[cfg(feature = "stdio")] + { + use core::fmt::Write; + use libc::exit; + + let _ = StderrWritter.write_str("libbzip2-rs: internal error:\n"); + let _ = StderrWritter.write_fmt(format_args!("{}", _info.message())); + + unsafe { + exit(3); + } } - unsafe { bz_internal_error(-1) } - loop {} + #[cfg(not(feature = "stdio"))] + { + extern "C" { + fn bz_internal_error(errcode: core::ffi::c_int); + } + + unsafe { bz_internal_error(-1) } + loop {} + } } From 885a63609c2ffd54116d01dd8318767a5c481ac6 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:29:05 +0100 Subject: [PATCH 6/7] Don't allow disabling c-allocator support in the cdylib --- .github/workflows/checks.yaml | 2 +- libbzip2-rs-sys-cdylib/Cargo.toml | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 197150b0a..da69976f1 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -351,7 +351,7 @@ jobs: LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" working-directory: libbzip2-rs-sys-cdylib run: | - cargo build --release --target ${{matrix.target}} --no-default-features --features="c-allocator" + cargo build --release --target ${{matrix.target}} --no-default-features cc -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ ./bzpipe < Cargo.toml | ./bzpipe -d > out.txt cmp -s Cargo.toml out.txt diff --git a/libbzip2-rs-sys-cdylib/Cargo.toml b/libbzip2-rs-sys-cdylib/Cargo.toml index 9b7f9085f..ba6484dca 100644 --- a/libbzip2-rs-sys-cdylib/Cargo.toml +++ b/libbzip2-rs-sys-cdylib/Cargo.toml @@ -16,14 +16,13 @@ crate-type = ["cdylib"] test = false [features] -default = ["c-allocator", "stdio"] # when used as a cdylib crate, use the c allocator +default = ["stdio"] stdio = ["libbzip2-rs-sys/stdio"] -c-allocator = ["libbzip2-rs-sys/c-allocator"] # by default, use malloc/free for memory allocation custom-prefix = ["libbzip2-rs-sys/custom-prefix"] # use the LIBBZIP2_RS_SYS_PREFIX to prefix all exported symbols capi = [] [dependencies] -libbzip2-rs-sys = { version = "0.0.0", path = "../libbzip2-rs-sys", default-features = false } +libbzip2-rs-sys = { version = "0.0.0", path = "../libbzip2-rs-sys", default-features = false, features = ["c-allocator"] } libc = "0.2" [package.metadata.capi.library] From 67f62d040639e088a5a0f41e45515f54b9e0299b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:21:31 +0100 Subject: [PATCH 7/7] Add back test for disabling stdio --- .github/workflows/checks.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index da69976f1..7deb9822c 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -346,6 +346,15 @@ jobs: cc -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ ./bzpipe < Cargo.toml | ./bzpipe -d > out.txt cmp -s Cargo.toml out.txt + - name: "cdylib: no stdio" + env: + LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" + working-directory: libbzip2-rs-sys-cdylib + run: | + cargo build --release --target ${{matrix.target}} --no-default-features + cc -DNO_STD -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ + ./bzpipe < Cargo.toml | ./bzpipe -d > out.txt + cmp -s Cargo.toml out.txt - name: "staticlib: no stdio" env: LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps"