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

Add random_get WASI function #195

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

GorogPeter
Copy link
Contributor

No description provided.

)
)

(assert_return (invoke "_start") (i32.const 0))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't _start be called automatically? I remember there was a discussion about it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is discussed in #187 , but not yet completed

@zherczeg
Copy link
Collaborator

Is random returns with 0? Strange.

@kulcsaradam
Copy link
Contributor

For the record: WASI functions return error codes only and have their ,,result'' written into memory. Error code 0 is succes.

@GorogPeter
Copy link
Contributor Author

Oooops... I forgot about this patch, sorry for not responding. :(
Thank you @kulcsaradam for responding.

This test only calls the random_get function without checking its result (I mean the randomness of the numbers).

Comment on lines +75 to +77
void* buf = (void*)(instance->memory(0)->buffer() + argv[0].asI32());
uvwasi_size_t length = argv[1].asI32();
result[0] = Value(uvwasi_random_get(WASI::m_uvwasi, buf, length));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the spec,
random_get function writes a random data into a buffer input.
Does this buffer mean memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, but I'll ask my co-worker about it to be sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they do mean the memory.

https://github.com/WebAssembly/WASI/blob/main/legacy/application-abi.md

Regardless of the kind, all modules accessing WASI APIs also export a linear memory with the name memory. Data pointers in WASI API calls are relative to this memory's index space.

Maybe in the future if multiple memory ever becomes a standard then these funcitons may also gain an index to which memory to use.

Copy link
Collaborator

@clover2123 clover2123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@clover2123 clover2123 merged commit b62ff1a into Samsung:main Nov 29, 2023
@GorogPeter GorogPeter deleted the add_random_get branch November 29, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants