Matthew Dempsky [Tue, 5 Dec 2023 20:56:04 +0000 (12:56 -0800)]
cmd/compile: fix escape analysis of string min/max
When I was plumbing min/max support through the compiler, I was
thinking mostly about numeric argument types. As a result, I forgot
that escape analysis would need to be aware that min/max can operate
on string values, which contain pointers.
Fixes #64565.
Change-Id: I36127ce5a2da942401910fa0f9de922726c9f94d
Reviewed-on: https://go-review.googlesource.com/c/go/+/547715 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Alan Donovan [Tue, 5 Dec 2023 20:16:18 +0000 (15:16 -0500)]
doc/go1.22: document ast.Object deprecation
The api.txt changes were originally recorded by mistake (sorry)
into go1.21.txt; see CL 504915, which made the actual change
in August.
Change-Id: If46b48d9714f01605888a6e975c1a03ccfce3b3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/547637
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com>
Alan Donovan [Tue, 5 Dec 2023 19:22:53 +0000 (14:22 -0500)]
doc/go_spec.html: rename golang.org to go.dev
Fixes #64513
Change-Id: I45d6a4ba2170308260f3b8f7965affc9948f3fcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/547476 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Russ Cox [Sun, 6 Aug 2023 03:26:28 +0000 (13:26 +1000)]
math/rand, math/rand/v2: use ChaCha8 for global rand
Move ChaCha8 code into internal/chacha8rand and use it to implement
runtime.rand, which is used for the unseeded global source for
both math/rand and math/rand/v2. This also affects the calculation of
the start point for iteration over very very large maps (when the
32-bit fastrand is not big enough).
The benefit is that misuse of the global random number generators
in math/rand and math/rand/v2 in contexts where non-predictable
randomness is important for security reasons is no longer a
security problem, removing a common mistake among programmers
who are unaware of the different kinds of randomness.
The cost is an extra 304 bytes per thread stored in the m struct
plus 2-3ns more per random uint64 due to the more sophisticated
algorithm. Using PCG looks like it would cost about the same,
although I haven't benchmarked that.
Before this, the math/rand and math/rand/v2 global generator
was wyrand (https://github.com/wangyi-fudan/wyhash).
For math/rand, using wyrand instead of the Mitchell/Reeds/Thompson
ALFG was justifiable, since the latter was not any better.
But for math/rand/v2, the global generator really should be
at least as good as one of the well-studied, specific algorithms
provided directly by the package, and it's not.
(Wyrand is still reasonable for scheduling and cache decisions.)
Good randomness does have a cost: about twice wyrand.
Also rationalize the various runtime rand references.
Russ Cox [Thu, 30 Nov 2023 17:25:43 +0000 (12:25 -0500)]
math/rand/v2: add ChaCha8
This is a replay of CL 516859, after its rollback in CL 543895,
with big-endian systems fixed and the tests disabled on RISC-V
since the compiler is broken there (#64285).
ChaCha8 provides a cryptographically strong generator
alongside PCG, so that people who want stronger randomness
have access to that. On systems with 128-bit vector math
assembly (amd64 and arm64), ChaCha8 runs at about the same
speed as PCG (25% slower on amd64, 2% faster on arm64).
Fixes #64284.
Change-Id: I6290bb8ace28e1aff9a61f805dbe380ccdf25b94
Reviewed-on: https://go-review.googlesource.com/c/go/+/546020 Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Wed, 29 Nov 2023 22:18:32 +0000 (22:18 +0000)]
internal/trace/v2: handle thread ID reuse correctly
To begin with, CL 545515 made the trace parser tolerant of
GoCreateSyscall having a P, but that was wrong. Because dropm trashes
the M's syscalltick, that case should never be possible. So the first
thing this change does is it rewrites the test that CL introduced to
expect a failure instead of a success.
What I'd misinterpreted as a case that should be allowed was actually
the same as the other issues causing #64060, which is that the parser
doesn't correctly implement what happens to Ps when a thread calls back
into Go on non-pthread platforms, and what happens when a thread dies
on pthread platorms (or more succinctly, what the runtime does when it
calls dropm).
Specifically, the GoDestroySyscall event implies that if any P is still
running on that M when it's called, that the P stops running. This is
what is intended by the runtime trashing the M's syscalltick; when it
calls back into Go, the tracer models that thread as obtaining a new P
from scratch.
Handling this incorrectly manifests in one of two ways.
On pthread platforms, GoDestroySyscall is only emitted when a C thread
that previously called into Go is destroyed. However, that thread ID can
be reused. Because we have no thread events, whether it's the same
thread or not is totally ambiguous to the tracer. Therefore, the tracer
may observe a thread that previously died try to start running with a
new P under the same identity. The association to the old P is still
intact because the ID is the same, and the tracer gets confused -- it
appears as if two Ps are running on the same M!
On non-pthread platforms, GoDestroySyscall is emitted on every return to
C from Go code. In this case, the same thread with the same identity is
naturally going to keep calling back into Go. But again, since the
runtime trashes syscalltick in dropm, it's always going to acquire a P
from the tracer's perspective. But if this is a different P than before,
just like the pthread case, the parser is going to get confused, since
it looks like two Ps are running on the same M!
The case that CL 545515 actually handled was actually the non-pthread
case, specifically where the same P is reacquired by an M calling back
into Go. In this case, if we tolerate having a P, then what we'll
observe is the M stealing its own P from itself, then running with it.
Now that we know what the problem is, how do we fix it? This change
addresses the problem by emitting an extra event when encountering a
GoDestroySyscall with an active P in its context. In this case, it emits
an additional ProcSteal event to steal from itself, indicating that the
P stopped running. This removes any association between that M and that
P, resolving any ambiguity in the tracer.
There's one other minor detail that needs to be worked out, and that's
what happens to any *real* ProcSteal event that stole the P we're now
emitting an extra ProcSteal event for. Since, this event is going to
look for an M that may have moved on already and the P at this point is
already idle. Luckily, we have *exactly* the right fix for this. The
handler for GoDestroySyscall now moves any active P it has to the
ProcSyscallAbandoned state, indicating that we've lost information about
the P and that it should be treated as already idle. Conceptually this
all makes sense: this is a P in _Psyscall that has been abandoned by the
M it was previously bound to.
It's unfortunate how complicated this has all ended up being, but we can
take a closer look at that in the future.
Fixes #64060.
Change-Id: Ie9e6eb9cf738607617446e3487392643656069a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/546096 Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 5 Dec 2023 17:31:34 +0000 (17:31 +0000)]
runtime: add the disablethp GODEBUG setting
Go 1.21.1 and Go 1.22 have ceased working around an issue with Linux
kernel defaults for transparent huge pages that can result in excessive
memory overheads. (https://bugzilla.kernel.org/show_bug.cgi?id=93111)
Many Linux distributions disable huge pages altogether these days, so
this problem isn't quite as far-reaching as it used to be. Also, the
problem only affects Go programs with very particular memory usage
patterns.
That being said, because the runtime used to actively deal with this
problem (but with some unpredictable behavior), it's preventing users
that don't have a lot of control over their execution environment from
upgrading to Go beyond Go 1.20.
This change adds a GODEBUG to smooth over the transition. The GODEBUG
setting disables transparent huge pages for all heap memory on Linux,
which is much more predictable than restoring the old behavior.
Fixes #64332.
Change-Id: I73b1894337f0f0b1a5a17b90da1221e118e0b145
Reviewed-on: https://go-review.googlesource.com/c/go/+/547475 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Robert Griesemer [Mon, 4 Dec 2023 18:45:50 +0000 (10:45 -0800)]
doc: add release note for math/big.Rat.FloatPrec
For #50489.
Change-Id: I4544a24327196eb3ed62af64ae5ddb1f60441d12
Reviewed-on: https://go-review.googlesource.com/c/go/+/546357 Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com>
Damien Neil [Tue, 7 Nov 2023 18:47:56 +0000 (10:47 -0800)]
net/http: limit chunked data overhead
The chunked transfer encoding adds some overhead to
the content transferred. When writing one byte per
chunk, for example, there are five bytes of overhead
per byte of data transferred: "1\r\nX\r\n" to send "X".
Chunks may include "chunk extensions",
which we skip over and do not use.
For example: "1;chunk extension here\r\nX\r\n".
A malicious sender can use chunk extensions to add
about 4k of overhead per byte of data.
(The maximum chunk header line size we will accept.)
Track the amount of overhead read in chunked data,
and produce an error if it seems excessive.
Fixes #64433
Fixes CVE-2023-39326
Change-Id: I40f8d70eb6f9575fb43f506eb19132ccedafcf39
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2076135 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/547335 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Keith Randall [Fri, 1 Dec 2023 06:59:04 +0000 (22:59 -0800)]
maps: fix aliasing problems with Clone
Make sure to alloc+copy large keys and values instead of aliasing them,
when they might be updated by a future assignment.
Fixes #64474
Change-Id: Ie2226a81cf3897e4e2ee24472f2966d397ace53f
Reviewed-on: https://go-review.googlesource.com/c/go/+/546515 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Cherry Mui [Sat, 2 Dec 2023 21:37:17 +0000 (16:37 -0500)]
cmd/compile: update PGO profile
Since the default.pgo profile is collected, there has been a lot
of development and refactoring in the compiler. It's time to
refresh the compiler's PGO profile. As we are in the freeze, there
will probably not be many major changes to the compiler, so this
should be good for Go 1.22.
Profile collected by running the cmd/compile/profile.sh script on
the linux-amd64-perf gomote.
Michael Anthony Knyszek [Thu, 30 Nov 2023 19:36:46 +0000 (19:36 +0000)]
doc: add release notes for allocation headers
For #61422.
Change-Id: Ia7cb2721e52d9ccd932a49e0c331868eb7749f94
Reviewed-on: https://go-review.googlesource.com/c/go/+/546027 Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Bryan C. Mills [Thu, 30 Nov 2023 16:00:56 +0000 (11:00 -0500)]
testing: add regression tests for reentrant calls to T.Run
These tests represent two patterns of usage, found in Google-internal
tests, that deadlocked after CL 506755.
TestConcurrentRun is a minor variation on TestParallelSub, with the
additional expectation that the concurrent calls to Run (without
explicit calls to Parallel) proceed without blocking. It replaces
TestParallelSub.
TestParentRun is similar, but instead of calling Run concurrently it
calls Run from within the subtest body. It almost certainly represents
an accidental misuse of T.Run, but since that pattern used to run to
completion we don't want to break it accidentally. (Perhaps it should
be diagnosed with a vet check instead?)
While we are testing concurrency, this also cleans up
TestConcurrentCleanup to use a clearer synchronization pattern.
Fixes #64402.
Change-Id: I14fc7e7085a994c284509eac28190c3a8feb04cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/546019
Auto-Submit: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
This was released in 1.21, release noted in CL 524675.
(I think it was missed originally and then and backported,
but then somehow incorrectly made it into the 1.22 dragnet.)
Change-Id: I45f3182f14f77f8b92a3b7d5ef0011b71fd3c176
Reviewed-on: https://go-review.googlesource.com/c/go/+/546675 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Bypass: Keith Randall <khr@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Cherry Mui [Fri, 1 Dec 2023 19:04:35 +0000 (14:04 -0500)]
cmd/link: update flag doc
Update the go doc for linker flags. Remove flags that no longer
exist. Also remove flags that are intended for debugging the
linker from user docs. Add -aslr to the doc.
The -n flag does nothing except print a nearly useless message on
XCOFF linking. Deprecate it.
Fixes #64476.
Change-Id: I518c9c6cc009eae50b7c11308348524ad6a62b69
Reviewed-on: https://go-review.googlesource.com/c/go/+/546615 Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Joel Sing [Sat, 25 Nov 2023 15:48:05 +0000 (02:48 +1100)]
cmd/compile: correct code generation for right shifts on riscv64
The code generation on riscv64 will currently result in incorrect
assembly when a 32 bit integer is right shifted by an amount that
exceeds the size of the type. In particular, this occurs when an
int32 or uint32 is cast to a 64 bit type and right shifted by a
value larger than 31.
Fix this by moving the SRAW/SRLW conversion into the right shift
rules and removing the SignExt32to64/ZeroExt32to64. Add additional
rules that rewrite to SRAIW/SRLIW when the shift is less than the
size of the type, or replace/eliminate the shift when it exceeds
the size of the type.
Add SSA tests that would have caught this issue. Also add additional
codegen tests to ensure that the resulting assembly is what we
expect in these overflow cases.
Fixes #64285
Change-Id: Ie97b05668597cfcb91413afefaab18ee1aa145ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/545035 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: M Zhuo <mzh@golangcn.org> Reviewed-by: Mark Ryan <markdryan@rivosinc.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Thu, 30 Nov 2023 18:13:51 +0000 (18:13 +0000)]
runtime: emit the correct P status from a safepoint in the v2 tracer
The GoSyscallBegin event is a signal for both the P and the G to enter a
syscall state for the trace parser. (Ps can't have their own event
because it's too hard to model. As soon as the P enters _Psyscall it can
get stolen out of it.) But there's a window in time between when that
event is emitted and when the P enters _Psyscall where the P's status
can get emitted. In this window the tracer will emit the wrong status:
Running instead of Syscall. Really any call into the tracer could emit a
status event for the P, but in this particular case it's when running a
safepoint function that explicitly emits an event for the P's status.
The fix is straightforward. The source-of-truth on syscall status is the
G's status, so the function that emits the P's status just needs to
check the status of any G attached to it. If it's in _Gsyscall, then the
tracer should emit a Syscall status for the P if it's in _Prunning.
Fixes #64318.
Change-Id: I3b0fb0d41ff578e62810b04fa5a3ef73e2929b0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/546025
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Keith Randall [Thu, 30 Nov 2023 18:04:16 +0000 (10:04 -0800)]
cmd/compile: fix memcombine pass for big endian, > 1 byte elements
The shift amounts were wrong in this case, leading to miscompilation
of load combining.
Also the store combining was not triggering when it should.
Fixes #64468
Change-Id: Iaeb08972c5fc1d6f628800334789c6af7216e87b
Reviewed-on: https://go-review.googlesource.com/c/go/+/546355 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Bryan C. Mills [Wed, 29 Nov 2023 21:13:07 +0000 (16:13 -0500)]
testing: remove tests from the running log while they are waiting on parallel subtests
The parallel subtests are themselves removed from the running map
while they are blocked on calls to t.Parallel, so it is misleading to
log their parents as if they are running when we know they cannot be
making any kind of meaningful progress.
Alan Donovan [Mon, 20 Nov 2023 22:22:37 +0000 (17:22 -0500)]
go/types: set correct Var.scopePos for parameters/results
Previously, its value was unset (NoPos), but the correct
value is a point after the signature (FuncType.End) and
before the body.
Also, fix a bug in Scope.Innermost whereby it would return
the wrong (outer) scope when the query position was in
the FuncType portion of a Func{Decl,Lit}.
The fix is to set the scope's pos/end to those of the
complete Func{Decl,Lit}. This is now documented at
Info.Scopes, along with other missing information.
Also, fix a bug in the go/types (but not types2) scope
test, in which comments were discarded by the parser,
causing the entire test to be a no-op (!).
Also, make failures of TestScopeLookupParent more
informative.
Also, add a release note about the change in behavior.
Fixes #64292
Fixes #64295
Change-Id: Ib681f59d1b0b43de977666db08302d7524d3305f
Reviewed-on: https://go-review.googlesource.com/c/go/+/544035 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Cosmos Nicolaou [Wed, 29 Nov 2023 19:04:57 +0000 (11:04 -0800)]
runtime/pprof: retry vmmap invocation if it failed due to a reported temporary resource shortage
As per #62352 the invocation of vmmap may fail (very rarely) due to
a temporary lack of resources on the test runner machine. This PR
allows for retrying the invocation a fixed number of times before
giving up. This is because we suspect the failure is due to
sensible to retry.
Dmitri Shuralyov [Wed, 29 Nov 2023 20:42:53 +0000 (15:42 -0500)]
all: update vendored dependencies
The Go 1.22 code freeze has recently started. This is a time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
Generated with:
go install golang.org/x/build/cmd/updatestd@latest
go install golang.org/x/tools/cmd/bundle@latest
updatestd -goroot=$(pwd) -branch=master
For #36905.
Change-Id: I76525261b9a954ed21a3bd3cb6c4a12e6c031d80
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-linux-amd64-longtest,gotip-linux-386-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/546055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Keith Randall [Tue, 28 Nov 2023 21:09:42 +0000 (13:09 -0800)]
doc: release notes for GOARM hardfloat/softfloat change
See CL 514907
Change-Id: Ieba2d7737115c66990b0ea7629033e787a99be93
Reviewed-on: https://go-review.googlesource.com/c/go/+/545655 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ludi Rehak <ludi317@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Michael Anthony Knyszek [Tue, 28 Nov 2023 17:10:46 +0000 (17:10 +0000)]
internal/trace/v2: tolerate having a P in GoCreateSyscall
On non-pthread platforms, it's totally possible for the same M to
GoCreateSyscall/GoDestroySyscall on the same thread multiple times. That
same thread may hold onto its P through all those calls.
For #64060.
Change-Id: Ib968bfd439ecd5bc24fc98d78c06145b0d4b7802
Reviewed-on: https://go-review.googlesource.com/c/go/+/545515 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Mon, 27 Nov 2023 22:27:32 +0000 (22:27 +0000)]
runtime: put ReadMemStats debug assertions behind a double-check mode
ReadMemStats has a few assertions it makes about the consistency of the
stats it's about to produce. Specifically, how those stats line up with
runtime-internal stats. These checks are generally useful, but crashing
just because some stats are wrong is a heavy price to pay.
For a long time this wasn't a problem, but very recently it became a
real problem. It turns out that there's real benign skew that can happen
wherein sysmon (which doesn't synchronize with a STW) generates a trace
event when tracing is enabled, and may mutate some stats while
ReadMemStats is running its checks.
Fix this by synchronizing with both sysmon and the tracer. This is a bit
heavy-handed, but better that than false positives.
Also, put the checks behind a debug mode. We want to reduce the risk of
backporting this change, and again, it's not great to crash just because
user-facing stats are off. Still, enable this debug mode during the
runtime tests so we don't lose quite as much coverage from disabling
these checks by default.
Fixes #64401.
Change-Id: I9adb3e5c7161d207648d07373a11da8a5f0fda9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/545277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Rhys Hiltner [Tue, 21 Nov 2023 23:31:26 +0000 (15:31 -0800)]
runtime: test for contention in both semaphore paths
Most contention on the runtime locks inside semaphores is observed in
runtime.semrelease1, but it can also appear in runtime.semacquire1. When
examining contention profiles in TestRuntimeLockMetricsAndProfile, allow
call stacks that include either.
For #64253
Change-Id: Id4f16af5e9a28615ab5032a3197e8df90f7e382f
Reviewed-on: https://go-review.googlesource.com/c/go/+/544375 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Rhys Hiltner <rhys@justin.tv>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Keith Randall [Tue, 28 Nov 2023 01:21:08 +0000 (17:21 -0800)]
cmd/compile: use correct type for slice pointer
The type of the data pointer field of a slice should be a pointer
to the element type, not a *uint8.
This ensures that the SSA value representing the slice's data pointer
can be spilled to the stack slot for the corresponding argument.
Before this change the types didn't match so we ended up spilling the
argument to an autotmp instead of to the dedicated argument slot.
Fixes #64414
Change-Id: I09ee39e93f05aee07e3eceb14e39736d7fd70a33
Reviewed-on: https://go-review.googlesource.com/c/go/+/545357
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
Cherry Mui [Mon, 27 Nov 2023 20:17:11 +0000 (15:17 -0500)]
syscall: remove ptrace1 on darwin
On Darwin, the ptrace syscall is called in ptrace1, which then be
called in ptrace. This allows ptrace1 be disabled on iOS (by
implementing ptrace differently). But we can also achieve this by
adding a conditional directly in ptrace. This reduces stack usage
with -N -l, while keeping ptrace disabled on iOS.
For #64113.
Change-Id: I89d8e317e77352fffdbb5a25ba21ee9cdf2e1e20
Reviewed-on: https://go-review.googlesource.com/c/go/+/545276 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Wed, 22 Nov 2023 20:37:13 +0000 (20:37 +0000)]
doc: update release notes from the relnote tool
For #61422.
Change-Id: I0e091c30a445dbc55054c31837c6f051fea3c07d
Reviewed-on: https://go-review.googlesource.com/c/go/+/544537
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sean Liao [Thu, 23 Nov 2023 18:43:12 +0000 (18:43 +0000)]
.github: clean up issue forms
bugs:
* drop unused labels
* drop the reproduce checkbox:
it's not a strong signal and introduces clutter in github as a task list
* link go.dev/play
govuln:
* use correct label
* ask for version of the tool
* link go.dev/play
telemetry:
* align title with purpose
Change-Id: Id7dd876e518c75dc22e9aec43d9af6e18af088fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/544775 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
cui fliter [Fri, 3 Nov 2023 11:18:00 +0000 (19:18 +0800)]
path: add available godoc link
Change-Id: I6d40a59cde4c3f1d5094f5126fdbc1195285195f
Reviewed-on: https://go-review.googlesource.com/c/go/+/539577 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Dmitri Shuralyov [Wed, 22 Nov 2023 19:31:37 +0000 (14:31 -0500)]
.github: try "import/path: issue title" instead of "affected/package: "
The multiple issue templates pre-populate the issue title with a prefix
that Go issues customarily have. The "affected/package" phrase is short
for "the import path of the affected package". Let's try simplifying it
to just "import/path", and also include "issue title" to make the title
a more representative template of what the final title should look like.
Updates #29839.
Change-Id: I9736d24cf3d0a51536ac13dd07dd189fb51da021
Reviewed-on: https://go-review.googlesource.com/c/go/+/544556
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
Michael Anthony Knyszek [Wed, 22 Nov 2023 16:36:23 +0000 (16:36 +0000)]
runtime: throw from the systemstack in wirep
The exitsyscall path, since the introduction of the new execution
tracer, stores a just little bit more data in the exitsyscall stack
frame, causing a build failure from exceeding the nosplit limit with
'-N -l' set on all packages (like Delve does).
One of the paths through which this fails is "throw" from wirep, called
by a callee of exitsyscall. By switching to the systemstack on this
path, we can avoid hitting the nosplit limit, fixing the build. It's
also not totally unreasonable to switch to the systemstack for the
throws in this function, since the function has to be nosplit anyway. It
gives the throw path a bit more wiggle room to dump information than it
otherwise would have.
Fixes #64113.
Change-Id: I56e94e40614a202b8ac2fdc8b8b731493b74e5d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/544535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 23:57:36 +0000 (23:57 +0000)]
runtime: hold sched.lock over traceThreadDestroy in dropm
This is required by traceThreadDestroy, though it's not strictly
necessary in this case. The requirement to hold sched.lock comes from
the assumption that traceThreadDestroy is getting called when the thread
leaves the tracer's view, but in this case the extra m that dropm is
dropping never leaves the allm list. Nevertheless, traceThreadDestroy
requires it just as a safety measure, and that's reasonable. dropm is
generally rare on pthread platforms, so the extra lock acquire over this
short critical section (and only when tracing is enabled) is fine.
Change-Id: Ib631820963c74f2f087d14a0067d0441d75d6785
Reviewed-on: https://go-review.googlesource.com/c/go/+/544396 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Tue, 21 Nov 2023 23:46:48 +0000 (23:46 +0000)]
runtime: move the wakeableSleep lock under sched in the lock rank
Currently the wakeableSleep lock is placed just after timers in the
ranking, but it turns out the timers lock can never be held over a timer
func, so that's wrong. Meanwhile, wakeableSleep can acquire sched.lock.
wakeableSleep, as it turns out, doesn't have any dependencies -- it's
always acquired in a (mostly) regular goroutine context.
Change-Id: Icc8ea76a8b309fbaf0f02215f16e5f706d49cd95
Reviewed-on: https://go-review.googlesource.com/c/go/+/544395
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 23:24:59 +0000 (23:24 +0000)]
runtime: don't hold trace.lock over semrelease in readTrace0
semrelease may unblock a goroutine, but the act of unblocking a
goroutine may emit an event, which in turn may try to acquire trace.lock
again.
It's safe to release trace.lock in readTrace0 for this because all of
the state (one variable) it uses under the lock will be recomputed when
it reacquires the lock. There's also no other synchronization
requirement to hold trace.lock. This is just a mistake.
Change-Id: Iff6c6b02efa298ebed8e60cdf6539ec161d5ec48
Reviewed-on: https://go-review.googlesource.com/c/go/+/544178
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 22:42:35 +0000 (22:42 +0000)]
runtime: don't hold the table lock in (*traceStackTable).dump
There's a conceptual cycle between traceStackTable.lock and
allocation-related locks, but it can't happen in practice because the
caller guarantees that there are no more writers to the table at the
point that dump is called.
But if that's true, then the lock isn't necessary at all. It would be
difficult to model this quiesence in the lockrank mode, so just don't
hold the lock and expand the documentation of the dump method.
Change-Id: Id4db61363f075b7574135529915e8bd4f4f4c082
Reviewed-on: https://go-review.googlesource.com/c/go/+/544177 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Davis Goodin [Wed, 22 Nov 2023 01:12:48 +0000 (17:12 -0800)]
cmd/link/internal/loadpe: fix .xdata unwind info parsing
Unwind info in .xdata was being parsed incorrectly, causing targetOff to
be incorrect and miss finding data in .xdata that it should have found.
This causes a linker issue when using the MinGW MSVCRT compiler.
Contains several fixes based on the exception handling docs: the offset
used to get the number of unwind codes, the calculation of the target
offset based on the dynamic size of the unwind data, and the
UNW_FLAG_CHAININFO flag's value.
Fixes #64200
Change-Id: I6483d921b2bf8a2512a95223bf3c8ce8bc63dc4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/544415
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Than McIntosh [Tue, 21 Nov 2023 19:44:41 +0000 (14:44 -0500)]
cmd/link/internal/ld: fix DWARF type DIE "go kind" bug for synthetic ptr types
The code path in linker DWARF type generation that synthesizes pointer
type DIEs needed for other synthesized types wasn't properly setting
the DW_AT_go_kind attribute for the new pointer types.
Fixes #64231.
Change-Id: I70c338d2b33ae3b93a4c6f201e5836d91d368086
Reviewed-on: https://go-review.googlesource.com/c/go/+/544315 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Mauri de Souza Meneguzzo [Tue, 21 Nov 2023 19:34:36 +0000 (19:34 +0000)]
runtime/internal/atomic: deduplicate And/Or code on wasm
When I initially added the wasm code for these ops I did not saw that
wasm actually has the Cas operations implemented, although they are
merely pointer assignments since wasm is single threaded.
Now with a generic implementation for And/Or we can add wasm to the
build tags.
For #61395
Change-Id: I997dc90477c772882d6703df1b795dfc0d90a699
GitHub-Last-Rev: 92736a6e34104a9d234c791673fe0bb79fc97b0b
GitHub-Pull-Request: golang/go#64300
Reviewed-on: https://go-review.googlesource.com/c/go/+/544116
Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Srinivas Pokala [Mon, 16 Oct 2023 09:18:50 +0000 (11:18 +0200)]
cmd/asm: fix the KMCTR instruction encoding and argument passing
KMCTR encoding arguments incorrect way, which leading illegal instruction wherver we call KMCTR instruction.IBM z13 machine test's TestAESGCM test using gcmASM implementation, which uses KMCTR instruction to encrypt using AES in counter mode and the KIMD instruction for GHASH. z14+ machines onwards uses gcmKMA implementation for the same.
Fixes #63387
Change-Id: I86aeb99573c3f636a71908c99e06a9530655aa5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/535675 Reviewed-by: Vishwanatha HD <vishwanatha.hd@ibm.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org>
fanzha02 [Tue, 21 Nov 2023 06:52:54 +0000 (06:52 +0000)]
runtime: change the name of variables in asan-related assembly functions
Variables in functions implemented in assembly should have the
same names as when they were defined. The names of some variables
in asan-related assembly functions do not follow the above rule,
which will cause the runtime test to fail. This CL fixes this issue.
Change-Id: I1abf6b220b9802028f8ad5eebc8d3b7cfa3e89ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/541756 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Joel Sing <joel@sing.id.au>
Run-TryBot: M Zhuo <mzh@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Wang Yaduo <wangyaduo@linux.alibaba.com> Reviewed-by: Mark Ryan <markdryan@rivosinc.com>
Meng Zhuo [Sun, 12 Nov 2023 09:05:57 +0000 (17:05 +0800)]
cmd/internal/obj/riscv: add support of PCALIGN directive
Add support for PCALIGN directive on riscv.
This directive can be used within Go asm to align instruction
by padding NOP directives.
This patch also adds a test to verify the correctness of the PCALIGN
directive.
Original credit by Cooper Qu (Alibaba)
https://gitee.com/xuantie_riscv/xuantie-patch
Change-Id: I8b6524a2bf81a1baf7c9d04b7da2db6c1a7b428f
Reviewed-on: https://go-review.googlesource.com/c/go/+/541740
Run-TryBot: M Zhuo <mzh@golangcn.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Wang Yaduo <wangyaduo@linux.alibaba.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Mark Ryan <markdryan@rivosinc.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Filippo Valsorda [Tue, 21 Nov 2023 22:16:56 +0000 (23:16 +0100)]
crypto/tls: check and record godebugs more granularly
We should call Value as late as possible to allow programs to set
GODEBUG with os.Setenv, and IncNonDefault only when (and every time) the
GODEBUG has an effect on a connection (that we'd have regularly
rejected).
Change-Id: If7a1446de407db7ca2d904d41dda13558b684dda
Reviewed-on: https://go-review.googlesource.com/c/go/+/544335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Michael Anthony Knyszek [Tue, 21 Nov 2023 16:51:57 +0000 (16:51 +0000)]
internal/trace/v2: dump text trace on failure only if it fits in the log
Currently we dump text traces to the build log on failure
unconditionally, but this may cause the old infrastructure's builds'
logs to get truncated. Avoid that by setting a threshold on the maximum
size of the text trace we're willing to dump.
We don't need this workaround on the new infrastructure -- logs don't
get truncated there.
Change-Id: I0f50f50bb4b90f87250b673fbe56f48235325610
Reviewed-on: https://go-review.googlesource.com/c/go/+/544216
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Bryan C. Mills [Wed, 28 Jun 2023 13:37:47 +0000 (09:37 -0400)]
testing: simplify concurrency and cleanup logic
While investigating #60083, I found a couple of bugs (notably #61034)
that had slipped through code review in part because the concurrency
patterns used in the testing package were too complex for me to fully
reason about. This change adjusts those patterns to be more in line
with current idioms, and to reduce the number of special cases that
depend on details that should be orthogonal. (For example: the details
of how we invoke the Cleanup functions should not depend on whether
the test happened to run any parallel subtests.)
In the process, this change fixes a handful of bugs:
- Concurrent calls to Run (explicitly allowed by TestParallelSub)
could previously drive the testcontext.running count negative,
causing the number of running parallel tests to exceed the -parallel
flag.
- The -failfast flag now takes effect immediately on failure. It no
longer delays until the test finishes, and no longer misses failures
during cleanup (fixing #61034).
- If a Cleanup function calls runtime.Goexit (typically via t.FailNow)
during a panic, Cleanup functions from its parent tests are no
longer skipped and buffered logs from its parent tests are now
flushed.
- The time reported for a test with subtests now includes the time spent
running those subtests, regardless of whether they are parallel.
(Previously, non-parallel subtests were included but parallel subtests
were not.)
- Calls to (*B).Run in iterations after the first are now diagnosed
with a panic. (This diagnoses badly-behaved benchmarks: if Run is
called during the first iteration, no subsequent iterations are
supposed to occur.)
Fixes #61034.
Change-Id: I3797f6ef5210a3d2d5d6c2710d3f35c0219b02ea
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-linux-amd64-longtest-race,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/506755
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The v2 execution tracer has a rudimentary deadlock detector, but it's
based on an arbitrary threshold that an actually get hit even if there's
no deadlock. This ends up breaking tests sometimes, and it would be bad
if this just appeared in production logs.
Put this 'deadlock detector' behind a flag.
For #55317.
Change-Id: I286f0c05b3ac9600f4f2f9696065cac8bbd25f00
Reviewed-on: https://go-review.googlesource.com/c/go/+/544235 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 03:23:05 +0000 (03:23 +0000)]
runtime: emit a ProcSteal from entersyscall_gcwait
Currently entersyscall_gcwait always emits a ProcStop event. Most of the
time, this is correct, since the thread that just put the P into
_Psyscall is the same one that is putting it into _Pgcstop. However it's
possible for another thread to steal the P, start running a goroutine,
and then enter another syscall, putting the P back into _Psyscall. In
this case ProcStop is incorrect; the P is getting stolen. This leads to
broken traces.
Fix this by always emitting a ProcSteal event from entersyscall_gcwait.
This means that most of the time a thread will be 'stealing' the proc
from itself when it enters this function, but that's theoretically fine.
A ProcSteal is really just a fancy ProcStop.
Well, it would be if the parser correctly handled a self-steal. This is
a minor bug that just never came up before, but it's an update order
error (the mState is looked up and modified, but then it's modified
again at the end of the function to match newCtx). There's really no
reason a self-steal shouldn't be allowed, so fix that up and add a test.
Change-Id: Iec3d7639d331e3f2d127f92ce50c2c4a7818fcd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/544215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Kir Kolyshkin [Wed, 13 Sep 2023 07:58:20 +0000 (00:58 -0700)]
internal/syscall/unix: add PidFDSendSignal for Linux
CL 520266 added pidfd_send_signal linux syscall numbers to the
syscall package for the sake of a unit test.
As pidfd_send_signal will be used from the os package, let's revert the
changes to syscall package, add the pidfd_send_signal syscall numbers
and the implementation to internal/syscall/unix, and change the above
test to use it.
Updates #51246.
For #62654.
Change-Id: I862174c3c1a64baf1080792bdb3a1c1d1b417bb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/528436
Run-TryBot: Kirill Kolyshkin <kolyshkin@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Olivier Mengué [Thu, 26 Oct 2023 20:18:33 +0000 (22:18 +0200)]
errors: clarify references to Unwrap in doc
CL 535080 incorrectly links the unclear mention of Unwrap to the func
Unwrap in doc for errors.Is and errors.As
Instead we clarify that "Unwrap" is a reference
to the "Unwrap() error" or "Unwrap() []error" methods, not to the
"Unwrap(error) error" function which is also available in the package.
Change-Id: I8314993932e1e7a2dc77400f74d81f3a8aa891de
Reviewed-on: https://go-review.googlesource.com/c/go/+/538155 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: qiulaidongfeng <2645477756@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 21:19:35 +0000 (21:19 +0000)]
cmd/trace/v2: emit regions in the goroutine-oriented task view
This change emits regions in the goroutine-oriented task view (the
/trace endpoint with the taskid query variable set) in the same way the
old cmd/trace does.
For #60773.
Fixes #63960.
Change-Id: If6c3e7072c694c84a7d2d6c34df668f48d3acc2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/543995 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 19:29:42 +0000 (19:29 +0000)]
cmd/trace/v2: add support for the goroutine-oriented task view
This change adds support for a goroutine-oriented task view via the
/trace?taskid=<taskid> endpoint. This works but it's missing regions.
That will be implemented in a follow-up CL.
For #60773.
For #63960.
Change-Id: I086694143e5c71596ac22fc551416868f0b80923
Reviewed-on: https://go-review.googlesource.com/c/go/+/543937
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 07:28:37 +0000 (07:28 +0000)]
cmd/trace/v2: add thread-oriented mode for v2 traces
This is a nice-to-have that's now straightforward to do with the new
trace format. This change adds a new query variable passed to the
/trace endpoint called "view," which indicates the type of view to
use. It is orthogonal with task-related views.
Unfortunately a goroutine-based view isn't included because it's too
likely to cause the browser tab to crash.
For #60773.
For #63960.
Change-Id: Ifbcb8f2d58ffd425819bdb09c586819cb786478d
Reviewed-on: https://go-review.googlesource.com/c/go/+/543695
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 05:37:36 +0000 (05:37 +0000)]
cmd/trace/v2: add support for a task-oriented procs-based view
This change implements support for the trace?focustask=<taskid> endpoint
in the trace tool for v2 traces.
Note: the one missing feature in v2 vs. v1 is that the "irrelevant" (but
still rendered) events are not grayed out. This basically includes
events that overlapped with events that overlapped with other events
that were in the task time period, but aren't themselves directly
associated. This is probably fine -- the UI already puts a very obvious
focus on the period of time the selected task was running.
For #60773.
For #63960.
Change-Id: I5c78a220ae816e331b74cb67c01c5cd98be40dd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/543596
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Sat, 18 Nov 2023 05:50:50 +0000 (05:50 +0000)]
cmd/trace/v2: add support for goroutine filtering
This change adds support for the trace?goid=<goid> endpoint to the trace
tool for v2 traces.
In effect, this change actually implements a per-goroutine view. I tried
to add a link to the main page to enable a "view by goroutines" view
without filtering, but the web trace viewer broke the browser tab when
there were a few hundred goroutines. The risk of a browser hang probably
isn't worth the cases where this is nice, especially since filtering by
goroutine already works. Unfortunate, but c'est l'vie. Might be worth
revisiting if we change out the web viewer in the future.
For #60773.
For #63960.
Change-Id: I8e29f4ab8346af6708fd8824505c30f2c43db796
Reviewed-on: https://go-review.googlesource.com/c/go/+/543595
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 14 Nov 2023 06:19:07 +0000 (06:19 +0000)]
cmd/trace: factor out durationHistogram
This code will be useful for the new tracer, and there's no need to
duplicate it. This change copies it to internal/trace/traceviewer, adds
some comments, and renames it to TimeHistogram.
While we're here, let's get rid of the unused String method which has a
comment talking about how awful the rendering is.
Also, let's get rid of uses of niceDuration. We'd have to bring it
with us in the move and I don't think it's worth it. The difference
between the default time.Duration rendering and the niceDuration
rendering is usually a few extra digits of precision. Yes, it's noisier,
but AFAICT it's not substantially worse. It doesn't seem worth the new
API, even if it's just internal. We can also always bring it back later.
For #60773.
For #63960.
Change-Id: I795f58f579f1d503c540c3a40bed12e52bce38e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/542001
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Mon, 13 Nov 2023 21:06:40 +0000 (21:06 +0000)]
internal/trace: add task analysis for v2 traces
For v1 traces, cmd/trace contains code for analyzing tasks separately
from the goroutine analysis code present in internal/trace. As I started
to look into porting that code to v2 traces, I noticed that it wouldn't
be too hard to just generalize the existing v2 goroutine summary code to
generate exactly the same information.
This change does exactly that.
For #60773.
For #63960.
Change-Id: I0cdd9bf9ba11fb292a9ffc37dbf18c2a6a2483b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/542076
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Fri, 17 Nov 2023 17:56:27 +0000 (17:56 +0000)]
internal/trace/v2: redefine NoTask and add BackgroundTask
The v2 trace parser currently handles task inheritance and region task
association incorrectly. It assumes that a TaskID of 0 means that there
is no task. However, this is only true for task events. A TaskID of 0
means that a region gets assigned to the "background task." The parser
currently has no concept of a "background task."
Fix this by defining the background task as task ID 0 and redefining
NoTask to ^uint64(0). This aligns the TaskID values more closely with
other IDs in the parser and also enables disambiguating these two cases.
For #60773.
For #63960.
Change-Id: I09c8217b33b87c8f8f8ea3b0203ed83fd3b61e11
Reviewed-on: https://go-review.googlesource.com/c/go/+/543019 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Mon, 13 Nov 2023 18:57:06 +0000 (18:57 +0000)]
cmd/trace/v2: add support for pprof endpoints
This change adds support for the pprof endpoints to cmd/trace/v2.
In the process, I realized we need to pass the goroutine summaries to
more places, and previous CLs had already done the goroutine analysis
during cmd/trace startup. This change thus refactors the goroutine
analysis API once again to operate in a streaming manner, and to run
at the same time as the initial trace parsing. Now we can include it in
the parsedTrace type and pass that around as the de-facto global trace
context.
Note: for simplicity, this change redefines "syscall" profiles to
capture *all* syscalls, not just syscalls that block. IIUC, this choice
was partly the result of a limitation in the previous trace format that
syscalls don't all have complete durations and many short syscalls are
treated as instant. To this end, this change modifies the text on the
main trace webpage to reflect this change.
For #60773.
For #63960.
Change-Id: I601d9250ab0849a0bfaef233fd9b1e81aca9a22a
Reviewed-on: https://go-review.googlesource.com/c/go/+/541999
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Sun, 12 Nov 2023 23:27:31 +0000 (23:27 +0000)]
internal/trace/traceviewer: make the mmu handler more self-contained
The last change made the MMU rendering code common and introduced a new
API, but it was kind of messy. Part of the problem was that some of the
Javascript in the template for the main page referred to specific
endpoints on the server.
Fix this by having the Javascript access the same endpoint but with a
different query variable. Now the Javascript code doesn't depend on
specific endpoints, just on query variables for the current endpoint.
For #60773.
For #63960.
Change-Id: I1c559d9859c3a0d62e2094c9d4ab117890b63b31
Reviewed-on: https://go-review.googlesource.com/c/go/+/541259
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Sun, 12 Nov 2023 23:10:04 +0000 (23:10 +0000)]
cmd/trace: common up the mmu page and add it to cmd/trace/v2
This change moves the MMU HTTP handlers and functionality into the
traceviewer package, since unlike the goroutine pages the vast majority
of that functionality is identical between v1 and v2. This change
involves some refactoring so that callers can plug in their own mutator
utilization computation functions (which is the only point of difference
between v1 and v2). The new interface isn't especially nice, but part of
the problem is the MMU handlers depend on specific endpoints to exist. A
follow-up CL will clean this up a bit.
Like the previous CL did for goroutine analysis, modify the v2 mutator
utilization API to accept a slice of trace events. Again, we might as
well reuse what was already parsed and will be needed for other
purposes. It also simplifies the API slightly.
For #60773.
For #63960.
Change-Id: I6c21ec8d1bf7e95eff5363d0e0005c9217fa00e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/541258
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Sun, 12 Nov 2023 21:48:31 +0000 (21:48 +0000)]
cmd/trace/v2: add goroutine analysis pages
This is a complete fork and most of a rewrite of the goroutine analysis
pages for v2 traces. It fixes an issue with the old page where GC time
didn't really make any sense, generalizes the page and breaks things
down further, and adds clarifying text.
This change also modifies the SummarizeGoroutines API to not stream the
trace. This is unfortunate, but we're already reading and holding the
entire trace in memory for the trace viewer. We can revisit this
decision in the future. Also, we want to do this now because the
GoroutineSummary holds on to pointers to events, and these events will
be used by the user region and user task analyses. While tracev2 events
are values and they should be equivalent no matter how many times we
parse a trace, this lets us reference the event in the slice directly.
For #60773.
For #63960.
Fixes #62443.
Change-Id: I1c5ab68141869378843f4f2826686038e4533090
Reviewed-on: https://go-review.googlesource.com/c/go/+/541257 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>