Roland Shoemaker [Fri, 16 Oct 2020 01:32:20 +0000 (18:32 -0700)]
crypto/tls: de-prioritize AES-GCM ciphers when lacking hardware support
When either the server or client are lacking hardware support for
AES-GCM ciphers, indicated by the server lacking the relevant
instructions and by the client not putting AES-GCM ciphers at the top
of its preference list, reorder the preference list to de-prioritize
AES-GCM based ciphers when they are adjacent to other AEAD ciphers.
Also updates a number of recorded openssl TLS tests which previously
only specified TLS 1.2 cipher preferences (using -cipher), but not
TLS 1.3 cipher preferences (using -ciphersuites), to specify both
preferences, making these tests more predictable.
Fixes #41181.
Change-Id: Ied896c96c095481e755aaff9ff0746fb4cb9568e
Reviewed-on: https://go-review.googlesource.com/c/go/+/262857
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reason for revert: Some edge cases not properly covered due to changes within runtime traceback generation since 2017, that need to be examined. This change landed very late in the Go1.16 cycle.
Change-Id: I8cf6f46ea0ef6161d878e79943e6c7cdac94bccf
Reviewed-on: https://go-review.googlesource.com/c/go/+/268577
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Filippo Valsorda [Mon, 9 Nov 2020 13:54:55 +0000 (14:54 +0100)]
crypto/x509: drop the cgo implementation of root_darwin_amd64.go
This code was preserved just to do side-by-side testing while
transitioning to the Go implementation. There haven't been mismatch
issues, so drop the cgo code, which was making it hard to improve the Go
code without diverging.
Filippo Valsorda [Wed, 24 Jun 2020 21:01:00 +0000 (17:01 -0400)]
crypto/tls: ensure the server picked an advertised ALPN protocol
This is a SHALL in RFC 7301, Section 3.2.
Also some more cleanup after NPN, which worked the other way around
(with the possibility that the client could pick a protocol the server
did not suggest).
Change-Id: I83cc43ca1b3c686dfece8315436441c077065d82
Reviewed-on: https://go-review.googlesource.com/c/go/+/239748
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Roland Shoemaker <roland@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Michael Matloob [Tue, 3 Nov 2020 18:12:19 +0000 (13:12 -0500)]
cmd/go: support cgo files in overlays
This is a roll-forward of golang.org/cl/267197, which was reverted in
golang.org/cl/267357. It makes the following changes in addition to
the ones in the next paragraph: It avoids outputting trimpath
arguments for an overlay unless the overlay affects the package being
compiled (to avoid hitting windows command line argument limits), and
it fixes processing of regexps in the script test framework to treat
the first *non flag* argument to grep, stdout, and stderr as a regexp,
not just the first argument.
golang.org/cl/267917 was a roll-forward of golang.org/cl/262618, which
was reverted in golang.org/cl/267037. The only differences between
this CL and the original were the three calls to fflush from the C
files in build_overlay.txt, to guarantee that the string we were
expecting was
actually written out.
The CL requires rewriting the paths of the files passed to the cgo
tool toolchain to use the overlaid paths instead of the disk paths of
files. Because the directories of the overlaid paths don't exist in
general, the cgo tool have been updated to run in base.Cwd instead of
the package directory.
For #39958
Change-Id: I1bd96db257564bcfd95b3502aeca14d04bd28618
Reviewed-on: https://go-review.googlesource.com/c/go/+/267797
Trust: Michael Matloob <matloob@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Filippo Valsorda [Mon, 31 Aug 2020 21:09:57 +0000 (17:09 -0400)]
crypto/tls: drop macFunction abstraction
Since we dropped SSLv3, there is only one MAC scheme, and it doesn't
need any state beyond a keyed HMAC, so we can replace the macFunction
with the hash.Hash it wraps.
Pointed out by mtp@.
Change-Id: I5545be0e6ccb34a3055fad7f6cb5f628ff748e9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/251859
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Johan Brandhorst [Sat, 1 Aug 2020 11:18:31 +0000 (12:18 +0100)]
crypto/tls: add HandshakeContext method to Conn
Adds the (*tls.Conn).HandshakeContext method. This allows
us to pass the context provided down the call stack to
eventually reach the tls.ClientHelloInfo and
tls.CertificateRequestInfo structs.
These contexts are exposed to the user as read-only via Context()
methods.
This allows users of (*tls.Config).GetCertificate and
(*tls.Config).GetClientCertificate to use the context for
request scoped parameters and cancellation.
Replace uses of (*tls.Conn).Handshake with (*tls.Conn).HandshakeContext
where appropriate, to propagate existing contexts.
Fixes #32406
Change-Id: I33c228904fe82dcf57683b63627497d3eb841ff2
Reviewed-on: https://go-review.googlesource.com/c/go/+/246338
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Koen [Tue, 13 Oct 2020 23:05:43 +0000 (01:05 +0200)]
crypto/x509: return additional chains from Verify on Windows
Previously windows only returned the certificate-chain with the highest quality.
This change makes it so chains with a potentially lower quality
originating from other root certificates are also returned by verify.
Tests in verify_test flagged with systemLax are now allowed to pass if the system returns additional chains
Roland Shoemaker [Sun, 10 May 2020 18:23:58 +0000 (11:23 -0700)]
crypto/x509: add additional convenience fields to CertificateRequest
Adds the following additional convenience fields to CertificateRequest:
* KeyUsage
* ExtKeyUsage
* UnknownExtKeyUsage
* IsCA
* MaxPathLen
* BasicConstraintsValid
* MaxPathLenZero
* SubjectKeyId
* PolicyIdentifier
These fields are parsed during ParseCertificateRequest and marshalled
during CreateCertificateRequest. The parsing/marshalling code is
factored out of parseCertificate and buildExtensions (which is renamed
buildCertExtensions). This has the side effect of making these methods
somewhat easier to read.
Documentation for the fields is copied from Certificate.
Example CSR created with all of these fields parsed with openssl:
$ openssl req -in ~/test-csr.pem -noout -text
Certificate Request:
Data:
Version: 0 (0x0)
Subject:
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:a4:cb:64:35:8e:dd:8c:2b:a6:f1:aa:39:d1:be:
d0:b9:95:1e:59:19:82:76:28:d3:85:1b:c6:88:62:
e1:15:33:be:26:18:80:14:fe:f4:d4:91:66:4e:a4:
a4:47:bd:53:db:f7:2e:e3:31:ce:5f:86:cb:92:59:
93:bb:d0:7f:a2
ASN1 OID: prime256v1
NIST CURVE: P-256
Attributes:
Requested Extensions:
X509v3 Key Usage: critical
Certificate Sign
X509v3 Extended Key Usage:
Any Extended Key Usage, 1.2.3
X509v3 Basic Constraints: critical
CA:TRUE, pathlen:0
X509v3 Subject Key Identifier:
01:02:03
X509v3 Certificate Policies:
Policy: 1.2.3
Cherry Zhang [Sat, 7 Nov 2020 03:46:18 +0000 (22:46 -0500)]
cmd/compile: make sure linkname'd symbol is non-package
When a variable symbol is both imported (possibly through
inlining) and linkname'd, make sure its LSym is marked as
non-package for symbol indexing in the object file, so it is
resolved by name and dedup'd with the original definition.
Fixes #42401.
Change-Id: I8e90c0418c6f46a048945c5fdc06c022b77ed68d
Reviewed-on: https://go-review.googlesource.com/c/go/+/268178
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org>
Russ Cox [Thu, 29 Oct 2020 14:57:38 +0000 (10:57 -0400)]
cmd/go: add GOVCS setting to control version control usage
The go command runs commands like git and hg to download modules.
In the past, we have had problems with security bugs in version
control systems becoming security bugs in “go get”.
The original modules draft design removed use of these commands
entirely, saying:
> We want to move away from invoking version control tools such as bzr,
> fossil, git, hg, and svn to download source code. These fragment the
> ecosystem: packages developed using Bazaar or Fossil, for example, are
> effectively unavailable to users who cannot or choose not to install
> these tools. The version control tools have also been a source of
> exciting security problems. It would be good to move them outside the
> security perimeter.
The removal of these commands was not possible in the end: being able
to fetch directly from Git repos is too important, especially for
closed source. But the security exposure has not gone away.
We remain vulnerable to problems in VCS systems, especially the less
scrutinized ones.
This change adds a GOVCS setting to let users control which version
control systems are allowed by default.
It also changes the default allowed version control systems to git and hg
for public code and any version control system for private code
(import path or module path matched by the GOPRIVATE setting).
See the changes in alldocs.go for detailed documentation.
See #41730 for proposal and discussion.
Fixes #41730.
[Replay of CL 266420. See changes from Patch Set 1 for updates to fix
a few long tests.]
Change-Id: I4fe93804548956c42aea985368b4571bdb220f48
Reviewed-on: https://go-review.googlesource.com/c/go/+/267888
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Dmitri Shuralyov [Sun, 18 Oct 2020 17:02:06 +0000 (17:02 +0000)]
cmd/go: add /v2 to another require example
This is the same change as in CL 144917, but applied to a nearby line.
For #28374.
Change-Id: I6e1693d3a14e2517d863d1052a06c1156fc1edd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/263437
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Alberto Donizetti [Sun, 8 Nov 2020 08:44:33 +0000 (09:44 +0100)]
test/codegen: go fmt
Fixes #42445
Change-Id: I9653ef094dba2a1ac2e3daaa98279d10df17a2a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/268257
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Trust: Martin Möhrmann <moehrmann@google.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Austin Clements [Mon, 2 Nov 2020 13:48:23 +0000 (08:48 -0500)]
testing: increase benchmark output to four significant figures
Currently, the benchmark output from the testing package prints small
values with three significant figures. This means it can only
distinguish 1 part in 100, or a 1% error, which can be enough to throw
off further analysis of the output. This CL increases it to four
significant figures. For time values, at least, anything beyond four
significant figures is almost certainly noise.
Fixes #34626.
Change-Id: I3bcf305427130026276e6a4c78167989319f280c
Reviewed-on: https://go-review.googlesource.com/c/go/+/267102
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Brad Fitzpatrick [Sat, 7 Nov 2020 16:00:15 +0000 (08:00 -0800)]
crypto/x509: keep smaller root cert representation in memory until needed
Instead of parsing the PEM files and then storing the *Certificate
values forever, still parse them to see if they're valid and pick out
some fields, but then only store the decoded pem.Block.Bytes until
that cert is first needed.
Saves about 500K of memory on my (Debian stable) machine after doing a
tls.Dial or calling x509.SystemCertPool.
A more aggressive version of this is still possible: we can not keep
the pem.Block.Bytes in memory either, and re-read them from disk when
necessary. But dealing with files disappearing and even large
multi-cert PEM files changing (with offsets sliding around) made this
conservative version attractive. It doesn't change the
slurp-roots-on-startup semantics. It just does so with less memory
retained.
Change-Id: I3aea333f4749ae3b0026042ec3ff7ac015c72204
Reviewed-on: https://go-review.googlesource.com/c/go/+/230025
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
crypto/x509: add support for CertPool to load certs lazily
This will allow building CertPools that consume less memory. (Most
certs are never accessed. Different users/programs access different
ones, but not many.)
This CL only adds the new internal mechanism (and uses it for the
old AddCert) but does not modify any existing root pool behavior.
(That is, the default Unix roots are still all slurped into memory as
of this CL)
Change-Id: Ib3a42e4050627b5e34413c595d8ced839c7bfa14
Reviewed-on: https://go-review.googlesource.com/c/go/+/229917
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Joshua M. Clulow [Fri, 6 Nov 2020 21:11:58 +0000 (13:11 -0800)]
os/exec: use "pfiles" for fd debugging on illumos
On illumos (and Solaris) systems, the native "pfiles" tool provides the
best information about open file descriptors for a process:
https://illumos.org/man/1/pfiles
Use that instead of "lsof" when debugging file descriptor leaks.
Updates #42431.
Change-Id: If1250c4e6c9e8adbd076495a09fb1ce63abcc68b
Reviewed-on: https://go-review.googlesource.com/c/go/+/268019
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Cholerae Hu [Fri, 31 Jul 2020 05:57:48 +0000 (13:57 +0800)]
cmd/compile: check indirect connection between if block and phi block in addLocalInductiveFacts
CL 244579 added guard clauses to prevent a faulty state that was
possible under the incorrect logic of the uniquePred loop in
addLocalInductiveFacts. That faulty state was still making the
intended optimization, but not for the correct reason.
Removing the faulty state also removed the overly permissive application
of the optimization, and therefore made these two tests fail.
We disabled the tests of this optimization in CL 244579 to allow us to
quickly apply the fix in the CL. This CL now corrects the logic of the
uniquePred loop in order to apply the optimization correctly.
The comment above the uniquePred loop says that it will follow unique
predecessors until it reaches a join point. Without updating the child
node on each iteration, it cannot follow the chain of unique
predecessors more than one step. Adding the update to the child node
on each iteration of the loop allows the logic to follow the chain of
unique predecessors until reaching a join point (because a non-unique
predecessor will signify a join point).
Updates #40502.
Change-Id: I23d8367046a2ab3ce4be969631f9ba15dc533e6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/246157
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Artyom Pervukhin [Wed, 4 Nov 2020 12:28:25 +0000 (15:28 +0300)]
archive/zip: fix documentation to mention fs.FS interface
Fixes #42374
Change-Id: I0ed1eb052d79bcc65810b74bff48f1e615e1dc1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/267657 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Emmanuel T Odeke [Sat, 18 Feb 2017 10:03:32 +0000 (03:03 -0700)]
runtime: make stack traces of endless recursion print only top and bottom 50
This CL makes it so that instead of printing massive stack traces during
endless recursion, which spams users and aren't useful, it now prints out
the top and bottom 50 frames. If the number of frames <= 100
(_TracebackMaxFrames), we'll just print all the frames out.
Modified gentraceback to return counts of:
* ntotalframes
* nregularframes
which allows us to get accurate counts of the various kinds of frames.
While here, also fixed a bug that resulted from CL 37222, in which we
no longer accounted for decrementing requested frame skips, and assumed
that when printing, that skip would always be 0. The fix is instead to add
precondition that we'll only print if skip <= 0, but also decrement skip
as we iterate.
Fixes #7181.
Fixes #24628.
Change-Id: Ie31ec6413fdfbe43827b254fef7d99ea26a5277f
Reviewed-on: https://go-review.googlesource.com/c/go/+/37222
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Matthew Dempsky [Thu, 29 Oct 2020 08:13:16 +0000 (01:13 -0700)]
cmd/compile: support inlining of type switches
This CL adds support for inlining type switches, including exporting
and importing them.
Type switches are represented mostly the same as expression switches.
However, if the type switch guard includes a short variable
declaration, then there are two differences: (1) there's an ONONAME
(in the OTYPESW's Left) to represent the overall pseudo declaration;
and (2) there's an ONAME (in each OCASE's Rlist) to represent the
per-case variables.
For simplicity, this CL simply writes out each variable separately
using iimport/iiexport's normal Vargen mechanism for disambiguating
identically named variables within a function. This could be improved
somewhat, but inlinable type switches are probably too uncommon to
merit the complexity.
While here, remove "case OCASE" from typecheck1. We only type check
"case" clauses as part of a "select" or "switch" statement, never as
standalone statements.
Fixes #37837
Change-Id: I8f42f6c9afdd821d6202af4a6bf1dbcbba0ef424
Reviewed-on: https://go-review.googlesource.com/c/go/+/266203
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
This commit is a copy of filepath.WalkDir adapted to use fs.FS
instead of the native OS file system. It is the last implementation
piece of the io/fs proposal.
The original io/fs proposal was to adopt filepath.Walk, but we
have since introduced the more efficient filepath.WalkDir (#42027),
so this CL adopts that more efficient option instead.
(The changes in path/filepath bring the two copies more in line
with each other. The main change is unembedding the field
in statDirEntry, so that the fs.DirEntry passed to the WalkDirFunc
for the root of the tree does not have any extra methods.)
For #41190.
Change-Id: I9359dfcc110338c0ec64535f22cafb38d0b613a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/243916
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Russ Cox [Thu, 29 Oct 2020 16:20:53 +0000 (12:20 -0400)]
path/filepath: add WalkDir
WalkDir is like Walk but can use ReadDir to read directories,
instead of Readdirnames + Lstat on every entry,
which is usually a significant performance improvement.
(The Lstat can still happen if the walk function calls d.Info.)
Fixes #42027.
[Replay of CL 266240 after it was reverted due to accidentally
enabling on Windows a test that does not work on Windows.
The original code only ran the test on os.Getuid() > 0.
The rolled-back CL skipped the test on os.Getuid() == 0.
But on Windows, os.Getuid(), it turns out, always returns -1.
So what looked like a test for root was also excluding Windows.
This CL revises the test to skip Windows explicitly.]
Change-Id: I9b3661013d6449b7486532445d934ae91e5393ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/267887
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Michael Munday [Mon, 11 May 2020 16:44:48 +0000 (09:44 -0700)]
cmd/compile: optimize shift pairs and masks on s390x
Optimize combinations of left and right shifts by a constant value
into a 'rotate then insert selected bits [into zero]' instruction.
Use the same instruction for contiguous masks since it has some
benefits over 'and immediate' (not restricted to 32-bits, does not
overwrite source register).
To keep the complexity of this change under control I've only
implemented 64 bit operations for now.
There are a lot more optimizations that can be done with this
instruction family. However, since their function overlaps with other
instructions we need to be somewhat careful not to break existing
optimization rules by creating optimization dead ends. This is
particularly true of the load/store merging rules which contain lots
of zero extensions and shifts.
This CL does interfere with the store merging rules when an operand
is shifted left before it is stored:
binary.BigEndian.PutUint64(b, x << 1)
This is unfortunate but it's not critical and somewhat complex so
I plan to fix that in a follow up CL.
Change-Id: I7591cce41e06867ba10a745daae9333513062746
Reviewed-on: https://go-review.googlesource.com/c/go/+/233317
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Trust: Michael Munday <mike.munday@ibm.com>
Joel Sing [Thu, 5 Nov 2020 05:41:32 +0000 (16:41 +1100)]
cmd/dist: remove openbsd/mips64 from incomplete ports
Remove openbsd/mips64 from incomplete ports lists - all of the necessary code
has landed and we want to run tests so we can see/deal with remaining failures.
Update #40995
Change-Id: I5d4f89af82ff3abe57570a9a8abf889498093d32
Reviewed-on: https://go-review.googlesource.com/c/go/+/267606
Trust: Joel Sing <joel@sing.id.au> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Jonathan Swinney [Wed, 4 Nov 2020 16:18:23 +0000 (16:18 +0000)]
cmd/compile: improve atomic swap intrinsics on arm64
ARMv8.1 has added new instructions for atomic memory operations. This
change builds on the previous change which added support for atomic add, 0a7ac93c27c9ade79fe0f66ae0bb81484c241ae5, to include similar support for
atomic-compare-and-swap, atomic-swap, atomic-or, and atomic-and
intrinsics. Since the new instructions are not guaranteed to be present,
we guard their usages with a branch on a CPU feature.
Peformance on an ARMv8.1 machine:
name old time/op new time/op delta
CompareAndSwap-16 37.9ns ±16% 24.1ns ± 4% -36.44% (p=0.000 n=10+9)
CompareAndSwap64-16 38.6ns ±15% 24.1ns ± 3% -37.47% (p=0.000 n=10+10)
name old time/op new time/op delta
Swap-16 46.9ns ±32% 12.5ns ± 6% -73.40% (p=0.000 n=10+10)
Swap64-16 53.4ns ± 1% 12.5ns ± 6% -76.56% (p=0.000 n=10+10)
Than McIntosh [Thu, 5 Nov 2020 19:19:47 +0000 (14:19 -0500)]
cmd/link: report error if builtin referenced but not defined
When the compiler refers to a runtime builtin, it emits an indexed
symbol reference in the object file via predetermined/preassigned ID
within the PkgIdxBuiltin pseudo-package. At link time when the loader
encounters these references, it redirects them to the corresponding
defined symbol in the runtime package. This redirection process
currently assumes that if a runtime builtin is referenced, we'll
always have a definition for it. This assumption holds in most cases,
however for the builtins "runtime.racefuncenter" and
"runtime.racefuncexit", we'll only see definitions if the runtime
package we're linking against was built with "-race".
In the bug in question, build passes "-gcflags=-race" during
compilation of the main package, but doesn't pass "-race" directly to
'go build', and as a result the final link combines a
race-instrumented main with a non-race runtime; this results in R_CALL
relocations with zero-valued target symbols, resulting in a panic
during stack checking.
This patch changes the loader's resolve method to detect situations
where we're asking for builtin "runtime.X", but the runtime package
read in doesn't contain a definition for X.
Fixes #42396.
Change-Id: Iafd38bd3b0f7f462868d120ccd4d7d1b88b27436
Reviewed-on: https://go-review.googlesource.com/c/go/+/267881
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Rob Findley [Tue, 27 Oct 2020 02:44:54 +0000 (22:44 -0400)]
go/types: add unexported start and end positions to type checker errors
Tools often need to associate errors not with a single position, but
with a span of source code. For example, gopls currently estimates
diagnostic spans using heuristics to expand the positions reported by
the type checker to surrounding source code. Unfortunately this is often
inaccurate.
This CL lays the groundwork to solve this within go/types by adding a
start and end position to type checker errors. This is an experimental
API, both because we are uncertain of the ideal representation for these
spans and because their initial positioning is naive. In most cases this
CL simply expands errors to the surrounding ast.Node being typechecked,
if available. This might not be the best error span to present to the
user. For these reasons the API is unexported -- gopls can read these
positions using reflection, allowing us to gain experience and improve
them during the next development cycle.
For golang/go#42290
Change-Id: I39a04d70ea2bb2134b4d4c937f32b2ddb4456430
Reviewed-on: https://go-review.googlesource.com/c/go/+/265250
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Michael Matloob [Tue, 3 Nov 2020 18:12:19 +0000 (13:12 -0500)]
cmd/go: account for flags when parsing regexps in TestScript
Test script expects the regexp argument for stdout, stderr, and cmp
to be the first argument after the command, but that might not be the
case if the -q or -count flags are provided. Treat the first argument
after a flag as a regexp instead.
For #39958
Change-Id: I369926109ec10cca8b2c3baca27e7a3f7baf364b
Reviewed-on: https://go-review.googlesource.com/c/go/+/267877
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Pratt [Mon, 2 Nov 2020 21:34:51 +0000 (16:34 -0500)]
runtime: disable preemption in startm
startm contains a critical section from when it takes ownership of a P
(either on function entry or call to pidleput) until it wakes the M
receiving the P. If preempted in this critical section, the owned P is
left in limbo. If preempted for a GC stop, there will be nothing to stop
the owned P and STW will wait forever.
golang.org/cl/232298 introduced the first call to startm that is not on
the system stack (via a wakep call), introducing the possibility of
preemption. Disable preemption in startm to ensure this remains
non-preemptible.
Since we're not always on the system stack anymore, we also need to be
careful in allocm.
Updates #42237
Change-Id: Icb95eef9eb262121856485316098331beea045da
Reviewed-on: https://go-review.googlesource.com/c/go/+/267257 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Trust: Michael Pratt <mpratt@google.com>
cmd/go/internal/modget: resolve paths at the requested versions
Previously, we resolved each argument to 'go get' to a package path or
module path based on what was in the build list at existing versions,
even if the argument specified a different version explicitly. That
resulted in bugs like #37438, in which we variously resolved the wrong
version or guessed the wrong argument type for what is unambiguously a
package argument at the requested version.
We were also using a two-step upgrade/downgrade algorithm, which could
not only upgrade more that is strictly necessary, but could also
unintentionally upgrade *above* the requested versions during the
downgrade step.
This change instead uses an iterative approach, with an explicit
disambiguation step for the (rare) cases where an argument could match
the same package path in multiple modules. We use a hook in the
package loader to halt package loading as soon as an incorrect version
is found — preventing over-resolving — and verify that the result
after applying downgrades successfully obtained the requested versions
of all modules.
Making 'go get' be correct and usable is especially important now that
we are defaulting to read-only mode (#40728), for which we are
recommending 'go get' more heavily.
While I'm in here refactoring, I'm also reworking the API boundary
between the modget and modload packages. Previously, the modget
package edited the build list directly, and the modload package
accepted the edited build list without validation. For lazy loading
(#36460), the modload package will need to maintain additional
metadata about the requirement graph, so it needs tighter control over
the changes to the build list.
As of this change, modget no longer invokes MVS directly, but instead
goes through the modload package. The resulting API gives clearer
reasons in case of updates, which we can use to emit more useful
errors.
Fixes #37438
Updates #36460
Updates #40728
Change-Id: I596f0020f3795870dec258147e6fc26a3292c93a
Reviewed-on: https://go-review.googlesource.com/c/go/+/263267
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Bryan C. Mills [Sat, 31 Oct 2020 01:00:41 +0000 (21:00 -0400)]
cmd/go/internal/modload: fix (*mvsReqs).Max when the second argument is the empty string
As far as I can tell, this bug had gone unnoticed because everything
that uses Max so far happened to only ever present the empty string as
the first argument.
For #37438
Change-Id: Ie8c42313157d2c2c17e4058c53b5bb026b95a1c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/266860
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Bryan C. Mills [Fri, 30 Oct 2020 16:24:51 +0000 (12:24 -0400)]
cmd/go/internal/modload: return a module-only result from QueryPattern
This allows a single QueryPattern call to resolve a path that could be
either a package or a module. It is important to be able to make a
single QueryPattern call — rather than a QueryPattern followed by a
Query for the specific module path — to provide appropriate fallback
behavior: if the proxy returns package results but does not contain a
module result, we don't want to fall back to the next proxy to look
for the (probably-nonexistent) module.
For #37438
Change-Id: I419b8bb3ab4565f443bb5cee9a8b206f453b9801
Reviewed-on: https://go-review.googlesource.com/c/go/+/266657
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Rob Findley [Tue, 3 Nov 2020 22:54:24 +0000 (17:54 -0500)]
go/types: simplify error messages for untyped value assignability
CL 242083 corrected an inaccurate error message related to the
assignability of untyped constant values. Previously the error message
was of the form "cannot convert ... to ...", which is misleading when
there is no explicit conversion in the syntax. The new error message
corrected this to "cannot use ... as ... in ...", but also appended an
inner error message that can be quite verbose. For example:
cannot use "123" (untyped string constant) as int value in assignment:
cannot convert "123" (untyped string constant) to int"
This might be more accurate, but is a regression in readability. Correct
this by only including the inner error message in the rare cases where
it is helpful: if the constant value overflows or is truncated.
For golang/go#22070
Change-Id: I8b8ee6ef713f64facc319894be09398b0b5ea500
Reviewed-on: https://go-review.googlesource.com/c/go/+/267717
Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Martin Möhrmann [Thu, 5 Nov 2020 04:59:34 +0000 (05:59 +0100)]
internal/cpu: fix and cleanup ARM64 cpu feature fields and options
Remove all cpu features from the ARM64 struct that are not initialized
to reduce cache lines used and to avoid those features being
accidentially used without actual detection if they are present.
Add missing option to mask the CPUID feature.
Change-Id: I94bf90c0655de1af2218ac72117ac6c52adfc289
Reviewed-on: https://go-review.googlesource.com/c/go/+/267658
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Martin Möhrmann <moehrmann@google.com>
Emmanuel T Odeke [Tue, 26 May 2020 21:14:08 +0000 (14:14 -0700)]
net/mail: avoid ParseDate confusion if day name includes "T"
Fixes the check for RFC 5322 "obsolete time zone" to ensure
that we correctly extract the entire date from the "T" of the
implied time zone.
Obsolete Time zones come in the form:
* GMT
* PST
* MDT
etc, as per Section 4.3 of RFC 5322,
https://tools.ietf.org/html/rfc5322#section-4.3.
The prior check from CL 117596 erronenously used strings.Index
which selects the first "T", and that meant that dates containing
days "Tue" or "Thu" could not be parsed.
We also now deal with "T" in the CFWS "Comment Folding White Space".
Thus we'll now accept dates:
* Thu, 20 Nov 1997 09:55:06 MDT
* Thu, 20 Nov 1997 09:55:06 MDT (MDT)
* Fri, 21 Nov 1997 09:55:06 MDT (This comment)
* Fri, 21 Nov 1997 09:55:06 MDT (MDT
Fixes #39260
Change-Id: I6d59d99bc4f05a82582c826b5c5a080a25fd999b
Reviewed-on: https://go-review.googlesource.com/c/go/+/235200
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Johan Knutzen [Wed, 21 Oct 2020 18:45:03 +0000 (18:45 +0000)]
syscall: expose bInheritHandles of CreateProcess
Certain use cases require this parameter to be false. This includes
spawning a child process in a different windows session than session 0.
Docs regarding the behavior of this parameter to CreateProcess:
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa
Fixes #42098
Change-Id: If998f57d6f2962824aacbee75e1b508b255ab293
GitHub-Last-Rev: 584eb13e36a3ef7e0cd959295e92fb129f21d1f8
GitHub-Pull-Request: golang/go#41957
Reviewed-on: https://go-review.googlesource.com/c/go/+/261917
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Russ Cox [Thu, 29 Oct 2020 14:57:38 +0000 (10:57 -0400)]
cmd/go: add GOVCS setting to control version control usage
The go command runs commands like git and hg to download modules.
In the past, we have had problems with security bugs in version
control systems becoming security bugs in “go get”.
The original modules draft design removed use of these commands
entirely, saying:
> We want to move away from invoking version control tools such as bzr,
> fossil, git, hg, and svn to download source code. These fragment the
> ecosystem: packages developed using Bazaar or Fossil, for example, are
> effectively unavailable to users who cannot or choose not to install
> these tools. The version control tools have also been a source of
> exciting security problems. It would be good to move them outside the
> security perimeter.
The removal of these commands was not possible in the end: being able
to fetch directly from Git repos is too important, especially for
closed source. But the security exposure has not gone away.
We remain vulnerable to problems in VCS systems, especially the less
scrutinized ones.
This change adds a GOVCS setting to let users control which version
control systems are allowed by default.
It also changes the default allowed version control systems to git and hg
for public code and any version control system for private code
(import path or module path matched by the GOPRIVATE setting).
See the changes in alldocs.go for detailed documentation.
See #41730 for proposal and discussion.
Fixes #41730.
Change-Id: I1999ddf7445b36a7572965be5897c7a1ff7f4265
Reviewed-on: https://go-review.googlesource.com/c/go/+/266420
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Russ Cox [Thu, 29 Oct 2020 16:20:53 +0000 (12:20 -0400)]
path/filepath: add WalkDir
WalkDir is like Walk but can use ReadDir to read directories,
instead of Readdirnames + Lstat on every entry,
which is usually a significant performance improvement.
(The Lstat can still happen if the walk function calls d.Info.)
Fixes #42027.
Change-Id: Ie11024b23be2656e320d41fd81ff0d8810aa729e
Reviewed-on: https://go-review.googlesource.com/c/go/+/266240
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Paul E. Murphy [Tue, 3 Nov 2020 22:59:25 +0000 (16:59 -0600)]
cmd/asm: fix rlwnm reg,reg,const,reg encoding on ppc64
The wrong value for the first reg parameter was selected.
Likewise the wrong opcode was selected. This should match
rlwnm (rrr type), not rlwinm (irr type).
Similarly, fix the optab matching rules so clrlslwi does
not match reg,reg,const,reg arguments. This is not a valid
operand combination for clrlslwi.
Russ Cox [Wed, 4 Nov 2020 11:55:59 +0000 (06:55 -0500)]
os: avoid nil returns from Readdirnames, Readdir, ReadDir
The refactoring of this code while adding ReadDir stopped
pre-allocating a 100-entry slice for the results.
That seemed like a good idea in general, since many
directories have nowhere near 100 entries, but it had the
side effect of returning a nil slice for an empty directory.
Some “golden” tests that are too sensitive about nil vs not
inside Google broke because Readdirnames(-1) was now
returning nil instead of []string{} on an empty directory.
It seems likely there are other such tests in the wild, and
it doesn't seem worth breaking them.
This commit restores the non-nil-ness of the old result,
without restoring the excessive preallocation.
Fixes #42367.
Change-Id: I2be72030ac703346e859a97c2d4e456fadfce9b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/267637
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Curtis La Graff [Tue, 3 Nov 2020 20:26:17 +0000 (20:26 +0000)]
cmd/go/internal/modfetch/codehost: add support for new fossil info hash prefix
A recent update of the Fossil SCM application changes
the line prefix when the fossil info command is used.
Instead of the revision hash starting with "uuid:", it has been
changed to "hash:".
To support older and new versions, fossilParseStat will
now check for either version of the prefix when attempting
to find the line containing the hash of the desired revision.
Fixes #42323
Change-Id: I6eff49f9989b37b295322a8569e222a1fd02f6e3
GitHub-Last-Rev: f4e6652307732fd3213684f13e42d17528271d88
GitHub-Pull-Request: golang/go#42324
Reviewed-on: https://go-review.googlesource.com/c/go/+/267080
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Bryan C. Mills <bcmills@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Joel Sing [Tue, 3 Nov 2020 13:29:39 +0000 (00:29 +1100)]
cmd/dist: enable additional cgo tests on openbsd architectures
OpenBSD gained __thread support quite some time ago.
Change-Id: I7de0a5c0c4de1a7ce59e48ac939fc2daf56be8f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/267318
Trust: Joel Sing <joel@sing.id.au> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Jonathan Swinney [Mon, 2 Nov 2020 16:36:10 +0000 (16:36 +0000)]
cmd/asm: rename arm64 instructions LDANDx to LDCLRx
The LDANDx instructions were misleading because they correspond to the
mnemonic LDCLRx as defined in the Arm Architecture Reference Manual for
Armv8. This changes the assembler to use the same mnemonic as the GNU
assembler and the manual.
Joel Sing [Sun, 1 Nov 2020 16:58:08 +0000 (03:58 +1100)]
runtime: allow physical page aligned stacks to be allocated
Add a physPageAlignedStack boolean which if set, results in over allocation
by a physical page, the allocation being rounded to physical page alignment
and the unused memory surrounding the allocation being freed again.
OpenBSD/octeon has 16KB physical pages and requires stacks to be physical page
aligned in order for them to be remapped as MAP_STACK. This change allows Go
to work on this platform.
Based on a suggestion from mknyszek in issue #41008.
Updates #40995
Fixes #41008
Change-Id: Ia5d652292b515916db473043b41f6030094461d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/266919
Trust: Joel Sing <joel@sing.id.au> Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Ben Hoyt [Tue, 3 Nov 2020 21:13:42 +0000 (10:13 +1300)]
strconv: revert ParseFloat/ParseComplex error on incorrect bitSize
This is a partial revert of https://go-review.googlesource.com/c/go/+/248219
because we found that a non-trivial amount of code erroneously calls
ParseFloat(s, 10) or even ParseFloat(s, 0) and expects it to work --
before that change was merged, ParseFloat accepted a bitSize of
anything other than 32 or 64 to mean 64 (and ParseComplex was similar).
So revert that behavior to avoid breaking people's code, and add tests
for this.
I may add a vet check to flag ParseFloat(s, not_32_or_64) in a later
change.
See #42297 for more details.
Change-Id: I4bc0156bd74f67a39d5561b6e5fde3f2d20bd622
Reviewed-on: https://go-review.googlesource.com/c/go/+/267319
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Tue, 3 Nov 2020 05:31:06 +0000 (21:31 -0800)]
cmd/go: in cgoflags, permit -DX1, prohibit -Wp,-D,opt
Restrict -D and -U to ASCII C identifiers, but do permit trailing digits.
When using -Wp, prohibit commas in -D values.
Change-Id: Ibfc4dfdd6e6c258e131448e7682610c44eee9492
Reviewed-on: https://go-review.googlesource.com/c/go/+/267277
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Joel Sing [Tue, 19 May 2020 08:56:01 +0000 (18:56 +1000)]
cmd/dist,cmd/go,runtime: add support for cgo on linux/riscv64
Fixes #36641
Change-Id: I51868d83ce341d78d33b221d184c5a5110c60d14
Reviewed-on: https://go-review.googlesource.com/c/go/+/263598
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Change-Id: Ia14659a9c44f9e1504eb88b5693932b9dd4bb286
Reviewed-on: https://go-review.googlesource.com/c/go/+/252939
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
kemalelmizan [Thu, 27 Aug 2020 23:39:43 +0000 (06:39 +0700)]
cmd/doc: adding validation before adding comment marker
Previous fix in issue #20929 for adding comment marker does
not check whether string field have // prefix or not.
This commit ensures string field does not contain // before adding
prefix to the line. Test also included in this commit.
Fixes #40992
Change-Id: Ibc5e8ef147eeb2ed732fb9e19815c8b21fcfb2ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/251237
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Michael Anthony Knyszek [Mon, 2 Nov 2020 19:03:16 +0000 (19:03 +0000)]
runtime: decouple consistent stats from mcache and allow P-less update
This change modifies the consistent stats implementation to keep the
per-P sequence counter on each P instead of each mcache. A valid mcache
is not available everywhere that we want to call e.g. allocSpan, as per
issue #42339. By decoupling these two, we can add a mechanism to allow
contexts without a P to update stats consistently.
In this CL, we achieve that with a mutex. In practice, it will be very
rare for an M to update these stats without a P. Furthermore, the stats
reader also only needs to hold the mutex across the update to "gen"
since once that changes, writers are free to continue updating the new
stats generation. Contention could thus only arise between writers
without a P, and as mentioned earlier, those should be rare.
A nice side-effect of this change is that the consistent stats acquire
and release API becomes simpler.
Fixes #42339.
Change-Id: Ied74ab256f69abd54b550394c8ad7c4c40a5fe34
Reviewed-on: https://go-review.googlesource.com/c/go/+/267158
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Trust: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Mon, 2 Nov 2020 16:58:38 +0000 (16:58 +0000)]
runtime: make getMCache inlineable
This change moves the responsibility of throwing if an mcache is not
available to the caller, because the inlining cost of throw is set very
high in the compiler. Even if it was reduced down to the cost of a usual
function call, it would still be too expensive, so just move it out.
This choice also makes sense in the context of #42339 since we're going
to have to handle the case where we don't have an mcache to update stats
in a few contexts anyhow.
Also, add getMCache to the list of functions that should be inlined to
prevent future regressions.
getMCache is called on the allocation fast path and because its not
inlined actually causes a significant regression (~10%) in some
microbenchmarks.
Fixes #42305.
Change-Id: I64ac5e4f26b730bd4435ea1069a4a50f55411ced
Reviewed-on: https://go-review.googlesource.com/c/go/+/267157
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Michael Matloob [Mon, 2 Nov 2020 19:19:12 +0000 (19:19 +0000)]
cmd/go: support cgo files in overlays
This is a roll-forward of golang.org/cl/262618, which was reverted in
golang.org/cl/267037. The only differences between this CL and the
original are the three calls to fflush from the C files in
build_overlay.txt, to guarantee that the string we're expecting is
actually written out.
This requires rewriting the paths of the files passed to the cgo tool
toolchain to use the overlaid paths instead of the disk paths of
files. Because the directories of the overlaid paths don't exist in
general, the cgo tool have been updated to run in base.Cwd instead of
the package directory.
For #39958
Change-Id: If7e5e057c62c0c22ddb724f9fe650902fc5f4832
Reviewed-on: https://go-review.googlesource.com/c/go/+/267197 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Michael Matloob <matloob@golang.org>
Austin Clements [Sun, 1 Nov 2020 22:48:42 +0000 (17:48 -0500)]
runtime: default to MADV_DONTNEED on Linux
In Go 1.12, we changed the runtime to use MADV_FREE when available on
Linux (falling back to MADV_DONTNEED) in CL 135395 to address issue
#23687. While MADV_FREE is somewhat faster than MADV_DONTNEED, it
doesn't affect many of the statistics that MADV_DONTNEED does until
the memory is actually reclaimed under OS memory pressure. This
generally leads to poor user experience, like confusing stats in top
and other monitoring tools; and bad integration with management
systems that respond to memory usage.
We've seen numerous issues about this user experience, including
#41818, #39295, #37585, #33376, and #30904, many questions on Go
mailing lists, and requests for mechanisms to change this behavior at
run-time, such as #40870. There are also issues that may be a result
of this, but root-causing it can be difficult, such as #41444 and
#39174. And there's some evidence it may even be incompatible with
Android's process management in #37569.
This CL changes the default to prefer MADV_DONTNEED over MADV_FREE, to
favor user-friendliness and minimal surprise over performance. I think
it's become clear that Linux's implementation of MADV_FREE ultimately
doesn't meet our needs. We've also made many improvements to the
scavenger since Go 1.12. In particular, it is now far more prompt and
it is self-paced, so it will simply trickle memory back to the system
a little more slowly with this change. This can still be overridden by
setting GODEBUG=madvdontneed=0.
Fixes #42330 (meta-issue).
Fixes #41818, #39295, #37585, #33376, #30904 (many of which were
already closed as "working as intended").
Bryan C. Mills [Mon, 2 Nov 2020 14:12:28 +0000 (09:12 -0500)]
all: update dependency on golang.org/x/sys and regenerate Windows syscalls
Steps run:
$ cd $(go env GOROOT)/src
$ go get -d golang.org/x/sys
$ go mod tidy
$ go mod vendor
$ go generate syscall/... internal/syscall/...
$ cd cmd
$ go get -d golang.org/x/sys
$ go mod tidy
$ go mod vendor
$ cd ..
$ git add .
This change subsumes CL 260860.
For #36905
Change-Id: I7c677c6aa1ad61b9cbd8cf9ed208ed5a30f29c87
Reviewed-on: https://go-review.googlesource.com/c/go/+/267103
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>