Skip to content

Commit

Permalink
Fixes for cabi_realloc when new_size = 0 (#465)
Browse files Browse the repository at this point in the history
* gen-guest-c: Fix cabi_realloc behavior when new_size is zero

Match the behavior of the Rust guest cabi_realloc by returning `align`
if new_size == 0.

* guest-rust: Add an assert to cabi_realloc

The code generator is expected not to emit calls that might have
`old_size == 0 && new_size == 0`, but if that condition is not met
the resulting call to `alloc::realloc` is documented as having undefined
behavior.

* Add test exercising cabi_realloc new_size = 0
  • Loading branch information
lann authored Feb 6, 2023
1 parent 26bc5dd commit 67c7d74
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 4 deletions.
9 changes: 5 additions & 4 deletions crates/gen-guest-c/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,14 +418,15 @@ impl C {
// Note that these intrinsics are declared as `weak` so they can be
// overridden from some other symbol.
self.src.c_fns(
"
__attribute__((weak, export_name(\"cabi_realloc\")))
void *cabi_realloc(void *ptr, size_t orig_size, size_t org_align, size_t new_size) {
r#"
__attribute__((weak, export_name("cabi_realloc")))
void *cabi_realloc(void *ptr, size_t old_size, size_t align, size_t new_size) {
if (new_size == 0) return (void*) align;

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith Mar 22, 2024

This will cause memory leaks in c++?

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Mar 22, 2024

Member

It does not in practice as this function is not used to free memory, only allocate or resize existing memory.

void *ret = realloc(ptr, new_size);
if (!ret) abort();
return ret;
}
",
"#,
);
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/guest-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub mod rt {
layout = Layout::from_size_align_unchecked(new_len, align);
alloc::alloc(layout)
} else {
debug_assert_ne!(new_len, 0, "non-zero old_len requires non-zero new_len!");
layout = Layout::from_size_align_unchecked(old_len, align);
alloc::realloc(old_ptr, layout, new_len)
};
Expand Down
1 change: 1 addition & 0 deletions tests/runtime/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ fn run() -> Result<()> {

fn run_test(exports: Strings, store: &mut Store<crate::Wasi<MyImports>>) -> Result<()> {
exports.call_test_imports(&mut *store)?;
assert_eq!(exports.call_return_empty(&mut *store)?, "");
assert_eq!(exports.call_roundtrip(&mut *store, "str")?, "str");
assert_eq!(
exports.call_roundtrip(&mut *store, "🚀🚀🚀 𠈄𓀀")?,
Expand Down
4 changes: 4 additions & 0 deletions tests/runtime/strings/wasm_utf16.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ void strings_test_imports() {
strings_string_free(&str2);
}

void strings_return_empty(strings_string_t *ret) {
strings_string_dup(ret, u""); // Exercise cabi_realloc new_size = 0
}

void strings_roundtrip(strings_string_t *str, strings_string_t *ret) {
assert(str->len > 0);
ret->len = str->len;
Expand Down
1 change: 1 addition & 0 deletions tests/runtime/strings/world.wit
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ default world strings {
import imports: self.imports

export test-imports: func()
export return-empty: func() -> string
export roundtrip: func(s: string) -> string
}

0 comments on commit 67c7d74

Please sign in to comment.