cmd/link: lock down future uses of linkname #67401
Comments
gopherbot
added
the
compiler/runtime
|
Change https://go.dev/cl/585820 mentions this issue: |
|
I think
However key differences:
As a downstream of quic-go I clearly understood the situation, the worst was the need to wait a couple of days for In the case of
instead of creating a compile time error they could have stubbed their API by forwarding calls to What I actually propose:Continue to allow the Pull kind of //go:build go1.23.4 && !go1.23.5 && !go1.24To know if a file is properly locked down, the toolchain can evaluate the build tags with the next future release and it MUST fail to be allowed. That means if a file pass the current version but not the next one, then Pull linknames from the std would be allowed. There is a downside to this approach which is that if there is no change required between two releases you still need a different file per version with the same implementation, to satisfy this constraint. Maybe parsing the build tags would be better. I am not sure if |
I completely disagree. Quic-go's use of linkname caused all manner of problems for us release after release too, because anyone using quic-go couldn't update to a new Go version until quic-go did. |
|
Change https://go.dev/cl/585916 mentions this issue: |
|
Change https://go.dev/cl/585915 mentions this issue: |
This comment has been minimized.
This comment has been minimized.
|
We can't fix what we don't know about.
We're open to any reports from closed-source packages. It would be particularly useful to hear about these when the release candidate comes out so they can make the .0 release. |
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Misuse is undesirable, disabling is even worse, you want to change you maintain that package (go-json), that doesn't solve it? |
|
I would like to say that //go:linkname is very useful for certain types of low level programming such as Ebitengine , etc. While I agree the situation with goccy/go-json is bad, we should look at how it is being used in detail and think of alternatives for the legitimate uses. |
|
@bjorndm should't
|
|
In C, static symbols are not accessible outside of the compilation unit, full stop. There is no way to pull a static symbol from a C library. A number of other languages have similar strict visibility rules. They are very successful languages and are widely used. This suggests that a lot programs can be written and things can go very well without a mechanism to break into a library's internal details. I don't think Go is fundamentally different. Ideally we could also have strict visibility rules. I would think Go unexported symbols are meant to be similar to C static symbols. In fact, that is what gccgo does. Unfortunately for the gc toolchain it is not the case today. But we can get closer to it. And as we care a lot about compatibility, we'll keep the existing code continue to build in Go 1.23 (Step 2 in the plan). And we have a linker flag to disable the restriction (e.g. for experiments; as far as I know, the C linker doesn't seem to have such an option). Also, I think the authors of the code should have a way to decide which symbols are visible externally and which are not. |
|
Purego which is a dependency of Oto, Beep, and Ebitengine as well as others doesn't just pull symbols it also pushes since it reimplements Another potential solution for us is to prebuild Of course, if the Go team wanted to port |
Push linknames are still allowed. If they are currently pushed from
This might be a possible option, but I think we need to understand the rationales better. The |
No, Purego pushes the same symbols that
Indeed, |
|
Thanks.
I don't think there is any plan to break the use case of Purego's pushes. If we do anything to restrict push-only ones, they will be equally applied to the ones runtime/cgo pushing to runtime. So we'll need to fix those first (I think many of them are already in handshake form, but it is possible we missed some). And that should make Purego work as well. |
dmitshur
added
the
NeedsInvestigation
|
Change https://go.dev/cl/586259 mentions this issue: |
|
Change https://go.dev/cl/586137 mentions this issue: |
github.com/DataDog/datadog-agent has stopped using runtime_setProfLabel and runtime_getProfLabel, remove them from the hall of shame. Updates #67401 Change-Id: I4a66c5e70397d43d7f064aeae5bad064e168316f Reviewed-on: https://go-review.googlesource.com/c/go/+/634476 Auto-Submit: Ian Lance Taylor <[email protected]> Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
This comment has been minimized.
This comment has been minimized.
|
For questions please refer to https://github.com/golang/go/wiki/Questions |
This comment has been minimized.
This comment has been minimized.
|
Change https://go.dev/cl/646535 mentions this issue: |
github.com/cilium/ebpf no longer accesses getAuxv using linkname but now uses the golang.org/x/sys/unix.Auxv wrapper introduced in go.dev/cl/644295. Also adjust the list of users to include x/sys/unix. Updates #67839 Updates #67401 Change-Id: Ieee266360b22cc0bc4be8f740e0302afd7dbd14f Reviewed-on: https://go-review.googlesource.com/c/go/+/646535 Reviewed-by: Ian Lance Taylor <[email protected]> Auto-Submit: Tobias Klauser <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
Starting from Go 1.23, direct access to `encoding/gob.nameToConcreteType` is no longer possible due to the removal of internal symbol linking. While this change improves encapsulation, it also removes the ability to retrieve a list of all registered types in the gob package. Relates: golang#67401 Closes: golang#71602
|
Change https://go.dev/cl/647536 mentions this issue: |
Starting from Go 1.23, direct access to `encoding/gob.nameToConcreteType` is no longer possible due to the removal of internal symbol linking. While this change improves encapsulation, it also removes the ability to retrieve a list of all registered types in the gob package. Relates: golang#67401 Closes: golang#71602 []string -> iter.Seq2[string, reflect.Type]
…ype` is no longer possible due to the removal of internal symbol linking. While this change improves encapsulation, it also removes the ability to retrieve a list of all registered types in the gob package. Relates: golang#67401 Closes: golang#71602
Starting from Go 1.23, direct access to `encoding/gob.nameToConcreteType` is no longer possible due to the removal of internal symbol linking. While this change improves encapsulation, it also removes the ability to retrieve a list of all registered types in the gob package. Relates: golang#67401 Closes: golang#71602
Introduce the RegisteredTypes function to return a sequence of all registered types. This makes it easier to inspect registered types, which can be useful for debugging and understanding type usage. Relates: golang#67401 Fixes: golang#71605
|
The package: https://github.com/pkujhd/goloader allows compiled object files (*.o) to be used in other projects at runtime (i.e. it loads them as if they were plugins). It is a very powerful package. It uses It does not build without I was under the impression that current links (as the project is a few years old) to the standard library were permitted without the My suspicion is this package gets used privately/internally, based on online discussions. List of go:linkname uses with build tags (expand to view)reflect.(*rtype).uncommon
reflect.(*rtype).Kind
reflect.(*rtype).NumField
reflect.(*rtype).Field
reflect.(*rtype).NumIn
reflect.(*rtype).In
reflect.(*rtype).NumOut
reflect.(*rtype).Out
reflect.(*rtype).Key
reflect.(*rtype).Elem
reflect.(*rtype).NumMethod
reflect.(*rtype).Method
reflect.(*rtype).ChanDir
reflect.(*rtype).Len
reflect.(*rtype).IsVariadic
reflect.(*rtype).Name
reflect.(*rtype).String
reflect.(*rtype).PkgPath
runtime.typelinksinit
runtime.lock
runtime.unlock
runtime.atomicstorep
runtime.firstmoduledata
runtime.step
runtime.findfunc
runtime.funcdata
runtime.funcname
runtime.gostringnocopy
runtime.moduledataverify1
runtime.modulesinit
runtime.progToPointerMask
runtime.doInit [go1.13 && !go1.21]
runtime.hash [go1.8 && !go1.10]
runtime.ifaceLock [go1.8 && !go1.10]
runtime.additab [go1.8 && !go1.10]
runtime.itabTable [go1.10 && !go1.25]
runtime.itabLock [go1.10 && !go1.25]
runtime.itabAdd [go1.10 && !go1.25]
runtime.add
runtime.doInit1 [go1.21 && !go1.25]
src/cmd/internal/goobj.importPathToPrefix [go1.8 && !go1.9] |
|
@prattmic |
|
We tried to find all the linknames used by popular packages. It's possible that we missed some. According to https://pkg.go.dev/github.com/pkujhd/goloader that package is only imported by 7 other packages, which is probably why it didn't make the list. |
|
It does have quite a few forks and is an "unlinked" fork of a prior project which is no longer maintained: https://github.com/dearplain/goloader |
|
It's a pretty discouraging list of symbols. It would be truly awful if we have to keep all of those working indefinitely. Not sure what to do here. |
|
cc @golang/compiler - while not an immediate solution, it may be less of a headache long-term to improve plugin support - or create some better v2 version of plugins - rather than exposing many more runtime internals for an external implementation. |
|
Hi @pjebs, note that the initial round of locking down Go linknames was in Go 1.23, which was released ~6 months ago. Do you know if anyone reported a problem to the core Go team with goloader not working since then, or maybe one of its forks? (I poked around briefly in the issue tracker and didn't see one; there are some comments about plugin support and goloader, but I might have missed a linkname-related complaint). The goloader README currently seems to say:
Maybe that is viewed as a workable compromise by at least some percentage of the historical users of goloader? Separately, is your interest in goloader in support of a similar use case to what you described in #71099 (comment)? (If so, my personal opinion is that for distributing a proprietary closed-source Go library, a Go obfuscator like https://github.com/burrowers/garble is a better choice for most people than something like goloader based on my experience with commercial software, but of course you can have a different opinion ;) FWIW, I think that the core Go team supporting all of the symbols that goloader uses is the type of thing that would delay and/or reduce the chances of a better plugin-type solution in the future (both from direct costs and indirect costs). (edit: typos) |
|
I'd assumed this was a regression from 1.23 to 1.24 like #71672, but I don't think there is anything for us to do here. Behavior is in line with the original intention that we support linknames for modules with >= 50 dependents to avoid breaking large portions of the ecosystem. This package did not meet that bar. As @thepudds mentions, the package already documents that it requires building with |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hi @benduckwdesign, note that Ian did not say that it should not be discussed. It's just that this is not the right place. It can be discussed elsewhere. Separately, the core Go team has requested multiple times on this issue that people should open specific issues about enhancements to avoid the use of linknames (e.g., see #67401 (comment), #67401 (comment), #67401 (comment)), such as Ian wrote:
Also, if we were to instead use a single issue to interleave your conversation about plugins with someone else's conversation about netpoller internals with yet other conversations about reflect performance and so on, it makes it much harder to make progress on any of those conversations, including yours. (And personally, I don't know if there's an existing crisp proposal for what a new plugin system might look like, aside from suggestions that today it's likely better to pick an alternative such as communicating with a separate process via rpc or stdin or similar, but there are existing issues about problems with the current plugins like #20461, #19282, #43749, #29525, #24245). |
and others
Overuse of //go:linkname to reach into Go standard library internals (especially runtime internals) means that when we do change the standard library internals in ways that should not matter, we can end up breaking packages that are depended on by a large swath of the Go ecosystem. For example, https://go.dev/cl/583756 broke github.com/goccy/go-json because it turns out that package copied most of the runtime's internal type API. Now we can't change anything in that list, despite that being an ostensibly internal package, without breaking goccy/go-json. And goccy is used by many packages, including Kubernetes.
This situation is unsustainable. Internals are internal for a reason. We can't keep Go programs working when they create explicit dependencies on details that we have kept internal. But we also care a lot about compatibility: we don't want to break Go programs either. The obvious conclusion is that we have to stop Go programs from being able to create these dependencies on internal details in the first place.
This issue tracks work to prevent new //go:linkname-based dependencies and contain existing ones.
Right now, if package A has a symbol and package B wants to refer to it with //go:linkname, there are three patterns:
(Push) Package A uses a //go:linkname to rename one of its own symbols to B.foo, and then B declares
func foo()without a body. In this form, A clearly intends for B to use foo, although the compiler cannot quite tell what's going on in B and warns about foo not having a body unless you create an empty dummy.s file.(Pull) Package A defines foo without any annotation, and package B uses //go:linkname to access A.foo. In this form, A may not intend for B to use foo at all. That's a serious problem: when A renames foo and/or changes its type signature, B breaks, and A may never even have heard of B.
(Handshake) Package A defines foo with a //go:linkname and package B defines foo also with a //go:linkname, and the two agree on the name (either A.foo or B.foo). This is the ideal form, and it avoids the dummy.s workaround that is needed in the Push case.
The ideal goal state is a world where all //go:linkname usage must be in the Handshake form: both sides must agree to use linkname for a given symbol in order for it to succeed. This will mean that arbitrary packages cannot create new dependencies on runtime internals. At the same time, we realize that the current world is not this ideal world, and we don't want to break all existing uses.
Our plan is as follows.
Introduce a new -checklinkname=1 flag to cmd/link that requires the Handshake form for symbols in the standard library. That flag is already landed in at tip, but it is not the default.
Survey all existing open-source Go packages to find standard library symbols that are being //go:linkname'd (behind our backs!) using the Pull pattern. Add the necessary //go:linkname annotations to the standard library to keep those working, documenting why each exists. The explicit //go:linkname lines and documentation will help avoid accidental breakage in future refactoring. We have done a preliminary survey, but we haven't yet added all the necessary //go:linkname lines.
Make -checklinkname=1 the default for Go 1.23. If this breaks anything, users can use -ldflags=-checklinkname=0 to get unbroken, and we hope they will also file reports letting us know what we missed.
As we get reports of additional breakage we missed, add more //go:linkname annotations to the standard library.
At the completion of that plan, we won't be in the ideal world, but we will have accomplished two important things:
We won't have broken anything.
We will have stopped new damage from accumulating: there will be no more new references to runtime internals introduced. In particular, new internals we added during the Go 1.23 cycle, like coro and weak pointers, cannot be linknamed, now or ever. And anything that wasn't linknamed yet won't grow new linknames in the future.
Note that anyone who wants to experiment can always build with -ldflags=-checklinkname=0 and linkname whatever they like. That's fine. We like experimenting too. But the fact that the code won't build without special flags should help prevent code that digs into internal details from becoming a core dependency in the Go ecosystem that we end up having to maintain forever.
Note also that for now, //go:linkname can still be used in Pull mode to get at internals of non-standard library packages. We'd like to change that eventually too, insisting on Handshakes everywhere. For now, we are starting with the standard library. If all goes well, we'll circle back and try to devise a plan for the rest of the ecosystem.
The text was updated successfully, but these errors were encountered: