Skip to content
New issue

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

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

Already on GitHub? # to your account

Disabling backtrace support while using -Z build-std: what went wrong? #1

Open
eddyb opened this issue Jul 28, 2022 · 0 comments
Open

Comments

@eddyb
Copy link

eddyb commented Jul 28, 2022

The README in this repo currently states:

The std crate relies on backtrace, which depends on libunwind in the default builds for unix. To work around this, cosmopolitan.a currently has stubs for the functions that backtrace relies on. However, it might be easier to provide a build flag in Cargo.toml to use the noop module of backtrace.

A small change needs to be submitted to the source code of backtrace (in the cfg_if! here) to allow choosing noop when building as part of the std crate. This conditional compilation flag should be accessible when building the std crate either via Cargo.toml or something like -Z use-std-backtrace-noop in the build command.

Cargo's unstable flags docs say this for -Z build-std-features:

This will configure the features enabled for the standard library itself when building the standard library. The default enabled features, at this time, are backtrace and panic_unwind. This flag expects a comma-separated list and, if provided, will override the default list of features enabled.

The instructions in the README contain -Zbuild-std-features="", and if we assume the Cargo docs aren't wrong/outdated, there's no flag handling bug etc. we arrive here:

rust-lang/rust/library/std/Cargo.toml:50-56

[features]
backtrace = [
  "gimli-symbolize",
  'addr2line/rustc-dep-of-std',
  'object/rustc-dep-of-std',
  'miniz_oxide/rustc-dep-of-std',
]

By using -Zbuild-std-features="" you have turned off the backtrace default, i.e.:

  • disabled 4 optional dependencies (gimli-symbolize, addr2line, object, miniz_oxide)
  • removed --cfg feature="backtrace" from the std build
    • i.e. cfg(feature = "backtrace") will not apply
    • as std includes rust-lang/backtrace-rs as a module within itself (for cyclic dependency reasons IIRC?), all of that code will also observe a lack of cfg(feature = "backtrace")
    • at the same time, std's build script unconditionally forces --cfg backtrace_in_libstd to be passed - i.e. across std and the backtrace-rs code, cfg(backtrace_in_libstd) will always apply
      • (my understanding is that this signals the backtrace-rs code that it's being "embedded" in this strange way into std, and allows it to adjust itself for the task)

So upstream already contains the machinery to opt out of backtrace support, and you engaged it.

Let's see what else that turns off - searching for feature = "backtrace" we find this:

rust-lang/backtrace-rs/src/symbolize/mod.rs:466-485

cfg_if::cfg_if! {
    if #[cfg(miri)] {
        mod miri;
        use miri as imp;
    } else if #[cfg(all(windows, target_env = "msvc", not(target_vendor = "uwp")))] {
        mod dbghelp;
        use dbghelp as imp;
    } else if #[cfg(all(
        any(unix, windows),
        not(target_vendor = "uwp"),
        not(target_os = "emscripten"),
        any(not(backtrace_in_libstd), feature = "backtrace"),
    ))] {
        mod gimli;
        use gimli as imp;
    } else {
        mod noop;
        use noop as imp;
    }
}

