Skip to content

Commit

Permalink
util/deephash: fix TestArrayAllocs
Browse files Browse the repository at this point in the history
Unfortunately this test fails on certain architectures.
The problem comes down to inconsistencies in the Go escape analysis
where specific variables are marked as escaping on certain architectures.
The variables escaping to the heap are unfortunately in crypto/sha256,
which makes it impossible to fixthis locally in deephash.

For now, fix the test by compensating for the allocations that
occur from calling sha256.digest.Sum.

See golang/go#48055

Fixes #2727

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
  • Loading branch information
dsnet committed Aug 30, 2021
1 parent ffd2205 commit d8b9a0c
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions util/deephash/deephash_test.go
Expand Up @@ -8,9 +8,11 @@ import (
"archive/tar"
"bufio"
"bytes"
"crypto/sha256"
"fmt"
"math"
"reflect"
"runtime"
"testing"

"inet.af/netaddr"
Expand Down Expand Up @@ -332,15 +334,30 @@ func TestArrayAllocs(t *testing.T) {
if version.IsRace() {
t.Skip("skipping test under race detector")
}

// In theory, there should be no allocations. However, escape analysis on
// certain architectures fails to detect that certain cases do not escape.
// This discrepency currently affects sha256.digest.Sum.
// See https://golang.org/issue/48055.
var b []byte
h := sha256.New()
want := int(testing.AllocsPerRun(1000, func() {
b = h.Sum(b[:0])
}))
switch runtime.GOARCH {
case "amd64", "arm64":
want = 0 // ensure no allocations on popular architectures
}

type T struct {
X [32]byte
}
x := &T{X: [32]byte{1: 1, 2: 2, 3: 3, 4: 4}}
n := int(testing.AllocsPerRun(1000, func() {
got := int(testing.AllocsPerRun(1000, func() {
sink = Hash(x)
}))
if n > 0 {
t.Errorf("allocs = %v; want 0", n)
if got > want {
t.Errorf("allocs = %v; want %v", got, want)
}
}

Expand Down

0 comments on commit d8b9a0c

Please sign in to comment.