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 issue #11141 broken stack pointer corrector #11150

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

roberth
Copy link
Member

@roberth roberth commented Jul 22, 2024

Motivation

Fixes #11141

The stack pointer corrector assigned the base instead of the top (which resides at the low end of the stack).
Sounds confusing?
Stacks grow downwards, so that's why this mistake happened.
My manual testing did not uncover this in #10835, because it didn't rely on the stack enough.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@roberth roberth requested a review from edolstra as a code owner July 22, 2024 12:51
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jul 22, 2024
@roberth roberth force-pushed the issue-11141-broken-sp-corrector branch from 9b07359 to 380becf Compare July 22, 2024 12:52
@edolstra
Copy link
Member

#11152 should allow us to get rid of the Boehm coroutine hacks altogether, but this fix and the test looks good.

@roberth
Copy link
Member Author

roberth commented Jul 22, 2024

#11152 should allow us to get rid of the Boehm coroutine hacks altogether,

We might need this when multiple threads perform filtering, but not 100% sure.

but this fix and the test looks good.

Unfortunately I've had to disable it because its 2GB memory consumption is too much for our GHA CI, and probably not something we should require from users either.
My attempts to reduce memory consumption all made the test ineffective or flaky in terms of catching the bug.

@edolstra edolstra merged commit babfd0c into NixOS:master Jul 22, 2024
12 checks passed
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-07-22-nix-team-meeting-minutes-163/49544/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evaluation error in flake regression test suite
3 participants