Skip to content

wamr: bump the default stack guard size #2241

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 19, 2024
Merged

wamr: bump the default stack guard size #2241

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions interpreters/wamr/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,18 @@ config INTERPRETERS_WAMR_ENABLE_SPEC_TEST

config INTERPRETERS_WAMR_STACK_GUARD_SIZE
int "Custom stack guard size"
default 0
default 1024
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Dec 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is it good to resolve 1KB for stack guard by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean by "resolve" in this context?

Copy link
Contributor

Choose a reason for hiding this comment

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

typo I mean reserve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1KB might be a bit too large for typical nuttx targets.
but it's definitely safer than 0.
also, 1KB is what wamr uses by default for other platforms.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that the stack size consumed by wamr function is fixed, why wasm engine can't catch the stack overflow before jump to the new function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because your assumption is wrong?
at least in the current wamr aot implementation, a caller doesn't have precise knowledge about how much stack the callee consumes. the callees need to check the overflow by itself and the check itself might consume stack. (thus the small amount of stack is reserved here.)

Copy link
Contributor

Choose a reason for hiding this comment

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

But 1KB isn't a small amount of stack.

---help---
Reserve some space in stack as guard to detect stack overflow,
use stack base by default but may not safe enough.
Reserve some space in stack as guard to detect stack overflow.

Please consider this extra stack consumption when deciding the stack
size of your NuttX task on which you run WAMR.

The default value here (1024) was chosen to match the default value on
the majority of other platforms, including Linux. It also matches
the expections in the WAMR AoT compiler.
On the other hand, it might be a bit larger than what's strictly
necessary, especially for typical NuttX targets.
Copy link
Contributor

Choose a reason for hiding this comment

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

so, why not give a small value match the nuttx normal case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just because it's difficult to fine tune this kind of things.


config INTERPRETERS_WAMR_THREAD_MGR
bool "Enable thread manager"
Expand Down