From 7fed92b3a4fd8b219fe576c60d31342b193cd92d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 6 May 2021 07:50:40 -0700 Subject: [PATCH] rustc: Allow safe #[target_feature] on wasm This commit updates the compiler's handling of the `#[target_feature]` attribute when applied to functions on WebAssembly-based targets. The compiler in general requires that any functions with `#[target_feature]` are marked as `unsafe` as well, but this commit relaxes the restriction for WebAssembly targets where the attribute can be applied to safe functions as well. The reason this is done is that the motivation for this feature of the compiler is not applicable for WebAssembly targets. In general the `#[target_feature]` attribute is used to enhance target CPU features enabled beyond the basic level for the rest of the compilation. If done improperly this means that your program could execute an instruction that the CPU you happen to be running on does not understand. This is considered undefined behavior where it is unknown what will happen (e.g. it's not a deterministic `SIGILL`). For WebAssembly, however, the target is different. It is not possible for a running WebAssembly program to execute an instruction that the engine does not understand. If this were the case then the program would not have validated in the first place and would not run at all. Even if this were allowed in some hypothetical future where engines have some form of runtime feature detection (which they do not right now) any implementation of such a feature would generate a trap if a module attempts to execute an instruction the module does not understand. This deterministic trap behavior would still not fall into the category of undefined behavior because the trap is deterministic. For these reasons the `#[target_feature]` attribute is now allowed on safe functions, but only for WebAssembly targets. This notably enables the wasm-SIMD intrinsics proposed for stabilization in #74372 to be marked as safe generally instead of today where they're all `unsafe` due to the historical implementation of `#[target_feature]` in the compiler. --- .../rustc_mir/src/transform/check_unsafety.rs | 6 +++ .../rustc_mir_build/src/check_unsafety.rs | 17 ++++--- compiler/rustc_typeck/src/collect.rs | 16 ++++++- src/test/ui/target-feature/wasm-safe.rs | 44 +++++++++++++++++++ 4 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/target-feature/wasm-safe.rs diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index 955be8cc81e18..9a4e51203d2b2 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -376,6 +376,12 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { /// Checks whether calling `func_did` needs an `unsafe` context or not, i.e. whether /// the called function has target features the calling function hasn't. fn check_target_features(&mut self, func_did: DefId) { + // Unsafety isn't required on wasm targets. For more information see + // the corresponding check in typeck/src/collect.rs + if self.tcx.sess.target.options.is_like_wasm { + return; + } + let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features; let self_features = &self.tcx.codegen_fn_attrs(self.body_did).target_features; diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 971b6dd9e1c89..91a8c601db958 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -166,13 +166,16 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { self.requires_unsafe(expr.span, CallToUnsafeFunction); } else if let &ty::FnDef(func_did, _) = self.thir[fun].ty.kind() { // If the called function has target features the calling function hasn't, - // the call requires `unsafe`. - if !self - .tcx - .codegen_fn_attrs(func_did) - .target_features - .iter() - .all(|feature| self.body_target_features.contains(feature)) + // the call requires `unsafe`. Don't check this on wasm + // targets, though. For more information on wasm see the + // is_like_wasm check in typeck/src/collect.rs + if !self.tcx.sess.target.options.is_like_wasm + && !self + .tcx + .codegen_fn_attrs(func_did) + .target_features + .iter() + .all(|feature| self.body_target_features.contains(feature)) { self.requires_unsafe(expr.span, CallToFunctionWith); } diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 0528f8812f920..5d83375e5a1b8 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2770,7 +2770,21 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } } else if tcx.sess.check_name(attr, sym::target_feature) { if !tcx.is_closure(id) && tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal { - if !tcx.features().target_feature_11 { + if tcx.sess.target.is_like_wasm { + // The `#[target_feature]` attribute is allowed on + // WebAssembly targets on all functions, including safe + // ones. Other targets require that `#[target_feature]` is + // only applied to unsafe funtions (pending the + // `target_feature_11` feature) because on most targets + // execution of instructions that are not supported is + // considered undefined behavior. For WebAssembly which is a + // 100% safe target at execution time it's not possible to + // execute undefined instructions, and even if a future + // feature was added in some form for this it would be a + // deterministic trap. There is no undefined behavior when + // executing WebAssembly so `#[target_feature]` is allowed + // on safe functions (but again, only for WebAssembly) + } else if !tcx.features().target_feature_11 { let mut err = feature_err( &tcx.sess.parse_sess, sym::target_feature_11, diff --git a/src/test/ui/target-feature/wasm-safe.rs b/src/test/ui/target-feature/wasm-safe.rs new file mode 100644 index 0000000000000..4b868684a5206 --- /dev/null +++ b/src/test/ui/target-feature/wasm-safe.rs @@ -0,0 +1,44 @@ +// only-wasm32 +// check-pass + +#![feature(wasm_target_feature)] +#![allow(dead_code)] + +#[target_feature(enable = "nontrapping-fptoint")] +fn foo() {} + +#[target_feature(enable = "nontrapping-fptoint")] +extern "C" fn bar() {} + +trait A { + fn foo(); + fn bar(&self); +} + +struct B; + +impl B { + #[target_feature(enable = "nontrapping-fptoint")] + fn foo() {} + #[target_feature(enable = "nontrapping-fptoint")] + fn bar(&self) {} +} + +impl A for B { + #[target_feature(enable = "nontrapping-fptoint")] + fn foo() {} + #[target_feature(enable = "nontrapping-fptoint")] + fn bar(&self) {} +} + +fn no_features_enabled_on_this_function() { + bar(); + foo(); + B.bar(); + B::foo(); + ::foo(); + ::bar(&B); +} + +#[target_feature(enable = "nontrapping-fptoint")] +fn main() {}