Skip to content

Commit

Permalink
Rollup merge of rust-lang#84988 - alexcrichton:safe-target-feature-wa…
Browse files Browse the repository at this point in the history
…sm, r=joshtriplett

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 rust-lang#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.
  • Loading branch information
Dylan-DPC authored Jun 3, 2021
2 parents 11acbff + 7fed92b commit 66eaa42
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 8 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_mir/src/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
17 changes: 10 additions & 7 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions src/test/ui/target-feature/wasm-safe.rs
Original file line number Diff line number Diff line change
@@ -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();
<B as A>::foo();
<B as A>::bar(&B);
}

#[target_feature(enable = "nontrapping-fptoint")]
fn main() {}

0 comments on commit 66eaa42

Please sign in to comment.