about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRalf Jung <post@ralfj.de>2024-08-26 17:01:46 +0200
committerRalf Jung <post@ralfj.de>2024-08-26 17:08:52 +0200
commit7a290fce907da1cda0687fe5e48372c08fe40ab8 (patch)
treeead12271778144c209ce507f29379eb1cd4e5aa9
parent22572d0994593197593e2a1b7b18d720a9a349a7 (diff)
downloadrust-7a290fce907da1cda0687fe5e48372c08fe40ab8.tar.gz
rust-7a290fce907da1cda0687fe5e48372c08fe40ab8.zip
interpret: do not make const-eval query result depend on tcx.sess
-rw-r--r--compiler/rustc_const_eval/messages.ftl3
-rw-r--r--compiler/rustc_const_eval/src/interpret/call.rs44
-rw-r--r--compiler/rustc_const_eval/src/interpret/machine.rs22
-rw-r--r--src/tools/miri/src/machine.rs42
-rw-r--r--src/tools/miri/tests/panic/target_feature_wasm.rs (renamed from src/tools/miri/tests/pass/function_calls/target_feature_wasm.rs)4
-rw-r--r--tests/ui/consts/const-eval/const_fn_target_feature.rs4
-rw-r--r--tests/ui/consts/const-eval/const_fn_target_feature.stderr9
-rw-r--r--tests/ui/consts/const-eval/const_fn_target_feature_wasm.rs4
8 files changed, 70 insertions, 62 deletions
diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl
index d7d64180ec9..db788b6b151 100644
--- a/compiler/rustc_const_eval/messages.ftl
+++ b/compiler/rustc_const_eval/messages.ftl
@@ -402,9 +402,6 @@ const_eval_unallowed_mutable_refs =
 const_eval_unallowed_op_in_const_context =
     {$msg}
 
-const_eval_unavailable_target_features_for_fn =
-    calling a function that requires unavailable target features: {$unavailable_feats}
-
 const_eval_uninhabited_enum_variant_read =
     read discriminant of an uninhabited enum variant
 const_eval_uninhabited_enum_variant_written =
diff --git a/compiler/rustc_const_eval/src/interpret/call.rs b/compiler/rustc_const_eval/src/interpret/call.rs
index 917a2fa7c6d..4b21a3a6a3d 100644
--- a/compiler/rustc_const_eval/src/interpret/call.rs
+++ b/compiler/rustc_const_eval/src/interpret/call.rs
@@ -311,34 +311,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         Ok(())
     }
 
-    fn check_fn_target_features(&self, instance: ty::Instance<'tcx>) -> InterpResult<'tcx, ()> {
-        // Calling functions with `#[target_feature]` is not unsafe on WASM, see #84988
-        let attrs = self.tcx.codegen_fn_attrs(instance.def_id());
-        if !self.tcx.sess.target.is_like_wasm
-            && attrs
-                .target_features
-                .iter()
-                .any(|feature| !self.tcx.sess.target_features.contains(&feature.name))
-        {
-            throw_ub_custom!(
-                fluent::const_eval_unavailable_target_features_for_fn,
-                unavailable_feats = attrs
-                    .target_features
-                    .iter()
-                    .filter(|&feature| !feature.implied
-                        && !self.tcx.sess.target_features.contains(&feature.name))
-                    .fold(String::new(), |mut s, feature| {
-                        if !s.is_empty() {
-                            s.push_str(", ");
-                        }
-                        s.push_str(feature.name.as_str());
-                        s
-                    }),
-            );
-        }
-        Ok(())
-    }
-
     /// The main entry point for creating a new stack frame: performs ABI checks and initializes
     /// arguments.
     #[instrument(skip(self), level = "trace")]
@@ -360,20 +332,18 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             throw_unsup_format!("calling a c-variadic function is not supported");
         }
 
-        if M::enforce_abi(self) {
-            if caller_fn_abi.conv != callee_fn_abi.conv {
-                throw_ub_custom!(
-                    fluent::const_eval_incompatible_calling_conventions,
-                    callee_conv = format!("{:?}", callee_fn_abi.conv),
-                    caller_conv = format!("{:?}", caller_fn_abi.conv),
-                )
-            }
+        if caller_fn_abi.conv != callee_fn_abi.conv {
+            throw_ub_custom!(
+                fluent::const_eval_incompatible_calling_conventions,
+                callee_conv = format!("{:?}", callee_fn_abi.conv),
+                caller_conv = format!("{:?}", caller_fn_abi.conv),
+            )
         }
 
         // Check that all target features required by the callee (i.e., from
         // the attribute `#[target_feature(enable = ...)]`) are enabled at
         // compile time.
-        self.check_fn_target_features(instance)?;
+        M::check_fn_target_features(self, instance)?;
 
         if !callee_fn_abi.can_unwind {
             // The callee cannot unwind, so force the `Unreachable` unwind handling.
diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs
index 761ab81e228..1c71780bb45 100644
--- a/compiler/rustc_const_eval/src/interpret/machine.rs
+++ b/compiler/rustc_const_eval/src/interpret/machine.rs
@@ -173,11 +173,6 @@ pub trait Machine<'tcx>: Sized {
         false
     }
 
-    /// Whether function calls should be [ABI](CallAbi)-checked.
-    fn enforce_abi(_ecx: &InterpCx<'tcx, Self>) -> bool {
-        true
-    }
-
     /// Whether Assert(OverflowNeg) and Assert(Overflow) MIR terminators should actually
     /// check for overflow.
     fn ignore_optional_overflow_checks(_ecx: &InterpCx<'tcx, Self>) -> bool;
@@ -238,6 +233,13 @@ pub trait Machine<'tcx>: Sized {
         unwind: mir::UnwindAction,
     ) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>>;
 
+    /// Check whether the given function may be executed on the current machine, in terms of the
+    /// target features is requires.
+    fn check_fn_target_features(
+        _ecx: &InterpCx<'tcx, Self>,
+        _instance: ty::Instance<'tcx>,
+    ) -> InterpResult<'tcx>;
+
     /// Called to evaluate `Assert` MIR terminators that trigger a panic.
     fn assert_panic(
         ecx: &mut InterpCx<'tcx, Self>,
@@ -615,6 +617,16 @@ pub macro compile_time_machine(<$tcx: lifetime>) {
     }
 
     #[inline(always)]
+    fn check_fn_target_features(
+        _ecx: &InterpCx<$tcx, Self>,
+        _instance: ty::Instance<$tcx>,
+    ) -> InterpResult<$tcx> {
+        // For now we don't do any checking here. We can't use `tcx.sess` because that can differ
+        // between crates, and we need to ensure that const-eval always behaves the same.
+        Ok(())
+    }
+
+    #[inline(always)]
     fn call_extra_fn(
         _ecx: &mut InterpCx<$tcx, Self>,
         fn_val: !,
diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs
index a7493d48d6a..8d4866517f2 100644
--- a/src/tools/miri/src/machine.rs
+++ b/src/tools/miri/src/machine.rs
@@ -947,15 +947,47 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
     }
 
     #[inline(always)]
-    fn enforce_abi(_ecx: &MiriInterpCx<'tcx>) -> bool {
-        true
-    }
-
-    #[inline(always)]
     fn ignore_optional_overflow_checks(ecx: &MiriInterpCx<'tcx>) -> bool {
         !ecx.tcx.sess.overflow_checks()
     }
 
+    fn check_fn_target_features(
+        ecx: &MiriInterpCx<'tcx>,
+        instance: ty::Instance<'tcx>,
+    ) -> InterpResult<'tcx> {
+        let attrs = ecx.tcx.codegen_fn_attrs(instance.def_id());
+        if attrs
+            .target_features
+            .iter()
+            .any(|feature| !ecx.tcx.sess.target_features.contains(&feature.name))
+        {
+            let unavailable = attrs
+                .target_features
+                .iter()
+                .filter(|&feature| {
+                    !feature.implied && !ecx.tcx.sess.target_features.contains(&feature.name)
+                })
+                .fold(String::new(), |mut s, feature| {
+                    if !s.is_empty() {
+                        s.push_str(", ");
+                    }
+                    s.push_str(feature.name.as_str());
+                    s
+                });
+            let msg = format!(
+                "calling a function that requires unavailable target features: {unavailable}"
+            );
+            // On WASM, this is not UB, but instead gets rejected during validation of the module
+            // (see #84988).
+            if ecx.tcx.sess.target.is_like_wasm {
+                throw_machine_stop!(TerminationInfo::Abort(msg));
+            } else {
+                throw_ub_format!("{msg}");
+            }
+        }
+        Ok(())
+    }
+
     #[inline(always)]
     fn find_mir_or_eval_fn(
         ecx: &mut MiriInterpCx<'tcx>,
diff --git a/src/tools/miri/tests/pass/function_calls/target_feature_wasm.rs b/src/tools/miri/tests/panic/target_feature_wasm.rs
index 5056f32de44..c67d2983f78 100644
--- a/src/tools/miri/tests/pass/function_calls/target_feature_wasm.rs
+++ b/src/tools/miri/tests/panic/target_feature_wasm.rs
@@ -2,7 +2,9 @@
 //@compile-flags: -C target-feature=-simd128
 
 fn main() {
-    // Calling functions with `#[target_feature]` is not unsound on WASM, see #84988
+    // Calling functions with `#[target_feature]` is not unsound on WASM, see #84988.
+    // But if the compiler actually uses the target feature, it will lead to an error when the module is loaded.
+    // We emulate this with an "unsupported" error.
     assert!(!cfg!(target_feature = "simd128"));
     simd128_fn();
 }
diff --git a/tests/ui/consts/const-eval/const_fn_target_feature.rs b/tests/ui/consts/const-eval/const_fn_target_feature.rs
index ee669abb51e..8db41ba11c0 100644
--- a/tests/ui/consts/const-eval/const_fn_target_feature.rs
+++ b/tests/ui/consts/const-eval/const_fn_target_feature.rs
@@ -1,6 +1,7 @@
 //@ only-x86_64
 // Set the base cpu explicitly, in case the default has been changed.
 //@ compile-flags: -C target-cpu=x86-64 -C target-feature=+ssse3
+//@ check-pass
 
 #![crate_type = "lib"]
 
@@ -9,7 +10,8 @@ const A: () = unsafe { ssse3_fn() };
 
 // error (avx2 not enabled at compile time)
 const B: () = unsafe { avx2_fn() };
-//~^ ERROR evaluation of constant value failed
+// FIXME: currently we do not detect this UB, since we don't want the result of const-eval
+// to depend on `tcx.sess` which can differ between crates in a crate graph.
 
 #[target_feature(enable = "ssse3")]
 const unsafe fn ssse3_fn() {}
diff --git a/tests/ui/consts/const-eval/const_fn_target_feature.stderr b/tests/ui/consts/const-eval/const_fn_target_feature.stderr
deleted file mode 100644
index d3a00b57ebb..00000000000
--- a/tests/ui/consts/const-eval/const_fn_target_feature.stderr
+++ /dev/null
@@ -1,9 +0,0 @@
-error[E0080]: evaluation of constant value failed
-  --> $DIR/const_fn_target_feature.rs:11:24
-   |
-LL | const B: () = unsafe { avx2_fn() };
-   |                        ^^^^^^^^^ calling a function that requires unavailable target features: avx2
-
-error: aborting due to 1 previous error
-
-For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/const-eval/const_fn_target_feature_wasm.rs b/tests/ui/consts/const-eval/const_fn_target_feature_wasm.rs
index deed09e4b2a..8ddfe61943c 100644
--- a/tests/ui/consts/const-eval/const_fn_target_feature_wasm.rs
+++ b/tests/ui/consts/const-eval/const_fn_target_feature_wasm.rs
@@ -7,7 +7,9 @@
 #[cfg(target_feature = "simd128")]
 compile_error!("simd128 target feature should be disabled");
 
-// Calling functions with `#[target_feature]` is not unsound on WASM, see #84988
+// Calling functions with `#[target_feature]` is not unsound on WASM, see #84988.
+// (It can still lead to a runtime error though so we'd be in our right to abort execution,
+// just not to declare it UB.)
 const A: () = simd128_fn();
 
 #[target_feature(enable = "simd128")]