proc-macro2 relies on unwinding which -Zno-landing-pads breaks

[llebout]

We are hitting that here:

https://github.com/stjepang/smol/pull/13/checks?check_run_id=618319573#step:4:342

I'm not really sure what's the cause yet. I'll post more details once I have them.

[llebout]

Looks like it may be this:

        // We have stack overflows on Servo's CI.
        let handle = Builder::new()
            .stack_size(128 * 1024 * 1024)
            .spawn(move || {
                match_byte::expand(&input, &output);
            })
            .unwrap();

https://github.com/servo/rust-cssparser/blob/v0.25.9/build.rs#L35

What can we do here?

[SimonSapin]

Is this a stack overflow? I can’t see the log, maybe because the repo is private. Do you have steps to reproduce?

[SimonSapin]

This build script parses an entire module with syn which has a recursive parser whose stack frames are apparently large-ish in debug mode. This plus a deeply-nested parse tree together caused stack overflows before, leading to the larger stack size that you see. But I thought 128 MB was a conservative over-estimate, I’m kinda surprised we’d still overflow that.

[llebout]

@SimonSapin, oh sorry I'm stupid. It's private yes but will be public soon, and it's just a SIGABRT there's nothing more interesting in the log.

What I'm thinking is that the 128MB stack size is over the runner ulimit for a stack, so it crashes when you try to raise it that big.

[llebout]

@SimonSapin

Here's the Github Actions workflow file to reproduce:

name: Code Coverage

on: [push]

jobs:
  grcov:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - uses: actions-rs/toolchain@v1
        with:
          toolchain: nightly
          override: true
      - uses: actions-rs/cargo@v1
        with:
          command: test
          args: --all-features --no-fail-fast
        env:
          CARGO_INCREMENTAL: '0'
          RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Zno-landing-pads'
      - uses: actions-rs/grcov@v0.1

[SimonSapin]

Alright, I can reproduce with RUSTFLAGS='-Zno-landing-pads' cargo +nightly build. The other rust flags appear not to have an impact on this.

     Running `/home/simon/projects/rust-cssparser/target/debug/build/cssparser-fa47ccf6fa2a94f8/build-script-build`
error: failed to run custom build command for `cssparser v0.27.1 (/home/simon/projects/rust-cssparser)`

Caused by:
  process didn't exit successfully: `/home/simon/projects/rust-cssparser/target/debug/build/cssparser-fa47ccf6fa2a94f8/build-script-build` (signal: 6, SIGABRT: process abort signal)
--- stdout
cargo:rustc-cfg=rustc_has_pr45225
cargo:rerun-if-changed=/home/simon/projects/rust-cssparser/src/tokenizer.rs

--- stderr
fatal runtime error: failed to initiate panic, error 5

… and in gdb:

CARGO_MANIFEST_DIR=. OUT_DIR=/tmp/a gdb target/debug/build/cssparser-fa47ccf6fa2a94f8/build-script-build

Here is the top of the stack in gdb:

#0  0x00007ffff7db8ce5 in raise () from /usr/lib/libc.so.6
#1  0x00007ffff7da2857 in abort () from /usr/lib/libc.so.6
#2  0x000055555587fbf7 in std::sys::unix::abort_internal () at src/libstd/sys/unix/mod.rs:167
#3  0x000055555587c995 in std::sys_common::util::abort () at src/libstd/sys_common/util.rs:20
#4  0x000055555587de0e in rust_panic () at src/libstd/panicking.rs:565
#5  0x000055555587dcc5 in std::panicking::rust_panic_with_hook () at src/libstd/panicking.rs:533
#6  0x000055555585567e in std::panicking::begin_panic ()
    at /rustc/3360cc3a0ea33c84d0b0b1163107b1c1acbf2a69/src/libstd/panicking.rs:438
#7  0x000055555585d9c0 in proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::with::{{closure}} ()
    at src/libproc_macro/bridge/client.rs:323
[…]
#17 0x00005555558427fa in proc_macro2::imp::nightly_works::{{closure}}::{{closure}} ()
    at /home/simon/tmp/cargo-home/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.10/src/wrapper.rs:77

• I first problem is that we get a failed to initiate panic, error 5 message instead of the actual panic message.

  Frame 4 is at https://github.com/rust-lang/rust/blob/3360cc3a0/src/libstd/panicking.rs#L565 which is rtabort!("failed to initiate panic, error {}", code) after the __rust_start_panic(obj as usize) call returned. Presumably it’s not supposed to return. Maybe it does when -Zno-landing-pads is used without also using -C panic=abort, or panic = "abort" in Cargo.

• A second problem is that the build script is panicking in the first place.

  Frame 7 is at https://github.com/rust-lang/rust/blob/3360cc3a0/src/libproc_macro/bridge/client.rs#L323 which is panic!("procedural macro API is used outside of a procedural macro"). Indeed we are running a build script not a proc macro.

  Frame 17 is at https://github.com/alexcrichton/proc-macro2/blob/1.0.10/src/wrapper.rs#L77 which is let works = panic::catch_unwind(|| proc_macro::Span::call_site()).is_ok();. It seems to expect this particular panic and try to catch it.

In short, the proc-macro2 crate relies on unwinding, which rely on landing pads.

Why do you need -Zno-landing-pads?

[SimonSapin]

The panic message is not printed because proc-macro2 sets a custom panic hook that does not print: https://github.com/alexcrichton/proc-macro2/blob/1.0.10/src/wrapper.rs#L72-L84

Per https://rust-lang.github.io/rfcs/1513-less-unwinding.html -Zno-landing-pads appears to have been stabilized as -C panic=abort (which also causes libpanic_abort to be linked instead of libpanic_unwind).

Unsurprisingly, building with RUSTFLAGS='-C panic=abort' gives very similar results, except with panic_abort::__rust_start_panic::abort () at src/libpanic_abort/lib.rs:44 near the top of the stack.

However configuring Cargo.toml with:

[profile.dev]
panic = "abort"

… does succeed. This is because https://doc.rust-lang.org/cargo/reference/profiles.html#panic

Tests, benchmarks, build scripts, and proc macros ignore the panic setting. The rustc test harness currently requires unwind behavior.

So… consider doing that instead?

[llebout]

@SimonSapin thanks a lot for the detailed analysis! We will look into using abort, however modifying the Cargo.toml is a bit too much, is there no way to set it with a command line argument? I see you tried and it failed, weird?

[llebout]

And I'm using no-landing-pads because the code coverage tool apparently needs it, I got that Github Actions configuration from: https://github.com/actions-rs/grcov/blob/master/README.md#example-workflow

[llebout]

They say no-landing-pads is required here: mozilla/grcov#249 (comment)

[SimonSapin]

modifying the Cargo.toml is a bit too much

Looks like you also set profile settings in Cargo’s configuration, including .cargo/config in your repo: https://doc.rust-lang.org/cargo/reference/config.html#profile

is there no way to set it with a command line argument? I see you tried and it failed, weird?

Not as far as I know. The command above uses the RUSTFLAGS environment variable which affects all invocations of rustc, but here you want to separate build scripts v.s. "normal" code.

[llebout]

@SimonSapin Hey, so just to follow up: we handled this by not requiring cssparser in the code coverage run. So it's sorted out for us, but you might want to keep this open for later? grcov is a really common tool for code coverage in Rust.

[SimonSapin]

I don’t mind keeping this issue open, but I don’t know if there is any code change we can make in this repository that would help with this issue.

[nuxeh]

Hi, I'm also having this issue, I haven't been able to run grcov on my project for some time because it fails building cssparser with the required compiler flags.

[emilio]

We use proper proc macros now.