aboutsummaryrefslogtreecommitdiff
path: root/src/runtime/race.go
AgeCommit message (Collapse)Author
2021-02-20all: go fmt std cmd (but revert vendor)Russ Cox
Make all our package sources use Go 1.17 gofmt format (adding //go:build lines). Part of //go:build change (#41184). See https://golang.org/design/draft-gobuild Change-Id: Ia0534360e4957e58cd9a18429c39d0e32a6addb4 Reviewed-on: https://go-review.googlesource.com/c/go/+/294430 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-11-13runtime: swap the order of raceacquire() and racerelease()Daniel S Fava
In chansend() and chanrecv() of chan.go, the order of calls to raceacquire() and racerelease() was swapped, which meant that the code was not following the memory model "by the letter of the law." Similar for bufrecv and bufsend in select.go The memory model says: - A send happens before the corresponding receive completes, and - the kth receive on a channel with capacity C happens before the k+C send on that channel completes. The operative word here is "completes." For example, a sender obtains happens-before information on completion of the send-operation, which means, after the sender has deposited its message onto the channel. Similarly for receives. If the order of raceacquire() and racerelease() is incorrect, the race detector may fail to report some race conditions. The fix is minimal from the point of view of Go. The fix does, however, rely on a new function added to TSan: https://reviews.llvm.org/D76322 This commit only affects execution when race detection is enabled. Added two tests into `runtime/race/output_test.go`: - `chanmm` tests for the issue addressed by this patch - `mutex` is a test for inverted semaphores, which must not be broken by this (or any other) patch Fixes #37355 Change-Id: I5e886879ead2bd456a4b7dd1d17253641b767f63 Reviewed-on: https://go-review.googlesource.com/c/go/+/220419 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Trust: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
2020-01-09runtime: protect against external code calling ExitProcessAustin Clements
On Windows, we implement asynchronous preemption using SuspendThread to suspend other threads in our process. However, SuspendThread is itself actually asynchronous (it enqueues a kernel "asynchronous procedure call" and returns). Unfortunately, Windows' ExitProcess API kills all threads except the calling one and then runs APCs. As a result, if SuspendThread and ExitProcess are called simultaneously, the exiting thread can be suspended and the suspending thread can be exited, leaving behind a ghost process consisting of a single thread that's suspended. We've already protected against the runtime's own calls to ExitProcess, but if Go code calls external code, there's nothing stopping that code from calling ExitProcess. For example, in #35775, our own call to racefini leads to C code calling ExitProcess and occasionally causing a deadlock. This CL fixes this by introducing synchronization between calling external code on Windows and preemption. It adds an atomic field to the M that participates in a simple CAS-based synchronization protocol to prevent suspending a thread running external code. We use this to protect cgocall (which is used for both cgo calls and system calls on Windows) and racefini. Tested by running the flag package's TestParse test compiled in race mode in a loop. Before this change, this would reliably deadlock after a few minutes. Fixes #35775. Updates #10958, #24543. Change-Id: I50d847abcdc2688b4f71eee6a75eca0f2fee892c Reviewed-on: https://go-review.googlesource.com/c/go/+/213837 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
2019-11-04runtime: clean up power-of-two rounding code with align functionsMichael Anthony Knyszek
This change renames the "round" function to the more appropriately named "alignUp" which rounds an integer up to the next multiple of a power of two. This change also adds the alignDown function, which is almost like alignUp but rounds down to the previous multiple of a power of two. With these two functions, we also go and replace manual rounding code with it where we can. Change-Id: Ie1487366280484dcb2662972b01b4f7135f72fec Reviewed-on: https://go-review.googlesource.com/c/go/+/190618 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Keith Randall <khr@golang.org>
2019-10-23runtime: add race detector support for new timersIan Lance Taylor
Since the new timers run on g0, which does not have a race context, we add a race context field to the P, and use that for timer functions. This works since all timer functions are in the standard library. Updates #27707 Change-Id: I8a5b727b4ddc8ca6fc60eb6d6f5e9819245e395b Reviewed-on: https://go-review.googlesource.com/c/go/+/171882 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
2019-09-23runtime: allow the Go runtime to return multiple stack frames for a single PCKeith Randall
Upgrade the thread sanitizer to handle mid-stack inlining correctly. We can now return multiple stack frames for each pc that the thread sanitizer gives us to symbolize. To fix #33309, we still need to modify the tsan library with its portion of this fix, rebuild the .syso files on all supported archs, and check them into runtime/race. Update #33309 Change-Id: I340013631ffc8428043ab7efe3a41b6bf5638eaf Reviewed-on: https://go-review.googlesource.com/c/go/+/195781 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
2019-05-16runtime: fix 'go vet -race runtime'Russ Cox
This updates the Go function declarations to match race_amd64.s. Change-Id: I2b541a6b335ce732f4c31652aa615240ce7bb1c3 Reviewed-on: https://go-review.googlesource.com/c/go/+/177397 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Austin Clements <austin@google.com>
2019-01-08runtime: make FuncForPC return the innermost inlined frameKeith Randall
Returning the innermost frame instead of the outermost makes code that walks the results of runtime.Caller{,s} still work correctly in the presence of mid-stack inlining. Fixes #29582 Change-Id: I2392e3dd5636eb8c6f58620a61cef2194fe660a7 Reviewed-on: https://go-review.googlesource.com/c/156364 Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-11-12runtime: correct ABI information for all functionsAustin Clements
There are three cases where we don't currently have the visibility to get the ABIs of runtime symbols right, which this CL fixes: 1. For Go functions referenced from non-Go code in other packages. This is runtime.morestackc (which is referenced from function prologues) and a few syscall symbols. For these we need to generate ABI0 wrappers, so this CL adds dummy calls in the assembly code to force wrapper generation. There are many other cross-package references to runtime and runtime/internal/atomic, but these are handled specially by cmd/go. 2. For calls generated by the compiler to runtime Go functions, there are a few symbols that aren't declared in builtins.go because we've never needed their type information before. Now we at least need their ABI information, so these are added to builtins.go. 3. For calls generated by the compiler to runtime assembly functions, the compiler is going to assume the internal ABI is available, so we add Go stubs to the runtime to trigger wrapper generation. For these we're probably going to want to provide internal ABI definitions directly in the assembly for performance, but for now the ABIs are the same so it doesn't matter. For #27539. Change-Id: I9c224e7408d2ef4dd9b0e4c9d7e962ddfe111245 Reviewed-on: https://go-review.googlesource.com/c/146822 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2018-11-12internal/bytealg, runtime: provide linknames for pushed symbolsAustin Clements
The internal/bytealg package defines several symbols in the runtime, bytes, and strings packages in assembly, and the runtime package defines symbols in reflect and sync/atomic. Currently, there's no corresponding Go prototype for these symbols in the defining package. We're going to start depending on Go prototypes in the same package as their assembly definitions in order to provide ABI wrappers. Plus, these are good documentation and colocate type information with definitions, which could be useful for vet if it learned a little about linkname. This CL adds linknamed Go prototypes for all pushed symbols in internal/bytealg and runtime. For #27539. Change-Id: I9b0c12d935a75bb6af46b6761180d451c00f11b8 Reviewed-on: https://go-review.googlesource.com/c/146820 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2018-06-11runtime/race: implement race detector for ppc64leLynn Boger
This adds the support to enable the race detector for ppc64le. Added runtime/race_ppc64le.s to manage the calls from Go to the LLVM tsan functions, mostly converting from the Go ABI to the PPC64 ABI expected by Clang generated code. Changed racewalk.go to call racefuncenterfp instead of racefuncenter on ppc64le to allow the caller pc to be obtained in the asm code before calling the tsan version. Changed the set up code for racecallbackthunk so it doesn't use the autogenerated save and restore of the link register since that sequence uses registers inconsistent with the normal ppc64 ABI. Made various changes to recognize that race is supported for ppc64le. Ensured that tls_g is updated and accessible from race_linux_ppc64le.s so that the race ctx can be obtained and passed to tsan functions. This enables the race tests for ppc64le in cmd/dist/test.go and increases the timeout when running the benchmarks with the -race option to avoid timing out. Updates #24354, #23731 Change-Id: Ib97dc7ac313e6313c836dc7d2fb698f9d8fba3ef Reviewed-on: https://go-review.googlesource.com/107935 Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-15runtime: split object finding out of heapBitsForObjectAustin Clements
heapBitsForObject does two things: it finds the base of the object and it creates the heapBits for the base of the object. There are several places where we just care about the base of the object. Furthermore, greyobject only needs the heapBits in the checkmark path and can easily compute them only when needed. Once we eliminate passing the heap bits to grayobject, almost all uses of heapBitsForObject don't need the heap bits. Hence, this splits heapBitsForObject into findObject and heapBitsForAddr (the latter already exists), removes the hbits argument to grayobject, and replaces all heapBitsForObject calls with calls to findObject. In addition to making things cleaner overall, heapBitsForAddr is going to get more expensive shortly, so it's important that we don't do it needlessly. Note that there's an interesting performance pitfall here. I had originally moved findObject to mheap.go, since it made more sense there. However, that leads to a ~2% slow down and a whopping 11% increase in L1 icache misses on both the x/garbage and compilebench benchmarks. This suggests we may want to be more principled about this, but, for now, let's just leave findObject in mbitmap.go. (I tried to make findObject small enough to inline by splitting out the error case, but, sadly, wasn't quite able to get it under the inlining budget.) Change-Id: I7bcb92f383ade565d22a9f2494e4c66fd513fb10 Reviewed-on: https://go-review.googlesource.com/85878 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
2018-02-15runtime: replace mlookup and findObject with heapBitsForObjectAustin Clements
These functions all serve essentially the same purpose. mlookup is used in only one place and findObject in only three. Use heapBitsForObject instead, which is the most optimized implementation. (This may seem slightly silly because none of these uses care about the heap bits, but we're about to split up the functionality of heapBitsForObject anyway. At that point, findObject will rise from the ashes.) Change-Id: I906468c972be095dd23cf2404a7d4434e802f250 Reviewed-on: https://go-review.googlesource.com/85877 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
2017-09-22runtime: fix typo in the word "preceding"Tobias Klauser
Change-Id: I6d8c8ca0dee972cabfcc95fda23aea25692633a5 Reviewed-on: https://go-review.googlesource.com/65350 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
2017-08-29runtime: add comments to race annotationsDmitry Vyukov
Change-Id: Icfb68e73ac38d0a0acc0cda1e41f9e9c5b75ecf5 Reviewed-on: https://go-review.googlesource.com/58110 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-03-06runtime: avoid repeated findmoduledatap callsAustin Clements
Currently almost every function that deals with a *_func has to first look up the *moduledata for the module containing the function's entry point. This means we almost always do at least two identical module lookups whenever we deal with a *_func (one to get the *_func and another to get something from its module data) and sometimes several more. Fix this by making findfunc return a new funcInfo type that embeds *_func, but also includes the *moduledata, and making all of the functions that currently take a *_func instead take a funcInfo and use the already-found *moduledata. This transformation is trivial for the most part, since the *_func type is usually inferred. The annoying part is that we can no longer use nil to indicate failure, so this introduces a funcInfo.valid() method and replaces nil checks with calls to valid. Change-Id: I9b8075ef1c31185c1943596d96dec45c7ab5100f Reviewed-on: https://go-review.googlesource.com/37331 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
2017-02-27runtime: remove unused RaceSemacquire declarationDmitry Vyukov
These functions are not defined and are not used. Fixes #19290 Change-Id: I2978147220af83cf319f7439f076c131870fb9ee Reviewed-on: https://go-review.googlesource.com/37448 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Dmitry Vyukov <dvyukov@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-11-03testing: mark tests and benchmarks failed if a race occurs during executionRuss Cox
Before: $ go test -race -v -run TestRace === RUN TestRace ================== WARNING: DATA RACE Write at 0x00c420076420 by goroutine 7: _/Users/rsc/go/src/cmd/go/testdata/src/testrace.TestRace.func1() /Users/rsc/go/src/cmd/go/testdata/src/testrace/race_test.go:10 +0x3b Previous write at 0x00c420076420 by goroutine 6: _/Users/rsc/go/src/cmd/go/testdata/src/testrace.TestRace() /Users/rsc/go/src/cmd/go/testdata/src/testrace/race_test.go:13 +0xcc testing.tRunner() /Users/rsc/go/src/testing/testing.go:656 +0x104 Goroutine 7 (running) created at: _/Users/rsc/go/src/cmd/go/testdata/src/testrace.TestRace() /Users/rsc/go/src/cmd/go/testdata/src/testrace/race_test.go:12 +0xbb testing.tRunner() /Users/rsc/go/src/testing/testing.go:656 +0x104 Goroutine 6 (running) created at: testing.(*T).Run() /Users/rsc/go/src/testing/testing.go:693 +0x536 testing.runTests.func1() /Users/rsc/go/src/testing/testing.go:877 +0xaa testing.tRunner() /Users/rsc/go/src/testing/testing.go:656 +0x104 testing.runTests() /Users/rsc/go/src/testing/testing.go:883 +0x4ac testing.(*M).Run() /Users/rsc/go/src/testing/testing.go:818 +0x1c3 main.main() _/Users/rsc/go/src/cmd/go/testdata/src/testrace/_test/_testmain.go:42 +0x20f ================== --- PASS: TestRace (0.00s) PASS Found 1 data race(s) FAIL _/Users/rsc/go/src/cmd/go/testdata/src/testrace 1.026s $ After: $ go test -race -v -run TestRace === RUN TestRace ================== WARNING: DATA RACE Write at 0x00c420076420 by goroutine 7: _/Users/rsc/go/src/cmd/go/testdata/src/testrace.TestRace.func1() /Users/rsc/go/src/cmd/go/testdata/src/testrace/race_test.go:10 +0x3b Previous write at 0x00c420076420 by goroutine 6: _/Users/rsc/go/src/cmd/go/testdata/src/testrace.TestRace() /Users/rsc/go/src/cmd/go/testdata/src/testrace/race_test.go:13 +0xcc testing.tRunner() /Users/rsc/go/src/testing/testing.go:656 +0x104 Goroutine 7 (running) created at: _/Users/rsc/go/src/cmd/go/testdata/src/testrace.TestRace() /Users/rsc/go/src/cmd/go/testdata/src/testrace/race_test.go:12 +0xbb testing.tRunner() /Users/rsc/go/src/testing/testing.go:656 +0x104 Goroutine 6 (running) created at: testing.(*T).Run() /Users/rsc/go/src/testing/testing.go:693 +0x536 testing.runTests.func1() /Users/rsc/go/src/testing/testing.go:877 +0xaa testing.tRunner() /Users/rsc/go/src/testing/testing.go:656 +0x104 testing.runTests() /Users/rsc/go/src/testing/testing.go:883 +0x4ac testing.(*M).Run() /Users/rsc/go/src/testing/testing.go:818 +0x1c3 main.main() _/Users/rsc/go/src/cmd/go/testdata/src/testrace/_test/_testmain.go:42 +0x20f ================== --- FAIL: TestRace (0.00s) testing.go:609: race detected during execution of test FAIL FAIL _/Users/rsc/go/src/cmd/go/testdata/src/testrace 0.022s $ Fixes #15972. Change-Id: Idb15b8ab81d65637bb535c7e275595ca4a6e450e Reviewed-on: https://go-review.googlesource.com/32615 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-10-30runtime/race: update race runtimeDmitry Vyukov
This updates the runtime to HEAD to keep it aligned and fixes some bugs. http://llvm.org/viewvc/llvm-project?view=revision&revision=285454 fixes the crash on darwin related to unaligned data section (#17065). http://llvm.org/viewvc/llvm-project?view=revision&revision=285451 enables core dumps by default (#16527). http://llvm.org/viewvc/llvm-project?view=revision&revision=285455 adds a hook to obtain number of races reported so far (#15972). Can now be obtained with: //go:nosplit func RaceReportCount() int { var n uint64 racecall(&__tsan_report_count, uintptr(unsafe.Pointer(&n)), 0, 0, 0) return int(n) } Fixes #16527. Fixes #17065. Update #15972. Change-Id: I8f869cb6275c9521a47303f3810a9965e9314357 Reviewed-on: https://go-review.googlesource.com/32160 Run-TryBot: Dmitry Vyukov <dvyukov@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-09-25runtime/race: don't crash on invalid PCsDmitry Vyukov
Currently raceSymbolizeCode uses funcline, which is internal runtime function which crashes on incorrect PCs. Use FileLine instead, it is public and does not crash on invalid data. Note: FileLine returns "?" file on failure. That string is not NUL-terminated, so we need to additionally check what FileLine returns. Fixes #17190 Change-Id: Ic6fbd4f0e68ddd52e9b2dd25e625b50adcb69a98 Reviewed-on: https://go-review.googlesource.com/29714 Run-TryBot: Dmitry Vyukov <dvyukov@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-05-18runtime: prevent racefini from being invoked more than onceJames Chacon
racefini calls __tsan_fini which is C code and at the end of it invoked the standard C library exit(3) call. This has undefined behavior if invoked more than once. Specifically in C++ programs it caused static destructors to run twice. At least on glibc impls it also means the at_exit handlers list (where those are stored) also free's a list entry when it completes these. So invoking twice results in a double free at exit which trips debug memory allocation tracking. Fix all of this by using an atomic as a boolean barrier around calls to racefini being invoked > 1 time. Fixes #15578 Change-Id: I49222aa9b8ded77160931f46434c61a8379570fc Reviewed-on: https://go-review.googlesource.com/22882 Reviewed-by: Dmitry Vyukov <dvyukov@google.com> Run-TryBot: Dmitry Vyukov <dvyukov@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-05-03runtime: per-P contexts for race detectorDmitry Vyukov
Race runtime also needs local malloc caches and currently uses a mix of per-OS-thread and per-goroutine caches. This leads to increased memory consumption. But more importantly cache of synchronization objects is per-goroutine and we don't always have goroutine context when feeing memory in GC. As the result synchronization object descriptors leak (more precisely, they can be reused if another synchronization object is recreated at the same address, but it does not always help). For example, the added BenchmarkSyncLeak has effectively runaway memory consumption (based on a real long running server). This change updates race runtime with support for per-P contexts. BenchmarkSyncLeak now stabilizes at ~1GB memory consumption. Long term, this will allow us to remove race runtime dependency on glibc (as malloc is the main cornerstone). I've also implemented a different scheme to pass P context to race runtime: scheduler notified race runtime about association between G and P by calling procwire(g, p)/procunwire(g, p). But it turned out to be very messy as we have lots of places where the association changes (e.g. syscalls). So I dropped it in favor of the current scheme: race runtime asks scheduler about the current P. Fixes #14533 Change-Id: Iad10d2f816a44affae1b9fed446b3580eafd8c69 Reviewed-on: https://go-review.googlesource.com/19970 Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Dmitry Vyukov <dvyukov@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-01all: make copyright headers consistent with one space after periodBrad Fitzpatrick
This is a subset of https://golang.org/cl/20022 with only the copyright header lines, so the next CL will be smaller and more reviewable. Go policy has been single space after periods in comments for some time. The copyright header template at: https://golang.org/doc/contribute.html#copyright also uses a single space. Make them all consistent. Change-Id: Icc26c6b8495c3820da6b171ca96a74701b4a01b0 Reviewed-on: https://go-review.googlesource.com/20111 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-10-18runtime: merge race1.go -> race.goNodir Turakulov
* append contents of race1.go to race.go * delete "Implementation of the race detector API." comment from race1.go Updates #12952 Change-Id: Ibdd9c4dc79a63c3bef69eade9525578063c86c1c Reviewed-on: https://go-review.googlesource.com/16023 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-02-20runtime: adjust program counters in race detectorDmitry Vyukov
In most cases we pass return PC to race detector, and race runtime subtracts one from them. However, in manual instrumentation in runtime we pass function start PC to race runtime. Race runtime can't distinguish these cases and so it does not subtract one from top PC. This leads to bogus line numbers in some cases. Make it consistent and always pass what looks like a return PC, so that race runtime can subtract one and still get PC in the same function. Also delete two unused functions. Update #8053 Change-Id: I4242dec5e055e460c9a8990eaca1d085ae240ed2 Reviewed-on: https://go-review.googlesource.com/4902 Reviewed-by: Ian Lance Taylor <iant@golang.org>
2014-12-29runtime: remove go prefix from a few routinesKeith Randall
They are no longer needed now that C is gone. goatoi -> atoi gofuncname/funcname -> funcname/cfuncname goroundupsize -> already existing roundupsize Change-Id: I278bc33d279e1fdc5e8a2a04e961c4c1573b28c7 Reviewed-on: https://go-review.googlesource.com/2154 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Minux Ma <minux@golang.org>
2014-11-12[dev.cc] runtime: delete scalararg, ptrarg; rename onM to systemstackRuss Cox
Scalararg and ptrarg are not "signal safe". Go code filling them out can be interrupted by a signal, and then the signal handler runs, and if it also ends up in Go code that uses scalararg or ptrarg, now the old values have been smashed. For the pieces of code that do need to run in a signal handler, we introduced onM_signalok, which is really just onM except that the _signalok is meant to convey that the caller asserts that scalarg and ptrarg will be restored to their old values after the call (instead of the usual behavior, zeroing them). Scalararg and ptrarg are also untyped and therefore error-prone. Go code can always pass a closure instead of using scalararg and ptrarg; they were only really necessary for C code. And there's no more C code. For all these reasons, delete scalararg and ptrarg, converting the few remaining references to use closures. Once those are gone, there is no need for a distinction between onM and onM_signalok, so replace both with a single function equivalent to the current onM_signalok (that is, it can be called on any of the curg, g0, and gsignal stacks). The name onM and the phrase 'm stack' are misnomers, because on most system an M has two system stacks: the main thread stack and the signal handling stack. Correct the misnomer by naming the replacement function systemstack. Fix a few references to "M stack" in code. The main motivation for this change is to eliminate scalararg/ptrarg. Rick and I have already seen them cause problems because the calling sequence m.ptrarg[0] = p is a heap pointer assignment, so it gets a write barrier. The write barrier also uses onM, so it has all the same problems as if it were being invoked by a signal handler. We worked around this by saving and restoring the old values and by calling onM_signalok, but there's no point in keeping this nice home for bugs around any longer. This CL also changes funcline to return the file name as a result instead of filling in a passed-in *string. (The *string signature is left over from when the code was written in and called from C.) That's arguably an unrelated change, except that once I had done the ptrarg/scalararg/onM cleanup I started getting false positives about the *string argument escaping (not allowed in package runtime). The compiler is wrong, but the easiest fix is to write the code like Go code instead of like C code. I am a bit worried that the compiler is wrong because of some use of uninitialized memory in the escape analysis. If that's the reason, it will go away when we convert the compiler to Go. (And if not, we'll debug it the next time.) LGTM=khr R=r, khr CC=austin, golang-codereviews, iant, rlh https://golang.org/cl/174950043
2014-11-11[dev.cc] runtime: convert race implementation from C to GoRuss Cox
The conversion was done with an automated tool and then modified only as necessary to make it compile and run. [This CL is part of the removal of C code from package runtime. See golang.org/s/dev.cc for an overview.] LGTM=r R=r CC=austin, dvyukov, golang-codereviews, iant, khr https://golang.org/cl/172250044
2014-09-08liblink, runtime: diagnose and fix C code running on Go stackRuss Cox
This CL contains compiler+runtime changes that detect C code running on Go (not g0, not gsignal) stacks, and it contains corrections for what it detected. The detection works by changing the C prologue to use a different stack guard word in the G than Go prologue does. On the g0 and gsignal stacks, that stack guard word is set to the usual stack guard value. But on ordinary Go stacks, that stack guard word is set to ^0, which will make any stack split check fail. The C prologue then calls morestackc instead of morestack, and morestackc aborts the program with a message about running C code on a Go stack. This check catches all C code running on the Go stack except NOSPLIT code. The NOSPLIT code is allowed, so the check is complete. Since it is a dynamic check, the code must execute to be caught. But unlike the static checks we've been using in cmd/ld, the dynamic check works with function pointers and other indirect calls. For example it caught sigpanic being pushed onto Go stacks in the signal handlers. Fixes #8667. LGTM=khr, iant R=golang-codereviews, khr, iant CC=golang-codereviews, r https://golang.org/cl/133700043
2014-09-08build: move package sources from src/pkg to srcRuss Cox
Preparation was in CL 134570043. This CL contains only the effect of 'hg mv src/pkg/* src'. For more about the move, see golang.org/s/go14nopkg.