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

runtime: marked free object in span [1.16 backport] #44659

Closed
gopherbot opened this issue Feb 27, 2021 · 3 comments
Closed

runtime: marked free object in span [1.16 backport] #44659

gopherbot opened this issue Feb 27, 2021 · 3 comments

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 27, 2021

@randall77 requested issue #44614 to be considered for backport to the next 1.16 minor release.

@gopherbot please open backport issues.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Feb 27, 2021

Change https://golang.org/cl/297290 mentions this issue: [release-branch.go1.16] cmd/compile: fix escape analysis of heap-allocated results

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Mar 1, 2021

Approving as a serious issue without a workaround. This backport applies to both 1.16 (this issue) and 1.15 (#44658).

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Mar 1, 2021

Closed by merging 292abd9 to release-branch.go1.16.

@gopherbot gopherbot closed this Mar 1, 2021
gopherbot pushed a commit that referenced this issue Mar 1, 2021
…cated results

One of escape analysis's responsibilities is to summarize whether/how
each function parameter flows to the heap so we can correctly
incorporate those flows into callers' escape analysis data flow
graphs.

As an optimization, we separately record when parameters flow to
result parameters, so that we can more precisely analyze parameter
flows based on how the results are used at the call site. However, if
a named result parameter itself needs to be heap allocated, this
optimization isn't safe and the parameter needs to be recorded as
flowing to heap rather than flowing to result.

Escape analysis used to get this correct because it conservatively
rewalked the data-flow graph multiple times. So even though it would
incorrectly record the result parameter flow, it would separately find
a flow to the heap. However, CL 196811 (specifically, case 3)
optimized the walking logic to reduce unnecessary rewalks causing us
to stop finding the extra heap flow.

This CL fixes the issue by correcting location.leakTo to be sensitive
to sink.escapes and not record result-flows when the result parameter
escapes to the heap.

Fixes #44659.

Change-Id: I48742ed35a6cab591094e2d23a439e205bd65c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/297289
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/297290
@toothrot toothrot modified the milestones: Go1.16.1, Go1.16.2 Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants