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

runtime: add linkname runtime.lastmoduledatap back for cloudwego/sonic #71672

Closed
AsterDY opened this issue Feb 12, 2025 · 9 comments · May be fixed by #71673
Closed

runtime: add linkname runtime.lastmoduledatap back for cloudwego/sonic #71672

AsterDY opened this issue Feb 12, 2025 · 9 comments · May be fixed by #71673
Assignees
@prattmic
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.25

Comments

@AsterDY
Copy link
Contributor

AsterDY commented Feb 12, 2025 •
edited
Loading

bytedance/sonic#738
Sonic has used this link for two years and just removed the link codes to sonic/loader since go1.23. Is this why Sonic misses the shame of hall on it ? Can you please add its linkname back? Otherwise it will affect a lot of Go applications... @rsc
@AsterDY AsterDY changed the title Please keep linkname of lastmoduledatap for cloudwego/sonic Please keep linkname of runtime.lastmoduledatap for cloudwego/sonic Feb 12, 2025
@AsterDY AsterDY mentioned this issue Feb 12, 2025
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/648537 mentions this issue: runtime: add linkname back on runtime.lastmoduledatap``

@ianlancetaylor
Copy link
Member

As far as I can tell we never had that package in the "hall of shame". And the reason for that is that the package is only imported by 3 other packages. Are there more popular packages that depend on this one?

The linkname was removed in https://go.dev/cl/609918 by @xiaost .

@AsterDY
Copy link
Contributor Author

AsterDY commented Feb 12, 2025 •
edited
Loading

no, it was dependent on github.com/bytedance/sonic, and sonic is dependent over 1w+ repos.. I guess you didn't consider indirect dependency? @ianlancetaylor

@seankhliao seankhliao changed the title Please keep linkname of runtime.lastmoduledatap for cloudwego/sonic runtime: add linkname of runtime.lastmoduledatap for cloudwego/sonic Feb 12, 2025
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 12, 2025
@prattmic
Copy link
Member

I'm not sure why this package wasn't included on this list. https://go.dev/cl/609918 also dropped runtime.moduledataverify1 and runtime.morestack_noctxt, which seem to be used by github.com/bytedance/sonic/loader. Are those needed as well?

@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 12, 2025
@mknyszek mknyszek added this to the Go1.25 milestone Feb 12, 2025
@AsterDY
Copy link
Contributor Author

AsterDY commented Feb 13, 2025

I'm not sure why this package wasn't included on this list. https://go.dev/cl/609918 also dropped runtime.moduledataverify1 and runtime.morestack_noctxt, which seem to be used by github.com/bytedance/sonic/loader. Are those needed as well?

updated

@AsterDY
Copy link
Contributor Author

AsterDY commented Feb 13, 2025

Btw, I want this commit to be released on go1.24, what should I do? @mknyszek

@ianlancetaylor
Copy link
Member

@gopherbot Please backport to the 1.24 branch.

This will let some existing programs continue to build with 1.24, as they did with earlier releases.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #71705 (for 1.24).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Feb 13, 2025
Closed
@AsterDY AsterDY changed the title runtime: add linkname of runtime.lastmoduledatap for cloudwego/sonic runtime: add linkname back of runtime.lastmoduledatap for cloudwego/sonic Feb 13, 2025
@AsterDY AsterDY changed the title runtime: add linkname back of runtime.lastmoduledatap for cloudwego/sonic runtime: add linkname runtime.lastmoduledatap back for cloudwego/sonic Feb 13, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/650375 mentions this issue: [release-branch.go1.24] runtime: add some linknames back for github.com/bytedance/sonic``

@github-project-automation github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Feb 18, 2025
gopherbot pushed a commit that referenced this issue Feb 19, 2025
@AsterDY @gopherbot
[release-branch.go1.24] runtime: add some linknames back for `github.…
cd0e528 
…com/bytedance/sonic`

Add some linknames back, therefore sonic (github.com/bytedance/sonic) can work correctly.

For #71672
Fixes #71705

Change-Id: Iae86c837d8a714855106a26766aa08b128e17e58
GitHub-Last-Rev: 4de0a48
GitHub-Pull-Request: #71673
Reviewed-on: https://go-review.googlesource.com/c/go/+/650375
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
@prattmic prattmic mentioned this issue Feb 19, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@prattmic prattmic

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Go1.25
Development

Successfully merging a pull request may close this issue.

runtime: add some linknames back for github.com/bytedance/sonic AsterDY/go
5 participants
@mknyszek @prattmic @ianlancetaylor @gopherbot @AsterDY