Updated code for changes to RFC, added additional error handling

Author: nebulark

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

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)
     ),
 
     // ==========================================================================

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.

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

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

@@ -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;
         }

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,

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

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 <https://github.com/rust-lang/rust/issues/9999> for more information
+   = note: see issue #123115 <https://github.com/rust-lang/rust/issues/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