runtime: reduce static stack guard size #51256
Comments
dmitshur
added
the
NeedsInvestigation
|
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 I'm slightly leaning toward option 4, but I worry that this whole mechanism is getting shaky. |
|
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.
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 |
|
@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. |
gopherbot
added
the
compiler/runtime
|
Change https://go.dev/cl/416859 mentions this issue: |
|
Sent CL 416859 to remove stackGuardMultiplierDefault, but this issue should remain open to track what @aclements wrote above, namely "whether we could eliminate or at least significantly reduce the static stack guard size". |
rsc
changed the title
Nothing seems to break, not even the noopt builder. For #51256 (the conversation there is headed toward additional changes). Change-Id: Icb7ca451159a74f351c25d2cefb32c773b9bb017 Reviewed-on: https://go-review.googlesource.com/c/go/+/416859 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Nothing seems to break, not even the noopt builder. For golang#51256 (the conversation there is headed toward additional changes). Change-Id: Icb7ca451159a74f351c25d2cefb32c773b9bb017 Reviewed-on: https://go-review.googlesource.com/c/go/+/416859 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
and others
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 theGO_GCFLAGSincludes-Nwhen 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 -lbuild 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,stackGuardMultiplierDefaultis still 1. So we're effectively keeping-N -lbuild 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 -lbuild that is actually useful. I can think of a few possibilities:-Nis specified. Requires-Nto be used for all packages or nothing. And needs some magic way to set the value in the runtime and tell the linker.double the stack guard if-Nis 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).-N -lto all packages and double the stack guard. Deprecate the direct use of-N -l.-Nno-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-Nis not debugging those packages)Maybe there are other options.
Or, drop
stackGuardMultiplierDefaultand keep-N -lbuild work with the non-doubled stack guard. Again, this is sometimes difficult.cc @aclements
The text was updated successfully, but these errors were encountered: