It's fairly common to call cgo functions with conversions to
unsafe.Pointer or other C types. Apply the simpler checking of address
expressions when possible when the address expression occurs within a
type conversion.
Change-Id: I5187d4eb4d27a6542621c396cad9ee4b8647d1cd
Reviewed-on: https://go-review.googlesource.com/18391
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
f90b48e intended to require the stack barrier lock in all cases of
sigprof that walked the user stack, but got it wrong. In particular,
if sp < gp.stack.lo || gp.stack.hi < sp, tracebackUser would be true,
but we wouldn't acquire the stack lock. If it then turned out that we
were in a cgo call, it would walk the stack without the lock.
In fact, the whole structure of stack locking is sigprof is somewhat
wrong because it assumes the G to lock is gp.m.curg, but all three
gentraceback calls start from potentially different Gs.
To fix this, we lower the gcTryLockStackBarriers calls much closer to
the gentraceback calls. There are now three separate trylock calls,
each clearly associated with a gentraceback and the locked G clearly
matches the G from which the gentraceback starts. This actually brings
the sigprof logic closer to what it originally was before stack
barrier locking.
This depends on "runtime: increase assumed stack size in
externalthreadhandler" because it very slightly increases the stack
used by sigprof; without this other commit, this is enough to blow the
profiler thread's assumed stack size.
Fixes#12528 (hopefully for real this time!).
For the 1.5 branch, though it will require some backporting. On the
1.5 branch, this will *not* require the "runtime: increase assumed
stack size in externalthreadhandler" commit: there's no pcvalue cache,
so the used stack is smaller.
Change-Id: Id2f6446ac276848f6fc158bee550cccd03186b83
Reviewed-on: https://go-review.googlesource.com/18328
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
On Windows, externalthreadhandler currently sets the assumed stack
size for the profiler thread and the ctrlhandler threads to 8KB. The
actual stack size is determined by the SizeOfStackReserve field in the
binary set by the linker, which is currently at least 64KB (and
typically 128KB).
It turns out the profiler thread is running within a few words of the
8KB-(stack guard) bound set by externalthreadhandler. If it overflows
this bound, morestack crashes unceremoniously with an access
violation, which we then fail to handle, causing the whole process to
exit without explanation.
To avoid this problem and give us some breathing room, increase the
assumed stack size in externalthreadhandler to 32KB (there's some
unknown amount of stack already in use, so it's not safe to increase
this all the way to the reserve size).
We also document the relationships between externalthreadhandler and
SizeOfStackReserve to make this more obvious in the future.
Change-Id: I2f9f9c0892076d78e09827022ff0f2bedd9680a9
Reviewed-on: https://go-review.googlesource.com/18304
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Minux Ma <minux@golang.org>
If a sigprof happens during a cgo call, we traceback from the entry
point of the cgo call. However, if the SP is outside of the G's stack,
we'll then ignore this traceback, even if it was successful, and
overwrite it with just _ExternalCode.
Fix this by accepting any successful traceback, regardless of whether
we got it from a cgo entry point or from regular Go code.
Fixes#13466.
Change-Id: I5da9684361fc5964f44985d74a8cdf02ffefd213
Reviewed-on: https://go-review.googlesource.com/18327
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
net has GODEBUG text already.
net/http still needs it (leaving for Brad).
For #13611.
Change-Id: Icea1027924a23a687cbbe4001985e8c6384629d7
Reviewed-on: https://go-review.googlesource.com/18346
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We were setting the signal mask of a new m to the signal mask of the m
that created it. That failed when that m happened to be the one created
by ensureSigM, which sets its signal mask to only include the signals
being caught by os/signal.Notify.
Fixes#13164.
Update #9896.
Change-Id: I705c196fe9d11754e10bab9e9b2e7530ecdfa367
Reviewed-on: https://go-review.googlesource.com/18064
Reviewed-by: Russ Cox <rsc@golang.org>
When calling a Go function on a C thread, if the C thread already has an
alternate signal stack, use that signal stack instead of installing a
new one.
Update #9896.
Change-Id: I62aa3a6a4a1dc4040fca050757299c8e6736987c
Reviewed-on: https://go-review.googlesource.com/18108
Reviewed-by: Russ Cox <rsc@golang.org>
Just saw a few dragonfly failures here.
I'm tempted to preemptively add plan9 here too, but I'll wait until
I see it fail.
Change-Id: Ic99fc088dbfd1aa21f509148aee98ccfe7f640bf
Reviewed-on: https://go-review.googlesource.com/18306
Reviewed-by: Russ Cox <rsc@golang.org>
Since Stop was introduced, it would revert to the system default for the
signal, rather than to the default Go behavior. Change it to revert to
the default Go behavior.
Change-Id: I345467ece0e49e31b2806d6fce2f1937b17905a6
Reviewed-on: https://go-review.googlesource.com/18229
Reviewed-by: Russ Cox <rsc@golang.org>
Avoids an msan error when runtime/cgo is explicitly rebuilt with
-fsanitize=memory.
Fixes#13815.
Change-Id: I70308034011fb308b63585bcd40b0d1e62ec93ef
Reviewed-on: https://go-review.googlesource.com/18263
Reviewed-by: Russ Cox <rsc@golang.org>
This test triggers a large number of usleep(100)s. linux/arm, openbsd,
and solaris have very poor timer resolution on the builders, so
usleep(100) actually gives up the whole scheduling quantum. On Linux
and OpenBSD (and probably Solaris), profiling signals are only
generated when a process completes a whole scheduling quantum, so this
test often gets zero profiling signals and fails.
Until we figure out what to do about this, skip this test on these
platforms.
Updates #13405.
Change-Id: Ica94e4a8ae7a8df3e5a840504f83ee2ec08727df
Reviewed-on: https://go-review.googlesource.com/18252
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Previously, when a program died because of a SIGHUP, SIGINT, or SIGTERM
signal it would exit with status 2. This CL fixes the runtime to exit
with a status indicating that the program was killed by a signal.
Change-Id: Ic2982a2562857edfdccaf68856e0e4df532af136
Reviewed-on: https://go-review.googlesource.com/18156
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Use the current ability to say that we don't do anything with SIGCONT by
default, but programs can catch it using signal.Notify if they want.
Fixes#8953.
Change-Id: I67d40ce36a029cbc58a235cbe957335f4a58e1c5
Reviewed-on: https://go-review.googlesource.com/18185
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Old behavior: 10 consecutive EPIPE errors on any descriptor cause the
program to exit with a SIGPIPE signal.
New behavior: an EPIPE error on file descriptors 1 or 2 cause the
program to raise a SIGPIPE signal. If os/signal.Notify was not used to
catch SIGPIPE signals, this will cause the program to exit with SIGPIPE.
An EPIPE error on a file descriptor other than 1 or 2 will simply be
returned from Write.
Fixes#11845.
Update #9896.
Change-Id: Ic85d77e386a8bb0255dc4be1e4b3f55875d10f18
Reviewed-on: https://go-review.googlesource.com/18151
Reviewed-by: Russ Cox <rsc@golang.org>
This CL changes the source file information in the
standard library's .a files to say "$GOROOT/src/runtime/chan.go"
(with a literal "$GOROOT") instead of spelling out the actual directory.
The linker then substitutes the actual $GOROOT (or $GOROOT_FINAL)
as appropriate.
If people download a binary distribution to an alternate location,
following the instructions at https://golang.org/doc/install#install,
the code before this CL would end up with source paths pointing to
/usr/local/go no matter where the actual sources were.
Now the source paths for built binaries will point to the actual sources
(hopefully).
The source line information in distributed binaries is not affected:
those will still say /usr/local/go. But binaries people build themselves
(their own programs, not the go distribution programs) will be correct.
Fixing this path also fixes the lookup of the runtime-gdb.py file.
Fixes#5533.
Change-Id: I03729baae3fbd8cd636e016275ee5ad2606e4663
Reviewed-on: https://go-review.googlesource.com/18200
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now there are just three programs to compile instead of many,
and repeated tests can reuse the compilation result instead of
rebuilding it.
Combined, these changes reduce the time spent testing runtime
during all.bash on my laptop from about 60 to about 30 seconds.
(All.bash itself runs in 5½ minutes.)
For #10571.
Change-Id: Ie2c1798b847f1a635a860d11dcdab14375319ae9
Reviewed-on: https://go-review.googlesource.com/18085
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Currently goroutineheader goes through some convolutions to *almost*
print the scan state of a G. However, the code path that would print
the scan state of the G refers to gStatusStrings where it almost
certainly meant to refer to gScanStatusStrings (which is unused), so
it winds up printing the regular status string without the scan state
either way. Furthermore, if the G is in _Gwaiting, we override the
status string and lose where this would indicate the scan state if it
worked.
This commit fixes this so the runtime prints the scan state. However,
rather than using a parallel list of status strings, this simply adds
a conditional print if the scan bit is set. This lets us remove the
string list, prints the scan state even in _Gwaiting, and lets us
strip off the scan bit at the beginning of the function, which
simplifies the rest of it.
Change-Id: Ic0adbe5c05abf4adda93da59f93b578172b28e3d
Reviewed-on: https://go-review.googlesource.com/18092
Reviewed-by: Keith Randall <khr@golang.org>
If non-Go code calls sigaltstack before a signal is received, use
sigaltstack to determine the current signal stack and set the gsignal
stack to use it. This makes the Go runtime more robust in the face of
non-Go code. We still can't handle a disabled signal stack or a signal
triggered with SA_ONSTACK clear, but we now give clear errors for those
cases.
Fixes#7227.
Update #9896.
Change-Id: Icb1607e01fd6461019b6d77d940e59b3aed4d258
Reviewed-on: https://go-review.googlesource.com/18102
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Only install signal handlers for synchronous signals that become
run-time panics. Set the SA_ONSTACK flag for other signal handlers as
needed.
Fixes#13028.
Update #12465.
Update #13034.
Update #13042.
Change-Id: I28375e70641f60630e10f3c86e24b6e4f8a35cc9
Reviewed-on: https://go-review.googlesource.com/17903
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It turns out that the second argument for sigaction on Darwin has a
different type than the first argument. The second argument is the user
visible sigaction struct, and does not have the sa_tramp field.
I base this on
http://www.opensource.apple.com/source/Libc/Libc-1081.1.3/sys/sigaction.c
not to mention actual testing.
While I was at it I removed a useless memclr in setsig, a relic of the C
code.
This CL is Darwin-specific changes. The tests for this CL are in
https://golang.org/cl/17903 .
Change-Id: I61fe305c72311df6a589b49ad7b6e49b6960ca24
Reviewed-on: https://go-review.googlesource.com/18015
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Programs that call panic to crash after detecting a serious problem
may wish to use SetTraceback to force printing of all goroutines first.
Change-Id: Ib23ad9336f405485aabb642ca73f454a14c8baf3
Reviewed-on: https://go-review.googlesource.com/18043
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, if sigprof determines that the G is in user code (not cgo
or libcall code), it will only traceback the G stack if it can acquire
the stack barrier lock. However, it has no such restriction if the G
is in cgo or libcall code. Because cgo calls count as syscalls, stack
scanning and stack barrier installation can occur during a cgo call,
which means sigprof could attempt to traceback a G in a cgo call while
scanstack is installing stack barriers in that G's stack. As a result,
the following sequence of events can cause the sigprof traceback to
panic with "missed stack barrier":
1. M1: G1 performs a Cgo call (which, on Windows, is any system call,
which could explain why this is easier to reproduce on Windows).
2. M1: The Cgo call puts G1 into _Gsyscall state.
3. M2: GC starts a scan of G1's stack. It puts G1 in to _Gscansyscall
and acquires the stack barrier lock.
4. M3: A profiling signal comes in. On Windows this is a global
(though I don't think this matters), so the runtime stops M1 and
calls sigprof for G1.
5. M3: sigprof fails to acquire the stack barrier lock (because the
GC's stack scan holds it).
6. M3: sigprof observes that G1 is in a Cgo call, so it calls
gentraceback on G1 with its Cgo transition point.
7. M3: gentraceback on G1 grabs the currently empty g.stkbar slice.
8. M2: GC finishes scanning G1's stack and installing stack barriers.
9. M3: gentraceback encounters one of the just-installed stack
barriers and panics.
This commit fixes this by only allowing cgo tracebacks if sigprof can
acquire the stack barrier lock, just like in the regular user
traceback case.
For good measure, we put the same constraint on libcall tracebacks.
This case is probably already safe because, unlike cgo calls, libcalls
leave the G in _Grunning and prevent reaching a safe point, so
scanstack cannot run during a libcall. However, this also means that
sigprof will always acquire the stack barrier lock without contention,
so there's no cost to adding this constraint to libcall tracebacks.
Fixes#12528. For 1.5.3 (will require some backporting).
Change-Id: Ia5a4b8e3d66b23b02ffcd54c6315c81055c0cec2
Reviewed-on: https://go-review.googlesource.com/18023
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently, setNextBarrierPC manipulates the stack barriers without
acquiring the stack barrier lock. This is mostly okay because
setNextBarrierPC also runs synchronously on the G and prevents safe
points, but this doesn't prevent a sigprof from occurring during a
setNextBarrierPC and performing a traceback.
Given that setNextBarrierPC simply sets one entry in the stack barrier
array, this is almost certainly safe in reality. However, given that
this depends on a subtle argument, which may not hold in the future,
and that setNextBarrierPC almost never happens, making it nowhere near
performance-critical, we can simply acquire the stack barrier lock and
be sure that the synchronization will work.
Updates #12528. For 1.5.3.
Change-Id: Ife696e10d969f190157eb1cbe762a2de2ebce079
Reviewed-on: https://go-review.googlesource.com/18022
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
I've already turned away one attempt to remove this field.
As the comment above the struct says, many tools know the layout.
The field cannot simply be removed.
It was one thing to remove the fields name, but the TODO should
not have been added.
Change-Id: If40eacf0eb35835082055e129e2b88333a0731b9
Reviewed-on: https://go-review.googlesource.com/17741
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestMemStats currently requires that NumGC != 0, but GC may
legitimately not have run (for example, if this test runs first, or
GOGC is set high, etc). Accept NumGC == 0 and instead sanity check
NumGC by making sure that all pause times after NumGC are 0.
Fixes#11989.
Change-Id: I4203859fbb83292d59a509f2eeb24d6033e7aabc
Reviewed-on: https://go-review.googlesource.com/17830
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
This matches SIGEMT on other systems that use it (SIGEMT is not used
for most linux systems).
Change-Id: If394c06c9ed1cb3ea2564385a8edfbed8b5566d1
Reviewed-on: https://go-review.googlesource.com/17874
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Currently, sysmon triggers a forced GC solely based on
memstats.last_gc. However, memstats.last_gc isn't updated until mark
termination, so once sysmon starts triggering forced GC, it will keep
triggering them until GC finishes. The first of these actually starts
a GC; the remainder up to the last print "GC forced", but gcStart
returns immediately because gcphase != _GCoff; then the last may start
another GC if the previous GC finishes (and sets last_gc) between
sysmon triggering it and gcStart checking the GC phase.
Fix this by expanding the condition for starting a forced GC to also
require that no GC is currently running. This, combined with the way
forcegchelper blocks until the GC cycle is started, ensures sysmon
only starts one GC when the time exceeds the forced GC threshold.
Fixes#13458.
Change-Id: Ie6cf841927f6085136be3f45259956cd5cf10d23
Reviewed-on: https://go-review.googlesource.com/17819
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The addition of stack barrier locking to copystack subsumes the
partial fix from commit bbd1a1c for SIGPROF during copystack. With the
stack barrier locking, this commit simplifies the rule in sigprof to:
the user stack can be traced only if sigprof can acquire the stack
barrier lock.
Updates #12932, #13362.
Change-Id: I1c1f80015053d0ac7761e9e0c7437c2aba26663f
Reviewed-on: https://go-review.googlesource.com/17192
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently, runtime/debug.SetGCPercent does not adjust the controller
trigger ratio. As a result, runtime reductions of GOGC don't take full
effect until after one more concurrent cycle has happened, which
adjusts the trigger ratio to account for the new gcpercent.
Fix this by lowering the trigger ratio if necessary in setGCPercent.
Change-Id: I4d23e0c58d91939b86ac60fa5d53ef91d0d89e0c
Reviewed-on: https://go-review.googlesource.com/17813
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently we drop worldsema and then print the gctrace. We did this so
that if stderr is a pipe or a blocked terminal, blocking on printing
the gctrace would not block another GC from starting. However, this is
a bit of a fool's errand because a blocked runtime print will block
the whole M/P, so after GOMAXPROCS GC cycles, the whole system will
freeze. Furthermore, now this is much less of an issue because
allocation will block indefinitely if it can't start a GC (whereas it
used to be that allocation could run away). Finally, this allows
another GC cycle to start while the previous cycle is printing the
gctrace, which leads to races on reading various statistics to print
them and the next GC cycle overwriting those statistics.
Fix this by moving the release of worldsema after the gctrace print.
Change-Id: I3d044ea0f77d80f3b4050af6b771e7912258662a
Reviewed-on: https://go-review.googlesource.com/17812
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently we reset the sweep stats just after gcMarkTermination starts
the world and releases worldsema. However, background sweeping can
start the moment we start the world and, in fact, pause sweeping can
start the moment we release worldsema (because another GC cycle can
start up), so these need to be cleared before starting the world.
Change-Id: I95701e3de6af76bb3fbf2ee65719985bf57d20b2
Reviewed-on: https://go-review.googlesource.com/17811
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently, we update memstats.heap_live from mcache.local_cachealloc
whenever we lock the heap (e.g., to obtain a fresh span or to release
an unused span). However, under the right circumstances,
local_cachealloc can accumulate allocations up to the size of
the *entire heap* without flushing them to heap_live. Specifically,
since span allocations from an mcentral don't lock the heap, if a
large number of pages are held in an mcentral and the application
continues to use and free objects of that size class (e.g., the
BinaryTree17 benchmark), local_cachealloc won't be flushed until the
mcentral runs out of spans.
This is a problem because, unlike many of the memory statistics that
are purely informative, heap_live is used to determine when the
garbage collector should start and how hard it should work.
This commit eliminates local_cachealloc, instead atomically updating
heap_live directly. To control contention, we do this only when
obtaining a span from an mcentral. Furthermore, we make heap_live
conservative: allocating a span assumes that all free slots in that
span will be used and accounts for these when the span is
allocated, *before* the objects themselves are. This is important
because 1) this triggers the GC earlier than necessary rather than
potentially too late and 2) this leads to a conservative GC rate
rather than a GC rate that is potentially too low.
Alternatively, we could have flushed local_cachealloc when it passed
some threshold, but this would require determining a threshold and
would cause heap_live to underestimate the true value rather than
overestimate.
Fixes#12199.
name old time/op new time/op delta
BinaryTree17-12 2.88s ± 4% 2.88s ± 1% ~ (p=0.470 n=19+19)
Fannkuch11-12 2.48s ± 1% 2.48s ± 1% ~ (p=0.243 n=16+19)
FmtFprintfEmpty-12 50.9ns ± 2% 50.7ns ± 1% ~ (p=0.238 n=15+14)
FmtFprintfString-12 175ns ± 1% 171ns ± 1% -2.48% (p=0.000 n=18+18)
FmtFprintfInt-12 159ns ± 1% 158ns ± 1% -0.78% (p=0.000 n=19+18)
FmtFprintfIntInt-12 270ns ± 1% 265ns ± 2% -1.67% (p=0.000 n=18+18)
FmtFprintfPrefixedInt-12 235ns ± 1% 234ns ± 0% ~ (p=0.362 n=18+19)
FmtFprintfFloat-12 309ns ± 1% 308ns ± 1% -0.41% (p=0.001 n=18+19)
FmtManyArgs-12 1.10µs ± 1% 1.08µs ± 0% -1.96% (p=0.000 n=19+18)
GobDecode-12 7.81ms ± 1% 7.80ms ± 1% ~ (p=0.425 n=18+19)
GobEncode-12 6.53ms ± 1% 6.53ms ± 1% ~ (p=0.817 n=19+19)
Gzip-12 312ms ± 1% 312ms ± 2% ~ (p=0.967 n=19+20)
Gunzip-12 42.0ms ± 1% 41.9ms ± 1% ~ (p=0.172 n=19+19)
HTTPClientServer-12 63.7µs ± 1% 63.8µs ± 1% ~ (p=0.639 n=19+19)
JSONEncode-12 16.4ms ± 1% 16.4ms ± 1% ~ (p=0.954 n=19+19)
JSONDecode-12 58.5ms ± 1% 57.8ms ± 1% -1.27% (p=0.000 n=18+19)
Mandelbrot200-12 3.86ms ± 1% 3.88ms ± 0% +0.44% (p=0.000 n=18+18)
GoParse-12 3.67ms ± 2% 3.66ms ± 1% -0.52% (p=0.001 n=18+19)
RegexpMatchEasy0_32-12 100ns ± 1% 100ns ± 0% ~ (p=0.257 n=19+18)
RegexpMatchEasy0_1K-12 347ns ± 1% 347ns ± 1% ~ (p=0.527 n=18+18)
RegexpMatchEasy1_32-12 83.7ns ± 2% 83.1ns ± 2% ~ (p=0.096 n=18+19)
RegexpMatchEasy1_1K-12 509ns ± 1% 505ns ± 1% -0.75% (p=0.000 n=18+19)
RegexpMatchMedium_32-12 130ns ± 2% 129ns ± 1% ~ (p=0.962 n=20+20)
RegexpMatchMedium_1K-12 39.5µs ± 2% 39.4µs ± 1% ~ (p=0.376 n=20+19)
RegexpMatchHard_32-12 2.04µs ± 0% 2.04µs ± 1% ~ (p=0.195 n=18+17)
RegexpMatchHard_1K-12 61.4µs ± 1% 61.4µs ± 1% ~ (p=0.885 n=19+19)
Revcomp-12 540ms ± 2% 542ms ± 4% ~ (p=0.552 n=19+17)
Template-12 69.6ms ± 1% 71.2ms ± 1% +2.39% (p=0.000 n=20+20)
TimeParse-12 357ns ± 1% 357ns ± 1% ~ (p=0.883 n=18+20)
TimeFormat-12 379ns ± 1% 362ns ± 1% -4.53% (p=0.000 n=18+19)
[Geo mean] 62.0µs 61.8µs -0.44%
name old time/op new time/op delta
XBenchGarbage-12 5.89ms ± 2% 5.81ms ± 2% -1.41% (p=0.000 n=19+18)
Change-Id: I96b31cca6ae77c30693a891cff3fe663fa2447a0
Reviewed-on: https://go-review.googlesource.com/17748
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
deductSweepCredit expects the size in bytes of the span being
allocated, but mCentral_CacheSpan passes the size of a single object
in the span. As a result, we don't sweep enough on that call and when
mCentral_CacheSpan later calls reimburseSweepCredit, it's very likely
to underflow mheap_.spanBytesAlloc, which causes the next call to
deductSweepCredit to think it owes a huge number of pages and finish
off the whole sweep.
In addition to causing the occasional allocation that triggers the
full sweep to be potentially extremely expensive relative to other
allocations, this can indirectly slow down many other allocations.
deductSweepCredit uses sweepone to sweep spans, which returns
fully-unused spans to the heap, where these spans are freed and
coalesced with neighboring free spans. On the other hand, when
mCentral_CacheSpan sweeps a span, it does so with the intent to
immediately reuse that span and, as a result, will not return the span
to the heap even if it is fully unused. This saves on the cost of
locking the heap, finding a span, and initializing that span. For
example, before this change, with GOMAXPROCS=1 (or the background
sweeper disabled) BinaryTree17 returned roughly 220K spans to the heap
and allocated new spans from the heap roughly 232K times. After this
change, it returns 1.3K spans to the heap and allocates new spans from
the heap 39K times. (With background sweeping these numbers are
effectively unchanged because the background sweeper sweeps almost all
of the spans with sweepone; however, parallel sweeping saves more than
the cost of allocating spans from the heap.)
Fixes#13535.
Fixes#13589.
name old time/op new time/op delta
BinaryTree17-12 3.03s ± 1% 2.86s ± 4% -5.61% (p=0.000 n=18+20)
Fannkuch11-12 2.48s ± 1% 2.49s ± 1% ~ (p=0.060 n=17+20)
FmtFprintfEmpty-12 50.7ns ± 1% 50.9ns ± 1% +0.43% (p=0.025 n=15+16)
FmtFprintfString-12 174ns ± 2% 174ns ± 2% ~ (p=0.539 n=19+20)
FmtFprintfInt-12 158ns ± 1% 158ns ± 1% ~ (p=0.300 n=18+20)
FmtFprintfIntInt-12 269ns ± 2% 269ns ± 2% ~ (p=0.784 n=20+18)
FmtFprintfPrefixedInt-12 233ns ± 1% 234ns ± 1% ~ (p=0.389 n=18+18)
FmtFprintfFloat-12 309ns ± 1% 310ns ± 1% +0.25% (p=0.048 n=18+18)
FmtManyArgs-12 1.10µs ± 1% 1.10µs ± 1% ~ (p=0.259 n=18+19)
GobDecode-12 7.81ms ± 1% 7.72ms ± 1% -1.17% (p=0.000 n=19+19)
GobEncode-12 6.56ms ± 0% 6.55ms ± 1% ~ (p=0.433 n=17+19)
Gzip-12 318ms ± 2% 317ms ± 1% ~ (p=0.578 n=19+18)
Gunzip-12 42.1ms ± 2% 42.0ms ± 0% -0.45% (p=0.007 n=18+16)
HTTPClientServer-12 63.9µs ± 1% 64.0µs ± 1% ~ (p=0.146 n=17+19)
JSONEncode-12 16.4ms ± 1% 16.4ms ± 1% ~ (p=0.271 n=19+19)
JSONDecode-12 58.1ms ± 1% 58.0ms ± 1% ~ (p=0.152 n=18+18)
Mandelbrot200-12 3.85ms ± 0% 3.85ms ± 0% ~ (p=0.126 n=19+18)
GoParse-12 3.71ms ± 1% 3.64ms ± 1% -1.86% (p=0.000 n=20+18)
RegexpMatchEasy0_32-12 100ns ± 2% 100ns ± 1% ~ (p=0.588 n=20+20)
RegexpMatchEasy0_1K-12 346ns ± 1% 347ns ± 1% +0.27% (p=0.014 n=17+20)
RegexpMatchEasy1_32-12 82.9ns ± 3% 83.5ns ± 3% ~ (p=0.096 n=19+20)
RegexpMatchEasy1_1K-12 506ns ± 1% 506ns ± 1% ~ (p=0.530 n=19+19)
RegexpMatchMedium_32-12 129ns ± 2% 129ns ± 1% ~ (p=0.566 n=20+19)
RegexpMatchMedium_1K-12 39.4µs ± 1% 39.4µs ± 1% ~ (p=0.713 n=19+20)
RegexpMatchHard_32-12 2.05µs ± 1% 2.06µs ± 1% +0.36% (p=0.008 n=18+20)
RegexpMatchHard_1K-12 61.6µs ± 1% 61.7µs ± 1% ~ (p=0.286 n=19+20)
Revcomp-12 538ms ± 1% 541ms ± 2% ~ (p=0.081 n=18+19)
Template-12 71.5ms ± 2% 71.6ms ± 1% ~ (p=0.513 n=20+19)
TimeParse-12 357ns ± 1% 357ns ± 1% ~ (p=0.935 n=19+18)
TimeFormat-12 352ns ± 1% 352ns ± 1% ~ (p=0.293 n=19+20)
[Geo mean] 62.0µs 61.9µs -0.21%
name old time/op new time/op delta
XBenchGarbage-12 5.83ms ± 2% 5.86ms ± 3% ~ (p=0.247 n=19+20)
Change-Id: I790bb530adace27ccf25d372f24a11954b88443c
Reviewed-on: https://go-review.googlesource.com/17745
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Analogous to https://go-review.googlesource.com/#/c/8457/ this
code synthesizes an set of program arguments for Android on the
arm64 architecture.
Change-Id: I851958b4b0944ec79d7a1426a3bb2cfc31746797
Reviewed-on: https://go-review.googlesource.com/17782
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
To prevent races with the garbage collector, stack spans cannot be
reused as heap spans during a GC. We deal with this by caching stack
spans during GC and releasing them at the end of mark termination.
However, while our cache lets us reuse small stack spans, currently
large stack spans are *not* reused. This can cause significant memory
growth in programs that allocate large stacks rapidly, but grow the
heap slowly (such as in issue #13552).
Fix this by adding logic to reuse large stack spans for other stacks.
Fixes#11466.
Fixes#13552. Without this change, the program in this issue creeps to
over 1GB of memory over the course of a few hours. With this change,
it stays rock solid at around 30MB.
Change-Id: If8b2d85464aa80c96230a1990715e39aa803904f
Reviewed-on: https://go-review.googlesource.com/17814
Reviewed-by: Keith Randall <khr@golang.org>
Currently we wake up new worker threads whenever we pass
through the scheduler with nmspinning==0. This leads to
lots of unnecessary thread wake ups.
Instead let only spinning threads wake up new spinning threads.
For the following program:
package main
import "runtime"
func main() {
for i := 0; i < 1e7; i++ {
runtime.Gosched()
}
}
Before:
$ time ./test
real 0m4.278s
user 0m7.634s
sys 0m1.423s
$ strace -c ./test
% time seconds usecs/call calls errors syscall
99.93 9.314936 3 2685009 17536 futex
After:
$ time ./test
real 0m1.200s
user 0m1.181s
sys 0m0.024s
$ strace -c ./test
% time seconds usecs/call calls errors syscall
3.11 0.000049 25 2 futex
Fixes#13527
Change-Id: Ia1f5bf8a896dcc25d8b04beb1f4317aa9ff16f74
Reviewed-on: https://go-review.googlesource.com/17540
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
debug.schedtrace is an int32. Convert it to int64 before
multiplying with constant 1000000. Otherwise, schedtrace
values more than 2147 result in int32 overflow causing
incorrect delays between traces.
Change-Id: I064e8d7b432c1e892a705ee1f31a2e8cdd2c3ea3
Reviewed-on: https://go-review.googlesource.com/17712
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
If reports like #13062 are really concurrent misuse of maps,
we can detect that, at least some of the time, with a cheap check.
There is an extra pair of memory writes for writing to a map,
but to the same cache line as h.count, which is often being modified anyway,
and there is an extra memory read for reading from a map,
but to the same cache line as h.count, which is always being read anyway.
So the check should be basically invisible and may help reduce the
number of "mysterious runtime crash due to map misuse" reports.
Change-Id: I0e71b0d92eaa3b7bef48bf41b0f5ab790092487e
Reviewed-on: https://go-review.googlesource.com/17501
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
(sig is unsigned, so sig-1 >= 0 is always true.)
Fixes#11281.
Change-Id: I4b9d784da6e3cc80816f2d2f7228d5d8a237e2d5
Reviewed-on: https://go-review.googlesource.com/17457
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
When run with "ulimit -s unlimited", the misc/cgo/test test binary
finds a stack size of 0x3000 returned by getcontext, causing the
runtime to try to stay within those bounds and then fault when
called back in the test after 64 kB has been used by C.
I suspect that Solaris is doing something clever like reporting the
current stack size and growing the stack as faults happen.
On all the other systems, getcontext reports the maximum stack size.
And when the ulimit is not unlimited, even Solaris reports the
maximum stack size.
Work around this by assuming that any stack on Solaris must be at least 1 MB.
Fixes#12210.
Change-Id: I0a6ed0afb8a8f50aa1b2486f32b4ae470ab47dbf
Reviewed-on: https://go-review.googlesource.com/17452
Reviewed-by: Ian Lance Taylor <iant@golang.org>
stackBarrier on amd64 sanity checks that it's unwinding the correct
entry in the stack barrier array. However, this check is wrong in two
ways that make it unlikely to catch anything, right or wrong:
1) It checks that savedLRPtr == SP, but, in fact, it should be that
savedLRPtr+8 == SP because the RET that returned to stackBarrier
popped the saved LR. However, we didn't notice this check was wrong
because,
2) the sense of the conditional branch is also wrong.
Fix both of these.
Change-Id: I38ba1f652b0168b5b2c11b81637656241262af7c
Reviewed-on: https://go-review.googlesource.com/17039
Reviewed-by: Russ Cox <rsc@golang.org>
On android, runtime.tls_g is a normal variable.
TLS offset is computed in x_cgo_inittls.
Change-Id: I18bc9a736d5fb2a89d0f798956c754e3c10d10e2
Reviewed-on: https://go-review.googlesource.com/17246
Reviewed-by: David Crawshaw <crawshaw@golang.org>
On android, runtime.tls_g is a normal variable.
TLS offset is computed in x_cgo_inittls.
Change-Id: I64cfd3543040776dcdf73cad8dba54fc6aaf6f35
Reviewed-on: https://go-review.googlesource.com/17245
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The ppc64le shared library ABI demands that r12 is set to a function's global
entrypoint before jumping to the global entrypoint. Not doing so means that
handling signals that usually panic actually crashes (and so, e.g. can't be
recovered). Fixes several failures of "cd test; go run run.go -linkshared".
Change-Id: Ia4d0da4c13efda68340d38c045a52b37c2f90796
Reviewed-on: https://go-review.googlesource.com/17280
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We need a runtime check because the original issue is encountered
when running cross compiled windows program from linux. It's better
to give a meaningful crash message earlier than to segfault later.
The added test should not impose any measurable overhead to Go
programs.
For #12415.
Change-Id: Ib4a24ef560c09c0585b351d62eefd157b6b7f04c
Reviewed-on: https://go-review.googlesource.com/14207
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Write barriers in gcFlushBgCredit lead to very subtle bugs because it
executes after the getfull barrier. I tracked some bugs of this form
down before go:nowritebarrierrec was implemented. Ensure that they
don't reappear by making gcFlushBgCredit go:nowritebarrierrec.
Change-Id: Ia5ca2dc59e6268bce8d8b4c87055bd0f6e19bed2
Reviewed-on: https://go-review.googlesource.com/17052
Reviewed-by: Russ Cox <rsc@golang.org>
sighandler may run during STW, so write barriers are not allowed.
Change-Id: Icdf46be10ea296fd87e73ab56ebb718c5d3c97ac
Reviewed-on: https://go-review.googlesource.com/17007
Reviewed-by: Russ Cox <rsc@golang.org>
Replace the cross platform but unsafe [4]uintptr type with a OS
specific type, sigset. Most OSes already define sigset, and this
change defines a suitable sigset for the OSes that don't (darwin,
openbsd). The OSes that don't use m.sigmask (windows, plan9, nacl)
now defines sigset as the empty type, struct{}.
The gain is strongly typed access to m.sigmask, saving a dynamic
size sanity check and unsafe.Pointer casting. Also, some storage is
saved for each M, since [4]uinptr was conservative for most OSes.
The cost is that OSes that don't need m.sigmask has to define sigset.
completes ./all.bash with GOOS linux, on amd64
completes ./make.bash with GOOSes openbsd, android, plan9, windows,
darwin, solaris, netbsd, freebsd, dragonfly, all amd64.
With GOOS=nacl ./make.bash failed with a seemingly unrelated error.
[Replay of CL 16942 by Elias Naur.]
Change-Id: I98f144d626033ae5318576115ed635415ac71b2c
Reviewed-on: https://go-review.googlesource.com/17033
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
These tests were failing on one of the versions of cl/9345
("runtime: simplify buffered channels").
Change-Id: I920ffcd28de428bcb7c2d5a300068644260e1017
Reviewed-on: https://go-review.googlesource.com/16416
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#11959Fixes#12035
Skip the CallbackGC test on linux/arm. This test takes between 30 and 60
seconds to run by itself, and is run 4 times over the course of ./run.bash
(once during the runtime test, three times more later in the build).
Change-Id: I4e7d3046031cd8c08f39634bdd91da6e00054caf
Reviewed-on: https://go-review.googlesource.com/14485
Reviewed-by: Russ Cox <rsc@golang.org>
Original code is mistakenly panics on VirtualAlloc failure - we want
it to go looking for smaller memory region that VirtualAlloc will
succeed to allocate. Also return immediately if VirtualAlloc succeeds.
See rsc comment on issue #12587 for details.
I still don't have a test for this. So I can only hope that this
Fixes#12587
Change-Id: I052068ec627fdcb466c94ae997ad112016f734b7
Reviewed-on: https://go-review.googlesource.com/17169
Reviewed-by: Russ Cox <rsc@golang.org>
These are methods that are "obviously" going to get inlined -- until you build
with -l, when they can trigger a stack split at a bad time.
Fixes#11482
Change-Id: Ia065c385978a2e7fe9f587811991d088c4d68325
Reviewed-on: https://go-review.googlesource.com/17165
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Commit bbd1a1c prevented SIGPROF from scanning stacks that were being
copied, but it didn't prevent a stack copy (specifically a stack
shrink) from happening while SIGPROF is scanning the stack. As a
result, a stack copy may adjust stack barriers while SIGPROF is in the
middle of scanning a stack, causing SIGPROF to panic when it detects
an inconsistent stack barrier.
Fix this by taking the stack barrier lock while adjusting the stack.
In addition to preventing SIGPROF from scanning this stack, this will
block until any in-progress SIGPROF is done scanning the stack.
For 1.5.2.
Fixes#13362.
Updates #12932.
Change-Id: I422219c363054410dfa56381f7b917e04690e5dd
Reviewed-on: https://go-review.googlesource.com/17191
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Calling timeBeginPeriod changes Windows global timer resolution
from 15ms to 1ms. This used to improve Go runtime scheduler
performance, but not anymore. Thanks to @aclements, scheduler now
behaves the same way if we call timeBeginPeriod or not.
Remove call to timeBeginPeriod, since it is machine global
resource, and there are downsides of using low timer resolution.
See issue #8687 for details.
Fixes#8687
Change-Id: Ib7e41aa4a81861b62a900e0e62776c9ef19bfb73
Reviewed-on: https://go-review.googlesource.com/17164
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Yasuhiro MATSUMOTO <mattn.jp@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
This improves the documentation comment on gcMarkDone, replaces a
recursive call with a simple goto, and disables preemption before
stopping the world in accordance with the documentation comment on
stopTheWorldWithSema.
Updates #13363, but, sadly, doesn't fix it.
Change-Id: I6cb2a5836b35685bf82f7b1ce7e48a7625906656
Reviewed-on: https://go-review.googlesource.com/17149
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This improves stack barrier debugging messages in various ways:
1) Rather than printing only the remaining stack barriers (of which
there may be none, which isn't very useful), print all of the G's
stack barriers with a marker at the position the stack itself has
unwound to and a marker at the problematic stack barrier (where
applicable).
2) Rather than crashing if we encounter a stack barrier when there are
no more stkbar entries, print the same debug message we would if we
had encountered a stack barrier at an unexpected location.
Hopefully this will help with debugging #12528.
Change-Id: I2e6fe6a778e0d36dd8ef30afd4c33d5d94731262
Reviewed-on: https://go-review.googlesource.com/17147
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The stack barrier locking functions use a simple cas lock because they
need to support trylock, but currently don't increment g.m.locks. This
is okay right now because they always run on the system stack or the
signal stack and are hence non-preemtible, but this could lead to
difficult-to-reproduce deadlocks if these conditions change in the
future.
Make these functions more robust by incrementing g.m.locks and making
them nosplit to enforce non-preemtibility.
Change-Id: I73d60a35bd2ad2d81c73aeb20dbd37665730eb1b
Reviewed-on: https://go-review.googlesource.com/17058
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
This test depends on GODEBUG=gcstackbarrierall, which doesn't work on
ppc64.
Updates #13334.
Change-Id: Ie554117b783c4e999387f97dd660484488499d85
Reviewed-on: https://go-review.googlesource.com/17120
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
During a crash showing goroutine stacks of all threads
(with GOTRACEBACK=crash), it can be that f == nil.
Only happens on Solaris; not sure why.
Change-Id: Iee2c394a0cf19fa0a24f6befbc70776b9e42d25a
Reviewed-on: https://go-review.googlesource.com/17110
Reviewed-by: Austin Clements <austin@google.com>
The nosplit stack is now much bigger, so we can afford to allocate
libcall on stack.
Fix asmsysvicall6 to not update errno if g == nil.
These two fixes TestCgoCallbackGC on solaris, which used to stuck
in a loop.
Change-Id: Id1b13be992dae9f059aa3d47ffffd37785300933
Reviewed-on: https://go-review.googlesource.com/17076
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Solaris needs to make system calls without a g,
and Solaris uses asmcgocall to make system calls.
I know, I know.
I hope this makes CL 16915, fixing #12277, work on Solaris.
Change-Id: If988dfd37f418b302da9c7096f598e5113ecea87
Reviewed-on: https://go-review.googlesource.com/17072
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Aram Hăvărneanu <aram@mgk.ro>
Run-TryBot: Russ Cox <rsc@golang.org>
sysmon runs without a P. This means it can't interact with the garbage
collector, so write barriers not allowed in anything that sysmon does.
Fixes#10600.
Change-Id: I9de1283900dadee4f72e2ebfc8787123e382ae88
Reviewed-on: https://go-review.googlesource.com/17006
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
allocm is a very unusual function: it is specifically designed to
allocate in contexts where m.p is nil by temporarily taking over a P.
Since allocm is used in many contexts where it would make sense to use
nowritebarrierrec, this commit teaches the nowritebarrierrec analysis
to stop at allocm.
Updates #10600.
Change-Id: I8499629461d4fe25712d861720dfe438df7ada9b
Reviewed-on: https://go-review.googlesource.com/17005
Reviewed-by: Russ Cox <rsc@golang.org>
gentraceback is used in many contexts where write barriers are
disallowed. This currently works because the only write barrier is in
assigning frame.argmap in setArgInfo and in practice frame is always
on the stack, so this write barrier is a no-op.
However, we can easily eliminate this write barrier, which will let us
statically disallow write barriers (using go:nowritebarrierrec
annotations) in many more situations. As a bonus, this makes the code
a little more idiomatic.
Updates #10600.
Change-Id: I45ba5cece83697ff79f8537ee6e43eadf1c18c6d
Reviewed-on: https://go-review.googlesource.com/17003
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
The assumption is that there are no nested function calls in complex expressions.
For the most part that assumption is true. It wasn't for these calls inserted during walk.
Fix that.
I looked through all the calls to mkcall in walk and these were the only cases
that emitted calls, that could be part of larger expressions (like not delete),
and that were not already handled.
Fixes#12225.
Change-Id: Iad380683fe2e054d480e7ae4e8faf1078cdd744c
Reviewed-on: https://go-review.googlesource.com/17034
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This adds a test that runs CPU profiling with a high load of stack
barriers and stack barrier insertion/removal operations and checks
that both 1) the runtime doesn't crash and 2) stackBarrier itself
never appears in a profile. Prior to the fix for gentraceback starting
in the middle of stackBarrier, condition 2 often failed.
Change-Id: Ic28860448859029779844c4bf3bb28ca84611e2c
Reviewed-on: https://go-review.googlesource.com/17037
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A sigprof during stack barrier insertion or removal can crash if it
detects an inconsistency between the stkbar array and the stack
itself. Currently we protect against this when scanning another G's
stack using stackLock, but we don't protect against it when unwinding
stack barriers for a recover or a memmove to the stack.
This commit cleans up and improves the stack locking code. It
abstracts out the lock and unlock operations. It uses the lock
consistently everywhere we perform stack operations, and pushes the
lock/unlock down closer to where the stack barrier operations happen
to make it more obvious what it's protecting. Finally, it modifies
sigprof so that instead of spinning until it acquires the lock, it
simply doesn't perform a traceback if it can't acquire it. This is
necessary to prevent self-deadlock.
Updates #11863, which introduced stackLock to fix some of these
issues, but didn't go far enough.
Updates #12528.
Change-Id: I9d1fa88ae3744d31ba91500c96c6988ce1a3a349
Reviewed-on: https://go-review.googlesource.com/17036
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, if a profiling signal happens in the middle of
stackBarrier, gentraceback may see inconsistencies between stkbar and
the barriers on the stack and it will certainly get the wrong return
PC for stackBarrier. In most cases, the return PC won't be a PC at all
and this will immediately abort the traceback (which is considered
okay for a sigprof), but if it happens to be a valid PC this may sent
gentraceback down a rabbit hole.
Fix this by detecting when the gentraceback starts in stackBarrier and
simulating the completion of the barrier to get the correct initial
frame.
Change-Id: Ib11f705ac9194925f63fe5dfbfc84013a38333e6
Reviewed-on: https://go-review.googlesource.com/17035
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Cgo-created threads transition between having associated Go g's and m's and not.
A signal arriving during the transition could think it was safe and appropriate to
run Go signal handlers when it was in fact not.
Avoid the race by masking all signals during the transition.
Fixes#12277.
Change-Id: Ie9711bc1d098391d58362492197a7e0f5b497d14
Reviewed-on: https://go-review.googlesource.com/16915
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mostly by avoiding CX entirely, sometimes by reloading it.
I also vetted the assembly in other packages, it's all fine.
Change-Id: I50059669aaaa04efa303cf22ac228f9d14d83db0
Reviewed-on: https://go-review.googlesource.com/16386
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Replace the cross platform but unsafe [4]uintptr type with a OS
specific type, sigset. Most OSes already define sigset, and this
change defines a suitable sigset for the OSes that don't (darwin,
openbsd). The OSes that don't use m.sigmask (windows, plan9, nacl)
now defines sigset as the empty type, struct{}.
The gain is strongly typed access to m.sigmask, saving a dynamic
size sanity check and unsafe.Pointer casting. Also, some storage is
saved for each M, since [4]uinptr was conservative for most OSes.
The cost is that OSes that don't need m.sigmask has to define sigset.
completes ./all.bash with GOOS linux, on amd64
completes ./make.bash with GOOSes openbsd, android, plan9, windows,
darwin, solaris, netbsd, freebsd, dragonfly, all amd64.
With GOOS=nacl ./make.bash failed with a seemingly unrelated error.
R=go1.7
Change-Id: Ib460379f063eb83d393e1c5efe7333a643c1595e
Reviewed-on: https://go-review.googlesource.com/16942
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
golang.org/cl/16796 broke android/386 by assuming behaviour specific to glibc's
dynamic linker. Copy bionic by using int $0x80 to invoke syscalls on
android/386 as the old alternative (CALL *runtime_vdso(SB)) cannot be compiled
without text relocations, which we want to get rid of on android.
Also remove "CALL *runtime_vdso(SB)" variant from the syscall package.
Change-Id: I6c01849f8dcbd073d000ddc8f13948a836b8b261
Reviewed-on: https://go-review.googlesource.com/16996
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Not all tests passing yet, but a good chunk are.
Change-Id: I5daebaeabf3aecb380674ece8830a86751a8d139
Reviewed-on: https://go-review.googlesource.com/16458
Reviewed-by: Rahul Chaudhry <rahulchaudhry@google.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Currently, if an allocation is large enough that arena_end + size
overflows (which is not hard to do on 32-bit), we go ahead and call
sysReserve with the impossible base and length and depend on this to
either directly fail because the kernel can't possibly fulfill the
requested mapping (causing mheap.sysAlloc to return nil) or to succeed
with a mapping at some other address which will then be rejected as
outside the arena.
In order to make this less subtle, less dependent on the kernel
getting all of this right, and to eliminate the hopeless system call,
add an explicit overflow check.
Updates #13143. This real issue has been fixed by 0de59c2, but this is
a belt-and-suspenders improvement on top of that. It was uncovered by
my symbolic modeling of that bug.
Change-Id: I85fa868a33286fdcc23cdd7cdf86b19abf1cb2d1
Reviewed-on: https://go-review.googlesource.com/16961
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
mcache.tiny is in non-GC'd memory, but points to heap memory. As a
result, there may or may not be write barriers when writing to
mcache.tiny. Make it clearer that funny things are going on by making
mcache.tiny a uintptr instead of an unsafe.Pointer.
Change-Id: I732a5b7ea17162f196a9155154bbaff8d4d00eac
Reviewed-on: https://go-review.googlesource.com/16963
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
