Stack depth management for CALL blocks #403
Merged
Conversation
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
bobbinth
force-pushed
the
mem-context
branch
2 times, most recently
from
f6a1c1f
to
31f825e
Compare
bobbinth
force-pushed
the
stack-refactor
branch
4 times, most recently
from
41ddd96
to
9ef0c77
Compare
bobbinth
force-pushed
the
stack-refactor
branch
from
9ef0c77
to
76af515
Compare
grjte
approved these changes
Sep 23, 2022
Comment on lines
+108
to
+113
| // if last_row_addr is ZERO, any rows in the overflow table are not accessible from the | ||
| // current context. Thus, we should not be able to remove them. | ||
| debug_assert_ne!( | ||
| self.last_row_addr, ZERO, | ||
| "overflow table is empty in the current context" | ||
| ); |
There was a problem hiding this comment.
given that removing an overflow row when the table is actually empty will cause a failure, I wonder if this should be a regular assert instead of debug_assert
There was a problem hiding this comment.
This should actually never happen under normal execution because we call Overflow::pop() only from Stack::shift_left() function. And there, we remove an item from the overflow table only if the stack depth is > 16.
bobbinth
force-pushed
the
stack-refactor
branch
from
76af515
to
62c078c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR implements management of stack depth for
CALLcode blocks. The rules are as follows:CALLoperation is executed, stack depth is set to 16 (the overflow table stays unchanged). This means that the callee function has access only the top 16 stack items of the caller.CALLblock is ended, stack depth is set to its pre-call value. We also need to ensure that the callee did not leave any extra items in the overflow table. To do this, we enforce that stack depth must be exactly 16 when aCALLblock ends.This PR includes the following:
MIN_STACK_DEPTH->STACK_TOP_SIZE), and fixing a couple of trace generation bugs related to population of stack helper registers.stack.start_context()andstack.restore_context()methods to facilitate starting/ending of execution contexts within a stack.Processstruct for test purposes was updated to ensure that during tests user operations are never executed atclk=0.CALLblock starts/ends execution. Also, components related to block stack table management were updated.This PR does not include updates to the docs describing changes to constraints etc.