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

cmd/internal/objabi, cmd/link: direct calls not correctly identified on riscv64 [1.21 backport] #63167

Open
gopherbot opened this issue Sep 22, 2023 · 11 comments
Labels
CherryPickCandidate Used during the release process for point releases WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@gopherbot
Copy link

@ianlancetaylor requested issue #62465 to be considered for backport to the next 1.21 minor release.

@gopherbot Please open backport to 1.21.

This is a build failure with no straightforward workaround. It is a regression from 1.20.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 22, 2023
@gopherbot gopherbot added this to the Go1.21.2 milestone Sep 22, 2023
@thanm
Copy link
Contributor

thanm commented Sep 25, 2023

Hi @4a6f656c the deadline for the next set of minor releases is approaching; let me know if you think you will be able to create cherry-picks for 1.21.2 or whether we'll need to punt this backport to 1.21.3, thanks. Last day for 1.21.2 cherry-picks is tomorrow. Thanks.

@ianlancetaylor ianlancetaylor changed the title cmd/compile,runtime: write barrier issue on riscv64 [1.21 backport] cmd/internal/objabi, cmd/link: direct calls not correctly identified on riscv64 [1.21 backport] Sep 26, 2023
@prattmic prattmic added the CherryPickApproved Used during the release process for point releases label Oct 4, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Oct 4, 2023
@cherrymui
Copy link
Member

Personally I feel CL https://go.dev/cl/520095 is pretty complex for a backport. Could we find a simpler workaround?
On the other hand, this is RISCV only, so if the RISCV port maintainers are fine with this, I'm okay. Thanks.

@4a6f656c
Copy link
Contributor

4a6f656c commented Oct 5, 2023

@thanm apologies for not seeing this earlier.

I agree with @cherrymui that this is a fairly complex backport - given that the probability of hitting this is very low and that there is a known/documented workaround, I question if a backport is even justified.

If a backport is still considered necessary, I can see what simpler option I can come up with, however that may take a week or so. One option would be to disable the map init deadcode handling on riscv64, which would avoid the first/only known case that triggers the linker bug (i.e. hard code the workaround).

@thanm
Copy link
Contributor

thanm commented Oct 5, 2023

Turning off map init deadcode for riscv64 seems like it would be a decent option, I am ok with that.

@cherrymui
Copy link
Member

however that may take a week or so

The next minor release will be early November (there will be one this week, but that is already cut). So timing shouldn't be a problem.

Disabling map init deadcode for riscv64 seems reasonable. Or if adding a relocation type to IsDirectCall (or even specifically in map init deadcode code) works without other issues, we could do that. Thanks.

@gopherbot gopherbot modified the milestones: Go1.21.2, Go1.21.3, Go1.21.4 Oct 5, 2023
@gopherbot gopherbot modified the milestones: Go1.21.4, Go1.21.5 Nov 7, 2023
@heschi heschi added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 7, 2023
@gopherbot gopherbot modified the milestones: Go1.21.5, Go1.21.6 Dec 5, 2023
@mdempsky
Copy link
Member

mdempsky commented Jan 3, 2024

Ping? Do we still want to backport this to 1.21?

@gopherbot gopherbot modified the milestones: Go1.21.6, Go1.21.7 Jan 9, 2024
@mknyszek
Copy link
Contributor

The minor release is coming up once again. Any updates here? Thanks.

@gopherbot gopherbot modified the milestones: Go1.21.7, Go1.21.8 Feb 6, 2024
@cagedmantis
Copy link
Contributor

The next minor release freeze is approaching. Any updates?

@thanm
Copy link
Contributor

thanm commented Mar 27, 2024

Another ping on this issue. If we're not going to create a cherry-pick CL for it we should probably remove the Approved label so that we don't have to keep revisiting it in our meetings. Thanks.

@gopherbot gopherbot modified the milestones: Go1.21.9, Go1.21.10 Apr 3, 2024
@joedian
Copy link

joedian commented Apr 22, 2024

Another ping on this issue. Any updates?

@joedian
Copy link

joedian commented Apr 26, 2024

Based on lack of movement and new info moving back into Cherrypick candidate state.

@joedian joedian added CherryPickCandidate Used during the release process for point releases and removed CherryPickApproved Used during the release process for point releases labels Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickCandidate Used during the release process for point releases WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

10 participants