stage1, tests, and landing pads

compiler
1

Since the last month or so, stage1 tests (make check-stage1) don’t work. The shallow cause is @alexcrichton’s PR 27176, which changed the behaviour of -Z no-landing-pads.

Landing pads are required to clean resources owned by the current process on unwinding. Programs that run multiple tasks in a single process and desire high availability require it to limit the effect of logic errors.

rustc, like many programs, is neither of these, and does not need that sort of cleanup. Landing pads have a compile-time, and potentionally a run-time, cost, which were once measured to be about 20% in total in the case of rustc. Therefore, it is desirable to compile it without landing pads. Currently, stage2, the final compilation output product, is compiled with them, but as an optimization stage1 does not use them.

However, rustc uses panics as part of its error handling mechanism - both to bail out of nested code, e.g. in the case of trait overflow while evaluating built-in kinds (while this is suboptimal, avoiding it in this case would significantly complicate the compiler), as well as to handle internal compiler errors which are unfortunately all too common.

Rust’s native reaction to panics is to print an ugly message to stderr and unwind. This is not desirable at least in rustc’s case, so it disables the ugly message and catches the panic, and then exits, potentiality after displaying a nicer message if there was an ICE. Obviously, leaking the stack is irrelevant here.

The problematic patch had made unwinding crash when used on a program compiled without landing pads. This change is probably for the better, as it makes programs compiled without landing pads not depend on unwinding in any way. Unfortunately, it makes rustc crash after it reports a fatal error, which breaks the tests on stage1, and requires compiling stage2 to run them. This can be easily fixed by not trying to unwind, see e.g. PR 28206 by yours truly.

@alexcrichton prefers to enable landing pads on stage1. His position isn’t clear to me and I would like for him to clarify it.

An additional, but small, issue is that libtest, and compiletest which depends on it, use panics to handle test failures. Until this is changed, they must always be compiled with landing pads enabled. This does not seem to be a point of significant interest.

2

Data:

compile-time costs:

libcore LLVM passes are `3.842` seconds without landing pads, `3.850` with (100.02%)
libstd LLVM passes are `5.777` seconds without landing pads, `6.600` with (114.25%)
libsyntax LLVM passes are `58.734` seconds without landing pads, `71.908` seconds with (122%)
librustc LLVM passes are `107.231` seconds without landing pads, `115.323` with (107.55%)

that’s 7 to 20%

       /        | body-noLP | body-LP | Δ  | trans-noLP | trans-LP | Δ
librustc        |   6.935s  | 6.891s  |-.7%|  14.480    | 14.279   |-1.4%
librustc_trans  |   3.565s  | 3.607s  | 1% |   6.228    | 6.422    | 3%
librustc_typeck |   2.845s  | 2.854s  |0.3%|   5.641    | 5.740    | 1.8%

There does not seem to be a significant effect on compiler runtime

3

I’m curious how much of that we could always save by checking whether the callee can unwind at all.

4

Would it be possible to compile libtest/compiletest with landing pads and the rest of the compiler without it?

I haven’t looked into the libtest code too deeply, but it should be possible to rewrite compiletest to handle test failures without panic!().

5

That’s what we are doing (we use the compiletest from stage2, which is compiled with landing pads). If we start compiling stage2 without landing pads, we would then need another version of libstd with landing pads for check-stage1 to be useful (“stage3” will always be compiled with landing pads, but using it ruins the point of check-stage1).

6

Ah, I see. Thanks for pointing that out.

7

I think the main thing to focus on here is getting tests and such in stage1 working basically as they were before. Beyond that I don’t think there’s many other issues to deal with. Along those lines, there are a few strategies proposed that can be taken:

  • Landing pads can just be turned on in all stages. This will make the bootstrap slightly longer (see measurements above)
  • The “raise an error” paths in the compiler could be different in stages to handle the case where we can’t panic if landing pads are disabled. The main drawback of this is that it introduces a split path in the compiler which is almost never exercised and may be difficult to maintain over time.
  • All errors in the compiler immediately exit the process. The drawback of this is that it’s reaching quite a bit into runtime internals which may be difficult to update over time and it’s also taking an unidiomatic approach to handling panics.
  • We could remove the logic for not emitting a landing pad for the try function even if no-landing-pads is enabled and instead just always emit a landing pad. This gets us back to the old behavior where cleanups aren’t run but panics won’t abort the process.

The use case of make check-stage1 is pretty niche so I don’t want to add too much infrastructure or reach for non-idiomatic handling strategies, it doesn’t seem worth the cost. Along those lines my preferred methods forward would be to just turn landing pads on or do the last bullet above, both of those are pretty un-invasive and won’t require any other changes to the compiler.

8

Can either @arielb1 or @alexcrichton summarize briefly what PR https://github.com/rust-lang/rust/pull/28206 did? Also, how does this discussion relate to the existing desire to have some ability to customize what happens on a panic (e.g., https://github.com/rust-lang/rfcs/pull/1100, which I really ought to digest).

9

There are two current PRs for fixing make check-stage1:

  • #27417 - This runs all tests with a stage2 compiletest binary (built with a stage1 compiler) so landing pads are enabled for that driver. It then has a #[cfg(stage0)] to call std::process:exit(101) in stage0 and otherwise panic in stage1+. This means that ICE messages are all silenced by default but normal compiler errors work as usual.
  • #28206 - This takes the same strategy of running all tests with a stage2 compiletest, but instead of #[cfg] logic it registers a custom panic handler with the infrastructure we currently have today. If the panicking thread is considered the rustc thread, then the process will always call exit(101) but it’ll also print ICE messages right before doing so.

I think that this is related to custom panic handlers and also disabling landing pads as a more official compiler option, but it’s also somewhat orthogonal in the sense that compiletest and our test infrastructure are not currently design to work with 0 landing pads, so something is gonna have to be caught somewhere.

10

Perhaps the TL;DR for me is that the gains in time don’t seem particularly large. Based on those figures I project the effect on overall bootstrap time would be quite small. Do we have any figure on that?

If that is correct, then I think we should go for the simplest solution possible, which I think is:

  1. Re-enable landing pads on stage1, so that things work in a uniform fashion.
  2. Refactor things so that we can disable landing pads everywhere
    • this presumably means using the custom panic handler stuff for rustc to print a nice msg on an ICE?
    • I’m not really sure what it means to refactor the test running infrastructure here
1 Like
11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.