Fixes#23617
Note that this CL does not affect darwin/arm and darwin/arm64,
still TBD what, if anything, needs to be done for those.
This is a fix of CL 105975 which was reverted in CL 106155.
Needed to use movl instead of movq for 386.
Change-Id: I0db7f8087173869e60cc22c6c3124fa0a0739b46
Reviewed-on: https://go-review.googlesource.com/106156
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit 76e92d1c9e.
Reason for revert: Seems to have broken the darwin/386 builder, the toolchain is barfing on the new inline assembly.
Change-Id: Ic83fa3c85148946529c5fd47d1e1669898031ace
Reviewed-on: https://go-review.googlesource.com/106155
Reviewed-by: Keith Randall <khr@golang.org>
After CL 104636 the feature flags in internal/cpu are initialized before
alginit and can now be used for aeshash feature detection. Also remove
now unused runtime variables:
x86:
support_ssse3
support_sse42
support_aes
arm64:
support_aes
Change-Id: I2f64198d91750eaf3c6cf2aac6e9e17615811ec8
Reviewed-on: https://go-review.googlesource.com/106015
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Once upon a time, the iOS exec wrapper needed to change the current
working directory for the binary being tested. To allow that, the
runtime raised a SIGINT signal that the wrapper caught, changed the
working directory and resumed the process.
These days, the current working directory is passed from the wrapper
to the runtime through a special entry in the app metadata and the
SIGINT handshake is not necessary anymore.
Remove the signaling from the runtime and the exec harness.
Change-Id: Ia53bcc9e4724d2ca00207e22b91ce80a05271b55
Reviewed-on: https://go-review.googlesource.com/106096
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
runtime.alginit needs runtime/support_{aes,ssse3,sse41} feature flag
to init aeshash function but internal/cpu.init not be called yet.
This CL will call internal/cpu.initialize before runtime.alginit, so
that we can move all cpu features related code to internal/cpu.
Change-Id: I00b8e403ace3553f8c707563d95f27dade0bc853
Reviewed-on: https://go-review.googlesource.com/104636
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The code reading these variables was removed in CL 41476. They are only
set but never read now, so remove them.
Change-Id: I6b0b8d813e9a3ec2a13586ff92746e00ad1b5bf0
Reviewed-on: https://go-review.googlesource.com/106095
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#23617
Note that this CL does not affect darwin/arm and darwin/arm64,
still TBD what, if anything, needs to be done for those.
Change-Id: Ie1ee02a9f4d4d1fb9cd5dc432d900f926cc157db
Reviewed-on: https://go-review.googlesource.com/105975
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This ensures that gdb tests run on Windows by ignoring any line ending.
Works with gdb 7.7, however with gdb 7.9 and 7.12 gets an error
error:
internal-error: buildsym_init: Assertion `free_pendings == NULL'
failed.
Updates #21380
Change-Id: I6a6e5b2a1b5efdca4dfce009fcb9c134c87497d6
Reviewed-on: https://go-review.googlesource.com/102419
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
There are three places where we wait for the GC mark phase to
complete. Factor these all into a single helper function.
Fixes#24362.
Change-Id: I47f6a7147974f5b9a2869c527a024519070ba6f3
Reviewed-on: https://go-review.googlesource.com/102605
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
adjustpointers loops over a bitmap.
If the length of that bitmap is zero,
we can skip making the call entirely.
This speeds up stack copying when there are
no pointers present in either args or locals.
name old time/op new time/op delta
StackCopyPtr-8 101ms ± 4% 90ms ± 4% -10.95% (p=0.000 n=87+93)
StackCopy-8 80.1ms ± 4% 72.6ms ± 4% -9.41% (p=0.000 n=98+100)
StackCopyNoCache-8 121ms ± 3% 113ms ± 3% -6.57% (p=0.000 n=98+97)
Change-Id: I7a272e19bc9a14fa3e3318771ebd082dc6247d25
Reviewed-on: https://go-review.googlesource.com/104737
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
OpenBSD's __threxit syscall takes a pointer to a 32-bit value that will be
zeroed immediately before the thread exits. Make use of this instead of
zeroing freeWait from the exitThread assembly and using hacks like switching
to a static stack, so this works on 386.
Change-Id: I3ec5ead82b6496404834d148f713794d5d9da723
Reviewed-on: https://go-review.googlesource.com/105055
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The go/printer (and thus gofmt) uses a heuristic to determine
whether to break alignment between elements of an expression
list which is spread across multiple lines. The heuristic only
kicked in if the entry sizes (character length) was above a
certain threshold (20) and the ratio between the previous and
current entry size was above a certain value (4).
This heuristic worked reasonably most of the time, but also
led to unfortunate breaks in many cases where a single entry
was suddenly much smaller (or larger) then the previous one.
The behavior of gofmt was sufficiently mysterious in some of
these situations that many issues were filed against it.
The simplest solution to address this problem is to remove
the heuristic altogether and have a programmer introduce
empty lines to force different alignments if it improves
readability. The problem with that approach is that the
places where it really matters, very long tables with many
(hundreds, or more) entries, may be machine-generated and
not "post-processed" by a human (e.g., unicode/utf8/tables.go).
If a single one of those entries is overlong, the result
would be that the alignment would force all comments or
values in key:value pairs to be adjusted to that overlong
value, making the table hard to read (e.g., that entry may
not even be visible on screen and all other entries seem
spaced out too wide).
Instead, we opted for a slightly improved heuristic that
behaves much better for "normal", human-written code.
1) The threshold is increased from 20 to 40. This disables
the heuristic for many common cases yet even if the alignment
is not "ideal", 40 is not that many characters per line with
todays screens, making it very likely that the entire line
remains "visible" in an editor.
2) Changed the heuristic to not simply look at the size ratio
between current and previous line, but instead considering the
geometric mean of the sizes of the previous (aligned) lines.
This emphasizes the "overall picture" of the previous lines,
rather than a single one (which might be an outlier).
3) Changed the ratio from 4 to 2.5. Now that we ignore sizes
below 40, a ratio of 4 would mean that a new entry would have
to be 4 times bigger (160) or smaller (10) before alignment
would be broken. A ratio of 2.5 seems more sensible.
Applied updated gofmt to all of src and misc. Also tested
against several former issues that complained about this
and verified that the output for the given examples is
satisfactory (added respective test cases).
Some of the files changed because they were not gofmt-ed
in the first place.
For #644.
For #7335.
For #10392.
(and probably more related issues)
Fixes#22852.
Change-Id: I5e48b3d3b157a5cf2d649833b7297b33f43a6f6e
Currently, the runtime falls back to asking for any address the OS can
offer for the heap when it runs out of hint addresses. However, the
race detector assumes the heap lives in [0x00c000000000,
0x00e000000000), and will fail in a non-obvious way if we go outside
this region.
Fix this by actively throwing a useful error if we run out of heap
hints in race mode.
This problem is currently being triggered by TestArenaCollision, which
intentionally triggers this fallback behavior. Fix the test to look
for the new panic message in race mode.
Fixes#24670.
Updates #24133.
Change-Id: I57de6d17a3495dc1f1f84afc382cd18a6efc2bf7
Reviewed-on: https://go-review.googlesource.com/104717
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make the StackCopyNoCache test easier to read.
Add a StackCopyPtr test that actually has some pointers
that need adjusting.
Change-Id: I5b07c26f40cb485c9de97ed63fac89a9e6f36650
Reviewed-on: https://go-review.googlesource.com/104195
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is the first commit of a series that will add WebAssembly
as an architecture target. The design document can be found at
https://docs.google.com/document/d/131vjr4DH6JFnb-blm_uRdaC0_Nv3OUwjEY5qVCxCup4.
The GOARCH name "wasm" is the official abbreviation of WebAssembly.
The GOOS name "js" got chosen because initially the host environment
that executes WebAssembly bytecode will be web browsers and Node.js,
which both use JavaScript to embed WebAssembly. Other GOOS values
may be possible later, see:
https://github.com/WebAssembly/design/blob/master/NonWeb.md
Updates #18892
Change-Id: Ia25b4fa26bba8029c25923f48ad009fd3681933a
Reviewed-on: https://go-review.googlesource.com/102835
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Decode AT_PAGESZ to determine physPageSize on dragonfly.
Change-Id: I7236d7cbe43433f16dffddad19c1655bc0c7f31d
Reviewed-on: https://go-review.googlesource.com/103257
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Decode AT_PAGESZ to determine physPageSize on netbsd.
Also rename vdso_none.go to auxv_none.go which matches its purpose more
closely.
Akin to CL 99780 which did the same for freebsd.
Change-Id: Iea4322f861ff0f3515e9051585dbb442f024326b
Reviewed-on: https://go-review.googlesource.com/102677
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the __vdso_clock_gettime fast path via the vDSO on linux/arm64 to
speed up nanotime and walltime. This results in the following
performance improvement for time.Now on Cavium ThunderX:
name old time/op new time/op delta
TimeNow 442ns ± 0% 163ns ± 0% -63.16% (p=0.000 n=10+10)
And benchmarks on VDSO
BenchmarkClockVDSOAndFallbackPaths/vDSO 10000000 166 ns/op
BenchmarkClockVDSOAndFallbackPaths/Fallback 3000000 456 ns/op
Change-Id: I326118c6dff865eaa0569fc45d1fc1ff95cb74f6
Reviewed-on: https://go-review.googlesource.com/99855
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#24546
Change-Id: I99ebd5bc18e5c5e42eee4689644a7a8b02405f31
Reviewed-on: https://go-review.googlesource.com/102616
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This was a typo mistake according to if cond and runtime/mheap.go:323
Change-Id: Id046d4afbfe0ea43cb29e1a9f400e1f130de221d
Reviewed-on: https://go-review.googlesource.com/102575
Reviewed-by: Austin Clements <austin@google.com>
As found by unparam. Picked the low-hanging fruit, consisting only of
errors that were always nil and results that were never used. Left out
those that were useful for consistency with other func signatures.
Change-Id: I06b52bbd3541f8a5d66659c909bd93cb3e172018
Reviewed-on: https://go-review.googlesource.com/102418
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
As pointed out by Josh Bleecher Snyder in CL 99780.
The check is for GOARM > 6, so suggest to recompile with either GOARM=5
or GOARM=6.
Change-Id: I6a97e87bdc17aa3932f5c8cb598bba85c3cf4be9
Reviewed-on: https://go-review.googlesource.com/101936
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
AT_HWCAP is not available on FreeBSD-11.1-RELEASE or earlier and the wrong const was used.
Use the correct value, and initialize hwcap with ^uint32(0) inorder not to fail the VFP tests.
Fixes#24507.
Change-Id: I5c3eed57bb53bf992b7de0eec88ea959806306b9
Reviewed-on: https://go-review.googlesource.com/102355
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#14327
Much of the code is based on the linux/amd64 code that implements these
build modes, and code is shared where possible.
Change-Id: Ia510f2023768c0edbc863aebc585929ec593b332
Reviewed-on: https://go-review.googlesource.com/93875
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For each replacement, test case is added to new 386enc.s file
with exception of EMMS, SYSENTER, MFENCE and LFENCE as they
are already covered in amd64enc.s (same on amd64 and 386).
The replacement became less obvious after go vet suggested changes
Before:
BYTE $0x0f; BYTE $0x7f; BYTE $0x44; BYTE $0x24; BYTE $0x08
Changed to MOVQ (this form is being tested):
MOVQ M0, 8(SP)
Refactored to FP-relative access (go vet advice):
MOVQ M0, val+4(FP)
Change-Id: I56b87cf3371b6ad81ad0cd9db2033aee407b5818
Reviewed-on: https://go-review.googlesource.com/101475
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Decode AT_PAGESZ to determine physPageSize on freebsd/{386,amd64,arm}
and AT_HWCAP for hwcap and hardDiv on freebsd/arm. Also use hwcap to
perform the FP checks in checkgoarm akin to the linux/arm
implementation.
Change-Id: I532810a1581efe66277e4305cb234acdc79ee91e
Reviewed-on: https://go-review.googlesource.com/99780
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
By moving exported methods to the front of method lists, filtering
down to only the exported methods just needs a count of how many
exported methods exist, which the compiler can statically
provide. This allows getting rid of the exported method cache.
For #22075.
Change-Id: I8eeb274563a2940e1347c34d673f843ae2569064
Reviewed-on: https://go-review.googlesource.com/100846
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When there are plugins, there may not be a unique copy of runtime
functions like goexit, mcall, etc. So identifying them by entry
address is problematic. Instead, keep track of each special function
using a field in the symbol table. That way, multiple copies of
the same runtime function will be treated identically.
Fixes#24351Fixes#23133
Change-Id: Iea3232df8a6af68509769d9ca618f530cc0f84fd
Reviewed-on: https://go-review.googlesource.com/100739
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The syscall package currently declares RawSyscall6 for every GOOS, but
does not define it on Solaris. This leads to code using said function
to compile but it will not link. Fix it by adding RawSyscall6 and make
it panic.
Also remove the obsolete comment above runtime.syscall_syscall as
pointed out by Aram.
Updates #24357
Change-Id: I1b1423121d1c99de2ecc61cd9a935dba9b39e3a4
Reviewed-on: https://go-review.googlesource.com/100655
Reviewed-by: Aram Hăvărneanu <aram@mgk.ro>
In CL 98015, findnull was rewritten so it uses bytes.IndexByte.
This broke the build on plan9/amd64 because the implementation
of bytes.IndexByte on AMD64 relies on SSE instructions while
floating point instructions are not allowed in the note handler.
This change fixes findnull by using the former implementation
on Plan 9, so it doesn't use bytes.IndexByte.
Fixes#24387.
Change-Id: I084d1a44d38d9f77a6c1ad492773f0a98226be16
Reviewed-on: https://go-review.googlesource.com/100577
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@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.
Change-Id: I7ffbd52fbc2a286931a2218038fda52ed6473cc9
Reviewed-on: https://go-review.googlesource.com/99078
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Generated by running gofmt -s on the files in question.
Change-Id: If6578b150e1bfced8657196d2af01f5d36879f93
Reviewed-on: https://go-review.googlesource.com/100135
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, order desugars map assignment operations like
m[k] op= r
into
m[k] = m[k] op r
which in turn is transformed during walk into:
tmp := *mapaccess(m, k)
tmp = tmp op r
*mapassign(m, k) = tmp
However, this is suboptimal, as we could instead produce just:
*mapassign(m, k) op= r
One complication though is if "r == 0", then "m[k] /= r" and "m[k] %=
r" will panic, and they need to do so *before* calling mapassign,
otherwise we may insert a new zero-value element into the map.
It would be spec compliant to just emit the "r != 0" check before
calling mapassign (see #23735), but currently these checks aren't
generated until SSA construction. For now, it's simpler to continue
desugaring /= and %= into two map indexing operations.
Fixes#23661.
Change-Id: I46e3739d9adef10e92b46fdd78b88d5aabe68952
Reviewed-on: https://go-review.googlesource.com/91557
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The implementation of runtime.abort on arm64 currently branches to
address 0, which results in a signal from PC 0, rather than from
runtime.abort, so the runtime fails to recognize it as an abort.
Fix runtime.abort on arm64 to read from address 0 like what other
architectures do and recognize this in the signal handler.
Should fix the linux/arm64 build.
Change-Id: I960ab630daaeadc9190287604d4d8337b1ea3853
Reviewed-on: https://go-review.googlesource.com/99895
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
bytes.IndexByte is heavily optimized. Use it in findnull.
This is second attempt, similar to CL97523.
In this version we never call IndexByte on region of memory,
that crosses page boundary. A bit slower than CL97523,
but still fast:
name old time/op new time/op delta
GoString-6 164ns ± 2% 118ns ± 0% -28.00% (p=0.000 n=10+6)
findnull is also used in gostringnocopy,
which is used in many hot spots in the runtime.
Fixes#23830
Change-Id: Id843dd4f65a34309d92bdd8df229e484d26b0cb2
Reviewed-on: https://go-review.googlesource.com/98015
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This lets SIGPROF signals get a useful traceback.
Without it we just see sysvicallN calling asmcgocall.
Updates #24142
Change-Id: I5dfe3add51f0c3a4cb1c98acb7738be6396214bc
Reviewed-on: https://go-review.googlesource.com/99617
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Borrowed from cmd/compile, TestSizeof ensures
that the size of important types doesn't change unexpectedly.
It also helps reviewers see the impact of intended changes.
Change-Id: If57955f0c3e66054de3f40c6bba585b88694c7be
Reviewed-on: https://go-review.googlesource.com/99837
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
hwcap is set in archauxv, setup_auxv no longer exists.
Change-Id: I0fc9393e0c1c45192e0eff4715e9bdd69fab2653
Reviewed-on: https://go-review.googlesource.com/99779
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It's a bit mysterious that _defer.sp is a uintptr that gets
stack-adjusted explicitly while _panic.argp is an unsafe.Pointer that
doesn't, but turns out to be critically important when a deferred
function grows the stack before doing a recover.
Add a comment explaining that this works because _panic values live on
the stack. Enforce this by marking _panic go:notinheap.
Change-Id: I9ca49e84ee1f86d881552c55dccd0662b530836b
Reviewed-on: https://go-review.googlesource.com/99735
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
On all non-x86 arches, runtime.abort simply reads from nil.
Unfortunately, if this happens on a user stack, the signal handler
will dutifully turn this into a panicmem, which lets user defers run
and which user code can even recover from.
To fix this, add an explicit check to the signal handler that turns
faults in abort into hard crashes directly in the signal handler. This
has the added benefit of giving a register dump at the abort point.
Change-Id: If26a7f13790745ee3867db7f53b72d8281176d70
Reviewed-on: https://go-review.googlesource.com/93661
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Everything except for amd64, amd64p32, and 386 currently defines and
uses an abort function. This CL makes these match. The next CL will
recognize the abort function to make this more useful.
Change-Id: I7c155871ea48919a9220417df0630005b444f488
Reviewed-on: https://go-review.googlesource.com/93660
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently, throw may grow the stack, which means whenever we call it
from a context where it's not safe to grow the stack, we first have to
switch to the system stack. This is pretty easy to get wrong.
Fix this by making throw switch to the system stack so it doesn't grow
the stack and is hence safe to call without a system stack switch at
the call site.
The only thing this complicates is badsystemstack itself, which would
now go into an infinite loop before printing anything (previously it
would also go into an infinite loop, but would at least print the
error first). Fix this by making badsystemstack do a direct write and
then crash hard.
Change-Id: Ic5b4a610df265e47962dcfa341cabac03c31c049
Reviewed-on: https://go-review.googlesource.com/93659
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently parts of unrecoverable panic handling (notably, printing
panic messages) can happen on the user stack. This may grow the stack,
which is generally fine, but if we're handling a runtime panic, it's
better to do as little as possible in case the runtime is in an
inconsistent state.
Hence, this commit rearranges the handling of unrecoverable panics so
that it's done entirely on the system stack.
This is mostly a matter of shuffling code a bit so everything can move
into a systemstack block. The one slight subtlety is in the "panic
during panic" case, where we now depend on startpanic_m's caller to
print the stack rather than startpanic_m itself. To make this work,
startpanic_m now returns a boolean indicating that the caller should
avoid trying to print any panic messages and get right to the stack
trace. Since the caller is already in a position to do this, this
actually simplifies things a little.
Change-Id: Id72febe8c0a9fb31d9369b600a1816d65a49bfed
Reviewed-on: https://go-review.googlesource.com/93658
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently if a profiling signal arrives while executing within a VDSO
the profiler will report _ExternalCode, which is needlessly confusing
for a pure Go program. Change the VDSO calling code to record the
caller's PC/SP, so that we can do a traceback from that point. If that
fails for some reason, report _VDSO rather than _ExternalCode, which
should at least point in the right direction.
This adds some instructions to the code that calls the VDSO, but the
slowdown is reasonably negligible:
name old time/op new time/op delta
ClockVDSOAndFallbackPaths/vDSO-8 40.5ns ± 2% 41.3ns ± 1% +1.85% (p=0.002 n=10+10)
ClockVDSOAndFallbackPaths/Fallback-8 41.9ns ± 1% 43.5ns ± 1% +3.84% (p=0.000 n=9+9)
TimeNow-8 41.5ns ± 3% 41.5ns ± 2% ~ (p=0.723 n=10+10)
Fixes#24142
Change-Id: Iacd935db3c4c782150b3809aaa675a71799b1c9c
Reviewed-on: https://go-review.googlesource.com/97315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This normalizes the Linux code to act like other targets. The size
argument to the rt_sigaction system call is pushed to a single
function, sysSigaction.
This is intended as a simplification step for CL 93875 for #14327.
Change-Id: I594788e235f0da20e16e8a028e27ac8c883907c4
Reviewed-on: https://go-review.googlesource.com/99077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
While working on standalone builds of gomobile bindings, I ran into
errors on the form:
gcc_darwin_arm.c:30:31: error: ambiguous expansion of macro 'nil' [-Werror,-Wambiguous-macro]
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS11.2.sdk/usr/include/MacTypes.h:94:15: note: expanding this definition of 'nil'
Fix it by undefining nil before defining it in libcgo.h.
Change-Id: I8e9660a68c6c351e592684d03d529f0d182c0493
Reviewed-on: https://go-review.googlesource.com/99215
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
They do not match the file name patterns of
*_GOOS
*_GOARCH
*_GOOS_GOARCH
therefore the implicit linux constraint was not being added.
Change-Id: Ie506c51cee6818db445516f96fffaa351df62cf5
Reviewed-on: https://go-review.googlesource.com/99116
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously, for slow map key types (i.e., any type other than a 32-bit
or 64-bit plain memory type), we would rewrite
defer delete(m, k)
into
ktmp := k
defer delete(m, &ktmp)
However, if the defer statement was inside a loop, we would end up
reusing the same ktmp value for all of the deferred deletes.
We already rewrite
defer print(x, y, z)
into
defer func(a1, a2, a3) {
print(a1, a2, a3)
}(x, y, z)
This CL generalizes this rewrite to also apply for slow map deletes.
This could be extended to apply even more generally to other builtins,
but as discussed on #24259, there are cases where we must *not* do
this (e.g., "defer recover()"). However, if we elect to do this more
generally, this CL should still make that easier.
Lastly, while here, fix a few isues in wrapCall (nee walkprintfunc):
1) lookupN appends the generation number to the symbol anyway, so "%d"
was being literally included in the generated function names.
2) walkstmt will be called when the function is compiled later anyway,
so no need to do it now.
Fixes#24259.
Change-Id: I70286867c64c69c18e9552f69e3f4154a0fc8b04
Reviewed-on: https://go-review.googlesource.com/99017
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 98095 got the check wrong. We should be testing
'getg() == getg().m.curg', not 'getg().m == getg().m.curg'.
Change-Id: I32f6238b00409b67afa8efe732513d542aec5bc7
Reviewed-on: https://go-review.googlesource.com/98855
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is an updated version of golang.org/cl/96395, with the fix to
TestUserSpan.
This reverts commit 7b6f6267e90a8e4eab37a3f2164ba882e6222adb.
Change-Id: I31eec8ba0997f9178dffef8dac608e731ab70872
Reviewed-on: https://go-review.googlesource.com/98236
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This was originally C code using names with underscores, which were
retained when the code was rewritten into Go. Change the code to use
Go-like camel case names.
The names that come from the ELF ABI are left unchanged.
Change-Id: I181bc5dd81284c07bc67b7df4635f4734b41d646
Reviewed-on: https://go-review.googlesource.com/98520
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also fix the indentation of the SYS_* definitions in sys_linux_mipsx.s
and order them numerically.
Change-Id: I0c454301c329a163e7db09dcb25d4e825149858c
Reviewed-on: https://go-review.googlesource.com/98448
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use the __vdso_clock_gettime fast path via the vDSO on linux/arm to
speed up nanotime and walltime. This results in the following
performance improvement for time.Now on a RaspberryPi 3 (running
32bit Raspbian, i.e. GOOS=linux/GOARCH=arm):
name old time/op new time/op delta
TimeNow 0.99µs ± 0% 0.39µs ± 1% -60.74% (p=0.000 n=12+20)
Change-Id: I3598278a6c88d7f6a6ce66c56b9d25f9dd2f4c9a
Reviewed-on: https://go-review.googlesource.com/98095
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Move the IndexByte function from the runtime to a new bytealg package.
The new package will eventually hold all the optimized assembly for
groveling through byte slices and strings. It seems a better home for
this code than randomly keeping it in runtime.
Once this is in, the next step is to move the other functions
(Compare, Equal, ...).
Update #19792
This change seems complicated enough that we might just declare
"not worth it" and abandon. Opinions welcome.
The core assembly is all unchanged, except minor modifications where
the code reads cpu feature bits.
The wrapper functions have been cleaned up as they are now actually
checked by vet.
Change-Id: I9fa75bee5d85db3a65b3fd3b7997e60367523796
Reviewed-on: https://go-review.googlesource.com/98016
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 7365fac2db.
Reason for revert: breaks the build on some architectures, reading unmapped pages?
Change-Id: I3a8c02dc0b649269faacea79ecd8213defa97c54
Reviewed-on: https://go-review.googlesource.com/97995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
bytes.IndexByte is heavily optimized.
Use it in findnull.
name old time/op new time/op delta
GoString-8 65.5ns ± 1% 40.2ns ± 1% -38.62% (p=0.000 n=19+19)
findnull is also used in gostringnocopy,
which is used in many hot spots in the runtime.
Fixes#23830
Change-Id: I2e6cb279c7d8078f8844065de684cc3567fe89d7
Reviewed-on: https://go-review.googlesource.com/97523
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: I030baaa0a0abf1e43449faaf676d389a28a868a3
Reviewed-on: https://go-review.googlesource.com/97857
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
When the slice/string length is very large,
probably artifically large as in CL 97523,
adding BX (length) to R11 (pointer) overflows.
As a result, checking DI < R11 yields the wrong result.
Since they will be equal when the loop is done,
just check DI != R11 instead.
Yes, the pointer itself could overflow, but if that happens,
something else has gone pretty wrong; not our concern here.
Fixes#24187
Change-Id: I2f60fc6ccae739345d01bc80528560726ad4f8c6
Reviewed-on: https://go-review.googlesource.com/97802
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
All functions defined in syscall2_solaris.go have the respective libc_*
var in syscall_solaris.go, except for libc_close. Move it from
os3_solaris.go
Remove unused libc_fstat.
Order go:cgo_import_dynamic and go:linkname lists in
syscall2_solaris.go alphabetically.
Change-Id: I9f12fa473cf1ae351448ac45597c82a67d799c31
Reviewed-on: https://go-review.googlesource.com/97736
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Plan 9 won't let brk shrink the data segment if it's shared with
other processes (which it is in the go runtime). So we keep track
of the notional end of the segment as it moves up and down, and
call brk only when it grows.
Corrects CL 94776.
Updates #23860.
Fixes#24013.
Change-Id: I754232decab81dfd71d690f77ee6097a17d9be11
Reviewed-on: https://go-review.googlesource.com/97595
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The first word of an interface is a pointer, but for the purposes
of GC we don't need to treat it as such.
1. If it is a non-empty interface, the pointer points to an itab
which is always in persistentalloc space.
2. If it is an empty interface, the pointer points to a _type.
a. If it is a compile-time-allocated type, it points into
the read-only data section.
b. If it is a reflect-allocated type, it points into the Go heap.
Reflect is responsible for keeping a reference to
the underlying type so it won't be GCd.
If we ever have a moving GC, we need to change this for 2b (as
well as scan itabs to update their itab._type fields).
Write barriers on the first word of interfaces have already been removed.
Change-Id: I643e91d7ac4de980ac2717436eff94097c65d959
Reviewed-on: https://go-review.googlesource.com/97518
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Minor improvements, noticed while investigating other things.
Shorten the prologue.
Make branch direction better for static branch prediction;
the most common case by far is switching stacks (g==curg).
Change-Id: Ib2211d3efecb60446355cda56194221ccb78057d
Reviewed-on: https://go-review.googlesource.com/97377
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
They have either already been called by preprintpanics, or they can
not be called safely because of the various conditions checked at the
start of gopanic.
Fixes#24059
Change-Id: I4a6233d12c9f7aaaee72f343257ea108bae79241
Reviewed-on: https://go-review.googlesource.com/96755
Reviewed-by: Austin Clements <austin@google.com>
Currently, we use 64MB heap arenas on 64-bit platforms. This works
well on UNIX-like OSes because they treat untouched pages as
essentially free. However, on Windows, committed memory is charged
against a process whether or not it has demand-faulted physical pages
in. Hence, on Windows, even a process with a tiny heap will commit
64MB for one heap arena, plus another 32MB for the arena map. Things
are much worse under the race detector, which increases the heap
commitment by a factor of 5.5X, leading to 384MB of committed memory
at runtime init.
Fix this by reducing the heap arena size to 4MB on Windows.
To counterbalance the effect of increasing the arena map size by a
factor of 16, and to further reduce the impact of the commitment for
the arena map, we switch from a single entry L1 arena map to a 64
entry L1 arena map.
Compared to the original arena design, this slows down the
x/benchmarks garbage benchmark by 0.49% (the slow down of this commit
alone is 1.59%, but the previous commit bought us a 1% speed-up):
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.28ms ± 1% 2.29ms ± 1% +0.49% (p=0.000 n=17+18)
(https://perf.golang.org/search?q=upload:20180223.1)
(This was measured on linux/amd64 by modifying its arena configuration
as above.)
Fixes#23900.
Change-Id: I6b7fa5ecebee2947bf20cfeb78c248809469c6b1
Reviewed-on: https://go-review.googlesource.com/96780
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, the heap arena map is a single, large array that covers
every possible arena frame in the entire address space. This is
practical up to about 48 bits of address space with 64 MB arenas.
However, there are two problems with this:
1. mips64, ppc64, and s390x support full 64-bit address spaces (though
on Linux only s390x has kernel support for 64-bit address spaces).
On these platforms, it would be good to support these larger
address spaces.
2. On Windows, processes are charged for untouched memory, so for
processes with small heaps, the mostly-untouched 32 MB arena map
plus a 64 MB arena are significant overhead. Hence, it would be
good to reduce both the arena map size and the arena size, but with
a single-level arena, these are inversely proportional.
This CL adds support for a two-level arena map. Arena frame numbers
are now divided into arenaL1Bits of L1 index and arenaL2Bits of L2
index.
At the moment, arenaL1Bits is always 0, so we effectively have a
single level map. We do a few things so that this has no cost beyond
the current single-level map:
1. We embed the L2 array directly in mheap, so if there's a single
entry in the L2 array, the representation is identical to the
current representation and there's no extra level of indirection.
2. Hot code that accesses the arena map is structured so that it
optimizes to nearly the same machine code as it does currently.
3. We make some small tweaks to hot code paths and to the inliner
itself to keep some important functions inlined despite their
now-larger ASTs. In particular, this is necessary for
heapBitsForAddr and heapBits.next.
Possibly as a result of some of the tweaks, this actually slightly
improves the performance of the x/benchmarks garbage benchmark:
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.28ms ± 1% 2.26ms ± 1% -1.07% (p=0.000 n=17+19)
(https://perf.golang.org/search?q=upload:20180223.2)
For #23900.
Change-Id: If5164e0961754f97eb9eca58f837f36d759505ff
Reviewed-on: https://go-review.googlesource.com/96779
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
There are too many places where I want to talk about "indexing into
the arena index". Make this less awkward and ambiguous by calling it
the "arena map" instead.
Change-Id: I726b0667bb2139dbc006175a0ec09a871cdf73f9
Reviewed-on: https://go-review.googlesource.com/96777
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
On amd64, the arena is no longer in address space order, but currently
the heap dumper assumes that it is. Fix this assumption.
Change-Id: Iab1953cd36b359d0fb78ed49e5eb813116a18855
Reviewed-on: https://go-review.googlesource.com/96776
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
There was still the word 'Hashmap' in gc_test.go, so I renamed it to just 'Map'
Previous renaming commit: https://golang.org/cl/90336
Change-Id: I5b0e5c2229d1c30937c7216247f4533effb81ce7
Reviewed-on: https://go-review.googlesource.com/96675
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Now that we support the full non-contiguous virtual address space of
amd64 hardware, some of the comments and constants related to this are
out of date.
This renames memLimitBits to heapAddrBits because 1<<memLimitBits is
no longer the limit of the address space and rewrites the comment to
focus first on hardware limits (which span OSes) and then discuss
kernel limits.
Second, this eliminates the memLimit constant because there's no
longer a meaningful "highest possible heap pointer value" on amd64.
Updates #23862.
Change-Id: I44b32033d2deb6b69248fb8dda14fc0e65c47f11
Reviewed-on: https://go-review.googlesource.com/95498
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
On amd64, the virtual address space, when interpreted as signed
values, is [-2^47, 2^47). Currently, we only support heap addresses in
the "positive" half of this, [0, 2^47). This suffices for linux/amd64
and windows/amd64, but solaris/amd64 can map user addresses in the
negative part of this range. Specifically, addresses
0xFFFF8000'00000000 to 0xFFFFFD80'00000000 are part of user space.
This leads to "memory allocated by OS not in usable address space"
panic, since we don't map heap arena index space for these addresses.
Fix this by offsetting addresses when computing arena indexes so that
arena entry 0 corresponds to address -2^47 on amd64. We already map
enough arena space for 2^48 heap addresses on 64-bit (because arm64's
virtual address space is [0, 2^48)), so we don't need to grow any
structures to support this.
A different approach would be to simply mask out the top 16 bits.
However, there are two advantages to the offset approach: 1) invalid
heap addresses continue to naturally map to invalid arena indexes so
we don't need extra checks and 2) it perturbs the mapping of addresses
to arena indexes more, which helps check that we don't accidentally
compute incorrect arena indexes somewhere that happen to be right most
of the time.
Several comments and constant names are now somewhat misleading. We'll
fix that in the next CL. This CL is the core change the arena
indexing.
Fixes#23862.
Change-Id: Idb8e299fded04593a286b01a9582da6ddbac2f9a
Reviewed-on: https://go-review.googlesource.com/95497
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Accessing the arena index is about to get slightly more complicated.
Abstract this away into a set of functions for going back and forth
between addresses and arena slice indexes.
For #23862.
Change-Id: I0b20e74ef47a07b78ed0cf0a6128afe6f6e40f4b
Reviewed-on: https://go-review.googlesource.com/95496
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently, bulkBarrierPreWrite uses inheap to decide whether the
destination is in the heap or whether to check for stack or global
data. However, this isn't the best question to ask.
Instead, get the span directly and query its state. This lets us
directly determine whether this might be a global, or is stack memory,
or is heap memory.
At this point, inheap is no longer used in the hot path, so drop it
from the must-be-inlined list and substitute spanOf.
This will help in a circuitous way with #23862, since fixing that is
going to push inheap very slightly over the inline-able threshold on a
few platforms.
Change-Id: I5360fc1181183598502409f12979899e1e4d45f7
Reviewed-on: https://go-review.googlesource.com/95495
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
We don't want to account the memory for mheap_.arenas because most of
it is never touched, so currently we pass the address of a uint64 on
the heap. However, at least on mips, it's possible for this uint64 to
be unaligned, which causes the atomic add in mSysStatInc to crash.
Fix this by instead passing a nil stat pointer.
Fixes#23946.
Change-Id: I091587df1b3066c330b6bb4d834e4596c407910f
Reviewed-on: https://go-review.googlesource.com/95695
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
reflect.unsafe_New is an often called function according
to profiling in a large production environment.
Since newobject is not inlined currently there
is call overhead that can be avoided by calling
mallocgc directly.
name old time/op new time/op delta
New 32.4ns ± 2% 29.8ns ± 1% -8.03% (p=0.000 n=19+20)
Change-Id: I572e4be830ed8e5c0da555dc3a8864c8363112be
Reviewed-on: https://go-review.googlesource.com/95015
Reviewed-by: Austin Clements <austin@google.com>
Avoid using make in gobytes which clears the byte slice backing
array unnecessarily since the content is overwritten immediately again.
Check that the user provided length is positive and below the maximum
allowed allocation size explicitly in gobytes as this was done in makeslice
before this change.
Fixes#23634
Change-Id: Id852619e932aabfc468871c42ad07d34da91f45c
Reviewed-on: https://go-review.googlesource.com/94760
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Follow CL 93655 which removed the (commented-out) usage of this
function.
Also remove unused constant _RLIMIT_AS and type rlimit.
Change-Id: Ifb6e6b2104f4c2555269f8ced72bfcae24f5d5e9
Reviewed-on: https://go-review.googlesource.com/94775
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Overall code is unchanged.
The functions for different types (32, 64, str) of map fast routines
are collected in map_fast.go that has grown to ~1300 lines.
Moving the functions for each map fast type into a separate file
allows for an easier overview and navigation within the map code.
Change-Id: Ic09e4212f9025a66a10b11ef8dac23ad49d1d5ae
Reviewed-on: https://go-review.googlesource.com/90335
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Rename all map implementation and test files to use "map"
as a file name prefix instead of "hashmap" for the implementation
and "map" for the test file names.
Change-Id: I7b317c1f7a660b95c6d1f1a185866f2839e69446
Reviewed-on: https://go-review.googlesource.com/90336
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On Plan 9, sysReserve was ignoring the address hint and allocating
memory wherever it is available. This causes the new
TestArenaCollision test to fail on 32-bit Plan 9. We now use the
address hint in the specific case where sysReserve is extending the
process address space at its end, and similarly we contract the
address space in the case where sysFree is releasing memory at
the end.
Fixes#23860
Change-Id: Ia5254779ba8f1698c999832720a88de400b5f91a
Reviewed-on: https://go-review.googlesource.com/94776
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Replace the test for nacl with testenv.MustHaveExec to also skip
test on iOS.
Change-Id: I6822714f6d71533d1b18bbb7894f6ad339d8aea1
Reviewed-on: https://go-review.googlesource.com/94755
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Now that we have memLimit, also having _MaxMem is a bit confusing.
Replace it with maxAlloc, which better conveys what it limits. We also
define maxAlloc slightly differently: since it's now clear that it
limits allocation size, we can account for a subtle difference between
32-bit and 64-bit.
Change-Id: Iac39048018cc0dae7f0919e25185fee4b3eed529
Reviewed-on: https://go-review.googlesource.com/85890
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently there's a detailed comment in lfstack_64bit.go about address
space limitations on various architectures. Since that's now relevant
to malloc, move it to a more prominent place in the documentation for
memLimitBits.
Updates #10460.
Change-Id: If9708291cf3a288057b8b3ba0ba6a59e3602bbd6
Reviewed-on: https://go-review.googlesource.com/85889
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently large sysReserve calls on some OSes don't actually reserve
the memory, but just check that it can be reserved. This was important
when we called sysReserve to "reserve" many gigabytes for the heap up
front, but now that we map memory in small increments as we need it,
this complication is no longer necessary.
This has one curious side benefit: currently, on Linux, allocations
that are large enough to be rejected by mmap wind up freezing the
application for a long time before it panics. This happens because
sysReserve doesn't reserve the memory, so sysMap calls mmap_fixed,
which calls mmap, which fails because the mapping is too large.
However, mmap_fixed doesn't inspect *why* mmap fails, so it falls back
to probing every page in the desired region individually with mincore
before performing an (otherwise dangerous) MAP_FIXED mapping, which
will also fail. This takes a long time for a large region. Now this
logic is gone, so the mmap failure leads to an immediate panic.
Updates #10460.
Change-Id: I8efe88c611871cdb14f99fadd09db83e0161ca2e
Reviewed-on: https://go-review.googlesource.com/85888
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This replaces the contiguous heap arena mapping with a potentially
sparse mapping that can support heap mappings anywhere in the address
space.
This has several advantages over the current approach:
* There is no longer any limit on the size of the Go heap. (Currently
it's limited to 512GB.) Hence, this fixes#10460.
* It eliminates many failures modes of heap initialization and
growing. In particular it eliminates any possibility of panicking
with an address space conflict. This can happen for many reasons and
even causes a low but steady rate of TSAN test failures because of
conflicts with the TSAN runtime. See #16936 and #11993.
* It eliminates the notion of "non-reserved" heap, which was added
because creating huge address space reservations (particularly on
64-bit) led to huge process VSIZE. This was at best confusing and at
worst conflicted badly with ulimit -v. However, the non-reserved
heap logic is complicated, can race with other mappings in non-pure
Go binaries (e.g., #18976), and requires that the entire heap be
either reserved or non-reserved. We currently maintain the latter
property, but it's quite difficult to convince yourself of that, and
hence difficult to keep correct. This logic is still present, but
will be removed in the next CL.
* It fixes problems on 32-bit where skipping over parts of the address
space leads to mapping huge (and never-to-be-used) metadata
structures. See #19831.
This also completely rewrites and significantly simplifies
mheap.sysAlloc, which has been a source of many bugs. E.g., #21044,
#20259, #18651, and #13143 (and maybe #23222).
This change also makes it possible to allocate individual objects
larger than 512GB. As a result, a few tests that expected huge
allocations to fail needed to be changed to make even larger
allocations. However, at the moment attempting to allocate a humongous
object may cause the program to freeze for several minutes on Linux as
we fall back to probing every page with addrspace_free. That logic
(and this failure mode) will be removed in the next CL.
Fixes#10460.
Fixes#22204 (since it rewrites the code involved).
This slightly slows down compilebench and the x/benchmarks garbage
benchmark.
name old time/op new time/op delta
Template 184ms ± 1% 185ms ± 1% ~ (p=0.065 n=10+9)
Unicode 86.9ms ± 3% 86.3ms ± 1% ~ (p=0.631 n=10+10)
GoTypes 599ms ± 0% 602ms ± 0% +0.56% (p=0.000 n=10+9)
Compiler 2.87s ± 1% 2.89s ± 1% +0.51% (p=0.002 n=9+10)
SSA 7.29s ± 1% 7.25s ± 1% ~ (p=0.182 n=10+9)
Flate 118ms ± 2% 118ms ± 1% ~ (p=0.113 n=9+9)
GoParser 147ms ± 1% 148ms ± 1% +1.07% (p=0.003 n=9+10)
Reflect 401ms ± 1% 404ms ± 1% +0.71% (p=0.003 n=10+9)
Tar 175ms ± 1% 175ms ± 1% ~ (p=0.604 n=9+10)
XML 209ms ± 1% 210ms ± 1% ~ (p=0.052 n=10+10)
(https://perf.golang.org/search?q=upload:20171231.4)
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.23ms ± 1% 2.25ms ± 1% +0.84% (p=0.000 n=19+19)
(https://perf.golang.org/search?q=upload:20171231.3)
Relative to the start of the sparse heap changes (starting at and
including "runtime: fix various contiguous bitmap assumptions"),
overall slowdown is roughly 1% on GC-intensive benchmarks:
name old time/op new time/op delta
Template 183ms ± 1% 185ms ± 1% +1.32% (p=0.000 n=9+9)
Unicode 84.9ms ± 2% 86.3ms ± 1% +1.65% (p=0.000 n=9+10)
GoTypes 595ms ± 1% 602ms ± 0% +1.19% (p=0.000 n=9+9)
Compiler 2.86s ± 0% 2.89s ± 1% +0.91% (p=0.000 n=9+10)
SSA 7.19s ± 0% 7.25s ± 1% +0.75% (p=0.000 n=8+9)
Flate 117ms ± 1% 118ms ± 1% +1.10% (p=0.000 n=10+9)
GoParser 146ms ± 2% 148ms ± 1% +1.48% (p=0.002 n=10+10)
Reflect 398ms ± 1% 404ms ± 1% +1.51% (p=0.000 n=10+9)
Tar 173ms ± 1% 175ms ± 1% +1.17% (p=0.000 n=10+10)
XML 208ms ± 1% 210ms ± 1% +0.62% (p=0.011 n=10+10)
[Geo mean] 369ms 373ms +1.17%
(https://perf.golang.org/search?q=upload:20180101.2)
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.22ms ± 1% 2.25ms ± 1% +1.51% (p=0.000 n=20+19)
(https://perf.golang.org/search?q=upload:20180101.3)
Change-Id: I5daf4cfec24b252e5a57001f0a6c03f22479d0f0
Reviewed-on: https://go-review.googlesource.com/85887
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This replaces all uses of the mheap_.arena_* fields outside of
mallocinit and sysAlloc. These fields fundamentally assume a
contiguous heap between two bounds, so eliminating these is necessary
for a sparse heap.
Many of these are replaced with checks for non-nil spans at the test
address (which in turn checks for a non-nil entry in the heap arena
array). Some of them are just for debugging and somewhat meaningless
with a sparse heap, so those we just delete.
Updates #10460.
Change-Id: I8345b95ffc610aed694f08f74633b3c63506a41f
Reviewed-on: https://go-review.googlesource.com/85886
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This abstracts the remaining direct accesses to mheap.spans into new
mheap.setSpan and mheap.setSpans methods.
For #10460.
Change-Id: Id1db8bc5e34a77a9221032aa2e62d05322707364
Reviewed-on: https://go-review.googlesource.com/85884
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This splits the heap bitmap into separate chunks for every 64MB of the
heap and introduces an index mapping from virtual address to metadata.
It modifies the heapBits abstraction to use this two-level structure.
Finally, it modifies heapBitsSetType to unroll the bitmap into the
object itself and then copy it out if the bitmap would span
discontiguous bitmap chunks.
This is a step toward supporting general sparse heaps, which will
eliminate address space conflict failures as well as the limit on the
heap size.
It's also advantageous for 32-bit. 32-bit already supports
discontiguous heaps by always starting the arena at address 0.
However, as a result, with a contiguous bitmap, if the kernel chooses
a high address (near 2GB) for a heap mapping, the runtime is forced to
map up to 128MB of heap bitmap. Now the runtime can map sections of
the bitmap for just the parts of the address space used by the heap.
Updates #10460.
This slightly slows down the x/garbage and compilebench benchmarks.
However, I think the slowdown is acceptably small.
name old time/op new time/op delta
Template 178ms ± 1% 180ms ± 1% +0.78% (p=0.029 n=10+10)
Unicode 85.7ms ± 2% 86.5ms ± 2% ~ (p=0.089 n=10+10)
GoTypes 594ms ± 0% 599ms ± 1% +0.70% (p=0.000 n=9+9)
Compiler 2.86s ± 0% 2.87s ± 0% +0.40% (p=0.001 n=9+9)
SSA 7.23s ± 2% 7.29s ± 2% +0.94% (p=0.029 n=10+10)
Flate 116ms ± 1% 117ms ± 1% +0.99% (p=0.000 n=9+9)
GoParser 146ms ± 1% 146ms ± 0% ~ (p=0.193 n=10+7)
Reflect 399ms ± 0% 403ms ± 1% +0.89% (p=0.001 n=10+10)
Tar 173ms ± 1% 174ms ± 1% +0.91% (p=0.013 n=10+9)
XML 208ms ± 1% 210ms ± 1% +0.93% (p=0.000 n=10+10)
[Geo mean] 368ms 371ms +0.79%
name old time/op new time/op delta
Garbage/benchmem-MB=64-12 2.17ms ± 1% 2.21ms ± 1% +2.15% (p=0.000 n=20+20)
Change-Id: I037fd283221976f4f61249119d6b97b100bcbc66
Reviewed-on: https://go-review.googlesource.com/85883
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
There are various places that assume the heap bitmap is contiguous and
scan it sequentially. We're about to split up the heap bitmap. This
commit modifies all of these except heapBitsSetType to use the
heapBits abstractions so they can transparently switch to a
discontiguous bitmap.
Updates #10460. This is a step toward supporting sparse heaps.
Change-Id: I2f3994a5785e4dccb66602fb3950bbd290d9392c
Reviewed-on: https://go-review.googlesource.com/85882
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently the heap bitamp is laid in reverse order in memory relative
to the heap itself. This was originally done out of "excessive
cleverness" so that computing a bitmap pointer could load only the
arena_start field and so that heaps could be more contiguous by
growing the arena and the bitmap out from a common center point.
However, this appears to have no actual performance benefit, it
complicates nearly every use of the bitmap, and it makes already
confusing code more confusing. Furthermore, it's still possible to use
a single field (the new bitmap_delta) for the bitmap pointer
computation by employing slightly different excessive cleverness.
Hence, this CL puts the bitmap into forward order.
This is a (very) updated version of CL 9404.
Change-Id: I743587cc626c4ecd81e660658bad85b54584108c
Reviewed-on: https://go-review.googlesource.com/85881
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The logic in the spanOf* functions is open-coded in a lot of places
right now. Replace these with calls to the spanOf* functions.
Change-Id: I3cc996aceb9a529b60fea7ec6fef22008c012978
Reviewed-on: https://go-review.googlesource.com/85880
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
I think we'd forgotten about the mheap.lookup APIs when we introduced
spanOf*, but, at any rate, the spanOf* functions are used far more
widely at this point, so this CL eliminates the mheap.lookup*
functions in favor of spanOf*.
Change-Id: I15facd0856e238bb75d990e838a092b5bef5bdfc
Reviewed-on: https://go-review.googlesource.com/85879
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
heapBitsForObject does two things: it finds the base of the object and
it creates the heapBits for the base of the object. There are several
places where we just care about the base of the object. Furthermore,
greyobject only needs the heapBits in the checkmark path and can
easily compute them only when needed. Once we eliminate passing the
heap bits to grayobject, almost all uses of heapBitsForObject don't
need the heap bits.
Hence, this splits heapBitsForObject into findObject and
heapBitsForAddr (the latter already exists), removes the hbits
argument to grayobject, and replaces all heapBitsForObject calls with
calls to findObject.
In addition to making things cleaner overall, heapBitsForAddr is going
to get more expensive shortly, so it's important that we don't do it
needlessly.
Note that there's an interesting performance pitfall here. I had
originally moved findObject to mheap.go, since it made more sense
there. However, that leads to a ~2% slow down and a whopping 11%
increase in L1 icache misses on both the x/garbage and compilebench
benchmarks. This suggests we may want to be more principled about
this, but, for now, let's just leave findObject in mbitmap.go.
(I tried to make findObject small enough to inline by splitting out
the error case, but, sadly, wasn't quite able to get it under the
inlining budget.)
Change-Id: I7bcb92f383ade565d22a9f2494e4c66fd513fb10
Reviewed-on: https://go-review.googlesource.com/85878
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
These functions all serve essentially the same purpose. mlookup is
used in only one place and findObject in only three. Use
heapBitsForObject instead, which is the most optimized implementation.
(This may seem slightly silly because none of these uses care about
the heap bits, but we're about to split up the functionality of
heapBitsForObject anyway. At that point, findObject will rise from the
ashes.)
Change-Id: I906468c972be095dd23cf2404a7d4434e802f250
Reviewed-on: https://go-review.googlesource.com/85877
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
I was spelunking Linux's address space code and found that some of the
information about maximum virtual addresses in lfstack's comments was
out of date. This expands and updates the comment.
Change-Id: I9f54b23e6b266b3c5cc20259a849231fb751f6e7
Reviewed-on: https://go-review.googlesource.com/85875
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Also add testdata for version 1.11 including UserTaskSpan test trace.
Change-Id: I673fb29bb3aee96a14fadc0ab860d4f5832143f5
Reviewed-on: https://go-review.googlesource.com/93795
Reviewed-by: Heschi Kreinick <heschi@google.com>
This implements the annotation API proposed in golang.org/cl/63274.
traceString is updated to protect the string map with trace.stringsLock
because the assumption that traceString is called by a single goroutine
(either at the beginning of tracing and at the end of tracing when
dumping all the symbols and function names) is no longer true.
traceString is used by the annotation apis (NewContext, StartSpan, Log)
to register frequently appearing strings (task and span names, and log
keys) after this change.
NewContext -> one or two records (EvString, EvUserTaskCreate)
end function -> one record (EvUserTaskEnd)
StartSpan -> one or two records (EvString, EvUserSpan)
span end function -> one or two records (EvString, EvUserSpan)
Log -> one or two records (EvString, EvUserLog)
EvUserLog record is of the typical record format written by traceEvent
except that it is followed by bytes that represents the value string.
In addition to runtime/trace change, this change includes
corresponding changes in internal/trace to parse the new record types.
Future work to improve efficiency:
More efficient unique task id generation instead of atomic. (per-P
counter).
Instead of a centralized trace.stringsLock, consider using per-P
string cache or something more efficient.
R=go1.11
Change-Id: Iec9276c6c51e5be441ccd52dec270f1e3b153970
Reviewed-on: https://go-review.googlesource.com/71690
Reviewed-by: Austin Clements <austin@google.com>
This CL presents the proposed user annotation API skeleton.
This CL bumps up the trace version to 1.11.
Design doc https://goo.gl/iqJfJ3
Implementation CLs are followed.
The API introduces three basic building blocks. Log, Span, and Task.
Log is for basic logging. When called, the message will be recorded
to the trace along with timestamp, goroutine id, and stack info.
trace.Log(ctx, messageType message)
Span can be thought as an extension of log to record interesting
time interval during a goroutine's execution. A span is local to a
goroutine by definition.
trace.WithSpan(ctx, "doVeryExpensiveOp", func(ctx context) {
/* do something very expensive */
})
Task is higher-level concept that aids tracing of complex operations
that encompass multiple goroutines or are asynchronous.
For example, an RPC request, a HTTP request, a file write, or a
batch job can be traced with a Task.
Note we chose to design the API around context.Context so it allows
easier integration with other tracing tools, often designed around
context.Context as well. Log and WithSpan APIs recognize the task
information embedded in the context and record it in the trace as
well. That allows the Go execution tracer to associate and group
the spans and log messages based on the task information.
In order to create a Task,
ctx, end := trace.NewContext(ctx, "myTask")
defer end()
The Go execution tracer measures the time between the task created
and the task ended for the task latency.
More discussion history in golang.org/cl/59572.
Update #16619
R=go1.11
Change-Id: I59a937048294dafd23a75cf1723c6db461b193cd
Reviewed-on: https://go-review.googlesource.com/63274
Reviewed-by: Austin Clements <austin@google.com>
Move the ELF32 and ELF64 structure definitions into their own files so
they can be reused when vDSO support is added for other architectures.
Change-Id: Id0171b4e5cea4add8635743c881e3bf3469597af
Reviewed-on: https://go-review.googlesource.com/93995
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
If copying from a slice to itself, skip the write barriers
and actual memory copies.
This happens in practice in code like this snippet from
the trim pass in the compiler, when k ends up being 0:
copy(s.Values[k:], s.Values[:m])
Change-Id: Ie6924acfd56151f874d87f1d7f1f74320b4c4f10
Reviewed-on: https://go-review.googlesource.com/94023
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The runtime.hmap type is known at compile time.
Using new(hmap) avoids loading the hmap type from the maptype
supplied as an argument to makemap which is only known at runtime.
This change makes makemap consistent with makemap_small
by using new(hmap) instead of newobject in both functions.
Change-Id: Ia47acfda527e8a71d15a1a7a4c2b54fb923515eb
Reviewed-on: https://go-review.googlesource.com/91775
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The runtime builtin functions that are tested in append_test.go
are defined in slice.go. Renaming the test file to slice_test.go
makes this relation explicit with a common file name prefix.
Change-Id: I2f89ec23a6077fe6b80d2161efc760df828c8cd4
Reviewed-on: https://go-review.googlesource.com/90655
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
By default futexes are permitted in shared memory regions, which
requires the kernel to translate the memory address. Since our futexes
are never in shared memory, set FUTEX_PRIVATE_FLAG, which makes futex
operations slightly more efficient.
Change-Id: I2a82365ed27d5cd8d53c5382ebaca1a720a80952
Reviewed-on: https://go-review.googlesource.com/80144
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
On nacl/arm, R12 is clobbered by the RET instruction in function
that has a frame. runtime.udiv doesn't have a frame, so it does
not clobber R12.
Change-Id: I0de448749f615908f6659e92d201ba3eb2f8266d
Reviewed-on: https://go-review.googlesource.com/93116
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The stackguard is set to stackPreempt earlier in reentersyscall, and
as it comes with throwsplit = true there's no way for the stackguard
to be set to anything else by the end of reentersyscall.
Change-Id: I4e942005b22ac784c52398c74093ac887fc8ec24
Reviewed-on: https://go-review.googlesource.com/65673
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Also order the syscall number list by numerically for mips64x.
Follow-up for CL 92895.
Change-Id: I5f01f8c626132a06160997fce8a2aef0c486bb1c
Reviewed-on: https://go-review.googlesource.com/93616
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 92916 added the GOMAXPROCS test in TestTraceSymbolize.
This test only succeeds when the value of GOMAXPROCS changes.
Since the test calls runtime.GOMAXPROCS(1), it will fails
on machines where GOMAXPROCS=1.
This change fixes the test by calling runtime.GOMAXPROCS(oldGoMaxProcs+1).
Fixes#23816.
Change-Id: I1183dbbd7db6077cbd7fa0754032ff32793b2195
Reviewed-on: https://go-review.googlesource.com/93735
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, if a sigpanic call is injected into C code, it's possible
for preparePanic to leave the stack in a state where traceback can't
unwind correctly past the sigpanic.
Specifically, shouldPushPanic sniffs the stack to decide where to put
the PC from the signal context. In the cgo case, it will find that
!findfunc(pc).valid() because pc is in C code, and then it will check
if the top of the stack looks like a Go PC. However, this stack slot
is just in a C frame, so it could be uninitialized and contain
anything, including what looks like a valid Go PC. For example, in
https://build.golang.org/log/c601a18e2af24794e6c0899e05dddbb08caefc17,
it sees 1c02c23a <runtime.newproc1+682>. When this condition is met,
it skips putting the signal PC on the stack at all. As a result, when
we later unwind from the sigpanic, we'll "successfully" but
incorrectly unwind to whatever PC was in this uninitialized slot and
go who knows where from there.
Fix this by making shouldPushPanic assume that the signal PC is always
usable if we're running C code, so we always make it appear like
sigpanic's caller.
This lets us be pickier again about unexpected return PCs in
gentraceback.
Updates #23640.
Change-Id: I1e8ade24b031bd905d48e92d5e60c982e8edf160
Reviewed-on: https://go-review.googlesource.com/91137
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This logic is duplicated in all of the preparePanic functions. Pull it
out into one architecture-independent function.
Change-Id: I7ef4e78e3eda0b7be1a480fb5245fc7424fb2b4e
Reviewed-on: https://go-review.googlesource.com/91255
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously find_goroutine determined whether a goroutine is
stopped by checking the sched.sp field. This heuristic doesn't
always hold but causes find_goroutine to return bogus pc/sp
info for running goroutines.
This change uses the atomicstatus bit to determine
the state which is more accurate.
R=go1.11
Change-Id: I537d432d9e0363257120a196ce2ba52da2970f59
Reviewed-on: https://go-review.googlesource.com/49691
Reviewed-by: Austin Clements <austin@google.com>
Instead evaluate and read the runtime internal constants
defined in runtime2.go
R=go1.11
Change-Id: If2f4b87e5b3f62f0c0ff1e86a90db8e37a78abb6
Reviewed-on: https://go-review.googlesource.com/87877
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
and reorganize test log messages for stack dumps
for easier debugging.
The error log will be formatted like the following:
trace_stack_test.go:282: Did not match event GoCreate with stack
runtime/trace_test.TestTraceSymbolize :39
testing.tRunner :0
Seen 30 events of the type
Offset 1890
runtime/trace_test.TestTraceSymbolize /go/src/runtime/trace/trace_stack_test.go:30
testing.tRunner /go/src/testing/testing.go:777
Offset 1899
runtime/trace_test.TestTraceSymbolize /go/src/runtime/trace/trace_stack_test.go:30
testing.tRunner /go/src/testing/testing.go:777
...
Change-Id: I0468de04507d6ae38ba84d99d13f7bf592e8d115
Reviewed-on: https://go-review.googlesource.com/92916
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Now that the buffered write barrier is implemented for all
architectures, we can remove the old eager write barrier
implementation. This CL removes the implementation from the runtime,
support in the compiler for calling it, and updates some compiler
tests that relied on the old eager barrier support. It also makes sure
that all of the useful comments from the old write barrier
implementation still have a place to live.
Fixes#22460.
Updates #21640 since this fixes the layering concerns of the write
barrier (but not the other things in that issue).
Change-Id: I580f93c152e89607e0a72fe43370237ba97bae74
Reviewed-on: https://go-review.googlesource.com/92705
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Calls to writebarrierptr can simply be actual pointer writes. Calls to
writebarrierptr_prewrite need to go through the write barrier buffer.
Updates #22460.
Change-Id: I92cee4da98c5baa499f1977563757c76f95bf0ca
Reviewed-on: https://go-review.googlesource.com/92704
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
CL 137410043 deleted support for split stacks, which means morestack
no longer needed to save its caller's frame or argument size or its
caller's argument pointer. However, this commit failed to update the
comment or delete the line that computed the caller's argument
pointer. Clean these up now.
Change-Id: I65725d3d42c86e8adb6645d5aa80c305d473363d
Reviewed-on: https://go-review.googlesource.com/92437
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -4/-8 with the NOFRAME flag in mips and
mips64 assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-[84]/\1|NOFRAME,\2$0/' $(find -name '*_mips*.s')
Plus a manual fix to mkduff.go.
The go binary is identical on both architectures before and after this
change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efb
Reviewed-on: https://go-review.googlesource.com/92044
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -8 with the NOFRAME flag in arm64 assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-8/\1|NOFRAME,\2$0/' $(find -name '*_arm64.s')
Plus a manual fix to mkduff.go.
The go binary is identical before and after this change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efa
Reviewed-on: https://go-review.googlesource.com/92043
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -4 with the NOFRAME flag in arm assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-4/\1|NOFRAME,\2$0/' $(find -name '*_arm.s')
Plus three manual comment changes found by:
grep '\$-4' $(find -name '*_arm.s')
The go binary is identical before and after this change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227ef9
Reviewed-on: https://go-review.googlesource.com/92042
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
"-8" is not a sensible frame size on arm and we're about to start
rejecting it. Replace it with -4.
Likewise, "-4" is not a sensible frame size on arm64 and we're about
to start rejecting it. Replace it with -8.
Finally, clean up some places we're weirdly inconsistent about using 0
versus -8.
Change-Id: If85e229993d5f7f1f0cfa9852b4e294d053bd784
Reviewed-on: https://go-review.googlesource.com/92038
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
preparePanic must set all registers expected by Go runtime conventions
in case the sigpanic is being injected into C code. However, on
mips64x it fails to restore RSB (R28). As a result, if C code modifies
RSB and then raises a signal that turns into a sigpanic call, sigpanic
may crash when it attempts to lock runtime.debuglock (the first global
it references).
Fix this by restoring RSB in the signal context using the same
convention as main and sigtramp.
Fixes#23641.
Change-Id: Ib47e83df89e2a3eece10f480e4e91ce9e4424388
Reviewed-on: https://go-review.googlesource.com/91156
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, gentraceback suppresses the "unexpected return pc" error
for sigpanic's caller if the M was running C code.
However, there are various situations where a sigpanic is injected
into C code that can cause traceback to unwind *past* the sigpanic
before realizing that it's in trouble (the traceback beyond the
sigpanic will be wrong).
Rather than try to fix these issues for Go 1.10, this CL simply
disables complaining about unexpected return PCs if we're in cgo
regardless of whether or not they're from the sigpanic frame. Go 1.9
never complained about unexpected return PCs when printing, so this is
simply a step closer to the old behavior.
This should fix the openbsd-386 failures on the dashboard, though this
issue could affect any architecture.
Fixes#23640.
Change-Id: I8c32c1ee86a70d2f280661ed1f8caf82549e324b
Reviewed-on: https://go-review.googlesource.com/91136
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If we're running C code and the code panics, the runtime will inject a
call to sigpanic into the C code just like it would into Go code.
However, the return PC from this sigpanic will be in C code. We used
to silently abort the traceback if we didn't recognize a return PC, so
this went by quietly. Now we're much louder because in general this is
a bad thing. However, in this one particular case, it's fine, so if
we're in cgo and are looking at the return PC of sigpanic, silence the
debug output.
Fixes#23576.
Change-Id: I03d0c14d4e4d25b29b1f5804f5e9ccc4f742f876
Reviewed-on: https://go-review.googlesource.com/90896
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
asmcgocall switches to the system stack and aligns the SP, so
gentraceback both can't unwind over it when it appears on the system
stack (it'll read some uninitialized stack slot as the return PC).
There's also no point in unwinding over it, so don't.
Updates #23576.
Change-Id: Idfcc9599c7636b80dec5451cb65ae892b4611981
Reviewed-on: https://go-review.googlesource.com/90895
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
pprof expects the samples are scaled and reflects unsampled numbers.
The legacy profile parser uses the sampling period in the output
and multiplies all values with the period.
0138a3cd6d/profile/legacy_profile.go (L815)
Apply the same scaling when we output the mutex profile
in the pprof proto format.
Block profile shares the same code, but how to infer unsampled
values is unclear. Legacy profile parser doesn't do anything special
so we do nothing for block profile here.
Tested by checking the profiles reported with debug=0 (proto format)
are similar to the profiles computed from legacy format profile
when the profile rate is a non-trivial number (e.g. 2) manually.
Change-Id: Iaa33f92051deed67d8be43ddffc7c1016db566ca
Reviewed-on: https://go-review.googlesource.com/89295
Reviewed-by: Peter Weinberger <pjw@google.com>
Currently, startpanic_m (which prepares for an unrecoverable panic)
goes out of its way to make it possible to allocate during panic
handling by allocating an mcache if there isn't one.
However, this is both potentially dangerous and unnecessary.
Allocating an mcache is a generally complex thing to do in an already
precarious situation. Specifically, it requires obtaining the heap
lock, and there's evidence that this may be able to deadlock (#23360).
However, it's also unnecessary because we never allocate from the
unrecoverable panic path.
This didn't use to be the case. The call to allocmcache was introduced
long ago, in CL 7388043, where it was in preparation for separating Ms
and Ps and potentially running an M without an mcache. At the time,
after calling startpanic, the runtime could call String and Error
methods on panicked values, which could do anything including
allocating. That was generally unsafe even at the time, and CL 19792
fixed this be pre-printing panic messages before calling startpanic.
As a result, we now no longer allocate after calling startpanic.
This CL not only removes the allocmcache call, but goes a step further
to explicitly disallow any allocation during unrecoverable panic
handling, even in situations where it might be safe. This way, if
panic handling ever does an allocation that would be unsafe in unusual
circumstances, we'll know even if it happens during normal
circumstances.
This would help with debugging #23360, since the deadlock in
allocmcache is currently masking the real failure.
Beyond all.bash, I manually tested this change by adding panics at
various points in early runtime init, signal handling, and the
scheduler to check unusual panic situations.
Change-Id: I85df21e2b4b20c6faf1f13fae266c9339eebc061
Reviewed-on: https://go-review.googlesource.com/88835
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, if a _SigPanic signal arrives in a throwsplit context,
nothing is stopping the runtime from injecting a call to sigpanic that
may attempt to grow the stack. This will fail and, in turn, mask the
real problem.
Fix this by checking for throwsplit in the signal handler itself
before injecting the sigpanic call.
Updates #21431, where this problem is likely masking the real problem.
Change-Id: I64b61ff08e8c4d6f6c0fb01315d7d5e66bf1d3e2
Reviewed-on: https://go-review.googlesource.com/87595
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, if anything goes wrong when printing a traceback, we simply
cut off the traceback without any further diagnostics. Unfortunately,
right now, we have a few issues that are difficult to debug because
the traceback simply cuts off (#21431, #23484).
This is an attempt to improve the debuggability of traceback failure
by printing a diagnostic message plus a hex dump around the failed
traceback frame when something goes wrong.
The failures look like:
goroutine 5 [running]:
runtime: unexpected return pc for main.badLR2 called from 0xbad
stack: frame={sp:0xc42004dfa8, fp:0xc42004dfc8} stack=[0xc42004d800,0xc42004e000)
000000c42004dea8: 0000000000000001 0000000000000001
000000c42004deb8: 000000c42004ded8 000000c42004ded8
000000c42004dec8: 0000000000427eea <runtime.dopanic+74> 000000c42004ded8
000000c42004ded8: 000000000044df70 <runtime.dopanic.func1+0> 000000c420001080
000000c42004dee8: 0000000000427b21 <runtime.gopanic+961> 000000c42004df08
000000c42004def8: 000000c42004df98 0000000000427b21 <runtime.gopanic+961>
000000c42004df08: 0000000000000000 0000000000000000
000000c42004df18: 0000000000000000 0000000000000000
000000c42004df28: 0000000000000000 0000000000000000
000000c42004df38: 0000000000000000 000000c420001080
000000c42004df48: 0000000000000000 0000000000000000
000000c42004df58: 0000000000000000 0000000000000000
000000c42004df68: 000000c4200010a0 0000000000000000
000000c42004df78: 00000000004c6400 00000000005031d0
000000c42004df88: 0000000000000000 0000000000000000
000000c42004df98: 000000c42004dfb8 00000000004ae7d9 <main.badLR2+73>
000000c42004dfa8: <00000000004c6400 00000000005031d0
000000c42004dfb8: 000000c42004dfd0 !0000000000000bad
000000c42004dfc8: >0000000000000000 0000000000000000
000000c42004dfd8: 0000000000451821 <runtime.goexit+1> 0000000000000000
000000c42004dfe8: 0000000000000000 0000000000000000
000000c42004dff8: 0000000000000000
main.badLR2(0x0)
/go/src/runtime/testdata/testprog/badtraceback.go:42 +0x49
For #21431, #23484.
Change-Id: I8718fc76ced81adb0b4b0b4f2293f3219ca80786
Reviewed-on: https://go-review.googlesource.com/89016
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Fix breakage introduced in CL 70530.
Change-Id: I87f3da6b20554d4f405a1143b0d894c5953b63aa
Reviewed-on: https://go-review.googlesource.com/88516
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
We used a mix of both before.
I've never heard anybody say "an arr-double you mutex" when speaking.
Fixes#23457
Change-Id: I802b5eb2339f885ca9d24607eeda565763165298
Reviewed-on: https://go-review.googlesource.com/87896
Reviewed-by: Andrew Bonventre <andybons@golang.org>
CL 67332 created the fast no-syscall path for time.Now in High Sierra
but managed to break Sierra and older by forcing them into the slow
syscall path: the version check based on commpage version was wrong.
This CL uses the Darwin version number instead.
The assembly diff is noisy because many variables had to be
renamed, but the only actual change is the version check.
Fixes#23419.
Change-Id: Ie31ef5fb88f66d1517a8693942a7fb6100c213b0
Reviewed-on: https://go-review.googlesource.com/87655
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The tool was moved to tools/Testing/selftests within the Linux kernel
source tree. Adjust the URL in the comments of vdso_linux.go
Change-Id: I86b9cae4b898c4a45bc7c54891ce6ead91a22670
Reviewed-on: https://go-review.googlesource.com/87815
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The cgo checker was issuing an error with cgocheck=2 when a timer
bucket was stored in a pollDesc. The pollDesc values are allocated
using persistentalloc, so they are not in the Go heap. The code is OK
since timer bucket pointers point into a global array, and as such are
never garbage collected or moved.
Mark timersBucket notinheap to avoid the problem. timersBucket values
only occur in the global timers array.
Fixes#23435
Change-Id: I835f31caafd54cdacc692db5989de63bb49e7697
Reviewed-on: https://go-review.googlesource.com/87637
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Suppose you build the Go toolchain in directory A,
move the whole thing to directory B, and then use
it from B to build a new program hello.exe, and then
run hello.exe, and hello.exe crashes with a stack
trace into the standard library.
Long ago, you'd have seen hello.exe print file names
in the A directory tree, even though the files had moved
to the B directory tree. About two years ago we changed
the compiler to write down these files with the name
"$GOROOT" (that literal string) instead of A, so that the
final link from B could replace "$GOROOT" with B,
so that hello.exe's crash would show the correct source
file paths in the stack trace. (golang.org/cl/18200)
Now suppose that you do the same thing but hello.exe
doesn't crash: it prints fmt.Println(runtime.GOROOT()).
And you run hello.exe after clearing $GOROOT from the
environment.
Long ago, you'd have seen hello.exe print A instead of B.
Before this CL, you'd still see hello.exe print A instead of B.
This case is the one instance where a moved toolchain
still divulges its origin. Not anymore. After this CL, hello.exe
will print B, because the linker sets runtime/internal/sys.DefaultGoroot
with the effective GOROOT from link time.
This makes the default result of runtime.GOROOT once again
match the file names recorded in the binary, after two years
of divergence.
With that cleared up, we can reintroduce GOROOT into the
link action ID and also reenable TestExecutableGOROOT/RelocatedExe.
When $GOROOT_FINAL is set during link, it is used
in preference to $GOROOT, as always, but it was easier
to explain the behavior above without introducing that
complication.
Fixes#22155.
Fixes#20284.
Fixes#22475.
Change-Id: Ifdaeb77fd4678fdb337cf59ee25b2cd873ec1016
Reviewed-on: https://go-review.googlesource.com/86835
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
findrunnable loops over allp to check run queues *after* it has
dropped its own P. This is unsafe because allp can change when nothing
is blocking safe-points. Hence, procresize could change allp
concurrently with findrunnable's loop. Beyond generally violating Go's
memory model, in the best case this could findrunnable to observe a
nil P pointer if allp has been grown but the new slots not yet
initialized. In the worst case, the reads of allp could tear, causing
findrunnable to read a word that isn't even a valid *P pointer.
Fix this by taking a snapshot of the allp slice header (but not the
backing store) before findrunnable drops its P and iterating over this
snapshot. The actual contents of allp are immutable up to len(allp),
so this fixes the race.
Updates #23098 (may fix).
Change-Id: I556ae2dbfffe9fe4a1bf43126e930b9e5c240ea8
Reviewed-on: https://go-review.googlesource.com/86215
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Commit c2c07c7989 (CL 49331) changed the linker and runtime to always
use 2MB stacks on 64-bit Windows. This is the corresponding change to
make 32-bit Windows always use large (1MB) stacks because it's
difficult to detect when Windows applications will call into arbitrary
C code that may expect a large stack.
This is done as a separate change because it's possible this will
cause too much address space pressure for a 32-bit address space. On
the other hand, cgo binaries on Windows already use 1MB stacks and
there haven't been complaints.
Updates #20975.
Change-Id: I8ce583f07cb52254fb4bd47250f1ef2b789bc490
Reviewed-on: https://go-review.googlesource.com/49610
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
golang.org/cl/81315 attempted to distinguish system goroutines
by examining the function name in the goroutine stack. It assumes that
the information would be available when GoSysBlock or GoInSyscall
events are processed, but it turned out the stack information is
set too late (when the goroutine gets a chance to run).
This change initializes the goroutine information entry when
processing GoCreate event which should be one of the very first
events for the every goroutine in trace.
Fixes#22574
Change-Id: I1ed37087ce2e78ed27c9b419b7d942eb4140cc69
Reviewed-on: https://go-review.googlesource.com/83595
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This attempts to symbolize the PC of morestack's caller when there's a
stack split at a bad time. The stack trace starts at the *caller* of
the function that attempted to grow the stack, so this is useful if it
isn't obvious what's being called at that point, such as in #21431.
Change-Id: I5dee305d87c8069611de2d14e7a3083d76264f8f
Reviewed-on: https://go-review.googlesource.com/84115
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, wbBufFlush does nothing if the goroutine is dying on the
assumption that the system is crashing anyway and running the write
barrier may crash it even more. However, it fails to reset the
buffer's "next" pointer. As a result, if there are later write
barriers on the same P, the write barrier will overflow the write
barrier buffer and start corrupting other fields in the P or other
heap objects. Often, this corrupts fields in the next allocated P
since they tend to be together in the heap.
Fix this by always resetting the buffer's "next" pointer, even if
we're not doing anything with the pointers in the buffer.
Updates #22987 and #22988. (May fix; it's hard to say.)
Change-Id: I82c11ea2d399e1658531c3e8065445a66b7282b2
Reviewed-on: https://go-review.googlesource.com/83016
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
heapBits.bits is used during bulkBarrierPreWrite via
heapBits.isPointer, which means it must not be preempted. If it is
preempted, several bad things can happen:
1. This could allow a GC phase change, and the resulting shear between
the barriers and the memory writes could result in a lost pointer.
2. Since bulkBarrierPreWrite uses the P's local write barrier buffer,
if it also migrates to a different P, it could try to append to the
write barrier buffer concurrently with another write barrier. This can
result in the buffer's next pointer skipping over its end pointer,
which results in a buffer overflow that can corrupt arbitrary other
fields in the Ps (or anything in the heap, really, but it'll probably
crash from the corrupted P quickly).
Fix this by marking heapBits.bits go:nosplit. This would be the
perfect use for a recursive no-preempt annotation (#21314).
This doesn't actually affect any binaries because this function was
always inlined anyway. (I discovered it when I was modifying heapBits
and make h.bits() no longer inline, which led to rampant crashes from
problem 2 above.)
Updates #22987 and #22988 (but doesn't fix because it doesn't actually
change the generated code).
Change-Id: I60ebb928b1233b0613361ac3d0558d7b1cb65610
Reviewed-on: https://go-review.googlesource.com/83015
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On DragonFly mmap with MAP_STACK returns the top of the region, not
the bottom. Rather than try to cope, just don't use the flag anywhere.
Fixes#23061
Change-Id: Ib5df4dd7c934b3efecfc4bc87f8989b4c37555d7
Reviewed-on: https://go-review.googlesource.com/83035
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change updates runtime.semasleep to no longer call
runtime.nanotime and instead calls lwp_park with a duration to sleep
relative to the monotonic clock, so the nanotime is never called.
(This requires updating to a newer version of the lwp_park system
call, which is safe, because Go 1.10 will require the unreleased
NetBSD 8+ anyway)
Additionally, this change makes the nanotime function use the
monotonic clock for netbsd/arm, which was forgotten from
https://golang.org/cl/81135 which updated netbsd/amd64 and netbsd/386.
Because semasleep previously depended on nanotime, the past few days
of netbsd have likely been unstable because lwp_park was then mixing
the monotonic and wall clocks. After this CL, lwp_park no longer
depends on nanotime.
Original patch submitted at:
https://www.netbsd.org/~christos/go-lwp-park-clock-monotonic.diff
This commit message (any any mistakes therein) were written by Brad
Fitzpatrick. (Brad migrated the patch to Gerrit and checked CLAs)
Updates #6007Fixes#22968
Also updates netbsd/arm to use monotonic time for
Change-Id: If77ef7dc610b3025831d84cdfadfbbba2c52acb2
Reviewed-on: https://go-review.googlesource.com/81715
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
MAP_ANON is the deprecated but more portable spelling of
MAP_ANONYMOUS. Use MAP_ANON to un-break the Darwin 10.10 builder.
Updates #22930.
Change-Id: Iedd6232b94390b3b2a7423c45cdcb25c1a5b3323
Reviewed-on: https://go-review.googlesource.com/81615
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Minor.
Makes reading failing runtime test stacktraces easier (by having fewer
goroutines to read) on machines where these gdb tests wouldn't have
ever run anyway.
Change-Id: I3fab0667e017f20ef3bf96a8cc4cfcc614d25b5c
Reviewed-on: https://go-review.googlesource.com/81575
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds logging for the expected duration of a growStack, plus
progress information on the growStack that timed out.
Updates #19381.
Change-Id: Ic358f8350f499ff22dd213b658aece7d1aa62675
Reviewed-on: https://go-review.googlesource.com/81556
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I think of "sending" a signal as calling kill, but sigsend is involved
in handling a signal and, specifically delivering it to the internal
signal queue. The term "delivery" is already used in
signalWaitUntilIdle, so this CL also uses it in the documentation for
sigsend.
Change-Id: I86e171f247f525ece884a680bace616fa9a3c7bd
Reviewed-on: https://go-review.googlesource.com/81235
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, when we minit on a thread that already has an alternate
signal stack (e.g., because the M was an extram being used for a cgo
callback, or to handle a signal on a C thread, or because the
platform's libc always allocates a signal stack like on Android), we
simply drop the Go-allocated gsignal stack on the floor.
This is a problem for Ms on the extram list because those Ms may later
be reused for a different thread that may not have its own alternate
signal stack. On tip, this manifests as a crash in sigaltstack because
we clear the gsignal stack bounds in unminit and later try to use
those cleared bounds when we re-minit that M. On 1.9 and earlier, we
didn't clear the bounds, so this manifests as running more than one
signal handler on the same signal stack, which could lead to arbitrary
memory corruption.
This CL fixes this problem by saving the Go-allocated gsignal stack in
a new field in the m struct when overwriting it with a system-provided
signal stack, and then restoring the original gsignal stack in
unminit.
This CL is designed to be easy to back-port to 1.9. It won't quite
cherry-pick cleanly, but it should be sufficient to simply ignore the
change in mexit (which didn't exist in 1.9).
Now that we always have a place to stash the original signal stack in
the m struct, there are some simplifications we can make to the signal
stack handling. We'll do those in a later CL.
Fixes#22930.
Change-Id: I55c5a6dd9d97532f131146afdef0b216e1433054
Reviewed-on: https://go-review.googlesource.com/81476
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 08f19bbde1.
Reason for revert:
The changed transformation takes effect on a larger set
of code snippets than expected.
For example, this:
func foo() {
// Comment
bar()
}
becomes:
func foo() {
// Comment
bar()
}
This is an unintended consequence.
Change-Id: Ifca88d6267dab8a8170791f7205124712bf8ace8
Reviewed-on: https://go-review.googlesource.com/81335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
