Auto merge of #79411 - tmiasko:naked-params, r=Amanieu

Validate use of parameters in naked functions

* Reject use of parameters inside naked function body.
* Reject use of patterns inside function parameters, to emphasize role
  of parameters a signature declaration (mirroring existing behaviour
  for function declarations) and avoid generating code introducing
  specified bindings.

Closes issues below by considering input to be ill-formed.

Closes #75922.
Closes #77848.
Closes #79350.

compiler/rustc_interface/src/passes.rs

@@ -816,6 +816,7 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {
                     let local_def_id = tcx.hir().local_def_id(module);
                     tcx.ensure().check_mod_loops(local_def_id);
                     tcx.ensure().check_mod_attrs(local_def_id);
+                    tcx.ensure().check_mod_naked_functions(local_def_id);
                     tcx.ensure().check_mod_unstable_api_usage(local_def_id);
                     tcx.ensure().check_mod_const_bodies(local_def_id);
                 });

compiler/rustc_middle/src/query/mod.rs

@@ -635,6 +635,10 @@ rustc_queries! {
             desc { |tcx| "checking loops in {}", describe_as_module(key, tcx) }
         }
 
+        query check_mod_naked_functions(key: LocalDefId) -> () {
+            desc { |tcx| "checking naked functions in {}", describe_as_module(key, tcx) }
+        }
+
         query check_mod_item_types(key: LocalDefId) -> () {
             desc { |tcx| "checking item types in {}", describe_as_module(key, tcx) }
         }

compiler/rustc_passes/src/lib.rs

@@ -7,6 +7,7 @@
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
 #![feature(const_fn)]
 #![feature(const_panic)]
+#![feature(crate_visibility_modifier)]
 #![feature(in_band_lifetimes)]
 #![feature(nll)]
 #![feature(or_patterns)]
@@ -32,6 +33,7 @@ pub mod layout_test;
 mod lib_features;
 mod liveness;
 pub mod loops;
+mod naked_functions;
 mod reachable;
 mod region;
 pub mod stability;
@@ -46,6 +48,7 @@ pub fn provide(providers: &mut Providers) {
     lang_items::provide(providers);
     lib_features::provide(providers);
     loops::provide(providers);
+    naked_functions::provide(providers);
     liveness::provide(providers);
     intrinsicck::provide(providers);
     reachable::provide(providers);

compiler/rustc_passes/src/naked_functions.rs

@@ -0,0 +1,113 @@
+use rustc_hir as hir;
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::intravisit::{ErasedMap, FnKind, NestedVisitorMap, Visitor};
+use rustc_middle::ty::query::Providers;
+use rustc_middle::ty::TyCtxt;
+use rustc_span::symbol::sym;
+use rustc_span::Span;
+
+fn check_mod_naked_functions(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
+    tcx.hir().visit_item_likes_in_module(
+        module_def_id,
+        &mut CheckNakedFunctions { tcx }.as_deep_visitor(),
+    );
+}
+
+crate fn provide(providers: &mut Providers) {
+    *providers = Providers { check_mod_naked_functions, ..*providers };
+}
+
+struct CheckNakedFunctions<'tcx> {
+    tcx: TyCtxt<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for CheckNakedFunctions<'tcx> {
+    type Map = ErasedMap<'tcx>;
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+
+    fn visit_fn(
+        &mut self,
+        fk: FnKind<'v>,
+        _fd: &'tcx hir::FnDecl<'tcx>,
+        body_id: hir::BodyId,
+        _span: Span,
+        _hir_id: hir::HirId,
+    ) {
+        match fk {
+            // Rejected during attribute check. Do not validate further.
+            FnKind::Closure(..) => return,
+            FnKind::ItemFn(..) | FnKind::Method(..) => {}
+        }
+
+        let naked = fk.attrs().iter().any(|attr| attr.has_name(sym::naked));
+        if naked {
+            let body = self.tcx.hir().body(body_id);
+            check_params(self.tcx, body);
+            check_body(self.tcx, body);
+        }
+    }
+}
+
+/// Checks that parameters don't use patterns. Mirrors the checks for function declarations.
+fn check_params(tcx: TyCtxt<'_>, body: &hir::Body<'_>) {
+    for param in body.params {
+        match param.pat.kind {
+            hir::PatKind::Wild
+            | hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, _, None) => {}
+            _ => {
+                tcx.sess
+                    .struct_span_err(
+                        param.pat.span,
+                        "patterns not allowed in naked function parameters",
+                    )
+                    .emit();
+            }
+        }
+    }
+}
+
+/// Checks that function parameters aren't referenced in the function body.
+fn check_body<'tcx>(tcx: TyCtxt<'tcx>, body: &'tcx hir::Body<'tcx>) {
+    let mut params = hir::HirIdSet::default();
+    for param in body.params {
+        param.pat.each_binding(|_binding_mode, hir_id, _span, _ident| {
+            params.insert(hir_id);
+        });
+    }
+    CheckBody { tcx, params }.visit_body(body);
+}
+
+struct CheckBody<'tcx> {
+    tcx: TyCtxt<'tcx>,
+    params: hir::HirIdSet,
+}
+
+impl<'tcx> Visitor<'tcx> for CheckBody<'tcx> {
+    type Map = ErasedMap<'tcx>;
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+
+    fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
+        if let hir::ExprKind::Path(hir::QPath::Resolved(
+            _,
+            hir::Path { res: hir::def::Res::Local(var_hir_id), .. },
+        )) = expr.kind
+        {
+            if self.params.contains(var_hir_id) {
+                self.tcx
+                    .sess
+                    .struct_span_err(
+                        expr.span,
+                        "use of parameters not allowed inside naked functions",
+                    )
+                    .emit();
+            }
+        }
+        hir::intravisit::walk_expr(self, expr);
+    }
+}

src/test/codegen/naked-functions.rs

@@ -1,4 +1,4 @@
-// compile-flags: -C no-prepopulate-passes -Zmir-opt-level=0
+// compile-flags: -C no-prepopulate-passes
 
 #![crate_type = "lib"]
 #![feature(naked_functions)]
@@ -15,11 +15,9 @@ pub fn naked_empty() {
 // CHECK: Function Attrs: naked
 #[no_mangle]
 #[naked]
-// CHECK-NEXT: define void @naked_with_args(i{{[0-9]+( %0)?}})
+// CHECK-NEXT: define void @naked_with_args(i{{[0-9]+( %a)?}})
 pub fn naked_with_args(a: isize) {
     // CHECK-NEXT: {{.+}}:
-    // CHECK-NEXT: %a = alloca i{{[0-9]+}}
-    &a; // keep variable in an alloca
     // CHECK: ret void
 }
 
@@ -34,53 +32,11 @@ pub fn naked_with_return() -> isize {
 }
 
 // CHECK: Function Attrs: naked
-// CHECK-NEXT: define i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+( %0)?}})
+// CHECK-NEXT: define i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+( %a)?}})
 #[no_mangle]
 #[naked]
 pub fn naked_with_args_and_return(a: isize) -> isize {
     // CHECK-NEXT: {{.+}}:
-    // CHECK-NEXT: %a = alloca i{{[0-9]+}}
-    &a; // keep variable in an alloca
-    // CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
-    a
-}
-
-// CHECK: Function Attrs: naked
-// CHECK-NEXT: define void @naked_recursive()
-#[no_mangle]
-#[naked]
-pub fn naked_recursive() {
-    // CHECK-NEXT: {{.+}}:
-    // CHECK-NEXT: call void @naked_empty()
-
-    // FIXME(#39685) Avoid one block per call.
-    // CHECK-NEXT: br label %bb1
-    // CHECK: bb1:
-
-    naked_empty();
-
-    // CHECK-NEXT: %_4 = call i{{[0-9]+}} @naked_with_return()
-
-    // FIXME(#39685) Avoid one block per call.
-    // CHECK-NEXT: br label %bb2
-    // CHECK: bb2:
-
-    // CHECK-NEXT: %_3 = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %_4)
-
-    // FIXME(#39685) Avoid one block per call.
-    // CHECK-NEXT: br label %bb3
-    // CHECK: bb3:
-
-    // CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %_3)
-
-    // FIXME(#39685) Avoid one block per call.
-    // CHECK-NEXT: br label %bb4
-    // CHECK: bb4:
-
-    naked_with_args(
-        naked_with_args_and_return(
-            naked_with_return()
-        )
-    );
-    // CHECK-NEXT: ret void
+    // CHECK: ret i{{[0-9]+}} 0
+    0
 }

src/test/ui/asm/naked-params.rs

@@ -0,0 +1,51 @@
+// Check that use of function parameters is validate in naked functions.
+//
+// ignore-wasm32 asm unsupported
+#![feature(asm)]
+#![feature(naked_functions)]
+#![feature(or_patterns)]
+#![crate_type = "lib"]
+
+#[repr(C)]
+pub struct P { x: u8, y: u16 }
+
+#[naked]
+pub unsafe extern "C" fn f(
+    mut a: u32,
+    //~^ ERROR patterns not allowed in naked function parameters
+    &b: &i32,
+    //~^ ERROR patterns not allowed in naked function parameters
+    (None | Some(_)): Option<std::ptr::NonNull<u8>>,
+    //~^ ERROR patterns not allowed in naked function parameters
+    P { x, y }: P,
+    //~^ ERROR patterns not allowed in naked function parameters
+) {
+    asm!("", options(noreturn))
+}
+
+#[naked]
+pub unsafe extern "C" fn inc(a: u32) -> u32 {
+    a + 1
+    //~^ ERROR use of parameters not allowed inside naked functions
+}
+
+#[naked]
+pub unsafe extern "C" fn inc_asm(a: u32) -> u32 {
+    asm!("/* {0} */", in(reg) a, options(noreturn));
+    //~^ ERROR use of parameters not allowed inside naked functions
+}
+
+#[naked]
+pub unsafe extern "C" fn sum(x: u32, y: u32) -> u32 {
+    // FIXME: Should be detected by asm-only check.
+    (|| { x + y})()
+}
+
+pub fn outer(x: u32) -> extern "C" fn(usize) -> usize {
+    #[naked]
+    pub extern "C" fn inner(y: usize) -> usize {
+        *&y
+        //~^ ERROR use of parameters not allowed inside naked functions
+    }
+    inner
+}

src/test/ui/asm/naked-params.stderr

@@ -0,0 +1,44 @@
+error: patterns not allowed in naked function parameters
+  --> $DIR/naked-params.rs:14:5
+   |
+LL |     mut a: u32,
+   |     ^^^^^
+
+error: patterns not allowed in naked function parameters
+  --> $DIR/naked-params.rs:16:5
+   |
+LL |     &b: &i32,
+   |     ^^
+
+error: patterns not allowed in naked function parameters
+  --> $DIR/naked-params.rs:18:6
+   |
+LL |     (None | Some(_)): Option<std::ptr::NonNull<u8>>,
+   |      ^^^^^^^^^^^^^^
+
+error: patterns not allowed in naked function parameters
+  --> $DIR/naked-params.rs:20:5
+   |
+LL |     P { x, y }: P,
+   |     ^^^^^^^^^^
+
+error: use of parameters not allowed inside naked functions
+  --> $DIR/naked-params.rs:28:5
+   |
+LL |     a + 1
+   |     ^
+
+error: use of parameters not allowed inside naked functions
+  --> $DIR/naked-params.rs:34:31
+   |
+LL |     asm!("/* {0} */", in(reg) a, options(noreturn));
+   |                               ^
+
+error: use of parameters not allowed inside naked functions
+  --> $DIR/naked-params.rs:47:11
+   |
+LL |         *&y
+   |           ^
+
+error: aborting due to 7 previous errors
+