That way we will never have to look up the file/line for the frame
that's next to be returned when the user stops calling Next.
For the benchmark from #32093:
name old time/op new time/op delta
Helper-4 948ns ± 1% 836ns ± 3% -11.89% (p=0.000 n=9+9)
(#32093 was fixed with a more specific, and better, fix, but this
fix is much more general.)
Change-Id: I89e796f80c9706706d8d8b30eb14be3a8a442846
Reviewed-on: https://go-review.googlesource.com/c/go/+/178077
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, this test allocates many objects and relies on heap-growth
scavenging to happen unconditionally on heap-growth. However with the
new pacing system for the scavenging, this is no longer true and the
test is flaky.
So, this change overhauls TestPhysicalMemoryUtilization to check the
same aspect of the runtime, but in a much more robust way.
Firstly, it sets up a much more constrained scenario: only 5 objects are
allocated total with a maximum worst-case (i.e. the test fails) memory
footprint of about 16 MiB. The test is now aware that scavenging will
only happen if the heap growth causes us to push way past our scavenge
goal, which is based on the heap goal. So, it makes the holes in the
test much bigger and the actual retained allocations much smaller to
keep the heap goal at the heap's minimum size. It does this twice to
create exactly two unscavenged holes. Because the ratio between the size
of the "saved" objects and the "condemned" object is so small, two holes
are sufficient to create a consistent test.
Then, the test allocates one enormous object (the size of the 4 other
objects allocated, combined) with the intent that heap-growth scavenging
should kick in and scavenge the holes. The heap goal will rise after
this object is allocated, so it's very important we do all the
scavenging in a single allocation that exceeds the heap goal because
otherwise the rising heap goal could foil our test.
Finally, we check memory use relative to HeapAlloc as before. Since the
runtime should scavenge the entirety of the remaining holes,
theoretically there should be no more free and unscavenged memory.
However due to other allocations that may happen during the test we may
still see unscavenged memory, so we need to have some threshold. We keep
the current 10% threshold which, while arbitrary, is very conservative
and should easily account for any other allocations the test makes.
Before, we also had to ensure the allocations we were making looked
large relative to the size of a heap arena since newly-mapped memory was
considered unscavenged, and so that could significantly skew the test.
However, thanks to the fix for #32012 we were able to reduce memory use
to 16 MiB in the worst case.
Fixes#32010.
Change-Id: Ia38130481e292f581da7fa3289c98c99dc5394ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/177237
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If we look at the issues in the past releases that are related
to go command that involved modules, its usually mention or ask about
the value of GO111MODULE, either in separate line or in separate
comment.
There are quite long time range before GO111MODULE will be removed
(unused). The next release is still default to auto [1], and until Go
1.13 unsupported (two releases after that) there is about one and half
years after that.
Since the change is not that big (one line) [2], maybe temporary adding
it to "go env" give more clarity and benefit in issue reporting rather
than not.
[1] https://github.com/golang/go/issues/31857Fixes#29656
Change-Id: I609ad6664774018e4f4147ec6158485172968e16
Reviewed-on: https://go-review.googlesource.com/c/go/+/176837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
CL 14870 added internal/race to factor out duplicated race thunks,
we should use it.
No signification changes in compile time and compile binary size.
Change-Id: I786af44dd5bb0f4ab6709432eeb603f27a5b6c63
Reviewed-on: https://go-review.googlesource.com/c/go/+/178118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The "After it is called, changing rw.Header will not affect
rw.HeaderMap" claim predates the Result method which changed how the
Recorder should be used.
Fixes#32144Fixes#32136
Change-Id: I95bdfa5ac489ce7b0202824bb5663f4da188e8a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/178058
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
The chunks that were referenced by fd.iovecs would not be GC.
Change-Id: I7bfcb91a3fef57a4a1861168e9cd3ab55ce1334e
GitHub-Last-Rev: e0b7f68447
GitHub-Pull-Request: golang/go#32138
Reviewed-on: https://go-review.googlesource.com/c/go/+/178037
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
callerName requested 2 PCs from Callers, and that causes
both to be looked up in the file/line mapping.
We really only need to do the work for one PC.
(And in fact the caller doesn't need file/line at all, but
the Callers API can't express that.)
We used to request 2 PCs because in 1.11 and earlier we
stored an inline skip count in the second entry.
That's not necessary any more (as of 1.12).
Fixes#32093
Change-Id: I7b272626ef6496e848ee8af388cdaafd2556857b
Reviewed-on: https://go-review.googlesource.com/c/go/+/177858
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>
In TLS 1.3 session tickets are delivered after the handshake, and it
looks like now the Google servers wait until the first flight of data to
send them (or our timeout is too low). Cause some data to be sent so we
can avoid the guessing game.
Fixes#32090
Change-Id: I54af4acb3a89cc70c9e14a5dfe18a44c29a841a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/177877
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The motivation for doing so is to avoid making inaccurate claims.
Logging may not go to os.Stderr if anyone overrides the log package's
default output via https://godoc.org/log#SetOutput. Saying that
the standard logger is used should be sufficient to explain the
behavior, and users can infer that os.Stderr is used by default,
unless it's changed.
This change is the same as what was applied to http.Server.ErrorLog
documentation in CL 53950.
Change-Id: I32873fc548ceee573f8616b4d49b8a8b98881803
Reviewed-on: https://go-review.googlesource.com/c/go/+/176817
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When we downgrade a module (using 'go get m@none' or similar), we
exclude versions of other modules that depend on it. We'll try
previous versions (in the "versions" list returned by the proxy or in
codeRepo.Versions for vcs) until we find a version that doesn't
require an excluded module version.
If older versions of a module are broken for some reason,
mvs.Downgrade currently panics. With this change, we ignore versions
with errors during downgrade.
A frequent cause of this is incompatible v2+ versions. These are
common if a repository tagged v2.0.0 before migrating to modules, then
tagged v2.0.1 with a go.mod file later. v2.0.0 is incorrectly
considered part of the v2 module.
Fixes#31942
Change-Id: Icaa75c5c93f73f18a400c22f18a8cc603aa4011a
Reviewed-on: https://go-review.googlesource.com/c/go/+/177337
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The current example illustrates using As when the error is able to be
interpreted as an os.PathError, but elides the "else" case. This CL adds the
small extra else case to make it clear that it's not safe to assume As will
return true.
This CL also squash the err instantiation and the err nil check into one line
for brevity.
Change-Id: I3d3ab483ffb38fb2788d0498b3f03229a87dd7c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/177717
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Support for Ed25519 certificates was added in CL 175478, this wires them
up into the TLS stack according to RFC 8422 (TLS 1.2) and RFC 8446 (TLS 1.3).
RFC 8422 also specifies support for TLS 1.0 and 1.1, and I initially
implemented that, but even OpenSSL doesn't take the complexity, so I
just dropped it. It would have required keeping a buffer of the
handshake transcript in order to do the direct Ed25519 signatures. We
effectively need to support TLS 1.2 because it shares ClientHello
signature algorithms with TLS 1.3.
While at it, reordered the advertised signature algorithms in the rough
order we would want to use them, also based on what curves have fast
constant-time implementations.
Client and client auth tests changed because of the change in advertised
signature algorithms in ClientHello and CertificateRequest.
Fixes#25355
Change-Id: I9fdd839afde4fd6b13fcbc5cc7017fd8c35085ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/177698
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
A timespec on netbsd/amd64 is int64/int64, not int64/int32.
This bug appears to have been introduced in 7777bac6e4.
Spotted by Cherry Zhang while reviewing https://go-review.googlesource.com/c/go/+/177120.
Change-Id: I163c55d926965defd981bdbfd2511de7d9d4c542
Reviewed-on: https://go-review.googlesource.com/c/go/+/177637
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
A cross C compiler should be specified when cross building go
for android on x86 host. This CL fixes the typo in README.
Change-Id: Ideda915364d9f7f0fcd53146abc1e74506a8a8e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/177777
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Elias Naur <mail@eliasnaur.com>
If a user runs 'go get mod@vers' where the module transitively
requires itself at a newer version, 'go get' attempts to perform a
downgrade, which necessarily excludes the requested version of the
module.
Previously, we called mvs.BuildList with the requested module
version as the target. This panicked because BuildList doesn't allow
the target module (typically the main module) to require a newer
version of itself.
With this change, when we lose an upgrade due to a downgrade, we call
mvs.BuildList through a wrapper that treats the lost module version as
requirement of a synthetic root module, rather than the target
module. This avoids the panic.
This change also starts reporting errors when an upgraded module is
lost entirely (downgrades caused the module to be completely removed
from the build list).
Fixes#31491
Change-Id: I70ca261c20af7553cad2d3b840a1eaf3d18a4191
Reviewed-on: https://go-review.googlesource.com/c/go/+/177602
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
On most platforms newly-mapped memory is untouched, meaning the pages
backing the region haven't been faulted in yet. However, we mark this
memory as unscavenged which means the background scavenger
aggressively "returns" this memory to the OS if the heap is small.
The only platform where newly-mapped memory is actually unscavenged (and
counts toward the application's RSS) is on Windows, since
(*mheap).sysAlloc commits the reservation. Instead of making a special
case for Windows, I change the requirements a bit for a sysReserve'd
region. It must now be both sysMap'd and sysUsed'd, with sysMap being a
no-op on Windows. Comments about memory allocation have been updated to
include a more up-to-date mental model of which states a region of memory
may be in (at a very low level) and how to transition between these
states.
Now this means we can correctly mark newly-mapped heap memory as
scavenged on every platform, reducing the load on the background
scavenger early on in the application for small heaps. As a result,
heap-growth scavenging is no longer necessary, since any actual RSS
growth will be accounted for on the allocation codepath.
Finally, this change also cleans up grow a little bit to avoid
pretending that it's freeing an in-use span and just does the necessary
operations directly.
Fixes#32012.
Fixes#31966.
Updates #26473.
Change-Id: Ie06061eb638162e0560cdeb0b8993d94cfb4d290
Reviewed-on: https://go-review.googlesource.com/c/go/+/177097
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
For:
http2: track reused connections
https://golang.org/cl/176720 (updates golang/go#31982)
Some x/sys/unix updates come along for the ride too.
I filed #32031 for making the bundling process less difficult and
error-prone in the future.
Change-Id: Ic822080991ffa2d50352c5f613e45648a327cf16
Reviewed-on: https://go-review.googlesource.com/c/go/+/177037
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The compiler appears to have a latent bug:
fusePlain calls invalidateCFG when it changes block structure,
but fuseIf does not.
Fix this by hoisting the call to invalidateCFG to the top level.
Change-Id: Ic960fb3ac963b15b4a225aad84863d58efa954e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/177198
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The CL 164718 mistyped the comparison flags. The rules for floating
point comparison should be GreaterThanF and GreaterEqualF. Fortunately,
the wrong optimizations were overwritten by other integer rules, so the
issue won't cause failure but just some performance impact.
The fixed CL optimizes the floating point test as follows.
source code: func foo(f float64) bool { return f > 4 || f < -4}
previous version: "FCMPD", "CSET\tGT", "CBZ"
fixed version: "FCMPD", BLE"
Add the test case.
Change-Id: Iea954fdbb8272b2d642dae0f816dc77286e6e1fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/177121
Reviewed-by: Ben Shi <powerman1st@163.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is CVE-2019-11888.
Previously, passing a nil environment but a non-nil token would result
in the new potentially unprivileged process inheriting the parent
potentially privileged environment, or would result in the new
potentially privileged process inheriting the parent potentially
unprivileged environment. Either way, it's bad. In the former case, it's
an infoleak. In the latter case, it's a possible EoP, since things like
PATH could be overwritten.
Not specifying an environment currently means, "use the existing
environment". This commit amends the behavior to be, "use the existing
environment of the token the process is being created for." The behavior
therefore stays the same when creating processes without specifying a
token. And it does the correct thing when creating processes when
specifying a token.
Fixes#32000
Change-Id: Ia57f6e89b97bdbaf7274d6a89c1d9948b6d40ef5
Reviewed-on: https://go-review.googlesource.com/c/go/+/176619
Run-TryBot: Jason Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
This is useful for debugging line number assignment and
also for making sense of pathological line number inputs.
Activated with
-gcflags=-d=ssa/number_lines/stats=1 (the bit matters)
-gcflags=-d=ssa/number_lines/debug
Stats:
"SUM_LINE_RANGE",
SUM for f in files {MAX line in f {line}-MIN line in f {line}}
"MAXMIN_LINE_RANGE",
MAX for f in files {MAX line in f {line}} -
MIN for f in files {MIN line in f {line}}
"MAXFILE", maxfile,
MAX for f in files {f}
"NFILES", len(entries)
| files |
Change-Id: I8a7336e6370452fe2e3a62de17606db9bd6a6fd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/174947
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This cuts the time for 'go test -short testing' from 0.9s to < 0.1s.
Change-Id: Ib8402f80239e1e96ea5221dfd5cd0db08170d85b
Reviewed-on: https://go-review.googlesource.com/c/go/+/177420
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 121936 added this diagnostic to avoid a panic accessing *short.
(Hence the "This shouldn't really be a panic" comment.)
That CL was right to produce a clearer error than a plain memory fault,
but I think wrong to print+exit instead of panicking. I just ran into
one of these in a real program, and there is no indication anywhere
of how the program reached this point. The panic will show that.
So change print+exit to a panic with a helpful message, in contrast
to the original panic with an unhelpful message and the current
helpful message without stack trace.
Change-Id: Ib2bae1dead4ccde92f00fa3a34c05241ff7690c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/177419
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This just makes all.bash a bit less chatty.
Change-Id: I7d2ecabf0c7d8df2065d7052718f611bb2907801
Reviewed-on: https://go-review.googlesource.com/c/go/+/177418
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These tests take 20 seconds each to rebuild the entire world
in their respective modes. That's too much for short mode,
including all.bash on developer machines.
Keep doing it on builders and if someone runs 'go test' by hand
in that directory.
For #26473.
Change-Id: I3dc6955bc3aa7a20fd170efcde72a7d19b37bdbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/177417
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Was 50 seconds on unloaded Mac laptop; now 27.
Still longer than I would like, but every little bit helps.
For #26473.
Change-Id: Id4be016ee1555cbc3512eca0ae10236d7f06bd02
Reviewed-on: https://go-review.googlesource.com/c/go/+/177398
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add information about action and build IDs,
commands run, time spent in commands,
and overall time to -debug-actiongraph output.
Also avoid a terrible outcome in:
go build -debug-actiongraph x.go
Change-Id: I7a3a638f4848d75f2bdc997517f4ab23656da4f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/177138
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The work of running full vet on std and cmd during local development
has moved to go test, which of course runs during all.bash.
For errors in other GOOS/GOARCH combinations, the misc-compile
builders (running buildall.bash) also now run go vet std cmd.
The vetall builder need not do anything anymore.
Make it a no-op until it can be retired, and remove
cmd/vet/all and its special case in the go command.
Fixes#31916.
Change-Id: I8f30d184c382ea7c2c8f520e5618f680db633968
Reviewed-on: https://go-review.googlesource.com/c/go/+/176440
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Now that the main tree complies with 'go vet', enable all vet checks
during 'go test' in the main tree. This helps surface helpful errors
while developing, instead of having to wait for the misc-vet-vetall builder.
During 'go test', the additional vet checks are essentially free:
the vet invocations themselves take only 8 seconds total for the entire tree.
Also update buildall.bash (used by the misc-compile builders)
to run 'go vet std cmd' for each GOOS/GOARCH pair.
This is not as free, since in general it can require recompiling
packages with their tests included before invoking vet.
(That compilation was going on anyway in the 'go test' case.)
On my Mac laptop, ./buildall.bash freebsd used to take
68+16+17+18 = 119 seconds for make.bash and then
the builds of the three freebsd architectures.
Now it takes 68+16+23+17+23+18+24 = 189 seconds, 60% longer.
Some of this is spent doing unnecessary cgo work.
Still, this lets us shard the vet checks and match all.bash.
Fixes#20119.
For #31916.
Change-Id: I6b0c40bac47708a688463c7fca12c0fc23ab2751
Reviewed-on: https://go-review.googlesource.com/c/go/+/176439
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This updates the Go function declarations to match race_amd64.s.
Change-Id: I2b541a6b335ce732f4c31652aa615240ce7bb1c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/177397
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
When fmt.Errorf is provided with a %w verb with an error operand,
return an error implementing an Unwrap method returning that operand.
It is invalid to use %w with other formatting functions, to use %w
multiple times in a format string, or to use %w with a non-error
operand. When the Errorf format string contains an invalid use of %w,
the returned error does not implement Unwrap.
Change-Id: I534e20d3b163ab22c2b137b1c9095906dc243221
Reviewed-on: https://go-review.googlesource.com/c/go/+/176998
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
This allows the use of CLONE_VFORK and CLONE_VM for fork/exec, preventing
"fork/exec ...: cannot allocate memory" failures from occuring when attempting
to execute commands from a Go process that has a large memory footprint.
Additionally, this should reduce the latency of fork/exec on these platforms.
The same problem was addressed on linux/amd64 via issue #5838.
Updates #31936
Change-Id: I7ae0fbbeaa29cab944a49a11272a380d497eb2d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/175697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Vet needs export data for the imports of the package it is analyzing.
Vet does not need export data for the package itself, since vet will
do its own type checking. Assuming that vet is just as good as the compiler
at detecting invalid programs, don't run the compiler unnecessarily.
This especially matters for tests without external test files or for
which the external test files do not import the test-augmented original
package. In that case, the test-augmented original package need not
be compiled at all.
Cuts time for 'go clean -cache && go vet -x cmd/compile/internal/ssa'
from 7.6r 24.3u 2.8s to 3.5r 8.5u 1.9s, by not running the compiler
on the augmented test package.
There is still more to be done here - if we do need to build a
test-augmented package, we rerun cgo unnecessarily.
But this is a big help.
Cuts time for 'go vet std cmd' by about 30%.
For #31916.
Change-Id: If6136b4d384f1da77aed90b43f1a6b95f09b5d86
Reviewed-on: https://go-review.googlesource.com/c/go/+/176438
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
