From 22d3431221df2e9136d303d1762ea7afdded1b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 25 Nov 2020 00:00:00 +0000 Subject: [PATCH] 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. --- compiler/rustc_interface/src/passes.rs | 1 + compiler/rustc_middle/src/query/mod.rs | 4 + compiler/rustc_passes/src/lib.rs | 3 + compiler/rustc_passes/src/naked_functions.rs | 113 +++++++++++++++++++ src/test/codegen/naked-functions.rs | 54 +-------- src/test/ui/asm/naked-params.rs | 51 +++++++++ src/test/ui/asm/naked-params.stderr | 44 ++++++++ 7 files changed, 221 insertions(+), 49 deletions(-) create mode 100644 compiler/rustc_passes/src/naked_functions.rs create mode 100644 src/test/ui/asm/naked-params.rs create mode 100644 src/test/ui/asm/naked-params.stderr diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 5fd560d7effb3..1f820024f77ef 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/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); }); diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 634d50368bd88..107336c5cee8e 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/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) } } diff --git a/compiler/rustc_passes/src/lib.rs b/compiler/rustc_passes/src/lib.rs index c32c9c8eaa68c..9759a500e0619 100644 --- a/compiler/rustc_passes/src/lib.rs +++ b/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); diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs new file mode 100644 index 0000000000000..6ef45cdd391de --- /dev/null +++ b/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 { + 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 { + 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); + } +} diff --git a/src/test/codegen/naked-functions.rs b/src/test/codegen/naked-functions.rs index 5e76f1d67e6f7..43a6be465bcf8 100644 --- a/src/test/codegen/naked-functions.rs +++ b/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 } diff --git a/src/test/ui/asm/naked-params.rs b/src/test/ui/asm/naked-params.rs new file mode 100644 index 0000000000000..46a4fc11e5a7c --- /dev/null +++ b/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>, + //~^ 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 +} diff --git a/src/test/ui/asm/naked-params.stderr b/src/test/ui/asm/naked-params.stderr new file mode 100644 index 0000000000000..1a99e5109fcdb --- /dev/null +++ b/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>, + | ^^^^^^^^^^^^^^ + +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 +