| Age | Commit message (Collapse) | Author |
|---|
|
On LR machine, consider F calling G calling H, which grows stack.
The stack looks like
...
G's frame:
... locals ...
saved LR = return PC in F <- SP points here at morestack
H's frame (to be created)
At morestack, we save
gp.sched.pc = H's morestack call
gp.sched.sp = H's entry SP (the arrow above)
gp.sched.lr = return PC in G
Currently, when unwinding through morestack (if _TraceJumpStack
is set), we switch PC and SP but not LR. We then have
frame.pc = H's morestack call
frame.sp = H's entry SP (the arrow above)
As LR is not set, we load it from stack at *sp, so
frame.lr = return PC in F
As the SP hasn't decremented at the morestack call,
frame.fp = frame.sp = H's entry SP
Unwinding a frame, we have
frame.pc = old frame.lr = return PC in F
frame.sp = old frame.fp = H's entry SP a.k.a. G's SP
The PC and SP don't match. The unwinding will go off if F and G
have different frame sizes.
Fix this by preserving the LR when switching stack.
Also add code to detect infinite loop in unwinding.
TODO: add some test. I can reproduce the infinite loop (or throw
with added check) but the frequency is low.
Fixes #53111.
Updates #52116.
Change-Id: I6e1294f1c6e55f664c962767a1cf6c466a0c0eff
Reviewed-on: https://go-review.googlesource.com/c/go/+/400575
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Eric Fang <eric.fang@arm.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
(cherry picked from commit 74f00094220f26c80fbaab6eca28c3a664897d24)
Reviewed-on: https://go-review.googlesource.com/c/go/+/408822
Reviewed-by: Austin Clements <austin@google.com>
|
|
skipPleaseUseCallersFrames was removed in CL 152537.
Change-Id: Ide47feec85a33a6fb6882e16baf9e21492521640
Reviewed-on: https://go-review.googlesource.com/c/go/+/325949
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
It is valid to see SPWRITE functions at the top of a GC stack traceback,
in the case where they self-preempted during the stack growth check
and haven't actually modified SP in a traceback-unfriendly manner yet.
The current check is therefore too aggressive.
isAsyncSafePoint is taking care of not async-preempting SPWRITE functions
because it doesn't async-preempt any assembly functions at all.
But perhaps it will in the future.
To keep a check that SPWRITE assembly functions are not async-preempted,
add one in preemptPark. Then relax the check in traceback to avoid
triggering on self-preempted SPWRITE functions.
The long and short of this is that the assembly we corrected in x/crypto
issue #44269 was incredibly dodgy but not technically incompatible with
the Go runtime. After this change, the original x/crypto assembly no longer
causes GC traceback crashes during "GOGC=1 go test -count=1000".
But we'll still leave the corrected assembly.
This also means that we don't need to worry about diagnosing SPWRITE
assembly functions that may exist in the wild. They will be skipped for
async preemption and no harm no foul.
Fixes #44269, which was open pending some kind of check for
bad SPWRITE functions in the wild. (No longer needed.)
Change-Id: I6000197b62812bbd2cd92da28eab422634cf75a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/317669
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Currently, when the runtime printing a stack track (at panic, or
when runtime.Stack is called), it prints the function arguments
as words in memory. With a register-based calling convention,
the layout of argument area of the memory changes, so the
printing also needs to change. In particular, the memory order
and the syntax order of the arguments may differ. To address
that, this CL lets the compiler to emit some metadata about the
memory layout of the arguments, and the runtime will use this
information to print arguments in syntax order.
Previously we print the memory contents of the results along with
the arguments. The results are likely uninitialized when the
traceback is taken, so that information is rarely useful. Also,
with a register-based calling convention the results may not
have corresponding locations in memory. This CL changes it to not
print results.
Previously the runtime simply prints the memory contents as
pointer-sized words. With a register-based calling convention,
as the layout changes, arguments that were packed in one word
may no longer be in one word. Also, as the spill slots are not
always initialized, it is possible that some part of a word
contains useful informationwhile the rest contains garbage.
Instead of letting the runtime recreating the ABI0 layout and
print them as words, we now print each component separately.
Aggregate-typed argument/component is surrounded by "{}".
For example, for a function
F(int, [3]byte, byte) int
when called as F(1, [3]byte{2, 3, 4}, 5), it used to print
F(0x1, 0x5040302, 0xXXXXXXXX) // assuming little endian, 0xXXXXXXXX is uninitilized result
Now prints
F(0x1, {0x2, 0x3, 0x4}, 0x5).
Note: the liveness tracking of the spill splots has not been
implemented in this CL. Currently the runtime just assumes all
the slots are live and print them all.
Increase binary sizes by ~1.5%.
old new
hello (println) 1171328 1187712 (+1.4%)
hello (fmt) 1877024 1901600 (+1.3%)
cmd/compile 22326928 22662800 (+1.5%)
cmd/go 13505024 13726208 (+1.6%)
Updates #40724.
Change-Id: I351e0bf497f99bdbb3f91df2fb17e3c2c5c316dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/304470
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
This change finishes off functionality register ABI for the reflect
package.
Specifically, it implements a call on a MakeFunc'd value by performing
the reverse process that reflect.Value.Call does, using the same ABI
steps. It implements a call on a method value created by reflect by
translating between the method value's ABI to the method's ABI.
Tests are added for both cases.
For #40724.
Change-Id: I302820b61fc0a8f94c5525a002bc02776aef41af
Reviewed-on: https://go-review.googlesource.com/c/go/+/298670
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
Correctly accessing allgs is a bit hairy. Some paths need to lock
allglock, some don't. Those that don't are safest using atomicAllG, but
usage is not consistent.
Rather than doing this ad-hoc, move all access* through forEachG /
forEachGRace, the locking and atomic versions, respectively. This will
make it easier to ensure safe access.
* markroot is the only exception, as it has a far-removed guarantee of
safe access via an atomic load of allglen far before actual use.
Change-Id: Ie1c7a8243e155ae2b4bc3143577380c695680e89
Reviewed-on: https://go-review.googlesource.com/c/go/+/279994
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
netbsd/amd64's Syscall9 changes SP using ADD and SUB,
which are treated as SPWRITEs (they are not accounted for
in the sp-adjust tracking, and there are too many functions that
would report mismatched stack adjustments at RET if they were).
A traceback starting in Syscall9 as saved by entersyscall complains
about the SPWRITE-ness unnecessarily, since the PC/SP are saved
at the start of the function. Ignore SPWRITE in that case.
netbsd/arm's Syscall6 also changes SP (R13), using a direct write.
So even if we could handle the ADD/SUB in the amd64 case or
rewrote that assembly, we'd still be stuck with a more difficult
problem in this case. Ignoring the SPWRITE fixes it.
Example crashes:
https://build.golang.org/log/160fc7b051a2cf90782b75a99984fff129329e66
https://build.golang.org/log/7879e2fecdb400eee616294285e1f952e5b17301
Change-Id: I0c8e9696066e90dafed6d4a93d11697da23f0080
Reviewed-on: https://go-review.googlesource.com/c/go/+/294072
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
|
|
Frame pointers were already enabled on linux, darwin, ios,
but not freebsd, android, openbsd, netbsd.
But the space was reserved on all platforms, leading to
two different arm64 framepointer conditions in different
parts of the code, one of which had no name
(framepointer_enabled || GOARCH == "arm64",
which might have been "framepointer_space_reserved").
So on the disabled systems, the stack layouts were still
set up for frame pointers and the only difference was not
actually maintaining the FP register in the generated code.
Reduce complexity by just enabling the frame pointer
completely on all the arm64 systems.
This commit passes on freebsd, android, netbsd.
I have not been able to try it on openbsd.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I83bd23369d24b76db4c6a648fa74f6917819a093
Reviewed-on: https://go-review.googlesource.com/c/go/+/288814
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
This was added in 2018 to fix a runtime crash during unwind
during a unhandled-panic-induced crash.
(See https://golang.org/cl/90895 and #23576.)
Clearly we cannot unwind past this function, and the change
did stop the unwind. But it's not a top-of-stack function, and
the real issue is that SP is changed.
The new SPWRITE bit takes care of this instead, so we can drop
it from the topofstack function.
At this point the topofstack function is only checking the
TOPFRAME bit, so we can inline that into the one call site.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I856552298032770e48e06c95a20823a1dbd5e38c
Reviewed-on: https://go-review.googlesource.com/c/go/+/288805
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
I added morestack to this list in 2013 with an explanation
that they were needed if we “start a garbage collection on g0
during a stack split or unsplit”.
(https://golang.org/cl/11533043)
This explanation no longer applies for a handful of reasons,
most importantly that if we did stop a stack scan in the middle
of a call to morestack, we'd ignore pointers above the split,
which would lead to memory corruption. But we don't scan
goroutine stacks during morestack now, so that can't happen.
If we did see morestack during a GC, that would be a good time
to crash the program.
The real problem with morestack is during profiling, as noted
in the code review conversation during 2013. And in profiling
we just need to know to stop and not unwind further, which
the new SPWRITE bit will do for us.
So remove from topofstack and let the program crash if GC
sees morestack and otherwise let the SPWRITE stop morestack
unwinding during profiling.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I06d95920b18c599c7c46f64c21028104978215d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/288804
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
I added mcall to this list in 2013 without explaining why.
(https://codereview.appspot.com/11085043/diff/61001/src/pkg/runtime/traceback_x86.c)
I suspect I was stopping crashes during profiling where the unwind
tried to walk up past mcall and got confused.
mcall is not something you can unwind past, because it switches
stacks, but it's also not something you should expect as a
standard top-of-stack frame. So if you do see it during say
a garbage collection stack walk, it would be important to crash
instead of silently stopping the walk prematurely.
This CL removes it from the topofstack list to avoid the silent stop.
Now that mcall is detected as SPWRITE, that will stop the
unwind (with a crash if encountered during GC, which we want).
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I666487ce24efd72292f2bc3eac7fe0477e16bddd
Reviewed-on: https://go-review.googlesource.com/c/go/+/288803
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
No change to actual runtime, but helps reduce the laundry list
of functions.
mcall, morestack, and asmcgocall are not actually top-of-frame,
so those need more attention in follow-up CLs.
mstart moved to assembly so that it can be marked TOPFRAME.
Since TOPFRAME also tells DWARF consumers not to unwind
this way, this change should also improve debuggers a
marginal amount.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: If1e0d46ca973de5e46b62948d076f675f285b5d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/288802
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
The old code was very clever about predicting whether a traceback was safe.
That cleverness has not aged well. In particular, the setsSP function is missing
a bunch of functions that write to SP and will confuse traceback.
And one such function - jmpdefer - was handled as a special case in
gentraceback instead of simply listing it in setsSP.
Throw away all the clever prediction about whether traceback will crash.
Instead, make traceback NOT crash, by checking whether the function
being walked writes to SP.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I3d55fe257a22745e4919ac4dc9a9378c984ba0da
Reviewed-on: https://go-review.googlesource.com/c/go/+/288801
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
Document what the values in internal/sys mean.
Remove various special cases for arm64 in the code using StackAlign.
Delete Uintreg - it was for GOARCH=amd64p32,
which was specific to GOOS=nacl and has been retired.
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: I40e8fa07b4e192298b6536b98a72a751951a4383
Reviewed-on: https://go-review.googlesource.com/c/go/+/288795
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
tracebackothers is called from fatal throw/panic.
A fatal throw may be taken with allglock held (notably in the allocator
when allglock is held), which would cause a deadlock in tracebackothers
when we try to take allglock again. Locking allglock here is also often
a lock order violation w.r.t. the locks held when throw was called.
Avoid the deadlock and ordering issues by skipping locking altogether.
It is OK to miss concurrently created Gs (which are generally avoided by
freezetheworld(), and which were possible previously anyways if created
after the loop).
Fatal throw/panic freezetheworld(), which should freeze other threads
that may be racing to modify allgs. However, freezetheworld() does _not_
guarantee that it stops all other threads, so we can't simply drop the
lock.
Fixes #42669
Updates #43175
Change-Id: I657aec46ed35fd5d1b3f1ba25b500128ab26b088
Reviewed-on: https://go-review.googlesource.com/c/go/+/270861
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Trust: Michael Pratt <mpratt@google.com>
|
|
Currently, gentraceback decides which frames to print or elide when
unwinding inlined frames using only the name of the outermost
function. If the outermost function should be elided, then inlined
functions will also be elided, even if they shouldn't be.
This happens in practice in at least one situation. As of CL 258938,
exported Go functions (and functions they call) can now be inlined
into the generated _cgoexp_HASH_FN function. The runtime elides
_cgoexp_HASH_FN from tracebacks because it doesn't contain a ".".
Because of this bug, it also elides anything that was inlined into it.
This CL fixes this by synthesizing a funcInfo for the inlined
functions to pass to showframe.
Fixes #42754.
Change-Id: Ie6c663a4a1ac7f0d4beb1aa60bc26fc8cddd0f9d
Reviewed-on: https://go-review.googlesource.com/c/go/+/272131
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
bottom 50"
This reverts commit 3a81338622eb5c8b94f11001855e2a68a9e36bed.
Reason for revert: Some edge cases not properly covered due to changes within runtime traceback generation since 2017, that need to be examined. This change landed very late in the Go1.16 cycle.
Change-Id: I8cf6f46ea0ef6161d878e79943e6c7cdac94bccf
Reviewed-on: https://go-review.googlesource.com/c/go/+/268577
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
This CL makes it so that instead of printing massive stack traces during
endless recursion, which spams users and aren't useful, it now prints out
the top and bottom 50 frames. If the number of frames <= 100
(_TracebackMaxFrames), we'll just print all the frames out.
Modified gentraceback to return counts of:
* ntotalframes
* nregularframes
which allows us to get accurate counts of the various kinds of frames.
While here, also fixed a bug that resulted from CL 37222, in which we
no longer accounted for decrementing requested frame skips, and assumed
that when printing, that skip would always be 0. The fix is instead to add
precondition that we'll only print if skip <= 0, but also decrement skip
as we iterate.
Fixes #7181.
Fixes #24628.
Change-Id: Ie31ec6413fdfbe43827b254fef7d99ea26a5277f
Reviewed-on: https://go-review.googlesource.com/c/go/+/37222
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
|
|
This redesigns the way calls work from C to exported Go functions. It
removes several steps from the call path, makes cmd/cgo no longer
sensitive to the Go calling convention, and eliminates the use of
reflectcall from cgo.
In order to avoid generating a large amount of FFI glue between the C
and Go ABIs, the cgo tool has long depended on generating a C function
that marshals the arguments into a struct, and then the actual ABI
switch happens in functions with fixed signatures that simply take a
pointer to this struct. In a way, this CL simply pushes this idea
further.
Currently, the cgo tool generates this argument struct in the exact
layout of the Go stack frame and depends on reflectcall to unpack it
into the appropriate Go call (even though it's actually
reflectcall'ing a function generated by cgo).
In this CL, we decouple this struct from the Go stack layout. Instead,
cgo generates a Go function that takes the struct, unpacks it, and
calls the exported function. Since this generated function has a
generic signature (like the rest of the call path), we don't need
reflectcall and can instead depend on the Go compiler itself to
implement the call to the exported Go function.
One complication is that syscall.NewCallback on Windows, which
converts a Go function into a C function pointer, depends on
cgocallback's current dynamic calling approach since the signatures of
the callbacks aren't known statically. For this specific case, we
continue to depend on reflectcall. Really, the current approach makes
some overly simplistic assumptions about translating the C ABI to the
Go ABI. Now we're at least in a much better position to do a proper
ABI translation.
For comparison, the current cgo call path looks like:
GoF (generated C function) ->
crosscall2 (in cgo/asm_*.s) ->
_cgoexp_GoF (generated Go function) ->
cgocallback (in asm_*.s) ->
cgocallback_gofunc (in asm_*.s) ->
cgocallbackg (in cgocall.go) ->
cgocallbackg1 (in cgocall.go) ->
reflectcall (in asm_*.s) ->
_cgoexpwrap_GoF (generated Go function) ->
p.GoF
Now the call path looks like:
GoF (generated C function) ->
crosscall2 (in cgo/asm_*.s) ->
cgocallback (in asm_*.s) ->
cgocallbackg (in cgocall.go) ->
cgocallbackg1 (in cgocall.go) ->
_cgoexp_GoF (generated Go function) ->
p.GoF
Notably:
1. We combine _cgoexp_GoF and _cgoexpwrap_GoF and move the combined
operation to the end of the sequence. This combined function also
handles reflectcall's previous role.
2. We combined cgocallback and cgocallback_gofunc since the only
purpose of having both was to convert a raw PC into a Go function
value. We instead construct the Go function value in cgocallbackg1.
3. cgocallbackg1 no longer reaches backwards through the stack to get
the arguments to cgocallback_gofunc. Instead, we just pass the
arguments down.
4. Currently, we need an explicit msanwrite to mark the results struct
as written because reflectcall doesn't do this. Now, the results are
written by regular Go assignments, so the Go compiler generates the
necessary MSAN annotations. This also means we no longer need to track
the size of the arguments frame.
Updates #40724, since now we don't need to teach cgo about the
register ABI or change how it uses reflectcall.
Change-Id: I7840489a2597962aeb670e0c1798a16a7359c94f
Reviewed-on: https://go-review.googlesource.com/c/go/+/258938
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
I think they are no longer experimental status. Might as well promote
them to permanent.
Change-Id: Id1259601b3dd2061dd60df86ee48080bfb575d2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/249857
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
|
|
CL [152537](https://go-review.googlesource.com/c/go/+/152537/) changed the way inlined frames are represented in tracebacks to no longer use skipPC
Change-Id: I42386fdcc5cf72f3c122e789b6af9cbd0c6bed4b
GitHub-Last-Rev: 79c26dcd532907eda4ffc30951845c1c01243501
GitHub-Pull-Request: golang/go#39829
Reviewed-on: https://go-review.googlesource.com/c/go/+/239701
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
The runtime has its own implementation of string indexing. To reduce
code duplication and cognitive load, replace this with calls to the
internal/bytealg package. We can't do this on Plan 9 because it needs
string indexing in a note handler (which isn't allowed to use the
optimized bytealg version because it uses SSE), so we can't just
eliminate the index function, but this CL does down-scope it so make
it clear it's only for note handlers on Plan 9.
Change-Id: Ie1a142678262048515c481e8c26313b80c5875df
Reviewed-on: https://go-review.googlesource.com/c/go/+/244537
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Unlike function calls, when processing instructions that directly
fault we must not subtract 1 from the pc before looking up the
file/line information.
Since the file/line lookup unconditionally subtracts 1, add 1 to
the faulting instruction PCs to compensate.
Fixes #34123
Change-Id: Ie7361e3d2f84a0d4f48d97e5a9e74f6291ba7a8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/196962
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
|
|
This CL adds support of call injection and async preemption on
ARM.
Injected call, like sigpanic, has special frame layout. Teach
traceback to handle it.
Change-Id: I887e90134fbf8a676b73c26321c50b3c4762dba4
Reviewed-on: https://go-review.googlesource.com/c/go/+/202338
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
|
|
Currently, the process of suspending a goroutine is tied to stack
scanning. In preparation for non-cooperative preemption, this CL
abstracts this into general purpose suspendG/resumeG functions.
suspendG and resumeG closely follow the existing scang and restartg
functions with one exception: the addition of a _Gpreempted status.
Currently, preemption tasks (stack scanning) are carried out by the
target goroutine if it's in _Grunning. In this new approach, the task
is always carried out by the goroutine that called suspendG. Thus, we
need a reliable way to drive the target goroutine out of _Grunning
until the requesting goroutine is ready to resume it. The new
_Gpreempted state provides the handshake: when a runnable goroutine
responds to a preemption request, it now parks itself and enters
_Gpreempted. The requesting goroutine races to put it in _Gwaiting,
which gives it ownership, but also the responsibility to start it
again.
This CL adds several TODOs about improving the synchronization on the
G status. The existing code already has these problems; we're just
taking note of them.
The next CL will remove the now-dead scang and preemptscan.
For #10958, #24543.
Change-Id: I16dbf87bea9d50399cc86719c156f48e67198f16
Reviewed-on: https://go-review.googlesource.com/c/go/+/201137
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
An extra goroutine is necessary to handle asynchronous events on wasm.
However, we do not want this goroutine to exist all the time.
This change makes it short-lived, so it ends after the asynchronous
event was handled.
Fixes #34768
Change-Id: I24626ff0af9d803a01ebe33fbb584d04d2059a44
Reviewed-on: https://go-review.googlesource.com/c/go/+/200497
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
Part 1: CL 199499 (GOOS nacl)
Part 2: CL 200077 (amd64p32 files, toolchain)
Part 3: stuff that arguably should've been part of Part 2, but I forgot
one of my grep patterns when splitting the original CL up into
two parts.
This one might also have interesting stuff to resurrect for any future
x32 ABI support.
Updates #30439
Change-Id: I2b4143374a253a003666f3c69e776b7e456bdb9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/200318
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
On wasm there is a special goroutine that handles asynchronous events.
Blocking this goroutine often causes a deadlock. However, the stack
trace of this goroutine was omitted when printing the deadlock error.
This change adds an exception so the goroutine is not considered as
an internal system goroutine and the stack trace gets printed, which
helps with debugging the deadlock.
Updates #32764
Change-Id: Icc8f5ba3ca5a485d557b7bdd76bf2f1ffb92eb3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/199537
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
This reverts CL 180761
Reason for revert: Reinstate the stack-allocated defer CL.
There was nothing wrong with the CL proper, but stack allocation of defers exposed two other issues.
Issue #32477: Fix has been submitted as CL 181258.
Issue #32498: Possible fix is CL 181377 (not submitted yet).
Change-Id: I32b3365d5026600069291b068bbba6cb15295eb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/181378
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
This reverts commit fff4f599fe1c21e411a99de5c9b3777d06ce0ce6.
Reason for revert: Seems to still have issues around GC.
Fixes #32452
Change-Id: Ibe7af629f9ad6a3d5312acd7b066123f484da7f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/180761
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
|
|
When a defer is executed at most once in a function body,
we can allocate the defer record for it on the stack instead
of on the heap.
This should make defers like this (which are very common) faster.
This optimization applies to 363 out of the 370 static defer sites
in the cmd/go binary.
name old time/op new time/op delta
Defer-4 52.2ns ± 5% 36.2ns ± 3% -30.70% (p=0.000 n=10+10)
Fixes #6980
Update #14939
Change-Id: I697109dd7aeef9e97a9eeba2ef65ff53d3ee1004
Reviewed-on: https://go-review.googlesource.com/c/go/+/171758
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
Normally, reflect.makeFuncStub records the context value at a known
point in the stack frame, so that the runtime can get the argument map
for reflect.makeFuncStub from that known location.
This doesn't work for defers or goroutines that haven't started yet,
because they haven't allocated a frame or run an instruction yet. The
argument map must be extracted from the context value. We already do
this for defers (the non-nil ctxt arg to getArgInfo), we just need to
do it for unstarted goroutines as well.
When we traceback a goroutine, remember the context value from
g.sched. Use it for the first frame we find.
(We never need it for deeper frames, because we normally don't stop at
the start of reflect.makeFuncStub, as it is nosplit. With this CL we
could allow makeFuncStub to no longer be nosplit.)
Fixes #25897
Change-Id: I427abf332a741a80728cdc0b8412aa8f37e7c418
Reviewed-on: https://go-review.googlesource.com/c/go/+/180258
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
If a for loop has a simple condition and begins with a simple
"if x { break; }"; we can simply add "!x" to the loop's condition.
While at it, simplify a few assignments to use the common pattern
"x := staticDefault; if cond { x = otherValue(); }".
Finally, simplify a couple of var declarations.
Change-Id: I413982c6abd32905adc85a9a666cb3819139c19f
Reviewed-on: https://go-review.googlesource.com/c/go/+/165342
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
In 1.11 we stored "return addresses" in the result of runtime.Callers.
I changed that behavior in CL 152537 to store an address in the call
instruction itself. This CL reverts that part of 152537.
The change in 152537 was made because we now store pcs of inline marks
in the result of runtime.Callers as well. This CL will now store the
address of the inline mark + 1 in the results of runtime.Callers, so
that the subsequent -1 done in CallersFrames will pick out the correct
inline mark instruction.
This CL means that the results of runtime.Callers can be passed to
runtime.FuncForPC as they were before. There are a bunch of packages
in the wild that take the results of runtime.Callers, subtract 1, and
then call FuncForPC. This CL keeps that pattern working as it did in
1.11.
The changes to runtime/pprof in this CL are exactly a revert of the
changes to that package in 152537 (except the locForPC comment).
Update #29582
Change-Id: I04d232000fb482f0f0ff6277f8d7b9c72e97eb48
Reviewed-on: https://go-review.googlesource.com/c/156657
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
As a followon to CL 152537, modify the panic-printing traceback
to also handle mid-stack inlining correctly.
Also declare -fm functions (aka method functions) as wrappers, so that
they get elided during traceback. This fixes part 2 of #26839.
Fixes #28640
Fixes #24488
Update #26839
Change-Id: I1c535a9b87a9a1ea699621be1e6526877b696c21
Reviewed-on: https://go-review.googlesource.com/c/153477
Reviewed-by: David Chase <drchase@google.com>
|
|
Work involved in getting a stack trace is divided between
runtime.Callers and runtime.CallersFrames.
Before this CL, runtime.Callers returns a pc per runtime frame.
runtime.CallersFrames is responsible for expanding a runtime frame
into potentially multiple user frames.
After this CL, runtime.Callers returns a pc per user frame.
runtime.CallersFrames just maps those to user frame info.
Entries in the result of runtime.Callers are now pcs
of the calls (or of the inline marks), not of the instruction
just after the call.
Fixes #29007
Fixes #28640
Update #26320
Change-Id: I1c9567596ff73dc73271311005097a9188c3406f
Reviewed-on: https://go-review.googlesource.com/c/152537
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
|
|
Follow-up for CL 147037 and after Brad noticed the "returns whether"
pattern during the review of CL 150621.
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns whether")
Created with:
$ perl -i -npe 's/returns whether/reports whether/' $(git grep -l "returns whether" | grep -v vendor)
Change-Id: I15fe9ff99180ad97750cd05a10eceafdb12dc0b4
Reviewed-on: https://go-review.googlesource.com/c/150918
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
This issue was found by the new vet's nilness check. _defer was already
checked against nil, so don't check it again.
Change-Id: I78725eaec7234b262b3c941e06441ca57f82bdd9
Reviewed-on: https://go-review.googlesource.com/c/148917
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
When a function triggers a signal (like a segfault which translates to
a nil pointer exception) during execution, a sigpanic handler is just
below it on the stack. The function itself did not stop at a
safepoint, so we have to figure out what safepoint we should use to
scan its stack frame.
Previously we used the site of the most recent defer to get the live
variables at the signal site. That answer is not quite correct, as
explained in #27518. Instead, use the site of a deferreturn call.
It has all the right variables marked as live (no args, all the return
values, except those that escape to the heap, in which case the
corresponding PAUTOHEAP variables will be live instead).
This CL requires stack objects, so that all the local variables
and args referenced by the deferred closures keep the right variables alive.
Fixes #27518
Change-Id: Id45d8a8666759986c203181090b962e2981e48ca
Reviewed-on: https://go-review.googlesource.com/c/134637
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
Currently, isSystemGoroutine varies on whether it considers the
finalizer goroutine a user goroutine or a system goroutine. For the
next CL, we're going to want to always consider the finalier goroutine
a user goroutine, so add a flag that indicates that.
Updates #26903. This is preparation for unifying STW GC and concurrent
GC.
Change-Id: Iafc92e519c13d9f8d879332cb5f0d12164104c33
Reviewed-on: https://go-review.googlesource.com/c/134778
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
|
|
During a call to a reflect-generated function or method (via
makeFuncStub or methodValueCall), when should we scan the return
values?
When we're starting a reflect call, the space on the stack for the
return values is not initialized yet, as it contains whatever junk was
on the stack of the caller at the time. The return space must not be
scanned during a GC.
When we're finishing a reflect call, the return values are
initialized, and must be scanned during a GC to make sure that any
pointers in the return values are found and their referents retained.
When the GC stack walk comes across a reflect call in progress on the
stack, it needs to know whether to scan the results or not. It doesn't
know the progress of the reflect call, so it can't decide by
itself. The reflect package needs to tell it.
This CL adds another slot in the frame of makeFuncStub and
methodValueCall so we can put a boolean in there which tells the
runtime whether to scan the results or not.
This CL also adds the args length to reflectMethodValue so the
runtime can restrict its scanning to only the args section (not the
results) if the reflect package says the results aren't ready yet.
Do a delicate dance in the reflect package to set the "results are
valid" bit. We need to make sure we set the bit only after we've
copied the results back to the stack. But we must set the bit before
we drop reflect's copy of the results. Otherwise, we might have a
state where (temporarily) no one has a live copy of the results.
That's the state we were observing in issue #27695 before this CL.
The bitmap used by the runtime currently contains only the args.
(Actually, it contains all the bits, but the size is set so we use
only the args portion.) This is safe for early in a reflect call, but
unsafe late in a reflect call. The test issue27695.go demonstrates
this unsafety. We change the bitmap to always include both args
and results, and decide at runtime which portion to use.
issue27695.go only has a test for method calls. Function calls were ok
because there wasn't a safepoint between when reflect dropped its copy
of the return values and when the caller is resumed. This may change
when we introduce safepoints everywhere.
This truncate-to-only-the-args was part of CL 9888 (in 2015). That
part of the CL fixed the problem demonstrated in issue27695b.go but
introduced the problem demonstrated in issue27695.go.
TODO, in another CL: simplify FuncLayout and its test. stack return
value is now identical to frametype.ptrdata + frametype.gcdata.
Fixes #27695
Change-Id: I2d49b34e34a82c6328b34f02610587a291b25c5f
Reviewed-on: https://go-review.googlesource.com/137440
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
Supporting frame-pointer makes Linux's perf and other profilers much more useful
because it lets them gather a stack trace efficiently on profiling events. Major
changes include:
1. save FP on the word below where RSP is pointing to (proposed by Cherry and Austin)
2. adjust some specific offsets in runtime assembly and wrapper code
3. add support to FP in goroutine scheduler
4. adjust link stack overflow check to take the extra word into account
5. adjust nosplit test cases to enable frame sizes which are 16 bytes aligned
Performance impacts on go1 benchmarks:
Enable frame-pointer (by default)
name old time/op new time/op delta
BinaryTree17-46 5.94s ± 0% 6.00s ± 0% +1.03% (p=0.029 n=4+4)
Fannkuch11-46 2.84s ± 1% 2.77s ± 0% -2.58% (p=0.008 n=5+5)
FmtFprintfEmpty-46 55.0ns ± 1% 58.9ns ± 1% +7.06% (p=0.008 n=5+5)
FmtFprintfString-46 102ns ± 0% 105ns ± 0% +2.94% (p=0.008 n=5+5)
FmtFprintfInt-46 118ns ± 0% 117ns ± 1% -1.19% (p=0.000 n=4+5)
FmtFprintfIntInt-46 181ns ± 0% 182ns ± 1% ~ (p=0.444 n=5+5)
FmtFprintfPrefixedInt-46 215ns ± 1% 214ns ± 0% ~ (p=0.254 n=5+4)
FmtFprintfFloat-46 292ns ± 0% 296ns ± 0% +1.46% (p=0.029 n=4+4)
FmtManyArgs-46 720ns ± 0% 732ns ± 0% +1.72% (p=0.008 n=5+5)
GobDecode-46 9.82ms ± 1% 10.03ms ± 2% +2.10% (p=0.008 n=5+5)
GobEncode-46 8.14ms ± 0% 8.72ms ± 1% +7.14% (p=0.008 n=5+5)
Gzip-46 420ms ± 0% 424ms ± 0% +0.92% (p=0.008 n=5+5)
Gunzip-46 48.2ms ± 0% 48.4ms ± 0% +0.41% (p=0.008 n=5+5)
HTTPClientServer-46 201µs ± 4% 201µs ± 0% ~ (p=0.730 n=5+4)
JSONEncode-46 17.1ms ± 0% 17.7ms ± 1% +3.80% (p=0.008 n=5+5)
JSONDecode-46 88.0ms ± 0% 90.1ms ± 0% +2.42% (p=0.008 n=5+5)
Mandelbrot200-46 5.06ms ± 0% 5.07ms ± 0% ~ (p=0.310 n=5+5)
GoParse-46 5.04ms ± 0% 5.12ms ± 0% +1.53% (p=0.008 n=5+5)
RegexpMatchEasy0_32-46 117ns ± 0% 117ns ± 0% ~ (all equal)
RegexpMatchEasy0_1K-46 332ns ± 0% 329ns ± 0% -0.78% (p=0.008 n=5+5)
RegexpMatchEasy1_32-46 104ns ± 0% 113ns ± 0% +8.65% (p=0.029 n=4+4)
RegexpMatchEasy1_1K-46 563ns ± 0% 569ns ± 0% +1.10% (p=0.008 n=5+5)
RegexpMatchMedium_32-46 167ns ± 2% 177ns ± 1% +5.74% (p=0.008 n=5+5)
RegexpMatchMedium_1K-46 49.5µs ± 0% 53.4µs ± 0% +7.81% (p=0.008 n=5+5)
RegexpMatchHard_32-46 2.56µs ± 1% 2.72µs ± 0% +6.01% (p=0.008 n=5+5)
RegexpMatchHard_1K-46 77.0µs ± 0% 81.8µs ± 0% +6.24% (p=0.016 n=5+4)
Revcomp-46 631ms ± 1% 627ms ± 1% ~ (p=0.095 n=5+5)
Template-46 81.8ms ± 0% 86.3ms ± 0% +5.55% (p=0.008 n=5+5)
TimeParse-46 423ns ± 0% 432ns ± 0% +2.32% (p=0.008 n=5+5)
TimeFormat-46 478ns ± 2% 497ns ± 1% +3.89% (p=0.008 n=5+5)
[Geo mean] 71.6µs 73.3µs +2.45%
name old speed new speed delta
GobDecode-46 78.1MB/s ± 1% 76.6MB/s ± 2% -2.04% (p=0.008 n=5+5)
GobEncode-46 94.3MB/s ± 0% 88.0MB/s ± 1% -6.67% (p=0.008 n=5+5)
Gzip-46 46.2MB/s ± 0% 45.8MB/s ± 0% -0.91% (p=0.008 n=5+5)
Gunzip-46 403MB/s ± 0% 401MB/s ± 0% -0.41% (p=0.008 n=5+5)
JSONEncode-46 114MB/s ± 0% 109MB/s ± 1% -3.66% (p=0.008 n=5+5)
JSONDecode-46 22.0MB/s ± 0% 21.5MB/s ± 0% -2.35% (p=0.008 n=5+5)
GoParse-46 11.5MB/s ± 0% 11.3MB/s ± 0% -1.51% (p=0.008 n=5+5)
RegexpMatchEasy0_32-46 272MB/s ± 0% 272MB/s ± 1% ~ (p=0.190 n=4+5)
RegexpMatchEasy0_1K-46 3.08GB/s ± 0% 3.11GB/s ± 0% +0.77% (p=0.008 n=5+5)
RegexpMatchEasy1_32-46 306MB/s ± 0% 283MB/s ± 0% -7.63% (p=0.029 n=4+4)
RegexpMatchEasy1_1K-46 1.82GB/s ± 0% 1.80GB/s ± 0% -1.07% (p=0.008 n=5+5)
RegexpMatchMedium_32-46 5.99MB/s ± 0% 5.64MB/s ± 1% -5.77% (p=0.016 n=4+5)
RegexpMatchMedium_1K-46 20.7MB/s ± 0% 19.2MB/s ± 0% -7.25% (p=0.008 n=5+5)
RegexpMatchHard_32-46 12.5MB/s ± 1% 11.8MB/s ± 0% -5.66% (p=0.008 n=5+5)
RegexpMatchHard_1K-46 13.3MB/s ± 0% 12.5MB/s ± 1% -6.01% (p=0.008 n=5+5)
Revcomp-46 402MB/s ± 1% 405MB/s ± 1% ~ (p=0.095 n=5+5)
Template-46 23.7MB/s ± 0% 22.5MB/s ± 0% -5.25% (p=0.008 n=5+5)
[Geo mean] 82.2MB/s 79.6MB/s -3.26%
Disable frame-pointer (GOEXPERIMENT=noframepointer)
name old time/op new time/op delta
BinaryTree17-46 5.94s ± 0% 5.96s ± 0% +0.39% (p=0.029 n=4+4)
Fannkuch11-46 2.84s ± 1% 2.79s ± 1% -1.68% (p=0.008 n=5+5)
FmtFprintfEmpty-46 55.0ns ± 1% 55.2ns ± 3% ~ (p=0.794 n=5+5)
FmtFprintfString-46 102ns ± 0% 103ns ± 0% +0.98% (p=0.016 n=5+4)
FmtFprintfInt-46 118ns ± 0% 115ns ± 0% -2.54% (p=0.029 n=4+4)
FmtFprintfIntInt-46 181ns ± 0% 179ns ± 0% -1.10% (p=0.000 n=5+4)
FmtFprintfPrefixedInt-46 215ns ± 1% 213ns ± 0% ~ (p=0.143 n=5+4)
FmtFprintfFloat-46 292ns ± 0% 300ns ± 0% +2.83% (p=0.029 n=4+4)
FmtManyArgs-46 720ns ± 0% 739ns ± 0% +2.64% (p=0.008 n=5+5)
GobDecode-46 9.82ms ± 1% 9.78ms ± 1% ~ (p=0.151 n=5+5)
GobEncode-46 8.14ms ± 0% 8.12ms ± 1% ~ (p=0.690 n=5+5)
Gzip-46 420ms ± 0% 420ms ± 0% ~ (p=0.548 n=5+5)
Gunzip-46 48.2ms ± 0% 48.0ms ± 0% -0.33% (p=0.032 n=5+5)
HTTPClientServer-46 201µs ± 4% 199µs ± 3% ~ (p=0.548 n=5+5)
JSONEncode-46 17.1ms ± 0% 17.2ms ± 0% ~ (p=0.056 n=5+5)
JSONDecode-46 88.0ms ± 0% 88.6ms ± 0% +0.64% (p=0.008 n=5+5)
Mandelbrot200-46 5.06ms ± 0% 5.07ms ± 0% ~ (p=0.548 n=5+5)
GoParse-46 5.04ms ± 0% 5.07ms ± 0% +0.65% (p=0.008 n=5+5)
RegexpMatchEasy0_32-46 117ns ± 0% 112ns ± 4% -4.27% (p=0.016 n=4+5)
RegexpMatchEasy0_1K-46 332ns ± 0% 330ns ± 1% ~ (p=0.095 n=5+5)
RegexpMatchEasy1_32-46 104ns ± 0% 110ns ± 1% +5.29% (p=0.029 n=4+4)
RegexpMatchEasy1_1K-46 563ns ± 0% 567ns ± 2% ~ (p=0.151 n=5+5)
RegexpMatchMedium_32-46 167ns ± 2% 166ns ± 0% ~ (p=0.333 n=5+4)
RegexpMatchMedium_1K-46 49.5µs ± 0% 49.6µs ± 0% ~ (p=0.841 n=5+5)
RegexpMatchHard_32-46 2.56µs ± 1% 2.49µs ± 0% -2.81% (p=0.008 n=5+5)
RegexpMatchHard_1K-46 77.0µs ± 0% 75.8µs ± 0% -1.55% (p=0.008 n=5+5)
Revcomp-46 631ms ± 1% 628ms ± 0% ~ (p=0.095 n=5+5)
Template-46 81.8ms ± 0% 84.3ms ± 1% +3.05% (p=0.008 n=5+5)
TimeParse-46 423ns ± 0% 425ns ± 0% +0.52% (p=0.008 n=5+5)
TimeFormat-46 478ns ± 2% 478ns ± 1% ~ (p=1.000 n=5+5)
[Geo mean] 71.6µs 71.6µs -0.01%
name old speed new speed delta
GobDecode-46 78.1MB/s ± 1% 78.5MB/s ± 1% ~ (p=0.151 n=5+5)
GobEncode-46 94.3MB/s ± 0% 94.5MB/s ± 1% ~ (p=0.690 n=5+5)
Gzip-46 46.2MB/s ± 0% 46.2MB/s ± 0% ~ (p=0.571 n=5+5)
Gunzip-46 403MB/s ± 0% 404MB/s ± 0% +0.33% (p=0.032 n=5+5)
JSONEncode-46 114MB/s ± 0% 113MB/s ± 0% ~ (p=0.056 n=5+5)
JSONDecode-46 22.0MB/s ± 0% 21.9MB/s ± 0% -0.64% (p=0.008 n=5+5)
GoParse-46 11.5MB/s ± 0% 11.4MB/s ± 0% -0.64% (p=0.008 n=5+5)
RegexpMatchEasy0_32-46 272MB/s ± 0% 285MB/s ± 4% +4.74% (p=0.016 n=4+5)
RegexpMatchEasy0_1K-46 3.08GB/s ± 0% 3.10GB/s ± 1% ~ (p=0.151 n=5+5)
RegexpMatchEasy1_32-46 306MB/s ± 0% 290MB/s ± 1% -5.21% (p=0.029 n=4+4)
RegexpMatchEasy1_1K-46 1.82GB/s ± 0% 1.81GB/s ± 2% ~ (p=0.151 n=5+5)
RegexpMatchMedium_32-46 5.99MB/s ± 0% 6.02MB/s ± 1% ~ (p=0.063 n=4+5)
RegexpMatchMedium_1K-46 20.7MB/s ± 0% 20.7MB/s ± 0% ~ (p=0.659 n=5+5)
RegexpMatchHard_32-46 12.5MB/s ± 1% 12.8MB/s ± 0% +2.88% (p=0.008 n=5+5)
RegexpMatchHard_1K-46 13.3MB/s ± 0% 13.5MB/s ± 0% +1.58% (p=0.008 n=5+5)
Revcomp-46 402MB/s ± 1% 405MB/s ± 0% ~ (p=0.095 n=5+5)
Template-46 23.7MB/s ± 0% 23.0MB/s ± 1% -2.95% (p=0.008 n=5+5)
[Geo mean] 82.2MB/s 82.3MB/s +0.04%
Frame-pointer is enabled on Linux by default but can be disabled by setting: GOEXPERIMENT=noframepointer.
Fixes #10110
Change-Id: I1bfaca6dba29a63009d7c6ab04ed7a1413d9479e
Reviewed-on: https://go-review.googlesource.com/61511
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
gentraceback handles system stack transitions, but only when they're
done by systemstack(). Handle morestack() too.
I tried to do this generically but systemstack and morestack are
actually *very* different functions. Most notably, systemstack returns
"normally", just messes with $sp along the way. morestack never
returns at all -- it calls newstack, and newstack then jumps both
stacks and functions back to whoever called morestack. I couldn't
think of a way to handle both of them generically. So don't.
The current implementation does not include systemstack on the generated
traceback. That's partly because I don't know how to find its stack frame
reliably, and partly because the current structure of the code wants to
do the transition before the call, not after. If we're willing to
assume that morestack's stack frame is 0 size, this could probably be
fixed.
For posterity's sake, alternatives tried:
- Have morestack put a dummy function into schedbuf, like systemstack
does. This actually worked (see patchset 1) but more by a series of
coincidences than by deliberate design. The biggest coincidence was that
because morestack_switch was a RET and its stack was 0 size, it actually
worked to jump back to it at the end of newstack -- it would just return
to the caller of morestack. Way too subtle for me, and also a little
slower than just jumping directly.
- Put morestack's PC and SP into schedbuf, so that gentraceback could
treat it like a normal function except for the changing SP. This was a
terrible idea and caused newstack to reenter morestack in a completely
unreasonable state.
To make testing possible I did a small redesign of testCPUProfile to
take a callback that defines how to check if the conditions pass to it
are satisfied. This seemed better than making the syntax of the "need"
strings even more complicated.
Updates #25943
Change-Id: I9271a30a976f80a093a3d4d1c7e9ec226faf74b4
Reviewed-on: https://go-review.googlesource.com/126795
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
gentraceback gets the currently running g to do some sanity checks, but
should use gp everywhere to do its actual work. Some noncritical checks
later accidentally used g instead of gp. This seems like it could be a
problem in many different contexts, but I noticed in Windows profiling,
where profilem calls gentraceback on a goroutine from a different
thread.
Change-Id: I3da27a43e833b257f6411ee6893bdece45a9323f
Reviewed-on: https://go-review.googlesource.com/128895
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Austin Clements <austin@google.com>
|
|
The hasprefix function is redundant and can be removed since it has
the same implementation as hasPrefix modulo variable names.
Fixes #25688
Change-Id: I499cc24a2b5c38d1301718a4e66f555fd138386f
Reviewed-on: https://go-review.googlesource.com/115835
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
|
|
If we're in a libc call and get a trap, don't try to traceback the libc call.
Start from the state we had at entry to libc.
If there are multiple libc calls outstanding, remember the outermost one.
Fixes #26393
Change-Id: Icfe8794b95bf3bfd1a0679b456dcde2481dcabf3
Reviewed-on: https://go-review.googlesource.com/124195
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Fixes #24518.
Change-Id: I99c79c5a2ab9dbe7f0d257c263da9d2b5d1d55c4
Reviewed-on: https://go-review.googlesource.com/121917
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Traceback matches the defer stack with the function call stack using
the SP recorded in defer frames when the defer frame is created.
However, on LR machines this is ambiguous: if function A pushes a
defer and then calls function B, where B is a leaf function with a
zero-sized frame, then both A and B have the same SP and will *both*
match the defer on the defer stack. Since traceback unwinds through B
first, it will incorrectly match up the defer with B's frame instead
of A's frame.
Where this goes particularly wrong is if function B causes a signal
that turns into a panic (e.g., a nil pointer dereference). In order to
handle the fact that we may not have a liveness map at the location
that caused the signal and injected a sigpanic call, traceback has
logic to unwind the panicking frame's continuation PC to the PC where
the most recent defer was pushed (this is safe because the frame is
dead other than any defers it pushed). However, if traceback
mis-matches the defer stack, it winds up reporting the B's
continuation PC is in A. If the runtime then uses this continuation PC
to look up PCDATA in B, it will panic because the PC is out of range
for B. This failure mode can be seen in
sync/atomic/atomic_test.go:TestNilDeref. An example failure is:
https://build.golang.org/log/8e07a762487839252af902355f6b1379dbd463c5
This CL fixes all of this by recognizing that a function that pushes a
defer must also have a non-zero-sized frame and using this fact to
refine the defer matching logic.
Fixes the build for arm64, mips, mipsle, ppc64, ppc64le, and s390x.
Fixes #25499.
Change-Id: Iff7c01d08ad42f3de22b3a73658cc2f674900101
Reviewed-on: https://go-review.googlesource.com/114078
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Currently isSystemGoroutine has a hard-coded list of known entry
points into system goroutines. This list is annoying to maintain. For
example, it's missing the ensureSigM goroutine.
Replace it with a check that simply looks for any goroutine with
runtime function as its entry point, with a few exceptions. This also
matches the definition recently added to the trace viewer (CL 81315).
Change-Id: Iaed723d4a6e8c2ffb7c0c48fbac1688b00b30f01
Reviewed-on: https://go-review.googlesource.com/81655
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Every time I poke at #14921, the g.waitreason string
pointer writes show up.
They're not particularly important performance-wise,
but it'd be nice to clear the noise away.
And it does open up a few extra bytes in the g struct
for some future use.
This is a re-roll of CL 99078, which was rolled
back because of failures on s390x.
Those failures were apparently due to an old version of gdb.
Change-Id: Icc2c12f449b2934063fd61e272e06237625ed589
Reviewed-on: https://go-review.googlesource.com/111256
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
|
