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: missing deferreturn on linux/ppc64le [1.14 backport] #39991

Closed
gopherbot opened this issue Jul 2, 2020 · 16 comments
Closed

runtime: missing deferreturn on linux/ppc64le [1.14 backport] #39991

gopherbot opened this issue Jul 2, 2020 · 16 comments

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 2, 2020

@ianlancetaylor requested issue #39049 to be considered for backport to the next 1.14 minor release.

@gopherbot please open a backport to 1.14

@laboger
Copy link
Contributor

@laboger laboger commented Jul 2, 2020

I can work on the patch for this unless someone else has started.

@laboger
Copy link
Contributor

@laboger laboger commented Jul 2, 2020

Oh I think @cherrymui has to do it.

@laboger
Copy link
Contributor

@laboger laboger commented Jul 2, 2020

Is it possible that this is a bug due to the new linker? Is there still a way to build with the old linker in Go 1.14 to test it out?

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 2, 2020

@laboger Yeah, I can do the backport.

No, I don't think it is the new linker. The original issue (#39049) was reported for Go 1.14.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jul 3, 2020

Change https://golang.org/cl/240917 mentions this issue: [release-branch.go1.14] cmd/link: detect trampoline of deferreturn call

@laboger
Copy link
Contributor

@laboger laboger commented Jul 6, 2020

Thanks Cherry!

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jul 6, 2020

@ianlancetaylor Can you please include a short rationale about why the backport might be needed? (Per MinorReleases.) Thanks.

@laboger
Copy link
Contributor

@laboger laboger commented Jul 6, 2020

RedHat has moved their build compiler to Go 1.14.4 and is seeing the problem reported in issue #39049. The fix for that issue was merged upstream but not backported to the Go 1.14 branch.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 6, 2020

@dmitshur Sorry for missing the rationale in the backport request. It is the preceding comment on the main issue: #39049 (comment)

This is a critical bug for kubernetes project, can someone help us cherry-picking this to 1.14 release?

@mkumatag
Copy link

@mkumatag mkumatag commented Jul 7, 2020

@dmitshur Sorry for missing the rationale in the backport request. It is the preceding comment on the main issue: #39049 (comment)

This is a critical bug for kubernetes project, can someone help us cherry-picking this to 1.14 release?

yes, @ianlancetaylor is right, kubernetes community recently adopted golang 1.14 version and hit with this issue for ppc64le platform, need to be fixed asap. -Thanks.

@mkumatag
Copy link

@mkumatag mkumatag commented Jul 10, 2020

@ianlancetaylor @dmitshur I'm wondering when this will get merged into the 1.14 branch and any tentative timeline for the 1.14.5 release so that I can plan a golang bump for Kubernetes project accordingly!

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jul 10, 2020

Approving as it is a serious problem without a workaround. It is a 1.14-only regression, so a fix for 1.13 isn't needed.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jul 10, 2020

@mkumatag There will be a security release on Tuesday, July 14, as recently pre-announced. It will include only the security fixes. The next minor release (Go 1.14.6) is scheduled to happen very soon after that, and it'll include all non-security fixes on the 1.14 release branch.

@mkumatag
Copy link

@mkumatag mkumatag commented Jul 10, 2020

@mkumatag There will be a security release on Tuesday, July 14, as recently pre-announced. It will include only the security fixes. The next minor release (Go 1.14.6) is scheduled to happen very soon after that, and it'll include all non-security fixes on the 1.14 release branch.

Thanks for the information..

gopherbot pushed a commit that referenced this issue Jul 10, 2020
This is a backport of CL 234105. This is not a clean cherry-pick,
as CL 234105 is for the new linker, whereas we still use the old
linker here. This CL backports the logic.

The runtime needs to find the PC of the deferreturn call in a few
places. So for functions that have defer, we record the PC of
deferreturn call in its funcdata.

For very large binaries, the deferreturn call could be made
through a trampoline. The current code of finding deferreturn PC
fails in this case. This CL handles the trampoline as well.

Fixes #39991.
Updates #39049.

Change-Id: I929be54d6ae436f5294013793217dc2a35f080d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234105
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/240917
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jul 10, 2020

Closed by merging b6f70c0 to release-branch.go1.14.

@gopherbot gopherbot closed this Jul 10, 2020
@andybons andybons modified the milestones: Go1.14.5, Go1.14.6 Jul 14, 2020
@hpidcock
Copy link

@hpidcock hpidcock commented Jul 17, 2020

Huge thanks to @cherrymui for fixing this from the juju/juju team.

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
8 participants
You can’t perform that action at this time.