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

Eliminate MEMORY_OVERFLOW ? #378

Closed
DavePearce opened this issue Oct 5, 2022 · 2 comments · Fixed by #379
Closed

Eliminate MEMORY_OVERFLOW ? #378

DavePearce opened this issue Oct 5, 2022 · 2 comments · Fixed by #379

Comments

@DavePearce
Copy link
Collaborator

DavePearce commented Oct 5, 2022

(See also #193, #250 and #265)

An interesting question is whether or not we can now eliminate the idea of a MEMORY_OVERFLOW. My gut feeling, from previous efforts at this, is that we cannot. However, I can't remember the exactly reasoning for this. Similarly, given that we now have gas calculation in place ... well, maybe it could work.

Therefore, this issue is for tracking whether or not this actually can work.

@DavePearce
Copy link
Collaborator Author

DavePearce commented Oct 5, 2022

  • MLOAD
  • MSTORE
  • MSTORE8
  • MSIZE
  • LOGX
  • CALL
  • CALLCODE
  • RETURN
  • DELEGATECALL
  • STATICCALL
  • REVERT
  • CALLDATACOPY
  • CODECOPY
  • EXTCODECOPY
  • CallReturn
  • CreateReturn

@DavePearce
Copy link
Collaborator Author

So, MSIZE and RETURNDATASIZE are problematic. This is because they load the length of memory onto the stack. For these bytecodes, we return the fictional exception MEMORY_OVERFLOW.

DavePearce added a commit that referenced this issue Oct 5, 2022
The `MEMORY_OVERFLOW` is a ficticious exception create to appease Dafny
in certain circumstances.  In particular, they Yellow Paper places no
restrictions on the bounds of memory.  And yet, there are implied
limits.  In particular, the `MSIZE` bytecode loads the length of memory
onto the stack as a `u256`.  But, the yellow paper says nothing about
what should happen in the event of an overflow.
@DavePearce DavePearce linked a pull request Oct 5, 2022 that will close this issue
DavePearce added a commit that referenced this issue Oct 5, 2022
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 a pull request may close this issue.

1 participant