Skip to content

Commit c74706a

Browse files
authored
feat: implement memory.atomic.notify,wait32,wait64 (#5255)
* feat: implement memory.atomic.notify,wait32,wait64 Added the parking_spot crate, which provides the needed registry for the operations. Signed-off-by: Harald Hoyer <[email protected]> * fix: change trap message for HeapMisaligned The threads spec test wants "unaligned atomic" instead of "misaligned memory access". Signed-off-by: Harald Hoyer <[email protected]> * tests: add test for atomic wait on non-shared memory Signed-off-by: Harald Hoyer <[email protected]> * tests: add tests/spec_testsuite/proposals/threads without pooling and reference types. Also "shared_memory" is added to the "spectest" interface. Signed-off-by: Harald Hoyer <[email protected]> * tests: add atomics_notify.wast checking that notify with 0 waiters returns 0 on shared and non-shared memory. Signed-off-by: Harald Hoyer <[email protected]> * tests: add tests for atomic wait on shared memory - return 2 - timeout for 0 - return 2 - timeout for 1000ns - return 1 - invalid value Signed-off-by: Harald Hoyer <[email protected]> * fixup! feat: implement memory.atomic.notify,wait32,wait64 Signed-off-by: Harald Hoyer <[email protected]> * fixup! feat: implement memory.atomic.notify,wait32,wait64 Signed-off-by: Harald Hoyer <[email protected]> Signed-off-by: Harald Hoyer <[email protected]>
1 parent fe2bfdb commit c74706a

File tree

21 files changed

+972
-114
lines changed

21 files changed

+972
-114
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ wasmtime-wast = { workspace = true, features = ['component-model'] }
6363
wasmtime-component-util = { workspace = true }
6464
component-macro-test = { path = "crates/misc/component-macro-test" }
6565
component-test-util = { workspace = true }
66+
bstr = "0.2.17"
6667

6768
[target.'cfg(windows)'.dev-dependencies]
6869
windows-sys = { workspace = true, features = ["Win32_System_Memory"] }

build.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::process::Command;
1212

1313
fn main() -> anyhow::Result<()> {
1414
println!("cargo:rerun-if-changed=build.rs");
15+
1516
let out_dir = PathBuf::from(
1617
env::var_os("OUT_DIR").expect("The OUT_DIR environment variable must be set"),
1718
);
@@ -43,6 +44,7 @@ fn main() -> anyhow::Result<()> {
4344
"tests/spec_testsuite/proposals/multi-memory",
4445
strategy,
4546
)?;
47+
test_directory_module(out, "tests/spec_testsuite/proposals/threads", strategy)?;
4648
} else {
4749
println!(
4850
"cargo:warning=The spec testsuite is disabled. To enable, run `git submodule \
@@ -62,7 +64,6 @@ fn main() -> anyhow::Result<()> {
6264
drop(Command::new("rustfmt").arg(&output).status());
6365
Ok(())
6466
}
65-
6667
fn test_directory_module(
6768
out: &mut String,
6869
path: impl AsRef<Path>,
@@ -91,7 +92,7 @@ fn test_directory(
9192
return None;
9293
}
9394
// Ignore files starting with `.`, which could be editor temporary files
94-
if p.file_stem()?.to_str()?.starts_with(".") {
95+
if p.file_stem()?.to_str()?.starts_with('.') {
9596
return None;
9697
}
9798
Some(p)
@@ -116,8 +117,7 @@ fn extract_name(path: impl AsRef<Path>) -> String {
116117
.expect("filename should have a stem")
117118
.to_str()
118119
.expect("filename should be representable as a string")
119-
.replace("-", "_")
120-
.replace("/", "_")
120+
.replace(['-', '/'], "_")
121121
}
122122

123123
fn with_test_module<T>(
@@ -163,7 +163,7 @@ fn write_testsuite_tests(
163163
" crate::wast::run_wast(r#\"{}\"#, crate::wast::Strategy::{}, {}).unwrap();",
164164
path.display(),
165165
strategy,
166-
pooling
166+
pooling,
167167
)?;
168168
writeln!(out, "}}")?;
169169
writeln!(out)?;

crates/environ/src/trap_encoding.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ pub enum Trap {
8484
/// When wasm code is configured to consume fuel and it runs out of fuel
8585
/// then this trap will be raised.
8686
OutOfFuel,
87+
88+
/// Used to indicate that a trap was raised by atomic wait operations on non shared memory.
89+
AtomicWaitNonSharedMemory,
90+
// if adding a variant here be sure to update the `check!` macro below
8791
}
8892

8993
impl fmt::Display for Trap {
@@ -93,7 +97,7 @@ impl fmt::Display for Trap {
9397
let desc = match self {
9498
StackOverflow => "call stack exhausted",
9599
MemoryOutOfBounds => "out of bounds memory access",
96-
HeapMisaligned => "misaligned memory access",
100+
HeapMisaligned => "unaligned atomic",
97101
TableOutOfBounds => "undefined element: out of bounds table access",
98102
IndirectCallToNull => "uninitialized element",
99103
BadSignature => "indirect call type mismatch",
@@ -104,6 +108,7 @@ impl fmt::Display for Trap {
104108
Interrupt => "interrupt",
105109
AlwaysTrapAdapter => "degenerate component adapter called",
106110
OutOfFuel => "all fuel consumed by WebAssembly",
111+
AtomicWaitNonSharedMemory => "atomic wait on non-shared memory",
107112
};
108113
write!(f, "wasm trap: {desc}")
109114
}
@@ -217,6 +222,7 @@ pub fn lookup_trap_code(section: &[u8], offset: usize) -> Option<Trap> {
217222
Interrupt
218223
AlwaysTrapAdapter
219224
OutOfFuel
225+
AtomicWaitNonSharedMemory
220226
}
221227

222228
if cfg!(debug_assertions) {

crates/fuzzing/src/oracles.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ pub fn spectest(mut fuzz_config: generators::Config, test: generators::SpecTest)
506506
fuzz_config.set_spectest_compliant();
507507
log::debug!("running {:?}", test.file);
508508
let mut wast_context = WastContext::new(fuzz_config.to_store());
509-
wast_context.register_spectest().unwrap();
509+
wast_context.register_spectest(false).unwrap();
510510
wast_context
511511
.run_buffer(test.file, test.contents.as_bytes())
512512
.unwrap();

crates/runtime/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ features = [
4545
"Win32_Security",
4646
]
4747

48+
[dev-dependencies]
49+
once_cell = { workspace = true }
50+
4851
[build-dependencies]
4952
cc = "1.0"
5053

crates/runtime/src/instance.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,17 @@ impl Instance {
196196
}
197197
}
198198

199+
/// Get a locally defined or imported memory.
200+
pub(crate) fn get_runtime_memory(&mut self, index: MemoryIndex) -> &mut Memory {
201+
if let Some(defined_index) = self.module().defined_memory_index(index) {
202+
unsafe { &mut *self.get_defined_memory(defined_index) }
203+
} else {
204+
let import = self.imported_memory(index);
205+
let ctx = unsafe { &mut *import.vmctx };
206+
unsafe { &mut *ctx.instance_mut().get_defined_memory(import.index) }
207+
}
208+
}
209+
199210
/// Return the indexed `VMMemoryDefinition`.
200211
fn memory(&self, index: DefinedMemoryIndex) -> VMMemoryDefinition {
201212
unsafe { VMMemoryDefinition::load(self.memory_ptr(index)) }

crates/runtime/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ mod instance;
4040
mod memory;
4141
mod mmap;
4242
mod mmap_vec;
43+
mod parking_spot;
4344
mod table;
4445
mod traphandlers;
4546
mod vmcontext;

crates/runtime/src/libcalls.rs

Lines changed: 59 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@
5555
//! ```
5656
5757
use crate::externref::VMExternRef;
58-
use crate::instance::Instance;
5958
use crate::table::{Table, TableElementType};
6059
use crate::vmcontext::{VMCallerCheckedAnyfunc, VMContext};
61-
use crate::TrapReason;
60+
use crate::{SharedMemory, TrapReason};
6261
use anyhow::Result;
6362
use std::mem;
6463
use std::ptr::{self, NonNull};
64+
use std::time::{Duration, Instant};
6565
use wasmtime_environ::{
6666
DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, TableIndex, Trap,
6767
};
@@ -434,81 +434,81 @@ unsafe fn externref_global_set(vmctx: *mut VMContext, index: u32, externref: *mu
434434
unsafe fn memory_atomic_notify(
435435
vmctx: *mut VMContext,
436436
memory_index: u32,
437-
addr: u64,
438-
_count: u32,
437+
addr_index: u64,
438+
count: u32,
439439
) -> Result<u32, TrapReason> {
440440
let memory = MemoryIndex::from_u32(memory_index);
441-
let instance = (*vmctx).instance();
442-
validate_atomic_addr(instance, memory, addr, 4, 4)?;
443-
Err(
444-
anyhow::anyhow!("unimplemented: wasm atomics (fn memory_atomic_notify) unsupported",)
445-
.into(),
446-
)
441+
let instance = (*vmctx).instance_mut();
442+
instance
443+
.get_memory(memory)
444+
.validate_addr(addr_index, 4, 4)?;
445+
446+
let shared_mem = instance.get_runtime_memory(memory).as_shared_memory();
447+
448+
if count == 0 {
449+
return Ok(0);
450+
}
451+
452+
let unparked_threads = shared_mem.map_or(0, |shared_mem| {
453+
// SAFETY: checked `addr_index` above
454+
unsafe { shared_mem.unchecked_atomic_notify(addr_index, count) }
455+
});
456+
457+
Ok(unparked_threads)
447458
}
448459

449460
// Implementation of `memory.atomic.wait32` for locally defined memories.
450461
unsafe fn memory_atomic_wait32(
451462
vmctx: *mut VMContext,
452463
memory_index: u32,
453-
addr: u64,
454-
_expected: u32,
455-
_timeout: u64,
464+
addr_index: u64,
465+
expected: u32,
466+
timeout: u64,
456467
) -> Result<u32, TrapReason> {
468+
// convert timeout to Instant, before any wait happens on locking
469+
let timeout = (timeout as i64 >= 0).then(|| Instant::now() + Duration::from_nanos(timeout));
470+
457471
let memory = MemoryIndex::from_u32(memory_index);
458-
let instance = (*vmctx).instance();
459-
validate_atomic_addr(instance, memory, addr, 4, 4)?;
460-
Err(
461-
anyhow::anyhow!("unimplemented: wasm atomics (fn memory_atomic_wait32) unsupported",)
462-
.into(),
463-
)
472+
let instance = (*vmctx).instance_mut();
473+
let addr = instance
474+
.get_memory(memory)
475+
.validate_addr(addr_index, 4, 4)?;
476+
477+
let shared_mem: SharedMemory = instance
478+
.get_runtime_memory(memory)
479+
.as_shared_memory()
480+
.ok_or(Trap::AtomicWaitNonSharedMemory)?;
481+
482+
// SAFETY: checked `addr_index` above
483+
let res = unsafe { shared_mem.unchecked_atomic_wait32(addr_index, addr, expected, timeout) };
484+
Ok(res)
464485
}
465486

466487
// Implementation of `memory.atomic.wait64` for locally defined memories.
467488
unsafe fn memory_atomic_wait64(
468489
vmctx: *mut VMContext,
469490
memory_index: u32,
470-
addr: u64,
471-
_expected: u64,
472-
_timeout: u64,
491+
addr_index: u64,
492+
expected: u64,
493+
timeout: u64,
473494
) -> Result<u32, TrapReason> {
474-
let memory = MemoryIndex::from_u32(memory_index);
475-
let instance = (*vmctx).instance();
476-
validate_atomic_addr(instance, memory, addr, 8, 8)?;
477-
Err(
478-
anyhow::anyhow!("unimplemented: wasm atomics (fn memory_atomic_wait64) unsupported",)
479-
.into(),
480-
)
481-
}
482-
483-
macro_rules! ensure {
484-
($cond:expr, $trap:expr) => {
485-
if !($cond) {
486-
return Err($trap);
487-
}
488-
};
489-
}
495+
// convert timeout to Instant, before any wait happens on locking
496+
let timeout = (timeout as i64 >= 0).then(|| Instant::now() + Duration::from_nanos(timeout));
490497

491-
/// In the configurations where bounds checks were elided in JIT code (because
492-
/// we are using static memories with virtual memory guard pages) this manual
493-
/// check is here so we don't segfault from Rust. For other configurations,
494-
/// these checks are required anyways.
495-
unsafe fn validate_atomic_addr(
496-
instance: &Instance,
497-
memory: MemoryIndex,
498-
addr: u64,
499-
access_size: u64,
500-
access_alignment: u64,
501-
) -> Result<(), Trap> {
502-
debug_assert!(access_alignment.is_power_of_two());
503-
ensure!(addr % access_alignment == 0, Trap::HeapMisaligned);
504-
505-
let length = u64::try_from(instance.get_memory(memory).current_length()).unwrap();
506-
ensure!(
507-
addr.saturating_add(access_size) < length,
508-
Trap::MemoryOutOfBounds
509-
);
510-
511-
Ok(())
498+
let memory = MemoryIndex::from_u32(memory_index);
499+
let instance = (*vmctx).instance_mut();
500+
let addr = instance
501+
.get_memory(memory)
502+
.validate_addr(addr_index, 8, 8)?;
503+
504+
let shared_mem: SharedMemory = instance
505+
.get_runtime_memory(memory)
506+
.as_shared_memory()
507+
.ok_or(Trap::AtomicWaitNonSharedMemory)?;
508+
509+
// SAFETY: checked `addr_index` above
510+
let res = unsafe { shared_mem.unchecked_atomic_wait64(addr_index, addr, expected, timeout) };
511+
Ok(res)
512512
}
513513

514514
// Hook for when an instance runs out of fuel.

0 commit comments

Comments
 (0)