cmd/link: lock down future uses of linkname

Question

cmd/link: lock down future uses of linkname

rsc opened this issue 8 months ago · 171 comments

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.

Answer 1 · 2024-05-15T21:45:34.000Z

Change https://go.dev/cl/585820 mentions this issue: runtime/coverage: remove uses of //go:linkname

Answer 2 · 2024-05-15T21:46:59.000Z

I think github.com/goccy/go-json pulled internal runtime APIs in a reckless manner and there are safe ways to pull internal packages from the std.
For example quic-go did an equally dangerous thing with crypto/tls by accessing private fields using unsafe.Pointer and maintaining forks of crypto/tls:

However key differences:

it was guarded by build tags on the go version
it had a future proof fallback which created a compile time error linking to documentation explaining the situation

As a downstream of quic-go I clearly understood the situation, the worst was the need to wait a couple of days for quic-go to release a new release compatible with the new go release before updating my go toolchain and the inability to run on tip or RCs.

In the case of github.com/goccy/go-json the situation could have been even better than quic-go's as it claims to be compatible with encoder/json:

Fast JSON encoder/decoder compatible with encoding/json for Go

instead of creating a compile time error they could have stubbed their API by forwarding calls to encoding/json when the release of go were to be unknown. (I don't know the details, maybe due to some edge cases or behaviors only go-json implements this wouldn't have been possible)

What I actually propose:

Continue to allow the Pull kind of linkname from std packages when the file is locked with build tags:

//go:build go1.23.4 && !go1.23.5 && !go1.24

To 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 goX.Y or goX.Y.Z should be used. goX.Y might be more dangerous in case a fix require breaking some internal API, quic-go used that and it was fine and created this couple of days period where you can't use latest go only every 6 months (note that goX.Y.Z build tags didn't existed back then).

Answer 3 · 2024-05-15T22:51:28.000Z

I think ... there are safe ways to pull internal packages from the std.

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.

Answer 4 · 2024-05-15T23:07:25.000Z

Change https://go.dev/cl/585916 mentions this issue: internal/coverage/cfile: remove //go:linkname into testing

Answer 5 · 2024-05-15T23:07:26.000Z

Change https://go.dev/cl/585915 mentions this issue: internal/coverage/cfile: remove more //go:linkname usage

Answer 6 · 2024-05-16T00:58:25.000Z

Survey all existing open-source Go packages to find standard library symbols

What about closed source packages?

Answer 7 · 2024-05-16T01:16:42.000Z

We can't fix what we don't know about.

As we get reports of additional breakage we missed, add more //go:linkname annotations to the standard library.

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.

Answer 8 · 2024-05-16T02:33:18.000Z

It would be nice to have a flag (maybe in -ldflags?) to allow "pull" style //go:linkname.

I know Go eschews knobs and flags, but closed source projects aren't as big of a problem as open source projects because any breakage is self-contained. And a flag won't be used much by open source projects because it's abnormal for Go packages to require setting specific compiler/linker flags.

I do think this proposal is a good idea, though.

Answer 9 · 2024-05-16T02:37:39.000Z

It would be nice to have a flag (maybe in -ldflags?) to allow "pull" style //go:linkname.

The proposal says:

Note that anyone who wants to experiment can always build with -ldflags=-checklinkname=0 and linkname whatever they like.

Answer 10 · 2024-05-16T02:43:38.000Z

Oops, sorry :)

Answer 11 · 2024-05-16T03:56:21.000Z

Misuse is undesirable, disabling is even worse, you want to change you maintain that package (go-json), that doesn't solve it?

Answer 12 · 2024-05-16T11:22:06.000Z

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.

Answer 13 · 2024-05-16T12:08:09.000Z

@bjorndm should't -ldflags=-checklinkname=0 which disables this check be enough for low level programming? And if you can always raise a proposal if something is missing and truly needed without ld flags.

As we get reports of additional breakage we missed, add more //go:linkname annotations to the standard library.

Answer 14 · 2024-05-16T14:26:41.000Z

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.

Answer 15 · 2024-05-16T16:29:49.000Z

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 runtime/cgo package when CGO_ENABLED=0 entirely in Go. Is there any way the symbols defined in the runtime that hook into that package also get comments to avoid breaking us?

Another potential solution for us is to prebuild runtime/cgo into a cgo_GOOS_GOARCH_GOVERSION.syso that ships with purego. That would save us from having to keep up with any changes that package has and allows the Go team the freedom to change it as they like. Is this actually possible? I tried with setting different -buildmode but none of them would link.

Of course, if the Go team wanted to port runtime/cgo to Go that would be optimal.

Answer 16 · 2024-05-16T19:56:53.000Z

Is there any way the symbols defined in the runtime that hook into that package also get comments to avoid breaking us?

Push linknames are still allowed. If they are currently pushed from runtime/cgo, I believe you can still push them from Purego. Does Purego push symbols more than runtime/cgo?

Of course, if the Go team wanted to port runtime/cgo to Go that would be optimal.

This might be a possible option, but I think we need to understand the rationales better. The runtime/cgo package is intended to work with cgo, that is, interacting with C code. I'm not sure I understand the use case of runtime/cgo with CGO_ENABLED=0. This is probably better to be a separate discussion. Thanks.

Answer 17 · 2024-05-16T21:18:25.000Z

Push linknames are still allowed. If they are currently pushed from runtime/cgo, I believe you can still push them from Purego. Does Purego push symbols more than runtime/cgo?

