Skip to content
This repository was archived by the owner on Dec 18, 2023. It is now read-only.

Set default stack size + static data size to 576KB#5

Open
guybedford wants to merge 3 commits into
wasdk:masterfrom
guybedford:patch-1
Open

Set default stack size + static data size to 576KB#5
guybedford wants to merge 3 commits into
wasdk:masterfrom
guybedford:patch-1

Conversation

@guybedford

Copy link
Copy Markdown

(correction to #4 setting valid values)

When using WASM Explorer or WASM Fiddle, any use of the stack will cause a memory out of bounds exception.

By passing the -s option to s2wasm we can initialize the stack value to the top of the initial memory.

This PR sets the total stack + static data size to 576KB (which seems like a sensible default). Ideally these should be parameters into the service though to tune requirements. Also it should be possible to inspect the static data size of a compilation and ensure that the stack size is always a constant stack size value regardless of the static data size, which would be an even more sensible default.

This at now will at least allow experimentation with stack things, as this service is hopefully intended for.

Comment thread scripts/compile.sh Outdated
rm "$sfile~"

$BYNARYEN_ROOT/s2wasm "$sfile" > "$wastfile"
$BYNARYEN_ROOT/s2wasm "$sfile" -i 589824 -s 524288 > "$wastfile"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Even better here may be -i 655360 to get a nice round (memory 10).

@guybedford

guybedford commented May 23, 2017

Copy link
Copy Markdown
Author

I've updated this just to set the -i initial memory option, which seems to automatically set the stack to the top of this initial allocation when used.

Just setting the -s option allows a constant stack size, with the initial memory automatically calculated, which seems the best.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant