cmd/dist, cmd/internal/objabi, runtime: stackGuardMultiplierDefault is not useful

[cherrymui]

What version of Go are you using (go version)?

tip (eaf0405)

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

In cmd/internal/objabi and runtime we have stackGuardMultiplierDefault, which is normally 1 but set to 2 by cmd/dist if the GO_GCFLAGS includes -N when building the toolchain and the standard library at make.bash(bat/rc) time. The idea is that for a no-opt build where the functions consume more stack space, we double the stack guard to avoid nosplit overflow.

But I suspect most users who use -N -l build do not build the toolchain and the standard library this way. It is more likely that one would build the toolchain normally but use -gcflags=all="-N -l" on the command line (at least since the introduce of build cache and automatic rebuild of the standard library packages). This way, stackGuardMultiplierDefault is still 1. So we're effectively keeping -N -l build work with the non-doubled stack guard, which sometimes is difficult.

What did you expect to see?

Have some way to double the stack guard with -N -l build that is actually useful. I can think of a few possibilities:

1. double the stack guard if -N is specified. Requires -N to be used for all packages or nothing. And needs some magic way to set the value in the runtime and tell the linker.
2. double the stack guard if -N is specified when compiling the runtime. The runtime and the linker will use the doubled stack guard (needs the same magic way). When compiling other packages it may still use the smaller one, but it probably doesn't hurt (besides slightly more frequent stack copying).
3. define a noopt (or debug or something) flag for the go command that applies -N -l to all packages and double the stack guard. Deprecate the direct use of -N -l.
4. make -N no-op for a few more nosplit-heavy packages. It is already no-op for the runtime package. Maybe it can also apply to reflect and syscall. (I suspect most times the user who use -N is not debugging those packages)

Maybe there are other options.

Or, drop stackGuardMultiplierDefault and keep -N -l build work with the non-doubled stack guard. Again, this is sometimes difficult.

cc @aclements

[aclements]

Stepping back, unless I'm mistaken, it seems like it's not necessarily safe to mix different size stack guards in the same process. If a splittable function in a package compiled with a small stack guard calls a nosplit function in a package compiled with a large stack guard, this could corrupt the stack. I think this means option 2 doesn't quite work.

I suppose the linker's stack size check could check for this, but it looks like that uses the dist-baked stack guard, so it isn't affected by -N and has no notion that different functions could ensure different guard zones are left. I think this means the linker's check is always conservative right now, which prevents us from corrupting the stack, but can cause surprise build failures.

I certainly agree that it seems like stackGuardMultiplierDefault is a footgun at best.

I'm slightly leaning toward option 4, but I worry that this whole mechanism is getting shaky.

[cherrymui]

Yeah, I agree option 2 doesn't work. It might work if the run-time check is more conservative (using larger stack guard) than the actual limit, but that doesn't help anything.

I think this means the linker's check is always conservative right now, which prevents us from corrupting the stack, but can cause surprise build failures.

Yes, this is what is happening now. It is safe. But sometimes we need to work quite hard to fit things into the threshold with -N build, e.g. #45698, #45658, #51247.

Yeah, option 4 isn't ideal. E.g. in the future we may add nosplit functions in other places. Also, what about -l? There are quite a few simple helper functions (for better readability) which would normally be inlined but could use some stack in -l build.

[aclements]

@cherrymui and I were just chatting about this. I charged her to think about whether we could eliminate or at least significantly reduce the static stack guard size. She proposed an interesting solution: when you're about to make a splittable -> nosplit transition, before setting up the arguments to the nosplit call, grow the stack to ensure there's enough space for the nosplit chain. It would be pretty easy for the linker to patch in the necessary stack depth (since only it really knows the depth of nosplit chains).

We'd have to be careful about nosplit functions put into closures (and perhaps those that have their PC taken). These should be pretty rare. Perhaps for those we keep a static stack guard size and any nosplit function reference that isn't part of a "grow stack and call" sequence gets checked against the static guard, like we do now.