Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cxxbridge doesn't run with panic=abort #130

Closed
adetaylor opened this issue Apr 20, 2020 · 3 comments · Fixed by #184
Closed

cxxbridge doesn't run with panic=abort #130

adetaylor opened this issue Apr 20, 2020 · 3 comments · Fixed by #184
Labels
integration How cxx fits into the big picture of an organization's codebase and builds

Comments

@adetaylor
Copy link
Collaborator

adetaylor commented Apr 20, 2020

This issue is not a bug; it is working as designed; and the issue should be closed. I'm writing it up here for the benefit of others who come across it.

Our build system provides -Cpanic=abort as flags to rustc in order to minimize binary size. As it happens we can guarantee that the various use-cases we're using on the target devices should not panic, and it's OK to abort if they do.

Unfortunately, that means we're building host-side tools using this setting as well. This is obviously not ideal, and this is what we need to fix.

Meanwhile, though, this means we cannot use cxxbridge:

lldb -- cxxbridge src.rs

gives

* thread #1, name = 'cxxbridge', stop reason = signal SIGABRT
  * frame #0: 0x00007ffff7df5f61 libc.so.6`__GI_raise(sig=2) at raise.c:51:1
    frame #1: 0x00007ffff7de1535 libc.so.6`__GI_abort at abort.c:79:7
    frame #2: 0x00005555556e6187 cxxbridge`panic_abort::__rust_start_panic::abort::h960e4a7803a480ad at lib.rs:44:9
    frame #3: 0x00005555556e6176 cxxbridge`__rust_start_panic at lib.rs:40:5
    frame #4: 0x00005555556e49b9 cxxbridge`rust_panic at panicking.rs:563:9
    frame #5: 0x00005555556e48c5 cxxbridge`std::panicking::rust_panic_with_hook::h3e147bda54969767 at panicking.rs:533:5
    frame #6: 0x00005555556b280e cxxbridge`std::panicking::begin_panic::h3bd0e88915da2a52 at panicking.rs:438:5
    frame #7: 0x00005555556b9ed0 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::client::_$LT$impl$u20$proc_macro..bridge..Bridge$GT$::with::_$u7b$$u7b$closure$u7d$$u7d$::hef27fa8a2f307990 at client.rs:316:17
    frame #8: 0x00005555556b9eb8 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::client::BridgeState::with::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h8cee55c2f64a576f at client.rs:286
    frame #9: 0x00005555556b9eb8 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::scoped_cell::ScopedCell$LT$T$GT$::replace::h4ad9de4a43aaff68 at scoped_cell.rs:74
    frame #10: 0x00005555556b9e8f cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::client::BridgeState::with::_$u7b$$u7b$closure$u7d$$u7d$::h514a0983f50d69c1 at client.rs:284
    frame #11: 0x00005555556b9e8f cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] std::thread::local::LocalKey$LT$T$GT$::try_with::he8b2e61f6c718460 at local.rs:262
    frame #12: 0x00005555556b9e8f cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] std::thread::local::LocalKey$LT$T$GT$::with::h50a55dfbdd5003c6 at local.rs:239
    frame #13: 0x00005555556b9e57 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::client::BridgeState::with::hcfaaee3b07185c29 at client.rs:283
    frame #14: 0x00005555556b9e57 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::client::_$LT$impl$u20$proc_macro..bridge..Bridge$GT$::with::h368bdd1197c10f7a at client.rs:314
    frame #15: 0x00005555556b9e57 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 [inlined] proc_macro::bridge::client::Span::call_site::h77c318776cb9a93c at client.rs:230
    frame #16: 0x00005555556b9e57 cxxbridge`proc_macro::Span::call_site::h5d43bfafa0f20648 at lib.rs:282
    frame #17: 0x00005555556aeb86 cxxbridge`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h8e30859ac2c38d8c [inlined] proc_macro2::imp::nightly_works::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h6d01106f1c5a7bd8 at wrapper.rs:77:44
    frame #18: 0x00005555556aeb80 cxxbridge`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h8e30859ac2c38d8c at panicking.rs:331
    frame #19: 0x00005555556aeb80 cxxbridge`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h8e30859ac2c38d8c at panicking.rs:274
    frame #20: 0x00005555556aeb80 cxxbridge`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h8e30859ac2c38d8c [inlined] std::panic::catch_unwind::hb5639680e697ce71 at panic.rs:394
    frame #21: 0x00005555556aeb80 cxxbridge`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h8e30859ac2c38d8c at wrapper.rs:77
    frame #22: 0x00005555556aeb59 cxxbridge`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h8e30859ac2c38d8c((null)=<unavailable>) at once.rs:264
    frame #23: 0x00005555556e13d8 cxxbridge`std::sync::once::Once::call_inner::h9b20529cf495b858 at once.rs:416:21
    frame #24: 0x00005555556aeb41 cxxbridge`std::sync::once::Once::call_once::hac51e1f1a526d042(self=<unavailable>, f=<unavailable>) at once.rs:264:9
    frame #25: 0x00005555556af224 cxxbridge`_$LT$proc_macro2..imp..TokenStream$u20$as$u20$core..str..FromStr$GT$::from_str::h11638729cffb2651 [inlined] proc_macro2::imp::nightly_works::hb6a7a378cf2afbd6 at wrapper.rs:69:5
    frame #26: 0x00005555556af20c cxxbridge`_$LT$proc_macro2..imp..TokenStream$u20$as$u20$core..str..FromStr$GT$::from_str::h11638729cffb2651(src=(data_ptr = "<snip>", length = 29169)) at wrapper.rs:150
    frame #27: 0x00005555556b0906 cxxbridge`_$LT$proc_macro2..TokenStream$u20$as$u20$core..str..FromStr$GT$::from_str::h8e11c987fa41028b [inlined] core::str::_$LT$impl$u20$str$GT$::parse::hef701df18d98369f(self=<unavailable>) at mod.rs:4201:9
    frame #28: 0x00005555556b08fd cxxbridge`_$LT$proc_macro2..TokenStream$u20$as$u20$core..str..FromStr$GT$::from_str::h8e11c987fa41028b(src=<unavailable>) at lib.rs:174
    frame #29: 0x000055555568b629 cxxbridge`syn::parse::Parser::parse_str::hc475de4a4e6668e7(self=<unavailable>, s=<unavailable>) at parse.rs:1115:21
    frame #30: 0x000055555568c7a1 cxxbridge`syn::parse_file::hbeb700c0876a8080 [inlined] syn::parse_str::h5d2a0e83d223d885(s=(data_ptr = "<snip>", length = 29169)) at lib.rs:946
    frame #32: 0x00005555555f1daf cxxbridge`cxxbridge::gen::generate::h64aa9d65ae0f6752 [inlined] cxxbridge::gen::generate::_$u7b$$u7b$closure$u7d$$u7d$::h8dc78f808c1fe8d8 at mod.rs:62:22
    frame #33: 0x00005555555f1d9d cxxbridge`cxxbridge::gen::generate::h64aa9d65ae0f6752(path=0x000055555571fbc0, opt=Opt @ 0x00007fffffffc710, header=<unavailable>) at mod.rs:61
    frame #34: 0x00005555555f2fb6 cxxbridge`cxxbridge::main::h77775307de2fc5af at main.rs:0
    frame #35: 0x00005555555efb28 cxxbridge`std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::hc6da6d696615393a at rt.rs:67:34
    frame #36: 0x00005555556e4cb8 cxxbridge`std::rt::lang_start_internal::h293a90d4ee9a7bff [inlined] std::rt::lang_start_internal::_$u7b$$u7b$closure$u7d$$u7d$::h5c36091523764794 at rt.rs:52:13
    frame #37: 0x00005555556e4cad cxxbridge`std::rt::lang_start_internal::h293a90d4ee9a7bff [inlined] std::panicking::try::do_call::h273850baa6701af6 at panicking.rs:331
    frame #38: 0x00005555556e4cad cxxbridge`std::rt::lang_start_internal::h293a90d4ee9a7bff [inlined] std::panicking::try::h7e4dc7963dfa3ab2 at panicking.rs:274
    frame #39: 0x00005555556e4cad cxxbridge`std::rt::lang_start_internal::h293a90d4ee9a7bff [inlined] std::panic::catch_unwind::h5e0313963a096103 at panic.rs:394
    frame #40: 0x00005555556e4cad cxxbridge`std::rt::lang_start_internal::h293a90d4ee9a7bff at rt.rs:51
    frame #41: 0x00005555555f3176 cxxbridge`main + 39
    frame #42: 0x00007ffff7de2bbb libc.so.6`__libc_start_main(main=(cxxbridge`main), argc=2, argv=0x00007fffffffd6e8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffd6d8) at libc-start.c:308:16
    frame #43: 0x00005555555cd07a cxxbridge`_start + 42

As far as I can tell, that's because of this code in proc_macro2's src/wrapper.rs:

fn nightly_works() -> bool {
    use std::sync::atomic::*;
    use std::sync::Once;

    static WORKS: AtomicUsize = AtomicUsize::new(0);
    static INIT: Once = Once::new();

    match WORKS.load(Ordering::SeqCst) {
        1 => return false,
        2 => return true,
        _ => {}
    }

    INIT.call_once(|| {
        type PanicHook = dyn Fn(&PanicInfo) + Sync + Send + 'static;

        let null_hook: Box<PanicHook> = Box::new(|_panic_info| { /* ignore */ });
        let sanity_check = &*null_hook as *const PanicHook;
        let original_hook = panic::take_hook();
        panic::set_hook(null_hook);

        let works = panic::catch_unwind(|| proc_macro::Span::call_site()).is_ok();
        WORKS.store(works as usize + 1, Ordering::SeqCst);

        // ...
    });
    nightly_works()
}

So, this isn't a bug in cxx. It's not even a bug in proc_macro2. It's intentional usage of exceptions within proc_macro2, and we need to adjust our build system. But I'm writing it up here for others' benefit.

@adetaylor adetaylor changed the title cxxbridge deosn cxxbridge doesn't run with panic=abort Apr 20, 2020
@dtolnay dtolnay reopened this Apr 21, 2020
@dtolnay
Copy link
Owner

