Remove branch target prologues from #[naked] fn
#98998
Conversation
rustbot
added
the
T-compiler
|
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
workingjubilee
force-pushed
the
naked-means-no-clothes-enforcement-technology
branch
from
831ef5b
to
e4d1612
Compare
This comment has been minimized.
This comment has been minimized.
We can apply nocf_check as a hack for now.
Same idea but for AArch64.
workingjubilee
force-pushed
the
naked-means-no-clothes-enforcement-technology
branch
from
e4d1612
to
530b5da
Compare
|
Hmm, probably should also get input from |
|
I am not the right reviewer for this. Do you know who would be better suited @workingjubilee? |
|
...honestly I have no idea, this feels like a deeply cursed segment of the compiler and not a lot of hands pass through here. Apparently a lot of recent touches to this were from tmiasko and flip1995, not sure if either are considered compiler reviewers. |
|
This approach seems sane to me. Perhaps @joshtriplett or @Amanieu might want to look at it? |
|
What about lowering naked functions into global asm blocks? That way no codegen backend can accidentally add a prologue or epilogue and no codegen backend has to implement naked functions themself, but can piggyback on rustc's lowering for it. |
|
LGTM! @bors r+
I actually have a WIP branch that does just that, but it's still incomplete and I've been working on other stuff lately. I think this is the best way forward for naked functions in the long term. |
bors
added
S-waiting-on-bors
Rollup of 7 pull requests Successful merges: - rust-lang#98839 (Add assertion that `transmute_copy`'s U is not larger than T) - rust-lang#98998 (Remove branch target prologues from `#[naked] fn`) - rust-lang#99198 (add missing null ptr check in alloc example) - rust-lang#99344 (rustdoc: avoid inlining items with duplicate `(type, name)`) - rust-lang#99351 (Use `typeck_results` to get accurate qpath res for arg mismatch error) - rust-lang#99378 (interpret/visitor: add missing early return) - rust-lang#99394 (Add regression test for rust-lang#95230) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Sure! I agree that using that approach for handling naked functions is likely quite profitable, though I still think that LLVM should be handling this more correctly in general. At the very least, we shouldn't have to emit per-architecture hints just to disable the same codegen feature on different architectures. |
This patch hacks around #98768 for now via injecting appropriate attributes into the LLVMIR we emit for naked functions. I intend to pursue this upstream so that these attributes can be removed in general, but it's slow going wading through C++ for me.