[go] Revert "crypto/sha1: add WriteString and WriteByte method"
23 views
Skip to first unread message
Gopher Robot (Gerrit)
May 3, 2023, 5:27:21 PM5/3/23
to Ian Lance Taylor, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Bryan Mills, Joel Sing, Roland Shoemaker, Filippo Valsorda, golang-co...@googlegroups.com
Gopher Robot submitted this change.
Approvals:
Ian Lance Taylor: Looks good to me, but someone else must approve; Automatically submit change; Ignore missing or failing TryBot-Result
Ian Lance Taylor: Run TryBots
Bryan Mills: Looks good to me, approved
Objections:
Gopher Robot: TryBots failed
Revert "crypto/sha1: add WriteString and WriteByte method"
This reverts CL 483815
Reason for revert: can cause cgo errors when using boringcrypto.
See #59954.
For #38776
For #59954
Change-Id: I1f7e0fb06b627971811623927e3d98c0fdbc730b
Reviewed-on: https://go-review.googlesource.com/c/go/+/492375
Auto-Submit: Ian Lance Taylor <ia...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
Run-TryBot: Ian Lance Taylor <ia...@golang.org>
Reviewed-by: Ian Lance Taylor <ia...@google.com>
TryBot-Bypass: Ian Lance Taylor <ia...@google.com>
---
M src/crypto/internal/boring/sha.go
M src/crypto/sha1/sha1.go
M src/crypto/sha1/sha1_test.go
M src/crypto/sha1/sha1block.go
M src/crypto/sha1/sha1block_386.s
M src/crypto/sha1/sha1block_amd64.go
M src/crypto/sha1/sha1block_amd64.s
M src/crypto/sha1/sha1block_arm.s
M src/crypto/sha1/sha1block_arm64.go
M src/crypto/sha1/sha1block_arm64.s
M src/crypto/sha1/sha1block_decl.go
M src/crypto/sha1/sha1block_generic.go
M src/crypto/sha1/sha1block_s390x.go
M src/crypto/sha1/sha1block_s390x.s
14 files changed, 46 insertions(+), 183 deletions(-)
diff --git a/src/crypto/internal/boring/sha.go b/src/crypto/internal/boring/sha.go
index f730e40..cf82f3f 100644
--- a/src/crypto/internal/boring/sha.go
+++ b/src/crypto/internal/boring/sha.go
@@ -145,20 +145,6 @@
return len(p), nil
}
-func (h *sha1Hash) WriteString(s string) (int, error) {
- if len(s) > 0 && C._goboringcrypto_SHA1_Update(h.noescapeCtx(), unsafe.Pointer(unsafe.StringData(s)), C.size_t(len(s))) == 0 {
- panic("boringcrypto: SHA1_Update failed")
- }
- return len(s), nil
-}
-
-func (h *sha1Hash) WriteByte(c byte) error {
- if C._goboringcrypto_SHA1_Update(h.noescapeCtx(), unsafe.Pointer(&c), 1) == 0 {
- panic("boringcrypto: SHA1_Update failed")
- }
- return nil
-}
-
func (h0 *sha1Hash) sum(dst []byte) []byte {
h := *h0 // make copy so future Write+Sum is valid
if C._goboringcrypto_SHA1_Final((*C.uint8_t)(noescape(unsafe.Pointer(&h.out[0]))), h.noescapeCtx()) == 0 {
diff --git a/src/crypto/sha1/sha1.go b/src/crypto/sha1/sha1.go
index 19f1767..43ab72a 100644
--- a/src/crypto/sha1/sha1.go
+++ b/src/crypto/sha1/sha1.go
@@ -120,10 +120,18 @@
func (d *digest) BlockSize() int { return BlockSize }
func (d *digest) Write(p []byte) (nn int, err error) {
+ boringUnreachable()
nn = len(p)
d.len += uint64(nn)
- n := fillChunk(d, p)
- p = p[n:]
+ if d.nx > 0 {
+ n := copy(d.x[d.nx:], p)
+ d.nx += n
+ if d.nx == chunk {
+ block(d, d.x[:])
+ d.nx = 0
+ }
+ p = p[n:]
+ }
if len(p) >= chunk {
n := len(p) &^ (chunk - 1)
block(d, p[:n])
@@ -135,55 +143,6 @@
return
}
-func (d *digest) WriteString(s string) (nn int, err error) {
- nn = len(s)
- d.len += uint64(nn)
- n := fillChunk(d, s)
-
- // This duplicates the code in Write, except that it calls
- // blockString rather than block. It would be nicer to pass
- // in a func, but as of this writing (Go 1.20) that causes
- // memory allocations that we want to avoid.
-
- s = s[n:]
- if len(s) >= chunk {
- n := len(s) &^ (chunk - 1)
- blockString(d, s[:n])
- s = s[n:]
- }
- if len(s) > 0 {
- d.nx = copy(d.x[:], s)
- }
- return
-}
-
-// fillChunk fills the remainder of the current chunk, if any.
-func fillChunk[S []byte | string](d *digest, p S) int {
- boringUnreachable()
- if d.nx == 0 {
- return 0
- }
- n := copy(d.x[d.nx:], p)
- d.nx += n
- if d.nx == chunk {
- block(d, d.x[:])
- d.nx = 0
- }
- return n
-}
-
-func (d *digest) WriteByte(c byte) error {
- boringUnreachable()
- d.len++
- d.x[d.nx] = c
- d.nx++
- if d.nx == chunk {
- block(d, d.x[:])
- d.nx = 0
- }
- return nil
-}
-
func (d *digest) Sum(in []byte) []byte {
boringUnreachable()
// Make a copy of d so that caller can keep writing and summing.
diff --git a/src/crypto/sha1/sha1_test.go b/src/crypto/sha1/sha1_test.go
index 2f0980a..85ed126 100644
--- a/src/crypto/sha1/sha1_test.go
+++ b/src/crypto/sha1/sha1_test.go
@@ -92,14 +92,6 @@
}
c.Reset()
}
- bw := c.(io.ByteWriter)
- for i := 0; i < len(g.in); i++ {
- bw.WriteByte(g.in[i])
- }
- s = fmt.Sprintf("%x", c.Sum(nil))
- if s != g.out {
- t.Errorf("sha1[WriteByte](%s) = %s want %s", g.in, s, g.out)
- }
}
}
@@ -229,8 +221,7 @@
if boring.Enabled {
t.Skip("BoringCrypto doesn't allocate the same way as stdlib")
}
- const ins = "hello, world!"
- in := []byte(ins)
+ in := []byte("hello, world!")
out := make([]byte, 0, Size)
h := New()
n := int(testing.AllocsPerRun(10, func() {
@@ -241,28 +232,6 @@
if n > 0 {
t.Errorf("allocs = %d, want 0", n)
}
-
- sw := h.(io.StringWriter)
- n = int(testing.AllocsPerRun(10, func() {
- h.Reset()
- sw.WriteString(ins)
- out = h.Sum(out[:0])
- }))
- if n > 0 {
- t.Errorf("string allocs = %d, want 0", n)
- }
-
- bw := h.(io.ByteWriter)
- n = int(testing.AllocsPerRun(10, func() {
- h.Reset()
- for _, b := range in {
- bw.WriteByte(b)
- }
- out = h.Sum(out[:0])
- }))
- if n > 0 {
- t.Errorf("byte allocs = %d, want 0", n)
- }
}
var bench = New()
diff --git a/src/crypto/sha1/sha1block.go b/src/crypto/sha1/sha1block.go
index 0b33285..1c1a7c5 100644
--- a/src/crypto/sha1/sha1block.go
+++ b/src/crypto/sha1/sha1block.go
@@ -17,7 +17,7 @@
// blockGeneric is a portable, pure Go version of the SHA-1 block step.
// It's used by sha1block_generic.go and tests.
-func blockGeneric[S []byte | string](dig *digest, p S) {
+func blockGeneric(dig *digest, p []byte) {
var w [16]uint32
h0, h1, h2, h3, h4 := dig.h[0], dig.h[1], dig.h[2], dig.h[3], dig.h[4]
diff --git a/src/crypto/sha1/sha1block_386.s b/src/crypto/sha1/sha1block_386.s
index 9421b4e..34d023d 100644
--- a/src/crypto/sha1/sha1block_386.s
+++ b/src/crypto/sha1/sha1block_386.s
@@ -98,11 +98,11 @@
FUNC4(a, b, c, d, e); \
MIX(a, b, c, d, e, 0xCA62C1D6)
-// func doBlock(dig *digest, p *byte, n int)
-TEXT ·doBlock(SB),NOSPLIT,$92-12
+// func block(dig *digest, p []byte)
+TEXT ·block(SB),NOSPLIT,$92-16
MOVL dig+0(FP), BP
MOVL p+4(FP), SI
- MOVL n+8(FP), DX
+ MOVL p_len+8(FP), DX
SHRL $6, DX
SHLL $6, DX
diff --git a/src/crypto/sha1/sha1block_amd64.go b/src/crypto/sha1/sha1block_amd64.go
index 528d65d..039813d 100644
--- a/src/crypto/sha1/sha1block_amd64.go
+++ b/src/crypto/sha1/sha1block_amd64.go
@@ -4,22 +4,19 @@
package sha1
-import (
- "internal/cpu"
- "unsafe"
-)
+import "internal/cpu"
//go:noescape
-func blockAVX2(dig *digest, p *byte, n int)
+func blockAVX2(dig *digest, p []byte)
//go:noescape
-func blockAMD64(dig *digest, p *byte, n int)
+func blockAMD64(dig *digest, p []byte)
var useAVX2 = cpu.X86.HasAVX2 && cpu.X86.HasBMI1 && cpu.X86.HasBMI2
func block(dig *digest, p []byte) {
if useAVX2 && len(p) >= 256 {
- // blockAVX2 calculates sha1 for 2 blocks per iteration
+ // blockAVX2 calculates sha1 for 2 block per iteration
// it also interleaves precalculation for next block.
// So it may read up-to 192 bytes past end of p
// We may add checks inside blockAVX2, but this will
@@ -29,25 +26,9 @@
if safeLen%128 != 0 {
safeLen -= 64
}
- blockAVX2(dig, unsafe.SliceData(p), safeLen)
- pRem := p[safeLen:]
- blockAMD64(dig, unsafe.SliceData(pRem), len(pRem))
+ blockAVX2(dig, p[:safeLen])
+ blockAMD64(dig, p[safeLen:])
} else {
- blockAMD64(dig, unsafe.SliceData(p), len(p))
- }
-}
-
-// blockString is a duplicate of block that takes a string.
-func blockString(dig *digest, s string) {
- if useAVX2 && len(s) >= 256 {
- safeLen := len(s) - 128
- if safeLen%128 != 0 {
- safeLen -= 64
- }
- blockAVX2(dig, unsafe.StringData(s), safeLen)
- sRem := s[safeLen:]
- blockAMD64(dig, unsafe.StringData(sRem), len(sRem))
- } else {
- blockAMD64(dig, unsafe.StringData(s), len(s))
+ blockAMD64(dig, p)
}
}
diff --git a/src/crypto/sha1/sha1block_amd64.s b/src/crypto/sha1/sha1block_amd64.s
index 23b47da..9bdf24c 100644
--- a/src/crypto/sha1/sha1block_amd64.s
+++ b/src/crypto/sha1/sha1block_amd64.s
@@ -96,10 +96,10 @@
FUNC4(a, b, c, d, e); \
MIX(a, b, c, d, e, 0xCA62C1D6)
-TEXT ·blockAMD64(SB),NOSPLIT,$64-24
+TEXT ·blockAMD64(SB),NOSPLIT,$64-32
MOVQ dig+0(FP), BP
- MOVQ p+8(FP), SI
- MOVQ n+16(FP), DX
+ MOVQ p_base+8(FP), SI
+ MOVQ p_len+16(FP), DX
SHRQ $6, DX
SHLQ $6, DX
@@ -1430,11 +1430,11 @@
-TEXT ·blockAVX2(SB),$1408-24
+TEXT ·blockAVX2(SB),$1408-32
MOVQ dig+0(FP), DI
- MOVQ p+8(FP), SI
- MOVQ n+16(FP), DX
+ MOVQ p_base+8(FP), SI
+ MOVQ p_len+16(FP), DX
SHRQ $6, DX
SHLQ $6, DX
diff --git a/src/crypto/sha1/sha1block_arm.s b/src/crypto/sha1/sha1block_arm.s
index db651db..2236533 100644
--- a/src/crypto/sha1/sha1block_arm.s
+++ b/src/crypto/sha1/sha1block_arm.s
@@ -38,10 +38,11 @@
#define Rctr R12 // loop counter
#define Rw R14 // point to w buffer
-// func doBlock(dig *digest, p *byte, n int)
+// func block(dig *digest, p []byte)
// 0(FP) is *digest
// 4(FP) is p.array (struct Slice)
// 8(FP) is p.len
+//12(FP) is p.cap
//
// Stack frame
#define p_end end-4(SP) // pointer to the end of data
@@ -135,10 +136,10 @@
MIX(Ra, Rb, Rc, Rd, Re)
-// func doBlock(dig *digest, p *byte, n int)
-TEXT ·doBlock(SB), 0, $352-12
+// func block(dig *digest, p []byte)
+TEXT ·block(SB), 0, $352-16
MOVW p+4(FP), Rdata // pointer to the data
- MOVW n+8(FP), Rt0 // number of bytes
+ MOVW p_len+8(FP), Rt0 // number of bytes
ADD Rdata, Rt0
MOVW Rt0, p_end // pointer to end of data
diff --git a/src/crypto/sha1/sha1block_arm64.go b/src/crypto/sha1/sha1block_arm64.go
index 846c882..08d3df0 100644
--- a/src/crypto/sha1/sha1block_arm64.go
+++ b/src/crypto/sha1/sha1block_arm64.go
@@ -4,10 +4,7 @@
package sha1
-import (
- "internal/cpu"
- "unsafe"
-)
+import "internal/cpu"
var k = []uint32{
0x5A827999,
@@ -17,22 +14,13 @@
}
//go:noescape
-func sha1block(h []uint32, p *byte, n int, k []uint32)
+func sha1block(h []uint32, p []byte, k []uint32)
func block(dig *digest, p []byte) {
if !cpu.ARM64.HasSHA1 {
blockGeneric(dig, p)
} else {
h := dig.h[:]
- sha1block(h, unsafe.SliceData(p), len(p), k)
- }
-}
-
-func blockString(dig *digest, s string) {
- if !cpu.ARM64.HasSHA1 {
- blockGeneric(dig, s)
- } else {
- h := dig.h[:]
- sha1block(h, unsafe.StringData(s), len(s), k)
+ sha1block(h, p, k)
}
}
diff --git a/src/crypto/sha1/sha1block_arm64.s b/src/crypto/sha1/sha1block_arm64.s
index e5e3243..d568384 100644
--- a/src/crypto/sha1/sha1block_arm64.s
+++ b/src/crypto/sha1/sha1block_arm64.s
@@ -19,12 +19,12 @@
SHA1H V3, V1 \
VMOV V2.B16, V3.B16
-// func sha1block(h []uint32, p *byte, n int, k []uint32)
+// func sha1block(h []uint32, p []byte, k []uint32)
TEXT ·sha1block(SB),NOSPLIT,$0
MOVD h_base+0(FP), R0 // hash value first address
- MOVD p+24(FP), R1 // message first address
- MOVD k_base+40(FP), R2 // k constants first address
- MOVD n+32(FP), R3 // message length
+ MOVD p_base+24(FP), R1 // message first address
+ MOVD k_base+48(FP), R2 // k constants first address
+ MOVD p_len+32(FP), R3 // message length
VLD1.P 16(R0), [V0.S4]
FMOVS (R0), F20
SUB $16, R0, R0
diff --git a/src/crypto/sha1/sha1block_decl.go b/src/crypto/sha1/sha1block_decl.go
index 9ef8709..8e20401 100644
--- a/src/crypto/sha1/sha1block_decl.go
+++ b/src/crypto/sha1/sha1block_decl.go
@@ -6,15 +6,5 @@
package sha1
-import "unsafe"
-
//go:noescape
-func doBlock(dig *digest, p *byte, n int)
-
-func block(dig *digest, p []byte) {
- doBlock(dig, unsafe.SliceData(p), len(p))
-}
-
-func blockString(dig *digest, s string) {
- doBlock(dig, unsafe.StringData(s), len(s))
-}
+func block(dig *digest, p []byte)
diff --git a/src/crypto/sha1/sha1block_generic.go b/src/crypto/sha1/sha1block_generic.go
index 4eb489f..ba35155 100644
--- a/src/crypto/sha1/sha1block_generic.go
+++ b/src/crypto/sha1/sha1block_generic.go
@@ -9,7 +9,3 @@
func block(dig *digest, p []byte) {
blockGeneric(dig, p)
}
-
-func blockString(dig *digest, s string) {
- blockGeneric(dig, s)
-}
diff --git a/src/crypto/sha1/sha1block_s390x.go b/src/crypto/sha1/sha1block_s390x.go
index 06c972d..446bf5d 100644
--- a/src/crypto/sha1/sha1block_s390x.go
+++ b/src/crypto/sha1/sha1block_s390x.go
@@ -4,13 +4,6 @@
package sha1
-import (
- "internal/cpu"
- "unsafe"
-)
+import "internal/cpu"
var useAsm = cpu.S390X.HasSHA1
-
-func doBlockGeneric(dig *digest, p *byte, n int) {
- blockGeneric(dig, unsafe.String(p, n))
-}
diff --git a/src/crypto/sha1/sha1block_s390x.s b/src/crypto/sha1/sha1block_s390x.s
index 3d08234..6ba6883 100644
--- a/src/crypto/sha1/sha1block_s390x.s
+++ b/src/crypto/sha1/sha1block_s390x.s
@@ -4,10 +4,10 @@
#include "textflag.h"
-// func doBlock(dig *digest, p *byte, n int)
-TEXT ·doBlock(SB), NOSPLIT|NOFRAME, $0-24
+// func block(dig *digest, p []byte)
+TEXT ·block(SB), NOSPLIT|NOFRAME, $0-32
MOVBZ ·useAsm(SB), R4
- LMG dig+0(FP), R1, R3 // R2 = p, R3 = n
+ LMG dig+0(FP), R1, R3 // R2 = &p[0], R3 = len(p)
MOVBZ $1, R0 // SHA-1 function code
CMPBEQ R4, $0, generic
@@ -17,4 +17,4 @@
RET
generic:
- BR ·doBlockGeneric(SB)
+ BR ·blockGeneric(SB)
To view, visit change 492375. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages