Updated code for changes to RFC, added additional error handling.

Author: nebulark

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -469,24 +469,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),
+                    ))
                 })
             }
             _ => {}

compiler/rustc_feature/src/builtin_attrs.rs

@@ -527,11 +527,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)
     ),
 
     // ==========================================================================

compiler/rustc_feature/src/unstable.rs

@@ -564,8 +564,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.

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);

compiler/rustc_session/src/config.rs

@@ -2933,8 +2933,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;
@@ -3231,11 +3232,14 @@ pub struct PatchableFunctionEntry {
 }
 
 impl PatchableFunctionEntry {
-    pub fn from_nop_count_and_offset(nop_count: u8, offset: u8) -> Option<PatchableFunctionEntry> {
-        if nop_count < offset {
+    pub fn from_total_and_prefix_nops(
+        total_nops: u8,
+        prefix_nops: u8,
+    ) -> Option<PatchableFunctionEntry> {
+        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 {

compiler/rustc_session/src/options.rs

@@ -380,7 +380,7 @@ mod desc {
     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)";
+        "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`";
@@ -712,7 +712,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
@@ -728,20 +727,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;
         }

compiler/rustc_span/src/symbol.rs

@@ -758,7 +758,7 @@ symbols! {
         enable,
         encode,
         end,
-        entry,
+        entry_nops,
         enumerate_method,
         env,
         env_CFG_RELEASE: env!("CFG_RELEASE"),
@@ -1408,7 +1408,7 @@ symbols! {
         prefetch_read_instruction,
         prefetch_write_data,
         prefetch_write_instruction,
-        prefix,
+        prefix_nops,
         preg,
         prelude,
         prelude_import,

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:

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

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() {}