cmd/compile: add nostackallocvariablemake goexperiment

[qmuntal]

CL 653856 made small variable-sized make calls to allocate on the stack rather than on the heap. This is causing widespread failures among Windows code using syscall and x/sys/windows, given that it is easy to take wrong assumptions about object lifespans and memory layout. See for example #73199 and #73170.

We will fix the bugs found in the Go standard library and in x/sys/windows, but we don't have enough code coverage to be 100% sure all bugs related to CL 653856 are found and fixed, and we can't control third-party code making wrong assumptions.

We should add a goexperiment setting to disable stack allocated variable-sized make calls to facilitate upgrading to Go 1.25. This setting could be named nostackallocvariablemake..

@randall77 @golang/compiler

[mateusz834]

Godebugs are a runtime thing, right? That would have to be a goexperiment as this was a compiler change, right?

[qmuntal]

Godebugs are a runtime thing, right? That would have to be a goexperiment as this was a compiler change, right?

Good catch, will update the issue.

[thepudds]

FWIW, when I was looking at some of the related problems earlier, I had been thinking along similar lines, except I was thinking it might be better to do a compiler debug flag (rather than a GOEXPERIMENT or GODEBUG flag). It's maybe a little more convenient to be able to enable/disable per package and there are a couple of other reasons, though GOEXPERIMENT could be a reasonable choice as well.

In any event, I sent https://go.dev/cl/663795 for discussion. (Still WIP).

@qmuntal, let me know if that overlaps with something you already have in progress.

We can probably hash out best approach here (GOEXPERIMENT or GODEBUG or cmd/compile debug flag or ...).

I think would also be nice to hook up to the hash-based bisect, which I plan to try in a bit.

[randall77]

I think a GOEXPERIMENT makes the most sense.
We could conceivably make this a GODEBUG, at the cost of a global flag check in the generated code. I will look into how feasible that would be.

[mateusz834]

We could conceivably make this a GODEBUG, at the cost of a global flag check in the generated code. I will look into how feasible that would be.

What about a Default GOEXPERIMENT value based on go.mod version? Is that possible?

[randall77]

That would certainly be possible. We did it for the loop variable change, I believe.
That said, that was for a change to Go program semantics. The changes we're discussing here are not in that category, so it may be overkill.

[gopherbot]

Change https://go.dev/cl/663795 mentions this issue: cmd/compile/internal/escape: add hash for bisecting stack allocation of variable-sized makeslice

[thepudds]

Hi @randall77, using a GOEXPERIMENT seems reasonable. It sounds like you are going to look into some options there.

Based on that, I updated my WIP https://go.dev/cl/663795 to git rid of the enable/disable compiler debug flag.

As I mentioned above, I was planning to hook up the hash-based bisect, which is now in the WIP https://go.dev/cl/663795, with an example result here:

$ bisect -compile=variablemake go test -skip TestListWireGuardDrivers
  [...]
  bisect: FOUND failing change set
  --- change set [#1](https://go.dev/issue/1) (enabling changes causes failure)
  ./security_windows.go:1321:38 (variablemake)
  ./security_windows.go:1321:38 (variablemake)
  ---

I think that is mostly orthogonal to GOEXPERIMENT, but if not, I can abandon that CL if that's easier for you.

[gopherbot]

Change https://go.dev/cl/663935 mentions this issue: cmd/compile: add debug flag to disable stack allocation of backing stores

[randall77]

The above change adds a GODEBUG. Pro, you can switch it on and off at run time, not compile time. Con 1, it takes 2 extra instructions per allocation. Con 2, there's no way to bisect it. It is on or off everywhere.

[randall77]

I do wonder about the usefulness of bisection here. The kind of failures that happen might be pretty nondeterministic, as they depend on when GC and/or stack copying run. That makes bisecting kind of a pain.

[gabyhelp]

Related Code Changes

• cmd/compile/internal/escape: add hash for bisecting stack allocation of variable-sized makeslice

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[randall77]

Another option: put this new mechanism behind -N. That's probably worth doing regardless of this issue. The question is, is that enough?

[thepudds]

I do wonder about the usefulness of bisection here. The kind of failures that happen might be pretty nondeterministic

Hi Keith, FWIW, of the three variations I've seen at the point, two seemed 100% reproducible and bisect worked. The third (#73170) was definitely nondeterministic.

That's just N=3, and there could be a bias where early finds might tend to be more deterministic, so it wouldn't be shocking if whatever else pops out could be mostly nondeterministic...

But bisect is kinda handy, and https://go.dev/cl/663795 is modest size. If we fast forward 6-9 months where people are opening issues, it could be nice to ask people to bisect if they do have something reproducible that they can't figure out on their own based on code inspection, diff'ing -m=1 logs, etc.

The above change adds a GODEBUG. Pro, you can switch it on and off at run time, not compile time. Con 1, it takes 2 extra instructions per allocation. Con 2, there's no way to bisect it. It is on or off everywhere.

We could in theory have both a GODEBUG (for being able to disable without recompile) and bisect, but it might be overkill to do both.

In any event, it might be that there are two mostly independent questions:

1. what's the choice for how to turn it off (GODEBUG, GOEXPERIMENT, -N, or maybe even using n with the debug hash flag, or something else), and then separately
2. do we want bisect?

I'm happy to go in any direction here.

[gopherbot]

Change https://go.dev/cl/664655 mentions this issue: cmd/compile: turn off variable-sized make() stack allocation with -N

[randall77]

Ok, I think I'm happy with turning things off with -N, and Jorropo's hash debug CL. I don't think we need the GODEBUG also. The GODEBUG costs a bit of runtime, and it doesn't help any with finding the bad instance.

[qmuntal]

Ok, I think I'm happy with turning things off with -N, and Jorropo's hash debug CL.

Works for me, thanks!

[thepudds]

Hi @randall77, sounds good. 👍

Which hash debug CL -- does Jorropo have one?

[randall77]

Sorry, me confused. I meant yours. 663795.

[thepudds]

@Jorropo is an unstoppable force, so I will very much take it as a compliment. 😅