Make asm label blocks safe context by nbdd0121 · Pull Request #131544 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits1 Checks6 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Tracking issue: #119364
asm!()
is forced to be wrapped inside unsafe. If there's no special treatment, the label blocks would also always be unsafe with no way of opting out. It was suggested that a simple fix is to make asm label blocks safe: #119364 (comment).
@rustbot labels: +A-inline-assembly +F-asm
asm!()
is forced to be wrapped inside unsafe. If there's no special
treatment, the label blocks would also always be unsafe with no way
of opting out.
rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r?
to explicitly pick a reviewer
Out { expr: None, reg: _, late: _ } |
---|
| Const { value: _, span: _ } |
| SymFn { value: _, span: _ } |
| SymStatic { def_id: _ } => {} |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default visitor uses visit_anon_const
and visit_qpath
in this place, don't you need them here?
To avoid potential mistakes, it probably makes sense to add a public fn walk_inline_asm_operand
helper to intravisit.rs.
match op { Label { .. } => self.in_safety_context(SafetyContext::Safe, walk_inline_asm_operand(...)), _ => walk_inline_asm_operand(...), }
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the THIR visitor which doesn't have visit_anon_const and visit_qpath.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I never noticed the update, it's better to set the S-waiting-on-review
label if a PR is waiting on review.
If this part cannot be improved, then it's good to go.
ojeda mentioned this pull request
95 tasks
📌 Commit 809dc73 has been approved by petrochenkov
It is now in the queue for this repository.
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-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 6 pull requests
Successful merges:
- rust-lang#130236 (unstable feature usage metrics)
- rust-lang#131544 (Make asm label blocks safe context)
- rust-lang#131586 (Support s390x z13 vector ABI)
- rust-lang#132489 (Fix closure arg extraction in
extract_callable_info
, generalize it to async closures) - rust-lang#133078 (tests: ui/inline-consts: add issue number to a test, rename other tests)
- rust-lang#133283 (Don't exclude relnotes from
needs-triage
label)
r? @ghost
@rustbot
modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#131544 - nbdd0121:asm_goto_safe_block, r=petrochenkov
Make asm label blocks safe context
Tracking issue: rust-lang#119364
asm!()
is forced to be wrapped inside unsafe. If there's no special treatment, the label blocks would also always be unsafe with no way of opting out. It was suggested that a simple fix is to make asm label blocks safe: rust-lang#119364 (comment).
@rustbot
labels: +A-inline-assembly +F-asm
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…rcote
Stabilize asm_goto
feature gate
Stabilize asm_goto
feature (tracked by rust-lang#119364). The issue will remain open and be updated to track asm_goto_with_outputs
.
Reference PR: rust-lang/reference#1693
Stabilization Report
This feature adds a label <block>
operand type to asm!
. <block>
must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the asm!
block returns and continues execution.
The block starts a new safety context and unsafe operations within must have additional unsafe
s; the effect of unsafe
that surrounds asm!
block is cancelled. See rust-lang#119364 (comment) and rust-lang#131544.
It's currently forbidden to use asm_goto
with output operands; that is still unstable under asm_goto_with_outputs
.
Example:
unsafe {
asm!(
"jmp {}",
label {
println!("Jumped from asm!");
}
);
}
Tests:
- tests/ui/asm/x86_64/goto.rs
- tests/ui/asm/x86_64/goto-block-safe.stderr
- tests/ui/asm/x86_64/bad-options.rs
- tests/codegen/asm/goto.rs
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#133870 - nbdd0121:asm, r=traviscross,nnethercote
Stabilize asm_goto
feature gate
Stabilize asm_goto
feature (tracked by rust-lang#119364). The issue will remain open and be updated to track asm_goto_with_outputs
.
Reference PR: rust-lang/reference#1693
Stabilization Report
This feature adds a label <block>
operand type to asm!
. <block>
must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the asm!
block returns and continues execution.
The block starts a new safety context and unsafe operations within must have additional unsafe
s; the effect of unsafe
that surrounds asm!
block is cancelled. See rust-lang#119364 (comment) and rust-lang#131544.
It's currently forbidden to use asm_goto
with output operands; that is still unstable under asm_goto_with_outputs
.
Example:
unsafe {
asm!(
"jmp {}",
label {
println!("Jumped from asm!");
}
);
}
Tests:
- tests/ui/asm/x86_64/goto.rs
- tests/ui/asm/x86_64/goto-block-safe.stderr
- tests/ui/asm/x86_64/bad-options.rs
- tests/codegen/asm/goto.rs
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
Stabilize asm_goto
feature gate
Stabilize asm_goto
feature (tracked by #119364). The issue will remain open and be updated to track asm_goto_with_outputs
.
Reference PR: rust-lang/reference#1693
Stabilization Report
This feature adds a label <block>
operand type to asm!
. <block>
must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the asm!
block returns and continues execution.
The block starts a new safety context and unsafe operations within must have additional unsafe
s; the effect of unsafe
that surrounds asm!
block is cancelled. See rust-lang/rust#119364 (comment) and rust-lang/rust#131544.
It's currently forbidden to use asm_goto
with output operands; that is still unstable under asm_goto_with_outputs
.
Example:
unsafe {
asm!(
"jmp {}",
label {
println!("Jumped from asm!");
}
);
}
Tests:
- tests/ui/asm/x86_64/goto.rs
- tests/ui/asm/x86_64/goto-block-safe.stderr
- tests/ui/asm/x86_64/bad-options.rs
- tests/codegen/asm/goto.rs
Labels
Area: Inline assembly (`asm!(…)`)
`#![feature(asm)]` (not `llvm_asm`)
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.