Skip to content

Initialize the stack bounds prior to checking the cookie - #24317

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:asan_test_stack
May 13, 2025
Merged

Initialize the stack bounds prior to checking the cookie#24317
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:asan_test_stack

Conversation

@sbc100

@sbc100 sbc100 commented May 13, 2025

Copy link
Copy Markdown
Collaborator

Without this change reading of the cookie value calls into __asan_loadN which then tries to use SP, but SP is not yet initialzed, so the asan code itself trigger a STACK_OVERFLOW_CHECK error.

This fixes the asan.test_stack test which was broken by #24314 but went unnoticed because we don't run all the asan tests in emscripten CI.

Without this change reading of the cookie value calls into __asan_loadN
which then tries to use SP, but SP is not yet initialzed, so the
asan code itself trigger a STACK_OVERFLOW_CHECK error.

This fixes the asan.test_stack test which was broken by emscripten-core#24314 but went
unnoticed because we don't run all the asan tests in emscripten CI.
@sbc100

sbc100 commented May 13, 2025

Copy link
Copy Markdown
Collaborator Author

My guess is that the reason this is needed is that the previous function (_asan_c_load_4u) did not touch SP, but the new function (__asan_loadN) does. So stack limits now need to be set prior to checkStackCookie.

@RReverser

Copy link
Copy Markdown
Collaborator

did not touch SP, but the new function (__asan_loadN) does

Yeah they get stack address for error reporting differently, since __asan_load{num} variants are optimised for inlining (which is not really relevant for our calls from JS, but does change access patterns I guess).

Comment thread test/core/test_stack.c

@RReverser RReverser left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@sbc100
sbc100 merged commit 870b3b2 into emscripten-core:main May 13, 2025
@sbc100
sbc100 deleted the asan_test_stack branch May 13, 2025 19:13
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