Open nebulark opened 2 months ago
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:
@rustbot concern reason-for-concern
<description of the concern>
Concerns can be lifted with:
@rustbot resolve reason-for-concern
See documentation at https://forge.rust-lang.org
cc @rust-lang/compiler @rust-lang/compiler-contributors
Proposal
Add a compiler flag that forces the compiler to emit functions that are hotpatchable, which would unblock tools such as Live++ or Recode to be developed for Rust. It would work the same as the MSVC /HOTPATCH flag (see https://learn.microsoft.com/en-us/cpp/build/reference/hotpatch-create-hotpatchable-image?view=msvc-170&viewFallbackFrom=vs-2019).
For a function to be hotpatchable the first instruction must be two bytes long and there must not be a jump to the first instruction. This way it is possible to overwrite the first instruction with a relative jump. This can then work together with passing -functionpadmin to the linker to ensure there is always a nearby, overwriteable space to jump to, from which then a absolute jump can be performed.
This is similar, but not the same as patchable-function-entries (https://github.com/rust-lang/rfcs/pull/3543). The main difference is that the hotpatch compliler flag would ensure that functions can be hotpatched. Most functions are already hotpatchable and would not be affected. Only for functions that don't fit the requirement additional nops are inserted. This is in contrast to patchable-function-entries which will unconditionally inserts nops.
Inlining is not a concern for this feature. If a function is inlined, the function in which it is inlined in can be hotpatched instead.
Zulip discussion that led to this MCP: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Adding.20hotpatch.20flag.20to.20the.20compiler
Implementation
For the implementation I would add the attribute '"patchable-function", "prologue-short-redirect"' to each function. This causes LLVM to use 'TargetOpcode::PATCHABLE_OP' which ensures that the function is hotpatchable. This only works on x86/x64.
In additon the code must be marked as hotpatchable via Targetmachine options, as otherwise -functionpadmin will not work. See: https://github.com/llvm/llvm-project/blob/d703b922961e0d02a5effdd4bfbb23ad50a3cc9f/lld/COFF/Writer.cpp#L1298
As a first step I would only implement this on x86/x64. I already have a candidate implementation: https://github.com/rust-lang/rust/pull/124966 I used the Clang implementation as a template: https://github.com/llvm/llvm-project/commit/5af2433e1794ebf7e58e848aa612c7912d71dc78
For other platforms patchable-function-entries (https://github.com/rust-lang/rfcs/pull/3543) with prefix-nops set to 2 could be used once it lands. Although it would not be as good as the x86 version, it can serve as a placeholder and be improved in the future. This way it can work on every platorm.
Mentors or Reviewers
None yet.
Process
The main points of the Major Change Process are as follows:
@rustbot second
.-C flag
, then full team check-off is required.@rfcbot fcp merge
on either the MCP or the PR.You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
Add Hotpatch flag to unblock tooling