C# Fixes wasi http header bug and adds a test for it#1215
C# Fixes wasi http header bug and adds a test for it#1215silesmo wants to merge 8 commits intobytecodealliance:mainfrom
Conversation
jsturtevant
left a comment
jsturtevant
left a comment
There was a problem hiding this comment.
Is the fix here around the alignment? How was the issue presenting?
| " | ||
| void* {address}; | ||
| if (({size} * {list}.Count) < 1024) {{ | ||
| var {ret_area} = stackalloc {element_type}[({array_size}*{list}.Count)+1]; |
There was a problem hiding this comment.
looks like this moved the logic to add the additional 1 to the dotnet_aligned_array function, we should update
wit-bindgen/crates/csharp/src/function.rs
Line 1294 in 6541e44
| 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}); |
There was a problem hiding this comment.
I believe this line is still required or in some cases this won't align properly since stackalloc won't align on the correct memory boundary (dotnet/csharplang#1799). Unless something else is subtly addressing this?
There was a problem hiding this comment.
Refactor void* AlignStackPtr(void* stackAddress, uint alignment) helper and use it everywhere.
| ); | ||
| } | ||
| results.push(format!("(nint){ptr}")); | ||
| results.push(format!("(int){ptr}")); |
There was a problem hiding this comment.
Because it generates
BitConverter.TryWriteBytes(new Span<byte>((void*)(basePtr + 8), 4), (nint)listPtr);And that uses TryWriteBytes(Span<Byte>, UInt64) which is wrong. There is no TryWriteBytes overload for nint.
And then it returns false but we ignore it.
Let's stop using TryWriteBytes.
Instead it could be just
new Span<nint>((void*)(basePtr + 8), 1)[0] = listPtr;There was a problem hiding this comment.
This is what was causing the issue @jsturtevant
| @@ -26,6 +26,8 @@ interface test { | |||
| list-roundtrip: func(a: list<u8>) -> list<u8>; | |||
|
|
|||
| string-roundtrip: func(a: string) -> string; | |||
|
|
|||
| wasi-http-headers-roundtrip: func(a: list<tuple<string, list<u8>>>) -> list<tuple<string, list<u8>>>; | |||
There was a problem hiding this comment.
will need to add this to https://github.com/bytecodealliance/wit-bindgen/tree/main/tests/runtime-new/lists since this test has moved to the new framework #1221
|
I ran into this issue as well. Are there any plans to land this fix or similar? Thanks 🙏 |
# Conflicts: # crates/csharp/src/function.rs # crates/csharp/src/interface.rs # crates/csharp/src/world_generator.rs # tests/runtime/lists.rs # tests/runtime/lists/test.wit # tests/runtime/lists/wasm.cs
4 participants
Fixes a bug where headers returned from a C# component wouldn't be properly set. Adds a test for it and some cleanup.