From ac7595fdb1ee2aafecdd99cd8a3e56192639ada6 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Tue, 12 Dec 2023 13:32:43 -0800 Subject: [PATCH 1/4] Support for -Z patchable-function-entry `-Z patchable-function-entry` works like `-fpatchable-function-entry` on clang/gcc. The arguments are total nop count and function offset. See MCP rust-lang/compiler-team#704 --- compiler/rustc_codegen_llvm/src/attributes.rs | 26 +++++++++++++++++ compiler/rustc_interface/src/tests.rs | 1 + .../src/middle/codegen_fn_attrs.rs | 23 +++++++++++++++ compiler/rustc_session/src/config.rs | 29 ++++++++++++++++++- compiler/rustc_session/src/options.rs | 29 +++++++++++++++++++ .../patchable-function-entry.md | 24 +++++++++++++++ tests/codegen/patchable-function-entry.rs | 8 +++++ 7 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md create mode 100644 tests/codegen/patchable-function-entry.rs diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 48693895da13e..7cf789ab5d72e 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -53,6 +53,31 @@ fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll } } +#[inline] +fn patchable_function_entry_attrs<'ll>( + cx: &CodegenCx<'ll, '_>, +) -> SmallVec<[&'ll Attribute; 2]> { + let mut attrs = SmallVec::new(); + let patchable_spec = cx.tcx.sess.opts.unstable_opts.patchable_function_entry; + let entry = patchable_spec.entry(); + let prefix = patchable_spec.prefix(); + if entry > 0 { + attrs.push(llvm::CreateAttrStringValue( + cx.llcx, + "patchable-function-entry", + &format!("{}", entry), + )); + } + if prefix > 0 { + attrs.push(llvm::CreateAttrStringValue( + cx.llcx, + "patchable-function-prefix", + &format!("{}", prefix), + )); + } + attrs +} + /// Get LLVM sanitize attributes. #[inline] pub fn sanitize_attrs<'ll>( @@ -421,6 +446,7 @@ pub fn from_fn_attrs<'ll, 'tcx>( llvm::set_alignment(llfn, align); } to_add.extend(sanitize_attrs(cx, codegen_fn_attrs.no_sanitize)); + to_add.extend(patchable_function_entry_attrs(cx)); // Always annotate functions with the target-cpu they are compiled for. // Without this, ThinLTO won't inline Rust functions into Clang generated diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 6ffc518097ef0..e4b50e7f5ff26 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -813,6 +813,7 @@ 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!(plt, Some(true)); tracked!(polonius, Polonius::Legacy); tracked!(precise_enum_drop_elaboration, false); diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 3fa5054baed1e..0fce26dcbbdbb 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -47,6 +47,29 @@ pub struct CodegenFnAttrs { pub alignment: Option, } +#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)] +pub struct PatchableFunctionEntry { + /// Nops to prepend to the function + prefix: u8, + /// Nops after entry, but before body + entry: u8, +} + +impl PatchableFunctionEntry { + pub fn from_config(config: rustc_session::config::PatchableFunctionEntry) -> Self { + Self { prefix: config.prefix(), entry: config.entry() } + } + pub fn from_prefix_and_entry(prefix: u8, entry: u8) -> Self { + Self { prefix, entry } + } + pub fn prefix(&self) -> u8 { + self.prefix + } + pub fn entry(&self) -> u8 { + self.entry + } +} + #[derive(Clone, Copy, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)] pub struct CodegenFnAttrFlags(u32); bitflags::bitflags! { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 839cc51efcea8..2534152267e14 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2963,7 +2963,7 @@ pub(crate) mod dep_tracking { CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn, InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, - Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, + PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, }; use crate::lint; @@ -3071,6 +3071,7 @@ pub(crate) mod dep_tracking { OomStrategy, LanguageIdentifier, NextSolverConfig, + PatchableFunctionEntry, Polonius, InliningThreshold, FunctionReturn, @@ -3248,6 +3249,32 @@ impl DumpMonoStatsFormat { } } +/// `-Z patchable-function-entry` representation - how many nops to put before and after function +/// entry. +#[derive(Clone, Copy, PartialEq, Hash, Debug, Default)] +pub struct PatchableFunctionEntry { + /// Nops before the entry + prefix: u8, + /// Nops after the entry + entry: u8, +} + +impl PatchableFunctionEntry { + pub fn from_nop_count_and_offset(nop_count: u8, offset: u8) -> Option { + if nop_count < offset { + None + } else { + Some(Self { prefix: offset, entry: nop_count - offset }) + } + } + pub fn prefix(&self) -> u8 { + self.prefix + } + pub fn entry(&self) -> u8 { + self.entry + } +} + /// `-Zpolonius` values, enabling the borrow checker polonius analysis, and which version: legacy, /// or future prototype. #[derive(Clone, Copy, PartialEq, Hash, Debug, Default)] diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 9a10adeb6d1a1..f4f8a16662d75 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -379,6 +379,8 @@ mod desc { pub const parse_passes: &str = "a space-separated list of passes, or `all`"; pub const parse_panic_strategy: &str = "either `unwind` or `abort`"; 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)"; 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`"; @@ -723,6 +725,7 @@ 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 @@ -734,6 +737,30 @@ mod parse { true } + pub(crate) fn parse_patchable_function_entry( + slot: &mut PatchableFunctionEntry, + v: Option<&str>, + ) -> bool { + let mut nop_count = 0; + let mut offset = 0; + + if !parse_number(&mut nop_count, v) { + let parts = v.and_then(|v| v.split_once(',')).unzip(); + if !parse_number(&mut nop_count, parts.0) { + return false; + } + if !parse_number(&mut offset, parts.1) { + return false; + } + } + + if let Some(pfe) = PatchableFunctionEntry::from_nop_count_and_offset(nop_count, offset) { + *slot = pfe; + return true; + } + false + } + pub(crate) fn parse_oom_strategy(slot: &mut OomStrategy, v: Option<&str>) -> bool { match v { Some("panic") => *slot = OomStrategy::Panic, @@ -1859,6 +1886,8 @@ options! { "panic strategy for panics in drops"), parse_only: bool = (false, parse_bool, [UNTRACKED], "parse only; do not compile, assemble, or link (default: no)"), + patchable_function_entry: PatchableFunctionEntry = (PatchableFunctionEntry::default(), parse_patchable_function_entry, [TRACKED], + "nop padding at function entry"), plt: Option = (None, parse_opt_bool, [TRACKED], "whether to use the PLT when calling into shared libraries; only has effect for PIC code on systems with ELF binaries 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 new file mode 100644 index 0000000000000..a701b9e37719c --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md @@ -0,0 +1,24 @@ +# `patchable-function-entry` + +-------------------- + +The `-Z patchable-function-entry=M,N` or `-Z patchable-function-entry=M` +compiler flag enables nop padding of function entries with M nops, with +an offset for the entry of the function at N nops. In the second form, +N defaults to 0. + +As an illustrative example, `-Z patchable-function-entry=3,2` would produce: + +``` +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 hte `-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. diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs new file mode 100644 index 0000000000000..f06739303d2e2 --- /dev/null +++ b/tests/codegen/patchable-function-entry.rs @@ -0,0 +1,8 @@ +// compile-flags: -Z patchable-function-entry=15,10 + +#![crate_type = "lib"] + +#[no_mangle] +pub fn foo() {} +// CHECK: @foo() unnamed_addr #0 +// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } From 9b0ae75ecc485d668232c9c85f0090fb85668312 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Tue, 12 Dec 2023 13:37:04 -0800 Subject: [PATCH 2/4] Support `#[patchable_function_entries]` See [RFC](https://github.com/maurer/rust-rfcs/blob/patchable-function-entry/text/0000-patchable-function-entry.md) (yet to be numbered) TODO before submission: * Needs an RFC * Improve error reporting for malformed attributes --- compiler/rustc_codegen_llvm/src/attributes.rs | 9 ++++--- .../rustc_codegen_ssa/src/codegen_attrs.rs | 26 ++++++++++++++++++- compiler/rustc_feature/src/builtin_attrs.rs | 7 +++++ compiler/rustc_feature/src/unstable.rs | 3 +++ .../src/middle/codegen_fn_attrs.rs | 4 +++ compiler/rustc_span/src/symbol.rs | 3 +++ tests/codegen/patchable-function-entry.rs | 20 ++++++++++++++ .../feature-gate-patchable-function-entry.rs | 3 +++ ...ature-gate-patchable-function-entry.stderr | 12 +++++++++ 9 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 tests/ui/feature-gates/feature-gate-patchable-function-entry.rs create mode 100644 tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 7cf789ab5d72e..cd82894af18eb 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -2,7 +2,7 @@ use rustc_codegen_ssa::traits::*; use rustc_hir::def_id::DefId; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; +use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, PatchableFunctionEntry}; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::config::{FunctionReturn, OptLevel}; use rustc_span::symbol::sym; @@ -56,9 +56,12 @@ fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll #[inline] fn patchable_function_entry_attrs<'ll>( cx: &CodegenCx<'ll, '_>, + attr: Option, ) -> SmallVec<[&'ll Attribute; 2]> { let mut attrs = SmallVec::new(); - let patchable_spec = cx.tcx.sess.opts.unstable_opts.patchable_function_entry; + let patchable_spec = attr.unwrap_or_else(|| { + PatchableFunctionEntry::from_config(cx.tcx.sess.opts.unstable_opts.patchable_function_entry) + }); let entry = patchable_spec.entry(); let prefix = patchable_spec.prefix(); if entry > 0 { @@ -446,7 +449,7 @@ pub fn from_fn_attrs<'ll, 'tcx>( llvm::set_alignment(llfn, align); } to_add.extend(sanitize_attrs(cx, codegen_fn_attrs.no_sanitize)); - to_add.extend(patchable_function_entry_attrs(cx)); + to_add.extend(patchable_function_entry_attrs(cx, codegen_fn_attrs.patchable_function_entry)); // Always annotate functions with the target-cpu they are compiled for. // Without this, ThinLTO won't inline Rust functions into Clang generated diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index fb71cdaa8ff2a..8924ddb2aedd3 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -5,7 +5,9 @@ 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 +465,28 @@ 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 = 0; + let mut entry = 0; + 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, + }; + match sym { + sym::prefix => prefix = val, + sym::entry => entry = val, + // FIXME possibly emit error here? + _ => continue, + } + } + } + Some(PatchableFunctionEntry::from_prefix_and_entry(prefix, entry)) + }) + } _ => {} } } diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index c53bf96513951..b5f9f2c715f4f 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -584,6 +584,13 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ pointee, Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::No, derive_smart_pointer, experimental!(pointee) ), + + // FIXME RFC + // `#[patchable_function_entry(prefix(n), entry(n))]` + gated!( + patchable_function_entry, Normal, template!(List: "prefix(n), entry(n)"), ErrorPreceding, + experimental!(patchable_function_entry) + ), // ========================================================================== // Internal attributes: Stability, deprecation, and unsafe: diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 2dfaac8f6e731..796475e766f6b 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -565,6 +565,9 @@ declare_features! ( (unstable, offset_of_slice, "CURRENT_RUSTC_VERSION", Some(126151)), /// 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)), /// Allows postfix match `expr.match { ... }` (unstable, postfix_match, "1.79.0", Some(121618)), /// Allows `use<'a, 'b, A, B>` in `impl Trait + use<...>` for precise capture of generic args. diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 0fce26dcbbdbb..23fe72c537a1c 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -45,6 +45,9 @@ pub struct CodegenFnAttrs { /// The `#[repr(align(...))]` attribute. Indicates the value of which the function should be /// aligned to. pub alignment: Option, + /// The `#[patchable_function_entry(...)]` attribute. Indicates how many nops should be around + /// the function entry. + pub patchable_function_entry: Option, } #[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)] @@ -147,6 +150,7 @@ impl CodegenFnAttrs { no_sanitize: SanitizerSet::empty(), instruction_set: None, alignment: None, + patchable_function_entry: None, } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 6d4a8c29bc902..ea7fe7e76c412 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -768,6 +768,7 @@ symbols! { enable, encode, end, + entry, enumerate_method, env, env_CFG_RELEASE: env!("CFG_RELEASE"), @@ -1383,6 +1384,7 @@ symbols! { passes, pat, pat_param, + patchable_function_entry, path, pattern_complexity, pattern_parentheses, @@ -1421,6 +1423,7 @@ symbols! { prefetch_read_instruction, prefetch_write_data, prefetch_write_instruction, + prefix, preg, prelude, prelude_import, diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs index f06739303d2e2..dc20c0a2c6dad 100644 --- a/tests/codegen/patchable-function-entry.rs +++ b/tests/codegen/patchable-function-entry.rs @@ -1,8 +1,28 @@ +#![feature(patchable_function_entry)] // compile-flags: -Z patchable-function-entry=15,10 #![crate_type = "lib"] +// This should have the default, as set by the compile flags #[no_mangle] pub fn foo() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix(1), entry(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))] +pub fn baz() {} + // CHECK: @foo() unnamed_addr #0 +// CHECK: @bar() unnamed_addr #1 +// CHECK: @baz() unnamed_addr #2 + // CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } +// CHECK: attributes #2 = { {{.*}} } diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs new file mode 100644 index 0000000000000..0e16e873a1eaa --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs @@ -0,0 +1,3 @@ +#[patchable_function_entry(entry(1), prefix(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 new file mode 100644 index 0000000000000..c4d57d774e35d --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr @@ -0,0 +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))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #9999 for more information + = help: add `#![feature(patchable_function_entry)]` to the crate attributes to enable + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0658`. From 7c56398e912ea9e81d8e56b2ca7459b4e62b6636 Mon Sep 17 00:00:00 2001 From: Florian Schmiderer Date: Thu, 2 May 2024 23:19:02 +0200 Subject: [PATCH 3/4] Updated code for changes to RFC, added additional error handling, added tests --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 62 ++++++++++++----- compiler/rustc_feature/src/builtin_attrs.rs | 10 +-- 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 | 18 ++--- compiler/rustc_span/src/symbol.rs | 4 +- .../patchable-function-entry.md | 12 ++-- tests/codegen/patchable-function-entry.rs | 28 -------- .../patchable-function-entry-both-flags.rs | 64 ++++++++++++++++++ .../patchable-function-entry-no-flag.rs | 39 +++++++++++ .../patchable-function-entry-one-flag.rs | 66 +++++++++++++++++++ .../feature-gate-patchable-function-entry.rs | 2 +- ...ature-gate-patchable-function-entry.stderr | 7 +- .../patchable-function-entry-attribute.rs | 17 +++++ .../patchable-function-entry-attribute.stderr | 32 +++++++++ .../patchable-function-entry-flags.rs | 2 + .../patchable-function-entry-flags.stderr | 2 + 18 files changed, 312 insertions(+), 79 deletions(-) delete mode 100644 tests/codegen/patchable-function-entry.rs create mode 100644 tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs create mode 100644 tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs create mode 100644 tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-flags.rs create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 8924ddb2aedd3..84041811bbbf2 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -1,5 +1,6 @@ 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; @@ -467,24 +468,55 @@ 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, - }; - match sym { - sym::prefix => prefix = val, - sym::entry => entry = val, - // FIXME possibly emit error here? - _ => continue, + let Some(meta_item) = item.meta_item() else { + tcx.dcx().span_err(item.span(), "Expected name value pair."); + continue; + }; + + let Some(name_value_lit) = meta_item.name_value_literal() else { + tcx.dcx().span_err(item.span(), "Expected name value pair."); + continue; + }; + + let attrib_to_write = match meta_item.name_or_empty() { + sym::prefix_nops => &mut prefix, + sym::entry_nops => &mut entry, + _ => { + tcx.dcx().span_err( + item.span(), + format!( + "Unexpected parameter name. Allowed names: {}, {}", + sym::prefix_nops, + sym::entry_nops + ), + ); + continue; } - } + }; + + let rustc_ast::LitKind::Int(Pu128(val @ 0..=255), _) = name_value_lit.kind + else { + tcx.dcx().span_err( + name_value_lit.span, + "Expected integer value between 0 and 255.", + ); + continue; + }; + + *attrib_to_write = Some(val.try_into().unwrap()); } - 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 b5f9f2c715f4f..9371a288f09f7 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -584,12 +584,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ pointee, Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::No, derive_smart_pointer, experimental!(pointee) ), - - // 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 796475e766f6b..29c28b4efd2f8 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -566,8 +566,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 Trait + use<...>` for precise capture of generic args. diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index e4b50e7f5ff26..e23d556ad4610 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; @@ -813,7 +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_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 2534152267e14..e716abedf0733 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2963,8 +2963,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; @@ -3260,11 +3261,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 f4f8a16662d75..80f7ca544f3c5 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -379,8 +379,7 @@ mod desc { pub const parse_passes: &str = "a space-separated list of passes, or `all`"; pub const parse_panic_strategy: &str = "either `unwind` or `abort`"; 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)"; + 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)"; 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`"; @@ -725,7 +724,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 @@ -741,20 +739,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 ea7fe7e76c412..3b6147c4c0f64 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -768,7 +768,7 @@ symbols! { enable, encode, end, - entry, + entry_nops, enumerate_method, env, env_CFG_RELEASE: env!("CFG_RELEASE"), @@ -1423,7 +1423,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..4a9bf47a29011 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 @@ -2,14 +2,14 @@ -------------------- -The `-Z patchable-function-entry=M,N` or `-Z patchable-function-entry=M` -compiler flag enables nop padding of function entries with M nops, with -an offset for the entry of the function at N nops. In the second form, -N defaults to 0. +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: @@ -18,7 +18,7 @@ nop ``` This flag is used for hotpatching, especially in the Linux kernel. The flag -arguments are modeled after hte `-fpatchable-function-entry` flag as defined +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. diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs deleted file mode 100644 index dc20c0a2c6dad..0000000000000 --- a/tests/codegen/patchable-function-entry.rs +++ /dev/null @@ -1,28 +0,0 @@ -#![feature(patchable_function_entry)] -// compile-flags: -Z patchable-function-entry=15,10 - -#![crate_type = "lib"] - -// This should have the default, as set by the compile flags -#[no_mangle] -pub fn foo() {} - -// The attribute should override the compile flags -#[no_mangle] -#[patchable_function_entry(prefix(1), entry(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))] -pub fn baz() {} - -// CHECK: @foo() unnamed_addr #0 -// CHECK: @bar() unnamed_addr #1 -// CHECK: @baz() unnamed_addr #2 - -// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } -// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } -// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } -// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } -// CHECK: attributes #2 = { {{.*}} } diff --git a/tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs b/tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs new file mode 100644 index 0000000000000..72204c78a4906 --- /dev/null +++ b/tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs @@ -0,0 +1,64 @@ +//@ compile-flags: -Z patchable-function-entry=15,10 + +#![feature(patchable_function_entry)] +#![crate_type = "lib"] + +// This should have the default, as set by the compile flags +#[no_mangle] +pub fn fun0() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] +pub fn fun1() {} + +// If we override an attribute to 0 or unset, the attribute should go away +#[no_mangle] +#[patchable_function_entry(entry_nops = 0)] +pub fn fun2() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 20, entry_nops = 1)] +pub fn fun3() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 2, entry_nops = 19)] +pub fn fun4() {} + +// The attribute should override patchable-function-entry to 3 and +// patchable-function-prefix to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(entry_nops = 3)] +pub fn fun5() {} + +// The attribute should override patchable-function-prefix to 4 +// and patchable-function-entry to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(prefix_nops = 4)] +pub fn fun6() {} + +// CHECK: @fun0() unnamed_addr #0 +// CHECK: @fun1() unnamed_addr #1 +// CHECK: @fun2() unnamed_addr #2 +// CHECK: @fun3() unnamed_addr #3 +// CHECK: @fun4() unnamed_addr #4 +// CHECK: @fun5() unnamed_addr #5 +// CHECK: @fun6() unnamed_addr #6 + +// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } + +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } +// CHECK: attributes #2 = { {{.*}} } + +// CHECK: attributes #3 = { {{.*}}"patchable-function-entry"="1"{{.*}}"patchable-function-prefix"="20" {{.*}} } +// CHECK: attributes #4 = { {{.*}}"patchable-function-entry"="19"{{.*}}"patchable-function-prefix"="2" {{.*}} } + +// CHECK: attributes #5 = { {{.*}}"patchable-function-entry"="3"{{.*}} } +// CHECK-NOT: attributes #5 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #6 = { {{.*}}"patchable-function-prefix"="4"{{.*}} } +// CHECK-NOT: attributes #6 = { {{.*}}patchable-function-entry{{.*}} } diff --git a/tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs b/tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs new file mode 100644 index 0000000000000..3a7078fe55107 --- /dev/null +++ b/tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs @@ -0,0 +1,39 @@ +#![feature(patchable_function_entry)] +#![crate_type = "lib"] + +// No patchable function entry should be set +#[no_mangle] +pub fn fun0() {} + +// The attribute should work even without compiler flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] +pub fn fun1() {} + +// The attribute should work even without compiler flags +// and only set patchable-function-entry to 3. +#[no_mangle] +#[patchable_function_entry(entry_nops = 3)] +pub fn fun2() {} + +// The attribute should work even without compiler flags +// and only set patchable-function-prefix to 4. +#[no_mangle] +#[patchable_function_entry(prefix_nops = 4)] +pub fn fun3() {} + +// CHECK: @fun0() unnamed_addr #0 +// CHECK: @fun1() unnamed_addr #1 +// CHECK: @fun2() unnamed_addr #2 +// CHECK: @fun3() unnamed_addr #3 + +// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } + +// CHECK: attributes #2 = { {{.*}}"patchable-function-entry"="3"{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #3 = { {{.*}}"patchable-function-prefix"="4"{{.*}} } +// CHECK-NOT: attributes #3 = { {{.*}}patchable-function-entry{{.*}} } diff --git a/tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs b/tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs new file mode 100644 index 0000000000000..8bdd61e461b81 --- /dev/null +++ b/tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs @@ -0,0 +1,66 @@ +//@ compile-flags: -Z patchable-function-entry=15 + +#![feature(patchable_function_entry)] +#![crate_type = "lib"] + +// This should have the default, as set by the compile flags +#[no_mangle] +pub fn fun0() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] +pub fn fun1() {} + +// If we override an attribute to 0 or unset, the attribute should go away +#[no_mangle] +#[patchable_function_entry(entry_nops = 0)] +pub fn fun2() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 20, entry_nops = 1)] +pub fn fun3() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 2, entry_nops = 19)] +pub fn fun4() {} + +// The attribute should override patchable-function-entry to 3 +// and patchable-function-prefix to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(entry_nops = 3)] +pub fn fun5() {} + +// The attribute should override patchable-function-prefix to 4 +// and patchable-function-entry to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(prefix_nops = 4)] +pub fn fun6() {} + +// CHECK: @fun0() unnamed_addr #0 +// CHECK: @fun1() unnamed_addr #1 +// CHECK: @fun2() unnamed_addr #2 +// CHECK: @fun3() unnamed_addr #3 +// CHECK: @fun4() unnamed_addr #4 +// CHECK: @fun5() unnamed_addr #5 +// CHECK: @fun6() unnamed_addr #6 + +// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="15" {{.*}} } +// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } + +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } +// CHECK: attributes #2 = { {{.*}} } + +// CHECK: attributes #3 = { {{.*}}"patchable-function-entry"="1"{{.*}}"patchable-function-prefix"="20" {{.*}} } +// CHECK: attributes #4 = { {{.*}}"patchable-function-entry"="19"{{.*}}"patchable-function-prefix"="2" {{.*}} } + +// CHECK: attributes #5 = { {{.*}}"patchable-function-entry"="3"{{.*}} } +// CHECK-NOT: attributes #5 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #6 = { {{.*}}"patchable-function-prefix"="4"{{.*}} } +// CHECK-NOT: attributes #6 = { {{.*}}patchable-function-entry{{.*}} } 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 diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs new file mode 100644 index 0000000000000..d7231ef416077 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs @@ -0,0 +1,17 @@ +#![feature(patchable_function_entry)] +fn main() {} + +#[patchable_function_entry(prefix_nops = 256, entry_nops = 0)]//~error: Expected integer value between 0 and 255. +pub fn too_high_pnops() {} + +#[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)]//~error: Expected integer value between 0 and 255. +pub fn non_int_nop() {} + +#[patchable_function_entry]//~error: malformed `patchable_function_entry` attribute input +pub fn malformed_attribute() {} + +#[patchable_function_entry(prefix_nops = 10, something = 0)]//~error: Unexpected parameter name. Allowed names: prefix_nops, entry_nops +pub fn unexpected_parameter_name() {} + +#[patchable_function_entry()]//~error: Must specify at least one parameter. +pub fn no_parameters_given() {} diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr new file mode 100644 index 0000000000000..a270106925f44 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr @@ -0,0 +1,32 @@ +error: malformed `patchable_function_entry` attribute input + --> $DIR/patchable-function-entry-attribute.rs:10:1 + | +LL | #[patchable_function_entry] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]` + +error: Expected integer value between 0 and 255. + --> $DIR/patchable-function-entry-attribute.rs:4:42 + | +LL | #[patchable_function_entry(prefix_nops = 256, entry_nops = 0)] + | ^^^ + +error: Expected integer value between 0 and 255. + --> $DIR/patchable-function-entry-attribute.rs:7:42 + | +LL | #[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)] + | ^^^^^^^^^^^^^ + +error: Unexpected parameter name. Allowed names: prefix_nops, entry_nops + --> $DIR/patchable-function-entry-attribute.rs:13:46 + | +LL | #[patchable_function_entry(prefix_nops = 10, something = 0)] + | ^^^^^^^^^^^^^ + +error: Must specify at least one parameter. + --> $DIR/patchable-function-entry-attribute.rs:16:1 + | +LL | #[patchable_function_entry()] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-flags.rs b/tests/ui/patchable-function-entry/patchable-function-entry-flags.rs new file mode 100644 index 0000000000000..cb5bc62b6b347 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-flags.rs @@ -0,0 +1,2 @@ +//@ compile-flags: -Z patchable-function-entry=1,2 +fn main() {} diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr b/tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr new file mode 100644 index 0000000000000..b09af94a61541 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr @@ -0,0 +1,2 @@ +error: incorrect value `1,2` for unstable option `patchable-function-entry` - either two comma separated integers (total_nops,prefix_nops), with prefix_nops <= total_nops, or one integer (total_nops) was expected + From 8d246b01020edc47c966bc62d9d607c9f480fb07 Mon Sep 17 00:00:00 2001 From: Florian Schmiderer Date: Wed, 26 Jun 2024 19:11:25 +0200 Subject: [PATCH 4/4] Updated diagnostic messages --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 52 +++++++++++++------ compiler/rustc_interface/src/tests.rs | 3 +- .../patchable-function-entry-attribute.rs | 8 +-- .../patchable-function-entry-attribute.stderr | 14 ++--- 4 files changed, 49 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 84041811bbbf2..bd0c4dc225c05 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -1,7 +1,6 @@ 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_errors::{codes::*, struct_span_code_err, DiagMessage, SubdiagMessage}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; @@ -472,45 +471,66 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { 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"); continue; }; let Some(name_value_lit) = meta_item.name_value_literal() else { - tcx.dcx().span_err(item.span(), "Expected name value pair."); + tcx.dcx().span_err(item.span(), "expected name value pair"); continue; }; + fn emit_error_with_label( + tcx: TyCtxt<'_>, + span: Span, + error: impl Into, + label: impl Into, + ) { + let mut err: rustc_errors::Diag<'_, _> = + tcx.dcx().struct_span_err(span, error); + err.span_label(span, label); + err.emit(); + } + let attrib_to_write = match meta_item.name_or_empty() { sym::prefix_nops => &mut prefix, sym::entry_nops => &mut entry, _ => { - tcx.dcx().span_err( + emit_error_with_label( + tcx, item.span(), - format!( - "Unexpected parameter name. Allowed names: {}, {}", - sym::prefix_nops, - sym::entry_nops - ), + "unexpected parameter name", + format!("expected {} or {}", sym::prefix_nops, sym::entry_nops), ); continue; } }; - let rustc_ast::LitKind::Int(Pu128(val @ 0..=255), _) = name_value_lit.kind - else { - tcx.dcx().span_err( + let rustc_ast::LitKind::Int(val, _) = name_value_lit.kind else { + emit_error_with_label( + tcx, + name_value_lit.span, + "invalid literal value", + "value must be an integer between `0` and `255`", + ); + continue; + }; + + let Ok(val) = val.get().try_into() else { + emit_error_with_label( + tcx, name_value_lit.span, - "Expected integer value between 0 and 255.", + "integer value out of range", + "value must be between `0` and `255`", ); continue; }; - *attrib_to_write = Some(val.try_into().unwrap()); + *attrib_to_write = Some(val); } if let (None, None) = (prefix, entry) { - tcx.dcx().span_err(attr.span, "Must specify at least one parameter."); + tcx.dcx().span_err(attr.span, "must specify at least one parameter"); } Some(PatchableFunctionEntry::from_prefix_and_entry( diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index e23d556ad4610..02322c9b2828f 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -815,7 +815,8 @@ fn test_unstable_options_tracking_hash() { 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 must be greater than or equal to prefix") ); tracked!(plt, Some(true)); tracked!(polonius, Polonius::Legacy); diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs index d7231ef416077..1e376c9ff3c1a 100644 --- a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs +++ b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs @@ -1,17 +1,17 @@ #![feature(patchable_function_entry)] fn main() {} -#[patchable_function_entry(prefix_nops = 256, entry_nops = 0)]//~error: Expected integer value between 0 and 255. +#[patchable_function_entry(prefix_nops = 256, entry_nops = 0)]//~error: integer value out of range pub fn too_high_pnops() {} -#[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)]//~error: Expected integer value between 0 and 255. +#[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)]//~error: invalid literal value pub fn non_int_nop() {} #[patchable_function_entry]//~error: malformed `patchable_function_entry` attribute input pub fn malformed_attribute() {} -#[patchable_function_entry(prefix_nops = 10, something = 0)]//~error: Unexpected parameter name. Allowed names: prefix_nops, entry_nops +#[patchable_function_entry(prefix_nops = 10, something = 0)]//~error: unexpected parameter name pub fn unexpected_parameter_name() {} -#[patchable_function_entry()]//~error: Must specify at least one parameter. +#[patchable_function_entry()]//~error: must specify at least one parameter pub fn no_parameters_given() {} diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr index a270106925f44..d9710c6e6a2f5 100644 --- a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr +++ b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr @@ -4,25 +4,25 @@ error: malformed `patchable_function_entry` attribute input LL | #[patchable_function_entry] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]` -error: Expected integer value between 0 and 255. +error: integer value out of range --> $DIR/patchable-function-entry-attribute.rs:4:42 | LL | #[patchable_function_entry(prefix_nops = 256, entry_nops = 0)] - | ^^^ + | ^^^ value must be between `0` and `255` -error: Expected integer value between 0 and 255. +error: invalid literal value --> $DIR/patchable-function-entry-attribute.rs:7:42 | LL | #[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)] - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ value must be an integer between `0` and `255` -error: Unexpected parameter name. Allowed names: prefix_nops, entry_nops +error: unexpected parameter name --> $DIR/patchable-function-entry-attribute.rs:13:46 | LL | #[patchable_function_entry(prefix_nops = 10, something = 0)] - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ expected prefix_nops or entry_nops -error: Must specify at least one parameter. +error: must specify at least one parameter --> $DIR/patchable-function-entry-attribute.rs:16:1 | LL | #[patchable_function_entry()]