naked functions #1689
naked functions #1689
Conversation
src/inline-assembly.md
Outdated
| - The compiler cannot assume that the instructions in the asm are the ones that will actually end up executed. | ||
| - This effectively means that the compiler must treat the `naked_asm!` as a black box and only take the interface specification into account, not the instructions themselves. | ||
| - Runtime code patching is allowed, via target-specific mechanisms. | ||
| - However there is no guarantee that each `naked_asm!` directly corresponds to a single instance of instructions in the object file: the compiler is free to duplicate or deduplicate `naked_asm!` blocks. |
There was a problem hiding this comment.
This is not true, naked functions cannot be duplicated/merged, unlike asm!.
There was a problem hiding this comment.
and reading this over, in this case it seems important that we do actually guarantee (and hence the compiler can assume) that the instructions in the naked_asm! invocation are exactly the ones that will be executed. or is that too strict?
There was a problem hiding this comment.
I think that poses problems on SPIR-V or NVPTX targets. I seem to recall that the loader just inlines the functions at the Shader Assembly level.
There was a problem hiding this comment.
@chorman0773 just to be sure, you're referring to Amanieu's comment right?
What we do in rustc (or well, we will once that PR makes it through the queue) is that a naked function
#[naked]
extern "C" fn foo() {}is emitted as something similar to
core::arch::global_asm!(
"foo:",
"ret"
);
extern "C" {
fn foo();
}and we're relying on the compiler (rustc, llvm) not duplicating that bit of global assembly. I suspect we already rely on that in general, because global assembly can define symbols today.
If you still think this is problematic, how could we verify the behavior of this new codegen strategy for the targets you list?
There was a problem hiding this comment.
and reading this over, in this case it seems important that we do actually guarantee (and hence the compiler can assume) that the instructions in the
naked_asm!invocation are exactly the ones that will be executed. or is that too strict?
No, they may not be the same instructions that are executed since you can still do runtime patching. We only guarantee that assembly code only appears once in the object file for the purposes of symbols.
ehuss
added
S-waiting-on-stabilization
src/attributes/codegen.md
Outdated
| r[attributes.codegen.naked.no-duplication] | ||
| The asm code may not be duplicated by the compiler. | ||
| This property is important for naked functions that define symbols in the assembly code. |
There was a problem hiding this comment.
so, something I just thought of: we do duplicate the assembly when monomorphizing generic naked functions. So this phrasing should be a bit more specific I think.
folkertdev
force-pushed
the
stabilize-naked-functions
branch
from
5deb3b5 to
54695c9
Compare
This comment has been minimized.
This comment has been minimized.
folkertdev
force-pushed
the
stabilize-naked-functions
branch
from
b2ee83c to
1a8abe7
Compare
This comment has been minimized.
This comment has been minimized.
folkertdev
force-pushed
the
stabilize-naked-functions
branch
from
1a8abe7 to
7fdadec
Compare
This comment has been minimized.
This comment has been minimized.
folkertdev
force-pushed
the
stabilize-naked-functions
branch
2 times, most recently
from
d1a523d to
1caf564
Compare
also maybe make links work?
Co-authored-by: bjorn3 <[email protected]>
Co-authored-by: Laine Taffin Altman <[email protected]>
this only applies to `asm!` (even though the rule has been removed there since copying it for naked asm
folkertdev
force-pushed
the
stabilize-naked-functions
branch
from
1caf564 to
0677e1e
Compare
folkertdev
force-pushed
the
stabilize-naked-functions
branch
from
f92f75f to
54ddde1
Compare
| # #[cfg(target_arch = "x86_64")] { | ||
| #[naked] | ||
| extern "C-unwind" fn naked_function() { | ||
| unsafe { | ||
| core::arch::naked_asm!( | ||
| ".cfi_startproc", | ||
| "push rbp", | ||
| ".cfi_def_cfa_offset 16", | ||
| ".cfi_offset rbp, -16", | ||
| "mov rbp, rsp", | ||
| ".cfi_def_cfa_register rbp", | ||
| "", | ||
| "call {function}", | ||
| "", | ||
| "pop rbp", | ||
| ".cfi_def_cfa rsp, 8", | ||
| "ret", | ||
| ".cfi_endproc", | ||
| function = sym function_that_panics, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| extern "C-unwind" fn function_that_panics() { | ||
| panic!("unwind!"); | ||
| } | ||
| # } |
There was a problem hiding this comment.
I don't think I can currently run this because #[naked] is still unstable. So reminder to self: update this before merging.
cc @traviscross
There was a problem hiding this comment.
We won't merge this anyway until the stabilization lands, so you can just update it and leave the CI red until then (add the feature flag and test it locally though). That's what we normally do.
| The *`naked` [attribute]* prevents the compiler from emitting a function prologue and | ||
| epilogue for the attributed function. |
There was a problem hiding this comment.
We're unwrapping lines on new things added, so go ahead and unwrap these all if you would.
tracking issue: rust-lang/rust#90957
earlier version: #1153
Pending on rust-lang/rust#128004. Once merged, we'll move for stabliziation.
I'm working off of the original PR to the reference, written 2+ years ago when the reference was less strict. I suspect some refinement and editing will be needed here to satisfy the quality requirements of the spec, and update the text to accurately reflect the current state and the exact guarantees/requirements.
Stabilization PR:
naked_functionsrust#134213cc @bstrie @traviscross @Amanieu @bjorn3