summaryrefslogtreecommitdiff
path: root/src/runtime
Commit message (Collapse)AuthorAgeFilesLines
* runtime/cgo: store M for C-created thread in pthread keyCherry Mui2023-05-1737-66/+808
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reapplies CL 485500, with a fix drafted in CL 492987 incorporated. CL 485500 is reverted due to #60004 and #60007. #60004 is fixed in CL 492743. #60007 is fixed in CL 492987 (incorporated in this CL). [Original CL 485500 description] This reapplies CL 481061, with the followup fixes in CL 482975, CL 485315, and CL 485316 incorporated. CL 481061, by doujiang24 <doujiang24@gmail.com>, speed up C to Go calls by binding the M to the C thread. See below for its description. CL 482975 is a followup fix to a C declaration in testprogcgo. CL 485315 is a followup fix for x_cgo_getstackbound on Illumos. CL 485316 is a followup cleanup for ppc64 assembly. CL 479915 passed the G to _cgo_getstackbound for direct updates to gp.stack.lo. A G can be reused on a new thread after the previous thread exited. This could trigger the C TSAN race detector because it couldn't see the synchronization in Go (lockextra) preventing the same G from being used on multiple threads at the same time. We work around this by passing the address of a stack variable to _cgo_getstackbound rather than the G. The stack is generally unique per thread, so TSAN won't see the same address from multiple threads. Even if stacks are reused across threads by pthread, C TSAN should see the synchonization in the stack allocator. A regression test is added to misc/cgo/testsanitizer. [Original CL 481061 description] This reapplies CL 392854, with the followup fixes in CL 479255, CL 479915, and CL 481057 incorporated. CL 392854, by doujiang24 <doujiang24@gmail.com>, speed up C to Go calls by binding the M to the C thread. See below for its description. CL 479255 is a followup fix for a small bug in ARM assembly code. CL 479915 is another followup fix to address C to Go calls after the C code uses some stack, but that CL is also buggy. CL 481057, by Michael Knyszek, is a followup fix for a memory leak bug of CL 479915. [Original CL 392854 description] In a C thread, it's necessary to acquire an extra M by using needm while invoking a Go function from C. But, needm and dropm are heavy costs due to the signal-related syscalls. So, we change to not dropm while returning back to C, which means binding the extra M to the C thread until it exits, to avoid needm and dropm on each C to Go call. Instead, we only dropm while the C thread exits, so the extra M won't leak. When invoking a Go function from C: Allocate a pthread variable using pthread_key_create, only once per shared object, and register a thread-exit-time destructor. And store the g0 of the current m into the thread-specified value of the pthread key, only once per C thread, so that the destructor will put the extra M back onto the extra M list while the C thread exits. When returning back to C: Skip dropm in cgocallback, when the pthread variable has been created, so that the extra M will be reused the next time invoke a Go function from C. This is purely a performance optimization. The old version, in which needm & dropm happen on each cgo call, is still correct too, and we have to keep the old version on systems with cgo but without pthreads, like Windows. This optimization is significant, and the specific value depends on the OS system and CPU, but in general, it can be considered as 10x faster, for a simple Go function call from a C thread. For the newly added BenchmarkCGoInCThread, some benchmark results: 1. it's 28x faster, from 3395 ns/op to 121 ns/op, in darwin OS & Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz 2. it's 6.5x faster, from 1495 ns/op to 230 ns/op, in Linux OS & Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz [CL 479915 description] Currently, when C calls into Go the first time, we grab an M using needm, which sets m.g0's stack bounds using the SP. We don't know how big the stack is, so we simply assume 32K. Previously, when the Go function returns to C, we drop the M, and the next time C calls into Go, we put a new stack bound on the g0 based on the current SP. After CL 392854, we don't drop the M, and the next time C calls into Go, we reuse the same g0, without recomputing the stack bounds. If the C code uses quite a bit of stack space before calling into Go, the SP may be well below the 32K stack bound we assumed, so the runtime thinks the g0 stack overflows. This CL makes needm get a more accurate stack bound from pthread. (In some platforms this may still be a guess as we don't know exactly where we are in the C stack), but it is probably better than simply assuming 32K. [CL 492987 description] On the first call into Go from a C thread, currently we set the g0 stack's high bound imprecisely based on the SP. With CL 485500, we keep the M and don't recompute the stack bounds when it calls into Go again. If the first call is made when the C thread uses some deep stack, but a subsequent call is made with a shallower stack, the SP may be above g0.stack.hi. This is usually okay as we don't check usually stack.hi. One place where we do check for stack.hi is in the signal handler, in adjustSignalStack. In particular, C TSAN delivers signals on the g0 stack (instead of the usual signal stack). If the SP is above g0.stack.hi, we don't see it is on the g0 stack, and throws. This CL makes it get an accurate stack upper bound with the pthread API (on the platforms where it is available). Also add some debug print for the "handler not on signal stack" throw. Fixes #51676. Fixes #59294. Fixes #59678. Fixes #60007. Change-Id: Ie51c8e81ade34ec81d69fd7bce1fe0039a470776 Reviewed-on: https://go-review.googlesource.com/c/go/+/495855 Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: publish netpoll info after incrementing fdseqIan Lance Taylor2023-05-171-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | I think there is a theoretical possibility of a mistake before this CL. pollCache.free would increment fdseq, but would not update atomicInfo. The epoll code could compare to fdseq before the increment, but suspend before calling setEventErr. The pollCache could get reallocated, and pollOpen could clear eventErr. Then the setEventErr could continue and set it again. Then pollOpen could call publishInfo. Avoid this rather remote possibility by calling publishInfo after incrementing fdseq. That ensures that delayed setEventErr will not modify the eventErr flag. Fixes #60133 Change-Id: I69e336535312544690821c9fd53f3023ff15b80c Reviewed-on: https://go-review.googlesource.com/c/go/+/495297 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
* runtime: replace sysBlockTraced with tracedSyscallEnterMichael Anthony Knyszek2023-05-172-9/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sysBlockTraced is a subtle and confusing flag. Currently, it's only used in one place: a condition around whether to traceGoSysExit when a goroutine is about to start running. That condition looks like "gp.syscallsp != 0 && gp.trace.sysBlockTraced". In every case but one, "gp.syscallsp != 0" is equivalent to "gp.trace.sysBlockTraced." That one case is where a goroutine is running without a P and racing with trace start (world is stopped). It switches itself back to _Grunnable from _Gsyscall before the trace start goroutine notices, such that the trace start goroutine fails to emit a EvGoInSyscall event for it (EvGoInSyscall or EvGoSysBlock must precede any EvGoSysExit event). sysBlockTraced is set unconditionally on every syscall entry and the trace start goroutine clears it if there was no EvGoInSyscall event emitted (i.e. did not observe _Gsyscall on the goroutine). That way when the goroutine-without-a-P wakes up and gets scheduled, it only emits EvGoSysExit if the flag is set, i.e. trace start didn't _clear_ the flag. What makes this confusing is the fact that the flag is set unconditionally and the code relies on it being *cleared*. Really, all it's trying to communicate is whether the tracer is aware of a goroutine's syscall at the point where a goroutine that lost its P during a syscall is trying to run again. Therefore, we can replace this flag with a less subtle one: tracedSyscallEnter. It is set when GoSysCall is traced, indicating on the goroutine that the tracer is aware of the syscall. Later, if traceGoSysExit is called, the tracer knows its safe to emit an event because the tracer is aware of the syscall. This flag is then also set at trace start, when it emits EvGoInSyscall, which again, lets the goroutine know the tracer is aware of its syscall. The flag is cleared by GoSysExit to indicate that the tracer is no longer aware of any syscalls on the goroutine. It's also cleared by trace start. This is necessary because a syscall may have been started while a trace was stopping. If the GoSysExit isn't emitted (because it races with the trace end STW) then the flag will be left set at the start of the next trace period, which will result in an erroneous GoSysExit. Instead, the flag is cleared in the same way sysBlockTraced is today: if the tracer doesn't notice the goroutine is in a syscall, it makes that explicit to the goroutine. A more direct flag to use here would be one that explicitly indicates whether EvGoInSyscall or EvGoSysBlock specifically were already emitted for a goroutine. The reason why we don't just do this is because setting the flag when EvGoSysBlock is emitted would be racy: EvGoSysBlock is emitted by whatever thread is stealing the P out from under the syscalling goroutine, so it would need to synchronize with the goroutine its emitting the event for. The end result of all this is that the new flag can be managed entirely within trace.go, hiding another implementation detail about the tracer. Tested with `stress ./trace.test -test.run="TestTraceStressStartStop"` which was occasionally failing before the CL in which sysBlockTraced was added (CL 9132). I also confirmed also that this test is still sensitive to `EvGoSysExit` by removing the one use of sysBlockTraced. The result is about a 5% error rate. If there is something very subtly wrong about how this CL emits `EvGoSysExit`, I would expect to see it as a test failure. Instead: 53m55s: 200434 runs so far, 0 failures Change-Id: If1d24ee6b6926eec7e90cdb66039a5abac819d9b Reviewed-on: https://go-review.googlesource.com/c/go/+/494715 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> Auto-Submit: Michael Knyszek <mknyszek@google.com>
* runtime: hide sysExitTicks a little betterMichael Anthony Knyszek2023-05-172-9/+10
| | | | | | | | | | | Just another step to hiding implementation details. Change-Id: I71b7cc522d18c23f03a9bf32e428279e62b39a89 Reviewed-on: https://go-review.googlesource.com/c/go/+/494192 Run-TryBot: Michael Knyszek <mknyszek@google.com> Auto-Submit: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: capture per-p trace state in a typeMichael Anthony Knyszek2023-05-173-25/+31
| | | | | | | | | | | More tightening up of the tracer's interface. Change-Id: I992141c7f30e5c2d5d77d1fcd6817d35bc6e5f6d Reviewed-on: https://go-review.googlesource.com/c/go/+/494191 Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: capture per-m trace state in a typeMichael Anthony Knyszek2023-05-172-10/+19
| | | | | | | | | | | | | | More tightening up of the tracer's interface. While we're here, clarify why waittraceskip isn't included by explaining what the wait* fields in the M are really for. Change-Id: I0e7b4cac79fb77a7a0b3ca6b6cc267668e3610bc Reviewed-on: https://go-review.googlesource.com/c/go/+/494190 Auto-Submit: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: capture per-g trace state in a typeMichael Anthony Knyszek2023-05-174-58/+65
| | | | | | | | | | | | | | | More tightening up of the tracer's interface. This increases the size of each G very slightly, which isn't great, but we stay within the same size class, so actually memory use will be unchanged. Change-Id: I7d1f5798edcf437c212beb1e1a2619eab833aafb Reviewed-on: https://go-review.googlesource.com/c/go/+/494188 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com>
* hash/maphash: weaken avalanche test a bitKeith Randall2023-05-171-1/+1
| | | | | | | | | | | | | | | | | Give the test a bit more wiggle room. Previously the allowed range was about 46.5% to 53.5%. Now it is about 43% TO 57%. Fixes #60170 Change-Id: Ieda471e0986c52edb9f6d31beb8e41917876d6c5 Reviewed-on: https://go-review.googlesource.com/c/go/+/495415 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Keith Randall <khr@google.com> Run-TryBot: Keith Randall <khr@golang.org>
* runtime: factor our oneNewExtraM trace codeMichael Anthony Knyszek2023-05-172-6/+14
| | | | | | | | | | | | In the interest of further cleaning up the trace.go API, move the trace logic in oneNewExtraM into its own function. Change-Id: I5cf478cb8cd0d301ee3b068347ed48ce768b8882 Reviewed-on: https://go-review.googlesource.com/c/go/+/494186 Reviewed-by: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> Auto-Submit: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: consistently define fcntlIan Lance Taylor2023-05-1734-86/+209
| | | | | | | | | | | | | | | | Clean up and consolidate on a single consistent definition of fcntl, which takes three int32 arguments and returns either a positive result or a negative errno value. Change-Id: Id9505492712db4b0aab469c6bd15e4fce3c9ff6e Reviewed-on: https://go-review.googlesource.com/c/go/+/495075 Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: improve ppc64x memclr for tail bytesLynn Boger2023-05-161-5/+15
| | | | | | | | | | | | | This improves memclr for the last few bytes when compiling for power9 or earlier. Change-Id: I46940ebc7e98e27a2e48d4b319acb7d2106a6f29 Reviewed-on: https://go-review.googlesource.com/c/go/+/495035 Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Paul Murphy <murp@ibm.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
* os, runtime: remove unused implementations of os.sigpipeIan Lance Taylor2023-05-122-10/+0
| | | | | | | | | | | | | | | Clean up instances that are unused since CL 6450058. Change-Id: I0e9ae28cfa83fcc8abda8f5eca9c7dfc2c1c4ad1 Reviewed-on: https://go-review.googlesource.com/c/go/+/477396 Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
* reflect: do not escape Value.TypeCherry Mui2023-05-121-0/+3
| | | | | | | | | | | | | | | | | | | Types are either static (for compiler-created types) or heap allocated and always reachable (for reflection-created types, held in the central map). So there is no need to escape types. With CL 408826 reflect.Value does not always escape. Some functions that escapes Value.typ would make the Value escape without this CL. Had to add a special case for the inliner to keep (*Value).Type still inlineable. Change-Id: I7c14d35fd26328347b509a06eb5bd1534d40775f Reviewed-on: https://go-review.googlesource.com/c/go/+/413474 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: David Chase <drchase@google.com> Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* reflect: allow Value be stack allocatedCherry Mui2023-05-122-3/+3
| | | | | | | | | | | | | | | | | | | Currently, reflect.ValueOf forces the referenced object to be heap allocated. This CL makes it possible to be stack allocated. We need to be careful to make sure the compiler's escape analysis can do the right thing, e.g. channel send, map assignment, unsafe pointer conversions. Tests will be added in a later CL. CL 408827 might help ensure the correctness. Change-Id: I8663651370c7c8108584902235062dd2b3f65954 Reviewed-on: https://go-review.googlesource.com/c/go/+/408826 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
* runtime: remove unused skip arg from fpTracebackPCsFelix Geisendörfer2023-05-123-7/+7
| | | | | | | | | | | | This was accidentally left behind when moving the logic to set the skip sentinel in pcBuf to the caller. Change-Id: Id7565f6ea4df6b32cf18b99c700bca322998d182 Reviewed-on: https://go-review.googlesource.com/c/go/+/489095 Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
* runtime: remove crash_cgo_test CgoRaceSignal timeoutIan Lance Taylor2023-05-122-14/+1
| | | | | | | | | | | | | | | | | | The test had a 5 second timeout. Running the test on a Darwin system sometimes took less than 5 seconds but often took up to 8 seconds. We don't need a timeout anyhow. Instead, use testenv.Command to run the program, which uses the test timeout. Fixes #59807 Change-Id: Ibf3eda9702731bf98601782f4abd11c3caa0bf40 Reviewed-on: https://go-review.googlesource.com/c/go/+/494456 Run-TryBot: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
* runtime: save/fetch g register during VDSO on linux/loong64Guoqi Chen2023-05-112-5/+41
| | | | | | | | | | | | | | | | | | Like arm64, ppc64 and risv64, on loong64, the G register may be temporarily broken during a VDSO call. If a signal is received during a VDSO call, an invalid G may be obtained. See #34391. Change-Id: Iaffa8cce4f0ef8ef74225c355ec3c20ed238025f Reviewed-on: https://go-review.googlesource.com/c/go/+/426355 Reviewed-by: WANG Xuerui <git@xen0n.name> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Run-TryBot: WANG Xuerui <git@xen0n.name> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
* runtime: remove unused traceFutileWakeupMichael Anthony Knyszek2023-05-111-12/+2
| | | | | | | | | | | Also, document traceEvFutileWakeup as not currently used. Change-Id: I75831a43d39b6c6ceb5a9b6320c3ae9455681572 Reviewed-on: https://go-review.googlesource.com/c/go/+/494184 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: hide trace.shutdown behind traceShuttingDownMichael Anthony Knyszek2023-05-112-2/+7
| | | | | | | | | Change-Id: I0b123e65f40570caeee611679d80dc27034d5a52 Reviewed-on: https://go-review.googlesource.com/c/go/+/494183 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Michael Knyszek <mknyszek@google.com>
* runtime: replace trace.enabled with traceEnabledMichael Anthony Knyszek2023-05-118-60/+60
| | | | | | | | | | | | | [git-generate] cd src/runtime grep -l 'trace\.enabled' *.go | grep -v "trace.go" | xargs sed -i 's/trace\.enabled/traceEnabled()/g' Change-Id: I14c7821c1134690b18c8abc0edd27abcdabcad72 Reviewed-on: https://go-review.googlesource.com/c/go/+/494181 Run-TryBot: Michael Knyszek <mknyszek@google.com> Auto-Submit: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: add traceEnabled functionMichael Anthony Knyszek2023-05-111-0/+5
| | | | | | | | | | | | | | | This change introduces the trivial traceEnabled function to help tighten up the execution tracer's API in preparation for the execution trace redesign GOEXPERIMENT. A follow-up change will refactor the runtime to use it. Change-Id: I19c8728e30aefe543b4a826d95446affa14897e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/494180 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
* runtime: hide trace lock init detailsMichael Anthony Knyszek2023-05-112-4/+9
| | | | | | | | | | | | This change is in service of hiding more execution trace implementation details for big changes to come. Change-Id: I49b9716a7bf285d23c86b58912a05eff4ddc2213 Reviewed-on: https://go-review.googlesource.com/c/go/+/494182 Reviewed-by: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> Auto-Submit: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* Revert "runtime: rename getcallerfp to getfp"Michael Pratt2023-05-1115-35/+26
| | | | | | | | | | | | | | This reverts CL 481617. Reason for revert: breaks test build on Windows Change-Id: Ifc1a323b0cc521e7a5a1f7de7b3da667f5fee375 Reviewed-on: https://go-review.googlesource.com/c/go/+/494377 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: rename getcallerfp to getfpFelix Geisendörfer2023-05-1115-26/+35
| | | | | | | | | | | | | | | | | | | | | The previous name was wrong due to the mistaken assumption that calling f->g->getcallerpc and f->g->getcallersp would respectively return the pc/sp at g. However, they are actually referring to their caller's caller, i.e. f. Rename getcallerfp to getfp in order to stay consistent with this naming convention. Also see discussion on CL 463835. For #16638 Change-Id: I07990645da78819efd3db92f643326652ee516f8 Reviewed-on: https://go-review.googlesource.com/c/go/+/481617 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: move per-type types to internal/abiDavid Chase2023-05-1118-507/+376
| | | | | | | | | Change-Id: I1f031f0f83a94bebe41d3978a91a903dc5bcda66 Reviewed-on: https://go-review.googlesource.com/c/go/+/489276 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: redefine _type to abi.Type; add rtype for methods.David Chase2023-05-1116-71/+73
| | | | | | | | | Change-Id: I1c478b704d84811caa209006c657dda82d9c4cf9 Reviewed-on: https://go-review.googlesource.com/c/go/+/488435 Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
* internal/abi,reflectlite,reflect,runtime: common up chan typeDavid Chase2023-05-102-9/+5
| | | | | | | | | Change-Id: I085b61c544b85d70fabb1c0d9fe91207826dd21a Reviewed-on: https://go-review.googlesource.com/c/go/+/484858 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
* internal/reflectlite, runtime: move more constants and types into internal/abiDavid Chase2023-05-105-66/+14
| | | | | | | | | Change-Id: If5da1057ead34eb3e4c7f42bbe6ad3d350b97725 Reviewed-on: https://go-review.googlesource.com/c/go/+/484856 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* internal/abi: common up ArrayTypeDavid Chase2023-05-105-23/+18
| | | | | | | | | | | | | | | | This refactoring is more problematic because the client package wrap abi.Type, thus the self-referential fields within ArrayType need to be downcast to the client wrappers in several places. It's not clear to me this is worthwhile; this CL is for additional comment, before I attempt similar changes for other self-referential types. Change-Id: I41e517e6d851b32560c41676b91b76d7eb17c951 Reviewed-on: https://go-review.googlesource.com/c/go/+/466236 Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
* internal/abi: common up Method, Imethod, UncommonType typesDavid Chase2023-05-103-38/+20
| | | | | | | | | | | | was two commits, the first contained a lot of intermediate work, better this way. Change-Id: I7c5b79ef78b21a85828c8aaf9baeae86bb144db7 Reviewed-on: https://go-review.googlesource.com/c/go/+/463118 Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* all: fix a lot of commentscui fliter2023-05-102-2/+2
| | | | | | | | | | | | | Fix comments, including duplicate is, wrong phrases and articles, misspellings, etc. Change-Id: I8bfea53b9b275e649757cc4bee6a8a026ed9c7a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/493035 Reviewed-by: Benny Siegert <bsiegert@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: shuang cui <imcusg@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
* runtime: fix misaligned SP for libfuzzer entryKeith Randall2023-05-101-0/+1
| | | | | | | | | | | | | | | | | | | | | libfuzzer is written in C and so requires by the C abi that SP be aligned correctly mod 16. Normally CALLs need to have SP aligned to 0 mod 16, but because we're simulating a CALL (which pushes a return address) with a JMP (which doesn't), we need to align to 8 mod 16 before JMPing. This is not causing any current problems that I know of. All the functions called from this callsite that I checked don't rely on correct alignment. So this CL is just futureproofing. Update #49075 Change-Id: I13fcbe9aaf2853056a6d44dc3aa64b7db689e144 Reviewed-on: https://go-review.googlesource.com/c/go/+/494117 Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
* runtime: exclude extra M's from debug.SetMaxThreadsMichael Pratt2023-05-091-4/+25
| | | | | | | | | | | | | | | | | | | The purpose of the debug.SetMaxThreads limit is to avoid accidental fork bomb from something like millions of goroutines blocking on system calls, causing the runtime to create millions of threads. By definition we don't create threads created in C, so this isn't a problem for those threads, and we can exclude them from the limit. If C wants to create tens of thousands of threads, who are we to say no? Fixes #60004. Change-Id: I62b875890718b406abca42a9a4078391e25aa21b Reviewed-on: https://go-review.googlesource.com/c/go/+/492743 Auto-Submit: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Michael Pratt <mpratt@google.com>
* runtime: calculate nanoseconds in usleep on linux/loong64Guoqi Chen2023-05-091-10/+10
| | | | | | | | | | | | | | Change-Id: Ia4cfdea3df8834e6260527ce8e6e894a0547070f Reviewed-on: https://go-review.googlesource.com/c/go/+/425299 Reviewed-by: xiaodong liu <teaofmoli@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Run-TryBot: WANG Xuerui <git@xen0n.name> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: WANG Xuerui <git@xen0n.name> Reviewed-by: Meidan Li <limeidan@loongson.cn>
* runtime: remove TestCrashExitCodeqmuntal2023-05-081-32/+6
| | | | | | | | | | | | | | | | | | | TestCrashExitCode was added in CL 491935 to test that the exit code is honored when using GOTRACEBACK=crash, which is what normally happens on a stock Windows. The problem is that some applications (not only WER, as I incorrectly assumed in CL 491935) can hijack a crashing process and change its exit code. There is no way to tell if a crashing process using GOTRACEBACK=crash/ wer will have its error code hijacked, so we better don't test this behavior, which in fact is not documented by the Go runtime. Change-Id: Ib8247a8a1fe6303c4c7812a1bf2ded5f4e89acb1 Reviewed-on: https://go-review.googlesource.com/c/go/+/493495 Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: clean up extra M APIMichael Pratt2023-05-082-36/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are quite a few locations that get/put Ms from the extra M list, but the API is pretty clumsy to use. Add an easier to use getExtraM / putExtraM API. There are only two minor semantic changes: 1. dropm no longer calls setg(nil) inside the lockextra critical section. It is important that this thread no longer references the G (and in turn M) once it is published to the extra M list and another thread could acquire it. But there is no reason that needs to happen only after lockextra. 2. extraMLength (renamed from extraMCount) is no longer protected by lockextra and is instead simply an atomic (though writes are still in the critical section). The previous readers all dropped lockextra before using the value they read anyway. For #60004. Change-Id: Ifca4d6c84d605423855d89f49af400ca07de56f4 Reviewed-on: https://go-review.googlesource.com/c/go/+/492742 Run-TryBot: Michael Pratt <mpratt@google.com> Commit-Queue: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
* runtime, runtime/pprof: record instantiated symbol name in CPU profileCherry Mui2023-05-053-1/+22
| | | | | | | | | | | | | | | | | | For generic functions, the previous CL makes it record the full instantiated symbol name in the runtime func table. This CL changes the pprof package to use that name in CPU profile. This way, it matches the symbol name the compiler sees, so it can apply PGO. TODO: add a test. Fixes #58712. Change-Id: If40db01cbef5f73c279adcc9c290a757ef6955b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/491678 Reviewed-by: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Cherry Mui <cherryyz@google.com>
* cmd/link, runtime: include full symbol name for generic functions in runtime ↵Cherry Mui2023-05-054-15/+110
| | | | | | | | | | | | | | | | | | | | | | | | | table For generic functions and methods, we replace the instantiated shape type parameter name to "...", to make the function name printed in stack traces looks more user friendly. Currently, this is done in the binary's runtime func table at link time, and the runtime has no way to access the full symbol name. This causes the profile to also contain the replaced name. For PGO, this also cause the compiler to not be able to find out the original fully instantiated function name from the profile. With this CL, we change the linker to record the full name, and do the name substitution at run time when a printing a function's name in traceback. For #58712. Change-Id: Ia0ea0989a1ec231f3c4fbf59365c9333405396c6 Reviewed-on: https://go-review.googlesource.com/c/go/+/491677 Reviewed-by: Michael Pratt <mpratt@google.com> Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* internal/abi: refactor (basic) type struct into one definitionDavid Chase2023-05-0529-362/+329
| | | | | | | | | | | | | | | This touches a lot of files, which is bad, but it is also good, since there's N copies of this information commoned into 1. The new files in internal/abi are copied from the end of the stack; ultimately this will all end up being used. Change-Id: Ia252c0055aaa72ca569411ef9f9e96e3d610889e Reviewed-on: https://go-review.googlesource.com/c/go/+/462995 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org> Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
* Revert "runtime/cgo: store M for C-created thread in pthread key"Chressie Himpel2023-05-0537-802/+64
| | | | | | | | | | | | This reverts CL 485500. Reason for revert: This breaks internal tests at Google, see b/280861579 and b/280820455. Change-Id: I426278d400f7611170918fc07c524cb059b9cc55 Reviewed-on: https://go-review.googlesource.com/c/go/+/492995 Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Chressie Himpel <chressie@google.com>
* runtime: don't check the exit code in TestWERDialogueqmuntal2023-05-051-12/+37
| | | | | | | | | | | | | | | | | | | | | TestWERDialogue intent is to check that the WER dialog doesn't pop-up when `GOTRACEBACK=wer` is set. CL 474915 extended the test to also check the error code of the crashed process. This change is causing failures in Microsoft internal test pipelines because some WER setups can modify the exit code of the crashed application, for example to signal that the crash dump has been collected. Fix this issue by not checking the error code in TestWERDialogue. Also, add a new test, TestCrashExitCode, which does the same but using `GOTRACEBACK=crash` instead, so that we have one test that checks the error code. Change-Id: Iedde09e1df7223009ebef38a32a460f1ab07e31a Reviewed-on: https://go-review.googlesource.com/c/go/+/491935 Run-TryBot: Quim Muntal <quimmuntal@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
* runtime: don't run TestStackGrowth in parallel with other testsCherry Mui2023-05-041-2/+0
| | | | | | | | | | | | | | This test calls runtime.GC quite a number of times. GC is a global operation. To reduce interference with other tests, don't run this test in parallel with other tests. May fix #57601. Change-Id: I6efadb62c4dada37a927455f5c6cd98cafb88aaf Reviewed-on: https://go-review.googlesource.com/c/go/+/492715 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
* runtime/cgo: preserve VRs across crosscall_ppc64 on linuxPaul E. Murphy2023-05-041-91/+47
| | | | | | | | | | | | | | | Rework this function to closely match the PPC64 crosscall2, but written in gnu asm. Likewise, fix this to store TOC in the new frame, not the caller's, as is required by the ELF ABIs. Change-Id: I8902c74f2607e3436260882a7bea52e72a67b8f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/486335 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Paul Murphy <murp@ibm.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Auto-Submit: Carlos Amedee <carlos@golang.org>
* runtime/cgo: preserve nonvolatile VRs when crosscalling into Go on PPC64Paul E. Murphy2023-05-041-103/+9
| | | | | | | | | | | | | | ELFv1 and ELFv2 declare V20-V31 as nonvolatile (callee save) registers. Go does not. Preserve them before calling into Go. Change-Id: If60a6d8f71b51b1136a86eab2b90d964900becd7 Reviewed-on: https://go-review.googlesource.com/c/go/+/480657 Auto-Submit: Carlos Amedee <carlos@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Paul Murphy <murp@ibm.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime: skip TestSegv/SegvInCgo earlier on iosBryan C. Mills2023-05-041-0/+4
| | | | | | | | | | | | | | It appears to sometimes hang instead of crashing, which can cause subsequent tests to time out. For #59947. Change-Id: Id4ac3c0cd5f7f345334d3e0ed3f48e40b9ff191c Reviewed-on: https://go-review.googlesource.com/c/go/+/492075 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
* runtime/coverage: fix problematic test from issue 59563Than McIntosh2023-05-031-3/+6
| | | | | | | | | | | | | | | | | | | | Fix up the coverage testpoint TestIssue59563TruncatedCoverPkgAll to avoid spurious failures due to racy behavior. Specifically, we are only interested in verifying coverage for the larger function of the two in the test package (the smaller one is only there to trigger additional function registrations while the test is finalizing the cov data). Updates #59867. Updates #59563. Change-Id: Ibfbbcbf68e0ad7a4d9606cbcfc69d140375c7b87 Reviewed-on: https://go-review.googlesource.com/c/go/+/492175 Run-TryBot: Than McIntosh <thanm@google.com> Auto-Submit: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
* runtime: add test skips for iosBryan C. Mills2023-05-031-1/+7
| | | | | | | | | | | | | For #59912. For #59913. Updates #49182. Change-Id: I3fcdfaca3a4f7120404e7a36b4fb5f0e57dd8114 Reviewed-on: https://go-review.googlesource.com/c/go/+/491095 TryBot-Bypass: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Austin Clements <austin@google.com>
* runtime: remove systemstack logic from adjustframeFelix Geisendörfer2023-05-031-7/+0
| | | | | | | | | | | | | | | | Remove logic for skipping some adjustframe logic for systemstack (aka FuncID_systemstack_switch). This was introduced in 2014 by 9198ed4bd6ec7b7dd37aa2797e96f15ddbb1e6cd but doesn't seem to be needed anymore. Updates #59692 Change-Id: I2368d64f9bb28ced4e7f15c9b15dac7a29194389 Reviewed-on: https://go-review.googlesource.com/c/go/+/489116 Reviewed-by: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
* runtime: add test for systemstack frame pointer adjustmentFelix Geisendörfer2023-05-033-1/+40
| | | | | | | | | | | | | | | | | | | | Add TestSystemstackFramePointerAdjust as a regression test for CL 489015. By turning stackPoisonCopy into a var instead of const and introducing the ShrinkStackAndVerifyFramePointers() helper function, we are able to trigger the exact combination of events that can crash traceEvent() if systemstack restores a frame pointer that is pointing into the old stack. Updates #59692 Change-Id: I60fc6940638077e3b60a81d923b5f5b4f6d8a44c Reviewed-on: https://go-review.googlesource.com/c/go/+/489115 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
* runtime/trace: enable frame pointer unwinding by default for amd64 and arm64Nick Ripley2023-05-022-5/+2
| | | | | | | | | | | | | | | | | | | | | Re-enable frame pointer unwinding for execution tracing on amd64 by default, now that CL 489015 and CL 488755 have fixed recently-discovered crashes. This reverts CL 486382. These fixes, together with CL 241158 to fix up frame pointers when copying stacks on arm64, also make frame pointer unwinding for tracing safe to enable for arm64. This should significantly reduce the CPU and latency overhead of execution tracing on arm64, as it has for amd64. Co-Authored-By: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Change-Id: I64a88bd69dfd8cb13956ec46f8b1203dbeaa26a6 Reviewed-on: https://go-review.googlesource.com/c/go/+/490815 Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Run-TryBot: Nick Ripley <nick.ripley@datadoghq.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
View Lorry Controller Status