From 380becf0dbbf700c32aaa2e574cc8e05b6411056 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 22 Jul 2024 14:48:18 +0200 Subject: [PATCH 1/2] Fix #11141 broken sp corrector --- src/libexpr/eval-gc.cc | 2 +- tests/functional/lang-gc.sh | 34 ++++++++++ .../lang-gc/issue-11141-gc-coroutine-test.nix | 65 +++++++++++++++++++ tests/functional/local.mk | 1 + 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 tests/functional/lang-gc.sh create mode 100644 tests/functional/lang-gc/issue-11141-gc-coroutine-test.nix diff --git a/src/libexpr/eval-gc.cc b/src/libexpr/eval-gc.cc index baf9df332..914d48da8 100644 --- a/src/libexpr/eval-gc.cc +++ b/src/libexpr/eval-gc.cc @@ -121,7 +121,7 @@ void fixupBoehmStackPointer(void ** sp_ptr, void * _pthread_id) // NOTE: We assume the stack grows down, as it does on all architectures we support. // Architectures that grow the stack up are rare. if (sp >= osStackBase || sp < osStackLow) { // lo is outside the os stack - sp = osStackBase; + sp = osStackLow; } } diff --git a/tests/functional/lang-gc.sh b/tests/functional/lang-gc.sh new file mode 100644 index 000000000..8e2383854 --- /dev/null +++ b/tests/functional/lang-gc.sh @@ -0,0 +1,34 @@ +# shellcheck shell=bash + +# Regression tests for the evaluator +# These are not in lang.sh because they generally only need to run in CI, +# whereas lang.sh is often run locally during development + + +source common.sh + +set -o pipefail + +# Regression test for #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, because it didn't rely on the stack enough. +# https://github.com/NixOS/nix/issues/11141 +test_issue_11141() { + mkdir -p "$TEST_ROOT/issue-11141/src" + cp lang-gc/issue-11141-gc-coroutine-test.nix "$TEST_ROOT/issue-11141/" + ( + set +x; + n=10 + echo "populating $TEST_ROOT/issue-11141/src with $((n*100)) files..." + for i in $(seq 0 $n); do + touch "$TEST_ROOT/issue-11141/src/file-$i"{0,1,2,3,4,5,6,7,8,9}{0,1,2,3,4,5,6,7,8,9} + done + ) + + GC_INITIAL_HEAP_SIZE=$((1024 * 1024)) \ + NIX_SHOW_STATS=1 \ + nix eval -vvv\ + -f "$TEST_ROOT/issue-11141/issue-11141-gc-coroutine-test.nix" +} +test_issue_11141 diff --git a/tests/functional/lang-gc/issue-11141-gc-coroutine-test.nix b/tests/functional/lang-gc/issue-11141-gc-coroutine-test.nix new file mode 100644 index 000000000..4f311af75 --- /dev/null +++ b/tests/functional/lang-gc/issue-11141-gc-coroutine-test.nix @@ -0,0 +1,65 @@ + +# Run: +# GC_INITIAL_HEAP_SIZE=$[1024 * 1024] NIX_SHOW_STATS=1 nix eval -f gc-coroutine-test.nix -vvvv + +let + inherit (builtins) + foldl' + isList + ; + + # Generate a tree of numbers, n deep, such that the numbers add up to (1 + salt) * 10^n. + # The salting makes the numbers all different, increasing the likelihood of catching + # any memory corruptions that might be caused by the GC or otherwise. + garbage = salt: n: + if n == 0 + then [(1 + salt)] + else [ + (garbage (10 * salt + 1) (n - 1)) + (garbage (10 * salt - 1) (n - 1)) + (garbage (10 * salt + 2) (n - 1)) + (garbage (10 * salt - 2) (n - 1)) + (garbage (10 * salt + 3) (n - 1)) + (garbage (10 * salt - 3) (n - 1)) + (garbage (10 * salt + 4) (n - 1)) + (garbage (10 * salt - 4) (n - 1)) + (garbage (10 * salt + 5) (n - 1)) + (garbage (10 * salt - 5) (n - 1)) + ]; + + pow = base: n: + if n == 0 + then 1 + else base * (pow base (n - 1)); + + sumNestedLists = l: + if isList l + then foldl' (a: b: a + sumNestedLists b) 0 l + else l; + +in + assert sumNestedLists (garbage 0 3) == pow 10 3; + assert sumNestedLists (garbage 0 6) == pow 10 6; + builtins.foldl' + (a: b: + assert + "${ + builtins.path { + path = ./src; + filter = path: type: + # We're not doing common subexpression elimination, so this reallocates + # the fairly big tree over and over, producing a lot of garbage during + # source filtering, whose filter runs in a coroutine. + assert sumNestedLists (garbage 0 3) == pow 10 3; + true; + } + }" + == "${./src}"; + + # These asserts don't seem necessary, as the lambda value get corrupted first + assert a.okay; + assert b.okay; + { okay = true; } + ) + { okay = true; } + [ { okay = true; } { okay = true; } { okay = true; } ] diff --git a/tests/functional/local.mk b/tests/functional/local.mk index 49ee31284..797002e92 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -23,6 +23,7 @@ nix_tests = \ remote-store.sh \ legacy-ssh-store.sh \ lang.sh \ + lang-gc.sh \ characterisation-test-infra.sh \ experimental-features.sh \ fetchMercurial.sh \ From f2e0cecf34f6e3b23c2499bb65478a620c4340aa Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 22 Jul 2024 17:45:19 +0200 Subject: [PATCH 2/2] tests/functional/lang-gc: Disable for now --- tests/functional/lang-gc.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/functional/lang-gc.sh b/tests/functional/lang-gc.sh index 8e2383854..1746fa4c1 100644 --- a/tests/functional/lang-gc.sh +++ b/tests/functional/lang-gc.sh @@ -9,6 +9,8 @@ source common.sh set -o pipefail +skipTest "Too memory instensive for CI. Attempt to reduce memory usage was unsuccessful, because it made detection of the bug unreliable." + # Regression test for #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.