Stream: wit-bindgen

Topic: ✔ cabi_realloc quation (c)

Gordon Smith (Jul 05 2023 at 08:52):

The generated "c" code for cabi_realloc looks like this:

__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;
  void *ret = realloc(ptr, new_size);
  if (!ret)
    abort();
  return ret;
}

My instinct was for something more like:

__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) {
    free(ptr);
    return 0;
  }
  void *ret = realloc(ptr, new_size);
  if (!ret)
    abort();
  return ret;
}

This would allow the host to cleanup any wasm memory it had previously allocated via calls to cabi_realloc?

Lann Martin (Jul 05 2023 at 13:57):

POSIX realloc doesn't free on size = 0; thats a non-portable glibc behavior. (Which isn't to say that cabi_realloc couldn't adopt that behavior, just probably why it doesn't.)

Lann Martin (Jul 05 2023 at 13:59):

IMO the glibc behavior is more confusing/surprising.

Alex Crichton (Jul 05 2023 at 14:05):

I think technically this would be ok, but when the canonical ABI wants to free something it will always do so through cabi_free as opposed to cabi_realloc, so I don't think it's necessary to make the change as well

Gordon Smith (Jul 05 2023 at 14:15):

Ahhh - so the issue is more that my wit generated stubs didn't create an export for cabi_free?

FWIW I am testing a simple wit file:

world test {
  export echo: func(msg: string) -> string
}

And I create several wasm files that implement it (in various languages).

I am now writing a host which wants to call that function so I:

1. call cabi_realloc and copy the string into the wasm memory
2. Call the echo function
3. Copy the result from the wasm memory into my host memory
4. Call cabi_post_echo to free the memory that the result was using
5. Call cabi_realloc to "free" the memory I allocated in step 1.

Had there been a cabi_free exported from the wasm I would / should have called that?

This is what my exports look like from the c++ guest wasm:

  (export "memory" (memory 0))
  (export "cabi_post_echo" (func 150))
  (export "cabi_realloc" (func 151))
  (export "add" (func 152))
  (export "add3" (func 153))
  (export "sub" (func 154))
  (export "echo" (func 155))

Alex Crichton (Jul 05 2023 at 14:24):

oh sorry I'm thiking about the past where cabi_free was a thing, which it no longer is. In any case though your stub's cabi_post_echo is the "free" function, and it's specialized for the echo export

Gordon Smith (Jul 05 2023 at 14:39):

Which is fine for the "returned" string - but its not going to clean up the memory I allocated in step 1. ?

Lann Martin (Jul 05 2023 at 14:51):

Memory allocated for params becomes owned by the guest function. e.g. if your guest is Rust it will be wrapped in an owned String.

Gordon Smith (Jul 05 2023 at 15:02):

@Lann Martin - is that convention documented anywhere? I had a good look around the wit-bindgen docs and https://github.com/WebAssembly/component-model but didn't spot anything?

Also if that is the case, should it be taken care of in the generated stub:

__attribute__((__export_name__("echo")))
int32_t __wasm_export_test_echo(int32_t arg, int32_t arg0) {
  test_string_t arg1 = (test_string_t) { (char*)(arg), (size_t)(arg0) };
  test_string_t ret;
  test_echo(&arg1, &ret);
  -------->free(arg)<-------
  int32_t ptr = (int32_t) &RET_AREA;
  *((int32_t*)(ptr + 4)) = (int32_t) (ret).len;
  *((int32_t*)(ptr + 0)) = (int32_t) (ret).ptr;
  return ptr;
}

Type of thing?

Lann Martin (Jul 05 2023 at 15:09):

I don't know if it is explicitly described anywhere.

Lann Martin (Jul 05 2023 at 15:11):

Generated C would not free parameters because the guest code may want to keep them around. It would be up to the user to free parameters.

Lann Martin (Jul 05 2023 at 15:14):

This sort of thing would typically be documented with the function declaration; I'm not sure how best to document generated C bindings like that.

Gordon Smith (Jul 05 2023 at 15:41):

Ok as long as I know what the official intention is I am happy enough to have:

world test {
  //  guests dispose all params as needed
  //  hosts call cabi_post_XXX to dispose "results" as needed
  export echo: func(msg: string) -> string
  ...
}

(and it solves my initial issue with cabi_realloc not freeing!)

Notification Bot (Jul 05 2023 at 15:41):

Gordon Smith has marked this topic as resolved.

Gordon Smith (Jul 05 2023 at 16:04):

@Lann Martin - can I assume that the opposite would be true for host functions that are imported into the wasm?
(If so then we would still need a cabi_free or equivalent)...

Notification Bot (Jul 05 2023 at 16:10):

Gordon Smith has marked this topic as unresolved.

Lann Martin (Jul 05 2023 at 16:33):

For calling an import I believe the caller is responsible for cleaning up both parameters and results. The difference with imports is that the caller is only touching its own memory, so it doesn't need to call a cabi_realloc.

Lann Martin (Jul 05 2023 at 16:34):

The callee (e.g. the host) may call cabi_realloc to allocate the result

Lann Martin (Jul 05 2023 at 16:34):

If the callee wants access to the parameters after the function returns it has to copy them out of the caller's memory.

Lann Martin (Jul 05 2023 at 16:35):

All that said, I am most familiar with older versions of the ABI so hopefully someone will confirm my answer there :smile:

Lann Martin (Jul 05 2023 at 16:36):

Getting this written down in a less-formal way (than the canonical ABI's Python definition) so that everyone can understand is a pretty high priority for many of us. @Till Schneidereit

Till Schneidereit (Jul 05 2023 at 16:39):

agreed! Relatedly, I was just in a meeting where we discussed how specifying some details of the ABI that Component tooling otherwise abstracts away, because we want to enable runtimes to support WASI Preview2+ without requiring the whole component model

Gordon Smith (Jul 05 2023 at 17:26):

Great info - I did spend too much time looking for this info (but did learn a lot on the way). tldr:

  //  Imports  ---
  //  guests dispose params as needed
  //  guests dispose "results" as needed
  import dbglog: func(msg: string)

  //  Exports ---
  //  guests dispose params as needed
  //  hosts call cabi_post_XXX to dispose "results" as needed
  export echo: func(msg: string) -> string

Notification Bot (Jul 05 2023 at 17:26):

Gordon Smith has marked this topic as resolved.

Scott Waye (Sep 13 2023 at 01:22):

Thanks, I saw a PR that fixed up the imports for threads (moving the ret aret to the stack), so just wondered. (https://github.com/bytecodealliance/wit-bindgen/pull/326)

Last updated: Nov 22 2024 at 17:03 UTC