Bryan C. Mills [Wed, 11 Dec 2019 14:35:31 +0000 (09:35 -0500)]
net/http: use cancellation instead of a timeout in TestTransportProxyHTTPSConnectTimeout
The use of a timeout in this test caused it to be flaky: if the
timeout occurred before the connection was attempted, then the Accept
call on the Listener could hang indefinitely, and its goroutine would
not exit until that Listener was closed. That caused the test to fail.
A longer timeout would make the test less flaky, but it would become
even slower and would still be sensitive to timing.
Instead, replace the timeout with an explicit Context cancellation
after the CONNECT request has been read. That not only ensures that
the cancellation occurs at the appropriate point, but also makes the
test much faster: a test run with -count=1000 now executes in less
than 2s on my machine, whereas before it took upwards of 50s.
Andrew Stormont [Wed, 11 Dec 2019 00:31:44 +0000 (00:31 +0000)]
runtime: syscall_forkx on Solaris can return error on success
The syscall_forkx function returns the value of errno even on success. This can be a problem when using cgo where an atfork handler might be registered; if the atfork handler does something which causes errno to be set the caller of syscall_forkx can be misled into thinking the fork has failed. This causes the various exec functions in the runtime package to hang.
Change-Id: Ia1842179226078a0cbbea33d541aa1187dc47f68
GitHub-Last-Rev: 4dc4db75c82a826da9a50c323b7e3ddfe46ed6c0
GitHub-Pull-Request: golang/go#36076
Reviewed-on: https://go-review.googlesource.com/c/go/+/210742 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitri Shuralyov [Tue, 10 Dec 2019 20:31:47 +0000 (15:31 -0500)]
doc: remove Release History pages (moved to x/website)
These pages were moved to the x/website repo in CL 210797 (commit
golang/website@9aef1eefbbe663d448b04b7cc1b2b995f4cf4c0b).
Remove the old copies in this repo since they're no longer used.
Updates #36075
Updates #29206
Change-Id: I6e3ffaebd92fa753cb5f3b21e4238edfb7f5f0e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/210798 Reviewed-by: Alexander Rakoczy <alex@golang.org>
Lorenz Bauer [Tue, 10 Dec 2019 15:58:24 +0000 (15:58 +0000)]
syscall: use SOCK_CLOEXEC when creating sockets
LsfSocket, SetLsfPromisc and NetlinkRIB currently don't force the CLOEXEC
flag on the sockets they create. While the former two functions are
deprecated, NetlinkRIB is called by various functions related to
net.Interface.
Add a helper to create CLOEXEC sockets, and use it from SetLsfPromisc and
NetlinkRIB. LsfSocket is unchanged since we don't want to break callers.
Fixes #36053
Change-Id: I72fe2b167996797698d8a44b0d28165045c42d3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/210517
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Tue, 10 Dec 2019 14:27:53 +0000 (14:27 +0000)]
all: fix a number of misuses of the word "an"
After golang.org/cl/210124, I wondered if the same error had gone
unnoticed elsewhere. I quickly spotted another dozen mistakes after
reading through the output of:
git grep '\<[Aa]n [bcdfgjklmnpqrtvwyz][a-z]'
Many results are false positives for acronyms like "an mtime", since
it's pronounced "an em-time". However, the total amount of output isn't
that large given how simple the grep pattern is.
Change-Id: Iaa2ca69e42f4587a9e3137d6c5ed758887906ca6
Reviewed-on: https://go-review.googlesource.com/c/go/+/210678 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Zach Jones <zachj1@gmail.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In "2.9 Relocation" it shows relocation section contains five parts:
1. r_sym Elf64_Word Symbol index
2. r_ssym Elf64_Byte Special symbol
3. r_type3 Elf64_Byte Relocation type
4. r_type2 Elf64_Byte Relocation type
5. r_type Elf64_Byte Relocation type
Clément Chigot [Thu, 14 Nov 2019 14:43:55 +0000 (15:43 +0100)]
runtime: use mprotect in sysMap for aix/ppc64
AIX doesn't allow to mmap an already mmap address. The previous way to
deal with this behavior was to munmap before calling mmap again.
However, mprotect syscall is able to change protections on a memory
range. Thus, memory mapped by sysReserve can be remap using it. Note
that sysMap is always called with a non-nil pointer so mprotect is
always possible.
Austin Clements [Mon, 9 Dec 2019 03:24:10 +0000 (22:24 -0500)]
runtime: mlock top of signal stack on both amd64 and 386
CL 209899 worked around an issue that corrupts vector registers in
recent versions of the Linux kernel by mlocking the top page of every
signal stack on amd64. However, the underlying issue also affects the
XMM registers on 386. This CL applies the mlock fix to both amd64 and
386.
Fixes #35777 (again).
Change-Id: I9886f2dc4c23625421296bd5518d5fd3288bfe48
Reviewed-on: https://go-review.googlesource.com/c/go/+/210345
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Alberto Donizetti [Sat, 7 Dec 2019 15:52:10 +0000 (16:52 +0100)]
doc: add missing p in install from source page
The last paragraph in golang.org/doc/install/source#fetch is missing a
p tag, so it doesn't get formatted with the 'max-width: 50rem' like
all the other text in the page.
Uses 2 channels to synchronize that test, because
relying on sleeps creates flaky behavior, thus:
a) 1 buffered channel to send back the last spurious line
without having to reason about "happens before" behavior
a) 1 buffered channel at the end of the handler; it'll
be controlled by whether we expect to timeout or not,
but will always be closed when the test ends
Cherry Zhang [Fri, 6 Dec 2019 21:52:53 +0000 (16:52 -0500)]
cmd/link: skip gaps between PT_LOAD segments in TestPIESize
There may be gaps between non-writeable and writeable PT_LOAD
segments, and the gaps may be large as the segments may have
large alignment. Don't count those gaps in file size comparison.
Fixes #36023.
Change-Id: I68582bdd0f385ac5c6f87d485d476d06bc96db19
Reviewed-on: https://go-review.googlesource.com/c/go/+/210180
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Fri, 6 Dec 2019 19:48:26 +0000 (14:48 -0500)]
cmd/go: avoid generating "malformed module path" errors for standard-library paths
If the path looks like it belongs in GOROOT/src and isn't there, we
should mention that in the error message — instead of the fact
that the path is not a valid module path, which the user likely
already knows.
Fixes #34769
Fixes #35734
Change-Id: I3589336d102e420a5ad3bf246816e29f3cbe6d71
Reviewed-on: https://go-review.googlesource.com/c/go/+/210339
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
po3rin [Fri, 6 Dec 2019 19:44:39 +0000 (04:44 +0900)]
strings: fix nonexistent path in comment
There is a part in the comment that points to a non-existent file.
It seems to have been overlooked in following PR.
https://go-review.googlesource.com/c/go/+/98518/
Jay Conrod [Fri, 6 Dec 2019 18:10:53 +0000 (13:10 -0500)]
cmd/go: reduce redundancy in direct mode lookup error messages
get.RepoRootForImportPath now returns errors that satisfy
load.ImportPathError in cases where the import path appears in the
messages. (The import path probably should appear in all errors from
this function, but this CL does not change these errors).
Changed modfetch.notExistError to be a wrapper (with an Unwrap method)
instead of a string. This means errors.As works with notFoundError and
ImportPathError.
ImportMissingError no longer prints the package path if it wraps an
ImportPathError.
TestMissingImportErrorRepetition no longer counts the package path
within a URL (like https://...?go-get=1).
Fixes #35986
Change-Id: I38f795191c46d04b542c553e705f23822260c790
Reviewed-on: https://go-review.googlesource.com/c/go/+/210338
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Austin Clements [Thu, 5 Dec 2019 19:41:24 +0000 (14:41 -0500)]
runtime: give useful failure message on mlock failure
Currently, we're ignoring failures to mlock signal stacks in the
workaround for #35777. This means if your mlock limit is low, you'll
instead get random memory corruption, which seems like the wrong
trade-off.
This CL checks for mlock failures and panics with useful guidance.
Ian Lance Taylor [Thu, 5 Dec 2019 22:49:25 +0000 (14:49 -0800)]
sync: deflake TestWaitGroupMisuse3
If one of the helper goroutine panics, the main goroutine call to Wait
may hang forever waiting for something to call Done. Put that call in
a goroutine like the others.
Fixes #35774
Change-Id: I8d2b58d8f473644a49a95338f70111d4e6ed4e12
Reviewed-on: https://go-review.googlesource.com/c/go/+/210218
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Alex Brainman [Sat, 9 Nov 2019 08:06:24 +0000 (19:06 +1100)]
all: fix most of the remaining windows -d=checkptr violations
This change replaces
buf := [HUGE_CONST]*T)(unsafe.Pointer(p))[:]
with
buf := [HUGE_CONST]*T)(unsafe.Pointer(p))[:n:n]
Pointer p points to n of T elements. New unsafe pointer conversion
logic verifies that both first and last elements point into the same
Go variable.
This change replaces [:] with [:n:n] to please pointer checker.
According to @mdempsky, compiler specially recognizes when you
combine a pointer conversion with a full slice operation in a single
expression and makes an exception.
After this, only one failure in net remains when running:
go test -a -short -gcflags=all=-d=checkptr std cmd
Updates #34972
Change-Id: I2c8731650c856264bc788e4e07fa0530f7c250fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/208617
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Mon, 2 Dec 2019 17:38:03 +0000 (09:38 -0800)]
go/types: print package path in error messages if package name is not unique
Change package qualification to print the full package path for packages
that have non-unique names (that is, where multiple different packages
have the same name). Use the package name as qualifier in all other cases
(but don't print any qualification if we're talking about the package
being type-checked).
This matches the behavior of the compiler.
Fixes #35895.
Change-Id: I33ab8e7adfae1378907c01e33cabda114f65887f
Reviewed-on: https://go-review.googlesource.com/c/go/+/209578
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Tue, 3 Dec 2019 03:56:40 +0000 (22:56 -0500)]
cmd/compile: mark empty block preemptible
Currently, a block's control instruction gets the liveness info
of the last Value in the block. However, for an empty block, the
control instruction gets the invalid liveness info and therefore
not preemptible. One example is empty infinite loop, which has
only a control instruction. The control instruction being non-
preemptible makes the whole loop non-preemptible.
Fix this by using a different, preemptible liveness info for
empty block's control. We can choose an arbitrary preemptible
liveness info, as at run time we don't really use the liveness
map at that instruction.
As before, if the last Value in the block is non-preemptible, so
is the block control. For example, the conditional branch in the
write barrier test block is still non-preemptible.
Also, only update liveness info if we are actually emitting
instructions. So zero-width Values' liveness info (which are
always invalid) won't affect the block control's liveness info.
For example, if the last Values in a block is a tuple-generating
operation and a Select, the block control instruction is still
preemptible.
Cherry Zhang [Thu, 5 Dec 2019 23:56:54 +0000 (18:56 -0500)]
cmd/compile: don't fuse branches with side effects
Count Values with side effects but no use as live, and don't fuse
branches that contain such Values. (This can happen e.g. when it
is followed by an infinite loop.) Otherwise this may lead to
miscompilation (side effect fired at wrong condition) or ICE (two
stores live simultaneously).
Fixes #36005.
Change-Id: If202eae4b37cb7f0311d6ca120ffa46609925157
Reviewed-on: https://go-review.googlesource.com/c/go/+/210179 Reviewed-by: Keith Randall <khr@golang.org>
Rhys Hiltner [Thu, 5 Dec 2019 03:37:00 +0000 (19:37 -0800)]
cmd/go: print newline after GOOS/GOARCH error
The newline was dropped during the refactor in CL 194617.
Fixes #35984
Change-Id: I7e0d7aa2d7a4d1f44898921f8bb40401620d78b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/209965
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jay Conrod [Thu, 5 Dec 2019 18:28:57 +0000 (13:28 -0500)]
cmd/go: include imports in 'go list -e' output even after parse errors
If we aren't able to load imports from one file in a package due to a
parse error (scanner.ErrorList), 'go list -e' should still list
imports in other files.
Fixes #35973
Change-Id: I59f171877949bb7afaf252b6c8a970de22e60c7a
Reviewed-on: https://go-review.googlesource.com/c/go/+/210097
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Wed, 4 Dec 2019 22:58:04 +0000 (14:58 -0800)]
os: reset dirinfo when seeking on Darwin
The first Readdirnames calls opendir and caches the result.
The behavior of that cached opendir result isn't specified on a seek
of the underlying fd. Free the opendir result on a seek so that
we'll allocate a new one the next time around.
Also fix wasm behavior in this regard, so that a seek to the
file start resets the Readdirnames position, regardless of platform.
Dmitri Shuralyov [Thu, 5 Dec 2019 14:13:21 +0000 (17:13 +0300)]
doc: add CherryPickApproved filter to Release History links
Not all closed issues in a given minor milestone are included in that
release, only the ones that have been labeled as CherryPickApproved are.
Update the links to the GitHub issue tracker to include a filter on the
CherryPickApproved label, so that the default view shows only the
backports that were included in a given release. This should more useful
to most people than seeing all backports (considered and approved).
Do this only for Go 1.9.1 and newer releases, as that is when we started
using the CherryPickCandidate and CherryPickApproved labels.
Fixes #35988
Change-Id: I51e07c1bc3ab9c4a5744e8f668c5470adf78bffe
Reviewed-on: https://go-review.googlesource.com/c/go/+/209918 Reviewed-by: Alexander Rakoczy <alex@golang.org>
taisa [Wed, 4 Dec 2019 08:31:59 +0000 (17:31 +0900)]
testing: fix testing docs
The Perm function return 0 or 1 or 2 or 3. 4 is not returned,
so that changed the argument to 5.
Change-Id: Ic980c71a9f29f522bdeef4fce70a6c2dd136d791
Reviewed-on: https://go-review.googlesource.com/c/go/+/209777 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Tue, 26 Nov 2019 06:17:29 +0000 (22:17 -0800)]
cmd/link: when changing to Segrelrodata, reset datsize
Otherwise we leave a gap at the start of Segrelrodata equal to the
size of the read-only non-relro data, which causes -buildmode=pie
executables to be noticeably larger than -buildmode=exe executables.
Change-Id: I98956ef29d5b7a57ad8e633c823ac09d9ca36a45
Reviewed-on: https://go-review.googlesource.com/c/go/+/208897
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Austin Clements [Mon, 2 Dec 2019 22:36:25 +0000 (17:36 -0500)]
runtime: mlock top of signal stack on Linux 5.2–5.4.1
Linux 5.2 introduced a bug that can corrupt vector registers on return
from a signal if the signal stack isn't faulted in:
https://bugzilla.kernel.org/show_bug.cgi?id=205663
This CL works around this by mlocking the top page of all Go signal
stacks on the affected kernels.
Fixes #35326, #35777
Change-Id: I77c80a2baa4780827633f92f464486caa222295d
Reviewed-on: https://go-review.googlesource.com/c/go/+/209899
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> Reviewed-by: David Chase <drchase@google.com>
Tobias Klauser [Wed, 4 Dec 2019 10:44:06 +0000 (11:44 +0100)]
cmd/objdump: reference tracking bug in TestDisasmCode skip message
Issue #12559 was closed and split into #19158 for mips{,le} and #19156
for mips64{,le}. Instead of referencing the individual GOARCH-specific
issues in the skip test messages of TestDisasmCode use the tracking bug
Change-Id: I6929d25f4ec5aef4f069b7692c4e29106088ce65
Reviewed-on: https://go-review.googlesource.com/c/go/+/209817
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Anthony Knyszek [Thu, 14 Nov 2019 23:58:50 +0000 (23:58 +0000)]
runtime: convert page allocator bitmap to sparse array
Currently the page allocator bitmap is implemented as a single giant
memory mapping which is reserved at init time and committed as needed.
This causes problems on systems that don't handle large uncommitted
mappings well, or institute low virtual address space defaults as a
memory limiting mechanism.
This change modifies the implementation of the page allocator bitmap
away from a directly-mapped set of bytes to a sparse array in same vein
as mheap.arenas. This will hurt performance a little but the biggest
gains are from the lockless allocation possible with the page allocator,
so the impact of this extra layer of indirection should be minimal.
In fact, this is exactly what we see:
https://perf.golang.org/search?q=upload:20191125.5
This reduces the amount of mapped (PROT_NONE) memory needed on systems
with 48-bit address spaces to ~600 MiB down from almost 9 GiB. The bulk
of this remaining memory is used by the summaries.
Go processes with 32-bit address spaces now always commit to 128 KiB of
memory for the bitmap. Previously it would only commit the pages in the
bitmap which represented the range of addresses (lowest address to
highest address, even if there are unused regions in that range) used by
the heap.
Xiangdong Ji [Fri, 22 Nov 2019 17:02:06 +0000 (17:02 +0000)]
cmd/vet: honor analyzer flags when running vet outside $GOROOT/src
Additional vet flags specified by user are discarded if 'go vet'
is invoked outside $GOROOT/src to check a package under $GOROOT
(including those under "vendor" of $GOROOT), fix it by avoiding the
overwriting, the logic of detemining if the package under vetting
comes from $GOROOT remains untouched.
Also checked 'go tool vet <options> <cfg>' and 'go vet <options>
<user pkg>', both worked w./w.o this fix.
Fixes #35837.
Change-Id: I549af7964e40440afd35f2d1971f77eee6f8de34
Reviewed-on: https://go-review.googlesource.com/c/go/+/209498
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Ian Lance Taylor [Mon, 2 Dec 2019 20:07:22 +0000 (12:07 -0800)]
runtime: use current P's race context in timer code
We were using the race context of the P that held the timer,
but since we unlock the P's timers while executing a timer
that could lead to a race on the race context itself.
Updates #6239
Updates #27707
Fixes #35906
Change-Id: I5f9d5f52d8e28dffb88c3327301071b16ed1a913
Reviewed-on: https://go-review.googlesource.com/c/go/+/209580
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Than McIntosh [Mon, 25 Nov 2019 19:07:59 +0000 (14:07 -0500)]
cmd/link: additional fixes for -newobj and "ld -r" ELF host objects
The previous fix for this issue (CL 208479) was not general enough;
this patch revises it to handle more cases.
The problem with the original fix was that once a sym.Symbol is
created for a given static symbol and given a bogus anonymous version
of -1, we hit problems if some other non-anonymous symbol (created by
host object loading) had relocations targeting the static symbol.
In this patch instead of assigning a fixed anonymous version of -1 to
such symbols, each time loader.Create is invoked we create a new
(unique) anonymous version for the sym.Symbol, then enter the result
into the loader's extStaticSyms map, permitting it to be found in
lookups when processing relocation targets.
NB: this code will hopefully get a lot simpler once we can move host
object loading away from early sym.Symbol creation.
Richard Miller [Tue, 19 Nov 2019 13:15:28 +0000 (13:15 +0000)]
runtime: on plan9 don't return substitute address for sysReserve
Plan 9 doesn't have a way to reserve virtual memory, so the
implementation of sysReserve allocates memory space (which won't
be backed with real pages until the virtual pages are referenced).
If the space is then freed with sysFree, it's not returned to
the OS (because Plan 9 doesn't allow shrinking a shared address
space), but it must be cleared to zeroes in case it's reallocated
subsequently.
This interacts badly with the way mallocinit on 64-bit machines
sets up the heap, calling sysReserve repeatedly for a very large
(64MB?) arena with a non-nil address hint, and then freeing the space
again because it doesn't have the expected alignment. The
repeated clearing of multiple megabytes adds significant startup
time to every go program.
We correct this by restricting sysReserve to allocate memory only
when the caller doesn't provide an address hint. If a hint is
provided, sysReserve will now return nil instead of allocating memory
at a different address.
Fixes #27744
Change-Id: Iae5a950adefe4274c4bc64dd9c740d19afe4ed1c
Reviewed-on: https://go-review.googlesource.com/c/go/+/207917
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David du Colombier <0intro@gmail.com>
Michael Anthony Knyszek [Fri, 22 Nov 2019 21:06:15 +0000 (21:06 +0000)]
runtime: ready scavenger without next
This change makes it so that waking up the scavenger readies its
goroutine without "next" set, so that it doesn't interfere with the
application's use of the runnext feature in the scheduler which helps
fairness.
As of CL 201763 the scavenger began waking up much more often, and in
TestPingPongHog this meant that it would sometimes supercede either a
hog or light goroutine in runnext, leading to a skew in the results and
ultimately a test flake.
This change thus re-enables the TestPingPongHog test on the builders.
Michael Anthony Knyszek [Tue, 26 Nov 2019 21:16:43 +0000 (21:16 +0000)]
runtime: reset scavenge address in scavengeAll
Currently scavengeAll (which is called by debug.FreeOSMemory) doesn't
reset the scavenge address before scavenging, meaning it could miss
large portions of the heap. Fix this by reseting the address before
scavenging, which will ensure it is able to walk over the entire heap.
Brad Fitzpatrick [Tue, 26 Nov 2019 23:57:35 +0000 (23:57 +0000)]
net/http: update bundled x/net/http2
Updates bundled http2 to x/net git rev ef20fe5d7 for:
http2: make Transport.IdleConnTimeout consider wall (not monotonic) time
https://golang.org/cl/208798 (#29308)
http2: make CipherSuites validation error more verbose
https://golang.org/cl/200317 (#34776)
http2: track unread bytes when the pipe is broken
https://golang.org/cl/187377 (#28634)
http2: split cookie pair into separate hpack header fields
https://golang.org/cl/155657 (#29386)
Fixes #29308
Fixes #28634
Change-Id: I71a03ca62ccb5ff35a5cfadd8dc705a4491ae7ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/209077 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Fri, 15 Nov 2019 02:34:35 +0000 (21:34 -0500)]
runtime: print more information on stack overflow
Print the current SP and (old) stack bounds when the stack grows
too large. This helps to identify the problem: whether a large
stack is used, or something else goes wrong.
Cherry Zhang [Fri, 15 Nov 2019 01:23:17 +0000 (20:23 -0500)]
cmd/internal/obj: mark split-stack prologue nonpreemptible
When there are both a synchronous preemption request (by
clobbering the stack guard) and an asynchronous one (by signal),
the running goroutine may observe the synchronous request first
in stack bounds check, and go to the path of calling morestack.
If the preemption signal arrives at this point before the call to
morestack, the goroutine will be asynchronously preempted,
entering the scheduler. When it is resumed, the scheduler clears
the preemption request, unclobbers the stack guard. But the
resumed goroutine will still call morestack, as it is already on
its way. morestack will, as there is no preemption request,
double the stack unnecessarily. If this happens multiple times,
the stack may grow too big, although only a small amount is
actually used.
To fix this, we mark the stack bounds check and the call to
morestack async-nonpreemptible, starting after the memory
instruction (mostly a load, on x86 CMP with memory).
Not done for Wasm as it does not support async preemption.
Fixes #35470.
Change-Id: Ibd7f3d935a3649b80f47539116ec9b9556680cf2
Reviewed-on: https://go-review.googlesource.com/c/go/+/207350 Reviewed-by: David Chase <drchase@google.com>
Cherry Zhang [Thu, 14 Nov 2019 22:03:44 +0000 (17:03 -0500)]
cmd/internal/obj, runtime: use register map to mark unsafe points
Currently we use stack map index -2 to mark unsafe points, i.e.
PC ranges that is not safe for async preemption. This has a
problem: it cannot mark CALL instructions, because for stack scan
a valid stack map index is needed.
This CL switches to use register map index for marking unsafe
points instead, which does not conflict with stack scan and can
be applied on CALL instructions. This is necessary as next CL
will mark call to morestack nonpreemptible.
For #35470.
Change-Id: I357bf26c996e1fee1e7eebe4e6bb07d62930d3f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/207349 Reviewed-by: David Chase <drchase@google.com>
Cherry Zhang [Tue, 26 Nov 2019 04:29:02 +0000 (23:29 -0500)]
runtime: disable async preemption on darwin/arm(64) if no cgo
On darwin, we use libc calls, and cgo is required on ARM and
ARM64 so we have TLS set up to save/restore G during C calls. If
cgo is absent, we cannot save/restore G in TLS, and if a signal
is received during C execution we cannot get the G. Therefore
don't send signals (and hope that we won't receive any signal
during C execution).
This can only happen in the go_bootstrap program (otherwise cgo
is required).
Fixes #35800.
Change-Id: I6c02a9378af02c19d32749a42db45165b578188d
Reviewed-on: https://go-review.googlesource.com/c/go/+/208818
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Fri, 22 Nov 2019 21:06:11 +0000 (16:06 -0500)]
misc: remove use of relative directories in overlayDir functions
It turns out that the relative-path support never worked in the first
place.
It had been masked by the fact that we ~never invoke overlayDir with
an absolute path, which caused filepath.Rel to always return an error,
and overlayDir to always fall back to absolute paths.
Since the absolute paths seem to be working fine (and are simpler),
let's stick with those. As far as I can recall, the relative paths
were only a space optimization anyway.
Updates #28387
Updates #30316
Change-Id: Ie8cd28f3c41ca6497ace2799f4193d7f5dde7a37
Reviewed-on: https://go-review.googlesource.com/c/go/+/208481
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Dan Scales [Tue, 19 Nov 2019 20:41:51 +0000 (12:41 -0800)]
runtime: add go:nosplit to cgo_mmap.go:mmap() and sys_darwin.go:mmap()
cgo_mmap.go:mmap() is called by mem_linux.go:sysAlloc(), a low-level memory
allocation function. mmap() should be nosplit, since it is called in a lot of
low-level parts of the runtime and callers often assume it won't acquire any
locks.
As an example there is a potential deadlock involving two threads if mmap is not nosplit:
trace.bufLock acquired, then stackpool[order].item.mu, then mheap_.lock
- can happen for traceEvents that are not invoked on the system stack and cause
a traceFlush, which causes a sysAlloc, which calls mmap(), which may cause a
stack split. mheap_.lock
mheap_.lock acquired, then trace.bufLock
- can happen when doing a trace in reclaimChunk (which holds the mheap_ lock)
Also, sysAlloc() has a comment that it is nosplit because it may be invoked
without a valid G, in which case its callee mmap() should also be nosplit.
Similarly, sys_darwin.go:mmap() is called by mem_darwin.go:sysAlloc(), and should
be nosplit for the same reasons.
Extra gomote testing: linux/arm64, darwin/amd64
Change-Id: Ia4d10cec5cf1e186a0fe5aab2858c6e0e5b80fdc
Reviewed-on: https://go-review.googlesource.com/c/go/+/207844 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Than McIntosh [Sat, 23 Nov 2019 14:59:30 +0000 (09:59 -0500)]
cmd/link: disable new testpoint on mips pending investigation
Skip TestMinusRSymsWithSameName testpoint on MIPS for the time being
since it triggers failures on that arch. Will re-enable once the
problems are fixed.
Because of concurrent goroutines it is possible for multiple event
handlers to return at the same time. This was not properly supported
and caused the wrong goroutine to continue, which in turn caused
memory corruption.
This change adds a stack of events so it is always clear which is the
innermost event that needs to return next.
Than McIntosh [Fri, 22 Nov 2019 19:31:13 +0000 (14:31 -0500)]
cmd/link: add new linker testpoint for "ld -r" host object
This adds a new test that builds a small Go program with linked
against a *.syso file that is the result of an "ld -r" link. The
sysobj in question has multiple static symbols in the same section
with the same name, which triggered a bug in the loader in -newobj
mode.
Than McIntosh [Fri, 22 Nov 2019 20:27:01 +0000 (15:27 -0500)]
cmd/link: fix bug with -newobj and "ld -r" ELF host objects
When the ELF host object loader encounters a static/hidden symbol, it
creates a sym.Symbol for it but does not enter it into the sym.Symbols
lookup table. Under -newobj mode, this was not happening correctly; we
were adding the sym via loader.LookupOrCreate, which resulted in
collisions when it encountered symbols with the same name + version +
section (this can happen for "ld -r" objects).
Dmitri Shuralyov [Sat, 16 Nov 2019 23:19:33 +0000 (23:19 +0000)]
net/http: rename tests for Redirect and StripPrefix
Before May 2018, I mistakenly thought the _suffix naming convention¹
used by examples also applied to tests. Thanks to a code review comment²
from Ian Lance Taylor, I have since learned that is not true.
This trivial change fixes some collateral damage from my earlier
misunderstanding, resulting in improved test naming consistency.
Brad Fitzpatrick [Fri, 1 Nov 2019 18:23:05 +0000 (11:23 -0700)]
net/http: make Transport.IdleConnTimeout consider wall (not monotonic) time
Both laptops closing their lids and cloud container runtimes
suspending VMs both faced the problem where an idle HTTP connection
used by the Transport could be cached for later reuse before the
machine is frozen, only to wake up many minutes later to think that
their HTTP connection was still good (because only a second or two of
monotonic time passed), only to find out that the peer hung up on them
when they went to write.
HTTP/1 connection reuse is inherently racy like this, but no need for
us to step into a trap if we can avoid it. Also, not everybody sets
Request.GetBody to enable re-tryable POSTs. And we can only safely
retry requests in some cases.
So with this CL, before reusing an old connection, double check the walltime.
Testing was done both with a laptop (closing the lid for a bit) and
with QEMU, running "stop" and "cont" commands in the monitor and
sending QMP guest agent commands to update its wall clock after the
"cont":
Jay Conrod [Thu, 21 Nov 2019 21:55:11 +0000 (16:55 -0500)]
cmd/go: fix and re-enable build_trimpath test
The test was comparing a binary built from a list of files to a test
build from a named package. That should not (and did not) work. The
test now compares two binaries built the same way in different
directories.
Also add a portion of the test for GOPATH and fix the gccgo portion of
the test (verified manually).
Fixes #35435
Change-Id: I2535a0011c9d97d2274e5550ae277302dbb91e6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/208234
Run-TryBot: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills [Thu, 21 Nov 2019 21:26:09 +0000 (16:26 -0500)]
cmd/go: do not panic when computing Shlib for a package with no Target
In module mode, a non-main package lacks an install target.
The location of the .shlib corresponding to a given target is stored
in a .shlibname file alongside its install target, so in module mode
a non-main package also lacks a .shlibname file.
This also implies that such a package cannot be installed with
'go install -buildmode=linkshared', but that is a problem
for another day.
Fixes #35759
Updates #34347
Change-Id: Id3e0e068266d5fb9b061a59e70f9a65985d4973b
Reviewed-on: https://go-review.googlesource.com/c/go/+/208233
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>