On NetBSD and DragonFly a newly created thread inherits the signal stack
of the creating thread. That means that in a cgo program a C thread
created using pthread_create will get the signal stack of the creating
thread, most likely a Go thread. This will then lead to chaos if two
signals occur simultaneously.
We can't fix the general case. But we can fix the case of a C thread
that calls a Go function, by installing a new signal stack and then
dropping it when we return to C. That will break the case of a C thread
that calls sigaltstack and then calls Go, because we will drop the C
thread's alternate signal stack as we return from Go. Still, this is
the 1.5 behavior. And what else can we do?
Fixes#14051.
Fixes#14052.
Fixes#14067.
Change-Id: Iee286ca50b50ec712a4d929c7121c35e2383a7b9
Reviewed-on: https://go-review.googlesource.com/18835
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Use the standard names, for discoverability.
Use the standard register arguments, for correctness.
Implement all possible arguments, for completeness.
Enable the corresponding tests now that everything is standard.
Update the uses in package runtime.
Fixes#14068.
Change-Id: I8e1af9a41e7d02d98c2a82af3d4cdb3e9204824f
Reviewed-on: https://go-review.googlesource.com/18852
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It doesn't work and I don't know why.
Update #14063.
Change-Id: I42735012cf6247eca5336f29fcf713e08c8477f8
Reviewed-on: https://go-review.googlesource.com/18817
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On NetBSD a signal handler returns to the kernel by calling the
setcontext system call with the context passed to the signal handler.
The implementation of runtime·sigreturn_tramp for amd64, copied from the
NetBSD libc, expects that context address to be in r15. That works in
the NetBSD libc because r15 is preserved across the call to the signal
handler. It fails in the Go library because r15 is not preserved.
There are various ways to fix this; this one uses the simple approach,
essentially identical to the one in the NetBSD libc, of preserving r15
across the signal handler proper.
Looking at the code for 386 and arm suggests that they are OK. However,
I have not actually tested them.
Update #14052.
Change-Id: I2b516b1d05fe5d3b8911e65ca761d621dc37fa1b
Reviewed-on: https://go-review.googlesource.com/18815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On NetBSD and DragonFly a newly created thread inherits the signal stack
of the creating thread. This breaks horribly if both threads get a
signal at the same time. Fix this by dropping the signal stack in the
newly created thread. The right signal stack will then get installed
later.
Note that cgo code that calls pthread_create will have the wrong,
duplicated, signal stack in the newly created thread. I don't see any
way to fix that in Go. People using cgo to call pthread_create will
have to be aware of the problem.
Fixes#13945.
Fixes#13947.
Change-Id: I0c7bd2cdf9ada575d57182ca5e9523060de34931
Reviewed-on: https://go-review.googlesource.com/18814
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
I'm not sure what the convert function was intended to be.
Fixes#14011
Change-Id: I29d905bc1827936b9433b20b13b7a0b0ac5f502e
Reviewed-on: https://go-review.googlesource.com/18712
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is testing code in asm_GOARCH.s, so it's not necessary to run the
test on systems where it doesn't build.
Fixes#13991.
Change-Id: Ia7a2d3a34b32e6987dc67428c1e09e63baf0518a
Reviewed-on: https://go-review.googlesource.com/18707
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
GC assists check gcBlackenEnabled under the assist queue lock to avoid
going to sleep after gcWakeAllAssists has already woken all assists.
However, currently we clear gcBlackenEnabled shortly *after* waking
all assists, which opens a window where this exact race can happen.
Fix this by clearing gcBlackenEnabled before waking blocked assists.
However, it's unlikely this actually matters because the world is
stopped between waking assists and clearing gcBlackenEnabled and there
aren't any obvious allocations during this window, so I don't think an
assist could actually slip in to this race window.
Updates #13645.
Change-Id: I7571f059530481dc781d8fd96a1a40aadebecb0d
Reviewed-on: https://go-review.googlesource.com/18682
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Also adds missing nosplit to unminit.
Fixes#13964.
Change-Id: I07d93a8c872a255a89f91f808b66c889f0a6a69c
Reviewed-on: https://go-review.googlesource.com/18658
Reviewed-by: Ian Lance Taylor <iant@golang.org>
While the default behavior of eliding runtime frames from tracebacks
usually makes sense, this is not the case when you're trying to test
the runtime itself. Fix this by forcing the traceback level to at
least "system" in the runtime tests.
This will specifically help with debugging issue #13645, which has
proven remarkably resistant to reproduction outside of the build
dashboard itself.
Change-Id: I2a8356ba6c3c5badba8bb3330fc527357ec0d296
Reviewed-on: https://go-review.googlesource.com/18648
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
This doesn't fix a bug, but may improve performance in programs that
have many concurrent calls from C to Go. The old code made several
system calls between lockextra and unlockextra. That could be happening
while another thread is spinning acquiring lockextra. This changes the
code to not make any system calls while holding the lock.
Change-Id: I50576478e478670c3d6429ad4e1b7d80f98a19d8
Reviewed-on: https://go-review.googlesource.com/18548
Reviewed-by: Russ Cox <rsc@golang.org>
TestFutexsleep is supposed to clean up before returning by waking up
the goroutines it started and left blocked in futex sleeps. However,
it currently fails at this in several ways:
1. Both the sleep and wakeup are done on the address of tt.mtx, but in
both cases tt is a *local copy* of the futexsleepTest created by a
loop, so the sleep and wakeup happen on completely different
addresses. Fix this by making them both use the address of the
global tt.mtx.
2. If the sleep happens after the wakeup (not likely, but not
impossible), it won't wake up. Fix this by using the futex protocol
properly: sleep if the mutex's value is 0, and set the mutex's
value to non-zero before doing the wakeup.
3. If TestFutexsleep runs more than once, channels and mutex values
left over from the first run will interfere with later runs. Fix
this by clearing the mutex value and creating a new channel for
each test and waiting for goroutines to finish before returning
(lest they send their completion to the channel for the next run).
As an added bonus, this test now actually tests that futex
sleep/wakeup work. Previously this test would have been satisfied if
futexsleep was an infinite loop and futexwakeup was a no-op.
Change-Id: I1cbc6871cc9dcb8f4601b3621913bec2b79b0fc3
Reviewed-on: https://go-review.googlesource.com/18617
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
CMOVs were not introduced until P6. We need 386 to run on
Pentium MMX.
Fixes#13923
Change-Id: Iee9572cd83e64c3a1336bc1e6b300b048fbcc996
Reviewed-on: https://go-review.googlesource.com/18621
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Add several instructions that were used via BYTE and use them.
Instructions added: PEXTRB, PEXTRD, PEXTRQ, PINSRB, XGETBV, POPCNT.
Change-Id: I5a80cd390dc01f3555dbbe856a475f74b5e6df65
Reviewed-on: https://go-review.googlesource.com/18593
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
A bit cleanuppy for 1.6 maybe, but something I happened to notice.
Change-Id: I70f3b48445f4f527d67f7b202b6171195440b09f
Reviewed-on: https://go-review.googlesource.com/18550
Reviewed-by: Russ Cox <rsc@golang.org>
[Repeat of CL 18343 with build fixes.]
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: If26749fec06aa0ff84311f7941b88d140552e81d
Reviewed-on: https://go-review.googlesource.com/18432
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
It would certainly be a mistake to invoke a write barrier while
greying an object.
Change-Id: I34445a15ab09655ea8a3628a507df56aea61e618
Reviewed-on: https://go-review.googlesource.com/18533
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It used to be the case that repeatedly getting one GC pointer and
enqueuing one GC pointer could cause contention on the work buffers as
each operation passed over the boundary of a work buffer. As of
b6c0934, we use a two buffer cache that prevents this sort of
contention.
Change-Id: I4f1111623f76df9c5493dd9124dec1e0bfaf53b7
Reviewed-on: https://go-review.googlesource.com/18532
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This comment is probably a hold-over from when the heap bitmap was
interleaved and the shift was 0, 2, 4, or 6. Now the shift is 0, 1, 2,
or 3.
Change-Id: I096ec729e1ca31b708455c98b573dd961d16aaee
Reviewed-on: https://go-review.googlesource.com/18531
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
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>
Keith Randall
Ian Lance Taylor
Russ Cox
Michael Hudson-Doyle
Shenghou Ma
Austin Clements
Mikio Hara
Matthew Dempsky
Ilya Tocar
Rick Hudson
Brad Fitzpatrick