-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add unstable hotpatch flag to rustc #134004
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -378,6 +378,15 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>( | |||||||||||||||||||||||||||||||||||||||||||||||
to_add.push(llvm::CreateAttrString(cx.llcx, "use-sample-profile")); | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
// patchable-function is only implemented on x86 on LLVM | ||||||||||||||||||||||||||||||||||||||||||||||||
if cx.sess().opts.unstable_opts.hotpatch && cx.sess().target.is_x86() { | ||||||||||||||||||||||||||||||||||||||||||||||||
to_add.push(llvm::CreateAttrStringValue( | ||||||||||||||||||||||||||||||||||||||||||||||||
cx.llcx, | ||||||||||||||||||||||||||||||||||||||||||||||||
"patchable-function", | ||||||||||||||||||||||||||||||||||||||||||||||||
"prologue-short-redirect", | ||||||||||||||||||||||||||||||||||||||||||||||||
)); | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+381
to
+388
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check we talked about above can just be:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I think I want to allow aarch64 though. A tiny part of hotpatch is adding debuginfo that it is hotpatchable. See: https://github.com/rust-lang/rust/pull/134004/files#diff-a56b374664e290a55d70fa80e456b6280913830b382b73fb70c4483d3d4cf246R251 |
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
// FIXME: none of these functions interact with source level attributes. | ||||||||||||||||||||||||||||||||||||||||||||||||
to_add.extend(frame_pointer_type_attr(cx)); | ||||||||||||||||||||||||||||||||||||||||||||||||
to_add.extend(function_return_attr(cx)); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// check if functions get the attribute, so that LLVM ensures they are hotpatchable | ||
// the attribute is only implemented for x86, aarch64 does not require it | ||
|
||
//@ revisions: x32 x64 | ||
//@[x32] only-x86 | ||
//@[x64] only-x86_64 | ||
//@ compile-flags: -Z hotpatch | ||
|
||
#![crate_type = "lib"] | ||
|
||
#[no_mangle] | ||
pub fn foo() {} | ||
|
||
// CHECK-LABEL: @foo() unnamed_addr #0 | ||
// CHECK: attributes #0 = { {{.*}} "patchable-function"="prologue-short-redirect" {{.*}}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// to be able to hotpatch a function there are two requirements: | ||
// 1. the first instruction of a functin must be at least two bytes long | ||
// 2. there must not be a jump to the first instruction | ||
|
||
// the functions in this file already fulfill the conditions so hotpatch should not affect them | ||
|
||
// -------------------------------------------------------------------------------------------- | ||
|
||
#[no_mangle] | ||
#[inline(never)] | ||
pub fn return_42() -> i32 { | ||
42 | ||
} | ||
|
||
// -------------------------------------------------------------------------------------------- | ||
// This tailcall does not jump to the first instruction so hotpatch should leave it unaffected | ||
|
||
#[no_mangle] | ||
pub fn tailcall(a: i32) -> i32 { | ||
if a > 10000 { | ||
return a; | ||
} | ||
|
||
if a % 2 == 0 { tailcall(a / 2) } else { tailcall(a * 3 + 1) } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
// Check if hotpatch leaves the functions that are already hotpatchable untouched | ||
|
||
use run_make_support::{assertion_helpers, llvm, rustc}; | ||
|
||
fn main() { | ||
// hotpatch is only implemented for X86 and aarch64 | ||
#[cfg(any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64"))] | ||
{ | ||
fn base_rustc() -> rustc::Rustc { | ||
let mut rustc = rustc(); | ||
rustc.input("lib.rs").crate_type("lib").opt_level("3"); | ||
rustc | ||
} | ||
|
||
fn dump_lib(libname: &str) -> String { | ||
llvm::llvm_objdump() | ||
.arg("--disassemble-symbols=return_42,tailcall") | ||
.input(libname) | ||
.run() | ||
.stdout_utf8() | ||
} | ||
|
||
base_rustc().crate_name("regular").run(); | ||
let regular_dump = dump_lib("libregular.rlib"); | ||
|
||
base_rustc().crate_name("hotpatch").arg("-Zhotpatch").run(); | ||
let hotpatch_dump = dump_lib("libhotpatch.rlib"); | ||
|
||
{ | ||
let mut lines_regular = regular_dump.lines(); | ||
let mut lines_hotpatch = hotpatch_dump.lines(); | ||
|
||
loop { | ||
match (lines_regular.next(), lines_hotpatch.next()) { | ||
(None, None) => break, | ||
(Some(r), Some(h)) => { | ||
if r.contains("libregular.rlib") { | ||
assertion_helpers::assert_contains(h, "libhotpatch.rlib") | ||
} else { | ||
assertion_helpers::assert_equals(&r, &h) | ||
} | ||
} | ||
_ => panic!("expected files to have equal number of lines"), | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// to be able to hotpatch a function there are two requirements: | ||
// 1) the first instruction of a functin must be at least two bytes long | ||
// 2) there must not be a jump to the first instruction | ||
|
||
// The LLVM attribute we use '"patchable-function", "prologue-short-redirect"' only ensures 1) | ||
// However in practice 2) rarely matters. Its rare that it occurs and the problems it caused can be | ||
// avoided by the hotpatch tool. | ||
// In this test we check if 1) is ensured by inserted nops as needed | ||
|
||
// ---------------------------------------------------------------------------------------------- | ||
|
||
// empty_fn just returns. Note that 'ret' is a single byte instruction, but hotpatch requires | ||
// a two or more byte instructions to be at the start of the functions. | ||
// Preferably we would also tests a different single byte instruction, | ||
// but I was not able to find an example with another one byte intstruction. | ||
|
||
// check that if the first instruction is just a single byte, so our test is valid | ||
// CHECK-LABEL: <empty_fn>: | ||
// CHECK-NOT: 0: {{[0-9a-f][0-9a-f]}} {{[0-9a-f][0-9a-f]}} {{.*}} | ||
|
||
// check that the first instruction is at least 2 bytes long | ||
// HOTPATCH-LABEL: <empty_fn>: | ||
// HOTPATCH-NEXT: 0: {{[0-9a-f][0-9a-f]}} {{[0-9a-f][0-9a-f]}} {{.*}} | ||
|
||
#[no_mangle] | ||
#[inline(never)] | ||
pub fn empty_fn() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Check if hotpatch makes the functions hotpachable that were not | ||
// More details in lib.rs | ||
|
||
use run_make_support::{llvm, rustc}; | ||
|
||
fn main() { | ||
// hotpatch is only implemented for x86 and aarch64, but for aarch64 functions | ||
// are always hotpatchable so we don't need to check it | ||
#[cfg(any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64"))] | ||
{ | ||
fn base_rustc() -> rustc::Rustc { | ||
let mut rustc = rustc(); | ||
rustc.input("lib.rs").crate_type("lib").opt_level("3"); | ||
rustc | ||
} | ||
|
||
fn dump_lib(libname: &str) -> String { | ||
llvm::llvm_objdump() | ||
.arg("--disassemble-symbols=empty_fn") | ||
.input(libname) | ||
.run() | ||
.stdout_utf8() | ||
} | ||
|
||
{ | ||
base_rustc().crate_name("regular").run(); | ||
let regular_dump = dump_lib("libregular.rlib"); | ||
llvm::llvm_filecheck().patterns("lib.rs").stdin_buf(regular_dump).run(); | ||
} | ||
|
||
{ | ||
base_rustc().crate_name("hotpatch").arg("-Zhotpatch").run(); | ||
let hotpatch_dump = dump_lib("libhotpatch.rlib"); | ||
|
||
llvm::llvm_filecheck() | ||
.patterns("lib.rs") | ||
.check_prefix("HOTPATCH") | ||
.stdin_buf(hotpatch_dump) | ||
.run(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
// CHECK: S_OBJNAME{{.*}}hotpatch_pdb{{.*}}.o | ||
// CHECK: S_COMPILE3 | ||
// CHECK-NOT: S_ | ||
// CHECK: flags = {{.*}}hot patchable | ||
|
||
pub fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// Check if hotpatch flag is present in the Codeview. | ||
// While this is not strictly neccessary for functionpadmin to work, the currently linker | ||
// would ignore functionpadmin if this is not pressent | ||
|
||
use run_make_support::{llvm, rustc}; | ||
|
||
fn main() { | ||
// PDBs are windows only and hotpatch is only implemented for x86 and aarch64 | ||
#[cfg(all( | ||
target_os = "windows", | ||
any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64") | ||
))] | ||
{ | ||
let output = rustc() | ||
.input("main.rs") | ||
.arg("-g") | ||
.arg("-Zhotpatch") | ||
.crate_name("hotpatch_pdb") | ||
.crate_type("bin") | ||
.run(); | ||
|
||
let pdbutil_output = llvm::llvm_pdbutil() | ||
.arg("dump") | ||
.arg("-symbols") | ||
.input("hotpatch_pdb.pdb") | ||
.run() | ||
.stdout_utf8(); | ||
|
||
llvm::llvm_filecheck().patterns("main.rs").stdin_buf(&pdbutil_output).run(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this warn/error if both conditions aren't true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah I think we could add a check somewhere else to error/warn when hotpatch is used to compiler for platforms other than x86 or aarch64. For the latter we don't need to add this attribute as it already is inherently hotpatchable.