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

Setting MIRI_BACKTRACE should not affect CTFE #53355

Closed
RalfJung opened this issue Aug 14, 2018 · 10 comments
Closed

Setting MIRI_BACKTRACE should not affect CTFE #53355

RalfJung opened this issue Aug 14, 2018 · 10 comments
Assignees

Comments

@RalfJung
Copy link
Member

MIRI_BACKTRACE should configure miri, but should not have an effect on CTFE that's part of normal rustc operation.

Cc @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Aug 15, 2018

That seems very hard to differentiate. And I sometimes did actually need the MIRI_BACKTRACE for figuring out problems in CTFE.

Since you probably just don't want all the noise from CTFE while running miri, you can std::env::set_var the variable automatically in miri, this will happen after the compilation is done, so you'll only see miri backtraces.

@RalfJung
Copy link
Member Author

That seems very hard to differentiate.

How so? I imagined moving the decision about whether to backtrace into the machine trait. miri would base it in MIRI_BACKTRACE. CTFE would base it on something else; maybe CTFE_BACKTRACE?

@oli-obk
Copy link
Contributor

oli-obk commented Aug 15, 2018

the backtrace is generated in the From impl of EvalError. There's no Machine available and it would be very painful to thread it through.

impl<'tcx> From<EvalErrorKind<'tcx, u64>> for EvalError<'tcx> {
fn from(kind: EvalErrorKind<'tcx, u64>) -> Self {
match env::var("MIRI_BACKTRACE") {

@RalfJung
Copy link
Member Author

Oh, dang...

you can std::env::set_var the variable automatically in miri, this will happen after the compilation is done, so you'll only see miri backtraces.

So what would the UI be? We change librustc to look for CTFE_BACKTRACE, and then miri wil check for MIRI_BACKTRACE and set CTFE_BACKTRACE before execution starts?

@oli-obk
Copy link
Contributor

oli-obk commented Aug 15, 2018

yea, that seems a good solution.

@RalfJung
Copy link
Member Author

We also need to fix the way backtraces are printed... they should not be printed immediately when the error is converted, but only later when the error is shown to the user. We do still create errors that we never act on, and finding the right backtrace is pretty hard ATM.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 15, 2018

We used to do that. I changed it to the current system when I got rid of the old CTFE error enum. we can probably add an optional field to the EvalError type which stores the backtrace as long as it's not compared, hashed,. .. or interacts with any traits implemented for the type

@RalfJung
Copy link
Member Author

Okay... yeah I think we have to change it. Right now, for my debugging, MIRI_BACKTRACE is useless; all it does is produce several thousand lines of backtraces.

@RalfJung
Copy link
Member Author

I think I'd like the same for RUST_LOG... something that makes miri set that variable once compilation is done.

Of course, in a query-based compiler, I don't even know what that means...

@RalfJung RalfJung self-assigned this Sep 20, 2018
@RalfJung
Copy link
Member Author

I am working on that (PR is blocked by #53821 (comment))

bors added a commit that referenced this issue Oct 28, 2018
Delayed CTFE backtraces

This renames the env var that controls CTFE backtraces from `MIRI_BACKTRACE` to `RUST_CTFE_BACKTRACE` so that we can use `MIRI_BACKTRACE` in the miri tool to only show backtraces of the main miri execution.

It also makes `RUST_CTFE_BACKTRACE` only show backtraces that actually get rendered as errors, instead of showing them eagerly when the `Err` happens. The current behavior is near useless in miri because it shows about one gazillion backtraces for errors that we later catch and do not care about. However, @oli-obk likes the current behavior for rustc CTFE work so it is still available via `RUST_CTFE_BACKTRACE=immediate`.

NOTE: This is based on top of #53821. Only [the last three commits](oli-obk/rust@sanity_query...RalfJung:ctfe-backtrace) are new.

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

No branches or pull requests

2 participants