-
Notifications
You must be signed in to change notification settings - Fork 267
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
wasi: Implement random_get
function
#237
Closed
+2,052
−771
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
4861e84
wasi: Init random_get function
r8d8 3b40719
Update WASI `random_get`. Add tests.
r8d8 72d96be
jit(arm64): support for and, or, xor (#225)
summerwind d611136
Adds note clarifying status of WASI tests (#226)
codefromthecrypt 28627d7
Fix `fibonacci` example in Readme (#229)
r8d8 5d205ea
jit(arm64): support for shl, shr, rotl, and rotr (#230)
summerwind c07dc54
Adds Module.SectionSize and constrains text parsing rules (#231)
codefromthecrypt 1324e5c
Removes CustomSection except NameSection (#234)
codefromthecrypt eeb18d1
jit(arm64): support call instruction (#221)
mathetake 7011653
jit(arm64): Add common 'compile'/'maybeCompile' prefix (#235)
mathetake 814e103
Fix source formatting
r8d8 1d3707a
Fix review comments:
r8d8 5895804
Provided interface for random source.
r8d8 79db4ff
Adds CONTRIBUTING template (#239)
codefromthecrypt a69be1a
Adds editor config to ensure EOL before EOF (#240)
codefromthecrypt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
root = true | ||
|
||
[*] | ||
charset = utf-8 | ||
end_of_line = lf | ||
insert_final_newline = true | ||
trim_trailing_whitespace = true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
# Contributing | ||
|
||
We welcome contributions from the community. Please read the following guidelines carefully to maximize the chances of your PR being merged. | ||
|
||
## Coding Style | ||
|
||
- To ensure your change passes format checks, use run `make check`. To format your files, you can run `make format`. | ||
- We follow standard Go table-driven tests and use the [`testify/require`](https://github.com/stretchr/testify#require-package) library to assert correctness. To verify all tests pass, you can run `make test`. | ||
|
||
## DCO | ||
|
||
We require DCO signoff line in every commit to this repo. | ||
|
||
The sign-off is a simple line at the end of the explanation for the | ||
patch, which certifies that you wrote it or otherwise have the right to | ||
pass it on as an open-source patch. The rules are pretty simple: if you | ||
can certify the below (from | ||
[developercertificate.org](https://developercertificate.org/)): | ||
|
||
``` | ||
Developer Certificate of Origin | ||
Version 1.1 | ||
Copyright (C) 2004, 2006 The Linux Foundation and its contributors. | ||
660 York Street, Suite 102, | ||
San Francisco, CA 94110 USA | ||
Everyone is permitted to copy and distribute verbatim copies of this | ||
license document, but changing it is not allowed. | ||
Developer's Certificate of Origin 1.1 | ||
By making a contribution to this project, I certify that: | ||
(a) The contribution was created in whole or in part by me and I | ||
have the right to submit it under the open source license | ||
indicated in the file; or | ||
(b) The contribution is based upon previous work that, to the best | ||
of my knowledge, is covered under an appropriate open source | ||
license and I have the right under that license to submit that | ||
work with modifications, whether created in whole or in part | ||
by me, under the same open source license (unless I am | ||
permitted to submit under a different license), as indicated | ||
in the file; or | ||
(c) The contribution was provided directly to me by some other | ||
person who certified (a), (b) or (c) and I have not modified | ||
it. | ||
(d) I understand and agree that this project and the contribution | ||
are public and that a record of the contribution (including all | ||
personal information I submit with it, including my sign-off) is | ||
maintained indefinitely and may be redistributed consistent with | ||
this project or the open source license(s) involved. | ||
``` | ||
|
||
then you just add a line to every git commit message: | ||
|
||
Signed-off-by: Joe Smith <joe@gmail.com> | ||
|
||
using your real name (sorry, no pseudonyms or anonymous contributions.) | ||
|
||
You can add the sign off when creating the git commit via `git commit -s`. | ||
|
||
## Code Reviews | ||
|
||
* Indicate the priority of each comment, following this | ||
[feedback ladder](https://www.netlify.com/blog/2020/03/05/feedback-ladders-how-we-encode-code-reviews-at-netlify/). | ||
If none was indicated it will be treated as `[dust]`. | ||
* A single approval is sufficient to merge, except when the change cuts | ||
across several components; then it should be approved by at least one owner | ||
of each component. If a reviewer asks for changes in a PR they should be | ||
addressed before the PR is merged, even if another reviewer has already | ||
approved the PR. | ||
* During the review, address the comments and commit the changes _without_ squashing the commits. | ||
This facilitates incremental reviews since the reviewer does not go through all the code again to | ||
find out what has changed since the last review. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
This directory contains tests which use multiple packages. For example: | ||
|
||
- `bench` contains benchmark tests. | ||
- `codec` contains a test and benchmark on text and binary decoders. | ||
- `engine` contains variety of e2e tests, mainly to ensure the consistency in the behavior between engines. | ||
- `spectest` contains end-to-end tests with the [WebAssembly specification tests](https://github.com/WebAssembly/spec/tree/wg-1.0/test/core). | ||
* `bench` contains benchmark tests. | ||
* `codec` contains a test and benchmark on text and binary decoders. | ||
* `engine` contains variety of e2e tests, mainly to ensure the consistency in the behavior between engines. | ||
* `spectest` contains end-to-end tests with the [WebAssembly specification tests](https://github.com/WebAssembly/spec/tree/wg-1.0/test/core). | ||
|
||
*Note*: this doesn't contain WASI tests, as there's not yet an [official testsuite](https://github.com/WebAssembly/WASI/issues/9). | ||
Meanwhile, WASI functions are unit tested including via Text Format imports [here](../wasi/wasi_test.go) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
;; This is a wat file to just export clock WASI API to the host environment for testing the APIs. | ||
;; This is currently separated as a wat file and pre-compiled because our text parser doesn't | ||
;; implement 'memory' yet. After it supports 'memory', we can remove this file and embed this | ||
;; wat file in the Go test code. | ||
;; | ||
;; Note: Although this is a raw wat file which should be moved under /tests/wasi in principle, | ||
;; this file is put here for now, because this is a temporary file until the parser supports | ||
;; the enough syntax, and this file will be embedded in unit test codes after that. | ||
(module | ||
(import "wasi_snapshot_preview1" "random_get" | ||
(func $wasi.random_get (param $buf i32) (param $buf_len i32) (result (;errno;) i32))) | ||
(memory 1) ;; just an arbitrary size big enough for tests | ||
(export "memory" (memory 0)) | ||
;; Define wrapper functions instead of just exporting the imported WASI APIS for now | ||
;; because wazero's interpreter has a bug that it crashes when an imported-and-exported host function | ||
;; is called from the host environment, which will be fixed soon. | ||
;; After it's fixed, these wrapper functions are no longer necessary. | ||
(func $random_get (param i32 i32) (result i32) | ||
local.get 0 | ||
local.get 1 | ||
call $wasi.random_get | ||
) | ||
(export "random_get" (func $random_get)) | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this drifted and also we are probably being too specific on the mechanics of random.
maybe
For example, if bufLen = 5, we expect
ctx.Memory.Bufferto contain 5 random bytes:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codefromthecrypt
You mean not to use in comments concrete random bytes from specified seed?
And also not to check concrete random bytes form seed, cause crypto/rand non-deterministic and we can't seed it, right?
Just check in test that memory is filled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no I mean in this godoc we don't need to get into how secure random works. I still like that you are using tests that are resulting in a known value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we can seed math/rand from crypto/rand, like this
and in tests still use simple seed and check concrete bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, thanks for your feedback