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

data structure passed through InstanceContext isn't preserved #135

Closed
noot opened this issue Jun 12, 2020 · 8 comments
Closed

data structure passed through InstanceContext isn't preserved #135

noot opened this issue Jun 12, 2020 · 8 comments
Assignees
Labels
🐞 bug Something isn't working
Projects

Comments

@noot
Copy link
Contributor

noot commented Jun 12, 2020

Describe the bug

Hey, I'm currently using wasmer in gossamer. In our usage of wasmer, we pass in a Ctx struct as the InstanceContext here. We instantiate the module here. The Storage field in the struct has an underlying trie data structure. (relevant code is here and here) The instance imports various go functions here that use Storage to manipulate the trie key-values.

I've noticed that between calls to the imported functions, the trie values become incorrect (I've verified this isn't an issue with the actual trie structure by replicating the key-values that are being inserted) I first noticed this using the substrate runtime. I created a small test wasm program to isolate this here. It first calls the external function ext_set_storage setting a key-value pair in the trie. Then, it calls another external function ext_twox_128 which simply hashes some value and returns it. In between these two calls, the value stored in the trie at key changes.

Steps to reproduce

git clone https://github.com/ChainSafe/gossamer.git
cd gossamer
git checkout noot/execute-block
go test ./lib/runtime/ -v -run Test_mock_execute_block

this will download the test wasm file and call the mock_execute_block function that performs the above, in the logs you can see the external function call logs and the trie contents:

=== RUN   Test_mock_execute_block
t=2020-06-11T22:02:50-0400 lvl=dbug msg=[NewRuntime] runtimeCtx="{storage:0xc0005b6010 allocator:0xc0001b6280 keystore:0xc0000faaa0}"
t=2020-06-11T22:02:50-0400 lvl=trce msg="[ext_set_storage] executing..."
t=2020-06-11T22:02:50-0400 lvl=trce msg=[ext_set_storage] key=0x4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d val="[0 0 0 0]"
t=2020-06-11T22:02:50-0400 lvl=dbug msg="[teststate] SetStorage" key=0x4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d val=0x00000000
leaf prefix  key 4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d value 00000000

t=2020-06-11T22:02:50-0400 lvl=trce msg="[ext_twox_128] executing..."
t=2020-06-11T22:02:50-0400 lvl=trce msg="[ext_twox_128] hashing..." value=u\r\r"
leaf prefix  key 4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d value 08007500

after the first call, the key 4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d has value 00000000 which is correct. then, after the second call, the value is now 08007500.

Expected behavior

the values stored in the trie stay the same

Actual behavior

the values changed

Additional context

let me know if you need any more info - I've tried to isolate the issue, but please let me know if you need me to extract it more. I suspect the issue is something to do with the underlying memory being overwritten or some pointers getting mangled, but I'm not totally sure. maybe there's something I'm missing in the usage of InstanceContext. any help would be much appreciated!

@noot noot added the 🐞 bug Something isn't working label Jun 12, 2020
@AdamSLevy
Copy link
Contributor

This is due to the issue described in my comment here most likely:
#46 (comment)

@noot
Copy link
Contributor Author

noot commented Jun 12, 2020

@AdamSLevy thanks for pointing that out! I'll take a look and see if I can fix it

edit: hm, maybe I am missing something, but it looks like wasmer_instance_context_data_set does take an instance, not instanceContext?
https://github.com/wasmerio/wasmer/blob/ab106af422a5c9e263d08c1f8a8ae1fd859e55b8/lib/runtime-c-api/src/instance.rs#L439

@AdamSLevy
Copy link
Contributor

It's possible I'm mistaken. Apologies for a red herring.

@noot
Copy link
Contributor Author

noot commented Jun 12, 2020

no worries! I'll continue to investigate in the meantime.

@Hywan Hywan self-assigned this Jun 12, 2020
@Hywan Hywan added this to 📬 Backlog in Kanban via automation Jun 12, 2020
@Hywan Hywan moved this from 📬 Backlog to 🏁 Ready in Kanban Jun 12, 2020
@noot
Copy link
Contributor Author

noot commented Jun 22, 2020

@Hywan hey, any updates on the status of this issue? let me know if I can help speed this up in any way.

@Hywan
Copy link
Contributor

Hywan commented Jun 26, 2020

Sorry, the entire team is busy on a giant secret project. I'll get back to you very soon! Thank you for your patience :-).

@syrusakbary
Copy link
Member

@Hywan ping on this for next work :)

@Hywan
Copy link
Contributor

Hywan commented Feb 4, 2021

InstanceContext is no longer available in wasmer-go 1.0, so… problem solved :-D! We will add an API to pass any kind of environment soon.

@Hywan Hywan closed this as completed Feb 4, 2021
Kanban automation moved this from 🏁 Ready to 🎉 Done Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
Kanban
  
🎉 Done
Development

No branches or pull requests

4 participants