From 7ebf06fe8e8e75fbab67c185a573531fadb007d6 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Sun, 16 Aug 2026 13:44:05 +0700 Subject: [PATCH 1/6] keccak: test that the permutation kernel stays preemptible #32 removed NOSPLIT from the amd64 kernel so the runtime regains a preemption point in the absorb and squeeze loops. Nothing stops it coming back, and the failure is invisible in throughput benchmarks: it only shows up as a stop-the-world pause in whatever else the process is doing. TestKernelKeepsStackCheck reads the TEXT directives and rejects both ways of losing the stack-growth check: NOSPLIT, and a frame under 128 bytes, which makes the assembler mark a leaf NOSPLIT on its own. It covers both architectures from any runner, so an arm64 CI job still catches an amd64 regression. TestSTWPauseWhileHashing measures the property itself from the runtime's /sched/pauses/stopping/gc:seconds histogram. Its budget is relative: it hashes the same buffer through the pure-Go fallback first, which always has a preemption point, and only fails if the assembly path stalls the world four times longer than that and past 5ms. A slow or loaded runner moves both numbers together; only a real regression separates them. Verified by putting NOSPLIT back, assembly against the pure-Go floor: amd64 (EPYC 4344P) 20.48 us -> 83.886 ms arm64 (M4 Max) 327.68 us -> 100.663 ms, floor 163.84 us --- keccak_asm_test.go | 150 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/keccak_asm_test.go b/keccak_asm_test.go index 44a8f64..adbd976 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -4,7 +4,15 @@ package keccak import ( "math/rand" + "os" + "path/filepath" + "regexp" + "runtime" + "runtime/metrics" + "strconv" + "strings" "testing" + "time" ) // TestXorAndPermute verifies the fused XOR+permute assembly entry point @@ -34,3 +42,145 @@ func TestXorAndPermute(t *testing.T) { } } } + +// The permutation kernel's stack-growth check is the only preemption point in +// the absorb and squeeze loops: those loops call nothing else, and assembly is +// never async-preemptible (runtime/preempt.go, isAsyncSafePoint rejects any +// FuncFlagAsm frame). Two ways to lose it: +// +// NOSPLIT drops the check whatever the frame size +// a frame under 128 bytes makes the assembler mark a leaf NOSPLIT itself +// (abi.StackSmall, obj6.go/obj7.go) +// +// Either one lets a single large Sum256 or Write hold every P in +// stop-the-world for the length of the hash. +var kernelTEXT = regexp.MustCompile(`^TEXT\s+·(keccakF1600\w*)\(SB\),\s*(?:([A-Z0-9|+]+),\s*)?\$(\d+)-\d+`) + +func TestKernelKeepsStackCheck(t *testing.T) { + files, err := filepath.Glob("*.s") + if err != nil || len(files) == 0 { + t.Fatalf("no assembly files found: %v", err) + } + found := 0 + for _, f := range files { + src, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(string(src), "\n") { + m := kernelTEXT.FindStringSubmatch(line) + if m == nil { + continue + } + found++ + name, flags, frame := m[1], m[2], m[3] + where := f + ":" + strconv.Itoa(i+1) + if strings.Contains(flags, "NOSPLIT") { + t.Errorf("%s: %s is NOSPLIT, which drops the stack-growth check. "+ + "That check is the only preemption point in the block loops calling it; "+ + "without it a large Sum256 blocks every stop-the-world for the whole hash.", + where, name) + } + if n, _ := strconv.Atoi(frame); n < 128 { + t.Errorf("%s: %s has a %d-byte frame, under abi.StackSmall (128). The "+ + "assembler marks a leaf that small NOSPLIT itself, which drops the "+ + "stack-growth check. Keep the frame at 128 bytes or more.", where, name, n) + } + } + } + if found == 0 { + t.Fatal("no keccakF1600 TEXT directive matched; was the kernel renamed?") + } +} + +func stopTheWorldHistogram() []uint64 { + s := []metrics.Sample{{Name: "/sched/pauses/stopping/gc:seconds"}} + metrics.Read(s) + h := s[0].Value.Float64Histogram() + out := make([]uint64, len(h.Counts)) + copy(out, h.Counts) + return out +} + +// maxStopTheWorldSince reports the largest stop-the-world stopping pause +// recorded since the given histogram snapshot. +func maxStopTheWorldSince(before []uint64) time.Duration { + s := []metrics.Sample{{Name: "/sched/pauses/stopping/gc:seconds"}} + metrics.Read(s) + h := s[0].Value.Float64Histogram() + var worst float64 + for i := range h.Counts { + if h.Counts[i] > before[i] { + worst = h.Buckets[i+1] + } + } + return time.Duration(worst * float64(time.Second)) +} + +// pauseWhileHashing hashes buf a few times under GC pressure and reports the +// worst stop-the-world stopping pause the runtime recorded. +func pauseWhileHashing(buf []byte) time.Duration { + var garbage []byte + stop := make(chan struct{}) + done := make(chan struct{}) + go func() { // keep asking for a stop-the-world + defer close(done) + for { + select { + case <-stop: + return + default: + } + garbage = make([]byte, 1<<20) + runtime.GC() + time.Sleep(time.Millisecond) + } + }() + time.Sleep(200 * time.Millisecond) + + before := stopTheWorldHistogram() + for range 4 { + Sum256(buf) + } + time.Sleep(300 * time.Millisecond) // let a blocked stop-the-world finish and be recorded + + got := maxStopTheWorldSince(before) + close(stop) + <-done + _ = garbage + return got +} + +// TestSTWPauseWhileHashing measures the property the TEXT flags only imply: how +// long the runtime waits for a hashing goroutine to yield. +// +// The budget is relative to the same hash computed through the pure-Go fallback, +// which always has a preemption point, so a slow or noisy machine moves both +// numbers together and only a real regression separates them. +func TestSTWPauseWhileHashing(t *testing.T) { + if testing.Short() { + t.Skip("hashes 32 MiB") + } + if !useASM { + t.Skip("hardware acceleration unavailable on this CPU") + } + if runtime.GOMAXPROCS(0) < 2 { + t.Skip("needs GOMAXPROCS >= 2 to observe a stopping pause") + } + + buf := make([]byte, 32<<20) + + useASM = false // same call, preemptible implementation + floor := pauseWhileHashing(buf) + useASM = true + got := pauseWhileHashing(buf) + + t.Logf("max GC stop-the-world stopping pause: assembly %v, pure Go %v", got, floor) + + // Healthy is microseconds either way; a missing preemption point is tens of + // milliseconds, which is orders above both the floor and the absolute bound. + if got > 5*time.Millisecond && got > 4*floor { + t.Fatalf("assembly path stalls the world for %v against %v for the pure-Go path: "+ + "the block loop has no preemption point", got, floor) + } +} From 8fce91bbc0bba4efacf92ba783d5ebc9b545d4a5 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Sun, 16 Aug 2026 18:53:27 +0700 Subject: [PATCH 2/6] keccak: time a GC against the pure-Go path instead of reading the histogram The stopping-pause histogram is cumulative for the life of the process, so picking its highest non-empty bucket attributes any earlier pause -- another test, the warm-up -- to the code under test. Taking bucket deltas fixes that but needs a sleep afterwards so a blocked stop-the-world is recorded before the snapshot, which is easy to get wrong. Timing runtime.GC() from another goroutine measures the same thing without a histogram, and the budget is already relative to the pure-Go fallback, so the extra precision was not buying anything. Verified both ways on both architectures. Healthy: 327us assembly against 668us pure Go on M4 Max, 416us against 526us on EPYC 4344P. With NOSPLIT put back: 91.9ms against 1.1ms, and 73.8ms against 596us. Matches the same test in erigon and klauspost/compress. --- keccak_asm_test.go | 98 ++++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 69 deletions(-) diff --git a/keccak_asm_test.go b/keccak_asm_test.go index adbd976..69ded55 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "regexp" "runtime" - "runtime/metrics" "strconv" "strings" "testing" @@ -93,94 +92,55 @@ func TestKernelKeepsStackCheck(t *testing.T) { } } -func stopTheWorldHistogram() []uint64 { - s := []metrics.Sample{{Name: "/sched/pauses/stopping/gc:seconds"}} - metrics.Read(s) - h := s[0].Value.Float64Histogram() - out := make([]uint64, len(h.Counts)) - copy(out, h.Counts) - return out -} - -// maxStopTheWorldSince reports the largest stop-the-world stopping pause -// recorded since the given histogram snapshot. -func maxStopTheWorldSince(before []uint64) time.Duration { - s := []metrics.Sample{{Name: "/sched/pauses/stopping/gc:seconds"}} - metrics.Read(s) - h := s[0].Value.Float64Histogram() - var worst float64 - for i := range h.Counts { - if h.Counts[i] > before[i] { - worst = h.Buckets[i+1] - } - } - return time.Duration(worst * float64(time.Second)) -} - -// pauseWhileHashing hashes buf a few times under GC pressure and reports the -// worst stop-the-world stopping pause the runtime recorded. -func pauseWhileHashing(buf []byte) time.Duration { - var garbage []byte - stop := make(chan struct{}) +// gcWait reports the longest a GC had to wait while work ran in another +// goroutine. Work the runtime cannot preempt shows up here directly. +func gcWait(work func()) time.Duration { done := make(chan struct{}) - go func() { // keep asking for a stop-the-world - defer close(done) - for { - select { - case <-stop: - return - default: - } - garbage = make([]byte, 1<<20) - runtime.GC() - time.Sleep(time.Millisecond) + go func() { defer close(done); work() }() + var worst time.Duration + for { + select { + case <-done: + return worst + default: } - }() - time.Sleep(200 * time.Millisecond) - - before := stopTheWorldHistogram() - for range 4 { - Sum256(buf) + start := time.Now() + runtime.GC() + worst = max(worst, time.Since(start)) } - time.Sleep(300 * time.Millisecond) // let a blocked stop-the-world finish and be recorded - - got := maxStopTheWorldSince(before) - close(stop) - <-done - _ = garbage - return got } // TestSTWPauseWhileHashing measures the property the TEXT flags only imply: how // long the runtime waits for a hashing goroutine to yield. // // The budget is relative to the same hash computed through the pure-Go fallback, -// which always has a preemption point, so a slow or noisy machine moves both +// which always has a preemption point, so a slow or loaded runner moves both // numbers together and only a real regression separates them. func TestSTWPauseWhileHashing(t *testing.T) { - if testing.Short() { - t.Skip("hashes 32 MiB") + if testing.Short() || runtime.GOMAXPROCS(0) < 2 { + t.Skip("hashes 32 MiB, and needs GOMAXPROCS >= 2 to see a stalled GC") } if !useASM { t.Skip("hardware acceleration unavailable on this CPU") } - if runtime.GOMAXPROCS(0) < 2 { - t.Skip("needs GOMAXPROCS >= 2 to observe a stopping pause") - } - - buf := make([]byte, 32<<20) + buf := make([]byte, 32*1024*1024) useASM = false // same call, preemptible implementation - floor := pauseWhileHashing(buf) + floor := gcWait(func() { + for range 4 { + Sum256(buf) + } + }) useASM = true - got := pauseWhileHashing(buf) - - t.Logf("max GC stop-the-world stopping pause: assembly %v, pure Go %v", got, floor) + got := gcWait(func() { + for range 4 { + Sum256(buf) + } + }) - // Healthy is microseconds either way; a missing preemption point is tens of - // milliseconds, which is orders above both the floor and the absolute bound. + t.Logf("worst GC wait: assembly %v, pure Go %v", got, floor) if got > 5*time.Millisecond && got > 4*floor { - t.Fatalf("assembly path stalls the world for %v against %v for the pure-Go path: "+ + t.Fatalf("a GC waited %v on the assembly path against %v on the pure-Go path: "+ "the block loop has no preemption point", got, floor) } } From e95505caa79126f5fb30ac6aafc314fbccf4e588 Mon Sep 17 00:00:00 2001 From: Alex Sharov Date: Mon, 17 Aug 2026 14:54:45 +0700 Subject: [PATCH 3/6] =?UTF-8?q?Revert=20"Revert=20"keccakf=5Famd64:=20drop?= =?UTF-8?q?=20NOSPLIT=20so=20the=20runtime=20can=20preempt=20the=20bl?= =?UTF-8?q?=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9d4ac7db2e857bbd3019a0ae91046dea4d7ec4dc. --- gen_keccakf_bmi2.go | 7 ++++++- keccakf_amd64_bmi2.s | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/gen_keccakf_bmi2.go b/gen_keccakf_bmi2.go index 1799cd6..b28a7d4 100644 --- a/gen_keccakf_bmi2.go +++ b/gen_keccakf_bmi2.go @@ -79,8 +79,13 @@ func main() { // Single function: keccakF1600BMI2(a *[200]byte, buf *byte) // When buf != nil, XORs rate bytes into state before permuting. // When buf == nil, just permutes. + // Do NOT add NOSPLIT here. The frame is unused, but its stack check is + // the only preemption point in the callers' block loops: assembly is not + // async-preemptible, so without it a large Sum256 or Write holds every P + // in stop-the-world until it finishes. The arm64 kernel omits NOSPLIT for + // the same reason. p("// func keccakF1600BMI2(a *[200]byte, buf *byte)") - p("TEXT ·keccakF1600BMI2(SB), NOSPLIT, $%d-16", fsize) + p("TEXT ·keccakF1600BMI2(SB), $%d-16", fsize) p("\tMOVQ a+0(FP), DI") p("\tMOVQ buf+8(FP), BX") p("\tTESTQ BX, BX") diff --git a/keccakf_amd64_bmi2.s b/keccakf_amd64_bmi2.s index 272bb12..ba25fcf 100644 --- a/keccakf_amd64_bmi2.s +++ b/keccakf_amd64_bmi2.s @@ -5,7 +5,7 @@ #include "textflag.h" // func keccakF1600BMI2(a *[200]byte, buf *byte) -TEXT ·keccakF1600BMI2(SB), NOSPLIT, $200-16 +TEXT ·keccakF1600BMI2(SB), $200-16 MOVQ a+0(FP), DI MOVQ buf+8(FP), BX TESTQ BX, BX From 507d55111612e998f2dab40d3b6748fe90c8df53 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Fri, 28 Aug 2026 10:05:10 +0700 Subject: [PATCH 4/6] keccak: check every asm file has a guarded kernel Also fix the generator comment: the amd64 frame holds the round temp state, it is not unused. --- gen_keccakf_bmi2.go | 5 +++-- keccak_asm_test.go | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/gen_keccakf_bmi2.go b/gen_keccakf_bmi2.go index b28a7d4..ea48924 100644 --- a/gen_keccakf_bmi2.go +++ b/gen_keccakf_bmi2.go @@ -79,8 +79,9 @@ func main() { // Single function: keccakF1600BMI2(a *[200]byte, buf *byte) // When buf != nil, XORs rate bytes into state before permuting. // When buf == nil, just permutes. - // Do NOT add NOSPLIT here. The frame is unused, but its stack check is - // the only preemption point in the callers' block loops: assembly is not + // Do NOT add NOSPLIT here, and do not shrink the frame below 128 bytes. + // The frame holds the round temp state, and its stack check is the only + // preemption point in the callers' block loops: assembly is not // async-preemptible, so without it a large Sum256 or Write holds every P // in stop-the-world until it finishes. The arm64 kernel omits NOSPLIT for // the same reason. diff --git a/keccak_asm_test.go b/keccak_asm_test.go index 69ded55..bc859c9 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -60,12 +60,12 @@ func TestKernelKeepsStackCheck(t *testing.T) { if err != nil || len(files) == 0 { t.Fatalf("no assembly files found: %v", err) } - found := 0 for _, f := range files { src, err := os.ReadFile(f) if err != nil { t.Fatal(err) } + found := 0 for i, line := range strings.Split(string(src), "\n") { m := kernelTEXT.FindStringSubmatch(line) if m == nil { @@ -86,9 +86,10 @@ func TestKernelKeepsStackCheck(t *testing.T) { "stack-growth check. Keep the frame at 128 bytes or more.", where, name, n) } } - } - if found == 0 { - t.Fatal("no keccakF1600 TEXT directive matched; was the kernel renamed?") + if found == 0 { + t.Errorf("%s: no keccakF1600 TEXT directive matched, so this kernel is "+ + "unguarded; was it renamed?", f) + } } } From 5f0b3f830dac5263f73182f20ca38b6d11961b5c Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 2 Sep 2026 11:52:30 +0700 Subject: [PATCH 5/6] keccak: reject the whole flag field, not the NOSPLIT spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit textflag.h defines NOSPLIT as 4 and the assembler accepts the number, so `TEXT ·keccakF1600BMI2(SB), 4, $200-16` dropped the stack check while the guard stayed green - objdump shows zero morestack references. --- keccak_asm_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/keccak_asm_test.go b/keccak_asm_test.go index bc859c9..823a9c0 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -74,11 +74,15 @@ func TestKernelKeepsStackCheck(t *testing.T) { found++ name, flags, frame := m[1], m[2], m[3] where := f + ":" + strconv.Itoa(i+1) - if strings.Contains(flags, "NOSPLIT") { - t.Errorf("%s: %s is NOSPLIT, which drops the stack-growth check. "+ - "That check is the only preemption point in the block loops calling it; "+ - "without it a large Sum256 blocks every stop-the-world for the whole hash.", - where, name) + // Reject the whole flag field, not the spelling: textflag.h + // defines NOSPLIT as 4 and the assembler takes the number, so + // matching the name alone lets the regression back in. + if flags != "" { + t.Errorf("%s: %s carries assembler flags %q. NOSPLIT (spelled, or as its "+ + "numeric value 4) drops the stack-growth check, which is the only "+ + "preemption point in the block loops calling it; without it a large "+ + "Sum256 blocks every stop-the-world for the whole hash.", + where, name, flags) } if n, _ := strconv.Atoi(frame); n < 128 { t.Errorf("%s: %s has a %d-byte frame, under abi.StackSmall (128). The "+ From 2eb827bd87356a01038ff858d17489dfe2439e01 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 2 Sep 2026 12:06:16 +0700 Subject: [PATCH 6/6] keccak: guard every assembly symbol, not the keccakF1600 name Matching a name meant a renamed kernel matched nothing, and the test reported the rename rather than the missing check. Match any TEXT symbol: these files have no symbol that may keep NOSPLIT. --- keccak_asm_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/keccak_asm_test.go b/keccak_asm_test.go index 823a9c0..174daf6 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -53,7 +53,7 @@ func TestXorAndPermute(t *testing.T) { // // Either one lets a single large Sum256 or Write hold every P in // stop-the-world for the length of the hash. -var kernelTEXT = regexp.MustCompile(`^TEXT\s+·(keccakF1600\w*)\(SB\),\s*(?:([A-Z0-9|+]+),\s*)?\$(\d+)-\d+`) +var kernelTEXT = regexp.MustCompile(`^TEXT\s+·(\w+)\(SB\),\s*(?:([A-Z0-9|+]+),\s*)?\$(\d+)-\d+`) func TestKernelKeepsStackCheck(t *testing.T) { files, err := filepath.Glob("*.s") @@ -91,8 +91,8 @@ func TestKernelKeepsStackCheck(t *testing.T) { } } if found == 0 { - t.Errorf("%s: no keccakF1600 TEXT directive matched, so this kernel is "+ - "unguarded; was it renamed?", f) + t.Errorf("%s: no TEXT directive matched, so nothing in this file is "+ + "guarded; was the frame size turned into a #define?", f) } } }