Skip to content

C# Fixes wasi http header bug and adds a test for it #1215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

C# Fixes wasi http header bug and adds a test for it #1215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e830f5
Fixes wasi http header bug and adds a test for it
Mar 15, 2025
9dd1262
Fixes alignment
Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 37 additions & 23 deletions crates/csharp/src/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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]));
}
}

Expand All @@ -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]))
Expand All @@ -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);
}
Expand Down Expand Up @@ -754,7 +754,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
"
);
}
results.push(format!("(nint){ptr}"));
results.push(format!("(int){ptr}"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

results.push(format!("({list}).Length"));
}
Direction::Export => {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -882,20 +882,27 @@ 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;
uwrite!(self.src,
"
void* {address};
if (({size} * {list}.Count) < 1024) {{
var {ret_area} = stackalloc {element_type}[({array_size}*{list}.Count)+1];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

var {ret_area} = stackalloc {element_type}[{array_size}+1];
as well.

var {ret_area} = stackalloc {element_type}[{array_size}];
{address} = (void*)(((int){ret_area}) + ({align} - 1) & -{align});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

@pavelsavara pavelsavara Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor void* AlignStackPtr(void* stackAddress, uint alignment) helper and use it everywhere.

}}
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}));
}}
"
Expand All @@ -905,8 +912,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});
"
);
}
Expand Down Expand Up @@ -1046,7 +1053,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::<Vec<_>>().join(", ");
self.src.insert_str(0, &format!("fixed (void* {fixed})
{{
Expand Down Expand Up @@ -1077,7 +1084,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
}
}

if self.fixed_statments.len() > 0 {
if !self.fixed_statments.is_empty() {
uwriteln!(self.src, "}}");
}
}
Expand Down Expand Up @@ -1201,7 +1208,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
}
}
}
results.push(format!("{handle}"));
results.push(handle);
}

Instruction::HandleLift {
Expand Down Expand Up @@ -1253,7 +1260,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 { .. }
Expand Down Expand Up @@ -1288,15 +1295,22 @@ 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()
);
format!("{ptr}")
ptr
}
Direction::Export => {
// exports need their return area to be live until the post-return call.
Expand All @@ -1319,7 +1333,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
);
self.interface_gen.csharp_gen.needs_export_return_area = true;

format!("{ptr}")
ptr
}
}
}
Expand Down Expand Up @@ -1395,8 +1409,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
Expand Down
14 changes: 6 additions & 8 deletions crates/csharp/src/interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,12 @@ impl InterfaceGenerator<'_> {
fn gen_import_src(
&mut self,
func: &Function,
results: &Vec<TypeId>,
results: &[TypeId],
parameter_type: ParameterType,
) -> (String, String) {
let mut bindgen = FunctionBindgen::new(
self,
&func.item_name(),
func.item_name(),
&func.kind,
func.params
.iter()
Expand All @@ -317,7 +317,7 @@ impl InterfaceGenerator<'_> {
}
})
.collect(),
results.clone(),
results.to_vec(),
parameter_type,
);

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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!(
Expand Down
49 changes: 21 additions & 28 deletions crates/csharp/src/world_generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");

Expand All @@ -535,28 +535,27 @@ impl WorldGenerator for CSharp {
.world_fragments
.iter()
.flat_map(|f| &f.interop_usings)
.into_iter()
.collect::<HashSet<&String>>() // de-dup across world_fragments
.iter()
.map(|s| "using ".to_owned() + s + ";")
.collect::<Vec<String>>()
.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);
}
src.push_str("}\n");
src.push_str("}\n");
}

src.push_str("\n");
src.push('\n');

src.push_str("}\n");

Expand Down Expand Up @@ -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)
}
};
Expand Down Expand Up @@ -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() {{
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -731,7 +726,7 @@ impl WorldGenerator for CSharp {
.collect::<Vec<_>>()
.join("\n");

if body.len() > 0 {
if !body.is_empty() {
let body = format!(
"{header}
{0}
Expand Down Expand Up @@ -802,11 +797,13 @@ 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 = array_size.div_ceil(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()),
// 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()),
_ => todo!("unsupported return_area_align {}", required_alignment),
}
}
Expand Down Expand Up @@ -885,11 +882,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
Expand Down
Loading