-
Notifications
You must be signed in to change notification settings - Fork 389
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
feat: fuzz gnovm/pkg/transpiler.Transpile #3457
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
6eeae78
to
991623e
Compare
Adds a fuzzer for Transpile, which found bugs: * gnolang#3425 * gnolang#3426 * partially gnolang#3428 Updates gnolang#3087
991623e
to
58d6216
Compare
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.
Thanks for doing this. Before we give a LGTM, A couple of things:
- can I get you to clean up that one unused variable in the code that I pointed out?
- Could you add some more context into the PR body explaining what this PR does to give some more context to the issues you've linked it to?
Removed the |
Gotcha @n2p5, I have added more succinct context to the body/description of the PR per #3457 (comment), please take a look. |
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.
Spoke with @odeke-em on this and we reviewed together.
@n2p5 Why was this merged with a failing CI? |
ah, @thehowl , that was a mistake, I think I mixed up my tabs here and missed this one. |
Adds a fuzzer for Transpile, which found bugs: * #3425 in which this following Go program crashed the transpiler ```go package A import(""//" ""/***/) ``` * #3426 which generated an input that revealed the fact that Gno deviates from Go by allowing unused variables yet Go's standard is strict on unused variables like this program ```go package main func main() { const c1 = 1 < 8 main() 1 } ``` which we shouldn't allow * partially #3428 which revealed the discrepancy in Gno that the overflow detection is still lacking as the following program is invalid Go but Gno allowed it to run ```go package main func main() { const c1 = int8(1) << 8 println(c1) } ``` because 1<<8 (256) is higher than the range of int8 for which the maximum is `(1<<7) - 1 aka 127` Updates #3087 Co-authored-by: Nathan Toups <[email protected]>
Adds a fuzzer for Transpile, which found bugs:
which we shouldn't allow
because 1<<8 (256) is higher than the range of int8 for which the maximum is
(1<<7) - 1 aka 127
Updates #3087