From 5961134fa5530e8e07b5536b9577e4755ac1e04a Mon Sep 17 00:00:00 2001
From: Joe Tsai <joetsai@digital-static.net>
Date: Mon, 30 Aug 2021 10:25:11 -0700
Subject: [PATCH] crypto: avoid escaping Hash.Sum on generic architectures
For architectures without a specialized implementation (e.g. arm),
the generic implementation allocates because it does:
var block = blockGeneric
which causes the compiler to give up trying to analyze block
even though it is technically only ever one implementation.
Instead of a variable, declare a function that wraps blockGeneric.
We apply this fix to md5, sha1, and sha256,
while sha512 already had the equivalent change.
We add a test to all hashing packages to ensure no allocations.
Credit goes to Cuong Manh Le for more specifically identifying
the problem and Keith Randal for suggesting a concrete solution.
Fixes #48055
Change-Id: I1a6a2e028038e051c83fd72b10a8bf4d210df57d
Reviewed-on: https://go-review.googlesource.com/c/go/+/346209
Trust: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
---
src/crypto/md5/md5_test.go | 14 ++++++++++++++
src/crypto/md5/md5block_generic.go | 4 +++-
src/crypto/sha1/sha1_test.go | 14 ++++++++++++++
src/crypto/sha1/sha1block_generic.go | 4 +++-
src/crypto/sha256/sha256_test.go | 14 ++++++++++++++
src/crypto/sha256/sha256block_generic.go | 4 +++-
src/crypto/sha512/sha512_test.go | 14 ++++++++++++++
7 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/src/crypto/md5/md5_test.go b/src/crypto/md5/md5_test.go
index acd456af215..851e7fb10d4 100644
--- a/src/crypto/md5/md5_test.go
+++ b/src/crypto/md5/md5_test.go
@@ -211,6 +211,20 @@ func TestLargeHashes(t *testing.T) {
}
}
+func TestAllocations(t *testing.T) {
+ in := []byte("hello, world!")
+ out := make([]byte, 0, Size)
+ h := New()
+ n := int(testing.AllocsPerRun(10, func() {
+ h.Reset()
+ h.Write(in)
+ out = h.Sum(out[:0])
+ }))
+ if n > 0 {
+ t.Errorf("allocs = %d, want 0", n)
+ }
+}
+
var bench = New()
var buf = make([]byte, 1024*1024*8+1)
var sum = make([]byte, bench.Size())
diff --git a/src/crypto/md5/md5block_generic.go b/src/crypto/md5/md5block_generic.go
index ea4fbcd0b41..23ed75304f9 100644
--- a/src/crypto/md5/md5block_generic.go
+++ b/src/crypto/md5/md5block_generic.go
@@ -9,4 +9,6 @@ package md5
const haveAsm = false
-var block = blockGeneric
+func block(dig *digest, p []byte) {
+ blockGeneric(dig, p)
+}
diff --git a/src/crypto/sha1/sha1_test.go b/src/crypto/sha1/sha1_test.go
index c3e6010af12..ab43c7792d4 100644
--- a/src/crypto/sha1/sha1_test.go
+++ b/src/crypto/sha1/sha1_test.go
@@ -210,6 +210,20 @@ func TestLargeHashes(t *testing.T) {
}
}
+func TestAllocations(t *testing.T) {
+ in := []byte("hello, world!")
+ out := make([]byte, 0, Size)
+ h := New()
+ n := int(testing.AllocsPerRun(10, func() {
+ h.Reset()
+ h.Write(in)
+ out = h.Sum(out[:0])
+ }))
+ if n > 0 {
+ t.Errorf("allocs = %d, want 0", n)
+ }
+}
+
var bench = New()
var buf = make([]byte, 8192)
diff --git a/src/crypto/sha1/sha1block_generic.go b/src/crypto/sha1/sha1block_generic.go
index feaba5a23ab..105aa318322 100644
--- a/src/crypto/sha1/sha1block_generic.go
+++ b/src/crypto/sha1/sha1block_generic.go
@@ -7,4 +7,6 @@
package sha1
-var block = blockGeneric
+func block(dig *digest, p []byte) {
+ blockGeneric(dig, p)
+}
diff --git a/src/crypto/sha256/sha256_test.go b/src/crypto/sha256/sha256_test.go
index a2794b015db..702aa0b371a 100644
--- a/src/crypto/sha256/sha256_test.go
+++ b/src/crypto/sha256/sha256_test.go
@@ -289,6 +289,20 @@ func TestLargeHashes(t *testing.T) {
}
}
+func TestAllocations(t *testing.T) {
+ in := []byte("hello, world!")
+ out := make([]byte, 0, Size)
+ h := New()
+ n := int(testing.AllocsPerRun(10, func() {
+ h.Reset()
+ h.Write(in)
+ out = h.Sum(out[:0])
+ }))
+ if n > 0 {
+ t.Errorf("allocs = %d, want 0", n)
+ }
+}
+
var bench = New()
var buf = make([]byte, 8192)
diff --git a/src/crypto/sha256/sha256block_generic.go b/src/crypto/sha256/sha256block_generic.go
index 620c048b93c..0f2bf8b2315 100644
--- a/src/crypto/sha256/sha256block_generic.go
+++ b/src/crypto/sha256/sha256block_generic.go
@@ -7,4 +7,6 @@
package sha256
-var block = blockGeneric
+func block(dig *digest, p []byte) {
+ blockGeneric(dig, p)
+}
diff --git a/src/crypto/sha512/sha512_test.go b/src/crypto/sha512/sha512_test.go
index 0e1528fc69f..aea772c7da5 100644
--- a/src/crypto/sha512/sha512_test.go
+++ b/src/crypto/sha512/sha512_test.go
@@ -888,6 +888,20 @@ func TestLargeHashes(t *testing.T) {
}
}
+func TestAllocations(t *testing.T) {
+ in := []byte("hello, world!")
+ out := make([]byte, 0, Size)
+ h := New()
+ n := int(testing.AllocsPerRun(10, func() {
+ h.Reset()
+ h.Write(in)
+ out = h.Sum(out[:0])
+ }))
+ if n > 0 {
+ t.Errorf("allocs = %d, want 0", n)
+ }
+}
+
var bench = New()
var buf = make([]byte, 8192)
--
GitLab