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

Bootstrap fun with '-Cpanic=abort' #34061

Closed
MagaTailor opened this issue Jun 3, 2016 · 9 comments
Closed

Bootstrap fun with '-Cpanic=abort' #34061

MagaTailor opened this issue Jun 3, 2016 · 9 comments
Labels
A-codegen Area: Code generation A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@MagaTailor
Copy link

MagaTailor commented Jun 3, 2016

#33999 A rustc that doesn't care about crashing should be buildable with panic=abort. Could it work, even now, if building the libpanic_unwind crate, the flag simply got ignored?

rustc: arm-unknown-linux-gnueabihf/stage0/lib/rustlib/arm-unknown-linux-gnueabihf/lib/libpanic_unwind
Stored value type does not match pointer operand type!
  store i8* null, i32* %4
 i32LLVM ERROR: Broken function found, compilation aborted!
make: *** [arm-unknown-linux-gnueabihf/stage0/lib/rustlib/arm-unknown-linux-gnueabihf/lib/stamp.panic_unwind] Error 1
@jonas-schievink
Copy link
Contributor

Rustc uses unwinding to signal compilation errors, so the rustc you're going to build will abort on compile errors instead of exiting gracefully. Not sure what this would mean in practice (I've never used -Cpanic=abort), but it's something to look out for.

@MagaTailor
Copy link
Author

MagaTailor commented Jun 3, 2016

Ah, thanks for that clarification. I thought, similar to regular binaries, rustc could just make do without the abillity to panic gracefully.

Still, I was expecting to retain the abillity to create binaries with either strategy. Somewhat related is this patch by @japaric, I think.

@japaric
Copy link
Member

japaric commented Jun 3, 2016

@petevine That patch only removes the RUST_BACKTRACE functionality; it doesn't affect the panicking/unwinding mechanism. One can use that patch with either panic strategy.

@nagisa nagisa added the A-codegen Area: Code generation label Jun 3, 2016
@nagisa
Copy link
Member

nagisa commented Jun 3, 2016

That’s a weird codegen issue. Does it also happen with -Zorbit?

@MagaTailor
Copy link
Author

MagaTailor commented Jun 3, 2016

@nagisa Yes it does, with a small twist:

Stored value type does not match pointer operand type!
  store i8* null, i32* %tmp_ret

@nagisa nagisa added A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ labels Jun 3, 2016
@eddyb eddyb added the A-amusing label Jun 4, 2016
@eddyb
Copy link
Member

eddyb commented Jun 4, 2016

The issue is that building libpanic_unwind with -Cpanic=abort is unsupported, as it uses intrinsics::try (at least that's the simplest explanation - the personality function might be related).
If it compiled, the result would, at best, be unusable without -Cpanic=abort.

Could it work, even now, if building the libpanic_unwind crate, the flag simply got ignored?

That's probably the sane solution, although that's not the only crate you would have to treat like this.

@alexcrichton This is the general problem of providing multiple usable configurations: I don't see a good way around many builds of libstd, one for each specific configuration.
A libstd built without unwinding support could be better optimized than one built with, but at the same time it could only be used for -Cpanic=abort.
We're now also paying in MIR optimizations, not just LLVM ones. With unwinding turned off, a call Rvalue could be quite advantageous in call-heavy code and dynamic drop flags might also be reduced in number.

@eddyb
Copy link
Member

eddyb commented Jun 4, 2016

@petevine There are some problems preventing running rustc with unwinding disabled that @jonas-schievink alluded to (mostly related to compiletest), see #27417 and #28206.

I think we could fix this codegen bug and maybe then what you were trying to do would work, as long as you wouldn't bother running tests.
However, if there is an improvement in compile times by having unwinding disabled (hopefully without breaking RUST_BACKTRACE completely), then I would love to have it on by default, which requires running tests to work.

@alexcrichton
Copy link
Member

@eddyb yeah we will likely ship a panic=abort libstd one day for that exact reason (more optimized), and you're also correct that libpanic_unwind cannot currently be compiled with -C panic=abort. I don't quite remember why, but I suspect it's a relatively easy fix.

@MagaTailor
Copy link
Author

MagaTailor commented Jun 5, 2016

@eddyb It was just some honest experimentation but I got emboldened by the positive results I got from building parity successfully. Then I wanted to see some numbers but the benchmark suite failed to compile which could be a build script problem. (I ran cargo clean first though)

I guess it's partly explained here but the error message doesn't tell us which crate is the problem one.

edit:
#37097
#37252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

No branches or pull requests

6 participants