[yperbasis]

Summary

• give fallback hashers reusable digest storage
• remove the escaping 32-byte temporary from repeated Hasher.Sum256 calls
• enforce zero allocations for native, pure-Go, and runtime-fallback paths

Why

Passing a local [32]byte through the stored hash.Hash interface makes the compiler move it to the heap. Repeated fallback Sum256 calls therefore allocate 32 bytes each.

The runtime fallback reuses the otherwise inactive native sponge buffer. The pure-Go implementation keeps a 32-byte digest buffer in Hasher.

TDD

Both fallback variants initially failed their allocation test:

Hasher.Sum256 allocations = 1, want 0

Benchmark

Apple M2 Max, Go 1.25.7, forced runtime fallback, representative results:

before   362.1 ns/op   32 B/op   1 alloc/op
after    348.9 ns/op    0 B/op   0 allocs/op

Verification

• go test ./...
• go test -race ./...
• go test -tags purego ./...
• go vet ./...
• go mod tidy -diff
• golangci-lint run (twice)
• golangci-lint run --build-tags purego

[Copilot]

Pull request overview

Eliminates repeated fallback Hasher.Sum256 allocations by reusing digest storage.

Changes:

• Adds reusable digest buffers for fallback implementations.
• Adds zero-allocation tests and benchmarks for native, pure-Go, and runtime fallback paths.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
keccak_default.go	Reuses a stored digest buffer.
keccak_asm.go	Reuses the inactive sponge buffer during fallback.
keccak_alloc_test.go	Adds allocation tests and benchmarks.
keccak_alloc_asm_test.go	Exercises forced runtime fallback allocation behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yperbasis]

[P2] Use the slice returned by hash.Hash.Sum

In both keccak_asm.go:191 and keccak_default.go:48, the result of Sum is discarded. The hash.Hash contract guarantees that the digest is present in the returned slice; it does not guarantee that the supplied backing array is reused. This therefore makes correctness depend on the current x/crypto/sha3 implementation. A conforming implementation that allocates a new backing array causes Sum256 to return stale scratch bytes; a targeted contract test reproduced this as an all-zero digest.

Please use the returned slice:

sum := h.xc.Sum(h.buf[:0])
return [32]byte(sum)

and apply the equivalent change to h.h.Sum(h.sum[:0]).

I verified that this contract-safe form remains allocation-free in both forced-fallback and pure-Go benchmarks (0 B/op, 0 allocs/op).

Reviewed at c269a2f97e787182b554d2aa98cc235214224f11.

[AskAlexSharov]

Found

Sum256 became a data race. keccak_asm.go:193 and keccak_default.go:49 — reusing a receiver-held buffer turned a read-only exported method into one that writes through the *Hasher. Four goroutines each reading a digest from one shared hasher:

WARNING: DATA RACE
Write at 0x00c000100b50 by goroutine 7:
  golang.org/x/crypto/sha3.(*state).Sum()   legacy_hash.go:203
  fastkeccak.(*Hasher).Sum256()             keccak_asm.go:193
Previous write at 0x00c000100b50 by goroutine 10:  ... same site

The same test is clean on master. It is invisible on a BMI2 / FEAT_SHA3 box, because the sponge path stays read-only — it hits every purego build, every amd64 without BMI1/BMI2, and every arm64 without FEAT_SHA3.

xcState embedded the KeccakState interface, which promotes only that interface's method set, so *xcState lost MarshalBinary. Merged with #22 that made Clone return errCloneUnsupported for every fallback hasher.

Fixed

The production change is out. The 32 B is inherent: routing a local array through the interface call is what allocates it, and both ways to remove it are worse — a receiver-held buffer races, a buffer inside Hasher heap-allocates the whole ~400-byte struct on the primary path.

What stays is the test that catches either regression. It compares a stack-allocated Hasher's whole lifecycle against a //go:noinline heap-allocated one and requires the stack one to be strictly cheaper, so it needs no per-platform constant (a fixed count cannot work: 0 allocs on the asm path, 2 on the fallback). Verified: putting the buffer back in Hasher makes stack == heap and fails on both builds. 1ab1f60

With this, #22 and #24 now merge cleanly and pass go test, -tags purego and -race.