Repository: bytecodealliance/wasmtime
Pull request #9039 (2024-07-29 19:05:10)

Improve unsupported architecture errors in Wasmtime
Don't rely purely on `compile_error!` which doesn't halt compilation. Instead fill out some stubs which won't ever actually get compiled but prevent other compiler errors from cropping up.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/Cargo.toml b/Cargo.toml
index f16d74958..b6fe7b899 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -176,7 +176,10 @@ unused-lifetimes = 'warn'
 # they're all turned off by default. Selective lints are then enabled below as
 # necessary.
 all = { level = 'allow', priority = -1 }
-clone_on_copy = 'deny'
+clone_on_copy = 'warn'
+map_clone = 'warn'
+unnecessary_to_owned = 'warn'
+manual_strip = 'warn'
 
 [workspace.dependencies]
 arbitrary = { version = "1.3.1" }
diff --git a/cranelift/codegen/src/ir/immediates.rs b/cranelift/codegen/src/ir/immediates.rs
index d9efcac27..78b7dc860 100644
--- a/cranelift/codegen/src/ir/immediates.rs
+++ b/cranelift/codegen/src/ir/immediates.rs
@@ -218,9 +218,9 @@ fn parse_u64(s: &str) -> Result<u64, &'static str> {
 
     if s.starts_with("-0x") {
         return Err("Invalid character in hexadecimal number");
-    } else if s.starts_with("0x") {
+    } else if let Some(num) = s.strip_prefix("0x") {
         // Hexadecimal.
-        for ch in s[2..].chars() {
+        for ch in num.chars() {
             match ch.to_digit(16) {
                 Some(digit) => {
                     digits += 1;
@@ -934,10 +934,10 @@ fn parse_float(s: &str, w: u8, t: u8) -> Result<u128, &'static str> {
     debug_assert!(1 + w + t <= 128, "Too large IEEE format for u128");
     debug_assert!((t + w + 1).is_power_of_two(), "Unexpected IEEE format size");
 
-    let (sign_bit, s2) = if s.starts_with('-') {
-        (1u128 << (t + w), &s[1..])
-    } else if s.starts_with('+') {
-        (0, &s[1..])
+    let (sign_bit, s2) = if let Some(num) = s.strip_prefix('-') {
+        (1u128 << (t + w), num)
+    } else if let Some(num) = s.strip_prefix('+') {
+        (0, num)
     } else {
         (0, s)
     };
@@ -959,18 +959,18 @@ fn parse_float(s: &str, w: u8, t: u8) -> Result<u128, &'static str> {
             // Canonical quiet NaN: e = max, t = quiet.
             return Ok(sign_bit | max_e_bits | quiet_bit);
         }
-        if s2.starts_with("NaN:0x") {
+        if let Some(nan) = s2.strip_prefix("NaN:0x") {
             // Quiet NaN with payload.
-            return match u128::from_str_radix(&s2[6..], 16) {
+            return match u128::from_str_radix(nan, 16) {
                 Ok(payload) if payload < quiet_bit => {
                     Ok(sign_bit | max_e_bits | quiet_bit | payload)
                 }
                 _ => Err("Invalid NaN payload"),
             };
         }
-        if s2.starts_with("sNaN:0x") {
+        if let Some(nan) = s2.strip_prefix("sNaN:0x") {
             // Signaling NaN with payload.
-            return match u128::from_str_radix(&s2[7..], 16) {
+            return match u128::from_str_radix(nan, 16) {
                 Ok(payload) if 0 < payload && payload < quiet_bit => {
                     Ok(sign_bit | max_e_bits | payload)
                 }
diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs
index 0191e7cd5..fa4938389 100644
--- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs
+++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs
@@ -63,7 +63,7 @@ pub(crate) fn lower_branch(
     // TODO: reuse the ISLE context across lowerings so we can reuse its
     // internal heap allocations.
     let mut isle_ctx = IsleContext { lower_ctx, backend };
-    generated_code::constructor_lower_branch(&mut isle_ctx, branch, &targets.to_vec())
+    generated_code::constructor_lower_branch(&mut isle_ctx, branch, targets)
 }
 
 pub struct ExtendedValue {
diff --git a/cranelift/codegen/src/isa/riscv64/lower/isle.rs b/cranelift/codegen/src/isa/riscv64/lower/isle.rs
index 5695700f4..b39a02bf9 100644
--- a/cranelift/codegen/src/isa/riscv64/lower/isle.rs
+++ b/cranelift/codegen/src/isa/riscv64/lower/isle.rs
@@ -647,5 +647,5 @@ pub(crate) fn lower_branch(
     // TODO: reuse the ISLE context across lowerings so we can reuse its
     // internal heap allocations.
     let mut isle_ctx = RV64IsleContext::new(lower_ctx, backend);
-    generated_code::constructor_lower_branch(&mut isle_ctx, branch, &targets.to_vec())
+    generated_code::constructor_lower_branch(&mut isle_ctx, branch, targets)
 }
diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs
index f2937856b..6259b74a6 100644
--- a/cranelift/codegen/src/isa/s390x/lower/isle.rs
+++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs
@@ -74,7 +74,7 @@ pub(crate) fn lower_branch(
     // TODO: reuse the ISLE context across lowerings so we can reuse its
     // internal heap allocations.
     let mut isle_ctx = IsleContext { lower_ctx, backend };
-    generated_code::constructor_lower_branch(&mut isle_ctx, branch, &targets.to_vec())
+    generated_code::constructor_lower_branch(&mut isle_ctx, branch, targets)
 }
 
 impl generated_code::Context for IsleContext<'_, '_, MInst, S390xBackend> {
diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs
index b65c6fe74..c79e2ae51 100644
--- a/cranelift/codegen/src/isa/x64/lower/isle.rs
+++ b/cranelift/codegen/src/isa/x64/lower/isle.rs
@@ -65,7 +65,7 @@ pub(crate) fn lower_branch(
     // TODO: reuse the ISLE context across lowerings so we can reuse its
     // internal heap allocations.
     let mut isle_ctx = IsleContext { lower_ctx, backend };
-    generated_code::constructor_lower_branch(&mut isle_ctx, branch, &targets.to_vec())
+    generated_code::constructor_lower_branch(&mut isle_ctx, branch, &targets)
 }
 
 impl Context for IsleContext<'_, '_, MInst, X64Backend> {
diff --git a/cranelift/codegen/src/machinst/pcc.rs b/cranelift/codegen/src/machinst/pcc.rs
index a88fcc4d7..0c70a0238 100644
--- a/cranelift/codegen/src/machinst/pcc.rs
+++ b/cranelift/codegen/src/machinst/pcc.rs
@@ -11,7 +11,7 @@ pub(crate) fn get_fact_or_default<I: VCodeInst>(vcode: &VCode<I>, reg: Reg, widt
     );
     vcode
         .vreg_fact(reg.into())
-        .map(|f| f.clone())
+        .cloned()
         .unwrap_or_else(|| Fact::max_range_for_width(width))
 }
 
diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs
index 299b7c892..466e1003d 100644
--- a/cranelift/reader/src/parser.rs
+++ b/cranelift/reader/src/parser.rs
@@ -788,9 +788,9 @@ impl<'a> Parser<'a> {
         if let Some(Token::Integer(text)) = self.token() {
             self.consume();
             // Lexer just gives us raw text that looks like an integer.
-            if text.starts_with("0x") {
+            if let Some(num) = text.strip_prefix("0x") {
                 // Parse it as a u8 in hexadecimal form.
-                u8::from_str_radix(&text[2..], 16)
+                u8::from_str_radix(num, 16)
                     .map_err(|_| self.error("unable to parse u8 as a hexadecimal immediate"))
             } else {
                 // Parse it as a u8 to check for overflow and other issues.
diff --git a/crates/environ/src/fact/signature.rs b/crates/environ/src/fact/signature.rs
index 0bb9805a9..328ec085e 100644
--- a/crates/environ/src/fact/signature.rs
+++ b/crates/environ/src/fact/signature.rs
@@ -40,7 +40,7 @@ impl ComponentTypesBuilder {
         let results = match self.flatten_types(
             &options.options,
             MAX_FLAT_RESULTS,
-            self[ty.results].types.iter().map(|ty| *ty),
+            self[ty.results].types.iter().copied(),
         ) {
             Some(list) => list,
             None => {
diff --git a/crates/environ/src/fact/trampoline.rs b/crates/environ/src/fact/trampoline.rs
index 7414ed724..0097f801d 100644
--- a/crates/environ/src/fact/trampoline.rs
+++ b/crates/environ/src/fact/trampoline.rs
@@ -432,13 +432,13 @@ impl Compiler<'_, '_> {
         let src_tys = self.types[src_tys]
             .types
             .iter()
-            .map(|ty| *ty)
+            .copied()
             .collect::<Vec<_>>();
         let dst_tys = self.types[adapter.lower.ty].results;
         let dst_tys = self.types[dst_tys]
             .types
             .iter()
-            .map(|ty| *ty)
+            .copied()
             .collect::<Vec<_>>();
         let lift_opts = &adapter.lift.options;
         let lower_opts = &adapter.lower.options;
diff --git a/crates/wasmtime/src/runtime/gc/enabled/rooting.rs b/crates/wasmtime/src/runtime/gc/enabled/rooting.rs
index fcbeaf2a8..eb798602d 100644
--- a/crates/wasmtime/src/runtime/gc/enabled/rooting.rs
+++ b/crates/wasmtime/src/runtime/gc/enabled/rooting.rs
@@ -200,7 +200,7 @@ pub struct GcRootIndex {
 const _: () = {
     // NB: these match the C API which should also be updated if this changes
     assert!(mem::size_of::<GcRootIndex>() == 16);
-    assert!(mem::align_of::<GcRootIndex>() == 8);
+    assert!(mem::align_of::<GcRootIndex>() == mem::align_of::<u64>());
 };
 
 impl GcRootIndex {
@@ -1302,9 +1302,9 @@ const _: () = {
 
     // NB: these match the C API which should also be updated if this changes
     assert!(mem::size_of::<ManuallyRooted<AnyRef>>() == 16);
-    assert!(mem::align_of::<ManuallyRooted<AnyRef>>() == 8);
+    assert!(mem::align_of::<ManuallyRooted<AnyRef>>() == mem::align_of::<u64>());
     assert!(mem::size_of::<ManuallyRooted<ExternRef>>() == 16);
-    assert!(mem::align_of::<ManuallyRooted<ExternRef>>() == 8);
+    assert!(mem::align_of::<ManuallyRooted<ExternRef>>() == mem::align_of::<u64>());
 };
 
 impl<T: GcRef> Debug for ManuallyRooted<T> {
diff --git a/crates/wasmtime/src/runtime/vm/arch/mod.rs b/crates/wasmtime/src/runtime/vm/arch/mod.rs
index a46f152db..ea4b5e4f1 100644
--- a/crates/wasmtime/src/runtime/vm/arch/mod.rs
+++ b/crates/wasmtime/src/runtime/vm/arch/mod.rs
@@ -10,23 +10,44 @@
 cfg_if::cfg_if! {
     if #[cfg(target_arch = "x86_64")] {
         mod x86_64;
-        pub use x86_64::*;
+        use x86_64 as imp;
     } else if #[cfg(target_arch = "aarch64")] {
         mod aarch64;
-        pub use aarch64::*;
+        use aarch64 as imp;
     } else if #[cfg(target_arch = "s390x")] {
         mod s390x;
-        pub use s390x::*;
+        use s390x as imp;
     } else if #[cfg(target_arch = "riscv64")] {
         mod riscv64;
-        pub use riscv64::*;
+        use riscv64 as imp;
     } else {
-        compile_error!(
-            "Wasmtime is being compiled for an architecture \
-             that it does not support. If this architecture is \
-             one you would like to see supported you may file an \
-             issue on Wasmtime's issue tracker: \
-             https://github.com/bytecodealliance/wasmtime/issues/new\
-        ");
+        mod unsupported;
+        use unsupported as imp;
     }
 }
+
+// Functions defined in this module but all the implementations delegate to each
+// `imp` module. This exists to assert that each module internally provides the
+// same set of functionality with the same types for all architectures.
+
+pub fn get_stack_pointer() -> usize {
+    imp::get_stack_pointer()
+}
+
+pub unsafe fn get_next_older_pc_from_fp(fp: usize) -> usize {
+    imp::get_next_older_pc_from_fp(fp)
+}
+
+pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_FP_FROM_FP_OFFSET;
+
+pub fn reached_entry_sp(fp: usize, entry_sp: usize) -> bool {
+    imp::reached_entry_sp(fp, entry_sp)
+}
+
+pub fn assert_entry_sp_is_aligned(sp: usize) {
+    imp::assert_entry_sp_is_aligned(sp)
+}
+
+pub fn assert_fp_is_aligned(fp: usize) {
+    imp::assert_fp_is_aligned(fp)
+}
diff --git a/crates/wasmtime/src/runtime/vm/arch/unsupported.rs b/crates/wasmtime/src/runtime/vm/arch/unsupported.rs
new file mode 100644
index 000000000..d4073fbae
--- /dev/null
+++ b/crates/wasmtime/src/runtime/vm/arch/unsupported.rs
@@ -0,0 +1,44 @@
+compile_error!("Wasmtime's runtime is being compiled for an architecture that it does not support");
+
+cfg_if::cfg_if! {
+    if #[cfg(target_arch = "x86")] {
+    compile_error!("\
+the tracking issue for i686 support is https://github.com/bytecodealliance/wasmtime/issues/1980 \
+");
+    } else if #[cfg(target_arch = "arm")] {
+    compile_error!("\
+the tracking issue for arm support is https://github.com/bytecodealliance/wasmtime/issues/1173 \
+");
+    } else if #[cfg(target_arch = "riscv32")] {
+    compile_error!("\
+the tracking issue for riscv32 support is https://github.com/bytecodealliance/wasmtime/issues/8768 \
+");
+    } else {
+    compile_error!("\
+if you'd like feel free to file an issue for platform support at
+https://github.com/bytecodealliance/wasmtime/issues/new
+");
+    }
+}
+
+pub fn get_stack_pointer() -> usize {
+    panic!()
+}
+
+pub unsafe fn get_next_older_pc_from_fp(_fp: usize) -> usize {
+    panic!()
+}
+
+pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = 0;
+
+pub fn reached_entry_sp(_fp: usize, _entry_sp: usize) -> bool {
+    panic!()
+}
+
+pub fn assert_entry_sp_is_aligned(_sp: usize) {
+    panic!()
+}
+
+pub fn assert_fp_is_aligned(_fp: usize) {
+    panic!()
+}
diff --git a/crates/wasmtime/src/runtime/vm/gc/gc_ref.rs b/crates/wasmtime/src/runtime/vm/gc/gc_ref.rs
index 5e6333124..ae22b37c7 100644
--- a/crates/wasmtime/src/runtime/vm/gc/gc_ref.rs
+++ b/crates/wasmtime/src/runtime/vm/gc/gc_ref.rs
@@ -28,7 +28,7 @@ use wasmtime_environ::{VMGcKind, VMSharedTypeIndex};
 ///     ty: Option<VMSharedTypeIndex>,
 /// }
 /// ```
-#[repr(transparent)]
+#[repr(align(8))]
 pub struct VMGcHeader(u64);
 
 unsafe impl GcHeapObject for VMGcHeader {
diff --git a/crates/wasmtime/src/runtime/vm/instance/allocator/pooling.rs b/crates/wasmtime/src/runtime/vm/instance/allocator/pooling.rs
index 9dda05ba0..14c5e94ab 100644
--- a/crates/wasmtime/src/runtime/vm/instance/allocator/pooling.rs
+++ b/crates/wasmtime/src/runtime/vm/instance/allocator/pooling.rs
@@ -166,7 +166,10 @@ impl Default for InstanceLimits {
             // have 10k+ elements.
             table_elements: 20_000,
             max_memories_per_module: 1,
+            #[cfg(target_pointer_width = "64")]
             max_memory_size: 1 << 32, // 4G,
+            #[cfg(target_pointer_width = "32")]
+            max_memory_size: usize::MAX,
             #[cfg(feature = "gc")]
             total_gc_heaps: 1000,
         }
diff --git a/crates/wasmtime/src/runtime/vm/sys/unix/signals.rs b/crates/wasmtime/src/runtime/vm/sys/unix/signals.rs
index 91db01bab..9ea5732e1 100644
--- a/crates/wasmtime/src/runtime/vm/sys/unix/signals.rs
+++ b/crates/wasmtime/src/runtime/vm/sys/unix/signals.rs
@@ -320,6 +320,7 @@ unsafe fn get_pc_and_fp(cx: *mut libc::c_void, _signum: libc::c_int) -> (*const
         }
         else {
             compile_error!("unsupported platform");
+            panic!();
         }
     }
 }
diff --git a/crates/wasmtime/src/runtime/vm/vmcontext.rs b/crates/wasmtime/src/runtime/vm/vmcontext.rs
index 073f6d80a..27a058c05 100644
--- a/crates/wasmtime/src/runtime/vm/vmcontext.rs
+++ b/crates/wasmtime/src/runtime/vm/vmcontext.rs
@@ -1018,7 +1018,7 @@ pub union ValRaw {
 // matched in C.
 const _: () = {
     assert!(mem::size_of::<ValRaw>() == 16);
-    assert!(mem::align_of::<ValRaw>() == 8);
+    assert!(mem::align_of::<ValRaw>() == mem::align_of::<u64>());
 };
 
 // This type is just a bag-of-bits so it's up to the caller to figure out how
diff --git a/pulley/fuzz/src/interp.rs b/pulley/fuzz/src/interp.rs
index 1ef5ee10d..abc15033e 100644
--- a/pulley/fuzz/src/interp.rs
+++ b/pulley/fuzz/src/interp.rs
@@ -107,6 +107,6 @@ fn extended_op_is_safe_for_fuzzing(op: &ExtendedOp) -> bool {
     match op {
         ExtendedOp::Trap(_) => true,
         ExtendedOp::Nop(_) => true,
-        ExtendedOp::GetSp(_) => true,
+        ExtendedOp::GetSp(GetSp { dst, .. }) => !dst.is_special(),
     }
 }


Test patch:
diff --git a/cranelift/filetests/src/match_directive.rs b/cranelift/filetests/src/match_directive.rs
index bb379f25c..54944194e 100644
--- a/cranelift/filetests/src/match_directive.rs
+++ b/cranelift/filetests/src/match_directive.rs
@@ -10,11 +10,7 @@ pub fn match_directive<'a>(comment: &'a str, directive: &str) -> Option<&'a str>
         "Directive must include trailing colon"
     );
     let text = comment.trim_start_matches(';').trim_start();
-    if text.starts_with(directive) {
-        Some(text[directive.len()..].trim())
-    } else {
-        None
-    }
+    text.strip_prefix(directive).map(|s| s.trim())
 }
 
 #[test]