There was only one meaningful caller, which changes to call time_now.
This clearly separates systems that use walltime1 to be just those
that use the stub version of time_now. That is to say, those that do
not provide an assembler version of time_now.
Change-Id: I14c06cc402070bd705f953af6f9966785015e2a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/314769
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Build other definitions with the !faketime build tag.
This makes it easy for us to add new assembly implementations of time.now.
Change-Id: I4e48e41a4a04ab001030e6d1cdd9cebfa0161b0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/314274
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Block profiles were biased towards infrequent long events over frequent
short events. This fix corrects the bias by aggregating shorter events
as longer but less frequent in the profiles. As a result their
cumulative duration will be accurately represented in the profile
without skewing their sample mean (duration/count).
Credit to @dvyukov for suggesting to adjust the count in the
saveblockevent function.
Fixes#44192.
Change-Id: I71a99d7f6ebdb2d484d44890a2517863cceb4004
Reviewed-on: https://go-review.googlesource.com/c/go/+/299991
Trust: Michael Pratt <mpratt@google.com>
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Currently tiny allocations are not represented in either MemStats or
runtime/metrics, but they're represented in MemStats (indirectly) via
Mallocs. Add them to runtime/metrics by first merging
memstats.tinyallocs into consistentHeapStats (just for simplicity; it's
monotonic so metrics would still be self-consistent if we just read it
atomically) and then adding /gc/heap/tiny/allocs:objects to the list of
supported metrics.
Change-Id: Ie478006ab942a3e877b4a79065ffa43569722f3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/312909
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
While debugging issue #45638, I discovered that some tests were using
--buildmode command line parameter instead of -buildmode.
The --buildmode parameter is handled properly by the flag package - it
is read as -buildmode. But we should correct code anyway.
Updates #45638
Change-Id: I75cf95c7d11dcdf4aeccf568b2dea77bd8942352
Reviewed-on: https://go-review.googlesource.com/c/go/+/313351
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
CL 211139 added TestLibraryCtrlHandler. But the CL left out import "C"
line in the test file that is supposed to be build with Cgo.
While debugging issue #45638, I discovered that the DLL built during
TestLibraryCtrlHandler does not have Dummy function. Adding import "C"
makes Dummy function appear in DLL function list.
TestLibraryCtrlHandler does not actually calls Dummy function. So I
don't see how this change affects issue #45638, but still let's make
this code correct.
Updates #45638
Change-Id: Ibab8fed29ef2ae446d0815842cf0bd040a5fb943
Reviewed-on: https://go-review.googlesource.com/c/go/+/313350
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
In CL 288092 we made Darwin syscall wrappers as ABIInternal, so
their addresses taken from Go using funcPC are the actual function
entries, not the wrappers.
As we introduced internal/abi.FuncPCABIxxx intrinsics, use that.
And change the assembly functions back to ABI0.
Change-Id: I50645af74883e2d5dfcd67a5e8c739222c6f645b
Reviewed-on: https://go-review.googlesource.com/c/go/+/313250
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently there's a minor bug where the constant for the min fraction of
time spent scavenging is rounded down to zero. I don't think this
affects anything in practice because this case is exceedingly rare and
extreme, but currently it doesn't properly prevent the pacing parameters
from getting out of hand in these extreme cases.
Fixes#44036.
Change-Id: I7de644ab0ecac33765c337a736482a0966882780
Reviewed-on: https://go-review.googlesource.com/c/go/+/313249
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Currently this test attempts to trigger a concurrent GC in a very
indirect way, but the way it does so is extremely error-prone. This test
is virtually always prone to flaking based on test order. For example if
the test that executed immediately before this one made a big heap but
didn't clean it up, then this test could easily fail to trigger a GC.
I was able to prove this with a small reproducer.
This roundabout way of triggering a GC is also way overkill for this
test. It just wants to get goroutines in a select and shrink their
stacks. Every GC will schedule a stack for shrinking if it can.
Replace all the complicated machinery with a single runtime.GC call.
I've confirmed that the test consistently triggers a stack shrink,
noting that both shrinkstack's copystack call is made and that
syncadjustsudogs (the relevant function that's being indirectly tested)
are both called.
Fixes#44610.
Change-Id: Ib1c091e0d1475bf6c596f56dc9b85eaea366fc73
Reviewed-on: https://go-review.googlesource.com/c/go/+/313109
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Fixes the previously failing TestStdcallAndCDeclCallbacks
for the 9+ argument case.
The last time this code passed, the invisible frame pointer
below SP was apparently not enabled on windows/arm64.
Change-Id: Ifc3064e894b2f39d6410f3be51c17309ebab08a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/312042
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When ABI wrappers are used, there are cases where in Go code we
need the PC of the defined function instead of the ABI wrapper.
Currently we work around this by define such functions as
ABIInternal, even if they do not actually follow the internal ABI.
This CL introduces internal/abi.FuncPCABIxxx functions as compiler
intrinsics, which return the underlying defined function's entry
PC if the argument is a direct reference of a function of the
expected ABI, and reject it if it is of a different ABI.
As a proof of concept, change runtime.goexit back to ABI0 and use
internal/abi.FuncPCABI0 to retrieve its PC.
Updates #44065.
Change-Id: I02286f0f9d99e6a3090f9e8169dbafc6804a2da6
Reviewed-on: https://go-review.googlesource.com/c/go/+/304232
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This change adds a metric to track scheduling latencies, defined as the
cumulative amount of time a goroutine spends being runnable before
running again. The metric is an approximations and samples instead of
trying to record every goroutine scheduling latency.
This change was primarily authored by mknyszek@google.com.
Change-Id: Ie0be7e6e7be421572eb2317d3dd8dd6f3d6aa152
Reviewed-on: https://go-review.googlesource.com/c/go/+/308933
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When an M transitions from spinning to non-spinning state, it must
recheck most sources of work to avoid missing work submitted between its
initial check and decrementing sched.nmspinning (see "delicate dance"
comment).
Ever since the scheduler rewrite in Go 1.1 (golang.org/cl/7314062), we
have performed this recheck on all Ms before stopping, regardless of
whether or not they were spinning.
Unfortunately, there is a problem with this approach: non-spinning Ms
are not eligible to steal work (note the skip over the stealWork block),
but can detect work during the recheck. If there is work available, this
non-spinning M will jump to top, skip stealing, land in recheck again,
and repeat. i.e., it will spin uselessly.
The spin is bounded. This can only occur if there is another spinning M,
which will either take the work, allowing this M to stop, or take some
other work, allowing this M to upgrade to spinning. But the spinning is
ultimately just a fancy spin-wait.
golang.org/issue/43997 discusses several ways to address this. This CL
takes the simplest approach: skipping the recheck on non-spinning Ms and
allowing them to go to stop.
Results for scheduler-relevant runtime and time benchmarks can be found
at https://perf.golang.org/search?q=upload:20210420.5.
The new BenchmarkCreateGoroutinesSingle is a characteristic example
workload that hits this issue hard. A single M readies lots of work
without itself parking. Other Ms must spin to steal work, which is very
short-lived, forcing those Ms to spin again. Some of the Ms will be
non-spinning and hit the above bug.
With this fixed, that benchmark drops in CPU usage by a massive 68%, and
wall time 24%. BenchmarkNetpollBreak shows similar drops because it is
unintentionally almost the same benchmark (create short-living Gs in a
loop). Typical well-behaved programs show little change.
We also measure scheduling latency (time from goready to execute). Note
that many of these benchmarks are very noisy because they don't involve
much scheduling. Those that do, like CreateGoroutinesSingle, are
expected to increase as we are replacing unintentional spin waiting with
a real park.
Fixes#43997
Change-Id: Ie1d1e1800f393cee1792455412caaa5865d13562
Reviewed-on: https://go-review.googlesource.com/c/go/+/310850
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently, when the runtime printing a stack track (at panic, or
when runtime.Stack is called), it prints the function arguments
as words in memory. With a register-based calling convention,
the layout of argument area of the memory changes, so the
printing also needs to change. In particular, the memory order
and the syntax order of the arguments may differ. To address
that, this CL lets the compiler to emit some metadata about the
memory layout of the arguments, and the runtime will use this
information to print arguments in syntax order.
Previously we print the memory contents of the results along with
the arguments. The results are likely uninitialized when the
traceback is taken, so that information is rarely useful. Also,
with a register-based calling convention the results may not
have corresponding locations in memory. This CL changes it to not
print results.
Previously the runtime simply prints the memory contents as
pointer-sized words. With a register-based calling convention,
as the layout changes, arguments that were packed in one word
may no longer be in one word. Also, as the spill slots are not
always initialized, it is possible that some part of a word
contains useful informationwhile the rest contains garbage.
Instead of letting the runtime recreating the ABI0 layout and
print them as words, we now print each component separately.
Aggregate-typed argument/component is surrounded by "{}".
For example, for a function
F(int, [3]byte, byte) int
when called as F(1, [3]byte{2, 3, 4}, 5), it used to print
F(0x1, 0x5040302, 0xXXXXXXXX) // assuming little endian, 0xXXXXXXXX is uninitilized result
Now prints
F(0x1, {0x2, 0x3, 0x4}, 0x5).
Note: the liveness tracking of the spill splots has not been
implemented in this CL. Currently the runtime just assumes all
the slots are live and print them all.
Increase binary sizes by ~1.5%.
old new
hello (println) 1171328 1187712 (+1.4%)
hello (fmt) 1877024 1901600 (+1.3%)
cmd/compile 22326928 22662800 (+1.5%)
cmd/go 13505024 13726208 (+1.6%)
Updates #40724.
Change-Id: I351e0bf497f99bdbb3f91df2fb17e3c2c5c316dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/304470
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
cgocallback calls cgocallbackg after switching the stack. Call it
indirectly to bypass the linker's nosplit check.
Apparently (at least on Windows) cgocallbackg can use quite a bit
stack space in a nosplit chain. We have been running over the
nosplit limit, or very close to the limit. Since it switches
stack in cgocallback, it is not meaningful to count frames above
cgocallback and below cgocallbackg together. Bypass the check.
For #45658.
Change-Id: Ie22017e3f82d2c1fcc37336696f2d02757856399
Reviewed-on: https://go-review.googlesource.com/c/go/+/312669
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
It sometimes seems to time out on slow systems, perhaps due to
being run at the same time as a lot of other work.
Also move the code to testdata/testprog, so that we don't have to
build it separately.
I hope that this
Fixes#35356
Change-Id: I875b858fa23836513ae14d3116461e22fffd5352
Reviewed-on: https://go-review.googlesource.com/c/go/+/312510
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Currently we call runtimeQPC as ABIInternal because it shaves off 24
bytes by not having an extra wrapper, and at the time we were exceeding
the nosplit stack limit in some cases.
However, this code was written before we had the regabiargs GOEXPERIMENT
flag, and wasn't properly flagged. Naturally, with regabiargs enabled,
it leads to garbage being returned, because it needs to store
runtimeQPC's result to the stack.
We didn't notice this because today runtimeQPC is only used in Wine, not
on any native Windows platform.
Back when I wrote this code, it appeared to be necessary on even native
Windows, but it turns out that's not true anymore. Turn it back into a
native call through a wrapper.
For #40724.
Change-Id: Ia2e5901965ef46c5f299daccef49952026854fe6
Reviewed-on: https://go-review.googlesource.com/c/go/+/312429
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
The runtime support for syscall.AllThreadsSyscall() functions had
some corner case deadlock issues when signal handling was in use.
This was observed in at least 3 build test failures on ppc64 and
amd64 architecture CGO_ENABLED=0 builds over the last few months.
The fixes involve more controlled handling of signals while the
AllThreads mechanism is being executed. Further details are
discussed in bug #44193.
The all-threads syscall support is new in go1.16, so earlier
releases are not affected by this bug.
Fixes#44193
Change-Id: I01ba8508a6e1bb2d872751f50da86dd07911a41d
Reviewed-on: https://go-review.googlesource.com/c/go/+/305149
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Pratt <mpratt@google.com>
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
When rechecking for work after transitioning from a spinning to
non-spinning M, checking timers before GC isn't useful. That is, if
there is GC work available, it will run immediately and the updated
pollUntil is unused.
Move this check to just before netpoll, where pollUntil is used. While
this technically improves efficiency in the (rare) case that we find
GC work in this block, the primary motivation is simply to improve
clarity by moving the update closer to use.
For #43997
Change-Id: Ibc7fb308ac4a582875c200659c9e272121a89f3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/308654
Trust: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Signals can be delivered on a different thread. There is no necessary
happens-before relationship between setting sig.inuse in signal_enable
and checking it in sigsend. It is theoretically possible, if unlikely,
that sig.inuse is set by thread 1, thread 2 receives a signal, does not
see that sig.inuse is set, and discards the signal. This could happen
if the signal is received immediately after the first call to signal_enable.
For #33174
Change-Id: Idb0f1c77847b7d4d418bd139e801c0c4460531d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/312131
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Break the main components of the findrunnable spinning -> non-spinning
recheck out into their own functions, which simplifies both findrunnable
and the new functions, which can make use of fancy features like early
returns.
This CL should have no functional changes.
For #43997
For #44313
Change-Id: I6d3060fcecda9920a3471ff338f73d53b1d848a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/307914
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
findrunnable has grown very large and hard to follow over the years.
Parts we can split out into logical chunks should help make it more
understandable and easier to change in the future.
The work stealing loop is one such big chunk that is fairly trivial to
split out into its own function, and even has the advantage of
simplifying control flow by removing a goto around work stealing.
This CL should have no functional changes.
For #43997.
For #44313.
Change-Id: Ie69670c7bc60bd6c114e860184918717829adb22
Reviewed-on: https://go-review.googlesource.com/c/go/+/307913
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Chris Hines <chris.cs.guy@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When -clobberdead compiler flag is set, the compiler inserts
instructions that set dead slots a specific value. If the GC sees
this value as a live pointer, something is probably wrong. Crash.
Only do this on AMD64 for now, as it is the only platform where
compiler's clobberdead mode is implemented. And on AMD64 the
clobberdead address can never be a valid address.
Change-Id: Ica687b132b5d3ba2a062500d13264fa730405d11
Reviewed-on: https://go-review.googlesource.com/c/go/+/310330
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
mcall calls fn with an argument. Currently, in the regabi version
of mcall it does not reserve space for that argument's spill slot.
If the callee spills its argument, it may clobber things on the
g0 stack at 0(SP) (e.g. the old SP saved in cgocallback).
Reserve the space.
Change-Id: I85a314273cd996c7fac8fd0b03cd9033faae9c5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/311489
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
The overview comments discuss readying goroutines, which is the most
common source of work, but timers and idle-priority GC work also require
the same synchronization w.r.t. spinning Ms.
This CL should have no functional changes.
For #43997
Updates #44313
Change-Id: I7910a7f93764dde07c3ed63666277eb832bf8299
Reviewed-on: https://go-review.googlesource.com/c/go/+/307912
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
findrunnable has a couple places where delta is recomputed from a new
pollUntil value. This proves to be a pain in refactoring, as it is easy
to forget to do properly.
Move computation of delta closer to its use, where it is more logical
anyways.
This CL should have no functional changes.
For #43997.
For #44313.
Change-Id: I89980fd7f40f8a4c56c7540cae03ff99e12e1422
Reviewed-on: https://go-review.googlesource.com/c/go/+/307910
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
These functions take the address of an argument and expect to be able
to reach later arguments from that pointer. This means they must be
laid out sequentially in memory (using ABI0) and all arguments must be
live even though they don't all appear to be referenced. This is
exactly what go:cgo_unsafe_args does.
Without this, GOEXPERIMENT=regabi,regabiargs on windows/amd64 crashes
on runtime startup because the stdcall functions are called with their
arguments in registers, so taking the address of one of them has no
bearing on the memory locations of the following arguments.
With this, GOEXPERIMENT=regabi,regabiargs on windows/amd64 passes
all.bash.
For #40724.
Change-Id: I4a4d6a913f85799b43f61c234d21ebb113a9b527
Reviewed-on: https://go-review.googlesource.com/c/go/+/310733
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The previous CL introduced macros for transitions from the Windows ABI
to the Go ABI. This CL does the same for SysV and uses them in almost
all places where we transition from the C ABI to the Go ABI.
Compared to Windows, this transition is much simpler and I didn't find
any places that were getting it wrong. But this does let us unify a
lot of code nicely and introduces some degree of abstraction around
these ABI transitions.
Change-Id: Ib6bdecafce587ce18fca4c8300fcf401284a2bcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/309930
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
There are several assembly functions that transition from the Windows
ABI to the Go ABI. These all need to save all registers that are
callee-save in the Windows ABI and caller-save in the Go ABI and
prepare the register state for Go. However, they all do this slightly
differently and most of them don't save the necessary XMM registers
for this transition (which could corrupt them in the C caller).
Furthermore, now that we have a carefully specified Go ABI, it's clear
that none of these actually get all of the details 100% right.
So, unify this code into two macros in a shared header in
runtime/cgo/abi_amd64.h that handle all necessary registers and setup
and use these macros everywhere on Windows that handles transitions
from C to Go.
Change-Id: I62f41345a507aad1ca383814ac8b7e2a9ffb821e
Reviewed-on: https://go-review.googlesource.com/c/go/+/309769
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This function bounces between the C and Go ABIs a few times. This CL
narrows the scope of the Go -> C transition to just around the branch
that calls C. This lets us take advantage of C callee-save registers
to simplify the code a little.
Change-Id: I1ffa0b9e50325425c5ec66596978aeb6450a6b57
Reviewed-on: https://go-review.googlesource.com/c/go/+/309929
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The debug call tests currently assume that the target Go function is
ABI0; this is clearly no longer true when we switch to the new ABI, so
make the tests set up argument register state in the debug call handler
and copy back results returned in registers.
A small snag in calling a Go function that follows the new ABI is that
the debug call protocol depends on the AX register being set to a
specific value as it bounces in and out of the handler, but this
register is part of the new register ABI, so results end up being
clobbered. Use R12 instead.
Next, the new desugaring behavior for "go" statements means that
newosproc1 must always call a function with no frame; if it takes any
arguments, it closes over them and they're passed in the context
register. Currently when debugCallWrap creates a new goroutine, it uses
newosproc1 directly and passes a non-zero-sized frame, so that needs to
be updated. To fix this, briefly use the g's param field which is
otherwise only used for channels to pass an explicitly allocated object
containing the "closed over" variables. While we could manually do the
desugaring ourselves (we cannot do so automatically because the Go
compiler prevents heap-allocated closures in the runtime), that bakes in
more ABI details in a place that really doesn't need to care about them.
Finally, there's an old bug here where the context register was set up
in CX, so technically closure calls never worked. Oops. It was otherwise
harmless for other types of calls before, but now CX is an argument
register, so now that interferes with regular calls, too.
For #40724.
Change-Id: I652c25ed56a25741bb04c24cfb603063c099edde
Reviewed-on: https://go-review.googlesource.com/c/go/+/309169
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
