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

crypto/rand: Read argument escapes to heap #66779

Open
FiloSottile opened this issue Apr 11, 2024 · 3 comments · May be fixed by #66801
Open

crypto/rand: Read argument escapes to heap #66779

FiloSottile opened this issue Apr 11, 2024 · 3 comments · May be fixed by #66801
Assignees
@mateusz834
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted.
Milestone
Backlog

Comments

@FiloSottile
Copy link
Contributor

Currently, Read is implemented as io.ReadFull(Reader, b) which escape analysis can't track, so b escapes to the heap.

This is somewhat annoying, since it causes allocations in paths that could otherwise have none.

It should be possible to rewrite the package so that it doesn't escape on major platforms.
@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 11, 2024
@FiloSottile FiloSottile added this to the Backlog milestone Apr 11, 2024
@mateusz834
Copy link
Member

I' ve been investigating this once, and the remaining problem was that the rand.Reader is an interface and cases like:

var Reader io.Reader = &someStruct{}

will always cause the buffer passed to rand.Reader.Read to be escaped.

@mateusz834
Copy link
Member

But i guess we can make the rand.Read function escape-free.

@mateusz834 mateusz834 self-assigned this Apr 12, 2024
@mateusz834 mateusz834 linked a pull request Apr 12, 2024 that will close this issue
Open
@mateusz834 mateusz834 added FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Apr 12, 2024
@gopherbot
Copy link

Change https://go.dev/cl/578516 mentions this issue: crypto/rand: make Read not escape the byte slice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mateusz834 mateusz834

Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

crypto/rand: make Read not escape the byte slice mateusz834/go
3 participants
@FiloSottile @gopherbot @mateusz834