| Age | Commit message (Collapse) | Author |
|---|
|
Currently, gentraceback's loop ends with a call to tracebackCgoContext
to process cgo frames. This requires spreading various parts of the
printing and pcbuf logic across these two functions.
Clean this up by moving cgo unwinding into unwinder and then lifting
the printing and pcbuf logic from tracebackCgoContext into
gentraceback along with the other printing and pcbuf logic.
Updates #54466.
Change-Id: Ic71afaa5ae110c0ea5be9409e267e4284e36a8c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/468299
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Updates #54466.
Change-Id: If070cf3f484e3e02b8e586bff466e0018b1a1845
Reviewed-on: https://go-review.googlesource.com/c/go/+/468298
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This is a really nice simplification for all of these call sites.
It also achieves a nice performance improvement for stack copying:
goos: linux
goarch: amd64
pkg: runtime
cpu: Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz
│ before │ after │
│ sec/op │ sec/op vs base │
StackCopyPtr-48 89.25m ± 1% 79.78m ± 1% -10.62% (p=0.000 n=20)
StackCopy-48 83.48m ± 2% 71.88m ± 1% -13.90% (p=0.000 n=20)
StackCopyNoCache-48 2.504m ± 2% 2.195m ± 1% -12.32% (p=0.000 n=20)
StackCopyWithStkobj-48 21.66m ± 1% 21.02m ± 2% -2.95% (p=0.000 n=20)
geomean 25.21m 22.68m -10.04%
Updates #54466.
Change-Id: I31715b7b6efd65726940041d3052bb1c0a1186f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/468297
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Currently, all stack walking logic is in one venerable, large, and
very, very complicated function: runtime.gentraceback. This function
has three distinct operating modes: printing, populating a PC buffer,
or invoking a callback. And it has three different modes of unwinding:
physical Go frames, inlined Go frames, and cgo frames. It also has
several flags. All of this logic is very interwoven.
This CL reimplements the monolithic gentraceback function as an
"unwinder" type with an iterator API. It moves all of the logic for
stack walking into this new type, and gentraceback is now a
much-simplified wrapper around the new unwinder type that still
implements printing, populating a PC buffer, and invoking a callback.
Follow-up CLs will replace uses of gentraceback with direct uses of
unwinder.
Exposing traceback functionality as an iterator API will enable a lot
of follow-up work such as simplifying the open-coded defer
implementation (which should in turn help with #26813 and #37233),
printing the bottom of deep stacks (#7181), and eliminating the small
limit on CPU stacks in profiles (#56029).
Fixes #54466.
Change-Id: I36e046dc423c9429c4f286d47162af61aff49a0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/458218
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
|
|
Currently, gentraceback consumes the gp.cgoCtxt slice by copying the
slice header and then sub-slicing it as it unwinds. The code for this
is nice and clear, but we're about to lift this state into a structure
and mutating it is going to introduce write barriers that are
disallowed in gentraceback.
This CL replaces the mutable slice header with an index into
gp.cgoCtxt.
For #54466.
Change-Id: I6b701bb67d657290a784baaca34ed02d8247ede2
Reviewed-on: https://go-review.googlesource.com/c/go/+/466863
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This converts all places in the runtime that perform inline expansion
to use the new inlineUnwinder abstraction.
For #54466.
Change-Id: I48d996fb6263ed5225bd21d30914a27ae434528d
Reviewed-on: https://go-review.googlesource.com/c/go/+/466099
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Since srcFunc can represent information for either an real text
function or an inlined function, this means we no longer have to
synthesize a fake _func just to call showframe on an inlined frame.
This is cleaner and also eliminates the one case where _func values
live in the heap. This will let us mark them NotInHeap, which will in
turn eliminate pesky write barriers in the traceback rewrite.
For #54466.
Change-Id: Ibf5e24d01ee4bf384c825e1a4e2922ef444a438e
Reviewed-on: https://go-review.googlesource.com/c/go/+/466097
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Currently, gentraceback resolves the funcInfo of the caller prior to
processing the current frame (calling the callback, printing it, etc).
As a result, if this lookup fails in a verbose context, it will print
the failure before printing the frame that it's already resolved.
To fix this, move the resolution of LR to a funcInfo to after current
frame processing.
This also has the advantage that we can reduce the scope of "flr" (the
caller's funcInfo) to only the post-frame part of the loop, which will
make it easier to stack-rip gentraceback into an iterator.
For #54466.
Change-Id: I8be44d4eac598a686c32936ab37018b8aa97c00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/458217
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
|
|
gentraceback also tracks the funcID of the callee, which is more
general. Fix this up to happen in all cases and eliminate waspanic in
favor of checking the funcID of the caller.
For #54466.
Change-Id: Idc98365a6f05022db18ddcd5b3ed8684a6872a88
Reviewed-on: https://go-review.googlesource.com/c/go/+/458216
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Currently, gentraceback keeps a copy of the stack bounds of the stack
it's walking in the "stack" variable. Now that "gp" always refers to
the G whose stack it's walking, we can simply use gp.stack instead of
keeping a separate copy.
For #54466.
Change-Id: I68256e5dff6212cfcf14eda615487e66a92d4914
Reviewed-on: https://go-review.googlesource.com/c/go/+/458215
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Fixes #38651
Change-Id: Id46d684ee80e208c018791a06c26f304670ed159
Reviewed-on: https://go-review.googlesource.com/c/go/+/435337
Run-TryBot: Nick Ripley <nick.ripley@datadoghq.com>
Reviewed-by: Ethan Reesor <ethan.reesor@gmail.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Change-Id: I9ef4898d68dfd06618c0bd8e23f81a1d2c77a836
Signed-off-by: cui fliter <imcusg@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/447460
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
|
|
This adds the function "start line number" to runtime._func and
runtime.inlinedCall objects. The "start line number" is the line number
of the func keyword or TEXT directive for assembly.
Subtracting the start line number from PC line number provides the
relative line offset of a PC from the the start of the function. This
helps with source stability by allowing code above the function to move
without invalidating samples within the function.
Encoding start line rather than relative lines directly is convenient
because the pprof format already contains a start line field.
This CL uses a straightforward encoding of explictly including a start
line field in every _func and inlinedCall. It is possible that we could
compress this further in the future. e.g., functions with a prologue
usually have <line of PC 0> == <start line>. In runtime.test, 95% of
functions have <line of PC 0> == <start line>.
According to bent, this is geomean +0.83% binary size vs master and
-0.31% binary size vs 1.19.
Note that //line directives can change the file and line numbers
arbitrarily. The encoded start line is as adjusted by //line directives.
Since this can change in the middle of a function, `line - start line`
offset calculations may not be meaningful if //line directives are in
use.
For #55022.
Change-Id: Iaabbc6dd4f85ffdda294266ef982ae838cc692f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/429638
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
The traceback code has special "jump stack" logic, to trace back
stack switches through systemstack. If we're at the entry of
systemstack, the stack switch hasn't happened, so don't jump to
user stack.
The jump stack logic is only used if we're on the g0 stack. It can
happen that we're at the entry of a recursive systemstack call on
the g0 stack. In we jump stack here, there will be two problems:
1. There are frames between entering the g0 stack and this
recursive systemstack call. Those frames will be lost.
2. Worse, we switched frame.sp but frame.fp calculation will use
the entry SP delta (0), which will be wrong, which in turn
leads wrong frame.lr and things will go off.
For now, don't jump stack if we're at entry of systemstack (SP
delta is 0).
Using a per-PC SPWRITE marker may be a better fix. If we haven't
written the SP, we haven't switched the stack so we can just
unwind like a normal function.
May fix #55851.
Change-Id: I2b624c8c086b235b34d9c7d3cebd4a37264f00f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/437299
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Switch to the more Go-style name to match inlinedCall.nameOff.
Change-Id: I2115b27af8309e1ead7d61ecc65fe4fc966030f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/428657
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
The meaning of this field is unchanged, this CL simply gives it a more
descriptive name, as func_ makes it sound like a reference to the _func.
Change-Id: I70e54f34bede7636ce4d7b9dd0f7557308f02143
Reviewed-on: https://go-review.googlesource.com/c/go/+/427961
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
CL 424257 modified gentraceback to switch gp when jumping from a
system stack to a user stack to simplify reasoning through the rest of
the function. This has the unintended side-effect of also switching
all references to gp.m. The vast majority of the time, g0.m and curg.m
are the same across a stack switch, making this a no-op, but there's
at least one case where this isn't true: if a profiling signal happens
in execute between setting mp.curg and setting gp.m. In this case,
mp.curg.m is briefly nil, which can cause gentraceback to crash with a
nil pointer dereference. We see this failure (surprisingly
frequently!) in profiling tests in the morestack=mayMoreStackPreempt
testing mode (#48297).
Fix this by making only jumping stacks if doing so will not switch Ms.
This restores the original property that gp.m doesn't change across
the stack jump, and makes gentraceback a little more conservative
about jumping stacks.
Fixes #54885.
Change-Id: Ib1524c41c748eeff35896e0f3abf9a7efbe5969f
Reviewed-on: https://go-review.googlesource.com/c/go/+/428656
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Austin Clements <austin@google.com>
|
|
Use an atomic.Uint32 to represent the state of finalizer goroutine.
fingStatus will only be changed to fingWake in non fingWait state,
so it is safe to set fingRunningFinalizer status in runfinq.
name old time/op new time/op delta
Finalizer-8 592µs ± 4% 561µs ± 1% -5.22% (p=0.000 n=10+10)
FinalizerRun-8 694ns ± 6% 675ns ± 7% ~ (p=0.059 n=9+8)
Change-Id: I7e4da30cec98ce99f7d8cf4c97f933a8a2d1cae1
Reviewed-on: https://go-review.googlesource.com/c/go/+/400134
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
The stkframe struct and its methods are strewn across different source
files. Since they actually have a pretty coherent theme at this point,
migrate it all into a new file, stkframe.go. There are no code changes
in this CL.
For #54466, albeit rather indirectly.
Change-Id: Ibe53fc4b1106d131005e1c9d491be838a8f14211
Reviewed-on: https://go-review.googlesource.com/c/go/+/424516
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Auto-Submit: Austin Clements <austin@google.com>
|
|
Currently, stkframe.arglen and stkframe.argmap are populated by
gentraceback under a particular set of circumstances. But because they
can be constructed from other fields in stkframe, they don't need to
be computed eagerly at all. They're also rather misleading, as they're
only part of computing the actual argument map and most callers should
be using getStackMap, which does the rest of the work.
This CL drops these fields from stkframe. It shifts the functions that
used to compute them, getArgInfoFast and getArgInfo, into
corresponding methods stkframe.argBytes and stkframe.argMapInternal.
argBytes is expected to be used by callers that need to know only the
argument frame size, while argMapInternal is used only by argBytes and
getStackMap.
We also move some of the logic from getStackMap into argMapInternal
because the previous split of responsibilities didn't make much sense.
This lets us return just a bitvector from argMapInternal, rather than
both a bitvector, which carries a size, and an "actually use this
size".
The getArgInfoFast function was inlined before (and inl_test checked
this). We drop that requirement from stkframe.argBytes because the
uses of this have shifted and now it's only called from heap dumping
(which never happens) and conservative stack frame scanning (which
very, very rarely happens).
There will be a few follow-up clean-up CLs.
For #54466. This is a nice clean-up on its own, but it also serves to
remove pointers from the traceback state that would eventually become
troublesome write barriers once we stack-rip gentraceback.
Change-Id: I107f98ed8e7b00185c081de425bbf24af02a4163
Reviewed-on: https://go-review.googlesource.com/c/go/+/424514
Run-TryBot: Austin Clements <austin@google.com>
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Currently, when traceback jumps from the system stack to a user stack
(e.g., during profiling tracebacks), it leaves gp pointing at the g0.
This is currently harmless since it's only used during profiling, so
the code paths in gentraceback that care about gp aren't used, but
it's really confusing and would certainly break if _TraceJumpStack
were ever used in a context other than profiling.
Fix this by updating gp to point to the user g when we switch stacks.
For #54466.
Change-Id: I1541e004667a52e37671803ce45c91d8c5308830
Reviewed-on: https://go-review.googlesource.com/c/go/+/424257
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
|
|
The f funcInfo argument is always the same as frame.fn, so we don't
need to pass it. I suspect that was there to make the signatures of
getArgInfoFast and getArgInfo more similar, but it's not necessary.
For #54466.
Change-Id: Idc717f4df09e97cad49d52c5b7edf28090908cba
Reviewed-on: https://go-review.googlesource.com/c/go/+/424255
Run-TryBot: Austin Clements <austin@google.com>
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Currently, gentraceback tracks the closure context of the outermost
frame. This used to be important for "unstarted" calls to reflect
function stubs, where "unstarted" calls are either deferred functions
or the entry-point of a goroutine that hasn't run. Because reflect
function stubs have a dynamic argument map, we have to reach into
their closure context to fetch to map, and how to do this differs
depending on whether the function has started. This was discovered in
issue #25897.
However, as part of the register ABI, "go" and "defer" were made much
simpler, and any "go" or "defer" of a function that takes arguments or
returns results gets wrapped in a closure that provides those
arguments (and/or discards the results). Hence, we'll see that closure
instead of a direct call to a reflect stub, and can get its static
argument map without any trouble.
The one case where we may still see an unstarted reflect stub is if
the function takes no arguments and has no results, in which case the
compiler can optimize away the wrapper closure. But in this case we
know the argument map is empty: the compiler can apply this
optimization precisely because the target function has no argument
frame.
As a result, we no longer need to track the closure context during
traceback, so this CL drops all of that mechanism.
We still have to be careful about the unstarted case because we can't
reach into the function's locals frame to pull out its context
(because it has no locals frame). We double-check that in this case
we're at the function entry.
I would prefer to do this with some in-code PCDATA annotations of
where to find the dynamic argument map, but that's a lot of mechanism
to introduce for just this. It might make sense to consider this along
with #53609.
Finally, we beef up the test for this so it more reliably forces the
runtime down this path. It's fundamentally probabilistic, but this
tweak makes it better. Scheduler testing hooks (#54475) would make it
possible to write a reliable test for this.
For #54466, but it's a nice clean-up all on its own.
Change-Id: I16e4f2364ba2ea4b1fec1e27f971b06756e7b09f
Reviewed-on: https://go-review.googlesource.com/c/go/+/424254
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Updates #53821
Change-Id: I99b01f8e91b798e73275635c8a63fcdc4a8df9f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/423888
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
For #53821.
Change-Id: I93409f377881a3c029b41b0f1fbcef5e21091f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/419438
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Change-Id: I80e847451b697de049a01df051a73b5976b39d4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418589
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Change-Id: Idfd02781a9ee550da240ef53898943e8cd37dd64
Reviewed-on: https://go-review.googlesource.com/c/go/+/418574
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
If a goroutine is started within a user region, internal/trace assigns
the child goroutine a nameless region for its entire lifetime which is
assosciated the same task as the parent's region.
This is not strictly necessary: a child goroutine is not necessarily
related to the task unless it performs some task operation (in which
case it will be associated with the task through the standard means).
However, it can be quite handy to see child goroutines within a region,
which may be child worker goroutines that you simply didn't perform task
operations on.
If the first GC occurs during a region, the GC worker goroutines will
also inherit a child region. We know for sure that these aren't related
to the task, so filter them out from the region list.
Note that we can't exclude system goroutines from setting activeRegions
in EvGoCreate handling, because we don't know the goroutine start
function name until the first EvGoStart.
Fixes #53784.
Change-Id: Ic83d84e23858a8400a76d1ae2f1418ef49951178
Reviewed-on: https://go-review.googlesource.com/c/go/+/416858
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
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.
May fix #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>
|
|
This gives explicit names to the possible states of throwing (-1, 0, 1).
m.throwing is now one of:
throwTypeOff: not throwing, previously == 0
throwTypeUser: user throw, previously == -1
throwTypeRuntime: runtime throw, previously == 1
For runtime throws, we now always include frame metadata and system
goroutines regardless of GOTRACEBACK to aid in debugging the runtime.
For user throws, we no longer include frame metadata or runtime frames,
unless GOTRACEBACK=system or higher.
For #51485.
Change-Id: If252e2377a0b6385ce7756b937929be4273a56c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/390421
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
|
|
This CL adds support for debugger function calls on linux arm64
platform. The protocol is basically the same as in CL 109699, except for
the following differences:
1, The abi difference which affect parameter passing and frame layout.
2, Stores communication information in R20.
3, The closure register is R26.
4, Use BRK 0 instruction to generate a breakpoint. The saved PC in
sigcontext is the PC where the signal occurred, not the next PC.
In addition, this CL refactors the existing code (which is dedicated to
amd64) for easier multi-arch scaling.
Fixes #50614
Change-Id: I06b14e345cc89aab175f4a5f2287b765da85a86b
Reviewed-on: https://go-review.googlesource.com/c/go/+/395754
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This makes it easier to figure out where the crash is occurring.
Change-Id: Ie1f78a360367090dcd61c61b2a55c34f3e2ff2eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/390034
Trust: David Chase <drchase@google.com>
Reviewed-by: David Chase <drchase@google.com>
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Code essentially mirrors AMD64 implementation.
Change-Id: Ie97627a3041d1858fb1a30d2fc500302ab4011b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/373363
Trust: Eric Fang <eric.fang@arm.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Like the VDSO, we cannot directly traceback from the Linux kernel ARM
atomic/barrier helpers. However, unlike the VDSO, this functions are
extremely simple. Neither of the functions we use, kuser_cmpxchg and
kuser_memory_barrier, touch SP or LR.
We can use this to our advantage to read LR and simply start tracebacks
in the caller.
Fixes #49182
Change-Id: I890edbeb7c128938000fe7baf6f913c02a956edd
Reviewed-on: https://go-review.googlesource.com/c/go/+/362977
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Add explicit address sanitizer instrumentation to the runtime and
syscall packages. The compiler does not instrument the runtime
package. It does instrument the syscall package, but we need to add
a couple of cases that it can't see.
Refer to the implementation of the asan malloc runtime library,
this patch also allocates extra memory as the redzone, around the
returned memory region, and marks the redzone as unaddressable to
detect the overflows or underflows.
Updates #44853.
Change-Id: I2753d1cc1296935a66bf521e31ce91e35fcdf798
Reviewed-on: https://go-review.googlesource.com/c/go/+/298614
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: fannie zhang <Fannie.Zhang@arm.com>
|
|
Currently, for stack traces (e.g. at panic or when runtime.Stack
is called), we print argument values from the stack. With register
ABI, we may never store the argument to stack therefore the
argument value on stack may be meaningless. This causes confusion.
This CL makes the compiler keep trace of which argument stack
slots are meaningful. If it is meaningful, it will be printed in
stack traces as before. If it may not be meaningful, it will be
printed as the stack value with a question mark ("?"). In general,
the value could be meaningful on some code paths but not others
depending on the execution, and the compiler couldn't know
statically, so we still print the stack value, instead of not
printing it at all. Also note that if the argument variable is
updated in the function body the printed value may be stale (like
before register ABI) but still considered meaningful.
Arguments passed on stack are always meaningful therefore always
printed without a question mark. Results are never printed, as
before.
(Due to a bug in the compiler we sometimes don't spill args into
their dedicated spill slots (as we should), causing it having
fewer meaningful values than it should be.)
This increases binary sizes a bit:
old new
hello 1129760 1142080 +1.09%
cmd/go 13932320 14088016 +1.12%
cmd/link 6267696 6329168 +0.98%
Fixes #45728.
Change-Id: I308a0402e5c5ab94ca0953f8bd85a56acd28f58c
Reviewed-on: https://go-review.googlesource.com/c/go/+/352057
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
If we receive an async signal while running in the VDSO, such as a
SIGABRT or SIGSEGV sent from another process, we fail to print the
stacktrace with "runtime: unknown pc <vdso PC>".
We already have machinery to handle SIGPROF in the VDSO, but it isn't
hooked up for other signals. Add it to the general signal traceback
path.
This case is covered by TestSegv by making the test more strict w.r.t.
accepted output.
Fixes #47537
Change-Id: I755585f70e0c23e207e135bc6bd2aa68298e5d24
Reviewed-on: https://go-review.googlesource.com/c/go/+/339990
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
A subsequent change will alter the semantics of _func.entry.
To make that change obvious and clear, change _func.entry to a method,
and rename the field to _func.entryPC.
Change-Id: I05d66b54d06c5956d4537b0729ddf4290c3e2635
Reviewed-on: https://go-review.googlesource.com/c/go/+/351460
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Change-Id: I83180c472db8795803c1b9be3a33f35959e4dcc2
Reviewed-on: https://go-review.googlesource.com/c/go/+/336889
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
goarch.BigEndian [generated]
[git-generate]
cd src/runtime/internal/atomic
gofmt -w -r "sys.BigEndian -> goarch.BigEndian" .
goimports -w *.go
cd ../..
gofmt -w -r "sys.BigEndian -> goarch.BigEndian" .
goimports -w *.go
Change-Id: Iad35d2b367d8defb081a77ca837e7a7c805c2b7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/329190
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
[git-generate]
cd src/runtime
goimports -w *.go
Change-Id: I1387af0f2fd1a213dc2f4c122e83a8db0fcb15f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/329189
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
internal/goarch.PtrSize [generated]
[git-generate]
cd src/runtime/internal/math
gofmt -w -r "sys.PtrSize -> goarch.PtrSize" .
goimports -w *.go
cd ../..
gofmt -w -r "sys.PtrSize -> goarch.PtrSize" .
goimports -w *.go
Change-Id: I43491cdd54d2e06d4d04152b3d213851b7d6d423
Reviewed-on: https://go-review.googlesource.com/c/go/+/328337
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
Two non-conflict changes included because they're needed for all.bash:
1. Bump internal/goversion.Version to 18. This will happen eventually
anyway (dev.typeparams will not be merged back to Go 1.17), and is
needed for cmd/api to allow new API additions.
2. Add fixedbugs/issue46725.go (new test added on master) to the list
of known failures for -G=3. This test exercises a bug that was fixed
in typecheck, but -G=3 mode has duplicated that code and will need to
be fixed as well. That's outside of the scope of a merge.
Conflicts:
- src/runtime/traceback.go
Nearby lines were removed on both master and dev.typeparams.
Merge List:
+ 2021-06-14 fdab5be159 doc/go1.17: further revise OpenBSD release notes
+ 2021-06-14 326ea438bb cmd/compile: rewrite a, b = f() to use temporaries when type not identical
+ 2021-06-14 3249b645c9 cmd/compile: factor out rewrite multi-valued f()
+ 2021-06-13 14305bf0b9 misc/cgo: generate Windows import libraries for clang
+ 2021-06-13 24cff0f044 cmd/go, misc/cgo: skip test if no .edata
+ 2021-06-13 67b1b6a2e3 cmd/compile: allow ir.OSLICE2ARRPTR in mayCall
+ 2021-06-12 1ed0d129e9 runtime: testprogcgo: don't call exported Go functions directly from Go
+ 2021-06-12 9d46ee5ac4 reflect: handle stack-to-register translation in callMethod
+ 2021-06-11 e552a6d312 cmd/go: remove hint when no module is suggested
+ 2021-06-11 16b5d766d8 syscall: do not load native libraries on non-native powershell on arm
+ 2021-06-11 77aa209b38 runtime: loop on EINTR in macOS sigNoteSleep
+ 2021-06-11 e2dc6dd5c9 doc/go1.17: clean up formatting of gofmt section
+ 2021-06-11 2f1128461d cmd/go: match Windows paths in TestScript/mod_invalid_version
+ 2021-06-11 2721da2608 doc/go1.17: fix formatting near httptest
+ 2021-06-10 770f1de8c5 net/http: remove test-only private key from production binaries
+ 2021-06-10 8d11b1d117 cmd/go: report the imports of CompiledGoFiles in ImportMap
+ 2021-06-10 dc00dc6c6b crypto/tls: let HTTP/1.1 clients connect to servers with NextProtos "h2"
+ 2021-06-09 27f83723e9 api: promote next to go1.17
+ 2021-06-09 182157c81a doc/go1.17: remove lingering TODO
+ 2021-06-09 a5bc060b42 doc/go1.17: document strconv changes for Go 1.17
+ 2021-06-09 1402b27d46 strconv: document parsing of leading +/-
+ 2021-06-09 df35ade067 doc/go1.17: document //go:build lines
+ 2021-06-09 e4e7807d24 net/http: add AllowQuerySemicolons
+ 2021-06-09 ec3026d032 doc/go1.17: remove TODO for ports section
+ 2021-06-09 e6dda19888 net/url: reject query values with semicolons
+ 2021-06-09 139e935d3c math/big: comment division
+ 2021-06-09 aa5540cd82 cmd/compile: make map.zero symbol content-addressable
+ 2021-06-09 07ca28d529 cmd/link: fix bug in -strictdups checking of BSS symbols
+ 2021-06-08 bcecae2af6 doc/go1.17: mention new possibility of type conversion panicking
+ 2021-06-08 63dcab2e91 doc/go1.17: mention new vet checks sigchanyzer and stdmethods.
+ 2021-06-08 6551763a60 doc/go1.17: mention block profile bias fix
+ 2021-06-08 cb80937bf6 Revert "doc/go1.17: mention block profile bias fix"
+ 2021-06-08 d3e3d03666 net: reject leading zeros in IP address parsers
+ 2021-06-08 da4a640141 doc/go1.17: revise OpenBSD release notes
+ 2021-06-08 689f4c7415 doc/go1.17: mention block profile bias fix
+ 2021-06-08 9afe071c60 doc/go1.17: remove TODO for Tools section
+ 2021-06-08 f753d7223e doc/go1.17: resolve TODO for cmd/cover
+ 2021-06-08 9498b0155d cmd/go: in Go 1.17+ modules, add indirect go.mod dependencies separately from direct ones
+ 2021-06-08 949f00cebe doc/go1.17: add release notes for crypto packages
+ 2021-06-08 0fb3e2c184 doc/go1.17: add a release note for the '-compat' flag to 'go mod tidy'
+ 2021-06-08 2169deb352 cmd/compile: use t.AllMethods when sorting typesByString
+ 2021-06-08 c20bcb6488 runtime: remove out-of-date comments about frame skipping
+ 2021-06-07 39c39ae52f doc: document Go 1.17 language changes
+ 2021-06-07 dc8b558951 cmd/dist: pass -Wno-lto-type-mismatch in swig_callback_lto
+ 2021-06-07 909dd5e010 strconv: ParseFloat: always return ErrSyntax for bad syntax
Change-Id: Iffdf379d0275bbd12d50149ce38634773ced481d
|
|
tracebackdefers is used for scanning/copying deferred functions'
arguments. Now that deferred functions are always argumentless,
it does nothing. Remove.
Change-Id: I55bedabe5584ea41a12cdb03d55ec9692a5aacd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/325916
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Now that defer/go wrapping is used, deferred/go'd functions are
always argumentless. Remove the code handling arguments.
This CL is mostly removing the fallback code path. There are more
cleanups to be done, in later CLs.
Change-Id: I87bfd3fb2d759fbeb6487b8125c0f6992863d6e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/325915
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@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>
|
