Regression: `error: this directive must appear between .cfi_startproc and .cfi_endproc directives`

polarathene commented 1 day ago

I recently started getting failures when doing builds with basic projects using eyra which internally brings in unwinding.

The errors initially were lacking information about the crate producing the error but I've identified it's a recent change to unwinding.

When using nightly to build with -Z build-std=std,panic_abort paired with -C panic=abort I am getting this failure:

cargo init /tmp/example && cd /tmp/example
cargo add unwinding

# Add `extern crate unwinding;` to `src/main.rs`, then build:
RUSTFLAGS='-C panic=abort -C target-feature=+crt-static' cargo +nightly build \
  --release \
  --target x86_64-unknown-linux-gnu \
  -Z build-std=std,panic_abort

   Compiling unwinding v0.2.3
error: this directive must appear between .cfi_startproc and .cfi_endproc directives
  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/unwinding-0.2.3/src/unwinder/arch/x86_64.rs:66:13
   |
66 |             .cfi_def_cfa_offset 0xA0
   |             ^
   |
note: instantiated into assembly here
  --> <inline asm>:4:13
   |
4  |             .cfi_def_cfa_offset 0xA0
   |             ^

error: this directive must appear between .cfi_startproc and .cfi_endproc directives
  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/unwinding-0.2.3/src/unwinder/arch/x86_64.rs:90:13
   |
90 |             .cfi_def_cfa_offset 8
   |             ^
   |
note: instantiated into assembly here
  --> <inline asm>:28:13
   |
28 |             .cfi_def_cfa_offset 8
   |             ^

The failure cause when used indirectly via eyra seemed to not trigger provided I didn't have -C panic=abort or did not do -Z build-std (_which requires =std,panic_abort if abort is set_).

However with unwinding directly I noticed that it would fail like above with only -C panic=abort present:

# Failure as above:
RUSTFLAGS='-C panic=abort' cargo +nightly build --release --target x86_64-unknown-linux-gnu
RUSTFLAGS='-C panic=abort' cargo +nightly build --release --target x86_64-unknown-linux-gnu -Z build-std=std,panic_abort

# Without `-C panic=abort`, this works as expected:
cargo +nightly build --release --target x86_64-unknown-linux-gnu
cargo +nightly build --release --target x86_64-unknown-linux-gnu -Z build-std=std,panic_abort

# With the `panic-handler` feature enabled, same error as above:
cargo add unwinding --features panic-handler
# Previously this would fail for duplicate lang item (panic_impl):
RUSTFLAGS='-C panic=abort' cargo +nightly build --release --target x86_64-unknown-linux-gnu
RUSTFLAGS='-C panic=abort' cargo +nightly build --release --target x86_64-unknown-linux-gnu -Z build-std=std,panic_abort
# Failure duplicate lang item (panic_impl), as expected due to std use:
cargo +nightly build --release --target x86_64-unknown-linux-gnu

Previously it working with unwinding=0.2.2 on nightly before the naked_asm failure was introduced which 0.2.3 fixed and presumably introduced this regression?:

[toolchain]
profile = "minimal"
channel = "nightly-2024-10-04"
components = ["rust-src"]
targets = ["x86_64-unknown-linux-gnu"]

The prior cargo commands for the 0.2.2 release on that nightly all avoid the reported error.

nbdd0121 commented 9 hours ago

I cannot reproduce this

polarathene commented 8 hours ago

I cannot reproduce this

This should provide a fairly good reproduction:

docker run --rm -it --workdir /example fedora:41
dnf install -y nano gcc rustup-init
rustup-init -y --profile minimal --default-toolchain nightly && . "$HOME/.cargo/env"

cargo init
cargo add unwinding
# Add `extern crate unwinding;` at the top:
nano src/main.rs

# Build:
$ RUSTFLAGS='-C panic=abort' cargo +nightly build --release --target x86_64-unknown-linux-gnu

  Downloaded unwinding v0.2.3
  Downloaded gimli v0.31.1
  Downloaded libc v0.2.159
  Downloaded 3 crates (1.1 MB) in 0.35s
   Compiling libc v0.2.159
   Compiling gimli v0.31.1
   Compiling unwinding v0.2.3
error: this directive must appear between .cfi_startproc and .cfi_endproc directives
  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/unwinding-0.2.3/src/unwinder/arch/x86_64.rs:66:13
   |
66 |             .cfi_def_cfa_offset 0xA0
   |             ^
   |
note: instantiated into assembly here
  --> <inline asm>:4:13
   |
4  |             .cfi_def_cfa_offset 0xA0
   |             ^

error: this directive must appear between .cfi_startproc and .cfi_endproc directives
  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/unwinding-0.2.3/src/unwinder/arch/x86_64.rs:90:13
   |
90 |             .cfi_def_cfa_offset 8
   |             ^
   |
note: instantiated into assembly here
  --> <inline asm>:28:13
   |
28 |             .cfi_def_cfa_offset 8
   |             ^

[package]
name = "example"
version = "0.1.0"
edition = "2021"

[dependencies]
unwinding = "0.2.3"

extern crate unwinding;

fn main() {
    println!("Hello, world!");
}

If you cannot reproduce with the above environment, then I suppose it could be an issue with my kernel? Otherwise everything else should be the fairly well isolated within that Docker container?

$ uname -a
Linux 4065fe3fc76f 5.15.123.1-microsoft-standard-WSL2 #1 SMP Mon Aug 7 19:01:48 UTC 2023 x86_64 GNU/Linux

$ rustc --version --verbose

rustc 1.84.0-nightly (798fb83f7 2024-10-16)
binary: rustc
commit-hash: 798fb83f7d24e31b16acca113496f39ff168c143
commit-date: 2024-10-16
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.1

polarathene commented 7 hours ago

The errors do point to two lines from here:

https://github.com/nbdd0121/unwinding/blob/41a5258acc77878f065096a41ba13b4b9180e502/src/unwinder/arch/x86_64.rs#L59-L95

That function was modified for the 0.2.3 release where the regression was introduced.

polarathene commented 7 hours ago

Working fix?

@nbdd0121 I cloned this repo and followed the error output advice to include .cfi_startproc and .cfi_endproc and it was able to compile successfully.

These lines were inserted after and before the respective opening/closing " in the referenced function core::arch::naked_asm!() call above.

Actually I see that a PR isn't available for the 0.2.3 change that introduced the naked_asm! switch, but these .cfi lines were introduced via this PR which was also part of the 0.2.3 release.

Side-note: For some reason you haven't been tagging commits for releases after 0.2.1 btw, so it required scanning over the commit history to realize which release that change belonged to. It's possible that it would have also failed builds before the switch to naked_asm! as a result.

Affects other archs too

I don't know how to easily/properly test for the other archs, but I assume they'd likewise raise that failure if I did have.

I was able to produce the same failure for aarch64 via:

rustup target add aarch64-unknown-linux-gnu
dnf install -y zig
cargo install cargo-zigbuild
RUSTFLAGS='-C panic=abort' cargo +nightly build --release --target aarch64-unknown-linux-gnu

And applying the same change to get a successful build.

I did the same for riscv64gc but Zig failed to compile that for a different reason, something about .cfi_label, but that could be a Zig specific failure (it's not always reliable for builds).

Unsure if correct approach

I also have no idea if that's the right fix for this, as assembly isn't something I have any experience in, so the placement may be incorrect.

No clue why my specific build environment fails to build if you're able to reproduce it with that Docker reproduction, but not in a different build environment 🤷‍♂️ (any context as to what your build environment is?)

I did find this reference from rust-lang/rust tests where they seem to do the same approach? Might be correct after all?

nbdd0121 / unwinding