Skip to content

Commit

Permalink
Fix the C lowering for RecordLift to be compatible with C++. (#902)
Browse files Browse the repository at this point in the history
* Fix the C lowering for `RecordLift` to be compatible with C++.

C++ prohibits non-constant struct literal initializers from being implicitly
converted to narrower types, so insert explicit casts. This allows the
generated code to be compiled as both C and C++.

Specifically, this fixes the following error in C++:
```
test.c:40:5: error: non-constant-expression cannot be narrowed from type 'int32_t' (aka 'int') to 'enum_t' (aka 'unsigned char') in initializer list [-Wc++11-narrowing]
```

While debugging this, I also tidied up some redundant parens in `load_ext`.

* Add `-Wc++compat` to the test compiler flags.

* Fix more C++ compilation errors and compile the tests in C++ mode too.

Compile the C tests in C++ too, and fix several compilation errors that
this uncovered.

* Minor whitespace tidying.
  • Loading branch information
sunfishcode authored Mar 20, 2024
1 parent 53e07c7 commit f4851bb
Show file tree
Hide file tree
Showing 7 changed files with 150 additions and 106 deletions.
137 changes: 86 additions & 51 deletions crates/c/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ struct C {
return_pointer_area_align: usize,
names: Ns,
needs_string: bool,
needs_union_int32_float: bool,
needs_union_float_int32: bool,
needs_union_int64_double: bool,
needs_union_double_int64: bool,
prim_names: HashSet<String>,
world: String,
sizes: SizeAlign,
Expand Down Expand Up @@ -331,7 +335,7 @@ impl WorldGenerator for C {
self.src.h_helpers,
"
// Transfers ownership of `s` into the string `ret`
void {snake}_string_set({snake}_string_t *ret, {c_string_ty} *s);
void {snake}_string_set({snake}_string_t *ret, const {c_string_ty} *s);
// Creates a copy of the input nul-terminate string `s` and
// stores it into the component model string `ret`.
Expand All @@ -345,14 +349,14 @@ impl WorldGenerator for C {
uwrite!(
self.src.c_helpers,
"
void {snake}_string_set({snake}_string_t *ret, {c_string_ty} *s) {{
void {snake}_string_set({snake}_string_t *ret, const {c_string_ty} *s) {{
ret->ptr = ({ty}*) s;
ret->len = {strlen};
}}
void {snake}_string_dup({snake}_string_t *ret, const {c_string_ty} *s) {{
ret->len = {strlen};
ret->ptr = cabi_realloc(NULL, 0, {size}, ret->len * {size});
ret->ptr = ({ty}*) cabi_realloc(NULL, 0, {size}, ret->len * {size});
memcpy(ret->ptr, s, ret->len * {size});
}}
Expand All @@ -366,6 +370,30 @@ impl WorldGenerator for C {
",
);
}
if self.needs_union_int32_float {
uwriteln!(
self.src.c_helpers,
"\nunion int32_float {{ int32_t a; float b; }};"
);
}
if self.needs_union_float_int32 {
uwriteln!(
self.src.c_helpers,
"\nunion float_int32 {{ float a; int32_t b; }};"
);
}
if self.needs_union_int64_double {
uwriteln!(
self.src.c_helpers,
"\nunion int64_double {{ int64_t a; double b; }};"
);
}
if self.needs_union_double_int64 {
uwriteln!(
self.src.c_helpers,
"\nunion double_int64 {{ double a; int64_t b; }};"
);
}
let version = env!("CARGO_PKG_VERSION");
let mut h_str = wit_bindgen_core::Source::default();

Expand Down Expand Up @@ -570,6 +598,51 @@ impl C {
self.type_names.retain(|k, _| live_import_types.contains(k));
self.resources.retain(|k, _| live_import_types.contains(k));
}

fn perform_cast(&mut self, op: &str, cast: &Bitcast) -> String {
match cast {
Bitcast::I32ToF32 | Bitcast::I64ToF32 => {
self.needs_union_int32_float = true;
format!("((union int32_float){{ (int32_t) {} }}).b", op)
}
Bitcast::F32ToI32 | Bitcast::F32ToI64 => {
self.needs_union_float_int32 = true;
format!("((union float_int32){{ {} }}).b", op)
}
Bitcast::I64ToF64 => {
self.needs_union_int64_double = true;
format!("((union int64_double){{ (int64_t) {} }}).b", op)
}
Bitcast::F64ToI64 => {
self.needs_union_double_int64 = true;
format!("((union double_int64){{ {} }}).b", op)
}
Bitcast::I32ToI64 | Bitcast::LToI64 | Bitcast::PToP64 => {
format!("(int64_t) {}", op)
}
Bitcast::I64ToI32 | Bitcast::I64ToL => {
format!("(int32_t) {}", op)
}
// P64 is currently represented as int64_t, so no conversion is needed.
Bitcast::I64ToP64 | Bitcast::P64ToI64 => {
format!("{}", op)
}
Bitcast::P64ToP | Bitcast::I32ToP | Bitcast::LToP => {
format!("(uint8_t *) {}", op)
}

// Cast to uintptr_t to avoid implicit pointer-to-int conversions.
Bitcast::PToI32 | Bitcast::PToL => format!("(uintptr_t) {}", op),

Bitcast::I32ToL | Bitcast::LToI32 | Bitcast::None => op.to_string(),

Bitcast::Sequence(sequence) => {
let [first, second] = &**sequence;
let inner = self.perform_cast(op, first);
self.perform_cast(&inner, second)
}
}
}
}

pub fn imported_types_used_by_exported_interfaces(
Expand Down Expand Up @@ -2068,7 +2141,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> {
fn load_ext(&mut self, ty: &str, offset: i32, operands: &[String], results: &mut Vec<String>) {
self.load(ty, offset, operands, results);
let result = results.pop().unwrap();
results.push(format!("(int32_t) ({})", result));
results.push(format!("(int32_t) {}", result));
}

fn store(&mut self, ty: &str, offset: i32, operands: &[String]) {
Expand Down Expand Up @@ -2208,7 +2281,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {

Instruction::Bitcasts { casts } => {
for (cast, op) in casts.iter().zip(operands) {
let op = perform_cast(op, cast);
let op = self.gen.gen.perform_cast(op, cast);
results.push(op);
}
}
Expand All @@ -2223,11 +2296,12 @@ impl Bindgen for FunctionBindgen<'_, '_> {
results.push(format!("({}).{}", op, to_c_ident(&f.name)));
}
}
Instruction::RecordLift { ty, .. } => {
Instruction::RecordLift { ty, record, .. } => {
let name = self.gen.gen.type_name(&Type::Id(*ty));
let mut result = format!("({}) {{\n", name);
for op in operands {
uwriteln!(result, "{},", op);
for (field, op) in record.fields.iter().zip(operands.iter()) {
let field_ty = self.gen.gen.type_name(&field.ty);
uwriteln!(result, "({}) {},", field_ty, op);
}
result.push_str("}");
results.push(result);
Expand All @@ -2239,11 +2313,12 @@ impl Bindgen for FunctionBindgen<'_, '_> {
results.push(format!("({}).f{}", op, i));
}
}
Instruction::TupleLift { ty, .. } => {
Instruction::TupleLift { ty, tuple, .. } => {
let name = self.gen.gen.type_name(&Type::Id(*ty));
let mut result = format!("({}) {{\n", name);
for op in operands {
uwriteln!(result, "{},", op);
for (ty, op) in tuple.types.iter().zip(operands.iter()) {
let ty = self.gen.gen.type_name(&ty);
uwriteln!(result, "({}) {},", ty, op);
}
result.push_str("}");
results.push(result);
Expand Down Expand Up @@ -2943,46 +3018,6 @@ impl Bindgen for FunctionBindgen<'_, '_> {
}
}

fn perform_cast(op: &str, cast: &Bitcast) -> String {
match cast {
Bitcast::I32ToF32 | Bitcast::I64ToF32 => {
format!("((union {{ int32_t a; float b; }}){{ {} }}).b", op)
}
Bitcast::F32ToI32 | Bitcast::F32ToI64 => {
format!("((union {{ float a; int32_t b; }}){{ {} }}).b", op)
}
Bitcast::I64ToF64 => {
format!("((union {{ int64_t a; double b; }}){{ {} }}).b", op)
}
Bitcast::F64ToI64 => {
format!("((union {{ double a; int64_t b; }}){{ {} }}).b", op)
}
Bitcast::I32ToI64 | Bitcast::LToI64 | Bitcast::PToP64 => {
format!("(int64_t) {}", op)
}
Bitcast::I64ToI32 | Bitcast::I64ToL => {
format!("(int32_t) {}", op)
}
// P64 is currently represented as int64_t, so no conversion is needed.
Bitcast::I64ToP64 | Bitcast::P64ToI64 => {
format!("{}", op)
}
Bitcast::P64ToP | Bitcast::I32ToP | Bitcast::LToP => {
format!("(uint8_t *) {}", op)
}

// Cast to uintptr_t to avoid implicit pointer-to-int conversions.
Bitcast::PToI32 | Bitcast::PToL => format!("(uintptr_t) {}", op),

Bitcast::I32ToL | Bitcast::LToI32 | Bitcast::None => op.to_string(),

Bitcast::Sequence(sequence) => {
let [first, second] = &**sequence;
perform_cast(&perform_cast(op, first), second)
}
}
}

#[derive(Default, Clone, Copy)]
enum SourceType {
#[default]
Expand Down
1 change: 1 addition & 0 deletions crates/c/tests/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ fn verify(dir: &Path, name: &str) {
dir.to_str().unwrap(),
"-Wall",
"-Wextra",
"-Wc++-compat",
"-Werror",
"-Wno-unused-parameter",
"-c",
Expand Down
4 changes: 2 additions & 2 deletions tests/runtime/flavorful/wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,11 @@ void exports_test_flavorful_test_list_typedefs(exports_test_flavorful_test_list_
assert(memcmp(c->ptr[0].ptr, "typedef2", c->ptr[0].len) == 0);
exports_test_flavorful_test_list_typedef3_free(c);

ret0->ptr = malloc(8);
ret0->ptr = (uint8_t *) malloc(8);
ret0->len = 8;
memcpy(ret0->ptr, "typedef3", 8);

ret1->ptr = malloc(sizeof(flavorful_string_t));
ret1->ptr = (flavorful_string_t *) malloc(sizeof(flavorful_string_t));
ret1->len = 1;
flavorful_string_dup(&ret1->ptr[0], "typedef4");
}
Expand Down
4 changes: 2 additions & 2 deletions tests/runtime/lists/wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ void exports_test_lists_test_list_param4(lists_list_list_string_t *a) {
}

void exports_test_lists_test_list_result(lists_list_u8_t *ret0) {
ret0->ptr = malloc(5);
ret0->ptr = (uint8_t *) malloc(5);
ret0->len = 5;
ret0->ptr[0] = 1;
ret0->ptr[1] = 2;
Expand All @@ -322,7 +322,7 @@ void exports_test_lists_test_list_result2(lists_string_t *ret0) {

void exports_test_lists_test_list_result3(lists_list_string_t *ret0) {
ret0->len = 2;
ret0->ptr = malloc(2 * sizeof(lists_string_t));
ret0->ptr = (lists_string_t *) malloc(2 * sizeof(lists_string_t));

lists_string_dup(&ret0->ptr[0], "hello,");
lists_string_dup(&ret0->ptr[1], "world!");
Expand Down
105 changes: 56 additions & 49 deletions tests/runtime/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,57 +207,64 @@ fn tests(name: &str, dir_name: &str) -> Result<Vec<PathBuf>> {
let sdk = PathBuf::from(std::env::var_os("WASI_SDK_PATH").expect(
"point the `WASI_SDK_PATH` environment variable to the path of your wasi-sdk",
));
let mut cmd = Command::new(sdk.join("bin/clang"));
let out_wasm = out_dir.join(format!(
"c-{}.wasm",
path.file_stem().and_then(|s| s.to_str()).unwrap()
));
cmd.arg("--sysroot").arg(sdk.join("share/wasi-sysroot"));
cmd.arg(path)
.arg(out_dir.join(format!("{snake}.c")))
.arg(out_dir.join(format!("{snake}_component_type.o")))
.arg("-I")
.arg(&out_dir)
.arg("-Wall")
.arg("-Wextra")
.arg("-Werror")
.arg("-Wno-unused-parameter")
.arg("-mexec-model=reactor")
.arg("-g")
.arg("-o")
.arg(&out_wasm);
println!("{:?}", cmd);
let output = match cmd.output() {
Ok(output) => output,
Err(e) => panic!("failed to spawn compiler: {}", e),
};

if !output.status.success() {
println!("status: {}", output.status);
println!("stdout: ------------------------------------------");
println!("{}", String::from_utf8_lossy(&output.stdout));
println!("stderr: ------------------------------------------");
println!("{}", String::from_utf8_lossy(&output.stderr));
panic!("failed to compile");
}

// Translate the canonical ABI module into a component.
let module = fs::read(&out_wasm).expect("failed to read wasm file");
let component = ComponentEncoder::default()
.module(module.as_slice())
.expect("pull custom sections from module")
.validate(true)
.adapter("wasi_snapshot_preview1", &wasi_adapter)
.expect("adapter failed to get loaded")
.encode()
.expect(&format!(
"module {:?} can be translated to a component",
out_wasm
// Test both C mode and C++ mode.
for compiler in ["bin/clang", "bin/clang++"] {
let mut cmd = Command::new(sdk.join(compiler));
let out_wasm = out_dir.join(format!(
"c-{}.wasm",
path.file_stem().and_then(|s| s.to_str()).unwrap()
));
let component_path = out_wasm.with_extension("component.wasm");
fs::write(&component_path, component).expect("write component to disk");
cmd.arg("--sysroot").arg(sdk.join("share/wasi-sysroot"));
cmd.arg(path)
.arg(out_dir.join(format!("{snake}.c")))
.arg(out_dir.join(format!("{snake}_component_type.o")))
.arg("-I")
.arg(&out_dir)
.arg("-Wall")
.arg("-Wextra")
.arg("-Werror")
.arg("-Wno-unused-parameter")
.arg("-mexec-model=reactor")
.arg("-g")
.arg("-o")
.arg(&out_wasm);
// Disable the warning about compiling a `.c` file in C++ mode.
if compiler.ends_with("++") {
cmd.arg("-Wno-deprecated");
}
println!("{:?}", cmd);
let output = match cmd.output() {
Ok(output) => output,
Err(e) => panic!("failed to spawn compiler: {}", e),
};

if !output.status.success() {
println!("status: {}", output.status);
println!("stdout: ------------------------------------------");
println!("{}", String::from_utf8_lossy(&output.stdout));
println!("stderr: ------------------------------------------");
println!("{}", String::from_utf8_lossy(&output.stderr));
panic!("failed to compile");
}

result.push(component_path);
// Translate the canonical ABI module into a component.
let module = fs::read(&out_wasm).expect("failed to read wasm file");
let component = ComponentEncoder::default()
.module(module.as_slice())
.expect("pull custom sections from module")
.validate(true)
.adapter("wasi_snapshot_preview1", &wasi_adapter)
.expect("adapter failed to get loaded")
.encode()
.expect(&format!(
"module {:?} can be translated to a component",
out_wasm
));
let component_path = out_wasm.with_extension("component.wasm");
fs::write(&component_path, component).expect("write component to disk");

result.push(component_path);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions tests/runtime/resource_import_and_export/wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ resource_import_and_export_toplevel_export(resource_import_and_export_own_thing_
exports_test_resource_import_and_export_test_own_thing_t
exports_test_resource_import_and_export_test_constructor_thing(uint32_t v) {
exports_test_resource_import_and_export_test_thing_t *val =
(exports_test_resource_import_and_export_test_thing_t *)
malloc(sizeof(exports_test_resource_import_and_export_test_thing_t));
assert(val != NULL);
val->thing = test_resource_import_and_export_test_constructor_thing(v + 1);
Expand Down
4 changes: 2 additions & 2 deletions tests/runtime/strings/wasm_utf16.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

char16_t STR_BUFFER[500];

void assert_str(strings_string_t* str, char16_t* expected) {
void assert_str(strings_string_t* str, const char16_t* expected) {
size_t expected_len = 0;
while (expected[expected_len])
expected_len++;
Expand All @@ -32,7 +32,7 @@ void strings_return_empty(strings_string_t *ret) {
void strings_roundtrip(strings_string_t *str, strings_string_t *ret) {
assert(str->len > 0);
ret->len = str->len;
ret->ptr = malloc(ret->len * 2);
ret->ptr = (uint16_t *) malloc(ret->len * 2);
memcpy(ret->ptr, str->ptr, 2 * ret->len);
strings_string_free(str);
}

0 comments on commit f4851bb

Please sign in to comment.