Repository: bytecodealliance/wasmtime
Pull request #5866 (2023-02-23 15:43:06)

s390x: Fix integer overflow during negation
Use wrapping_neg in i{64,32,16}_from_negated_value to avoid Rust aborts due to integer overflow.  The resulting INT_MIN is already handled correctly in subsequent operations.

Fixes https://github.com/bytecodealliance/wasmtime/issues/5863.

CC @afonso360 @cfallin 

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

- [ ] This has been discussed in issue #..., or if not, please tell us why
  here.
- [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.
- [ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the [code of
conduct](https://github.com/bytecodealliance/wasmtime/blob/master/CODE_OF_CONDUCT.md).
-->



Patch:
diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs
index 520cd5f37..a6dfe21e5 100644
--- a/cranelift/codegen/src/isa/s390x/lower/isle.rs
+++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs
@@ -628,21 +628,21 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, S390xBackend> {
     #[inline]
     fn i64_from_negated_value(&mut self, val: Value) -> Option<i64> {
         let constant = self.u64_from_signed_value(val)? as i64;
-        let imm = -constant;
+        let imm = constant.wrapping_neg();
         Some(imm)
     }
 
     #[inline]
     fn i32_from_negated_value(&mut self, val: Value) -> Option<i32> {
         let constant = self.u64_from_signed_value(val)? as i64;
-        let imm = i32::try_from(-constant).ok()?;
+        let imm = i32::try_from(constant.wrapping_neg()).ok()?;
         Some(imm)
     }
 
     #[inline]
     fn i16_from_negated_value(&mut self, val: Value) -> Option<i16> {
         let constant = self.u64_from_signed_value(val)? as i64;
-        let imm = i16::try_from(-constant).ok()?;
+        let imm = i16::try_from(constant.wrapping_neg()).ok()?;
         Some(imm)
     }
 
diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs
index b78ca3bee..866aa39f1 100644
--- a/crates/wasmtime/src/store.rs
+++ b/crates/wasmtime/src/store.rs
@@ -980,7 +980,7 @@ impl<'a, T> StoreContext<'a, T> {
     /// Access the underlying data owned by this `Store`.
     ///
     /// Same as [`Store::data`].
-    pub fn data(&self) -> &T {
+    pub fn data(&self) -> &'a T {
         self.0.data()
     }
 
diff --git a/crates/wasmtime/src/store/context.rs b/crates/wasmtime/src/store/context.rs
index f50557f9d..4e15eaabf 100644
--- a/crates/wasmtime/src/store/context.rs
+++ b/crates/wasmtime/src/store/context.rs
@@ -161,6 +161,13 @@ impl<T> AsContextMut for StoreContextMut<'_, T> {
     }
 }
 
+impl<'a, T> From<StoreContextMut<'a, T>> for StoreContext<'a, T> {
+    #[inline]
+    fn from(store: StoreContextMut<'a, T>) -> StoreContext<'a, T> {
+        StoreContext(store.0)
+    }
+}
+
 // Implementations for internal consumers, but these aren't public types so
 // they're not publicly accessible for crate consumers.
 impl<T> AsContext for &'_ StoreInner<T> {


Test patch:
diff --git a/crates/test-programs/Cargo.toml b/crates/test-programs/Cargo.toml
index caaa4bc32..a9a30df23 100644
--- a/crates/test-programs/Cargo.toml
+++ b/crates/test-programs/Cargo.toml
@@ -13,7 +13,7 @@ cfg-if = "1.0"
 [dev-dependencies]
 wasi-common = { workspace = true }
 wasi-cap-std-sync = { workspace = true }
-wasmtime = { workspace = true }
+wasmtime = { workspace = true, features = ['cranelift'] }
 wasmtime-wasi = { workspace = true, features = ["tokio"] }
 target-lexicon = { workspace = true }
 tracing-subscriber = { version = "0.3.1", default-features = false, features = ['fmt'] }
diff --git a/tests/all/cli_tests/threads.wat b/tests/all/cli_tests/threads.wat
index 3e5b4397b..def5011f8 100644
--- a/tests/all/cli_tests/threads.wat
+++ b/tests/all/cli_tests/threads.wat
@@ -21,12 +21,15 @@
     (drop (call $__wasi_thread_spawn (i32.const 0)))
 
     ;; Wait for all the threads to notify us that they are done.
+    (local.set $i (i32.const 0))
     (loop $again
-      ;; Retrieve the i32 at address 128, compare it to -1 (it should always
-      ;; fail) and load it atomically to check if all three threads are
-      ;; complete. This wait is for 1ms or until notified, whichever is first.
-      (drop (memory.atomic.wait32 (i32.const 128) (i32.const -1) (i64.const 1000000)))
-      (br_if $again (i32.lt_s (i32.atomic.load (i32.const 128)) (i32.const 3)))
+      ;; Wait for the i32 at address 128 to be incremented by each thread. We
+      ;; maintain a local $i with the atomically loaded value as the expected
+      ;; wait value and to check if all three threads are complete. This wait is
+      ;; for 1ms or until notified, whichever is first.
+      (drop (memory.atomic.wait32 (i32.const 128) (local.get $i) (i64.const 1000000)))
+      (local.set $i (i32.atomic.load (i32.const 128)))
+      (br_if $again (i32.lt_s (local.get $i) (i32.const 3)))
     )
 
     ;; Print "Done".