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

Non recursive eval_exp() to avoid stack overflow. #252

Merged
merged 1 commit into from
May 3, 2024

Conversation

lvella
Copy link
Contributor

@lvella lvella commented May 2, 2024

In powdr we have some tests that stack overflows inside eval_exp(). This PR makes eval_exp() iterative, so it works on small stacks.

@lvella lvella force-pushed the iteractive_eval_exp branch from fcec08a to 1610838 Compare May 3, 2024 11:05
@@ -416,6 +416,43 @@ fn find_muladd(exp: &Expression) -> Expression {
exp.clone()
}

/// Iteratively traverse a tree in postorder, so that the process stack doesn't overflow.
Copy link
Member

Choose a reason for hiding this comment

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

is there any sample code that will raise the stack overflow? hence we can add that as an unit test.

Copy link
Contributor Author

@lvella lvella May 3, 2024

Choose a reason for hiding this comment

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

I don't have any self contained code. This is the list of powdr tests that causes stack overflow, if executed in a rust thread with the default stack size (2MB):

  • palindrome
  • vm_to_block_unique_interface
  • bit_access
  • functional_instructions

But I am not sure why. I don't know what these tests have that makes the expressions so deep.

@eigmax eigmax merged commit cf405b2 into 0xEigenLabs:main May 3, 2024
5 checks passed
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.

2 participants