Skip to content

Conversation

@quantum5
Copy link
Member

@quantum5 quantum5 commented Jul 19, 2019

This is already a case on the main thread. However, on WASM backend, STACK_BASE == STACK_MAX == the lowest address on the stack, i.e. the point where the stack can grow to. This makes it impossible to find the highest address on the stack, i.e. the bottom of the stack.

This PR makes the behaviour consistent between the main thread and other threads by making other threads follow the conventions of the main thread.

We shall see if the tests pass.

This is already a case on the main thread. However, on WASM backend,
STACK_BASE == STACK_MAX == the lowest address on the stack, i.e. the
point where the stack can grow to. This makes it impossible to find
the highest address on the stack, i.e. the bottom of the stack.

This PR makes the behaviour consistent between the main thread and
other threads by making other threads follow the conventions of the
main thread.
#if WASM_BACKEND
assert(max === e.data.stackBase);
assert(top > max);
assert(e.data.stackBase == max);
Copy link
Member Author

Choose a reason for hiding this comment

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

This assertion is redundant.

@quantum5 quantum5 merged commit 15b01e8 into emscripten-core:incoming Jul 20, 2019
@kripken
Copy link
Member

kripken commented Jul 25, 2019

It would be good to add a testcase for this - something that fails only before the fix.

belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
…ripten-core#9033)

This is already a case on the main thread. However, on WASM backend,
STACK_BASE == STACK_MAX == the lowest address on the stack, i.e. the
point where the stack can grow to. This makes it impossible to find
the highest address on the stack, i.e. the bottom of the stack.

This PR makes the behaviour consistent between the main thread and
other threads by making other threads follow the conventions of the
main thread.
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.

4 participants