Skip to content

Commit

Permalink
Auto merge of #79411 - tmiasko:naked-params, r=Amanieu
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Nov 25, 2020
2 parents 192c7db + 22d3431 commit b48cafd
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 49 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/passes.rs
Expand Up @@ -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);
});
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Expand Up @@ -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) }
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_passes/src/lib.rs
Expand Up @@ -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)]
Expand All @@ -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;
Expand All @@ -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);
Expand Down
113 changes: 113 additions & 0 deletions 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);
}
}
54 changes: 5 additions & 49 deletions 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)]
Expand All @@ -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
}

Expand All @@ -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
}
51 changes: 51 additions & 0 deletions 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
}
44 changes: 44 additions & 0 deletions 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

0 comments on commit b48cafd

Please sign in to comment.