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

cannon: Fix GC emulation of Go programs #11704

Merged
merged 15 commits into from
Sep 12, 2024
Merged

cannon: Fix GC emulation of Go programs #11704

merged 15 commits into from
Sep 12, 2024

Conversation

Inphi
Copy link
Contributor

@Inphi Inphi commented Aug 31, 2024

Meta

Necro of #11292.
Fixes #11658 and #11648

Description

This patch improves Linux/MIPS32 emulation for Go programs that utilize the garbage collector and goroutine scheduling.

This adds support for the following syscalls:

  • getpid - used by the go scheduler
  • clock_gettime - used by the go scheduler and for GC assists and to properly emulate time related operations such as time.Sleep.

Note on GC assists

The Go GC relies on clock_gettime for GC "assists", whereby a mutator can perform a little bit of GC without waiting for the scheduler to do so.
A monotonic clock (runtime.nanotime) is used to compute the current goroutine's compute budget. By modeling a MIPS32 CPU that runs at some clock speed (ex: 10 MHz), we can provide a consistent emulation of monotonic time needed by the Go runtime. All other clock_gettime flags are handled as unimplemented syscalls.

Memory Proofs

Emulating clock_gettime a special as it's the only syscall that requires multi-word memory writes. There are two obvious ways to deal with this:

  • Break up the syscall into two "microsteps" (similar to futex handling), where one step writes the 32-bit seconds field in the timespec struct, while the other writes the nanoseconds time field. This requires a single proof per microstep.
  • Issue two memory writes for the single step.

This patch uses the latter approach in order to not complicate the VM state machine that handles futex and thread traversal. However, this means proofs need to be carefully verified between memory accesses. Thus, MIPS2.sol requires a second memory proof when stepping on a clock_gettime syscall.
This second memory proof is used to verify that the leaf node associated with the second write corresponds to a valid memory root (i.e. a merkle tree containing the first memory write).

Testing

  • TestInstrumentedState_Alloc re-enabled and now asserts against a couple more allocation patterns.

@Inphi Inphi requested review from ajsutton and mbaxter August 31, 2024 19:59
@Inphi Inphi requested review from a team as code owners August 31, 2024 19:59
Improves Linux/MIPS32 emulation for Go programs that utilize the garbage
collector and goroutine scheduling.

This adds support for the following syscalls:

- getpid - used by the go scheduler
- clock_gettime - used by the go scheduler and for GC assists and to properly emulate
  time related operations such as `time.Sleep`.

Note on GC assists:

The Go GC relies on `clock_gettime` for GC "assists", whereby a mutator can perform a little bit
of GC without waiting for the scheduler to do so.
A monotonic clock (runtime.nanotime) is used to compute the current goroutine's compute budget.
By modeling a MIPS32 CPU that runs at some clock speed (ex: 10 MHz), we can provide a consistent
emulation of monotonic time needed by the Go runtime.
All other clock_gettime flags are handled as unimplemented syscalls.
cannon/mipsevm/exec/mips_syscalls.go Show resolved Hide resolved
cannon/mipsevm/exec/mips_syscalls.go Show resolved Hide resolved
cannon/mipsevm/multithreaded/mips.go Outdated Show resolved Hide resolved
packages/contracts-bedrock/src/cannon/MIPS2.sol Outdated Show resolved Hide resolved
packages/contracts-bedrock/src/cannon/MIPS2.sol Outdated Show resolved Hide resolved
packages/contracts-bedrock/test/cannon/MIPS2.t.sol Outdated Show resolved Hide resolved
@Inphi Inphi requested review from a team as code owners September 9, 2024 23:52
@Inphi Inphi requested a review from geoknee September 9, 2024 23:52
Copy link
Contributor

@mbaxter mbaxter left a comment

Choose a reason for hiding this comment

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

Looks good! Just one question on the ClockGetTime error type.

cannon/mipsevm/memory/memory.go Show resolved Hide resolved
cannon/mipsevm/multithreaded/mips.go Outdated Show resolved Hide resolved
cannon/mipsevm/multithreaded/mips.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 86.44068% with 8 lines in your changes missing coverage. Please review.

Project coverage is 78.78%. Comparing base (c21d0c5) to head (82f3e35).
Report is 13 commits behind head on develop.

Files with missing lines Patch % Lines
...non/mipsevm/multithreaded/testutil/expectations.go 50.00% 4 Missing ⚠️
cannon/mipsevm/exec/memory.go 71.42% 1 Missing and 1 partial ⚠️
cannon/mipsevm/testutil/oracle.go 75.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11704      +/-   ##
===========================================
+ Coverage    78.49%   78.78%   +0.29%     
===========================================
  Files           41       41              
  Lines         3208     3262      +54     
===========================================
+ Hits          2518     2570      +52     
  Misses         529      529              
- Partials       161      163       +2     
Flag Coverage Δ
cannon-go-tests 78.78% <86.44%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cannon/mipsevm/exec/mips_syscalls.go 93.66% <ø> (ø)
cannon/mipsevm/memory/memory.go 78.81% <100.00%> (+0.98%) ⬆️
cannon/mipsevm/multithreaded/instrumented.go 69.44% <100.00%> (+0.87%) ⬆️
cannon/mipsevm/multithreaded/mips.go 96.47% <100.00%> (+0.25%) ⬆️
cannon/mipsevm/exec/memory.go 80.00% <71.42%> (-4.62%) ⬇️
cannon/mipsevm/testutil/oracle.go 78.50% <75.00%> (+4.71%) ⬆️
...non/mipsevm/multithreaded/testutil/expectations.go 97.10% <50.00%> (-2.90%) ⬇️

@Inphi Inphi requested a review from a team as a code owner September 11, 2024 23:13
@Inphi Inphi removed the request for review from a team September 11, 2024 23:52
Copy link
Contributor

semgrep-app bot commented Sep 12, 2024

Semgrep found 1 todos_require_linear finding:

Please create a GitHub ticket for this TODO.

Ignore this finding from todos_require_linear.

@Inphi
Copy link
Contributor Author

Inphi commented Sep 12, 2024

@smartcontracts The heavy fuzz tests are timing out in CI due to slow MIPS test fuzzing. The MIPS tests utilize ffi which is terribly slow to execute for fuzzing. I've tweaked the heavy fuzzer to ignore the MIPS tests in order to unblock this PR. I'll follow up on this by completely moving all forge fuzz testing for the MIPS VM to Go (as done here) so that we can remove the tweaks.

We probably need to adjust the number of runs the heavy fuzzer uses as this issue may crop up again for other contracts.

@Inphi Inphi added this pull request to the merge queue Sep 12, 2024
Merged via the queue into develop with commit 7ff5e6e Sep 12, 2024
62 checks passed
@Inphi Inphi deleted the inphi/cannon-alloc branch September 12, 2024 02:38
@mbaxter mbaxter mentioned this pull request Sep 16, 2024
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.

Debug alloc test
2 participants