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.

Sign up for GitHub

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

Jump to bottom

Windows: Disable LLVM crash dialog boxes. #93402

Merged
merged 2 commits into from
Feb 4, 2022
Merged

Windows: Disable LLVM crash dialog boxes. #93402

merged 2 commits into from
Feb 4, 2022

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jan 28, 2022

This disables the crash dialog box on Windows. When LLVM hits an assertion, it will open a dialog box with Abort/Retry/Ignore. This is annoying on CI because CI will just hang until it times out (which can take hours).

Instead of opening a dialog box, it will print a message like this:

Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible type!", file D:\Proj\rust\rust\src\llvm-project\llvm\include\llvm/Support/Casting.h, line 255

Closes #92829

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 28, 2022
@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 28, 2022
@ehuss
Copy link
Contributor Author

ehuss commented Jan 28, 2022

I am unfamiliar with both Windows assertions and LLVM, so I do not know if this is a good idea. However, I wanted to open this PR up to start a discussion. DisableSystemDialogsOnCrash seemed like a good place to start. However, it does a few extra things that may not be wanted, like handling SIGBART. This could instead be narrowed down to _set_error_mode and _CrtSetReportHook, but the docs are not super clear to me what the minimum necessary is.

I also don't know if anyone finds these dialog boxes useful. With them enabled, it is possible to open a debugger and get a stack trace. With them disabled, I'm not sure how to do that easily on an assertion failure. So this may be too extreme.

Another option is to have a runtime check. It could be something simple like checking if the "CI" env var is set, or something more elaborate.

@ehuss ehuss mentioned this pull request Jan 28, 2022
Closed
@michaelwoerister
Copy link
Member

Thanks for the PR, @ehuss!

Another option is to have a runtime check. It could be something simple like checking if the "CI" env var is set

That sounds like a good idea.

@ehuss
Only disable dialogs on CI.
c64d6bf 
The "CI" environment var isn't universal (for example, I think Azure
uses TF_BUILD). However, we are mostly concerned with rust-lang/rust's
own CI which currently is GitHub Actions which does set "CI". And I
think most other providers use "CI" as well.
@ehuss
Copy link
Contributor Author

ehuss commented Feb 3, 2022

Sure, done.

@michaelwoerister
Copy link
Member

Alright, I think we can just merge this as is now, and come back later if it causes any trouble.

@bors r+

@bors
Copy link
Contributor

bors commented Feb 3, 2022

📌 Commit c64d6bf has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
@matthiaskrgr
Rollup merge of rust-lang#93402 - ehuss:llvm-dialog, r=michaelwoerister
22da463 
Windows: Disable LLVM crash dialog boxes.

This disables the crash dialog box on Windows. When LLVM hits an assertion, it will open a dialog box with Abort/Retry/Ignore. This is annoying on CI because CI will just hang until it times out (which can take hours).

Instead of opening a dialog box, it will print a message like this:

```
Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible type!", file D:\Proj\rust\rust\src\llvm-project\llvm\include\llvm/Support/Casting.h, line 255
```

Closes rust-lang#92829
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 3, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
@matthiaskrgr
Rollup merge of rust-lang#93402 - ehuss:llvm-dialog, r=michaelwoerister
1de6839 
Windows: Disable LLVM crash dialog boxes.

This disables the crash dialog box on Windows. When LLVM hits an assertion, it will open a dialog box with Abort/Retry/Ignore. This is annoying on CI because CI will just hang until it times out (which can take hours).

Instead of opening a dialog box, it will print a message like this:

```
Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible type!", file D:\Proj\rust\rust\src\llvm-project\llvm\include\llvm/Support/Casting.h, line 255
```

Closes rust-lang#92829
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 3, 2022
Closed
@bors
Copy link
Contributor

bors commented Feb 3, 2022

⌛ Testing commit c64d6bf with merge e281c7ba0a71e90a36def05d7eb56f70ffcb8369...

@bors
Copy link
Contributor

bors commented Feb 3, 2022

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 3, 2022
@ehuss
Copy link
Contributor Author

ehuss commented Feb 3, 2022 •
edited
Loading

@bors retry

dist-i586-gnu-i586-i686-musl appears to be hung

compiletest hung after finishing the ui tests without printing a summary.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2022
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Feb 4, 2022

⌛ Testing commit c64d6bf with merge eb2ec2ea810beeb8a97326b4208d7bfc98d76f92...

@bors
Copy link
Contributor

bors commented Feb 4, 2022

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 4, 2022
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@ehuss
Copy link
Contributor Author

ehuss commented Feb 4, 2022

@bors retry

#93329 i686-1 hung on rustc_macros.

I was really hoping that this PR would have illuminated why that was happening. ☹️

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 4, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 4, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2022
@bors
Auto merge of rust-lang#93645 - matthiaskrgr:rollup-eua2621, r=matthi…
cb18e83 
…askrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#92735 (Add crate filter parameter in URL)
 - rust-lang#93402 (Windows: Disable LLVM crash dialog boxes.)
 - rust-lang#93508 (Add rustdoc info to jsondocck output)
 - rust-lang#93551 (Add package.json in gitignore)
 - rust-lang#93555 (Link `try_exists` docs to `Path::exists`)
 - rust-lang#93585 (Missing tests for rust-lang#92630)
 - rust-lang#93593 (Fix ret > 1 bound if shadowed by const)
 - rust-lang#93630 (clippy::perf fixes)
 - rust-lang#93631 (rustc_mir_dataflow: use iter::once instead of Some().into_iter)
 - rust-lang#93632 (rustdoc: clippy::complexity fixes)
 - rust-lang#93638 (rustdoc: remove unused Hash impl)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f7e0f97 into rust-lang:master Feb 4, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@michaelwoerister michaelwoerister

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.60.0
Development

Successfully merging this pull request may close these issues.

LLVM assertion failures hang with a modal dialog on Windows
6 participants
@ehuss @rust-highfive @michaelwoerister @bors @rust-log-analyzer @rustbot