Fixes#13881.
Change-Id: Idff77db381640184ddd2b65022133bb226168800
Reviewed-on: https://go-review.googlesource.com/18449
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Currently, due to an oversight, we only balance work buffers
in background and idle workers and not in assists. As a
result, in assist-heavy workloads, assists are likely to tie
up large work buffers in per-P caches increasing the
likelihood that the global list will be empty. This increases
the likelihood that other GC workers will exit and assists
will block, slowing down the system as a whole. Fix this by
eagerly balancing work buffers as soon as the assists notice
that the global buffers are empty. This makes it much more
likely that work will be immediately available to other
workers and assists.
This change reduces the garbage benchmark time by 39% and
fixes the regresssion seen at CL 15893 golang.org/cl/15893.
Garbage benchmark times before and after this CL.
Before GOPERF-METRIC:time=4427020
After GOPERF-METRIC:time=2721645
Fixes#13827
Change-Id: I9cb531fb873bab4b69ce9c1617e30df6c49cdcfe
Reviewed-on: https://go-review.googlesource.com/18341
Reviewed-by: Austin Clements <austin@google.com>
The previous behaviour of installing the signal handlers in a separate
thread meant that Go initialization raced with non-Go initialization if
the non-Go initialization also wanted to install signal handlers. Make
installing signal handlers synchronous so that the process-wide behavior
is predictable.
Update #9896.
Change-Id: Ice24299877ec46f8518b072a381932d273096a32
Reviewed-on: https://go-review.googlesource.com/18150
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Go 1.6 simplified the GC phases. The "synchronize Ps" phase no longer
exists and "root scan" and "mark" phases have been combined.
Update the gctrace line implementation and documentation to remove the
unused phases.
Fixes#13536.
Change-Id: I4fc37a3ce1ae3a99d48c0be2df64cbda3e05dee6
Reviewed-on: https://go-review.googlesource.com/18458
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Sigh. Sleeps on FreeBSD also yield the rest of the time slice and
profiling signals are only delivered when a process completes a time
slice (worse, itimer time is only accounted to the process that
completes a time slice). It's less noticeable than the other BSDs
because the default tick rate is 1000Hz, but it's still failing
regularly.
Fixes#13846.
Change-Id: I41bf116bffe46682433b677183f86944d0944ed4
Reviewed-on: https://go-review.googlesource.com/18455
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
We're only getting away with it today by luck.
Change-Id: I24d1cceee4d20c5181ca64fceda152e875f6ad81
Reviewed-on: https://go-review.googlesource.com/18440
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Today, signal.Ignore(syscall.SIGTRAP) does nothing
while signal.Notify(make(chan os.Signal), syscall.SIGTRAP)
correctly discards user-generated SIGTRAPs.
The same applies to any signal that we throw on.
Make signal.Ignore work for these signals.
Fixes#12906.
Change-Id: Iba244813051e0ce23fa32fbad3e3fa596a941094
Reviewed-on: https://go-review.googlesource.com/18348
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
OS X unconditionally sets si_code = TRAP_BRKPT when sending SIGTRAP,
even if it was generated by kill -TRAP and not a breakpoint.
Correct the si_code by looking to see if the PC is after a breakpoint.
For #12906.
Change-Id: I998c2499f7f12b338e607282a325b045f1f4f690
Reviewed-on: https://go-review.googlesource.com/18347
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Before, NumGoroutine counted system goroutines and Stack (usually) didn't show them,
which was inconsistent and confusing.
To resolve which way they should be consistent, it seems like
package main
import "runtime"
func main() { println(runtime.NumGoroutine()) }
should print 1 regardless of internal runtime details. Make it so.
Fixes#11706.
Change-Id: I6bfe26a901de517728192cfb26a5568c4ef4fe47
Reviewed-on: https://go-review.googlesource.com/18343
Reviewed-by: Austin Clements <austin@google.com>
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>
Ian Lance Taylor
Rick Hudson
Austin Clements
Mikio Hara
Russ Cox
Brad Fitzpatrick
Shenghou Ma