Sadly any(not(backtrace_in_libstd), feature = "backtrace") is kind of (unnecessarily?) De Morgan'd and looks backwards, but we can pretend it was written like this:

    } else if #[cfg(all(
        any(unix, windows),
        not(target_vendor = "uwp"),
        not(target_os = "emscripten"),
        not(all(backtrace_in_libstd, not(feature = "backtrace"))),
    ))] {

So what it's checking for is exactly the condition you are in: this code is being built as part of std and feature = "backtrace" has been turned off - and because this is part of an if #[cfg(all(, it causes gimli to be skipped and noop to be picked.

And if this hadn't worked, you would've had a broken build, because the gimli dependency was already turned off when you passed -Zbuild-std-features="", so it couldn't have been used either way.


And this is all good and working for symbolize/mod.rs, but backtrace/mod.rs is still using unwind even though you turned the backtrace feature off.

This it's already linked from the README, so nothing new, but let me summarize:

rust-lang/backtrace-rs/src/backtrace/mod.rs:135-150

    } else if #[cfg(
        any(
            all(
                unix,
                not(target_os = "emscripten"),
                not(all(target_os = "ios", target_arch = "arm")),
            ),
            all(
                target_env = "sgx",
                target_vendor = "fortanix",
            ),
        )
    )] {
        mod libunwind;
        use self::libunwind::trace as trace_imp;
        pub(crate) use self::libunwind::Frame as FrameImp;

Since your target matches unix and doesn't fit into one of the existing exceptions, unwind is where it goes. But it's not checking at all whether backtraces are enabled in the first place!

Adding any(not(backtrace_in_libstd), feature = "backtrace") (or its equivalent not(all(... form) into the all(unix, ... case should be enough for this to work.

As for the rust-lang/rust side, it's likely only two files need to change:

Why doesn't it already work the way it should?

I'm not sure. Even the symbolize optionality was accidentally broken (relatively) recently:

But what's the full story of std backtrace optionality? (PRs in chronological order)

When the backtrace-rs repo and std got more intertwined, it became possible to directly determine whether std has its backtrace feature enabled, from backtrace-rs code, and that's where we are.

How can this be made more robust?

Perhaps every cfg_if! in the backtrace-rs repo should start with checking backtrace_in_libstd/feature = "backtrace" (i.e. "am I part of a std build w/ disabled backtraces?") and have that be its own case that duplicates the mod noop; use noop as imp;.

Though I would put it not first, but second, just after the miri check, since that has its own mechanism and there should be no harm in allowing it (since it's all an "emulated backtrace" not a native one).

cc @alexcrichton

My earlier suggestion of where to add that check would also work, just like the one in symbolize works today, but only to unbreak your build, not to more generally disable backtraces across platforms.

Does Cargo need more flags to allow controlling std builds?

(this is, btw, where I started writing up this issue, but curiosity got the better of me and that's why it's so long)

To get to the point, the README suggests -Z use-std-backtrace-noop.

However, simple opt-ins/opt-outs can always be handled by -Z build-std-features:

  • opt-ins, or "additive" features, are those that typically provide extra functionality (without hindering any code that isn't aware of the feature)
  • opt-outs: while this might be desired pretty often, it doesn't map directly to a feature
    • examples could be "remove that part of std I don't need/want", or even just "make this small crate not depend on std, it only needs memory allocation for something unimportant"
    • that latter case is the typical case study, aka no_std: while the Rust attribute to remove the implicit std dependency is #![no_std], and colloquially we say "crate foo has no_std support", you don't want to use that "polarity" to a Cargo feature, because Cargo's additive behavior doesn't match the expectation of "I'm fine with not needing std", but rather "allowing std access is unacceptable and must be an error" (which will immediately break anything that does need the std-specific functionality but didn't express sentiments either way)
    • so instead what we do is flip it back into an additive feature (named "std" most of the time): now it's perfectly composable because getting more than you asked for (and only because e.g. some other package in the dep graph did) isn't (usually) an error
    • in your case, use-std-backtrace-noop is an opt-out, so you could imagine:
      • -Z build-std-features=backtrace-not-noop opts-in (this could also be the default)
      • -Z build-std-features= (or some other list of features not including backtrace-not-noop) would opt-out by omission
      • but what is a backtrace-not-noop feature, other than just a double-negative name for backtrace, i.e. the feature that's also controllable today? (just not fully hooked up, as discussed earlier)

Hope that's useful enough, and good luck! (fwiw I didn't mean to go into that much rambling detail, oops)
(oh, and if nobody else gets around to submitting upstream PRs for this, I could try to help, but this is pretty much a libs issue and I mostly stick to the compiler stuff)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant