From 316bd646c057cfc90367930739051316fb6a6c22 Mon Sep 17 00:00:00 2001 From: Florian Schmiderer Date: Thu, 2 May 2024 23:19:02 +0200 Subject: [PATCH] Updated code for changes to RFC, added additional error handling. --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 60 ++++++++++++++----- compiler/rustc_feature/src/builtin_attrs.rs | 8 +-- compiler/rustc_feature/src/unstable.rs | 3 +- compiler/rustc_interface/src/tests.rs | 9 ++- compiler/rustc_session/src/config.rs | 14 +++-- compiler/rustc_session/src/options.rs | 20 +++---- compiler/rustc_span/src/symbol.rs | 4 +- .../patchable-function-entry.md | 2 +- tests/codegen/patchable-function-entry.rs | 6 +- .../feature-gate-patchable-function-entry.rs | 2 +- ...ature-gate-patchable-function-entry.stderr | 7 ++- 11 files changed, 86 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 49aeec80f19c8..01cbc04e24be2 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -457,24 +457,56 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } sym::patchable_function_entry => { codegen_fn_attrs.patchable_function_entry = attr.meta_item_list().and_then(|l| { - let mut prefix = 0; - let mut entry = 0; + let mut prefix = None; + let mut entry = None; for item in l { - if let Some((sym, lit)) = item.name_value_literal() { - let val = match lit.kind { - // FIXME emit error if too many nops requested - rustc_ast::LitKind::Int(i, _) => i as u8, - _ => continue, + let Some(metaitem) = item.meta_item() else { + continue; + }; + + let [single_segment] = &metaitem.path.segments[..] else { + continue; + }; + + let attrib_to_write = match single_segment.ident.name { + sym::prefix_nops => &mut prefix, + sym::entry_nops => &mut entry, + _ => { + tcx.dcx().span_err(metaitem.span, "Unexpected parameter."); + continue; + } + }; + + if let Some(metaitem) = + item.meta_item().map(|e| e.name_value_literal()).flatten() + { + let rustc_ast::LitKind::Int(val, _) = metaitem.kind else { + tcx.dcx().span_err( + metaitem.span, + "Expected integer value between 0 and 255", + ); + continue; }; - match sym { - sym::prefix => prefix = val, - sym::entry => entry = val, - // FIXME possibly emit error here? - _ => continue, - } + + let Ok(val) = val.get().try_into() else { + tcx.dcx().span_err( + metaitem.span, + "Integer value outside range between 0 and 255.", + ); + continue; + }; + + *attrib_to_write = Some(val); } } - Some(PatchableFunctionEntry::from_prefix_and_entry(prefix, entry)) + if let (None, None) = (prefix, entry) { + tcx.dcx().span_err(attr.span, "Must specify at least one parameter."); + } + + Some(PatchableFunctionEntry::from_prefix_and_entry( + prefix.unwrap_or(0), + entry.unwrap_or(0), + )) }) } _ => {} diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 43c1e66f8fa3c..256d7023ab160 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -533,11 +533,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ EncodeCrossCrate::No, coroutines, experimental!(coroutines) ), - // FIXME RFC - // `#[patchable_function_entry(prefix(n), entry(n))]` + // RFC 3543 + // `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]` gated!( - patchable_function_entry, Normal, template!(List: "prefix(n), entry(n)"), ErrorPreceding, - experimental!(patchable_function_entry) + patchable_function_entry, Normal, template!(List: "prefix_nops = m, entry_nops = n"), ErrorPreceding, + EncodeCrossCrate::Yes, experimental!(patchable_function_entry) ), // ========================================================================== diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 376531f9bea4f..ef0ca0c45c7f4 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -562,8 +562,7 @@ declare_features! ( /// Allows using `#[optimize(X)]`. (unstable, optimize_attribute, "1.34.0", Some(54882)), /// Allows specifying nop padding on functions for dynamic patching. - // FIXME this needs an RFC # - (unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(9999)), + (unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(123115)), /// Allows postfix match `expr.match { ... }` (unstable, postfix_match, "1.79.0", Some(121618)), /// Allows `use<'a, 'b, A, B>` in `impl use<...> Trait` for precise capture of generic args. diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 7c8d483aba219..1887a3684656c 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -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; @@ -815,7 +815,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_nop_count_and_offset(3, 4)); + tracked!( + patchable_function_entry, + PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total >= prefix") + ); tracked!(plt, Some(true)); tracked!(polonius, Polonius::Legacy); tracked!(precise_enum_drop_elaboration, false); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 922b07664b526..52e8ff100ab04 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2898,8 +2898,9 @@ pub(crate) mod dep_tracking { CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn, InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, - PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, - SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, + PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, + SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, + WasiExecModel, }; use crate::lint; use crate::utils::NativeLib; @@ -3196,11 +3197,14 @@ pub struct PatchableFunctionEntry { } impl PatchableFunctionEntry { - pub fn from_nop_count_and_offset(nop_count: u8, offset: u8) -> Option { - if nop_count < offset { + pub fn from_total_and_prefix_nops( + total_nops: u8, + prefix_nops: u8, + ) -> Option { + if total_nops < prefix_nops { None } else { - Some(Self { prefix: offset, entry: nop_count - offset }) + Some(Self { prefix: prefix_nops, entry: total_nops - prefix_nops }) } } pub fn prefix(&self) -> u8 { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index ebdb4e3e1c8c3..689cfb7b2e1e7 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -378,12 +378,9 @@ mod desc { pub const parse_time_passes_format: &str = "`text` (default) or `json`"; pub const parse_passes: &str = "a space-separated list of passes, or `all`"; pub const parse_panic_strategy: &str = "either `unwind` or `abort`"; -<<<<<<< HEAD pub const parse_on_broken_pipe: &str = "either `kill`, `error`, or `inherit`"; -======= pub const parse_patchable_function_entry: &str = - "nop_count,entry_offset or nop_count (defaulting entry_offset=0)"; ->>>>>>> 7814dd86eb5 (Support for -Z patchable-function-entry) + "total_nops and optionally prefix_nops (defaulting prefix_nops=0)"; pub const parse_opt_panic_strategy: &str = parse_panic_strategy; pub const parse_oom_strategy: &str = "either `panic` or `abort`"; pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`"; @@ -714,7 +711,6 @@ mod parse { true } - pub(crate) fn parse_on_broken_pipe(slot: &mut OnBrokenPipe, v: Option<&str>) -> bool { match v { // OnBrokenPipe::Default can't be explicitly specified @@ -730,20 +726,22 @@ mod parse { slot: &mut PatchableFunctionEntry, v: Option<&str>, ) -> bool { - let mut nop_count = 0; - let mut offset = 0; + let mut total_nops = 0; + let mut prefix_nops = 0; - if !parse_number(&mut nop_count, v) { + if !parse_number(&mut total_nops, v) { let parts = v.and_then(|v| v.split_once(',')).unzip(); - if !parse_number(&mut nop_count, parts.0) { + if !parse_number(&mut total_nops, parts.0) { return false; } - if !parse_number(&mut offset, parts.1) { + if !parse_number(&mut prefix_nops, parts.1) { return false; } } - if let Some(pfe) = PatchableFunctionEntry::from_nop_count_and_offset(nop_count, offset) { + if let Some(pfe) = + PatchableFunctionEntry::from_total_and_prefix_nops(total_nops, prefix_nops) + { *slot = pfe; return true; } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index b56e8ce54f2a3..58ae1f8b3e377 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -754,7 +754,7 @@ symbols! { enable, encode, end, - entry, + entry_nops, enumerate_method, env, env_CFG_RELEASE: env!("CFG_RELEASE"), @@ -1399,7 +1399,7 @@ symbols! { prefetch_read_instruction, prefetch_write_data, prefetch_write_instruction, - prefix, + prefix_nops, preg, prelude, prelude_import, diff --git a/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md index a701b9e37719c..7ad514213c94b 100644 --- a/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md +++ b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md @@ -9,7 +9,7 @@ N defaults to 0. As an illustrative example, `-Z patchable-function-entry=3,2` would produce: -``` +```text nop nop function_label: diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs index dc20c0a2c6dad..dc46758f8a14a 100644 --- a/tests/codegen/patchable-function-entry.rs +++ b/tests/codegen/patchable-function-entry.rs @@ -1,5 +1,5 @@ #![feature(patchable_function_entry)] -// compile-flags: -Z patchable-function-entry=15,10 +//@ compile-flags: -Z patchable-function-entry=15,10 #![crate_type = "lib"] @@ -9,12 +9,12 @@ pub fn foo() {} // The attribute should override the compile flags #[no_mangle] -#[patchable_function_entry(prefix(1), entry(2))] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] pub fn bar() {} // If we override an attribute to 0 or unset, the attribute should go away #[no_mangle] -#[patchable_function_entry(entry(0))] +#[patchable_function_entry(entry_nops = 0)] pub fn baz() {} // CHECK: @foo() unnamed_addr #0 diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs index 0e16e873a1eaa..b9642c7bfd4a6 100644 --- a/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs @@ -1,3 +1,3 @@ -#[patchable_function_entry(entry(1), prefix(1))] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 1)] //~^ ERROR: the `#[patchable_function_entry]` attribute is an experimental feature fn main() {} diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr index c4d57d774e35d..55fcdb4f7291e 100644 --- a/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr @@ -1,11 +1,12 @@ error[E0658]: the `#[patchable_function_entry]` attribute is an experimental feature --> $DIR/feature-gate-patchable-function-entry.rs:1:1 | -LL | #[patchable_function_entry(entry(1), prefix(1))] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #[patchable_function_entry(prefix_nops = 1, entry_nops = 1)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: see issue #9999 for more information + = note: see issue #123115 for more information = help: add `#![feature(patchable_function_entry)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error: aborting due to 1 previous error