-
Notifications
You must be signed in to change notification settings - Fork 13.8k
patchable-function-entry: Add unstable compiler flag and attribute #124741
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
Changes from 3 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,11 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_ast::{ast, attr, MetaItemKind, NestedMetaItem}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_data_structures::packed::Pu128; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_errors::{codes::*, struct_span_code_err}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_hir as hir; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_hir::def::DefKind; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_hir::{lang_items, weak_lang_items::WEAK_LANG_ITEMS, LangItem}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::middle::codegen_fn_attrs::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::mir::mono::Linkage; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::query::Providers; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rustc_middle::ty::{self as ty, TyCtxt}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -463,6 +466,59 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sym::patchable_function_entry => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
codegen_fn_attrs.patchable_function_entry = attr.meta_item_list().and_then(|l| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let mut prefix = None; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let mut entry = None; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for item in l { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let Some(meta_item) = item.meta_item() else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tcx.dcx().span_err(item.span(), "Expected name value pair."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
tcx.dcx().span_err(item.span(), "Expected name value pair."); | |
tcx.dcx().span_err(item.span(), "expected name value pair"); |
Outdated
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.
tcx.dcx().span_err(item.span(), "Expected name value pair."); | |
tcx.dcx().span_err(item.span(), "expected name value pair"); |
Outdated
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.
tcx.dcx().span_err( | |
item.span(), | |
format!( | |
"Unexpected parameter name. Allowed names: {}, {}", | |
sym::prefix_nops, | |
sym::entry_nops | |
), | |
); | |
tcx.dcx().struct_span_err( | |
item.span(), | |
"unexpected parameter name" | |
).span_label( | |
item.span(), | |
format!( | |
"expected {} or {}", | |
sym::prefix_nops, | |
sym::entry_nops | |
), | |
).emit(); |
Outdated
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.
tcx.dcx().span_err( | |
name_value_lit.span, | |
"Expected integer value between 0 and 255.", | |
); | |
tcx.dcx().struct_span_err( | |
name_value_lit.span, | |
"integer value out of range", | |
).span_label( | |
name_value_lit.span, | |
"value must be between `0` and `255`", | |
).emit(); |
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.
This error message would also be triggered when passing in a string, so the suggested wording might be a bit weird in that case. Do you think it makes sense to make the check for the string seperate again or is this just a minor issue?
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.
I think it is worth it to be more accurate whenever possible. you can make this be
tcx.dcx().span_err( | |
name_value_lit.span, | |
"Expected integer value between 0 and 255.", | |
); | |
let mut err = tcx.dcx().struct_span_err( | |
name_value_lit.span, | |
"invalid literal value", | |
); | |
if is_numeric { | |
err.span_label( | |
name_value_lit.span, | |
"value must be between `0` and `255`" | |
); | |
} else if is_str { | |
err.span_label( | |
name_value_lit.span, | |
"some appropriate message" | |
); | |
} | |
// Are there other cases to handle? can we hit this error without a numeric or str literal? | |
err.emit(); |
Outdated
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.
tcx.dcx().span_err(attr.span, "Must specify at least one parameter."); | |
tcx.dcx().span_err(attr.span, "must specify at least one parameter"); |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8,8 +8,8 @@ use rustc_session::config::{ | |||||||||
ErrorOutputType, ExternEntry, ExternLocation, Externs, FunctionReturn, InliningThreshold, | ||||||||||
Input, InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, | ||||||||||
LtoCli, NextSolverConfig, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, | ||||||||||
PacRet, Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, | ||||||||||
SymbolManglingVersion, WasiExecModel, | ||||||||||
PacRet, Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip, | ||||||||||
SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, | ||||||||||
}; | ||||||||||
use rustc_session::lint::Level; | ||||||||||
use rustc_session::search_paths::SearchPath; | ||||||||||
|
@@ -813,6 +813,10 @@ fn test_unstable_options_tracking_hash() { | |||||||||
tracked!(packed_bundled_libs, true); | ||||||||||
tracked!(panic_abort_tests, true); | ||||||||||
tracked!(panic_in_drop, PanicStrategy::Abort); | ||||||||||
tracked!( | ||||||||||
patchable_function_entry, | ||||||||||
PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total >= prefix") | ||||||||||
|
PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total >= prefix") | |
PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total < prefix") |
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.
Don't know if I understood you correctly, but to get a value the condition 'total >=prefix' must be true. The panic would happen if that condition is false. So I'd think it would be correct as written?
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.
What will happen if total < prefix
is that at runtime we'll get a panic that looks like the following:
thread 'main' panicked at compiler/rustc_interface/src/tests.rs:818:8:
total >= prefix
Instead we should provide additional context beyond that. Having said that, this is a nitpick to make it easier for people to understand that happened in this test, it's not a deal breaker.
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.
PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total >= prefix") | |
PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total must be greater than prefix") |
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.
maybe?
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.
or just unwrap, this is just a test, really, and the condition is trivially obviously true.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# `patchable-function-entry` | ||
|
||
-------------------- | ||
|
||
The `-Z patchable-function-entry=total_nops,prefix_nops` or `-Z patchable-function-entry=total_nops` | ||
compiler flag enables nop padding of function entries with 'total_nops' nops, with | ||
an offset for the entry of the function at 'prefix_nops' nops. In the second form, | ||
'prefix_nops' defaults to 0. | ||
|
||
As an illustrative example, `-Z patchable-function-entry=3,2` would produce: | ||
|
||
```text | ||
nop | ||
nop | ||
function_label: | ||
nop | ||
//Actual function code begins here | ||
``` | ||
|
||
This flag is used for hotpatching, especially in the Linux kernel. The flag | ||
arguments are modeled after the `-fpatchable-function-entry` flag as defined | ||
for both [Clang](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fpatchable-function-entry) | ||
and [gcc](https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fpatchable-function-entry) | ||
and is intended to provide the same effect. |
Uh oh!
There was an error while loading. Please reload this page.