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: Value live at entry [1.18 backport] #54918

Closed
cuonglm opened this issue Sep 7, 2022 · 3 comments
Closed

cmd/compile: Value live at entry [1.18 backport] #54918

cuonglm opened this issue Sep 7, 2022 · 3 comments
Assignees
@mdempsky
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Milestone
Go1.18.7

Comments

@cuonglm
Copy link
Member

cuonglm commented Sep 7, 2022

@cuonglm requested issue #54911 to be considered for backport to the next 1.18 minor release.

@gopherbot please backport this to go1.18
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 7, 2022
@gopherbot gopherbot added this to the Go1.18.7 milestone Sep 7, 2022
@cuonglm cuonglm added the CherryPickCandidate Used during the release process for point releases label Sep 7, 2022
@mdempsky
Copy link
Contributor

mdempsky commented Sep 7, 2022

See also my thoughts on backporting at #54911 (comment).

@mknyszek mknyszek added the CherryPickApproved Used during the release process for point releases label Sep 7, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Sep 7, 2022
@mknyszek mknyszek moved this to In Progress in Go Compiler / Runtime Sep 7, 2022
@julieqiu julieqiu removed this from Go Security Sep 8, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/429897 mentions this issue: [release-branch.go1.18] cmd/compile/internal/inline: fix latent CalleeEffects issue

gopherbot pushed a commit that referenced this issue Sep 19, 2022
@mdempsky @cherrymui
[release-branch.go1.18] cmd/compile/internal/inline: fix latent Calle…
d5a5db3 
…eEffects issue

ir.ClosureExpr implements ir.InitNode, so ir.InitExpr can prepend init
statements to it. However, CalleeEffects wasn't aware of this and
could cause the init statements to get dropped when inlining a call to
a closure.

This isn't an issue today, because we don't create closures with init
statements. But I ran into this within unified IR.

Easy and robust solution: just take advantage that ir.TakeInit can
handle any node.

Fixes #54918.

Change-Id: Ica05fbf6a8c5be4b11927daf84491a1140da5431
Reviewed-on: https://go-review.googlesource.com/c/go/+/422196
Reviewed-by: Than McIntosh <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/429897
Reviewed-by: Michael Knyszek <[email protected]>
@gopherbot
Copy link
Contributor

Closed by merging d5a5db3 to release-branch.go1.18.

Repository owner moved this from In Progress to Done in Go Compiler / Runtime Sep 19, 2022
@golang golang locked and limited conversation to collaborators Sep 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mdempsky mdempsky

Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
Go1.18.7
Development

No branches or pull requests
4 participants
@mdempsky @mknyszek @cuonglm @gopherbot