I'm trying to keep the code changes minimal for backporting to Go 1.13,
so it is still possible for a handful of entries to leak,
but the leaks are now O(1) instead of O(N) in the steady state.
Longer-term, I think it would be a good idea to coalesce idleMu with
connsPerHostMu and clear entries out of both queues as soon as their
goroutines are done waiting.
Cherry-picked from CL 191964.
Updates #33849
Updates #33850Fixes#33878
Change-Id: Ia66bc64671eb1014369f2d3a01debfc023b44281
Reviewed-on: https://go-review.googlesource.com/c/go/+/191964
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 94bf9a8d4a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191967
Update vendored x/arch repo to pick up the fix of issue #33802.
This is done with the following commands:
$ cd $GOROOT/src/cmd
$ go get -d golang.org/x/arch@latest
go: finding golang.org/x/arch latest
go: downloading golang.org/x/arch v0.0.0-20190815191158-8a70ba74b3a1
go: extracting golang.org/x/arch v0.0.0-20190815191158-8a70ba74b3a1
$ go mod tidy
$ go mod vendor
Updates #33802.
Fixes#33811.
Change-Id: I0a44f1d83d6f573124cea1f099378b1c851f3feb
Reviewed-on: https://go-review.googlesource.com/c/go/+/191619
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
(cherry picked from commit 66ff373911)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191737
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Update golang.org/x/net to v0.0.0-20190813141303-74dc4d7220e7 to import
the following security fix.
commit 74dc4d7220e7acc4e100824340f3e66577424772
Author: Filippo Valsorda <filippo@golang.org>
Date: Sun Aug 11 02:12:18 2019 -0400
http2: limit number of control frames in server send queue
An attacker could cause servers to queue an unlimited number of PING
ACKs or RST_STREAM frames by soliciting them and not reading them, until
the program runs out of memory.
Limit control frames in the queue to a few thousands (matching the limit
imposed by other vendors) by counting as they enter and exit the scheduler,
so the protection will work with any WriteScheduler.
Once the limit is exceeded, close the connection, as we have no way to
communicate with the peer.
Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
This change was generated with cmd/go and cmd/bundle:
$ go get -u golang.org/x/net
$ go mod tidy
$ go mod vendor
$ go generate net/http
Fixes CVE-2019-9512 and CVE-2019-9514
Fixes#33606
Change-Id: I464baf96175006aa101d65d3b0f6494f28a626ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/190137
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
(cherry picked from commit 145e193131)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191618
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As of Go 1.13rc1, TimeoutHandler supports the Flusher and Pusher interfaces and
this change corrects its documentation to say that.
Fixes#33769
Updates #29193
Change-Id: Ia0523f7f2e3dc1f8f0b68950b85a7bf81c4abe60
GitHub-Last-Rev: 5310d2c960
GitHub-Pull-Request: golang/go#33770
Reviewed-on: https://go-review.googlesource.com/c/go/+/191237
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit f3e3b71a50)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191169
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This reverts CL 151157.
CL 151157 introduced a crash when decoding into ",string" fields. It
came with a moderate speedup, so at this stage of the release cycle
let's just revert it, and reapply it in Go 1.14 with the fix in CL 190659.
Also applied the test cases from CL 190659.
Updates #33728
Change-Id: Ie46e2bc15224b251888580daf6b79d5865f3878e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190909
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This CL makes the go command understand that
GOSUMDB=sum.golang.google.cn should connect
to that domain but expect to find a checksum database
signed by sum.golang.org there.
The host sum.golang.google.cn is not yet completely
configured; we hope it will be available in a few weeks.
Change-Id: Ie0fc4323f0c7084dda59bd3b45fc406717fa16d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/191137
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
The docs refer to "the last two paragraphs", but in fact should refer to
the first two of the previous three paragraphs. Moved up the out of place
paragraph.
Updates #14295
Change-Id: I066da7a665bc6754d246782b941af214a385017a
Reviewed-on: https://go-review.googlesource.com/c/go/+/190839
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently test build fails with:
$ go test -tags=gofuzz encoding/json
encoding/json/fuzz.go:36:4: Println call has possible formatting directive %s
FAIL encoding/json [build failed]
Change-Id: I23aef44a421ed0e7bcf48b74ac5a8c6768a4841b
Reviewed-on: https://go-review.googlesource.com/c/go/+/190698
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 131196 optimized Time.Sub, but was reverted because
it incorrectly computed the nanoseconds in some edge cases.
This CL adds a test case to enforce the correct behavior
so that a future optimization does not break this again.
Updates #17858
Updates #33677
Change-Id: I596d8302ca6bf721cf7ca11cc6f939639fcbdd43
Reviewed-on: https://go-review.googlesource.com/c/go/+/190524
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 162337 changed go/ast to better handle block comments,
but was reverted because it introduced an off-by-one bug.
This CL adds a test case to enforce the correct behavior
so that future changes do not break this again.
Updates #18929
Updates #33538
Change-Id: I2d25c139d007f8db1091b7a48b1dd20c584e2699
Reviewed-on: https://go-review.googlesource.com/c/go/+/190523
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This reverts commit CL 131196 because there is a bug
in the calculation of nanoseconds.
Fixes#33677
Change-Id: Ic8e94c547ee29b8aeda1b9a5cb9764dbf47b14b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/190497
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
When calling a function obtained from reflect.Value.Method (or
MethodByName), we copy the arguments from the caller frame, which
does not include the receiver, to a new frame to call the actual
method, which does include the receiver. Here we need to align
the first (non-receiver) argument. As the receiver is pointer
sized, it is generally naturally aligned, except on amd64p32,
where the argument can have larger alignment, and this aligning
becomes necessary.
Fixes#33628.
Change-Id: I5bea0e20173f06d1602c5666d4f334e3d0de5c1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190297
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When Host is not valid per RFC 3986, the behavior of Hostname and Port
was wildly unpredictable, to the point that Host could have a suffix
that didn't appear in neither Hostname nor Port.
This is a security issue when applications are applying checks to Host
and expecting them to be meaningful for the contents of Hostname.
To reduce disruption, this change only aims to guarantee the following
two security-relevant invariants.
* Host is either Hostname or [Hostname] with Port empty, or
Hostname:Port or [Hostname]:Port.
* Port is only decimals.
The second invariant is the one that's most likely to cause disruption,
but I believe it's important, as it's conceivable an application might
do a suffix check on Host and expect it to be meaningful for the
contents of Hostname (if the suffix is not a valid port).
There are three ways to ensure it.
1) Reject invalid ports in Parse. Note that non-numeric ports are
already rejected if and only if the host starts with "[".
2) Consider non-numeric ports as part of Hostname, not Port.
3) Allow non-numeric ports, and hope they only flow down to net/http,
which will reject them (#14353).
This change adopts both 1 and 2. We could do only the latter, but then
these invalid hosts would flow past port checks, like in
http_test.TestTransportRejectsAlphaPort. Non-numeric ports weren't fully
supported anyway, because they were rejected after IPv6 literals, so
this restores consistency. We could do only the former, but at this
point 2) is free and might help with manually constructed Host values
(or if we get something wrong in Parse).
Note that net.SplitHostPort and net.Dial explicitly accept service names
in place of port numbers, but this is an URL package, and RFC 3986,
Section 3.2.3, clearly specifies ports as a number in decimal.
net/http uses a mix of net.SplitHostPort and url.Parse that would
deserve looking into, but in general it seems that it will still accept
service names in Addr fields as they are passed to net.Listen, while
rejecting them in URLs, which feels correct.
This leaves a number of invalid URLs to reject, which however are not
security relevant once the two invariants above hold, so can be done in
Go 1.14: IPv6 literals without brackets (#31024), invalid IPv6 literals,
hostnames with invalid characters, and more.
Tested with 200M executions of go-fuzz and the following Fuzz function.
u, err := url.Parse(string(data))
if err != nil {
return 0
}
h := u.Hostname()
p := u.Port()
switch u.Host {
case h + ":" + p:
return 1
case "[" + h + "]:" + p:
return 1
case h:
fallthrough
case "[" + h + "]":
if p != "" {
panic("unexpected Port()")
}
return 1
}
panic("Host is not a variant of [Hostname]:Port")
Fixes CVE-2019-14809
Updates #29098
Change-Id: I7ef40823dab28f29511329fa2d5a7fb10c3ec895
Reviewed-on: https://go-review.googlesource.com/c/go/+/189258
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The bundle included changes from a commit after the one referred to by
the go.mod, probably due to cmd/bundle using the GOPATH source.
Identified with the new go/packages based cmd/bundle from CL 189818.
$ go get golang.org/x/net@461777fb6f
$ go mod tidy
$ go mod vendor
$ go generate net/http # with CL 189818
Also, updated the socks_bundle.go generate command to drop obsolete
options and match h2_bundle.go. It caused no output changes.
Updates #32031
Change-Id: I0322d4e842dbfdad749455111072ca4872a62ad4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189897
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
There is real (albeit generated) code that exceeds the limit.
Fixes#33555
Change-Id: I668e85825d3d2a471970e869abe63f3492213cc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/189697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
If we don't know whether a path is a module path or a package path,
previously we would first try a module query for it, then fall back to
a package query.
If we are using a sequence of proxies with fallback (as will be the
default in Go 1.13), and the path is not actually a module path, that
initial module query will fail against the first proxy, then
immediately fall back to the next proxy in the sequence — even if the
query could have been satisfied by some other (prefix) module
available from the first proxy.
Instead, we now query the requested path as only one kind of path.
If we query it as a package path but it turns out to only exist as a
module, we can detect that as a PackageNotInModuleError with an
appropriate module path — we do not need to issue a second query to
classify it.
Fixes#31785
Change-Id: I581d44279196e41d1fed27ec25489e75d62654e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/189517
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
modload.ListModules now wraps errors as module.ModuleError as
appropriate. The resulting errors always include the module path and
will include the version, if known.
'go mod download' no longer ignores errors reported by ListModules.
Previously, it started requesting module info, go.mod, and zip. Those
requests would fail, overwriting the original failure. They were
usually less descriptive.
'go mod download' with a module not in the build list (and no version
query) is now an error. Previously, this was silently ignored.
Fixes#30743
Change-Id: Icee8c1c6c5240de135a8b6ba42d6bbcdb757cdac
Reviewed-on: https://go-review.googlesource.com/c/go/+/189323
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This reverts CL 162337.
Reason for revert: this introduces a regression
Fixes#33538
Updates #18929
Change-Id: Ib2320a840c6d3ec7912e8f414e933d04fbf11ab4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189379
Reviewed-by: Robert Griesemer <gri@golang.org>
Document goroutine label inheritance. Goroutine labels are copied upon
goroutine creation and there is a test enforcing this, but it was not
mentioned in the docstrings for `Do` or `SetGoroutineLabels`.
Add notes to both of those functions' docstrings so it's clear that one
does not need to set labels as soon as a new goroutine is spawned if
they want to propagate tags.
Updates #32223
Updates #23458
Change-Id: Idfa33031af0104b884b03ca855ac82b98500c8b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189317
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In modload.Import, confirm that the import path does not start with
"cmd/" before calling QueryPackage, which returns a less helpful
error.
In load.loadPackageData, don't wrap errors with "unknown import path".
The wrapped error should always include the import path, and it's also
repeated in the PackageError wrapper.
Fixes#31031
Change-Id: I071efa22e3842c62831d096f888a8006811fe724
Reviewed-on: https://go-review.googlesource.com/c/go/+/189157
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This mirrors the ELF fix in CL 188957. TestScript/version failed on
darwin after that change.
Fixes#31861
Change-Id: I4ce953ebec8dd5fa47e26d373c59d7e290b75a34
Reviewed-on: https://go-review.googlesource.com/c/go/+/189159
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Explain wrapping and how to use Is and As in the package doc.
Explain "chain" in Is and As.
Updates #33364.
Change-Id: Ic06362106dbd129e33dd47e63176ee5355492086
Reviewed-on: https://go-review.googlesource.com/c/go/+/188737
Reviewed-by: Rob Pike <r@golang.org>
CL 188817 improved the error message for a nonexistent version in a dependency.
This CL locks in that improvement in a regression test.
Fixes#33474
Change-Id: I6246b4995adee966f24eaebe491d35830aea8370
Reviewed-on: https://go-review.googlesource.com/c/go/+/188977
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Based on original fix from Mark Glines in golang.org/cl/186737
Fixes#31861
Change-Id: Ibd583a3aa8f8b8eefade998aa2ac757b55140937
Reviewed-on: https://go-review.googlesource.com/c/go/+/188957
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The two methods act the same, so make their documentation similar so
that people don't think they act differently.
Change-Id: If224692ef50870faf855d789380a614d1e724132
Reviewed-on: https://go-review.googlesource.com/c/go/+/188137
Reviewed-by: Rob Pike <r@golang.org>
CL #163058 moves interpretation of platform-specific errors to the
syscall package. Package syscall errors implement an Is method which
os.IsPermission etc. consult. This results in an unintended semantic
change to the os package predicate functions: The following program
now prints 'true' where it used to print 'false':
package main
import "os"
type myError struct{ error }
func (e myError) Is(target error) bool { return target == os.ErrPermission }
func main() { println(os.IsPermission(myError{})) }
Change the os package error predicate functions to only examine syscall
errors, avoiding this semantic change.
This CL does retain one minor semantic change: On Plan9, os.IsPermission
used to return true for any error with text containing the string
"permission denied". It now only returns true for a syscall.ErrorString
containing that text.
Change-Id: I6b512b1de6ced46c2f1cc8d264fa2495ae7bf9f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/188817
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The compiler can crash if the compiled code tries to
unconditionally read from a nil pointer. This should cause
the generated binary to panic, not the compiler.
Fixes#33438
Change-Id: Ic8fa89646d6968e2cc4e27da0ad9286662f8bc49
Reviewed-on: https://go-review.googlesource.com/c/go/+/188760
Reviewed-by: Austin Clements <austin@google.com>
It is unclear whether the current definition of os.IsTimeout is
desirable or not. Drop ErrTimeout for now so we can consider adding it
(or some other error) in a future release with a corrected definition.
Fixes#33411
Change-Id: I8b880da7d22afc343a08339eb5f0efd1075ecafe
Reviewed-on: https://go-review.googlesource.com/c/go/+/188758
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There's a race here with fork/exec, enable the close-on-exec flag
for the new file descriptor.
Fixes#33405
Change-Id: If95bae97a52b7026a930bb3427e47bae3b0032ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/188537
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As discussed in
https://github.com/golang/go/issues/32463#issuecomment-506833421
the classification of deadline-based timeouts as "temporary" errors is a
historical accident. I/O timeouts used to be duration-based, so they
really were temporary--retrying a timed-out operation could succeed. Now
that they're deadline-based, timeouts aren't temporary unless you reset
the deadline.
Drop ErrTemporary from Go 1.13, since its definition is wrong. We'll
consider putting it back in Go 1.14 with a clear definition and
deprecate net.OpError.Temporary.
Fixes#32463
Change-Id: I70cda664590d8872541e17409a5780da76920891
Reviewed-on: https://go-review.googlesource.com/c/go/+/188398
Reviewed-by: Jonathan Amsterdam <jba@google.com>
We shouldn't mask to desired registers if we haven't masked out all the
forbidden registers yet. In this path we haven't masked out the nospill
registers yet. If the resulting mask contains only nospill registers, then
allocReg fails.
This can only happen on resultNotInArgs-marked instructions, which exist
only on the ARM64, MIPS, MIPS64, and PPC64 ports.
Maybe there's a better way to handle resultNotInArgs instructions.
But for 1.13, this is a low-risk fix.
Fixes#33355
Change-Id: I1082f78f798d1371bde65c58cc265540480e4fa4
Reviewed-on: https://go-review.googlesource.com/c/go/+/188178
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Also add other gccgo options.
This ports CL 45695 and CL 48592 from the gofrontend repo to the gc repo.
CL 45695 (partial entry, other parts out of date and not ported):
cmd/go: gccgo: consistent results
Pass the -fdebug-prefix-map and -gno-record-gcc-switches compiler
options to gccgo to generate consistent results.
CL 48592:
cmd/go: use gccSupportsFlag for -fsplit-stack
Don't assume that all (or only) 386/amd64 compilers support
-fsplit-stack.
Fixes#33108
Change-Id: I61f9e5a67e4fb059f26750e97621d27afa566ec2
Reviewed-on: https://go-review.googlesource.com/c/go/+/187824
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Request.PostForm gets populated with form data for PATCH, POST, or PUT
http verbs.
Change-Id: I33065aa78a8470c4e9490aac830aa6f5963c61cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/187821
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
ioutil.WriteFile always truncates the destination file to 0 before
writing, which is inappropriate for unsynchronized, idempotent,
fixed-size files such as the cache entry files here.
Instead, truncate the file only after writing it, so that a second
write will never (even temporarily!) remove the contents of a
preceding write.
Fixes#29667
Change-Id: I16a53ce79d8a23d23580511cb6abd062f54b65ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/188157
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Currently when we coalesce memory we make a sysHugePage call
(MADV_HUGEPAGE) to ensure freed and coalesced huge pages are treated as
such so the scavenger's assumptions about performance are more in line
with reality.
Unfortunately we do it way too often because we do it if there was any
change to the huge page count for the span we're coalescing into, not
taking into account that it could coalesce with its neighbors and not
actually create a new huge page.
This change makes it so that it only calls sysHugePage if the original
huge page counts between the span to be coalesced into and its neighbors
do not add up (i.e. a new huge page was created due to alignment). Calls
to sysHugePage will now happen much less frequently, as intended.
Updates #32828.
Change-Id: Ia175919cb79b730a658250425f97189e27d7fda3
Reviewed-on: https://go-review.googlesource.com/c/go/+/186926
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change adds physHugePageShift which is defined such that
1 << physHugePageShift == physHugePageSize. The purpose of this variable
is to avoid doing expensive divisions in key functions, such as
(*mspan).hugePages.
This change also does a sweep of any place we might do a division or mod
operation with physHugePageSize and turns it into bit shifts and other
bitwise operations.
Finally, this change adds a check to mallocinit which ensures that
physHugePageSize is always a power of two. osinit might choose to ignore
non-powers-of-two for the value and replace it with zero, but mallocinit
will fail if it's not a power of two (or zero). It also derives
physHugePageShift from physHugePageSize.
This change helps improve the performance of most applications because
of how often (*mspan).hugePages is called.
Updates #32828.
Change-Id: I1a6db113d52d563f59ae8fd4f0e130858859e68f
Reviewed-on: https://go-review.googlesource.com/c/go/+/186598
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This is intended to help clear up confusion around the usage of the
Title and ToTitle functions. It includes a link to define title case
to distinguish it from upper case. It also includes an additional
example for the ToTitle function to showcase the difference in behavior
between it and the Title function.
Fixes#33302
Change-Id: I44e62962fb04d0d22966a39eda3a2d16de7a2291
Reviewed-on: https://go-review.googlesource.com/c/go/+/187825
Reviewed-by: Rob Pike <r@golang.org>
Overflow of the comparison caused very large (>=1<<32) allocations to
sometimes not get sampled at all. Use uintptr so the comparison will
never overflow.
Fixes#33342
Tested on the example in 33342. I don't want to check a test in that
needs that much memory, however.
Change-Id: I51fe77a9117affed8094da93c0bc5f445ac2d3d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/188017
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Document that *os.File is subject to resource limits
for concurrent operations. We aren't documenting
a specific number of concurrent operations because that
number is OS/system dependent. This limit comes from:
internal/poll/fd_mutex.go
where we use 20 bits to count locks.
Fixes#32544
Change-Id: I7d305d4aaba5b2dbc6f1ab8c447117fde5e31a66
Reviewed-on: https://go-review.googlesource.com/c/go/+/181841
Reviewed-by: Rob Pike <r@golang.org>
This reverts CL 176098.
Reason for revert: added complexity, but did not completely fix the
underlying problem. A complete solution would not be worth the
complexity, and as a partial solution this is probably not worth the
complexity either.
Updates #31859
Change-Id: Ifd34c292fd1b811c60afe3c339e5edd3f37190c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/186817
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>
Currently, GODEBUG=sbrk=1 mode aligns allocations by their type's
alignment. You would think this would be the right thing to do, but
because 64-bit fields are only 4-byte aligned right now (see #599),
this can cause a 64-bit field of an allocated object to be 4-byte
aligned, but not 8-byte aligned. If there is an atomic access to that
unaligned 64-bit field, it will crash.
This doesn't happen in normal allocation mode because the
size-segregated allocation and the current size classes will cause any
types larger than 8 bytes to be 8 byte aligned.
We fix this by making sbrk=1 mode use alignment based on the type's
size rather than its declared alignment. This matches how the tiny
allocator aligns allocations.
This was tested with
GOARCH=386 GODEBUG=sbrk=1 go test sync/atomic
This crashes with an unaligned access before this change, and passes
with this change.
This should be reverted when/if we fix#599.
Fixes#33159.
Change-Id: Ifc52c72c6b99c5d370476685271baa43ad907565
Reviewed-on: https://go-review.googlesource.com/c/go/+/186919
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
'go test' uses the Root field to determine the set of files that
invalidate test results, and there is no other sensible meaning of
“root” for code within a module.
Fixes#29111
Change-Id: Icf1be90a26d22665613e42cb968087b63c36e74c
Reviewed-on: https://go-review.googlesource.com/c/go/+/154100
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The go command invokes each tool with -V=full to discover its version
to compute a tool id. For release versions (that don't include the
word "devel"), the go command only used the third word in
the output (e.g., "go1.13"), ignoring any toolchain experiments that
followed. With this change, the go command will use whole version line
in the tool id for release versions.
Also, when -V=full is set and there are non-default experiments,
experiments are no longer printed twice.
Fixes#33091
Change-Id: I19b96f939c7e2fbc5d8befe3659156ee4b58daef
Reviewed-on: https://go-review.googlesource.com/c/go/+/186200
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
In Go1.12 and below, the logic in ReverseProxy.ServeHTTP would always
allocate request.Header even if it were not present in the incoming request.
CL 174324 added http.Request.Clone and re-factors ReverseProxy.ServeHTTP
to use the new Clone method. However, the new Clone logic is not equivalent
to the former logic. We preserve former semantics by explicitly allocating
the Header map if nil.
Fixes#33142
Change-Id: I356f94a915dd9779584ce3fe31e56e5474b9ad37
Reviewed-on: https://go-review.googlesource.com/c/go/+/186437
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Changes "was loaded as" to "was required as". This is slightly more
precise, since it hints at a requirement edge in the module version
graph.
Updates #28489
Change-Id: I636268c33f1ea9858c214fe275f271538186ed6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/186377
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
With gccgo, if we generate getg inlined, the backend may cache
the address of the TLS variable, which will become invalid after
a thread switch.
Currently there is no known bug for this. But if we didn't
implement this carefully, we may get subtle bugs. This CL adds a
test that will fail loudly if this is wrong. (See also
https://go.googlesource.com/gofrontend/+/refs/heads/master/libgo/runtime/proc.c#333
and an incorrect attempt CL 185337.)
Note: at least on Linux/AMD64, even with an incorrect
implementation, this only fails if the test is compiled with
-fPIC, which is not the default setting for gccgo test suite. So
some manual work is needed. Maybe we could extend the test suite
to run the runtime test with more settings (e.g. PIC and static).
Change-Id: I459a3b4c31f09b9785c0eca19b7756f80e8ef54c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186357
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Do not allow a pseudo-version derived from a canonical tag to refer to
the same revision as the tag itself. It's unnecessary (because
canonical tags already have a total ordering) and confusing (the
pseudo-version appears to come after the tag, but actually refers to
the exact same revision).
Updates #32879
Updates #27173
Change-Id: I02befedbe89c8819bdd93e470783ce63fc813193
Reviewed-on: https://go-review.googlesource.com/c/go/+/184720
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This error occurs when a module is loaded with one name (for example,
github.com/golang/lint) but declares a different path in its go.mod
(golang.org/x/lint). The current text "unexpected module path" is
confusing. It doesn't explain why the path was unexpected, and it's
not clear what was expected.
With this change, the error text includes the module and version
containing the go.mod file with the error, the declared module path,
and the loaded module path. The paths are vertically aligned so
differences are visually obvious. As with other module version errors,
the shortest chain of requirements is printed.
This change supercedes CL 158477.
Fixes#28489
Change-Id: Ieb07d00bcae182376d7be6aad111c84fbf784354
Reviewed-on: https://go-review.googlesource.com/c/go/+/185985
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
These functions are compiler generated, and as such are only available
in the internal ABI. Doing this avoids generating an alias symbol.
Doing that avoids confusion between unmangled and mangled type symbols.
Fixes#30768
Change-Id: I197a5ba6403aac11989ffa951dbe35bd0506de91
Reviewed-on: https://go-review.googlesource.com/c/go/+/186077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
If the module path declared in the go.mod file does not match the path
we are trying to resolve, a build using that module is doomed to fail.
Since we know that the module path does not match in the underlying
repo, we also know that the requested module does not exist at the
requested version.
Therefore, we should reject that version in Stat with a “not exist”
error — sooner rather than later — so that modload.Query will continue
to check other candidate paths (for example, with a major-version
suffix added or removed).
Fixes#33099
Change-Id: I43c980f78ed75fa6ace90f237cc3aad46c22d83a
Reviewed-on: https://go-review.googlesource.com/c/go/+/186237
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Session resumption is not a reliable TLS behavior: the server can decide
to reject a session ticket for a number of reasons, or no reason at all.
This makes this non-hermetic test extremely brittle.
It's currently broken on the builders for both TLS 1.2 and TLS 1.3, and
I could reproduce the issue for TLS 1.3 only. As I was debugging it, it
started passing entirely on my machine.
In practice, it doesn't get us any coverage as resumption is already
tested with the recorded exchange tests, and TestVerifyHostname still
provides a smoke test checking that we can in fact talk TLS.
Fixes#32978
Change-Id: I63505e22ff7704f25ad700d46e4ff14850ba5d3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186239
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This is a more minimial fix for the immediate symptom of 32917 and
30590, but does not improve 'list -e' behavior or error
messages resulting from other package loading issues.
Fixes#32917Fixes#30590
Change-Id: I6088d14d864410159ebf228d9392d186322fd2a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/185417
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The go test result must not be cached when each of known variables to go
command change.
To do this, add all known variables to test metadata.
Fixes#32285
Change-Id: I90be6a72f46c42d965aec4fed534c0623244cd3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/179040
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Add "upgrade" and "patch" to 'go help modules' section 'Module queries'.
Also explicitly call out the fact that @v2 will select the latest
version starting with v2, not the branch named v2, since this is a
common source of confusion.
Fixes#33010
Change-Id: I2fe27543b81a160fb6f6b8e8444a7a35f3661433
Reviewed-on: https://go-review.googlesource.com/c/go/+/185979
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
We already skipped rewriting the call if there were fewer args than
parameters. But we can also get a cgo crash if there are more args,
if at least one of the extra args uses a name qualified with "C.".
Skip the rewrite, since the build will fail later anyhow.
Fixes#33061
Change-Id: I62ff3518b775b502ad10c2bacf9102db4c9a531c
Reviewed-on: https://go-review.googlesource.com/c/go/+/185797
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This test sometimes times out when the machine is busy.
The reason behind is still a bit blurry. But it seems to comes from
the fact that on AIX, once a listen is performed a socket, every
connection will be accepted even before an accept is made (which only
occurs when a machine is busy). On Linux, a socket is created as a
"passive socket" which seems to wait for the accept before allowing
incoming connections.
Updates #29685
Change-Id: I41b053b7d5f5b4420b72d6a217be72e41220d769
Reviewed-on: https://go-review.googlesource.com/c/go/+/185717
Run-TryBot: Clément Chigot <clement.chigot@atos.net>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Update NewReplacer documentation to specify that in the case of
multiple matches at the same position, the matching old/new
pair that appears first in NewReplacer arguments takes precedence.
Fixes#32699
Change-Id: I9d0616d28e5cd8c9bfa301be201f2b0ebf361dff
Reviewed-on: https://go-review.googlesource.com/c/go/+/185099
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This adds comments explaining why it's important that some panics are
allowed in the runtime (even though this isn't ideal).
Change-Id: I04c6fc4f792f3793f951619ccaea6bfef2f1763c
Reviewed-on: https://go-review.googlesource.com/c/go/+/181737
Reviewed-by: Keith Randall <khr@golang.org>
According to the documentation "When passing a pointer to a field in a
struct, the Go memory in question is the memory occupied by the field,
not the entire struct.". checkAddr states that this should also work
with type conversions, which is implemented in isType. However,
ast.StarExpr must be enclosed in ast.ParenExpr according to the go spec
(see example below), which is not considered in the checks.
Example:
// struct Si { int i; int *p; }; void f(struct I *x) {}
import "C"
type S {
p *int
i C.struct_Si
}
func main() {
v := &S{new(int)}
C.f((*C.struct_I)(&v.i)) // <- panic
}
This example will cause cgo to emit a cgoCheck that checks the whole
struct S instead of just S.i causing the panic "cgo argument has Go
pointer to Go pointer".
This patch fixes this situation by adding support for ast.ParenExpr to
isType and adds a test, that fails without the fix.
Fixes#32970.
Change-Id: I15ea28c98f839e9fa708859ed107a2e5f1483133
Reviewed-on: https://go-review.googlesource.com/c/go/+/185098
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For OLSH/ORSH, the right node is not a uintptr-typed. However,
unsafeValue still be called recursively for it, causing the
compiler crashes.
To fixing, the right node only needs to be evaluated
for side-effects, so just discard its value.
Fixes#32959
Change-Id: I34d5aa0823a0545f6dad1ec34774235ecf11addc
Reviewed-on: https://go-review.googlesource.com/c/go/+/185039
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit https://golang.org/cl/161177/.
Reason for revert: this led to non-contiguous comments spaced
by an empty line to be grouped into a single CommentGroup
Fixes#32944
Updates #10858
Change-Id: I5e16663b308c3b560496da8e66c33befdf9ed9dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/185040
Reviewed-by: Robert Griesemer <gri@golang.org>
There were at least three races in the implementation of the pool of
idle HTTP connections before this CL.
The first race is that HTTP/2 connections can be shared for many
requests, but each requesting goroutine would take the connection out
of the pool and then immediately return it before using it; this
created unnecessary, tiny little race windows during which another
goroutine might dial a second connection instead of reusing the first.
This CL changes the idle pool to just leave the HTTP/2 connection in
the pool permanently (until there is reason to close it), instead of
doing the take-it-out-put-it-back dance race.
The second race is that “is there an idle connection?” and
“register to wait for an idle connection” were implemented as two
separate steps, in different critical sections. So a client could end
up registered to wait for an idle connection and be waiting or perhaps
dialing, not having noticed the idle connection sitting in the pool
that arrived between the two steps.
The third race is that t.getIdleConnCh assumes that the inability to
send on the channel means the client doesn't need the result, when it
could mean that the client has not yet entered the select.
That is, the main dial does:
idleConnCh := t.getIdleConnCh(cm)
select {
case v := <-dialc:
...
case pc := <-idleConnCh
...
...
}
But then tryPutIdleConn does:
waitingDialer := t.idleConnCh[key] // what getIdleConnCh(cm) returned
select {
case waitingDialer <- pconn:
// We're done ...
return nil
default:
if waitingDialer != nil {
// They had populated this, but their dial won
// first, so we can clean up this map entry.
delete(t.idleConnCh, key)
}
}
If the client has returned from getIdleConnCh but not yet reached the
select, tryPutIdleConn will be unable to do the send, incorrectly
conclude that the client does not care anymore, and put the connection
in the idle pool instead, again leaving the client dialing unnecessarily
while a connection sits in the idle pool.
(It's also odd that the success case does not clean up the map entry,
and also that the map has room for only a single waiting goroutine for
a given host.)
None of these races mattered too much before Go 1.11: at most they
meant that connections were not reused quite as promptly as possible,
or a few more than necessary would be created. But Go 1.11 added
Transport.MaxConnsPerHost, which limited the number of connections
created for a given host. The default is 0 (unlimited), but if a user
did explicitly impose a low limit (2 is common), all these misplaced
conns could easily add up to the entire limit, causing a deadlock.
This was causing intermittent timeouts in TestTransportMaxConnsPerHost.
The addition of the MaxConnsPerHost support added its own races.
For example, here t.incHostConnCount could increment the count
and return a channel ready for receiving, and then the client would
not receive from it nor ever issue the decrement, because the select
need not evaluate these two cases in order:
select {
case <-t.incHostConnCount(cmKey):
// count below conn per host limit; proceed
case pc := <-t.getIdleConnCh(cm):
if trace != nil && trace.GotConn != nil {
trace.GotConn(httptrace.GotConnInfo{Conn: pc.conn, Reused: pc.isReused()})
}
return pc, nil
...
}
Obviously, unmatched increments are another way to get to a deadlock.
TestTransportMaxConnsPerHost deadlocked approximately 100% of
the time with a small random sleep added between incHostConnCount
and the select:
ch := t.incHostConnCount(cmKey):
time.Sleep(time.Duration(rand.Intn(10))*time.Millisecond)
select {
case <-ch
// count below conn per host limit; proceed
case pc := <-t.getIdleConnCh(cm):
...
}
The limit also did not properly apply to HTTP/2, because of the
decrement being attached to the underlying net.Conn.Close
and net/http not having access to the underlying HTTP/2 conn.
The alternate decrements for HTTP/2 may also have introduced
spurious decrements (discussion in #29889). Perhaps those
spurious decrements or other races caused the other intermittent
non-deadlock failures in TestTransportMaxConnsPerHost,
in which the HTTP/2 phase created too many connections (#31982).
This CL replaces the buggy, racy code with new code that is hopefully
neither buggy nor racy.
Fixes#29889.
Fixes#31982.
Fixes#32336.
Change-Id: I0dfac3a6fe8a6cdf5f0853722781fe2ec071ac97
Reviewed-on: https://go-review.googlesource.com/c/go/+/184262
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
It wasn't obeyed in the case where the MarshalJSON method uses a pointer
receiver, and the encoder grabs the address of a value to find that
method. addrMarshalerEncoder is the function that does this work, but it
ignored opts.escapeHTML.
Here's the before and after of the added test case, which was failing
before the fix. Now the two cases are correct and consistent.
{"NonPtr":"<str>","Ptr":"\u003cstr\u003e"}
{"NonPtr":"<str>","Ptr":"<str>"}
Fixes#32896.
Change-Id: Idc53077ece074973558bd3bb5ad036380db0d02c
Reviewed-on: https://go-review.googlesource.com/c/go/+/184757
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Caleb Spare <cespare@gmail.com>
cgo produces dynamic imports for Go binaries by scanning the dynamic
imports table of a binary produced by the system C compiler and
linker. Currently, since it uses elf.File.ImportedSymbols, it only
reads global symbols. Unfortunately, recent versions of lld emit weak
symbol imports for several pthread symbols, which means the cgo tool
doesn't emit dynamic imports for them, which ultimately causes linking
of cgo binaries to fail.
Fix this by using elf.File.DynamicSymbols instead and filtering down
to both global and weak symbols.
Fixes#31912.
Change-Id: If346a7eca6733e3bfa2cccf74a9cda02a3e81d38
Reviewed-on: https://go-review.googlesource.com/c/go/+/184100
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, File.ImportedSymbols is the only API that exposes the GNU
symbol version information for dynamic symbols. Unfortunately, it also
filters to specific types of symbols, and only returns symbol names.
The cgo tool is going to need symbol version information for more
symbols. In order to support this and make the API more orthogonal,
this CL adds version information to the Symbol type and updates
File.DynamicSymbols to fill this in. This has the downside of
increasing the size of Symbol, but seems to be the most natural API
for exposing this. I also explored 1) adding a method to get the
version information for the i'th dynamic symbol, but we don't use
symbol indexes anywhere else in the API, and it's not clear if this
index would be 0-based or 1-based, and 2) adding a
DynamicSymbolVersions method that returns a slice of version
information that parallels the DynamicSymbols slice, but that's less
efficient to implement and harder to use.
For #31912.
Change-Id: I69052ac3894f7af2aa9561f7085275130e0cf717
Reviewed-on: https://go-review.googlesource.com/c/go/+/184099
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Running the example code when not having permissions
to bind to port 80 will cause the program to hang after
printing the error message.
Change-Id: I2433ba2629b362fc8f1731e40cab5eea72ec354f
GitHub-Last-Rev: 0bb3dc08b6
GitHub-Pull-Request: golang/go#32947
Reviewed-on: https://go-review.googlesource.com/c/go/+/185157
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This should have been part of CL 184440.
Updates #32846
Change-Id: I78a1326f4a67b3b526859bd15cb9653b4a8551a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/184920
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
'go get path@latest' may now downgrade a module required at a
pre-release or pseudo-version newer than the latest released
version. This restores the 1.12 behavior and the ability to easily
roll back from a temporary development version.
'go get path@upgrade' is like @latest but will not downgrade.
If no version suffix is specified ('go get path'), @upgrade is
implied.
Fixes#32846
Change-Id: Ibec0628292ab1c484716a5add0950d7a7ee45f47
Reviewed-on: https://go-review.googlesource.com/c/go/+/184440
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The documentation just said "alphanumeric", but underscores and dots
are also accepted.
Fixes#32886
Change-Id: I1ba872a220d5c5bf64f1d851ddba9eb3b1afb89a
Reviewed-on: https://go-review.googlesource.com/c/go/+/184917
Reviewed-by: Bryan C. Mills <bcmills@google.com>
I accidentally fetched an invalid version of rsc.io/quote from
proxy.golang.org, which the proxy then cached and now includes in
https://proxy.golang.org/rsc.io/quote/@v/list.
That causes 'go get rsc.io/quote` to resolve to a different version
depending on whether the proxy is used.
Adjust the test to fetch an explicit version instead, since the choice
of 'latest' is mostly irrelevant to the checksum database logic that
the test is intended to verify.
Updates #32805Fixes#32900
Change-Id: I075b1f62e8c71545d0fb2dd4bd77ba525fc2a36d
Reviewed-on: https://go-review.googlesource.com/c/go/+/184719
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Taking over CL 162240, the original CL hasn't been making progress.
I just took the parts that fix the immediate issue. I left the
signatslice changes out, I don't think they are necessary.
Fixes#30202
Change-Id: I5b347605f0841dd925d5a73150b8bf269fa82464
Reviewed-on: https://go-review.googlesource.com/c/go/+/183852
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's not correct to use atomic.CompareAndSwap to implement Once.Do,
and we don't, but why we don't is a question that has come up
twice on golang-dev in the past few months.
Add a comment to help others with the same question.
Change-Id: Ia89ec9715cc5442c6e7f13e57a49c6cfe664d32c
Reviewed-on: https://go-review.googlesource.com/c/go/+/184261
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
The implementation of semaphores, and therefore notes, used on Darwin
is not async-signal-safe. The runtime has one case where a note needs
to be woken up from a signal handler: the call to notewakeup in sigsend.
That notewakeup call is only called on a single note, and it doesn't
need the full functionality of notes: nothing ever does a timed wait on it.
So change that one note to use a different implementation on Darwin,
based on a pipe. This lets the wakeup code use the write call, which is
async-signal-safe.
Fixes#31264
Change-Id: If705072d7a961dd908ea9d639c8d12b222c64806
Reviewed-on: https://go-review.googlesource.com/c/go/+/184169
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The localPipe implementation assumes that every successful net.Dial
results in exactly one successful listener.Accept. I don't believe this
is guaranteed by essentially any operating system. For this test, we're
seeing flakes on dragonfly (#29583).
But see also #19519, flakes due to the same assumption on FreeBSD
and macOS in package net's own tests.
This CL rewrites localPipe to try a few times to get a matching pair
of connections on the dial and accept side.
Fixes#29583.
Change-Id: Idb045b18c404eae457f091df20456c5ae879a291
Reviewed-on: https://go-review.googlesource.com/c/go/+/184157
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TestVariousDeadlines starts a client and server.
The client dials the server, sets a timeout on the connection,
reads from it, gets a timeout error, closes the connection.
The server writes an infinite stream of a's to each connection
it accepts.
The test was trying to run these in lockstep:
run a client dial+read+timeout+close,
wait for server to accept+write+error out on write to closed connection,
repeat.
On FreeBSD 11.2 and less frequently on macOS we see
the test timeout waiting for the server to do its half of
the lockstep dance.
I believe the problem is that the client can do its step
of the dance with such a short timeout that the read,
timeout, and close happens before the server ever returns
from the accept(2) system call. For the purposes of testing
the client-side read timeout, this is fine. But I suspect
that under some circumstances, the "TCP-accepted"
connection does not translate into a "socket-layer-accepted"
connection that triggers a return from accept(2).
That is, the Go server never sees the connection at all.
And the test sits there waiting for it to acknowledge
being done with a connection it never started with.
Fix the problem by not trying to lockstep with the server.
This definitely fixes the flake, since the specific line that
was calling t.Fatal is now deleted.
This exposes a different flake, seen on a trybot run for an
early version of this CL, in which the client's io.Copy does
not stop within the time allotted. The problem now is that
there is no guarantee that a read beyond the deadline with
available data returns an error instead of the available data,
yet the test assumes this guarantee, and in fact the opposite
is usually true - we don't bother checking the deadline unless
the read needs to block. That is, deadlines don't cut off a
flood of available data, yet this test thinks they do.
This CL therefore also changes the server not to send an
infinite flood of data - don't send any data at all - so that
the read deadline is guaranteed to be exercised.
Fixes#19519.
Change-Id: I58057c3ed94ac2aebab140ea597f317abae6e65e
Reviewed-on: https://go-review.googlesource.com/c/go/+/184137
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills
Cherry Zhang
Emmanuel T Odeke
Filippo Valsorda
Jeff Hodges
Russ Cox
Dmitry Vyukov
Joe Tsai
Joe Tsai
Ian Lance Taylor
Jay Conrod
Derek Phan
David Finkel
Bharath Thiruveedula
Jonathan Amsterdam
Keith Randall
Mark Glines
Shivani Singhal
Damien Neil
Keith Randall
Baokun Lee
Darren McCleary
Eli Bendersky
Andrew Bonventre
David Ndungu
Michael Knyszek
Andrew Todd
Dominik Honnef
Jordi Martin
Austin Clements
Allen Li
LE Manh Cuong
Clément Chigot
Francesco Renzi
Gernot Vormayr
Agniva De Sarker
Daniel Martí
Patrik Lundin