Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make the stack handling more robust to sanitizers and -O3 #6143

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

eric-wieser
Copy link
Contributor

@eric-wieser eric-wieser commented Nov 20, 2024

This PR should make lean better-behaved around sanitizers, per google/sanitizers#1688.
As far as I can tell, https://github.com/google/sanitizers/wiki/AddressSanitizerUseAfterReturn#algorithm replaces local variables with heap allocations, and so taking the address of a local is not effective at producing a monotonic measure of stack usage.

The approach used here is the same as the one used by clang.

This should make lean better-behaved around sanitizers, per google/sanitizers#1688.

The approach used here is the same as the one used by clang
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Nov 20, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Nov 20, 2024

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase b30903d1fcae504013d493c8d15f368a62e0e89e --onto 4600bb16fcded0356d20ae232e7f8580c56a5955. (2024-11-20 22:14:43)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase b30903d1fcae504013d493c8d15f368a62e0e89e --onto 7fbe8e3b36faabc9cfcc45e65a4b7ef042c0b068. (2024-11-21 10:17:47)

@hargoniX hargoniX added the release-ci Enable all CI checks for a PR, like is done for releases label Nov 21, 2024
@eric-wieser eric-wieser changed the title fix: make the stack handling more robust to sanitizers fix: make the stack handling more robust to sanitizers and -O3 Nov 21, 2024
@eric-wieser
Copy link
Contributor Author

I guess this should be changelog-other?

@Kha Kha added the changelog-compiler Compiler, runtime, and FFI label Nov 22, 2024
@Kha
Copy link
Member

Kha commented Nov 22, 2024

changelog-compiler mentions "runtime", let's go with that :)

@Kha Kha added this pull request to the merge queue Nov 22, 2024
Merged via the queue into leanprover:master with commit 5adcd52 Nov 22, 2024
29 of 34 checks passed
JovanGerb pushed a commit to JovanGerb/lean4 that referenced this pull request Jan 21, 2025
…nprover#6143)

This PR should make lean better-behaved around sanitizers, per
google/sanitizers#1688.
As far as I can tell,
https://github.com/google/sanitizers/wiki/AddressSanitizerUseAfterReturn#algorithm
replaces local variables with heap allocations, and so taking the
address of a local is not effective at producing a monotonic measure of
stack usage.

The approach used here is the same as the one used by clang.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-compiler Compiler, runtime, and FFI release-ci Enable all CI checks for a PR, like is done for releases toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants