cabi: add Canonical ABI realloc (cabi_realloc) #3

ydnar · 2023-10-17T00:08:39Z

This PR adds a function exported from the Wasm module named cabi_realloc used by the Component Model to allocate memory in the guest for host → guest function calls.

Fixes #2.

ydnar · 2023-10-17T23:13:31Z

This is WIP, but probably safe to merge

Mossaka

Thanks! Asked a few questions for myself to understand the implementation.

Mossaka · 2023-10-18T19:22:03Z

abi/realloc.go

+//go:wasmexport cabi_realloc
+func realloc(ptr unsafe.Pointer, size, align, newsize uintptr) unsafe.Pointer {
+	if newsize <= size {
+		return unsafe.Add(ptr, offset(uintptr(ptr), align))


I understand that when newsize <= size, it returns the original pointer with additional offset. But is this really working without reallocating and copying the old memory block to a new memory block?

That’s an interesting example. If you realloc with a different alignment than the original allocation, presumably you’re storing a different type, in which case the copy shouldn’t happen. What does the spec say here?

Is there a spec for this realloc above and beyond what the C specification has? (If so, should we link to it?).

Good idea.

https://github.com/WebAssembly/component-model/blob/main/design/mvp/Explainer.md#canonical-abi

The (realloc ...) option specifies a core function that is validated to have the following core function type:

(func (param $originalPtr i32) (param $originalSize i32) (param $alignment i32) (param $newSize i32) (result i32))

The Canonical ABI will use realloc both to allocate (passing 0 for the first two parameters) and reallocate. If the Canonical ABI needs realloc, validation requires this option to be present (there is no default).

Mossaka · 2023-10-18T19:31:43Z

abi/realloc.go

+	}
+	newptr := alloc(newsize, align)
+	if size > 0 {
+		copy(unsafe.Slice((*byte)(newptr), newsize), unsafe.Slice((*byte)(ptr), size))


Why not deallocating the old memory block after copying?

While standard Go doesn't have a the equivalent of free(), since we're deferring to Go's runtime in both cases we should allow them to be garbage collected if possible. Unfortunately, there's no easy way to null out all references to the old pointer -- we just have to trust the client doesn't hold on to it.

@Mossaka does the spec confirm that once the memory is allocated with realloc and the pointer is passed to a guest func, the memory is owned by the guest?

@dgryski will using unsafe.Pointer in go:wasmexport function signatures be sufficient to signal to the compiler that the value passed in is a pointer that can be GCed if not used?

For garbage collection, if there are no references to the pointer in the guest address space, then it will be collected. As with other uses of realloc, we'll need to be careful to keep track of the pointer in the guest if we don't want it accidentally reclaimed.

i didn't find in the CM spec where it says the memory allocated with realloc will be owned by the guest...

Mossaka · 2023-10-18T19:33:04Z

abi/realloc.go

+//
+//go:export cabi_realloc
+//go:wasmexport cabi_realloc
+func realloc(ptr unsafe.Pointer, size, align, newsize uintptr) unsafe.Pointer {


What happens if allocation failed due to out of memory issue?

I assume the runtime will panic? What do you think is best way to handle this?

Both Go and TinyGo panic with out of memory if they can't allocate a block.

Is a runtime panic acceptable to the Component Model if it can't allocate memory?

@ydnar There is no way to capture that allocation failure and retry or notify the caller. It's an immediate, uncatchable, runtime panic(). We don't have a choice in this case.

A runtime failure seems reasonable to me.

ydnar · 2023-10-18T21:25:04Z

abi/realloc.go

+		s := make([]uint64, min(size/align, 1))
+		return unsafe.Pointer(unsafe.SliceData(s))
+	default:
+		s := make([][16]uint8, min(size/align, 1))


Thinking this should be a struct or complex128.

Mossaka · 2023-10-18T23:19:28Z

abi/realloc.go

+//go:export cabi_realloc
+//go:wasmexport cabi_realloc
+func realloc(ptr unsafe.Pointer, size, align, newsize uintptr) unsafe.Pointer {
+	if newsize <= size {


Don't you want to check if ptr is not nil, or uintptr(ptr) != 0?

If newsize is 0, and ptr is 0, then it will always return 0. This is somewhat based on the Rust implementation.

For #2

…inters

Mossaka

LGTM!

ydnar force-pushed the ydnar/realloc branch from c5749d6 to f964c78 Compare October 17, 2023 22:32

ydnar requested review from dgryski and Mossaka October 17, 2023 23:13

ydnar marked this pull request as ready for review October 17, 2023 23:13

ydnar self-assigned this Oct 17, 2023

Mossaka reviewed Oct 18, 2023

View reviewed changes

ydnar commented Oct 18, 2023

View reviewed changes

Mossaka reviewed Oct 18, 2023

View reviewed changes

ydnar changed the title ~~abi: add Realloc for Component Model ABI~~ abi: add Canonical ABI realloc (cabi_realloc) Oct 19, 2023

ydnar added 15 commits October 19, 2023 16:18

abi: basic implementation of cabi_realloc

bf641ff

For #2

abi: use unsafe.Pointer to appease GC

0e2de9e

abi: add unsafePointer helper

b551304

abi: use unsafe.Add to add offset into pointer for aligning unsafe.Po…

12d4088

…inters

abi: improve realloc tests

adc9c1a

abi: improve tests

1ffad87

abi: tests, dance with the GC

4286d29

abi: additional tests

63c4244

abi: use TinyGo magic comment for go:export

58940e5

abi: un-export realloc

9a60500

abi: streamline realloc

4075cf1

abi: add docs to alloc

d1298de

abi: unexport more

186952b

abi: memory -> realloc

e5d4ed8

abi, wit: align without division

696040f

ydnar force-pushed the ydnar/realloc branch from 7c7e2c4 to 696040f Compare October 19, 2023 23:18

ydnar added 3 commits October 19, 2023 16:23

abi: add README

a695183

abi: update README

4275146

abi: add test for alignment

19a587d

Mossaka approved these changes Oct 20, 2023

View reviewed changes

ydnar added 2 commits October 21, 2023 15:25

.github/workflows/go: pin to wasmtime v13

dd7a5df

abi → cabi

5f9165e

ydnar changed the title ~~abi: add Canonical ABI realloc (cabi_realloc)~~ cabi: add Canonical ABI realloc (cabi_realloc) Oct 22, 2023

ydnar merged commit cedd8b0 into main Oct 23, 2023
1 check passed

ydnar deleted the ydnar/realloc branch October 23, 2023 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cabi: add Canonical ABI realloc (cabi_realloc) #3

cabi: add Canonical ABI realloc (cabi_realloc) #3

ydnar commented Oct 17, 2023 •

edited

Loading

ydnar commented Oct 17, 2023

Mossaka left a comment

Mossaka Oct 18, 2023

ydnar Oct 18, 2023

dgryski Oct 18, 2023

ydnar Oct 18, 2023

Mossaka Oct 18, 2023

ydnar Oct 18, 2023

dgryski Oct 18, 2023

ydnar Oct 18, 2023

dgryski Oct 18, 2023

Mossaka Oct 18, 2023

Mossaka Oct 18, 2023

ydnar Oct 18, 2023

dgryski Oct 18, 2023

ydnar Oct 18, 2023

dgryski Oct 18, 2023

Mossaka Oct 18, 2023

ydnar Oct 18, 2023

Mossaka Oct 18, 2023

ydnar Oct 18, 2023

Mossaka left a comment

cabi: add Canonical ABI realloc (cabi_realloc) #3

cabi: add Canonical ABI realloc (cabi_realloc) #3

Conversation

ydnar commented Oct 17, 2023 • edited Loading

ydnar commented Oct 17, 2023

Mossaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mossaka left a comment

Choose a reason for hiding this comment

ydnar commented Oct 17, 2023 •

edited

Loading