-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
cannon: Finish emulating rest of 64-bit instructions #12483
Conversation
6a90eda
to
551622c
Compare
Semgrep found 6
Inputs to functions must be prepended with an underscore ( Semgrep found 1 No Semgrep found 1 Do not use Semgrep found 1 MarshalJSON with a pointer receiver has surprising results: golang/go#22967 Ignore this finding from marshal-json-pointer-receiver.Semgrep found 1 superfluous nil err check before return Ignore this finding from err-nil-check. |
551622c
to
b749c7f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #12483 +/- ##
===========================================
- Coverage 64.86% 63.79% -1.08%
===========================================
Files 54 54
Lines 4460 4538 +78
===========================================
+ Hits 2893 2895 +2
- Misses 1391 1461 +70
- Partials 176 182 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Looking good! Left a number of nits + a few questions.
Semgrep found 1 require() must include a reason string Ignore this finding from sol-style-require-reason.Semgrep found 1 Prefer Semgrep found 5
Inputs to functions must be prepended with an underscore ( Semgrep found 8
No |
@mbaxter I fixed a bug in I have verified the expected |
This fixes the 64-bit stubs for various instructions (except lld/scd).
709df10
to
e6b9bf2
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.
lgtm - just a few last questions on dmult and div/divu divide by zero
Semgrep found 1 Detected directly writing or similar in 'http.ResponseWriter.write()'. This bypasses HTML escaping that prevents cross-site scripting vulnerabilities. Instead, use the 'html/template' package and render data using 'template.Execute()'. Ignore this finding from no-direct-write-to-responsewriter.Semgrep found 1 Untrusted input could be used to tamper with a web page rendering, which can lead to a Cross-site scripting (XSS) vulnerability. XSS vulnerabilities occur when untrusted input executes malicious JavaScript code, leading to issues such as account compromise and sensitive information leakage. To prevent this vulnerability, validate the user input, perform contextual output encoding or sanitize the input. For more information, see: Go XSS prevention. View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>op-challenger/game/fault/trace/prestates/multi_test.go</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/ethereum-optimism/optimism/blob/498a36def44ac54d68189319fff89cc45994414b/op-challenger/game/fault/trace/prestates/multi_test.go#L194 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 194] r.URL</a>"]
end
%% Intermediate
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/ethereum-optimism/optimism/blob/498a36def44ac54d68189319fff89cc45994414b/op-challenger/game/fault/trace/prestates/multi_test.go#L194 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 194] w.Write([]byte(r.URL.Path))</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
File0:::invis
%% Connections
Source --> Sink
|
…programs cannon: Fix remaining mips64 emulation bugs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Semgrep found 1 Modifiers that don't do something before and after execution are banned. Ignore this finding from ban_non_wraparound_modifiers. |
…sm#12483) * cannon: Finish emulating rest of 64-bit instructions This fixes the 64-bit stubs for various instructions (except lld/scd). * review comments; fix dmult * add todo * test div by zero * add a couple more dmultu tests * remove dead code * cannon: Fix remaining mips64 emulation bugs * fix 64-bit Makefile build script; review comments * fix build script
This completes 64-bit instruction execution (except lld/scd which is handled separately).
Testing
dmult
These tests are only run on by the 64-bit VM. So they do not run on CI, except for the
dmult
fuzz test which is executed by thefuzz-golang
job.Meta
Fixes #12243 and #12248.