Skip to content

Conversation

@dtzWill
Copy link

@dtzWill dtzWill commented Apr 1, 2022

noteBottomOfStack:

Without this, checks against stack space within Clang don't work as
Clang doesn't know where the stack begins.

Needed per-thread, as early as possible.
(on threads using Clang)

Using Clang's desired stack size:

Additionally increase stack size of pthreads to Clang's desired size.

This is presently 8MB, and is used by Clang's stack management
mechanisms to check* if close to stack exhaustion when determining if
there's sufficient space (and warn or run on a new thread with more).
(see runWithSufficientStackSpace)

The constant is available in LLVM 7 onwards.

  • (abs(cur - bottom) > DesiredStackSize - threshold)

noteBottomOfStack:

Without this, checks against stack space within Clang don't work as
Clang doesn't know where the stack begins.

Needed per-thread, as early as possible.
(on threads using Clang)

Using Clang's desired stack size:

Additionally increase stack size of pthreads to Clang's desired size.

This is presently 8MB, and is used by Clang's stack management
mechanisms to check* if close to stack exhaustion when determining if
there's sufficient space (and warn or run on a new thread with more).
(see runWithSufficientStackSpace)

The constant is available in LLVM 7 onwards.

* (abs(cur - bottom) > DesiredStackSize - threshold)
@dtzWill
Copy link
Author

dtzWill commented Apr 1, 2022

  • Didn't add to the windows spawnThread, if that's desired (and otherwise this seems good) I can take a look! 👍
  • main maybe doesn't need to noteBottomOfStack (if it never calls into clang on that thread), but it doesn't hurt.
  • Reviewing the various threads spawned elsewhere (via std::thread) made it seem this (spawnThread) was the best point to add this, but it's maybe not ideal.

This fixes crashes encountered pointing ccls at CIRCT (+llvm submodule), which was my motivation. I haven't looked into making that reproducible (on other systems), so not sure if that's specific to my setup. Stack was very deep (just shy of 10k frames) and included many calls to runWithSufficientStackSpace that were doing nothing. For some reason this actually took down my ccls instance (despite CrashRecoveryContext/runSafely), which made it particularly problematic.

@MaskRay MaskRay force-pushed the master branch 3 times, most recently from db890d4 to cc13ced Compare November 6, 2024 05:57
@MaskRay MaskRay force-pushed the master branch 3 times, most recently from e403622 to cfc7c02 Compare August 15, 2025 06:45
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.

1 participant