From 7787f4fa4027c39d29d9d6b3960cff5a813b34a3 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Mon, 2 Mar 2026 17:05:42 -0700 Subject: [PATCH 1/2] fix signed 8- and 16-bit loads in wit-dylib bindgen --- crates/wit-dylib/src/bindgen.rs | 53 +++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/crates/wit-dylib/src/bindgen.rs b/crates/wit-dylib/src/bindgen.rs index deffa780e9..f862fa84d5 100644 --- a/crates/wit-dylib/src/bindgen.rs +++ b/crates/wit-dylib/src/bindgen.rs @@ -1344,24 +1344,24 @@ impl<'a> FunctionCompiler<'a> { fn lift(&mut self, src: &AbiLoc<'_>, ty: Type) { let i = self.adapter.intrinsics(); match ty { - Type::Bool => self.push_scalar(src, ValType::I32, i.push_bool, 0), - Type::Char => self.push_scalar(src, ValType::I32, i.push_char, 2), - Type::U8 => self.push_scalar(src, ValType::I32, i.push_u8, 0), - Type::S8 => self.push_scalar(src, ValType::I32, i.push_s8, 0), - Type::U16 => self.push_scalar(src, ValType::I32, i.push_u16, 1), - Type::S16 => self.push_scalar(src, ValType::I32, i.push_s16, 1), - Type::U32 => self.push_scalar(src, ValType::I32, i.push_u32, 2), - Type::S32 => self.push_scalar(src, ValType::I32, i.push_s32, 2), - Type::U64 => self.push_scalar(src, ValType::I64, i.push_u64, 3), - Type::S64 => self.push_scalar(src, ValType::I64, i.push_s64, 3), - Type::F32 => self.push_scalar(src, ValType::F32, i.push_f32, 2), - Type::F64 => self.push_scalar(src, ValType::F64, i.push_f64, 3), + Type::Bool => self.push_scalar(src, ValType::I32, i.push_bool, 0, false), + Type::Char => self.push_scalar(src, ValType::I32, i.push_char, 2, false), + Type::U8 => self.push_scalar(src, ValType::I32, i.push_u8, 0, false), + Type::S8 => self.push_scalar(src, ValType::I32, i.push_s8, 0, true), + Type::U16 => self.push_scalar(src, ValType::I32, i.push_u16, 1, false), + Type::S16 => self.push_scalar(src, ValType::I32, i.push_s16, 1, true), + Type::U32 => self.push_scalar(src, ValType::I32, i.push_u32, 2, false), + Type::S32 => self.push_scalar(src, ValType::I32, i.push_s32, 2, true), + Type::U64 => self.push_scalar(src, ValType::I64, i.push_u64, 3, false), + Type::S64 => self.push_scalar(src, ValType::I64, i.push_s64, 3, true), + Type::F32 => self.push_scalar(src, ValType::F32, i.push_f32, 2, false), + Type::F64 => self.push_scalar(src, ValType::F64, i.push_f64, 3, false), Type::String => { let push_string = i.push_string; let (src_ptr, src_len) = src.split_ptr_len(); self.local_get_ctx(); - self.load_abi_loc(&src_ptr, ValType::I32, 2); - self.load_abi_loc(&src_len, ValType::I32, 2); + self.load_abi_loc(&src_ptr, ValType::I32, 2, false); + self.load_abi_loc(&src_len, ValType::I32, 2, false); self.ins().call(push_string); } Type::ErrorContext => todo!("error-context"), @@ -1502,9 +1502,9 @@ impl<'a> FunctionCompiler<'a> { // ownership of the allocation. self.local_get_ctx(); self.ins().i32_const(list_index); - self.load_abi_loc(&src_ptr, ValType::I32, 2); + self.load_abi_loc(&src_ptr, ValType::I32, 2, false); let l_ptr = self.local_tee_new_tmp(ValType::I32); - self.load_abi_loc(&src_len, ValType::I32, 2); + self.load_abi_loc(&src_len, ValType::I32, 2, false); let l_len = self.local_tee_new_tmp(ValType::I32); self.ins().call(push_list); @@ -1643,7 +1643,7 @@ impl<'a> FunctionCompiler<'a> { let payload0 = iter.next().unwrap(); let payload1 = iter.next().unwrap(); - self.load_abi_loc(&discr_src, ValType::I32, tag_size_log2); + self.load_abi_loc(&discr_src, ValType::I32, tag_size_log2, false); self.ins().if_(BlockType::Empty); { lift_payload(self, payload1); @@ -1676,7 +1676,7 @@ impl<'a> FunctionCompiler<'a> { // Block to jump out of self.ins().block(BlockType::Empty); - self.load_abi_loc(&discr_src, ValType::I32, tag_size_log2); + self.load_abi_loc(&discr_src, ValType::I32, tag_size_log2, false); self.ins().br_table(1..(n + 1) as u32, 0); self.ins().end(); @@ -1711,17 +1711,24 @@ impl<'a> FunctionCompiler<'a> { ) { self.local_get_ctx(); self.ins().i32_const(type_index.try_into().unwrap()); - self.load_abi_loc(src, ValType::I32, size_log2); + self.load_abi_loc(src, ValType::I32, size_log2, false); self.ins().call(push_intrinsic); } - fn push_scalar(&mut self, src: &AbiLoc<'_>, ty: ValType, push_intrinsic: u32, size_log2: u32) { + fn push_scalar( + &mut self, + src: &AbiLoc<'_>, + ty: ValType, + push_intrinsic: u32, + size_log2: u32, + signed: bool, + ) { self.local_get_ctx(); - self.load_abi_loc(src, ty, size_log2); + self.load_abi_loc(src, ty, size_log2, signed); self.ins().call(push_intrinsic); } - fn load_abi_loc(&mut self, src: &AbiLoc<'_>, ty: ValType, size_log2: u32) { + fn load_abi_loc(&mut self, src: &AbiLoc<'_>, ty: ValType, size_log2: u32, signed: bool) { match src { AbiLoc::Stack([local]) => { self.ins().local_get(local.idx); @@ -1770,7 +1777,9 @@ impl<'a> FunctionCompiler<'a> { self.ins().local_get(mem.addr.idx); let memarg = mem.memarg(size_log2); match (ty, size_log2) { + (ValType::I32, 0) if signed => self.ins().i32_load8_s(memarg), (ValType::I32, 0) => self.ins().i32_load8_u(memarg), + (ValType::I32, 1) if signed => self.ins().i32_load16_s(memarg), (ValType::I32, 1) => self.ins().i32_load16_u(memarg), (ValType::I32, 2) => self.ins().i32_load(memarg), (ValType::I64, 3) => self.ins().i64_load(memarg), From 22062e4b5b7cd75f4e583b5887cfa2c3621d6702 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Thu, 5 Mar 2026 15:11:45 -0700 Subject: [PATCH 2/2] add tests for signed 8- and 16-bit loads Ideally, we'd model this as a runtime test which uses WAT to avoid compiler-optimization-induced unpredictability, but that would be a lot of work, so we settle for checking that the `wit-dylib` output does in fact use signed loads when the WIT world requires them. I've verified that these tests fail without my earlier fix. --- crates/wit-dylib/src/lib.rs | 59 +++++++++++++++++++++++++++++ crates/wit-dylib/tests/roundtrip.rs | 1 - 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/crates/wit-dylib/src/lib.rs b/crates/wit-dylib/src/lib.rs index fc515d713e..cf83f15923 100644 --- a/crates/wit-dylib/src/lib.rs +++ b/crates/wit-dylib/src/lib.rs @@ -1404,3 +1404,62 @@ fn dealias(resolve: &Resolve, mut id: TypeId) -> TypeId { } } } + +#[cfg(test)] +mod tests { + use wasmparser::{Operator, Parser, Payload}; + use wit_parser::Resolve; + + #[derive(Copy, Clone)] + enum Which { + S8, + S16, + } + + #[test] + fn s8_uses_signed_load() { + assert!(find_signed_load(Which::S8)); + } + + #[test] + fn s16_uses_signed_load() { + assert!(find_signed_load(Which::S16)); + } + + fn find_signed_load(which: Which) -> bool { + let mut resolve = Resolve::default(); + let ty = match which { + Which::S8 => "s8", + Which::S16 => "s16", + }; + let package = resolve + .push_str( + "wit", + &format!( + "package test:test; +world w {{ + export foo: func(v: list<{ty}>); +}} +" + ), + ) + .unwrap(); + let world = resolve.select_world(&[package], None).unwrap(); + let adapter = super::create(&resolve, world, None); + for payload in Parser::new(0).parse_all(&adapter) { + match payload.unwrap() { + Payload::CodeSectionEntry(body) => { + for operator in body.get_operators_reader().unwrap() { + match (operator.unwrap(), which) { + (Operator::I32Load16S { .. }, Which::S16) + | (Operator::I32Load8S { .. }, Which::S8) => return true, + _ => {} + } + } + } + _ => {} + } + } + false + } +} diff --git a/crates/wit-dylib/tests/roundtrip.rs b/crates/wit-dylib/tests/roundtrip.rs index c32d03881f..d04d9a3226 100644 --- a/crates/wit-dylib/tests/roundtrip.rs +++ b/crates/wit-dylib/tests/roundtrip.rs @@ -67,7 +67,6 @@ fn run_one(u: &mut Unstructured<'_>) -> Result<()> { config.streams = false; // TODO config.async_ = false; let wasm = wit_smith::smith(&config, u)?; - std::fs::write("./hello.wasm", &wasm).unwrap(); let (mut resolve, _pkg) = match wit_parser::decoding::decode(&wasm).unwrap() { DecodedWasm::WitPackage(resolve, pkg) => (resolve, pkg), DecodedWasm::Component(..) => unreachable!(),