Skip to content

Increase stylo stack size in asan and debug builds#42115

Merged
jschwe merged 2 commits intoservo:mainfrom
jschwe:update_stylo
Jan 28, 2026
Merged

Increase stylo stack size in asan and debug builds#42115
jschwe merged 2 commits intoservo:mainfrom
jschwe:update_stylo

Conversation

@jschwe
Copy link
Copy Markdown
Member

@jschwe jschwe commented Jan 23, 2026

This bumps stylo to allow configuring the stack size of the stylo threads, which fixes crashes with asan and debug builds.

Testing: Manual testing with --with-asan and with the debug profile.
Fixes: #40814

@jschwe jschwe marked this pull request as ready for review January 23, 2026 16:01
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 23, 2026
Comment on lines +146 to +150
if build_type.is_dev():
# Increase stylo thread stack size to 2 MiB for debug builds since the stack usage is higher
# and crashes have been reported. The default is 512 KiB.
# Note: This is placed after `build_sanitizer_env()` so that the higher stack size with ASAN takes
# priority over the debug build stack size, which matters for ASAN builds with the debug profile.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should be setting all environment variables in one place instead of many places. I think you can do this by passing in an optional sanitizer argument to build_env() and calling build_sanitizer_env from there. Sorry that I have not spotted this earlier.

Setting up the environment in one place makes it easier understand where environment variables get set.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I generally agree with you, but (partially) moving build_sanitizer_env into build_env() is not trivial and might require more time than I can currently spare. I moved this part into build_env() though, since it's easy enough.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jan 24, 2026
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jan 27, 2026
Stylo servo/stylo#269 recently added the option
to configure the stack size of the style threads.
We increase the stack size for asan and debug builds.

Signed-off-by: Jonathan Schwender <[email protected]>
Signed-off-by: Jonathan Schwender <[email protected]>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 28, 2026
@jschwe jschwe enabled auto-merge January 28, 2026 15:30
@jschwe jschwe added this pull request to the merge queue Jan 28, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 28, 2026
Merged via the queue into servo:main with commit 6b12ecf Jan 28, 2026
32 checks passed
@jschwe jschwe deleted the update_stylo branch January 28, 2026 16:47
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"illegal hardware instruction" on MacOS

3 participants