Skip to content

Custom panic hook for RMC including current item#304

Merged
danielsn merged 4 commits intomodel-checking:main-153-2021-07-02from
avanhatt:custom-panic-hook
Jul 8, 2021
Merged

Custom panic hook for RMC including current item#304
danielsn merged 4 commits intomodel-checking:main-153-2021-07-02from
avanhatt:custom-panic-hook

Conversation

@avanhatt
Copy link
Contributor

@avanhatt avanhatt commented Jul 7, 2021

Description of changes:

When RMC panics currently, you get the standard Rustc panic message, including a link to their bug reporting. Add a custom panic hook with our bug template link and include the current codegen item (usually function) causing the problem.

Example panic now:

   Compiling libc v0.2.93
thread 'rustc' panicked at 'oops', compiler/rustc_codegen_llvm/src/gotoc/mod.rs:208:9
stack backtrace:
   0: std::panicking::begin_panic
             at /home/ubuntu/rmc/library/std/src/panicking.rs:541:12
…

note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.55.0-dev running on x86_64-unknown-linux-gnu

note: compiler flags: -Z force-unstable-if-unmarked -Z trim-diagnostic-paths=no -Z codegen-backend=gotoc -C embed-bitcode=no -C debuginfo=2 --crate-type lib

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack

[RMC] current codegen item: codegen_static: DefId(0:8806 ~ core[e967]::fmt::num::DEC_DIGITS_LUT)

RMC unexpectedly panicked during code generation.

If you are seeing this message, please file an issue here instead of on the Rust compiler: https://github.com/model-checking/rmc/issues/new?labels=bug&template=bug_report.md
error: could not compile `core`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

Resolved issues:

Resolves #287

Call-outs:

Unfortunately, we can't use the same approach Cranelift uses to remove rustc's bug report link, since that would introduce a circular dependency on rustc_driver. Adding a comment explaining that.

Testing:

  • How is this change tested?

Manually introducing panics.

  • Is this a refactor change?

No.

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more descriptive name to use here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reset the item at the end, to avoid spuriously having a set item if there is none. We could use a RAII pattern, tho that might be overkill

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, reset after both loops.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what exactly RAII would look like here, feel free to follow up if this change is not sufficient?

@avanhatt avanhatt requested a review from danielsn July 7, 2021 19:07
@avanhatt avanhatt force-pushed the custom-panic-hook branch from 7aa4cec to 55fb5b4 Compare July 7, 2021 20:22
@adpaco-aws adpaco-aws mentioned this pull request Jul 8, 2021
4 tasks
@danielsn danielsn merged commit 41a5b04 into model-checking:main-153-2021-07-02 Jul 8, 2021
@zhassan-aws zhassan-aws mentioned this pull request Aug 6, 2021
4 tasks
@avanhatt avanhatt deleted the custom-panic-hook branch September 14, 2021 15:25
tedinski pushed a commit to tedinski/rmc that referenced this pull request Apr 26, 2022
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

Successfully merging this pull request may close these issues.

RMC should have its own panic hook

2 participants