Skip to content

Commit 7c56398

Browse files
nebularknebulark
committed
Updated code for changes to RFC, added additional error handling, added
tests
1 parent 9b0ae75 commit 7c56398

18 files changed

+312
-79
lines changed

‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

+47-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use rustc_ast::{ast, attr, MetaItemKind, NestedMetaItem};
22
use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr};
3+
use rustc_data_structures::packed::Pu128;
34
use rustc_errors::{codes::*, struct_span_code_err};
45
use rustc_hir as hir;
56
use rustc_hir::def::DefKind;
@@ -467,24 +468,55 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
467468
}
468469
sym::patchable_function_entry => {
469470
codegen_fn_attrs.patchable_function_entry = attr.meta_item_list().and_then(|l| {
470-
let mut prefix = 0;
471-
let mut entry = 0;
471+
let mut prefix = None;
472+
let mut entry = None;
472473
for item in l {
473-
if let Some((sym, lit)) = item.name_value_literal() {
474-
let val = match lit.kind {
475-
// FIXME emit error if too many nops requested
476-
rustc_ast::LitKind::Int(i, _) => i as u8,
477-
_ => continue,
478-
};
479-
match sym {
480-
sym::prefix => prefix = val,
481-
sym::entry => entry = val,
482-
// FIXME possibly emit error here?
483-
_ => continue,
474+
let Some(meta_item) = item.meta_item() else {
475+
tcx.dcx().span_err(item.span(), "Expected name value pair.");
476+
continue;
477+
};
478+
479+
let Some(name_value_lit) = meta_item.name_value_literal() else {
480+
tcx.dcx().span_err(item.span(), "Expected name value pair.");
481+
continue;
482+
};
483+
484+
let attrib_to_write = match meta_item.name_or_empty() {
485+
sym::prefix_nops => &mut prefix,
486+
sym::entry_nops => &mut entry,
487+
_ => {
488+
tcx.dcx().span_err(
489+
item.span(),
490+
format!(
491+
"Unexpected parameter name. Allowed names: {}, {}",
492+
sym::prefix_nops,
493+
sym::entry_nops
494+
),
495+
);
496+
continue;
484497
}
485-
}
498+
};
499+
500+
let rustc_ast::LitKind::Int(Pu128(val @ 0..=255), _) = name_value_lit.kind
501+
else {
502+
tcx.dcx().span_err(
503+
name_value_lit.span,
504+
"Expected integer value between 0 and 255.",
505+
);
506+
continue;
507+
};
508+
509+
*attrib_to_write = Some(val.try_into().unwrap());
486510
}
487-
Some(PatchableFunctionEntry::from_prefix_and_entry(prefix, entry))
511+
512+
if let (None, None) = (prefix, entry) {
513+
tcx.dcx().span_err(attr.span, "Must specify at least one parameter.");
514+
}
515+
516+
Some(PatchableFunctionEntry::from_prefix_and_entry(
517+
prefix.unwrap_or(0),
518+
entry.unwrap_or(0),
519+
))
488520
})
489521
}
490522
_ => {}

‎compiler/rustc_feature/src/builtin_attrs.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -584,12 +584,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
584584
pointee, Normal, template!(Word), ErrorFollowing,
585585
EncodeCrossCrate::No, derive_smart_pointer, experimental!(pointee)
586586
),
587-
588-
// FIXME RFC
589-
// `#[patchable_function_entry(prefix(n), entry(n))]`
587+
588+
// RFC 3543
589+
// `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]`
590590
gated!(
591-
patchable_function_entry, Normal, template!(List: "prefix(n), entry(n)"), ErrorPreceding,
592-
experimental!(patchable_function_entry)
591+
patchable_function_entry, Normal, template!(List: "prefix_nops = m, entry_nops = n"), ErrorPreceding,
592+
EncodeCrossCrate::Yes, experimental!(patchable_function_entry)
593593
),
594594

595595
// ==========================================================================

‎compiler/rustc_feature/src/unstable.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,7 @@ declare_features! (
566566
/// Allows using `#[optimize(X)]`.
567567
(unstable, optimize_attribute, "1.34.0", Some(54882)),
568568
/// Allows specifying nop padding on functions for dynamic patching.
569-
// FIXME this needs an RFC #
570-
(unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(9999)),
569+
(unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(123115)),
571570
/// Allows postfix match `expr.match { ... }`
572571
(unstable, postfix_match, "1.79.0", Some(121618)),
573572
/// Allows `use<'a, 'b, A, B>` in `impl Trait + use<...>` for precise capture of generic args.

‎compiler/rustc_interface/src/tests.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use rustc_session::config::{
88
ErrorOutputType, ExternEntry, ExternLocation, Externs, FunctionReturn, InliningThreshold,
99
Input, InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail,
1010
LtoCli, NextSolverConfig, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey,
11-
PacRet, Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath,
12-
SymbolManglingVersion, WasiExecModel,
11+
PacRet, Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip,
12+
SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
1313
};
1414
use rustc_session::lint::Level;
1515
use rustc_session::search_paths::SearchPath;
@@ -813,7 +813,10 @@ fn test_unstable_options_tracking_hash() {
813813
tracked!(packed_bundled_libs, true);
814814
tracked!(panic_abort_tests, true);
815815
tracked!(panic_in_drop, PanicStrategy::Abort);
816-
tracked!(patchable_function_entry, PatchableFunctionEntry::from_nop_count_and_offset(3, 4));
816+
tracked!(
817+
patchable_function_entry,
818+
PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total >= prefix")
819+
);
817820
tracked!(plt, Some(true));
818821
tracked!(polonius, Polonius::Legacy);
819822
tracked!(precise_enum_drop_elaboration, false);

‎compiler/rustc_session/src/config.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -2963,8 +2963,9 @@ pub(crate) mod dep_tracking {
29632963
CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn,
29642964
InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail,
29652965
LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes,
2966-
PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm,
2967-
SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
2966+
PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks,
2967+
SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion,
2968+
WasiExecModel,
29682969
};
29692970
use crate::lint;
29702971
use crate::utils::NativeLib;
@@ -3260,11 +3261,14 @@ pub struct PatchableFunctionEntry {
32603261
}
32613262

32623263
impl PatchableFunctionEntry {
3263-
pub fn from_nop_count_and_offset(nop_count: u8, offset: u8) -> Option<PatchableFunctionEntry> {
3264-
if nop_count < offset {
3264+
pub fn from_total_and_prefix_nops(
3265+
total_nops: u8,
3266+
prefix_nops: u8,
3267+
) -> Option<PatchableFunctionEntry> {
3268+
if total_nops < prefix_nops {
32653269
None
32663270
} else {
3267-
Some(Self { prefix: offset, entry: nop_count - offset })
3271+
Some(Self { prefix: prefix_nops, entry: total_nops - prefix_nops })
32683272
}
32693273
}
32703274
pub fn prefix(&self) -> u8 {

‎compiler/rustc_session/src/options.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -379,8 +379,7 @@ mod desc {
379379
pub const parse_passes: &str = "a space-separated list of passes, or `all`";
380380
pub const parse_panic_strategy: &str = "either `unwind` or `abort`";
381381
pub const parse_on_broken_pipe: &str = "either `kill`, `error`, or `inherit`";
382-
pub const parse_patchable_function_entry: &str =
383-
"nop_count,entry_offset or nop_count (defaulting entry_offset=0)";
382+
pub const parse_patchable_function_entry: &str = "either two comma separated integers (total_nops,prefix_nops), with prefix_nops <= total_nops, or one integer (total_nops)";
384383
pub const parse_opt_panic_strategy: &str = parse_panic_strategy;
385384
pub const parse_oom_strategy: &str = "either `panic` or `abort`";
386385
pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`";
@@ -725,7 +724,6 @@ mod parse {
725724
true
726725
}
727726

728-
729727
pub(crate) fn parse_on_broken_pipe(slot: &mut OnBrokenPipe, v: Option<&str>) -> bool {
730728
match v {
731729
// OnBrokenPipe::Default can't be explicitly specified
@@ -741,20 +739,22 @@ mod parse {
741739
slot: &mut PatchableFunctionEntry,
742740
v: Option<&str>,
743741
) -> bool {
744-
let mut nop_count = 0;
745-
let mut offset = 0;
742+
let mut total_nops = 0;
743+
let mut prefix_nops = 0;
746744

747-
if !parse_number(&mut nop_count, v) {
745+
if !parse_number(&mut total_nops, v) {
748746
let parts = v.and_then(|v| v.split_once(',')).unzip();
749-
if !parse_number(&mut nop_count, parts.0) {
747+
if !parse_number(&mut total_nops, parts.0) {
750748
return false;
751749
}
752-
if !parse_number(&mut offset, parts.1) {
750+
if !parse_number(&mut prefix_nops, parts.1) {
753751
return false;
754752
}
755753
}
756754

757-
if let Some(pfe) = PatchableFunctionEntry::from_nop_count_and_offset(nop_count, offset) {
755+
if let Some(pfe) =
756+
PatchableFunctionEntry::from_total_and_prefix_nops(total_nops, prefix_nops)
757+
{
758758
*slot = pfe;
759759
return true;
760760
}

‎compiler/rustc_span/src/symbol.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ symbols! {
768768
enable,
769769
encode,
770770
end,
771-
entry,
771+
entry_nops,
772772
enumerate_method,
773773
env,
774774
env_CFG_RELEASE: env!("CFG_RELEASE"),
@@ -1423,7 +1423,7 @@ symbols! {
14231423
prefetch_read_instruction,
14241424
prefetch_write_data,
14251425
prefetch_write_instruction,
1426-
prefix,
1426+
prefix_nops,
14271427
preg,
14281428
prelude,
14291429
prelude_import,

‎src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md

+6-6
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22

33
--------------------
44

5-
The `-Z patchable-function-entry=M,N` or `-Z patchable-function-entry=M`
6-
compiler flag enables nop padding of function entries with M nops, with
7-
an offset for the entry of the function at N nops. In the second form,
8-
N defaults to 0.
5+
The `-Z patchable-function-entry=total_nops,prefix_nops` or `-Z patchable-function-entry=total_nops`
6+
compiler flag enables nop padding of function entries with 'total_nops' nops, with
7+
an offset for the entry of the function at 'prefix_nops' nops. In the second form,
8+
'prefix_nops' defaults to 0.
99

1010
As an illustrative example, `-Z patchable-function-entry=3,2` would produce:
1111

12-
```
12+
```text
1313
nop
1414
nop
1515
function_label:
@@ -18,7 +18,7 @@ nop
1818
```
1919

2020
This flag is used for hotpatching, especially in the Linux kernel. The flag
21-
arguments are modeled after hte `-fpatchable-function-entry` flag as defined
21+
arguments are modeled after the `-fpatchable-function-entry` flag as defined
2222
for both [Clang](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fpatchable-function-entry)
2323
and [gcc](https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fpatchable-function-entry)
2424
and is intended to provide the same effect.

‎tests/codegen/patchable-function-entry.rs

-28
This file was deleted.

‎tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//@ compile-flags: -Z patchable-function-entry=15,10
2+
3+
#![feature(patchable_function_entry)]
4+
#![crate_type = "lib"]
5+
6+
// This should have the default, as set by the compile flags
7+
#[no_mangle]
8+
pub fn fun0() {}
9+
10+
// The attribute should override the compile flags
11+
#[no_mangle]
12+
#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)]
13+
pub fn fun1() {}
14+
15+
// If we override an attribute to 0 or unset, the attribute should go away
16+
#[no_mangle]
17+
#[patchable_function_entry(entry_nops = 0)]
18+
pub fn fun2() {}
19+
20+
// The attribute should override the compile flags
21+
#[no_mangle]
22+
#[patchable_function_entry(prefix_nops = 20, entry_nops = 1)]
23+
pub fn fun3() {}
24+
25+
// The attribute should override the compile flags
26+
#[no_mangle]
27+
#[patchable_function_entry(prefix_nops = 2, entry_nops = 19)]
28+
pub fn fun4() {}
29+
30+
// The attribute should override patchable-function-entry to 3 and
31+
// patchable-function-prefix to the default of 0, clearing it entirely
32+
#[no_mangle]
33+
#[patchable_function_entry(entry_nops = 3)]
34+
pub fn fun5() {}
35+
36+
// The attribute should override patchable-function-prefix to 4
37+
// and patchable-function-entry to the default of 0, clearing it entirely
38+
#[no_mangle]
39+
#[patchable_function_entry(prefix_nops = 4)]
40+
pub fn fun6() {}
41+
42+
// CHECK: @fun0() unnamed_addr #0
43+
// CHECK: @fun1() unnamed_addr #1
44+
// CHECK: @fun2() unnamed_addr #2
45+
// CHECK: @fun3() unnamed_addr #3
46+
// CHECK: @fun4() unnamed_addr #4
47+
// CHECK: @fun5() unnamed_addr #5
48+
// CHECK: @fun6() unnamed_addr #6
49+
50+
// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} }
51+
// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} }
52+
53+
// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} }
54+
// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} }
55+
// CHECK: attributes #2 = { {{.*}} }
56+
57+
// CHECK: attributes #3 = { {{.*}}"patchable-function-entry"="1"{{.*}}"patchable-function-prefix"="20" {{.*}} }
58+
// CHECK: attributes #4 = { {{.*}}"patchable-function-entry"="19"{{.*}}"patchable-function-prefix"="2" {{.*}} }
59+
60+
// CHECK: attributes #5 = { {{.*}}"patchable-function-entry"="3"{{.*}} }
61+
// CHECK-NOT: attributes #5 = { {{.*}}patchable-function-prefix{{.*}} }
62+
63+
// CHECK: attributes #6 = { {{.*}}"patchable-function-prefix"="4"{{.*}} }
64+
// CHECK-NOT: attributes #6 = { {{.*}}patchable-function-entry{{.*}} }

‎tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#![feature(patchable_function_entry)]
2+
#![crate_type = "lib"]
3+
4+
// No patchable function entry should be set
5+
#[no_mangle]
6+
pub fn fun0() {}
7+
8+
// The attribute should work even without compiler flags
9+
#[no_mangle]
10+
#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)]
11+
pub fn fun1() {}
12+
13+
// The attribute should work even without compiler flags
14+
// and only set patchable-function-entry to 3.
15+
#[no_mangle]
16+
#[patchable_function_entry(entry_nops = 3)]
17+
pub fn fun2() {}
18+
19+
// The attribute should work even without compiler flags
20+
// and only set patchable-function-prefix to 4.
21+
#[no_mangle]
22+
#[patchable_function_entry(prefix_nops = 4)]
23+
pub fn fun3() {}
24+
25+
// CHECK: @fun0() unnamed_addr #0
26+
// CHECK: @fun1() unnamed_addr #1
27+
// CHECK: @fun2() unnamed_addr #2
28+
// CHECK: @fun3() unnamed_addr #3
29+
30+
// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-entry{{.*}} }
31+
// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-prefix{{.*}} }
32+
33+
// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} }
34+
35+
// CHECK: attributes #2 = { {{.*}}"patchable-function-entry"="3"{{.*}} }
36+
// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} }
37+
38+
// CHECK: attributes #3 = { {{.*}}"patchable-function-prefix"="4"{{.*}} }
39+
// CHECK-NOT: attributes #3 = { {{.*}}patchable-function-entry{{.*}} }

0 commit comments

Comments
 (0)