Skip to content

Add __fastfail for Windows on arm/aarch64 #75990

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 4 commits into from
Aug 30, 2020
Merged

Add __fastfail for Windows on arm/aarch64 #75990

merged 4 commits into from
Aug 30, 2020

Conversation

rylev
Copy link
Member

@rylev rylev commented Aug 27, 2020

Fixes #73215

@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(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 Aug 27, 2020
@Mark-Simulacrum
Copy link
Member

r? @alexcrichton perhaps? It looks fine to me, though I've not double checked assembly myself.

@rylev
Copy link
Member Author

rylev commented Aug 27, 2020

FYI: I verified the assembly on aarch64 and it looked correct. I did not verify on arm though. I’m currently working to find a machine to manually verify it triggers the fastfail debugger path as expected.

@alexcrichton
Copy link
Member

Nice, thanks for this!

While you're at it, mind trying to switch to asm! as well? Otherwise r=me once you've checked on ARM

@rylev
Copy link
Member Author

rylev commented Aug 28, 2020

Switched to asm! . After testing with @danielframpton, we found that the docs don't seem to be 100% correct and that the __fastfail intrinsic on ARM actually uses brk ${0xF003,0xDEFB} instead of emitting an illegal opcode. Daniel tested on an aarch64 device and was able to trap in a debugger with the correct error code.

@alexcrichton
Copy link
Member

@bors: r+

Ok sounds good!

@bors
Copy link
Collaborator

bors commented Aug 28, 2020

📌 Commit 8bcc4d6 has been approved by alexcrichton

@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 Aug 28, 2020
@rylev
Copy link
Member Author

rylev commented Aug 28, 2020

@alexcrichton hold up on merging. We need to test on 32 bit still. Sorry!

@rylev
Copy link
Member Author

rylev commented Aug 28, 2020

It turns out that 32 bit does indeed want the illegal opcode and not a brk instruction like 64 bit. I've changed the 32-bit version back, and @danielframpton will test on his device.

@alexcrichton
Copy link
Member

Oops sorry, misinterpreted!

@danielframpton
Copy link
Contributor

I have validated this change with both aarch64-pc-windows-msvc and thumbv7a-pc-windows-msvc targets, which both terminate with a FAST_FAIL_FATAL_APP_EXIT.

@alexcrichton
Copy link
Member

Thanks!

@bors: r+

@bors
Copy link
Collaborator

bors commented Aug 28, 2020

📌 Commit 9e2228d has been approved by alexcrichton

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 29, 2020
@Dylan-DPC
Rollup merge of rust-lang#75990 - rylev:arm-fastfail, r=alexcrichton
993adf3 
Add __fastfail for Windows on arm/aarch64

Fixes rust-lang#73215
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Aug 29, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 29, 2020
@Dylan-DPC
Rollup merge of rust-lang#75990 - rylev:arm-fastfail, r=alexcrichton
214a80b 
Add __fastfail for Windows on arm/aarch64

Fixes rust-lang#73215
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Aug 29, 2020
Closed
@rylev
Copy link
Member Author

rylev commented Aug 29, 2020 •
edited
Loading

@alexcrichton @Amanieu I pushed a change to explicitly require "thumb-mode" on arm. The assembly looks to be the same when compiling for thumbv7a-pc-windows-msvc.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Aug 29, 2020

📌 Commit d931e97 has been approved by alexcrichton

@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Aug 29, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 30, 2020
@bors
Auto merge of rust-lang#76090 - Dylan-DPC:rollup-eksndcr, r=Dylan-DPC
62850d8 
Rollup of 14 pull requests

Successful merges:

 - rust-lang#75832 (Move to intra-doc links for wasi/ext/fs.rs, os_str_bytes.rs…)
 - rust-lang#75852 (Switch to intra-doc links in `core::hash`)
 - rust-lang#75874 (Shorten liballoc doc intra link while readable)
 - rust-lang#75881 (Expand rustdoc theme chooser x padding)
 - rust-lang#75885 (Fix another clashing_extern_declarations false positive.)
 - rust-lang#75892 (Fix typo in TLS Model in Unstable Book)
 - rust-lang#75910 (Add test for issue rust-lang#27130)
 - rust-lang#75917 (Move to intra doc links for core::ptr::non_null)
 - rust-lang#75975 (Allow --bess ing expect-tests in tools)
 - rust-lang#75990 (Add __fastfail for Windows on arm/aarch64)
 - rust-lang#76015 (Fix loading pretty-printers in rust-lldb script)
 - rust-lang#76022 (Clean up rustdoc front-end source code)
 - rust-lang#76029 (Move to intra-doc links for library/core/src/sync/atomic.rs)
 - rust-lang#76057 (Move retokenize hack to save_analysis)

Failed merges:

r? @ghost
@bors bors merged commit 96e0bc7 into rust-lang:master Aug 30, 2020
@rylev rylev deleted the arm-fastfail branch December 6, 2021 10:06
@cuviper cuviper added this to the 1.48.0 milestone Nov 17, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Amanieu Amanieu Amanieu left review comments

Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.

Use __fastfail in libpanic_abort on Windows
8 participants
@rylev @rust-highfive @Mark-Simulacrum @alexcrichton @bors @danielframpton @Amanieu @cuviper