dtolnay commented Apr 21, 2020

Cargo always build host tools (build scripts and macros) with panicking even if the final artifact uses panic = "abort". So ideally, as you observed, your build system would match that.

That said, I understand that this is an obstacle in non-Cargo codebases and I am not opposed to considering ways to fix it. The catch_unwind in proc-macro2 is used for determining whether or not we are currently executing inside of a proc macro. Inside a proc macro (like inside the implementation of structopt-derive which cxxbridge-cmd depends on), proc-macro2 must use the compiler's token representation because otherwise the compiler won't understand the provenance of the output tokens for correct name resolution and error reporting. Outside of a proc macro (such as when cxxbridge-cmd itself is parsing an input file) proc-macro2 must not use the compiler's token representation because that's backed by a client/server that rustc provides only to proc macros.

Two fixes would be:

  • Expose a non-panic based way to query whether the proc macro client/server provided by rustc is available for use. For example something like proc_macro::is_available() -> bool in rustc's libproc_macro.

  • Expose a way for the caller of proc-macro2 to override the client/server detection ahead of time. For example a cfg "no_proc_macro_detection" which makes proc-macro2 always assume it is in a macro, and a proc_macro2::i_am_not_macro() call that tools like cxxbridge-cmd would need to make at the top of main to switch to not assuming a proc macro.

Does it look like you will be able to fix this in your build system in a reasonable time or would it be better to pursue one of the above?

@adetaylor
Copy link
Collaborator Author

Thanks for the comment.

In theory, our non-Cargo build system can cope with this. It requires us to jump through significant hoops to parameterize the Rust toolchain details, but I do think those humps are worth jumping through. We're bound to hit the same problem elsewhere in the future.

So speaking purely from the point of view of our needs, let's not do anything here.

Speaking more generally though, it feels like proc_macro::is_available() -> bool would be architecturally nicer than the current situation and so it might be worth considering for the future, next time someone is updating the relevant bit of libproc_macro code? And it would have the benefit of solving this for future travelers.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 22, 2020
proc_macro::is_available()

This PR adds `proc_macro::is_available() -> bool` to determine whether proc_macro has been made accessible to the currently running program.

The proc_macro crate is only intended for use inside the implementation of procedural macros. All the functions in the crate panic if invoked from outside of a procedural macro, such as from a build script or unit test or ordinary Rust binary.

Unfortunately those panics made it impossible for libraries that are designed to support both macro and non-macro use cases (e.g. Syn) to be used from binaries that are compiled with panic=abort. In panic=unwind mode we're able to attempt a proc macro call inside catch_unwind and use libproc_macro's result if it succeeds, otherwise fall back to a non-macro alternative implementation. But in panic=abort there was no way to determine which implementation needs to be used.

r? @eddyb
attn: @petrochenkov @adetaylor
ref: dtolnay/cxx#130
@dtolnay
Copy link
Owner

dtolnay commented Apr 25, 2020

I've landed a non-panicking proc_macro::is_available() -> bool function, tracked in rust-lang/rust#71436, and filed dtolnay/proc-macro2#218 to begin using it once a 1.45-beta compiler is released.

@dtolnay dtolnay added blocked Can't make progress in the immediate term and removed blocked Can't make progress in the immediate term labels Apr 28, 2020
@dtolnay dtolnay added the integration How cxx fits into the big picture of an organization's codebase and builds label Aug 30, 2020
martinboehme added a commit to martinboehme/autocxx that referenced this issue Mar 19, 2021
Cargo always runs build scripts with panic=unwind, but other build
environments might not -- see here for a similar issue in cxx:

dtolnay/cxx#130

And the corresponding fix:

dtolnay/cxx#184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration How cxx fits into the big picture of an organization's codebase and builds
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants