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

Jump to bottom

plugin: program on linux/s390x sometimes hangs after calling "plugin.Open" [1.15 backport] #40693

Closed
gopherbot opened this issue Aug 11, 2020 · 7 comments
Closed

plugin: program on linux/s390x sometimes hangs after calling "plugin.Open" [1.15 backport] #40693

gopherbot opened this issue Aug 11, 2020 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.15.3

Comments

@gopherbot
Copy link

gopherbot commented Aug 11, 2020 •
edited by mundaym

@mundaym requested issue #40473 to be considered for backport to the next 1.15 minor release.

@gopherbot please open backport issues.

This is a bug that causes programs to intermittently crash with no workaround.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 11, 2020
@gopherbot gopherbot mentioned this issue Aug 11, 2020
Closed
@gopherbot gopherbot added this to the Go1.13.16 milestone Aug 11, 2020
@mundaym mundaym modified the milestones: Go1.13.16, Go1.15.1 Aug 11, 2020
@mundaym mundaym changed the title plugin: Golang app on linux/s390x sometimes hangs after calling "plugin.Open" [1.13 backport] plugin: Golang app on linux/s390x sometimes hangs after calling "plugin.Open" [1.15 backport] Aug 11, 2020
@gopherbot
Copy link
Author

Change https://golang.org/cl/248478 mentions this issue: [release-branch.go1.15] cmd/internal/obj: fix inline marker issue on s390x

@dmitshur dmitshur modified the milestones: Go1.15.1, Go1.15.2 Sep 1, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Sep 3, 2020

Approving per discussion in a release meeting. This backport applies to both 1.15 (this issue) and 1.14 (#40694).

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Sep 3, 2020
@dmitshur dmitshur mentioned this issue Sep 3, 2020
Closed
@dmitshur dmitshur changed the title plugin: Golang app on linux/s390x sometimes hangs after calling "plugin.Open" [1.15 backport] plugin: program on linux/s390x sometimes hangs after calling "plugin.Open" [1.15 backport] Sep 3, 2020
@dmitshur dmitshur modified the milestones: Go1.15.2, Go1.15.3 Sep 9, 2020
@mundaym
Copy link
Member

mundaym commented Sep 10, 2020 •
edited

@dmitshur Why was this bumped to 1.15.3? This bug is causing a real problem for a user and the fix has been ready to go for quite some time. I'd have appreciated some sort of warning if there was a reason it was going to miss the cutoff.

@mundaym
Copy link
Member

mundaym commented Sep 10, 2020 •
edited

I generally don't use release blocker tags because we aren't supposed to for the non-first class ports. Should I start doing that if I feel an issue needs to be resolved? Alternatively bumping issue milestones a week or so earlier would give me time to start a discussion if I feel it shouldn't be bumped.

EDIT: I've created a golang-dev post for discussion: https://groups.google.com/g/golang-dev/c/6b-rdo-i8sA

@dmitshur
Copy link
Contributor

I've answered more generally on the thread. The two backport CLs need to be reviewed, as they can't be submitted without a +2. Could you please find a code reviewer for them? Thank you.

@mvdan
Copy link
Member

mvdan commented Sep 10, 2020

Could you please find a code reviewer for them? Thank you.

For what it's worth, I disagree with this - if an external contributor is going through the extra trouble of sending fixes and backporting them for a bugfix release, we shouldn't rely on them to stay on top of the CL for multiple days or even weeks until it finally gets a review.

I personally think that, since we have https://dev.golang.org/owners and the release team already goes over all backport issues during the release process, it wouldn't be a lot of added work to simply ping the owners for a review of each backport CL well before the release is due. I imagine that should be enough for the vast majority of cases - if someone is listed as a package owner, I would argue that their top priority is reviewing and shipping critical bug fixes.

@gopherbot
Copy link
Author

Closed by merging bbee523 to release-branch.go1.15.

gopherbot pushed a commit that referenced this issue Sep 10, 2020
@mundaym @dmitshur
[release-branch.go1.15] cmd/internal/obj: fix inline marker issue on …
bbee523 
…s390x

The optimization that replaces inline markers with pre-existing
instructions assumes that 'Prog' values produced by the compiler are
still reachable after the assembler has run. This was not true on
s390x where the assembler was removing NOP instructions from the
linked list of 'Prog' values. This led to broken inlining data
which in turn caused an infinite loop in the runtime traceback code.

Fix this by stopping the s390x assembler backend removing NOP
values. It does not make any difference to the output of the
assembler because NOP instructions are 0 bytes long anyway.

Note: compiler check omitted from backport to reduce risk of change.

Fixes #40693.

Change-Id: I4eb570de13165cde342d5fb2ee3218945ddf4b52
Reviewed-on: https://go-review.googlesource.com/c/go/+/248478
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
claucece pushed a commit to claucece/go that referenced this issue Oct 22, 2020
@mundaym @claucece
[release-branch.go1.15] cmd/internal/obj: fix inline marker issue on …
bf131b7 
…s390x

The optimization that replaces inline markers with pre-existing
instructions assumes that 'Prog' values produced by the compiler are
still reachable after the assembler has run. This was not true on
s390x where the assembler was removing NOP instructions from the
linked list of 'Prog' values. This led to broken inlining data
which in turn caused an infinite loop in the runtime traceback code.

Fix this by stopping the s390x assembler backend removing NOP
values. It does not make any difference to the output of the
assembler because NOP instructions are 0 bytes long anyway.

Note: compiler check omitted from backport to reduce risk of change.

Fixes golang#40693.

Change-Id: I4eb570de13165cde342d5fb2ee3218945ddf4b52
Reviewed-on: https://go-review.googlesource.com/c/go/+/248478
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@golang golang locked and limited conversation to collaborators Sep 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.15.3
Development

No branches or pull requests
4 participants
@dmitshur @mvdan @gopherbot @mundaym