No, Purego pushes the same symbols that runtime/cgo does which means the "Handshake" is already satisfied for those. Step 2 of the suggested plan only mentions surveying for Pull linknames and marking them with comments to avoid future breakage. I'm wondering if there is any plans for Pushes as changes to those would break Purego?

This might be a possible option, but I think we need to understand the rationales better. The runtime/cgo package is intended to work with cgo, that is, interacting with C code. I'm not sure I understand the use case of runtime/cgo with CGO_ENABLED=0. This is probably better to be a separate discussion. Thanks.

Indeed, runtime/cgo is required to allow Go code and C code to play well with each other. Purego provides an entirely Go version of it so that you can call C code using purego.Dlopen and purego.Dlsym without the need of a C compiler so cross-compiling is again possible. We can discuss this further elsewhere.

Answer 18 · 2024-05-16T21:23:06.000Z

Thanks.

I'm wondering if there is any plans for Pushes as changes to those would break Purego?

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.

Answer 19 · 2024-05-17T16:14:01.000Z

Change https://go.dev/cl/586259 mentions this issue: runtime: move exit hooks into internal/runtime/exithook

Answer 20 · 2024-05-17T16:29:41.000Z

Change https://go.dev/cl/586137 mentions this issue: all: add push linknames to allow legacy pull linknames

Answer 21 · 2024-05-17T16:34:53.000Z

Change https://go.dev/cl/586476 mentions this issue: runtime: stop external packages from using typelinks

Answer 22 · 2024-05-17T16:34:53.000Z

Change https://go.dev/cl/585556 mentions this issue: cmd/link: enable checklinkname by default

Answer 23 · 2024-05-20T07:41:03.000Z

tfo-go implements TCP Fast Open support. It provides a set of APIs similar to the ones in net, with even the same underlying concrete types (*net.TCPConn, *net.TCPListener) as return values.

In order to do so, it has to tap into the net package to learn about the preferred address family, listener backlog, etc. On Windows, it has to call unexported functions in netpoll, because syscall.RawConn cannot be used for CONNECTEX calls.

Here's the list of pull linknames in this project. Can I open a CL to add them to the standard library?

// tfo-go/netpoll_windows.go

//go:linkname sockaddrToTCP net.sockaddrToTCP
//go:linkname execIO internal/poll.execIO
//go:linkname newFD net.newFD
//go:linkname netFDInit net.(*netFD).init
//go:linkname netFDClose net.(*netFD).Close
//go:linkname netFDCtrlNetwork net.(*netFD).ctrlNetwork
//go:linkname netFDWrite net.(*netFD).Write
//go:linkname netFDSetWriteDeadline net.(*netFD).SetWriteDeadline
//go:linkname rawConnControl net.(*rawConn).Control
//go:linkname rawConnRead net.(*rawConn).Read
//go:linkname rawConnWrite net.(*rawConn).Write

// tfo-go/sockopt_linux.go

//go:linkname listenerBacklog net.listenerBacklog

// tfo-go/tfo_supported.go

//go:linkname ipToSockaddr net.ipToSockaddr
//go:linkname loopbackIP net.loopbackIP
//go:linkname favoriteAddrFamily net.favoriteAddrFamily

Answer 24 · 2024-05-20T09:07:35.000Z

I've used go:linkname in quite a few places, I think usually to do reflection-type things that cause unnecessary allocations if you use the reflect package. I always understood the code might break with changes to Go. That would not upset me. I'd be much more upset by losing the mechanism to use these kinds of backdoors when it seems necessary.

Answer 25 · 2024-05-20T16:07:41.000Z

@philpearl You can continue to do what you want with your own projects by using the new linker flag. But if you release a package that relies on linknaming, and people start using that package, that becomes a problem for the overall Go ecosystem. We really don't want to break popular packages. If they use linkname we have to make a difficult choice between breaking a popular package or slowing down or even stopping ongoing Go development. Prohibiting new uses of linkname seems like a good path forward.

Answer 26 · 2024-05-20T17:19:15.000Z

Thanks for the response! I hadn't realised that the new flag would be permanent rather than a stop-gap until people had updated their code. (I do have some packages that use linkname, but I'd be shocked if they ever become popular!)

The two reasons I ever used linkname were, I believe

performance issues with reflect
no access to hash functions except via an interface

In the second case I decided it was better to use linkname than copy assembly code to get a fast hash function.

I'm afraid I've not recently checked whether these performance issues still exist - but I'd happily remove all use of linkname if I could get the performance I need (which is mostly just no unnecessary allocations) from using the standard library.

Answer 27 · 2024-05-20T18:24:21.000Z

I think the linker flag will probably stay.

performance issues with reflect

We have been (slowly) improving the performance of reflect. If there is a performance issue that is not already known, feel free to open an issue so we can investigate.

no access to hash functions except via an interface

maphash.Comparable (#54670) might help?

Answer 28 · 2024-05-20T19:18:42.000Z

I now had a chance to look at the impact that this will have on Gonum graph packages after work that Ian did. Initially, I was optimistic that the changes that he made in the iterator package and supporting proposed changes in reflect would allow us to maintain reasonable performance. This is not the case. In primitives I see ~30% increases in CPU cost and 1000% to 10,000% increases in allocs (10% to 1000% increases in allocation volume). This makes the Gonum graph packages untenable.

Answer 29 · 2024-05-21T14:27:25.000Z

Change https://go.dev/cl/586896 mentions this issue: runtime: push vdsoClockgettimeSym linkname on linux/arm64

Answer 30 · 2024-05-22T01:35:05.000Z

I ran a scan against an updated copy of my Go corpus, filtered to packages with at least N direct+indirect dependents. Here are the results for varying N. Each file cuts off where the next file begins, so for example linkname100.html only shows linknames from modules with >= 100 but < 200 dependents.

https://swtch.com/tmp/linkname1.html
https://swtch.com/tmp/linkname100.html
https://swtch.com/tmp/linkname200.html
https://swtch.com/tmp/linkname500.html
https://swtch.com/tmp/linkname1000.html
https://swtch.com/tmp/linkname2000.html
https://swtch.com/tmp/linkname5000.html
https://swtch.com/tmp/linkname10000.html
https://swtch.com/tmp/linkname20000.html
https://swtch.com/tmp/linkname50000.html
https://swtch.com/tmp/linkname100000.html

I will send some CLs adding the linknames I found, starting with the largest number of dependents, and we can decide where exactly we want to stop.

Answer 31 · 2024-05-22T01:38:12.000Z

@database64128 According to deps.dev, almost nothing depends on tfo-go (v1 v2) yet. I'd rather work with you to figure out a linkname-free way to do what you need to do than add those linknames. It's suspicious to be linknaming exported methods, since you can just call those methods. Can you open a separate bug for tfo-go's linknames and we can discuss there? Thanks!

Answer 32 · 2024-05-22T01:39:07.000Z

@kortschak Did you mean to comment on a different bug? This one is about locking down //go:linkname.

Answer 33 · 2024-05-22T02:05:32.000Z

@rsc #66125 and gonum/gonum#1937 describe the performance problems that gonum uses unsafe linkname-ing to work around. (Some code is here.)

Answer 34 · 2024-05-22T02:20:56.000Z

According to deps.dev, almost nothing depends on tfo-go (v1 v2) yet.

There are forks that are dependencies of projects with over 10k stars. They forked the project because I raised the minimum supported Go version to 1.21 but they want to support as old as 1.18.

MetaCubeX/tfo-go is a dependency of MetaCubeX/mihomo, which has 13k stars.
SagerNet/tfo-go is a dependency of SagerNet/sing-box, which has 14.3k stars.

It's suspicious to be linknaming exported methods, since you can just call those methods.

These are exported methods on unexported structs (net.(*netFD) and net.(*rawConn)).

I'd rather work with you to figure out a linkname-free way to do what you need to do than add those linknames.

Can you open a separate bug for tfo-go's linknames and we can discuss there?

Thanks, I'll open an issue later this week. But I'm afraid the whole process is going to take some time, and right now these projects need the linknames to be able to build with gotip.

Answer 35 · 2024-05-22T02:26:36.000Z

GitHub stars are not necessarily important. A starred but not-depended-on project will not cause much breakage (unless the dependent counts are wrong?). No projects need to build with gotip (and main packages can use the flag).

My focus right now is on projects with many dependents. I will circle back to tfo-go.

Answer 36 · 2024-05-22T03:44:04.000Z

@cespare and @kortschak, I have a pending CL that will re-enable linkname of mapiterkey, mapiterelem, mapiternext, which I see gonum uses.

Answer 37 · 2024-05-22T03:57:21.000Z

@rsc Thank you so much for that. I would love to move off the use that we have, and maybe that will be possible in the future, but at the moment, it is unfortunately not.

Answer 38 · 2024-05-22T04:04:42.000Z

@kortschak Completely understood. We just missed gonum's usage when preparing the initial list of users. I am working with a more complete list now.

Answer 39 · 2024-05-22T04:17:24.000Z

Reopening for adding the linknames I found in my more complete scan.

Answer 40 · 2024-05-22T04:20:46.000Z

I just mailed CLs to catch the linknames in all modules with >= 1000 dependents.
I will take a closer look at

https://swtch.com/tmp/linkname1.html
https://swtch.com/tmp/linkname100.html
https://swtch.com/tmp/linkname200.html
https://swtch.com/tmp/linkname500.html

later to see if we should lower the bar or cherry-pick specific ones.
(Bazel-gazelle's are probably worth doing, for example.)

Answer 41 · 2024-05-22T04:21:37.000Z

Change https://go.dev/cl/587218 mentions this issue: all: document legacy //go:linkname for modules with ≥20,000 dependents

Answer 42 · 2024-05-22T04:21:37.000Z

Change https://go.dev/cl/587216 mentions this issue: runtime: revert "move zeroVal to internal/abi"

Answer 43 · 2024-05-22T04:21:38.000Z

Change https://go.dev/cl/587219 mentions this issue: all: document legacy //go:linkname for modules with ≥10,000 dependents

Answer 44 · 2024-05-22T04:21:38.000Z

Change https://go.dev/cl/587217 mentions this issue: all: document legacy //go:linkname for modules with ≥50,000 dependents

Answer 45 · 2024-05-22T04:21:39.000Z

Change https://go.dev/cl/587220 mentions this issue: all: document legacy //go:linkname for modules with ≥5,000 dependents

Answer 46 · 2024-05-22T04:21:39.000Z

Change https://go.dev/cl/587215 mentions this issue: all: document legacy //go:linkname for modules with ≥100,000 dependents

Answer 47 · 2024-05-22T04:21:40.000Z

Change https://go.dev/cl/587222 mentions this issue: all: document legacy //go:linkname for modules with ≥1,000 dependents

Answer 48 · 2024-05-22T04:21:40.000Z

Change https://go.dev/cl/587221 mentions this issue: all: document legacy //go:linkname for modules with ≥2,000 dependents

Answer 49 · 2024-05-22T06:53:44.000Z

@rsc as you're making your way through the list, but not sure if it shows up in "volumes" w.r.t. Go dependent; looks like this broke OCI runc; more details / discussion here;

opencontainers/runc#4288

cc @lifubang @kolyshkin

Answer 50 · 2024-05-22T08:20:01.000Z

Although I don't believe using //go:linkname is a good practice, labeling widely-used libraries as 'members of the hall of shame' is childish and not a constructive way to address this issue.

Answer 51 · 2024-05-22T10:46:33.000Z

@anthonywong1221 It's an internal comment. The reason for that phrasing is to stop users from sending CLs adding themselves to the comments. The point was to be clear being listed in the Go source code this way is not some kind of badge of honor.

Answer 52 · 2024-05-22T10:48:31.000Z

@thaJeztah what is the Go bug that runc is trying to work around with a linkname? Has it been filed?

Answer 53 · 2024-05-22T10:50:23.000Z

@thaJeztah CL 587219 should fix opencontainers/runc#4288.

Answer 54 · 2024-05-22T10:54:37.000Z

@mvdan It's related to;

opencontainers/runc#4265
to address opencontainers/runc#4195

I've been a bit slow on my involvement in runc recently, but @lifubang and @kolyshkin may be able to fill in the details (don't know if it has been reported; I for sure would love to see a fix in runc that doesn't involve hacks!)

Answer 55 · 2024-05-22T10:55:39.000Z

@thaJeztah what is the Go bug that runc is trying to work around with a linkname? Has it been filed?

#66797

Answer 56 · 2024-05-22T11:10:48.000Z

As you can see from the gopherbot spam, I've gotten through the modules with >= 1000 dependents total on deps.dev. I'm going to be offline for the next 8 hours or so but after that I will keep going through the remaining lists, as well as taking a second look at tfo-go for @database64128.

Answer 57 · 2024-05-22T19:56:03.000Z

Change https://go.dev/cl/587576 mentions this issue: all: document legacy //go:linkname for modules with ≥200 dependents

Answer 58 · 2024-05-22T19:56:03.000Z

Change https://go.dev/cl/587575 mentions this issue: all: document legacy //go:linkname for modules with ≥500 dependents

Answer 59 · 2024-05-22T21:22:21.000Z

Change https://go.dev/cl/587598 mentions this issue: all: document legacy //go:linkname for modules with ≥100 dependents

Answer 60 · 2024-05-23T00:49:57.000Z

@rsc Thank you for your reply. I am curious about what you mean by 'internal comment.' Are those comments merged into the master branch and visible to all members of the open-source community?

Answer 61 · 2024-05-23T00:58:58.000Z

@anthonywong1221 I mean that it is a comment internal to the source code, in contrast to being in the exported package documentation or a document on the Go web site or anything like that. It is still publicly visible, of course, but there are different levels of public visibility.

I had originally written something more generic, but my concern was that people would send CLs adding their own projects to try to keep a linkname in place for them, or even with new linknames. The current wording was chosen to make clear that being on the list is not a good thing.

Answer 62 · 2024-05-23T01:19:40.000Z

@anthonywong1221 As a not-so-innocent bystander (i.e. someone who has contributed to this issue) I have no problem being called out for hampering work. I don't imagine any body else is likely to take this as an insult; it's just a way of communicating intention.

Answer 63 · 2024-05-23T01:58:07.000Z

After 9a3ef86, quic-go cannot be compiled. It should be that only go:linkname cipherSuitesTLS13 is added, but go:linkname defaultCipherSuitesTLS13 and go:linkname defaultCipherSuitesTLS13NoAES are removed.

Answer 64 · 2024-05-23T03:12:53.000Z

Change https://go.dev/cl/587755 mentions this issue: crypto/tls: add linkname comments dropped from CL 587220

Answer 65 · 2024-05-23T03:12:53.000Z

Change https://go.dev/cl/587756 mentions this issue: all: document legacy //go:linkname for final round of modules

Answer 66 · 2024-05-23T03:12:55.000Z

@wwqgtxx Sorry, forgot to write out the changes in defaults.go during the merge. go.dev/cl/587755 fixes that.

Answer 67 · 2024-05-23T03:40:37.000Z

CL 587756 includes linknames used by tfo-go on unix platforms, but not Windows. Was that intentional?

Answer 68 · 2024-05-23T03:43:47.000Z

Thanks for the fix in https://go.dev/cl/587755, however https://go.dev/cl/587756 removed the export of net.defaultNS and we still need to use this variable, and there is currently no Alternatives (public API) can achieve this functionality. It can be seen in the github code base that there are many similar usage methods, so request to restore the export of this variable.

Answer 69 · 2024-05-23T03:44:06.000Z

@database64128 You got here before I did. I was going to post here once I finished adjusting the CLs.

I took a closer look at tfo-go, and even with the other copies the usage count is still very low, meaning the complexity we take on to support it needs to be similarly low. I'm willing to commit to keeping the portable linknames working (which will suffice for all the Unix platforms), but the Windows-specific linknames are far too invasive. They are exactly the kind of thing we are trying to avoid by locking down linkname in this issue. And since tfo-go has minimal usage overall and therefore probably even more minimal usage on Windows, it seems reasonable to push back on the Windows ones and suggest finding a different way to do that part. It seems like something that cares less about the net package innards should be possible on Windows the same as on Unix systems.

Answer 70 · 2024-05-23T03:47:24.000Z

@wwqgtxx In general the bar for adding linknames needs to be a substantial number of dependents, and metacubex/mihomo is barely used. That said, for defaultNS I see a bunch of usage across a variety of packages, and that's probably enough combined with how trivial it will be to keep working. I'll add it to the final CL.

Update: https://go-review.googlesource.com/c/go/+/587756/4/src/net/dnsconfig.go

Answer 71 · 2024-05-23T05:27:16.000Z

It seems like something that cares less about the net package innards should be possible on Windows the same as on Unix systems.

Currently there's no way to execute arbitrary async IO with netpoll on Windows, other than linknaming against execIO and friends. To list a few roadblocks:

The FileConn, FileListener, FilePacketConn functions in the net package are unimplemented for Windows and return syscall.EWINDOWS.
The current syscall.RawConn implementation on Windows cannot be used for any meaningful IO operations. In internal/poll/fd_windows.go, the (*FD).RawRead method does an extra "fake" read, and the (*FD).RawWrite method simply gives up and returns syscall.EWINDOWS.

This deserves a separate issue, and I'll open one later. But working through the problems above is going to take a lot of time. The necessary infrastructure won't be ready until many releases later. And we want tfo-go to keep working on Windows.

So, is there a middle ground where we add the necessary linkname annotations to the standard library, but do not lock down the affected internal API? As you said, there isn't a whole lot of usage of tfo-go. Therefore breaking tfo-go will not have any impact on the overall ecosystem.

Answer 72 · 2024-05-23T06:55:23.000Z

Thank you for tackling this problem. I have three questions:

What about eBPF programs using uprobes to access package internals? I've seen lots of code hooking into scheduler internals like runtime.execute and runtime.casgstatus.
What happens after this initial phase of stabilizing the current situation?
Is it okay to change the performance characteristics of the "frozen internals"? I.e. add allocations to convert from a new internal data structure back to the old one expected by the frozen API?

I really admire the Go team's dedication to compatibility, but I'm worried that freezing internal APIs based on popular usage in the ecosystem is setting a dangerous precedent. What if people find new ways to hack around the visibility rules? Will they also be rewarded with semi-official API access to go internals? What I'd love to see is a clear message that even the "frozen APIs" are just a temporary measure to stabilize the ecosystem, but that library authors should migrate away from them as these guarantees will be removed at some point in the future.

Answer 73 · 2024-05-23T07:31:06.000Z

I'm sorry that quic-go's usage of go:linkname caused pain.

I'd love to remove it, but as far as I can tell, there's no other way to configure TLS cipher suites. I'd like to point out that we're only using go:linkname for testing purposes, and we don't expose any API to quic-go users for this. We use it in unit / integration tests and in interop tests os the QUIC interop runner. This is necessary because the QUIC header protection key derivation differs quite a bit depending on whether one of the AES or the ChaCha cipher suite is negotiated, and it would be irresponsible to not test these code paths.

As soon as there's a different way to test these code paths, I'll happily move away from go:linkname. I'm not suggesting reintroducing configurability of TLS cipher suite via the tls.Config. Maybe we could introduce a GODEBUG flag, or is this too much of a fringe use case?

Answer 74 · 2024-05-23T08:29:10.000Z

We use it in unit / integration tests

My comment is probably very off-topic for this discussion, but for such cases I'd still love to see the ability to import exported test functions from other test code. Currently such functions tend to land in "non-_test.go" files if they're needed from different packages, which now (implicitly) makes them "production" code, and at risk of being consumed in situations where they shouldn't be. (I guess internal/ and possibly build-flags can aleviate some of this, but of course wouldn't be able to cross module boundaries, and wouldn't be easily usable to expose package internals for testing). (I'm pretty sure this has been proposed before, but my search-foo is failing me 🙈)

Answer 75 · 2024-05-23T08:57:37.000Z

I think @felixge's question involves a basic contradiction. The author of the library needs some means to access the unopened internal implementations in the golang standard library to achieve some functions that cannot be achieved based only on the public API. But after "lock down future uses of linkname", these visits were limited to a subset designed by the official based on popularity statistics. This resulted in the shift of maintainers compatible with Golang's internal changes from third-party library authors to standard library maintainers.

In the past, since there were no restrictions on linkname, third-party library authors could adjust the code in time based on user feedback. Although this would prevent users from updating to the latest golang tool chain, generally popular projects would not need to wait too long.

After implementing strict restrictions, the authors of third-party libraries need to wait for the official to open a hole, and do not know when this hole will be closed. Standard library maintainers also need to pay a huge price for this. Once the frozen structure needs to be modified, they will be under tremendous pressure, because this may be the only way for a third-party library to implement a function. (Even if other methods are technically feasible, third-party libraries cannot access it due to the new restrictions on linkname.)

linkname is indeed ugly, but when a large number of third-party libraries are used, it actually shows that there is a lack of formal and legal means in the standard library to implement it. For example, the question mentioned by the author of tfo-go above:

Currently there's no way to execute arbitrary async IO with netpoll on Windows, other than linknaming against execIO and friends.

Waiting for the standard library to complete the public API may take a lot of time. In the meantime, third-party library authors using some magic means to achieve creative work should not be regarded as a violation of compatibility. Further up, some members said that the very successful C language strictly limits access to symbols, but as another popular language, Python widely supports monkey patch methods to supplement the missing functions of the standard library.

It is conceivable that after golang1.23 is released, some private libraries that rely on linkname are not counted, and some new functions can still only be implemented through linkname, and the targets of these linknames are not included in the whitelist. As a result, a large number of build scripts were forced to add -checklinkname=0 to bypass these restrictions. Eventually, as time goes by, this tag may become a common way of writing for more and more people. By then, will a new issue be raised here to discuss whether the checklinkname tag needs to be abolished to protect the future go ecosystem?

Answer 76 · 2024-05-23T11:56:35.000Z

@database64128, re:

So, is there a middle ground where we add the necessary linkname annotations to the standard library, but do not lock down the affected internal API? As you said, there isn't a whole lot of usage of tfo-go. Therefore breaking tfo-go will not have any impact on the overall ecosystem.

The middle ground is that users of tfo-go on Windows can still build their programs with -ldflags=-checklinkname=0. Since there isn't a whole log of usage of tfo-go, that shouldn't affect too many people. But it also stops most of the Windows-using Go ecosystem from accidentally taking a dependency on tfo-go.

Answer 77 · 2024-05-23T12:12:13.000Z

@felixge, re:

Thank you for tackling this problem. I have three questions:

What about eBPF programs using uprobes to access package internals? I've seen lots of code hooking into scheduler internals like runtime.execute and runtime.casgstatus.

I'm mainly concerned with widespread usage by programs that don't realize they are depending on packages that depend on packages that depend on packages that depend on packages that depend on //go:linknames into Go internals. I don't think there are many such situations where programs don't know they depend on eBPF programs, so breaking these eBPF programs would not cause the kind of damage we are trying to avoid.

That said we're also not planning gratuitous changes, and execute and casgstatus are fairly stable fundamental operations, so probably such eBPF programs will keep working reasonably well.

What happens after this initial phase of stabilizing the current situation?

The main thing that happens is no new //go:linkname usage can be introduced as a far-off dependency of large fractions of the Go ecosystem. I don't anticipate significant cleanup or removal of the now-documented linknames if that's what you meant. I have been fairly careful about what I was willing to add linknames for (such as pushing back against tfo-go's Windows changes) and I feel okay about keeping the vast majority of those working. That said, if we did manage to cut usage of a linkname down to the level where next to nothing needs it anymore, then we might remove it.

Is it okay to change the performance characteristics of the "frozen internals"? I.e. add allocations to convert from a new internal data structure back to the old one expected by the frozen API?

Yes, that is okay. Of course we don't want to do that gratuitously, but one example where that might be necessary is if we change to a different map representation, and the map iterator struct that people have copied and used with mapiterinit needs to be bigger or have more pointers. We might have the "real" map iterator API mapiterinit2, mapiternext2, etc., and then implement the linknamed old routines by allocating a new iterator state in mapiterinit and storing it in a pointer-typed word of the old iterator struct, leaving the rest of the old struct unused.

I really admire the Go team's dedication to compatibility, but I'm worried that freezing internal APIs based on popular usage in the ecosystem is setting a dangerous precedent. What if people find new ways to hack around the visibility rules?

Do you know of any? I'd be happy to lock them down. It's not like the existence of //go:linkname was a surprise to us: it was an intentional feature, just one that we didn't realize would be quite so widely used.

Will they also be rewarded with semi-official API access to go internals?

It depends on how much of the Go ecosystem breaks without locking the access in. I agree that it's not ideal, but it's better than breaking users who have no idea they are even depending on these things.

What I'd love to see is a clear message that even the "frozen APIs" are just a temporary measure to stabilize the ecosystem, but that library authors should migrate away from them as these guarantees will be removed at some point in the future.

I'd love to see that too, but the fact is that getting packages updated and then getting usage updated takes a very long time. Also as evidenced by the other discussion on this issue, in some cases we need to provide alternatives first, and getting public APIs we are happy with will also be a challenge. I can see these being temporary, but more on the scale of a decade than on the scale of a year.

Answer 78 · 2024-05-23T13:40:09.000Z

Change https://go.dev/cl/587795 mentions this issue: cmd/go,testdeps: move import of internal/coverage/cfile to testmain

Answer 79 · 2024-05-23T15:49:26.000Z

@wwqgtxx, re:

It is conceivable that after golang1.23 is released, some private libraries that rely on linkname are not counted, and some new functions can still only be implemented through linkname, and the targets of these linknames are not included in the whitelist. As a result, a large number of build scripts were forced to add -checklinkname=0 to bypass these restrictions. Eventually, as time goes by, this tag may become a common way of writing for more and more people. By then, will a new issue be raised here to discuss whether the checklinkname tag needs to be abolished to protect the future go ecosystem?

I don't think this is at all likely. The vast majority of the ecosystem will refuse to add -ldflags=-checklinkname=0 to their go install invocations. Most major projects in particular will not adopt it, and that should create significant backpressure to prevent widespread use of packages that need the flag.

I do agree with you that the linkname 'heap map' is a good indicator of where we are missing important APIs. Addressing that is a longer term project.

Answer 80 · 2024-05-23T20:47:37.000Z

Change https://go.dev/cl/587918 mentions this issue: syscall: rm go:linkname from origRlimitNofile

Answer 81 · 2024-05-24T04:58:16.000Z

@wwqgtxx proposed a change (MetaCubeX/tfo-go@7577e13) for tfo-go that would reduce the number of necessary linknames for Windows. Here's the new list:

//go:linkname sockaddrToTCP net.sockaddrToTCP
func sockaddrToTCP(sa syscall.Sockaddr) net.Addr

//go:linkname execIO internal/poll.execIO
func execIO(o *operation, submit func(o *operation) error) (int, error)

//go:linkname fdInit internal/poll.(*FD).Init
func fdInit(fd *pFD, net string, pollable bool) (string, error)

@rsc Can you please take another look and see if it's now eligible for inclusion? Thanks.

Answer 82 · 2024-05-24T09:00:20.000Z

I fully understand that you are intending to avoid unnecessary pulling of dependencies that breaks when go language standard libraries updates. I agree that is something that would be very welcomed in the community and I personally sometimes feels sad when a several libraries I imported don’t like each other as they depend on different versions of the same library, not to say different versions of the standard libraries. However, without viable alternatives to the link name as a means of accessing private functions that would be absolutely necessary to implement some features, libraries authors would need to rely on more unsupported means to accessing these functions like https://github.com/spance/go-callprivate.

There is a way to fix this with a one time effort that is to separate Go standard library from Go runtime, by making sure standard libraries itself only use exported public functionalities of the Go runtime, thus allow any developers to implement whatever functionality they needed for any particular system without having to resort to use golang’s standard library and create a level playing field for all Go developers. By doing so, there will be a finite amount of public runtime API that will need to be maintained, without further limiting what user could do with go programming languages.

The current way of creating an allowlist of functions that could be called is unfair and won’t work: it is unfair that Google affiliated project like gvisor could just add whatever they needed to the allowlist, while other developers are forced to remove functionalities from their projects because they are underprivileged; and it won’t work because these restrictions will be workaround easily with some engineering trick and reflect+unsafe already allow projects to break when private field of struct updates.

Answer 83 · 2024-05-24T13:58:28.000Z

@xiaokangwang use -ldflags=-checklinkname=0 and raise issue for missing APIs.

Russ Cox said about addressing that:

Addressing that (missing APIs) is a longer term project.

Answer 84 · 2024-05-24T14:45:53.000Z

@xiaokangwang use -ldflags=-checklinkname=0 and raise issue for missing APIs.

Russ Cox said about addressing that:

Addressing that (missing APIs) is a longer term project.

As we have already seen, they are not adding every API that are necessary to implement the features users wants, as they are prioritising projects with elevated privileges and refuse to make changes and maintain API for under privileged projects. This is not a solution that would work for every projects.

Answer 85 · 2024-05-24T18:50:01.000Z

As we have already seen, they are not adding every API that are necessary to implement the features users wants, as they are prioritising projects with elevated privileges and refuse to make changes and maintain API for under privileged projects.

No language ever does. The problem with separating standard library and runtime API is that you are forced to support it in future and stuck with the semantics you defined. I don't think any language allows full separate std implementation.

This is not a solution that would work for every projects.

While project is getting traction -ldflags=-checklinkname=0 should be enough. I agree that this is not ideal, but if my dep modules are trying to get into new or unopened internals I want to know why.

Answer 86 · 2024-05-25T00:46:56.000Z

@dmitshur Actually for C it is trivial to switch out the standard library which is why we have projects like musl. One downside of the current Go compiler is that that standard library and the compiler are very tightly coupled. This makes it harder to write an alternative standard library, and leads to people using this linkname hack to bypass the limitations of the standard library. In the long run it would be better to try and decouple the compiler and std library as much as possible.

Answer 87 · 2024-05-25T07:03:28.000Z

@bjorndm

Actually for C it is trivial to switch out the standard library which is why we have projects like musl.

Almost zero runtime. I think Rust tries to go there with no_std but I don't know how much progress they made.

One downside of the current Go compiler is that that standard library and the compiler are very tightly coupled

Go is garbage collected language with custom lightweight threads implementation. Both of those require very complicated runtime. Comparing with C is not exactly fair here.

Answer 88 · 2024-05-25T10:18:01.000Z

I admit the Go runtime is way more useful than and hence way more complicated than that of C. Still I am surprised how many projects are using linkname to hack the runtime. This seems to indicate certain needs of Go developers are not being met.

Answer 89 · 2024-05-25T23:44:21.000Z

There are other implementations of Go, e.g. gccgo, gollvm, TinyGo, and perhaps GopherJS. (They may have limitations, may be incomplete, or special purposed.) They have different implementation details. Using linkname to access internal details likely don't work with them. If one wants to write portable Go code, one should avoid that. Unfortunately, many people don't care.

Answer 90 · 2024-05-26T00:04:19.000Z

Yes, and I do appreciate the efforts of those who are making the alternative Go compilers, including your work on GCC go. What I am getting at is that it seems we need some portable way to do certain kinds of low level programming in Go.

Answer 91 · 2024-05-27T04:45:50.000Z

As the maintainer of Sonic (github.com/bytedance/sonic), I am deeply concerned about the proposal to discard go:linkname. Sonic relies heavily on go:linkname for implementing critical optimizations and features, such as JIT compilation and fast Go-C intercalls. These enhancements significantly boost the performance of applications developed using Go.

If this proposal is adopted, it will adversely affect one of the fastest JSON libraries available for Go. More importantly, it will dampen the enthusiasm of seasoned developers who leverage these advanced techniques to push the boundaries of Go's performance.

Answer 92 · 2024-05-27T06:22:52.000Z

As much as I understand the reasoning behind this. It completely goes against the promise of "not breaking things". Is go:linkname the best way to handle things? No, but it should be left to the developers of the broken packages to fix them, not Golang.

The main reason I've seen (and used) this directive is to gain access to functions that I'd argue shouldn't be hidden behind the runtime or change the behavior of the runtime from some undesirable behavior.

I could totally understand the vet check. That'd keep it as a hey you better know what you're doing but allow advanced users to do what they need to do. I understand the "opt-in" nature of the suggestion also, but it would cause multiple projects and workflows to now be reconfigured (yes I know it's small) over some issues with a small amount of packages.

The vet check should stay, the compile check nope.

Answer 93 · 2024-05-27T07:11:43.000Z

The go compatibility promise https://go.dev/doc/go1compat has an explicit exception for undefined and unspecified behavior. The //go:linkname directive is not specified by the Go spec, so it falls under this. Conformant Go compilers may simply consider this as a comment. While I agree that there is a need for low level access to the runtime, using undefined behavior to do this is a recipe for disaster. The Go developers are not bound to keep this working.

Answer 94 · 2024-05-27T07:55:35.000Z

I'm not quite sure if it is valid. If A.foo and B.foo agree with the name A.foo under the Handshake situation, and someday A.foo changes to A.bar. Now B should change to A.bar. But under the Pull situation, B should also change to bar to keep compatibility with A. It has no difference with the Handshake situation. So what's the point of making this change? Only for the internal-external concern?

If I have any mistake, pls comment and let me know. Thanks.

Answer 95 · 2024-05-27T12:35:09.000Z

i had no idea that projects were reaching into go runtime internals like this. I understand the compromise being made here to whitelist projects to avoid large breakages in the go ecosystem.

however, I want nothing to do with such dependencies direct or transitive. And in the medium term it would be great if the ecosystem identified these libraries and en masse removed them from their dependencies. to do that, we need an efficient way to identify them.

could checklinkname have an additional mode that would unconditionally error out whenever any third party library is trying to reach into runtime internals? This can’t be the default but i’d sure set it when i am compiling.

Answer 96 · 2024-05-27T19:20:22.000Z

@AsterDY

If this proposal is adopted, it will adversely affect one of the fastest JSON libraries available for Go. More importantly, it will dampen the enthusiasm of seasoned developers who leverage these advanced techniques to push the boundaries of Go's performance.

First of all there is -ldflags=-checklinkname=0 that people can use if they want to get as much as they are willing to risk. Second of all, demanding Go core team to support stuff which was never intended to be public in the first place is a bad tone.

@iDigitalFlame

The vet check should stay, the compile check nope.

There is no way to go vet modules recursively. Never was, it's too expensive.

I could totally understand the vet check. That'd keep it as a hey you better know what you're doing but allow advanced users to do what they need to do. I understand the "opt-in" nature of the suggestion also, but it would cause multiple projects and workflows to now be reconfigured (yes I know it's small) over some issues with a small amount of packages.

go:linkname was never covered by Go 1 compat. And never will be. The reason we are having this conversation right now is because some modules developers didn't care about making fallbacks and didn't properly build tag those go:linkname usages in the first place. While I constantly use go:linkname in my day-job, I fully support "explicit opt-in" because those things are internals, so I commit to support the situation where they may be removed.

The main reason I've seen (and used) this directive is to gain access to functions that I'd argue shouldn't be hidden behind the runtime or change the behavior of the runtime from some undesirable behavior.

Make a proposal. Raise an issue. But don't expect people to support private functions just because you need those.

Answer 97 · 2024-05-27T19:29:35.000Z

@DmitriyMV

There is no way to go vet modules recursively. Never was, it's too expensive.

Never said this should be done on modules. It's on the module developer to do this, so a go vet from the module developer's standpoint makes sense.

And never will be. The reason we are having this conversation right now is because some modules developers didn't care about making fallbacks and didn't properly build tag those go:linkname usages in the first place.

Sure, but that's a problem, I make modules that relay on it without breaking, so why should I have to deal with this because some other developer's code breaks?

While I constantly use go:linkname in my day-job, I fully support "explicit opt-in" because those things are internals, so I commit to support the situation where they may be removed.

That's fine, but I understand the internals may change but I update my code to support the changes. It isn't hard. However removing the way I can get the access is not something many of us agreed to (looking at the comments).

Make a proposal. Raise an issue. But don't expect people to support private functions just because you need those.

Good joke, I understand that it wouldn't happen, and that's fine. The Go team is very conservative in adding features, totally understandable. But why limit the people who use it? Especially for a feature they would never add? I'm pretty sure if you needed a feature that wouldn't pass a proposal, you'd have not qualms using go:linkname if you have to.

Answer 98 · 2024-05-27T19:32:01.000Z

@bjorndm

While I agree that there is a need for low level access to the runtime, using undefined behavior to do this is a recipe for disaster. The Go developers are not bound to keep this working.

And I understand that, but if you recognize the need then why be ok with this change?

Answer 99 · 2024-05-27T19:33:49.000Z

Just to be clear, I think the A.foo B.foo situation with external packages makes sense. But anything in the stdlib should be allowed, as there are many useful functions that are sometimes needed but gated away by export-ability.

Answer 100 · 2024-05-27T19:42:07.000Z

Never said this should be done on modules. It's on the module developer to do this, so a go vet from the module developer's standpoint makes sense.

If you are the only one who are using the code -ldflags=-checklinkname=0 should be a trivial change.

Sure, but that's a problem, I make modules that relay on it without breaking, so why should I have to deal with this because some other developer's code breaks?

Because someone decides to use your code as a dependency and someone else decides to use new code as their dependency and so on until we have big project relying on go:linkname usages somewhere down the line.

However removing the way I can get the access is not something many of us agreed to (looking at the comments).

Once again -ldflags=-checklinkname=0. Nothing changes from your perspective, Go team simply wants your confirmation about your intentions. And confirmation from people who are using your code.

I'm pretty sure if you needed a feature that wouldn't pass a proposal, you'd have not qualms using go:linkname if you have to.

Thats why I'm fine with -ldflags=-checklinkname=0.