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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: unnecessary bounds check with slice offset #66691

Open
dsnet opened this issue Apr 5, 2024 · 2 comments
Open

cmd/compile: unnecessary bounds check with slice offset #66691

dsnet opened this issue Apr 5, 2024 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Unplanned

Comments

@dsnet
Copy link
Member

dsnet commented Apr 5, 2024

Go version

go1.22

Output of go env in your module/workspace:

GOARCH=amd64
GOOS=linux

What did you do?

Compile the following:

package main

import "encoding/binary"

var src []byte
var dst uint64

func main() {
	const offset = 1
	if len(src) < offset+8 {
		return
	}
	dst = binary.BigEndian.Uint64(src[offset:])
}

What did you see happen?

I see the following in the assembly:

	0x003f 00063 (/usr/local/go1.22.0/src/encoding/binary/binary.go:183)	MOVL	$7, AX
	0x0044 00068 (/usr/local/go1.22.0/src/encoding/binary/binary.go:183)	PCDATA	$1, $0
	0x0044 00068 (/usr/local/go1.22.0/src/encoding/binary/binary.go:183)	CALL	runtime.panicIndex(SB)
	0x0049 00073 (/usr/local/go1.22.0/src/encoding/binary/binary.go:183)	XCHGL	AX, AX

What did you expect to see?

No bounds check.

The bounds check is avoided if offset is 0, but fails once it is non-zero.

However, this provably safe so long as offset+8 does not overflow an int.
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 5, 2024
@dsnet
Copy link
Member Author

dsnet commented Apr 5, 2024

Interesting. If I rewrite the conditional as len(src)-offset < 8, the compiler is happy again.

@dsnet dsnet mentioned this issue Apr 5, 2024
Open
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 5, 2024
@dmitshur dmitshur added this to the Backlog milestone Apr 5, 2024
@dmitshur
Copy link
Contributor

dmitshur commented Apr 5, 2024

CC @golang/compiler.

@mknyszek mknyszek modified the milestones: Backlog, Unplanned Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
Go Compiler / Runtime
Status: No status
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @dsnet @gopherbot