From 3e830f576d9d9edb592eb76198fda4a2466d9adf Mon Sep 17 00:00:00 2001 From: Timmy Silesmo Date: Sat, 15 Mar 2025 15:41:40 +0100 Subject: [PATCH 1/2] Fixes wasi http header bug and adds a test for it --- crates/csharp/src/function.rs | 46 ++++++++++++------------ crates/csharp/src/interface.rs | 14 ++++---- crates/csharp/src/world_generator.rs | 52 +++++++++++++-------------- tests/runtime/lists.rs | 28 +++++++++++++++ tests/runtime/lists/wasm.cs | 53 ++++++++++++++++------------ tests/runtime/lists/world.wit | 2 ++ 6 files changed, 114 insertions(+), 81 deletions(-) diff --git a/crates/csharp/src/function.rs b/crates/csharp/src/function.rs index 698d19ba9..6ad827c83 100644 --- a/crates/csharp/src/function.rs +++ b/crates/csharp/src/function.rs @@ -66,7 +66,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { resource_drops: Vec::new(), is_block: false, fixed_statments: Vec::new(), - parameter_type: parameter_type, + parameter_type, } } @@ -458,14 +458,14 @@ impl Bindgen for FunctionBindgen<'_, '_> { if flags.flags.len() > 32 { results.push(format!( "unchecked((int)(((long){}) & uint.MaxValue))", - operands[0].to_string() + operands[0] )); results.push(format!( "unchecked(((int)((long){} >> 32)))", - operands[0].to_string() + operands[0] )); } else { - results.push(format!("(int){}", operands[0].to_string())); + results.push(format!("(int){}", operands[0])); } } @@ -479,8 +479,8 @@ impl Bindgen for FunctionBindgen<'_, '_> { results.push(format!( "({})(unchecked((uint)({})) | (ulong)(unchecked((uint)({}))) << 32)", qualified_type_name, - operands[0].to_string(), - operands[1].to_string() + operands[0], + operands[1] )); } else { results.push(format!("({})({})", qualified_type_name, operands[0])) @@ -502,7 +502,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { let mut result = format!("new {} (\n", qualified_type_name); result.push_str(&operands.join(", ")); - result.push_str(")"); + result.push(')'); results.push(result); } @@ -754,7 +754,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { " ); } - results.push(format!("(nint){ptr}")); + results.push(format!("(int){ptr}")); results.push(format!("({list}).Length")); } Direction::Export => { @@ -834,7 +834,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { results.push(format!("(int){str_ptr}")); } - results.push(format!("{length}")); + results.push(length); if FunctionKind::Freestanding == *self.kind || self.interface_gen.direction == Direction::Export { self.interface_gen.require_interop_using("System.Text"); self.interface_gen.require_interop_using("System.Runtime.InteropServices"); @@ -889,13 +889,13 @@ impl Bindgen for FunctionBindgen<'_, '_> { " void* {address}; if (({size} * {list}.Count) < 1024) {{ - var {ret_area} = stackalloc {element_type}[({array_size}*{list}.Count)+1]; - {address} = (void*)(((int){ret_area}) + ({align} - 1) & -{align}); + var {ret_area} = stackalloc {element_type}[{array_size}*{list}.Count]; + {address} = (void*)((int){ret_area}); }} else {{ - var {buffer_size} = {size} * (nuint){list}.Count; - {address} = NativeMemory.AlignedAlloc({buffer_size}, {align}); + var {buffer_size} = {size} * {list}.Count; + {address} = NativeMemory.AlignedAlloc((nuint){buffer_size}, {align}); cleanups.Add(() => NativeMemory.AlignedFree({address})); }} " @@ -905,8 +905,8 @@ impl Bindgen for FunctionBindgen<'_, '_> { //cabi_realloc_post_return will be called to clean up this allocation uwrite!(self.src, " - var {buffer_size} = {size} * (nuint){list}.Count; - void* {address} = NativeMemory.AlignedAlloc({buffer_size}, {align}); + var {buffer_size} = {size} * {list}.Count; + void* {address} = NativeMemory.AlignedAlloc((nuint){buffer_size}, {align}); " ); } @@ -1046,7 +1046,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { } Instruction::Return { amt, .. } => { - if self.fixed_statments.len() > 0 { + if !self.fixed_statments.is_empty() { let fixed: String = self.fixed_statments.iter().map(|f| format!("{} = {}", f.ptr_name, f.item_to_pin)).collect::>().join(", "); self.src.insert_str(0, &format!("fixed (void* {fixed}) {{ @@ -1077,7 +1077,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { } } - if self.fixed_statments.len() > 0 { + if !self.fixed_statments.is_empty() { uwriteln!(self.src, "}}"); } } @@ -1201,7 +1201,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { } } } - results.push(format!("{handle}")); + results.push(handle); } Instruction::HandleLift { @@ -1253,7 +1253,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { } Instruction::Flush { amt } => { - results.extend(operands.iter().take(*amt).map(|v| v.clone())); + results.extend(operands.iter().take(*amt).cloned()); } Instruction::AsyncPostCallInterface { .. } @@ -1296,7 +1296,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { ", align = align.align_wasm32() ); - format!("{ptr}") + ptr } Direction::Export => { // exports need their return area to be live until the post-return call. @@ -1319,7 +1319,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { ); self.interface_gen.csharp_gen.needs_export_return_area = true; - format!("{ptr}") + ptr } } } @@ -1395,8 +1395,8 @@ fn perform_cast(op: &String, cast: &Bitcast) -> String { Bitcast::F64ToI64 => format!("BitConverter.DoubleToInt64Bits({op})"), Bitcast::I32ToI64 => format!("(long) ({op})"), Bitcast::I64ToI32 => format!("(int) ({op})"), - Bitcast::I64ToP64 => format!("{op}"), - Bitcast::P64ToI64 => format!("{op}"), + Bitcast::I64ToP64 => op.to_string(), + Bitcast::P64ToI64 => op.to_string(), Bitcast::LToI64 | Bitcast::PToP64 => format!("(long) ({op})"), Bitcast::I64ToL | Bitcast::P64ToP => format!("(int) ({op})"), Bitcast::I32ToP diff --git a/crates/csharp/src/interface.rs b/crates/csharp/src/interface.rs index 57aa499c0..09a1d20f2 100644 --- a/crates/csharp/src/interface.rs +++ b/crates/csharp/src/interface.rs @@ -299,12 +299,12 @@ impl InterfaceGenerator<'_> { fn gen_import_src( &mut self, func: &Function, - results: &Vec, + results: &[TypeId], parameter_type: ParameterType, ) -> (String, String) { let mut bindgen = FunctionBindgen::new( self, - &func.item_name(), + func.item_name(), &func.kind, func.params .iter() @@ -317,7 +317,7 @@ impl InterfaceGenerator<'_> { } }) .collect(), - results.clone(), + results.to_vec(), parameter_type, ); @@ -399,7 +399,7 @@ impl InterfaceGenerator<'_> { let mut bindgen = FunctionBindgen::new( self, - &func.item_name(), + func.item_name(), &func.kind, (0..sig.params.len()).map(|i| format!("p{i}")).collect(), results, @@ -565,9 +565,7 @@ impl InterfaceGenerator<'_> { let ty = &self.resolve.types[*id]; match &ty.kind { TypeDefKind::Type(ty) => self.is_primative_list(ty), - TypeDefKind::List(ty) if crate::world_generator::is_primitive(ty) => { - return true - } + TypeDefKind::List(ty) if crate::world_generator::is_primitive(ty) => true, _ => false, } } @@ -791,7 +789,7 @@ impl InterfaceGenerator<'_> { Direction::Export => { let prefix = key .map(|s| format!("{}#", self.resolve.name_world_key(s))) - .unwrap_or_else(String::new); + .unwrap_or_default(); self.require_interop_using("System.Runtime.InteropServices"); uwrite!( diff --git a/crates/csharp/src/world_generator.rs b/crates/csharp/src/world_generator.rs index d30a5b596..1907d38c2 100644 --- a/crates/csharp/src/world_generator.rs +++ b/crates/csharp/src/world_generator.rs @@ -299,7 +299,7 @@ impl WorldGenerator for CSharp { let world = &resolve.worlds[id]; let world_namespace = self.qualifier(); let world_namespace = world_namespace.strip_suffix(".").unwrap(); - let namespace = format!("{world_namespace}"); + let namespace = world_namespace; let name = world.name.to_upper_camel_case(); let version = env!("CARGO_PKG_VERSION"); @@ -521,12 +521,12 @@ impl WorldGenerator for CSharp { } if self.needs_rep_table { - src.push_str("\n"); + src.push('\n'); src.push_str(include_str!("RepTable.cs")); } if !&self.world_fragments.is_empty() { - src.push_str("\n"); + src.push('\n'); src.push_str("namespace exports {\n"); @@ -535,20 +535,19 @@ impl WorldGenerator for CSharp { .world_fragments .iter() .flat_map(|f| &f.interop_usings) - .into_iter() .collect::>() // de-dup across world_fragments .iter() .map(|s| "using ".to_owned() + s + ";") .collect::>() .join("\n"), ); - src.push_str("\n"); + src.push('\n'); src.push_str(&format!("{access} static class {name}World\n")); - src.push_str("{"); + src.push('{'); for fragment in &self.world_fragments { - src.push_str("\n"); + src.push('\n'); src.push_str(&fragment.csharp_interop_src); } @@ -556,7 +555,7 @@ impl WorldGenerator for CSharp { src.push_str("}\n"); } - src.push_str("\n"); + src.push('\n'); src.push_str("}\n"); @@ -594,11 +593,11 @@ impl WorldGenerator for CSharp { let (fragments, fully_qualified_namespace) = match stubs { Stubs::World(fragments) => { - let fully_qualified_namespace = format!("{namespace}"); + let fully_qualified_namespace = namespace.to_string(); (fragments, fully_qualified_namespace) } Stubs::Interface(fragments) => { - let fully_qualified_namespace = format!("{stub_namespace}"); + let fully_qualified_namespace = stub_namespace.clone(); (fragments, fully_qualified_namespace) } }; @@ -638,7 +637,7 @@ impl WorldGenerator for CSharp { //TODO: This is currently needed for mono even if it's built as a library. if self.opts.runtime == CSharpRuntime::Mono { files.push( - &"MonoEntrypoint.cs".to_string(), + "MonoEntrypoint.cs", indent(&format!( r#" {access} class MonoEntrypoint() {{ @@ -671,13 +670,9 @@ impl WorldGenerator for CSharp { // intended to be used non-interactively at link time, the // linker will have no additional information to resolve such // ambiguity. - let (resolve, world) = - wit_parser::decoding::decode_world(&wit_component::metadata::encode( - &resolve, - id, - self.opts.string_encoding, - None, - )?)?; + let (resolve, world) = wit_parser::decoding::decode_world( + &wit_component::metadata::encode(resolve, id, self.opts.string_encoding, None)?, + )?; let pkg = resolve.worlds[world].package.unwrap(); let mut printer = WitPrinter::default(); @@ -731,7 +726,7 @@ impl WorldGenerator for CSharp { .collect::>() .join("\n"); - if body.len() > 0 { + if !body.is_empty() { let body = format!( "{header} {0} @@ -802,11 +797,16 @@ enum Stubs<'a> { // We cant use "StructLayout.Pack" as dotnet will use the minimum of the type and the "Pack" field, // so for byte it would always use 1 regardless of the "Pack". pub fn dotnet_aligned_array(array_size: usize, required_alignment: usize) -> (usize, String) { + let num_elements = if array_size % required_alignment > 0 { + array_size / required_alignment + 1 + } else { + array_size / required_alignment + }; match required_alignment { - 1 => (array_size, "byte".to_owned()), - 2 => ((array_size + 1) / 2, "ushort".to_owned()), - 4 => ((array_size + 3) / 4, "uint".to_owned()), - 8 => ((array_size + 7) / 8, "ulong".to_owned()), + 1 => (num_elements, "byte".to_owned()), + 2 => (num_elements, "ushort".to_owned()), + 4 => (num_elements, "uint".to_owned()), + 8 => (num_elements, "ulong".to_owned()), _ => todo!("unsupported return_area_align {}", required_alignment), } } @@ -885,11 +885,7 @@ fn interface_name( ); if let Some(version) = &name.version { - let v = version - .to_string() - .replace('.', "_") - .replace('-', "_") - .replace('+', "_"); + let v = version.to_string().replace(['.', '-', '+'], "_"); ns = format!("{}v{}.", ns, &v); } ns diff --git a/tests/runtime/lists.rs b/tests/runtime/lists.rs index 712cd8003..0f68b1edd 100644 --- a/tests/runtime/lists.rs +++ b/tests/runtime/lists.rs @@ -75,6 +75,13 @@ impl test::lists::test::Host for MyImports { s.to_string() } + fn wasi_http_headers_roundtrip( + &mut self, + headers: Vec<(String, Vec)>, + ) -> Vec<(String, Vec)> { + headers.to_vec() + } + fn list_minmax8(&mut self, u: Vec, s: Vec) -> (Vec, Vec) { assert_eq!(u, [u8::MIN, u8::MAX]); assert_eq!(s, [i8::MIN, i8::MAX]); @@ -151,6 +158,27 @@ fn run_test(lists: Lists, store: &mut Store>) -> Result<( exports.call_string_roundtrip(&mut *store, "hello ⚑ world")?, "hello ⚑ world" ); + + let result = exports.call_wasi_http_headers_roundtrip( + &mut *store, + &vec![ + ("Content-Type".to_owned(), "text/plain".as_bytes().to_vec()), + ( + "Content-Length".to_owned(), + "Not found".len().to_string().as_bytes().to_vec(), + ), + ], + )?; + let (key, value) = result.get(0).unwrap(); + assert_eq!(key.as_str(), "Content-Type"); + assert_eq!(std::str::from_utf8(value).unwrap(), "text/plain"); + + let (key, value) = result.get(1).unwrap(); + assert_eq!(key.as_str(), "Content-Length"); + assert_eq!( + std::str::from_utf8(value).unwrap(), + "Not found".len().to_string().as_str() + ); // Ensure that we properly called `free` everywhere in all the glue that we // needed to. assert_eq!(bytes, lists.call_allocated_bytes(&mut *store)?); diff --git a/tests/runtime/lists/wasm.cs b/tests/runtime/lists/wasm.cs index 2a7b96258..ee1a261d4 100644 --- a/tests/runtime/lists/wasm.cs +++ b/tests/runtime/lists/wasm.cs @@ -4,7 +4,8 @@ using ListsWorld.wit.imports.test.lists; using System.Text; -namespace ListsWorld { +namespace ListsWorld +{ public class ListsWorldImpl : IListsWorld { @@ -57,28 +58,25 @@ public static void TestImports() TestInterop.ListParamLarge(randomStrings); { - byte[] result = TestInterop.ListResult(); - Debug.Assert(result.Length == 5); - Debug.Assert(result[0] == (byte)1); - Debug.Assert(result[1] == (byte)2); - Debug.Assert(result[2] == (byte)3); - Debug.Assert(result[3] == (byte)4); - Debug.Assert(result[4] == (byte)5); + byte[] result = TestInterop.ListResult(); + Debug.Assert(result.Length == 5); + Debug.Assert(result[0] == (byte)1); + Debug.Assert(result[1] == (byte)2); + Debug.Assert(result[2] == (byte)3); + Debug.Assert(result[3] == (byte)4); + Debug.Assert(result[4] == (byte)5); } { - string result = TestInterop.ListResult2(); - Console.WriteLine(result); - Debug.Assert(result == "hello!"); + string result = TestInterop.ListResult2(); + Debug.Assert(result == "hello!"); } { - List result = TestInterop.ListResult3(); - Debug.Assert(result.Count() == 2); - Console.WriteLine(result[0]); - Console.WriteLine(result[1]); - Debug.Assert(result[0] == "hello,"); - Debug.Assert(result[1] == "world!"); + List result = TestInterop.ListResult3(); + Debug.Assert(result.Count() == 2); + Debug.Assert(result[0] == "hello,"); + Debug.Assert(result[1] == "world!"); } string[] strings = { "x", "", "hello", "hello ⚑ world" }; @@ -91,9 +89,19 @@ public static void TestImports() Debug.Assert(bytes.SequenceEqual(TestInterop.ListRoundtrip(bytes))); } + { + var headers = new List<(string, byte[])>() { ("Content-Type", Encoding.UTF8.GetBytes("text/plain")), ("Content-Length", Encoding.UTF8.GetBytes("Not found".Count().ToString())) }; + var result = TestInterop.WasiHttpHeadersRoundtrip(headers); + for (var i = 0; i < result.Count(); i++) + { + Debug.Assert(result[i].Item1 == headers[i].Item1); + Debug.Assert(headers[i].Item2.SequenceEqual(result[i].Item2)); + } + } + { var (u, s) = TestInterop.ListMinmax8( - new byte[] { byte.MinValue,byte.MaxValue }, + new byte[] { byte.MinValue, byte.MaxValue }, new sbyte[] { sbyte.MinValue, sbyte.MaxValue } ); @@ -107,15 +115,11 @@ public static void TestImports() new short[] { short.MinValue, short.MaxValue } ); - Console.WriteLine(u[0]); - Console.WriteLine(u[1]); Debug.Assert(u.Length == 2, $"u.Length {u.Length}"); Debug.Assert(u[0] == ushort.MinValue, $"u[0] == {u[0]}"); Debug.Assert(u[1] == ushort.MaxValue, $"u[1] == {u[1]}"); Debug.Assert(s.Length == 2); - Console.WriteLine(s[0]); - Console.WriteLine(s[1]); Debug.Assert(s.Length == 2 && s[0] == short.MinValue && s[1] == short.MaxValue); } @@ -321,5 +325,10 @@ public static string StringRoundtrip(string a) { return a; } + + public static List<(string, byte[])> WasiHttpHeadersRoundtrip(List<(string, byte[])> a) + { + return a; + } } } diff --git a/tests/runtime/lists/world.wit b/tests/runtime/lists/world.wit index 3c9635339..c7ba3e5ba 100644 --- a/tests/runtime/lists/world.wit +++ b/tests/runtime/lists/world.wit @@ -26,6 +26,8 @@ interface test { list-roundtrip: func(a: list) -> list; string-roundtrip: func(a: string) -> string; + + wasi-http-headers-roundtrip: func(a: list>>) -> list>>; } world lists { From 9dd1262ef8c7cefa68b63a4ce3c62c393cb9e9e2 Mon Sep 17 00:00:00 2001 From: Timmy Silesmo Date: Wed, 19 Mar 2025 15:32:08 +0100 Subject: [PATCH 2/2] Fixes alignment --- crates/csharp/src/function.rs | 20 +++++++++++++++++--- crates/csharp/src/world_generator.rs | 7 ++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/csharp/src/function.rs b/crates/csharp/src/function.rs index 6ad827c83..d7370f9b5 100644 --- a/crates/csharp/src/function.rs +++ b/crates/csharp/src/function.rs @@ -882,6 +882,13 @@ impl Bindgen for FunctionBindgen<'_, '_> { ); let ret_area = self.locals.tmp("retArea"); + let array_size = if align > 1 { + // Add one additional element in case the starting address is not aligned + format!("{array_size} * {list}.Count + 1") + } else { + format!("{array_size} * {list}.Count") + }; + match realloc { None => { self.needs_cleanup = true; @@ -889,8 +896,8 @@ impl Bindgen for FunctionBindgen<'_, '_> { " void* {address}; if (({size} * {list}.Count) < 1024) {{ - var {ret_area} = stackalloc {element_type}[{array_size}*{list}.Count]; - {address} = (void*)((int){ret_area}); + var {ret_area} = stackalloc {element_type}[{array_size}]; + {address} = (void*)(((int){ret_area}) + ({align} - 1) & -{align}); }} else {{ @@ -1288,10 +1295,17 @@ impl Bindgen for FunctionBindgen<'_, '_> { // to align the allocation via the stackalloc command, unlike with a fixed array where the pointer will be aligned. // We get the final ptr to pass to the wasm runtime by shifting to the // correctly aligned pointer (sometimes it can be already aligned). + let array_size = if self.import_return_pointer_area_align > 1 { + // Add one additional element in case the starting address is not aligned + array_size + 1 + } else { + array_size + }; + uwrite!( self.src, " - var {ret_area} = stackalloc {element_type}[{array_size}+1]; + var {ret_area} = stackalloc {element_type}[{array_size}]; var {ptr} = ((int){ret_area}) + ({align} - 1) & -{align}; ", align = align.align_wasm32() diff --git a/crates/csharp/src/world_generator.rs b/crates/csharp/src/world_generator.rs index 1907d38c2..8e2d6832a 100644 --- a/crates/csharp/src/world_generator.rs +++ b/crates/csharp/src/world_generator.rs @@ -797,13 +797,10 @@ enum Stubs<'a> { // We cant use "StructLayout.Pack" as dotnet will use the minimum of the type and the "Pack" field, // so for byte it would always use 1 regardless of the "Pack". pub fn dotnet_aligned_array(array_size: usize, required_alignment: usize) -> (usize, String) { - let num_elements = if array_size % required_alignment > 0 { - array_size / required_alignment + 1 - } else { - array_size / required_alignment - }; + let num_elements = array_size.div_ceil(required_alignment); match required_alignment { 1 => (num_elements, "byte".to_owned()), + // Add one additional element in case the starting address is not aligned 2 => (num_elements, "ushort".to_owned()), 4 => (num_elements, "uint".to_owned()), 8 => (num_elements, "ulong".to_owned()),