crypto/sha256: escape analysis of digest.Sum disagrees on different architectures #48055

dsnet · 2021-08-30T07:51:42Z

It seems to me that the result of escape analysis should be independent of the exact target architecture. This may be an assumption that is totally unfounded as my understanding of the escape analyzer is limited.

To reproduce, try building crypto/sha256 on various architectures

for X in 386 amd64 amd64p32 arm armbe arm64 arm64be ppc64 ppc64le mips mipsle mips64 mips64le mips64p32 mips64p32le ppc riscv riscv64 s390 s390x sparc sparc64 wasm; do
    echo $X
    GOARCH=$X ../../../bin/go build -gcflags="-m" 2>&1 | egrep "d0|tmp"
   echo
done

The following architectures believe that d0 and tmp escape to the heap:

arm
ppc64
mips
mipsle
mips64
mips64le
riscv64

while the following do not:

386
amd64
amd64p32
armbe
arm64
arm64be
ppc64le
mips64p32
mips64p32le
ppc
riscv
s390
s390x
sparc
sparc64
wasm

Using GOARCH=arm as an example, -gcflags="-m=100 prints the relevant information:

./sha256.go:209:5:[1] (*digest).Sum stmt: d0 := *d
./sha256.go:209:2:[1] (*digest).Sum stmt: var d0 digest
./sha256.go:210:7:[1] (*digest).Sum stmt: hash := (*digest).checkSum(d0)
./sha256.go:210:2:[1] (*digest).Sum stmt: var hash [32]byte
./sha256.go:209:2: d0 escapes to heap:
./sha256.go:209:2:   flow: {heap} = &d0:
./sha256.go:209:2:     from d0 (address-of) at ./sha256.go:210:12
./sha256.go:209:2:     from (*digest).checkSum(d0) (call parameter) at ./sha256.go:210:21
./sha256.go:211:2:[1] (*digest).Sum stmt: if d0.is224 { return append(in, hash[:Size224]...) }
./sha256.go:212:3:[1] (*digest).Sum stmt: return append(in, hash[:Size224]...)
./sha256.go:214:2:[1] (*digest).Sum stmt: return append(in, hash[:]...)
./sha256.go:207:22: parameter in leaks to ~r0 with derefs=0:
./sha256.go:207:22:   flow: ~r0 = in:
./sha256.go:207:22:     from append(in, hash[:Size224]...) (call parameter) at ./sha256.go:212:16
./sha256.go:207:22:     from return append(in, hash[:Size224]...) (return) at ./sha256.go:212:3
./sha256.go:207:7: d does not escape
./sha256.go:207:22: leaking param: in to result ~r0 level=0
./sha256.go:209:2: moved to heap: d0

./sha256.go:220:6: tmp escapes to heap:
./sha256.go:220:6:   flow: {heap} = &tmp:
./sha256.go:220:6:     from tmp (address-of) at ./sha256.go:223:22
./sha256.go:220:6:     from tmp[0:56 - len % 64] (slice) at ./sha256.go:223:14
./sha256.go:220:6:     from (*digest).Write(d, tmp[0:56 - len % 64]) (call parameter) at ./sha256.go:223:10
./sha256.go:225:10:[1] (*digest).checkSum stmt: (*digest).Write(d, tmp[0:64 + 56 - len % 64])
./sha256.go:220:6: tmp escapes to heap:
./sha256.go:220:6:   flow: {heap} = &tmp:
./sha256.go:220:6:     from tmp (address-of) at ./sha256.go:225:25
./sha256.go:220:6:     from tmp[0:64 + 56 - len % 64] (slice) at ./sha256.go:225:14
./sha256.go:220:6:     from (*digest).Write(d, tmp[0:64 + 56 - len % 64]) (call parameter) at ./sha256.go:225:10

The tangible effect of this discrepancy is that sha256.digest.Sum always allocates on certain architectures.

The text was updated successfully, but these errors were encountered:

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>

cuonglm · 2021-08-30T11:00:59Z

This is because on these systems:

arm
ppc64
mips
mipsle
mips64
mips64le
riscv64

They use the generic version, which make the *digest escape to heap.

Other architectures has their own implementation, which does not, e.g, for arm64

go/src/crypto/sha256/sha256block_arm64.go

Line 12 in 56c3856

func sha256block(h []uint32, p []byte, k []uint32)

randall77 · 2021-08-30T16:32:44Z

This is kind of unfortunate, because the digest clearly does not escape in blockGeneric. The problem is in sha256block_generic.go, where we do

var block = blockGeneric

That makes callers of block not know the target function, and thus has to assume escaping arguments.

It would be better to use a wrapper

func block(dig *digest, p []byte) { blockGeneric(dig, p) }

The wrapper will get inlined, and escape analysis will see the flow properly.

dsnet · 2021-08-30T17:06:19Z

Thanks @cuonglm and @randall77. It seems the issue is more specific to crypto/sha256 than it is to cmd/compile. Renaming the issue.

gopherbot · 2021-08-30T17:31:11Z

Change https://golang.org/cl/346209 mentions this issue: crypto/sha256: avoid escaping on generic architectures

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>

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> (cherry picked from commit 3f1317e)

dsnet added the Performance label Aug 30, 2021

dsnet mentioned this issue Aug 30, 2021

util/deephash: fix TestArrayAllocs tailscale/tailscale#2731

Merged

dsnet changed the title ~~cmd/compile: escape analysis disagrees on different architectures~~ crypto/sha256: escape analysis of digest.Sum disagrees on different architectures Aug 30, 2021

cuonglm self-assigned this Aug 30, 2021

cuonglm removed their assignment Aug 30, 2021

cherrymui added this to the Unplanned milestone Aug 31, 2021

cherrymui added the NeedsFix label Aug 31, 2021

gopherbot closed this as completed in 5961134 Sep 23, 2021

golang locked and limited conversation to collaborators Sep 23, 2022

gopherbot added the FrozenDueToAge label Sep 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto/sha256: escape analysis of digest.Sum disagrees on different architectures #48055

crypto/sha256: escape analysis of digest.Sum disagrees on different architectures #48055

dsnet commented Aug 30, 2021 •

edited

Loading

cuonglm commented Aug 30, 2021

randall77 commented Aug 30, 2021

dsnet commented Aug 30, 2021 •

edited

Loading

gopherbot commented Aug 30, 2021

crypto/sha256: escape analysis of digest.Sum disagrees on different architectures #48055

crypto/sha256: escape analysis of digest.Sum disagrees on different architectures #48055

Comments

dsnet commented Aug 30, 2021 • edited Loading

cuonglm commented Aug 30, 2021

randall77 commented Aug 30, 2021

dsnet commented Aug 30, 2021 • edited Loading

gopherbot commented Aug 30, 2021

dsnet commented Aug 30, 2021 •

edited

Loading

dsnet commented Aug 30, 2021 •

edited

Loading