Skip to content
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.

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

How can I have []byte array in a struct that is passed as my Context #44

Closed
fproulx-dfuse opened this issue Jun 13, 2019 · 11 comments
Closed
Assignees
Labels
❓ question Further information is requested
Projects

Comments

@fproulx-dfuse
Copy link

Summary

I need to pass some byte[] array in addition to a few other values as my Context data. The unsafe Pointer thing with CGO seems to limit passing pointers with things other that value types.

Do you know of a workaround to allow passing more complex context object around.

@fproulx-dfuse fproulx-dfuse added the ❓ question Further information is requested label Jun 13, 2019
@Hywan Hywan self-assigned this Jun 14, 2019
@Hywan
Copy link
Contributor

Hywan commented Jun 14, 2019

Unfortunately, cgo has some runtime constraints about pointers passed to C. Everything is explained here https://tip.golang.org/cmd/cgo/#hdr-Passing_pointers.

If you share your exact usecase, maybe I can help?

@attente
Copy link

attente commented Aug 21, 2019

Hi, I think I'm running into a similar issue as the reporter. At a high level, I'm trying to write a wrapper that will use go-ext-wasm to wrap a wasm module generated in rust via wasm-bindgen. Modules produced with wasm-bindgen require some external implementation of a heap, so that needs to be passed around as context data to the import implementations, but can't be done because of the pointer passing problem.

@attente
Copy link

attente commented Aug 21, 2019

To add to the above, wasm-bindgen module imports might also need to call back into the instance itself in order to allocate slices of the module memory, for example. So the context data would also need to include a reference to the module instance itself, so that the implemented imports can call back into the instance.

@Hywan
Copy link
Contributor

Hywan commented Aug 28, 2019

Can you give an example so that I could replicate your issue please?

@attente
Copy link

attente commented Sep 5, 2019

I think it's as simple as doing:

instance.SetContextData(unsafe.Pointer(&instance))

which results in:

panic: runtime error: cgo argument has Go pointer to Go pointer

If there isn't a way to include the original instance in the context data somewhere (whether directly or as part of a larger structure), there isn't a way to call its exports from within one of its imports.

@Hywan
Copy link
Contributor

Hywan commented Sep 6, 2019

there isn't a way to call its exports from within one of its imports.

Sorry, I don't understand.

@attente
Copy link

attente commented Sep 6, 2019

For example, wasm-bindgen expects an import with the following signature:

func __wbindgen_json_serialize(context unsafe.Pointer, memStrDst, heapObjSrc int32) {
    // ...
}

The implementation of this function is expected to allocate memory for the JSON serialization of a heap object. wasm-bindgen exports a function called __wbindgen_malloc that can be called to allocate that memory. To call this, you would normally do:

instance.Exports["__wbindgen_malloc"](size)

for the current instance. So basically you need access to the current instance from within your implementation of __wbindgen_json_serialize. Ideally this could be passed in as instance context data via instance.SetContextData(unsafe.Pointer(&instance)) so that you could just do:

func __wbindgen_json_serialize(context unsafe.Pointer, memStrDst, heapObjSrc int32) {
    // ...

    instanceContext := wasmer.IntoInstanceContext(context)
    instance := *wasmer.Instance(instanceContext.Data())
    instance.Exports["__wbindgen_malloc"](size)

    // ...
}

But that leads us to the problem above where cgo prevents passing the instance pointer to C.

@losfair
Copy link

losfair commented Sep 6, 2019

The safe way of "passing" Go objects to cgo is to have a global array of Go objects and pass the index to cgo.

A fast but unsafe way is to somehow obfuscate the pointer at cgo boundaries, for example by bit-flipping. You need to make sure the pointer is kept somewhere else so that it won't be collected by the GC too early.

@attente
Copy link

attente commented Sep 6, 2019

The safe way of "passing" Go objects to cgo is to have a global array of Go objects and pass the index to cgo.

Thanks, I suspected this was the case and was about to give it a try. I'm still hoping it's avoidable via some other approach but I guess those are the limitations we're dealing with.

AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Nov 16, 2019
…for user data

Previously an unsafe.Pointer to user data was passed across the CGo FFI
which resulted in a panic if the user data included any other Go
pointers. This commit adds a global ctxData registry assigns a unique
int id to each instance that calls SetContextData. This id is stored in
the actual instance context data that crosses the CGo FFI and is used to
look up the actual user data. A sync.RWMutex is used to synchronize
access to the ctxData map.

BREAKING CHANGE: This changes the API for Instance.SetContextData and
InstanceContext.Data to accept and return an interface{} instead of an
unsafe.Pointer.

re wasmerio#44
AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Nov 16, 2019
…for user data

Previously an unsafe.Pointer to user data was passed across the CGo FFI
which resulted in a panic if the user data included any other Go
pointers. This commit adds a global ctxData registry assigns a unique
int id to each instance that calls SetContextData. This id is stored in
the actual instance context data that crosses the CGo FFI and is used to
look up the actual user data. A sync.RWMutex is used to synchronize
access to the ctxData map.

BREAKING CHANGE: This changes the API for Instance.SetContextData and
InstanceContext.Data to accept and return an interface{} instead of an
unsafe.Pointer.

re wasmerio#44
AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Nov 16, 2019
…for user data

Previously an unsafe.Pointer to user data was passed across the CGo FFI
which resulted in a panic if the user data included any other Go
pointers. This commit adds a global ctxData registry assigns a unique
int id to each instance that calls SetContextData. This id is stored in
the actual instance context data that crosses the CGo FFI and is used to
look up the actual user data. A sync.RWMutex is used to synchronize
access to the ctxData map.

BREAKING CHANGE: This changes the API for Instance.SetContextData and
InstanceContext.Data to accept and return an interface{} instead of an
unsafe.Pointer.

re wasmerio#44
AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Nov 16, 2019
…for user data

Previously an unsafe.Pointer to user data was passed across the CGo FFI
which resulted in a panic if the user data included any other Go
pointers. This commit adds a global ctxData registry assigns a unique
int id to each instance that calls SetContextData. This id is stored in
the actual instance context data that crosses the CGo FFI and is used to
look up the actual user data. A sync.RWMutex is used to synchronize
access to the ctxData map.

BREAKING CHANGE: This changes the API for Instance.SetContextData and
InstanceContext.Data to accept and return an interface{} instead of an
unsafe.Pointer.

re wasmerio#44
AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Nov 16, 2019
…for user data

Previously an unsafe.Pointer to user data was passed across the CGo FFI
which resulted in a panic if the user data included any other Go
pointers. This commit adds a global ctxData registry and assigns a
unique int id to each Instance that calls SetContextData. This id is
stored in the actual C Instance Context Data that crosses the CGo FFI
and is used to look up the actual user data. A sync.RWMutex is used to
synchronize access to the ctxData map.

BREAKING CHANGE: This changes the API for Instance.SetContextData and
InstanceContext.Data to accept and return an interface{} instead of an
unsafe.Pointer.

re wasmerio#44
AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Nov 18, 2019
Previously the given unsafe.Pointer to user data was passed directly
across the CGo FFI resulting in a panic for any reference types or types
that included any reference types or other Go pointers.

This commit avoids a panic by casting the unsafe.Pointer to a uintptr,
which is not analyzed by the runtime. In order for compatability with
the existing CGo API and bridge code, which expects (void*), a *uintptr
is passed. The memory for the user provided unsafe.Pointer and the
*uintptr is cached in the instance to protect it from garbage
collection.

The tests are expanded to ensure that reference types do not cause
panics.

re wasmerio#44 wasmerio#83
@Hywan
Copy link
Contributor

Hywan commented Nov 29, 2019

Can you try with the latest version of master please? #85 is likely to address your need.

AdamSLevy added a commit to AdamSLevy/go-ext-wasm that referenced this issue Dec 3, 2019
Previously an uintptr to user data was passed across the CGo FFI which
would become invalid if any stack frame occurred. This commit avoids
this by never passing any pointers across the CGo FFI. A registry map is
used instead to keep Go data on the Go side, and only an int index into
the map is passed across to the C side.

fix wasmerio#93
re wasmerio#44 wasmerio#85 wasmerio#83
@Hywan Hywan added this to 🏁 Ready in Kanban Dec 4, 2019
@Hywan Hywan moved this from 🏁 Ready to 🌱 In progress in Kanban Dec 4, 2019
@Hywan
Copy link
Contributor

Hywan commented Dec 6, 2019

Closing the issue. Please re-open if #85 doesn't solve your issue. Thanks!

@Hywan Hywan closed this as completed Dec 6, 2019
@Hywan Hywan moved this from 🌱 In progress to 🎉 Done in Kanban Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❓ question Further information is requested
Projects
Kanban
  
🎉 Done
Development

No branches or pull requests

4 participants