Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: multiple memories live at block start #59973

Closed
randall77 opened this issue May 4, 2023 · 4 comments
Closed

cmd/compile: multiple memories live at block start #59973

randall77 opened this issue May 4, 2023 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.

Comments

@randall77
Copy link
Contributor

package t

import (
	"encoding/binary"
)

type digest struct {
	h   [5]uint32
	x   [chunk]byte
	nx  int
	len uint64
}

const (
	chunk         = 64
	magic         = "sha\x01"
	marshaledSize = len(magic) + 5*4 + chunk + 8
)

func (d *digest) MarshalBinary() ([]byte, error) {
	b := make([]byte, 0, marshaledSize)
	b = append(b, magic...)
	b = binary.BigEndian.AppendUint32(b, d.h[0])
	b = binary.BigEndian.AppendUint32(b, d.h[1])
	b = binary.BigEndian.AppendUint32(b, d.h[2])
	b = binary.BigEndian.AppendUint32(b, d.h[3])
	b = binary.BigEndian.AppendUint32(b, d.h[4])
	b = append(b, d.x[:d.nx]...)
	b = b[:len(b)+len(d.x)-d.nx] // already zero
	b = binary.BigEndian.AppendUint64(b, d.len)
	return b, nil
}

Compile with GOAMD64=v3.
There are 2 live memories at the start of (*digest).MarshalBinary block b13.
The reason this happens is that there is a BSWAP in b13 and a load in a previous block. When we combine the bswap and the load, we put it in b13, not in the previous block. That's incorrect, we can't move the load - the new combined operation must go in the previous block.

This was noticed by @erifan adding a new compiler phase which, as a side effect, crashes when multiple memories are live at once.

@wdvxdr1123 @erifan

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 4, 2023
@gopherbot
Copy link

Change https://go.dev/cl/492616 mentions this issue: cmd/compile: fix bswap/load rewrite rules

@randall77
Copy link
Contributor Author

@gopherbot Please open backport issues.

This bug can lead to compiler crashes which would be hard to work around.
I'm not sure if it could lead to miscompilation. But maybe?

It is GOAMD64=v3 only.

@gopherbot
Copy link

Backport issue(s) opened: #59974 (for 1.19), #59975 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/492895 mentions this issue: Revert "Revert "cmd/compile: enhance tighten pass for memory values""

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

2 participants