This (temporarily) disables the use of recursively defined
generic types w/o repeating the type arguments, but such
type definitions need some more (type-checking) love anyway
before all conditions are checked.
Change-Id: I173f5ce2296b1c484ac95a3a887812c0b1f88245
- print type bounds when printing function signatures
- store (named) tyep bound rather than underlying type
- first example using parameterized interface type bound working
Change-Id: Ic7110039d1e09838c8f33040e887e6a4f038d75d
Accept (parse and type-check) parameterized interfaces with literal
type constraints, as in contracts. For instance, instead of
contract C(T) {
T add(T) T
T int, string
}
one can write
type C(type T) interface {
add(T) T
type int, string
}
Their use as type bounds still needs some work, though.
Change-Id: I9d59681dc0ba38054ee5627be141fc6e8a78381b
- set correct token position for ast.ContractType
- print "best effort" interface position in "complete interface" traces
- don't print trace output for types that are already set up
- print contract "signtature" when printing Contract type
Also: rebased on top of today's master branch
Change-Id: I6a528ce51a4152c25ad5d83ad52e01df8e89d283
This change expands type inference to parameterized named types
and clarifies handling of type parameters during type inference
(specifically, an inferred type may be a type parameter itself).
The change also implements method selectors with parameterized
receiver types by using type inference to deduce the receiver
type parameters.
Add more tests and enable many disabled tests that now work
correctly.
Change-Id: I8e10f9166fec9a8454f14b8fb089230c70422a1b
Also: disabled aggressive rewriting of methods upon type instantiation
(Need to first figure out what we really need to do.)
Change-Id: I6fba78d60963534ebfe130164058ac1fbed7e5a3
A parameterized named type is in one of two states: not yet
instantiated, or instantiated. It may be instantiated with
any type, including (outer) type parameters. An instantiated
parameterized type may needs its type parameters substituted
if it is part of an enclosing type (or function) that is
instantiated.
A Named type has an extra field targs which is set to the
type arguments if the type is instantiated.
As a result, we don't need a representation for (partially)
instantiated parameterized types anymore: the Parameterized type
is now unused (but still present in the code).
Added a unique id field to TypeParam types so they can be
printed with a unique name. This permits the creation of a
unique type name for instantiated types which then can be used
for hashing that type name.
Also, made interfaces as type bounds work in more cases.
A handful of places (in testdata/map.go2, map2.go2) are broken
(commented out) and need investigation.
Change-Id: I137d352b419b2520eadde1b07f823375e0273dab
Always accept them but complain if they are not permitted.
The analogous parser check was lost in a recent change.
Also: Fix index computation in collectTypeParams.
Change-Id: I7cb89944984df6dadd7f603b56f0c3941d60f7c5
- implemented for comparisons for now
- can type-check generic min
- also: rebased on top of Go tip
Change-Id: Id35582a59c4cddcb2b4ae9c7d7154ef8e41580ab
Also: Recognize method expressions with parameterized receivers
and report an error for now (not implemented yet).
Change-Id: I96405b2b739d8e9fff6e9840347d3e78bfe8b6ec
To make the import work, the import needs to be changed into
`import "./chan"`, the file testdata/chans.go2 needs to be
copies into a new directory testdata/chan and the file must
be renamed to chans.go.
But this exposes some problem with instantiating parameterized
types with other type parameters. Delaying this for now.
Change-Id: I1cd784fb89c2374131f3b1105493492eb0189abc
... and various related smaller fixes
We can now type-check the slices, chans, and maps
examples from the design doc (with the maps example
prodcing an error because importing chans doesn't
work yet). Progress!
Change-Id: Ifc00359a9a1cdad3bde1659a7de2028ac2544469
- moved type parameter collection out of resolver and into decl phase
- collect (meta-) type information (contracts) for type parameters
- first cut at checking contract satisfaction (methods only for now)
Change-Id: I46707969a172423738171aaea9d5282fb4b25a44
- collect contract methods in respective interfaces
- basic checking on contract type constraints
Contracts are not yet tested against or used to type check
function bodies.
Change-Id: I13b00c44524e599f92f1ba5b4b5d6734e2bf22e1
- type parameters can be printed
- empty contracts can be printed
- contract constraints are missing (only started)
Change-Id: I787898203aeb064b0b2ac49a7b858313cee5f45b
Also: Added -h flag to test framework; setting -h causes a panic when
an error is reported (for debugging).
Change-Id: Ib45d4ef38769f2ecdd3ce53fa3aed9fd99ef6bc8
Next known issue:
type List(type E) []E
var _ List(List(List(int))) = [](List(List(int))){}
This won't work because the RHS's composite literal type
is not fully instantiated - it has non-instantiated elements
which are ignored by Checker.instantiatedType. We could just
call Checker.subst w/o any parameters and have it walk the
incoming type and instantiate any element Parameterized types
but that is expensive and also appears to lead to stack over-
flow. Need to investigate.
Change-Id: I80bd09ab5f06e3991f198965424ce3322c7d6402
testdata/typeinst2.go has some complex examples that pass now.
But code elsewhere seems broken. go test has some issues at the
moment. Committing anyway to not lose the snapshot.
Change-Id: Id8f753a7b098405e2580a45ca1707ef6476c198e
- use the right type when instantiating a type
- substitute defined types when underlying types are changing
- improved printing of parametrized types
- improved printing of instantiated types
- more top-level trace output
Change-Id: Ie8a65c9cc51e80925d3f580f54dcc6c0b5abe4c6
- move init and method parameter checks to resolve phase as well
- more consistent error messages
- more tests
Change-Id: I6cb147b35385541ca5d7d7e3f87159df84cced76
Updated go/ast and go/parser. go/types doesn't process the
new data structure yet and is missing a good type representation
of contracts.
Change-Id: I101f7c9e98008840dd1edb55404bb97db5a66ccd
Additionally, simplify the syntax for contracts specified in a
type parameter list: It is now not possible to provide explicit
type parameters to a contract in a type parameter list - they
are always implicit. For instance
func f(type P1, P2 C(P1, P2)) ...
must be written as
func f(type P1, P2 C) ...
If a different order or different types are desired for C,
a new "intermediate" contract must be declared, as in
contract C'(A, B) {
C(B, int) // here we allow type parameters
}
func f(type P1, P2 C')
This simplification will remove confusion if we decide to
allow individual contracts in type parameter lists, such as
func f(type P C, P1, P2 C') ...
In this case, C accepts one type parameter (and applies to P),
and C' accepts two type parameters and applies to P1 and P2.
The simplification avoids questions such as whether this code
should be permitted:
func f(type P C(P2), P1, P2 C'(P, P1)) ...
(i.e., can pass P2 to C, or P1 to C', etc.)
This change switches parsing back to using () parentheses for
type parameters. The parser now also accepts contracts in
parametrized type declarations, and top-level declared
contracts as defined in the design draft:
contract C(T1, T2) { ... }
(Internally, they are mapped to an ast.ContractType as before.)
Added more tests, incl. map.go2 from the design draft, and removed
some unused parser functions.
Passes parser and types tests.
Known issue: Composite literals with instantiated composite literal
types are not recognized properly: T(P){...} fails to parse.
This change implements parsing of contracts based on the
most recent design (using a combination of methods and
explicit basic types as well as the short-hand notations
0, 0.0, 0i, ==, and !=). At the moment, a contract is
considered a "type" and declared as such:
type C contract(T1, T2) { ...}
This change also implements parsing of type instantiations
and type parameters for type declarations, using both the
() parentheses and [] brackets (if the flag useBrackets is
set in parser.go).
Not all parsed data structures are set up correctly in the
AST yet. The parser and ast tests pass.
Change-Id: I11ce64ad49e404c5a66ce6623edc8313e803e135
This change implements parsing and type-checking
of parametrized functions (without contracts).
Type-checking includes checking of generic function
calls using explicit type parameters as well as
implicit type parameters inferred from the actual
arguments.
Change-Id: I03c9c6912aa1e2ac79d9c5125fd5ac72df4e808a
This alert is triggering occasionally. I've investigated the
collisions that happen, and they all seem to be pairwise, so they are
not a big deal. "pairwise" = when there are 32 collisions, it is two
keys mapping to the same hash, 32 times, not 33 keys all mapping to
the same hash.
Add some t.Logf calls in case this comes back, which will help isolate
the problem.
Fixes#39352
Change-Id: I1749d7c8efd0afcf9024d8964d15bc0f58a86e4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/237718
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Support for linux/386 was added to Delve in version 1.4.1, but the
version of Delve currently installed on the linux-386-longtest
builder is 1.2.0. That isn't new enough, which causes the test
to fail. Skip it on that builder until it can be made to work.
The only reason it used to pass on the linux-386-longtest builder
before is because that builder was misconfigured to run tests for
linux/amd64. This was resolved in CL 234520.
Also improve internal documentation and the text of skip reasons.
Fixes#39309.
Change-Id: I395cb1f076e59dd3a3feb53e1dcdce5101e9a0f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/237603
Reviewed-by: David Chase <drchase@google.com>
We replaced http.DefaultClient with securityPreservingHTTPClient,
but we still need that too many redirects check. This issue introduced
by CL 156838.
We introduce a special path to test rediret requests in the script test
framework. You can specify the number of redirects in the path.
$GOPROXY/redirect/<count>/...
Redirect request sequence details(count=8):
request: $GOPROXY/mod/redirect/8/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/7/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/6/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/5/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/4/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/3/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/2/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/1/rsc.io/quote/@v/v1.2.0.mod
the last: $GOPROXY/mod/rsc.io/quote/@v/v1.2.0.mod
Fixes#39482
Change-Id: I149a3702b2b616069baeef787b2e4b73afc93b0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/237177
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This API and functionality was added late in the Go 1.15 release
cycle, and use within gopls has revealed some shortcomings. It's
possible (but not decided) that we'll want a different API long-term,
so for now this CL renames UsesCgo to a non-exported name to avoid
long-term commitment under the Go 1 compat guarantee.
Updates #16623.
Updates #39072.
Change-Id: I04bc0c161a84adebe43e926df5df406bc794c3db
Reviewed-on: https://go-review.googlesource.com/c/go/+/237417
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The scheduler assumes two special invariants that apply to tuple
selectors (Select0 and Select1 ops):
1. There is only one tuple selector of each type per generator.
2. Tuple selectors and generators reside in the same block.
Prior to this CL the assumption was that these invariants would
only be broken by the CSE pass. The CSE pass therefore contained
code to move and de-duplicate selectors to fix these invariants.
However it is also possible to write relatively basic optimization
rules that cause these invariants to be broken. For example:
(A (Select0 (B))) -> (Select1 (B))
This rule could result in the newly added selector (Select1) being
in a different block to the tuple generator (see issue #38356). It
could also result in duplicate selectors if this rule matches
multiple times for the same tuple generator (see issue #39472).
The CSE pass will 'fix' these invariants. However it will only do
so when optimizations are enabled (since disabling optimizations
disables the CSE pass).
This CL moves the CSE tuple selector fixup code into its own pass
and makes it mandatory even when optimizations are disabled. This
allows tuple selectors to be treated like normal ops for most of
the compilation pipeline until after the new pass has run, at which
point we need to be careful to maintain the invariant again.
Fixes#39472.
Change-Id: Ia3f79e09d9c65ac95f897ce37e967ee1258a080b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237118
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This removes the same logic from run.bat that was removed from
cmd/dist in CL 236819.
The duplicated logic was removed from run.bash and run.rc in CL 6531,
but that part of run.bat was apparently missed (and not noticed
because its effect was redundant).
Also fix a path-separator bug in cmd/addr2line.TestAddr2Line that was
exposed as a result.
Fixes#39478
Updates #39385
Change-Id: I00054966cf92ef92a03681bf23de7f45f46fbb5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/237359
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Restore previously sent SCTs and stapled OCSP response during session
resumption for both TLS 1.2 and 1.3. This behavior is somewhat
complicated for TLS 1.2 as SCTs are sent during the server hello,
so they override what is saved in ClientSessionState. It is likely
that if the server is sending a different set of SCTs there is probably
a reason for doing so, such as a log being retired, or SCT validation
requirements changing, so it makes sense to defer to the server in
that case.
Fixes#39075
Change-Id: I3c0fa2f69c6bf0247a447c48a1b4c733a882a233
Reviewed-on: https://go-review.googlesource.com/c/go/+/234237
Reviewed-by: Filippo Valsorda <filippo@golang.org>
When the arrangement specifier is "B16", the 30-bit should be 1 rather than 0.
This CL fixes this error.
Fixes#39445
Change-Id: Ib44881cdb8b3aab855cb30f2c52a085cd73a6a2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/236638
Run-TryBot: eric fang <eric.fang@arm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Also do not unset it by default in the tests for cmd/go.
GOROOT_FINAL affects the GOROOT value embedded in binaries,
such as 'cmd/cgo'. If its value changes and a build command
is performed that depends on one of those binaries, the binary
would be spuriously rebuilt.
Instead, only unset it in the specific tests that make assumptions
about the GOROOT paths embedded in specific compiled binaries.
That may cause those tests to do a little extra rebuilding when
GOROOT_FINAL is set, but that little bit of extra rebuilding
seems preferable to spuriously-stale binaries.
Fixes#39385
Change-Id: I7c87b1519bb5bcff64babf1505fd1033ffa4f4fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/236819
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
CL 236857 removed all uses of whitelist/blacklist, which is great.
But it substituted awkward phrasing using allowlist/blocklist,
especially as verbs or participles. This CL uses more standard English,
like "allow the function" or "blocked functions" instead of
"allowlist the function" or "blocklisted functions".
Change-Id: I9106a2fdbd62751c4cbda3a77181358a8a6d0f13
Reviewed-on: https://go-review.googlesource.com/c/go/+/236917
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The removed line assumed that the script's WORK directory is not a
child of any directory containing version-control metadata.
While that assumption does hold in most cases, it does not hold when,
for example, $TMPDIR is $HOME/tmp and $HOME/.git/config exists.
A similar situation may or may not arise when using
golang.org/x/build/cmd/release. Either way, the assertion is incorrect
and was interfering with local testing for #39385.
Updates #39385Fixes#39431
Change-Id: I67813d7ce455aa9b56a6eace6eddebf48d0f7fa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/236818
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
The page sweeper depends on spans being marked if any object in the
span is marked, but currently only greyobject does this.
gcmarknewobject and wbBufFlush1 also mark objects, but neither set
span marks. As a result, if there are live objects on a span, but
they're all marked via allocation or write barriers, then the span
itself won't be marked and the page reclaimer will free the span,
ultimately leading to memory corruption when the memory for those live
allocations gets reused.
Fix this by making gcmarknewobject and wbBufFlush1 also mark pages.
No test because I have no idea how to reliably (or even unreliably)
trigger this.
Fixes#39432.
Performance is a wash or very slightly worse. I benchmarked the
gcmarknewobject and wbBufFlush1 changes independently and both showed
a slight performance improvement, so I'm going to call this noise.
name old time/op new time/op delta
BiogoIgor 15.9s ± 2% 15.9s ± 2% ~ (p=0.758 n=25+25)
BiogoKrishna 15.7s ± 3% 15.7s ± 3% ~ (p=0.382 n=21+21)
BleveIndexBatch100 4.94s ± 3% 5.07s ± 4% +2.63% (p=0.000 n=25+25)
CompileTemplate 204ms ± 1% 205ms ± 1% +0.43% (p=0.000 n=21+23)
CompileUnicode 77.8ms ± 1% 78.1ms ± 1% ~ (p=0.130 n=23+23)
CompileGoTypes 731ms ± 1% 733ms ± 1% +0.30% (p=0.006 n=22+22)
CompileCompiler 3.64s ± 2% 3.65s ± 3% ~ (p=0.179 n=24+25)
CompileSSA 8.44s ± 1% 8.46s ± 1% +0.30% (p=0.003 n=22+23)
CompileFlate 132ms ± 1% 133ms ± 1% ~ (p=0.098 n=22+22)
CompileGoParser 164ms ± 1% 164ms ± 1% +0.37% (p=0.000 n=21+23)
CompileReflect 455ms ± 1% 457ms ± 2% +0.50% (p=0.002 n=20+22)
CompileTar 182ms ± 2% 182ms ± 1% ~ (p=0.382 n=22+22)
CompileXML 245ms ± 3% 245ms ± 1% ~ (p=0.070 n=21+23)
CompileStdCmd 16.5s ± 2% 16.5s ± 3% ~ (p=0.486 n=23+23)
FoglemanFauxGLRenderRotateBoat 12.9s ± 1% 13.0s ± 1% +0.97% (p=0.000 n=21+24)
FoglemanPathTraceRenderGopherIter1 18.6s ± 1% 18.7s ± 0% ~ (p=0.083 n=23+24)
GopherLuaKNucleotide 28.4s ± 1% 29.3s ± 1% +2.84% (p=0.000 n=25+25)
MarkdownRenderXHTML 252ms ± 0% 251ms ± 1% -0.50% (p=0.000 n=23+24)
Tile38WithinCircle100kmRequest 516µs ± 2% 516µs ± 2% ~ (p=0.763 n=24+25)
Tile38IntersectsCircle100kmRequest 689µs ± 2% 689µs ± 2% ~ (p=0.617 n=24+24)
Tile38KNearestLimit100Request 608µs ± 1% 606µs ± 2% -0.35% (p=0.030 n=19+22)
[Geo mean] 522ms 524ms +0.41%
https://perf.golang.org/search?q=upload:20200606.4
Change-Id: I8b331f310dbfaba0468035f207467c8403005bf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/236817
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Rather than hashing the encoding of the SPKI structure, hash the
bytes of the public key itself.
Fixes#39429
Change-Id: I55a0f8f08ab1f1b5702590b47d8b9a92d1dbcc1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/236878
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Previously, if there was a non-directory file with the name vendor or
testdata in the Go source tree, it was possible for some directories
to be skipped by filepath.Walk performed in findGorootModules.
As unusual and unlikely as such non-directory files are, it's better
to ensure all directories are visited, and all modules in the GOROOT
source tree are found.
This increases confidence that tests relying on findGorootModule
will not have unexpected false negatives.
For #36851.
For #36907.
Change-Id: I468e80d8f57119e2c72d546b3fd1e23c31fd6e6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/236600
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This is a followup to CL 96495.
It should be simpler and more robust to achieve .bat files having
CRLF line endings by treating it as a binary file, like all other
files, and checking it in with the desired CRLF line endings.
A test is used to check the entire Go tree, short of directories
starting with "." and named "testdata", for any .bat files that
have anything other than strict CRLF line endings. This will help
catch any accidental modifications to existing .bat files or check
ins of new .bat files.
Importantly, this is compatible with how Gerrit serves .tar.gz files,
making it so that CRLF line endings are preserved.
The Go project is supported on many different environments, some of
which may have limited git implementations available, or none at all.
Relying on fewer git features and special rules makes it easier to
have confidence in the exact content of all files. Additionally, Go
development started in Subversion, moved to Perforce, then Mercurial,
and now uses Git.¹ Reducing its reliance on git-specific features will
help if there will be another transition in the project's future.
There are only 5 .bat files in the entire Go source tree, so a new one
being added is a rare event, and we prefer to do things in Go instead.
We still have the option of improving the experience for developers by
adding a pre-commit converter for .bat files to the git-codereview tool.
¹ https://groups.google.com/d/msg/golang-dev/sckirqOWepg/YmyT7dWJiocJFixes#39391.
For #37791.
Change-Id: I6e202216322872f0307ac96f1b8d3f57cb901e6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/236437
Reviewed-by: Bryan C. Mills <bcmills@google.com>
There's been plenty of discussion on the usage of these terms in tech.
I'm not trying to have yet another debate. It's clear that there are
people who are hurt by them and who are made to feel unwelcome by their
use due not to technical reasons but to their historical and social
context. That's simply enough reason to replace them.
Anyway, allowlist and blocklist are more self-explanatory than whitelist
and blacklist, so this change has negative cost.
Didn't change vendored, bundled, and minified files. Nearly all changes
are tests or comments, with a couple renames in cmd/link and cmd/oldlink
which are extremely safe. This should be fine to land during the freeze
without even asking for an exception.
Change-Id: I8fc54a3c8f9cc1973b710bbb9558a9e45810b896
Reviewed-on: https://go-review.googlesource.com/c/go/+/236857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Khosrow Moossavi <khos2ow@gmail.com>
Reviewed-by: Leigh McCulloch <leighmcc@gmail.com>
Reviewed-by: Urban Ishimwe <urbainishimwe@gmail.com>
This change makes the direct call darwin loadSystemRoots implementation
match the existing cgo implementation, which in turn _mostly_ matches
the Apple implementation. The main change here is that when
SecTrustSettingsCopyTrustSettings the error is ignored, and can either
cause a fallback to check admin trust settings, or cause the
certificate to be marked kSecTrustSettingsResultUnspecified.
As well as updating the implementation to match the cgo one, this
change also updates the documentation of how the fallbacks work and
how they match the Apple implementations. References are made to the
Apple source where appropriate. This change does not update the
existing comments in the cgo implementation, since the goal is to
delete that code once the direct call implementation is matured.
Updates #38888
Change-Id: Id0344ea9d2eede3b715f341e9cbd3c1c661b7a90
Reviewed-on: https://go-review.googlesource.com/c/go/+/233360
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
The ConnectionState's CipherSuite was not set prior
to the VerifyConnection callback in TLS 1.2 servers,
both for full handshakes and resumptions.
Change-Id: Iab91783eff84d1b42ca09c8df08e07861e18da30
Reviewed-on: https://go-review.googlesource.com/c/go/+/236558
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
There's a comment on the Syscall function that's supposed to be an
internal implementation note, but since it's not separated from the
function definition, it appears in godoc. Add a blank line to prevent
this.
Change-Id: Iba307f1cc3844689ec3c6d82c21d441852e35bca
Reviewed-on: https://go-review.googlesource.com/c/go/+/236561
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing documentation of WriteFile does not make it clear for
non-native English speakers that it will not change the permissions if
the file already exists before.
Fixes#35711
Change-Id: If861c3e3700957fc9ac3d5313351c57d399d3f58
Reviewed-on: https://go-review.googlesource.com/c/go/+/218417
Reviewed-by: Rob Pike <r@golang.org>
"Fedora" and "Red Hat" are not numbers, it turns out.
Don't rely on version numbers, instead use a regexp to
handle variation across the 2 patterns thus far observed
for gdb-generated Go type names.
Change-Id: I18c81aa2848265a47daf1180d8f6678566ae3f19
Reviewed-on: https://go-review.googlesource.com/c/go/+/236280
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, for symbols defined in other packages and referenced
by index, we don't record its name in the object file, as the
linker doesn't need the name, only the index. As a consequence,
tools like objdump and nm also don't know the referenced symbol
names and cannot dump it properly.
This CL adds referenced symbol names to the object file. So the
object file is self-contained. And tools can retrieve referenced
symbol names properly.
Tools now should work as good for new object files as for old
object files.
Fixes#38875.
Change-Id: I16c685c1fd83273ab1faef474e19acf4af46396f
Reviewed-on: https://go-review.googlesource.com/c/go/+/236168
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This reverts CL 229246.
For new indexed object files, in CL 229246 we added symbol index
to tools (nm, objdump) output. This affects external tools that
parse those outputs. And the added index doesn't look very nice.
In this release we take it out. For future releases we may
introduce a flag to tools (nm, objdump) and optionally dump the
symbol index.
For refererenced (not defined) indexed symbols, currently the
symbol is still referenced only by index, not by name. The next
CL will make the object file self-contained, so tools can dump
the symbol names properly (as before).
For #38875.
Change-Id: I07375e85a8e826e15c82fa452d11f0eaf8535a00
Reviewed-on: https://go-review.googlesource.com/c/go/+/236167
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Also add a test that could reproduce this error and
ensure it doesn't occur in other configurations.
Fixes#39012
Change-Id: If792b5131f312c269fd2c5f08c9ed5c00188d1af
Reviewed-on: https://go-review.googlesource.com/c/go/+/233957
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Add a test case for an issue with how Go emits DWARF line tables,
specifically relating to the line table "end sequence" operator.
Updates #38192.
Change-Id: I878b262e6ca6c550c0e460c3d5a1969ac4a2c31b
Reviewed-on: https://go-review.googlesource.com/c/go/+/235917
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This reverts commit 7eab9506c9.
Reason for revert: Undoing to get back to semantics discussed in #8606.
Change-Id: If0cd7518c10c37a81fdbb4ae112239e04c0b1448
Reviewed-on: https://go-review.googlesource.com/c/go/+/236278
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit 1cc7be89a9.
Reason for revert: Undoing to get back to semantics discussed in #8606.
Change-Id: Ib44a2e79cf113b3d15c3546cd8aa6fc27860819e
Reviewed-on: https://go-review.googlesource.com/c/go/+/236146
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Hand-verified for listed gdb versions. Gdb (apparently)
changed the way it names certain Go types, and this change
broke the pretty-printer-activating code in runtime-gdb.py
runtime-gdb_test.go now checks channel, map, string, and slice
printing unconditionally (i.e., no opt-out for old versions).
Updates #39368.
Change-Id: I98d72e1291c66bd40d970990e1a377ff2ed0c5d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/236164
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current document mismatches Go syntax loads a signed-byte
instruction "MOVB" with GNU syntax loads an 64bit double-word
instruction "ldr". This is just a typo in the document, the
assembler has the correct encoding. This patch fix this error.
Fixes#39367
Change-Id: Idb8f65ca540514ee5bc8f07073e756838710ba93
Reviewed-on: https://go-review.googlesource.com/c/go/+/236217
Reviewed-by: Cherry Zhang <cherryyz@google.com>
During DWARF line table emission in the linker, prior to issuing a
DW_LNE_end_sequence op to mark the end of the line table for a
compilation unit, advance the PC to produce an address beyond the last
text address in the unit (this is required by the DWARF standard).
Because of the way that GDB interprets end-sequence ops, we were
effectively losing the last row in the line table for each unit, which
degraded the debugging experience.
This problem has been around for a while, but has surfaced recently
due to changes in line table generation. Prior to Go 1.14, the DWARF
line table was emitted entirely in the linker, and a single monolithic
line table was created for each Go package (including functions from
assembly). In 1.14 we moved to having the compiler emit line table
fragments for each function, and having the linker stitch together the
fragments. As part of this change we moved to a model in which each
"go tool compile/asm" output has its own DWARF line table instance,
meaning that there are many more "end sequence" ops, which made the
problem more visible.
Fixes#38192.
Change-Id: Ic29e2f6e0ac952360c81fcba5268ad70b2b44184
Reviewed-on: https://go-review.googlesource.com/c/go/+/235739
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Previously we did not steal timers from running P's, because that P
should be responsible for running its own timers. However, if the P
is running a CPU-bound G, this can cause measurable delays in running
ready timers. Also, in CL 214185 we avoided taking the timer lock of a P
with no ready timers, which reduces the chances of timer lock contention.
So, if we can't find any ready timers on sleeping P's, try stealing
them from running P's.
Fixes#38860
Change-Id: I0bf1d5dc56258838bdacccbf89493524e23d7fed
Reviewed-on: https://go-review.googlesource.com/c/go/+/232199
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This is needed for cgo support for libpsx.
Fixes: #39361
Change-Id: I500f5614ea4b82b085322af1f1ffeb1f55270a05
Reviewed-on: https://go-review.googlesource.com/c/go/+/236139
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The existing documentation for time format constants doesn't mention
that they may parse technically-invalid strings, such as single-digit
hours when a two-digit hour is required by a specification. This commit
adds a short warning note to that effect.
Fixes#37616
Change-Id: I6e5e12bd42dc368f8ca542b4c0527a2b7d30acaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/229460
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
"Something" changed the names of types in gdb, causing the
pretty-printer matchers to fail to match. This tracks that
change.
Updated runtime-gdb_test.go to include a slice and a channel printing test.
(The straightforward printing of a slicevar doesn't work because
of compiler DWARF problems describing the slicevar, not gdb problems).
Change-Id: I21607a955b9c894f11ecf3763aea2a6dd59a3f42
Reviewed-on: https://go-review.googlesource.com/c/go/+/235926
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This teaches vet to recognize %O in a fmt.Printf format string.
O has been supported since the 1.13 release, but vet would warn about it.
Fixes#29986
Change-Id: I3a7a1fc8141f32888c081c5d92620461624371f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/236138
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds a comment to the Verify documentation that indicates
that you can use URI and email style name constraints with a leading
period for DNS names (and explains what they do). This behavior is
not standards compliant, but matches the community application of
RFC 5280, so it makes sense to document it.
Fixes#37535
Change-Id: Ibd6f039e4fa46d40ad7ae1ab48eab86f13cf8eff
Reviewed-on: https://go-review.googlesource.com/c/go/+/233525
Reviewed-by: Ryan Sleevi <sleevi@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
There are data races on fd.[rw]aio and fd.[rw]timedout when Read/Write
is called on a polled fd concurrently with SetDeadline (see #38769).
Adding a mutex around accesses to each pair (read and write) prevents
the race, which was causing deadlocks in net/http tests on the builders.
Updates #38769.
Change-Id: I31719b3c9a664e81a775cda583cff31c0da946c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/235820
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Call FlushViewOfFile before unmapping the output file, for extra
safety. The documentation says the function does not wait for
the data to be written to disk, so it should be cheap.
Fixes#38440.
Change-Id: I05352f15d9305e6e7086a002f61802f74036b710
Reviewed-on: https://go-review.googlesource.com/c/go/+/235639
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
The version "" denotes the main module, which has no version. The
mvs.Reqs interface documentation hints this is allowed, but it's not
obvious from the implementation in modload.mvsReqs.Max.
Also, replace a related TODO with a comment in mvs.Downgrade.
Fixes#39042
Change-Id: I11e10908c9b3d8c2283eaa5c04bd8e1b936851fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/234003
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
When printing regular test output check the indentation of the output, and use
the report stack to find the appropriate test name for that output.
This change includes a whitespace change to some golden test files. The
indentation of tests was changed in CL 113177
from tabs to spaces. The golden files have been updated to match the new
output format. The tabs in the golden files cause problems because the indentation check
looks for 4 spaces.
Fixes#29755
Updates #25369
Change-Id: Iebab51816a9755168083a7a665b41497e9dfd85f
GitHub-Last-Rev: 898827f1a6
GitHub-Pull-Request: golang/go#34419
Reviewed-on: https://go-review.googlesource.com/c/go/+/196617
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This change fixes a race condition between beforeIdle waking up the
innermost event handler and a timer causing a different goroutine to
wake up at the exact same moment. This messes up the wasm event handling
and leads to memory corruption. The solution is to make beforeIdle
return the goroutine that must run next and have findrunnable pick
this goroutine without considering timers again.
Fixes#38093Fixes#38574
Change-Id: Iffbe99411d25c2730953d1c8b0741fd892f8e540
Reviewed-on: https://go-review.googlesource.com/c/go/+/230178
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Similarly to EGLDisplay, EGLConfig is declared as a pointer but may
contain non-pointer values.
I believe this is the root cause of https://todo.sr.ht/~eliasnaur/gio/121.
Change-Id: I412c4fbc2eef4aa028534d68bda95db98e3a365d
Reviewed-on: https://go-review.googlesource.com/c/go/+/235817
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The correct word to use here is 'another' not 'an other'
Change-Id: Ie4f748ae94a5945dceb52779222ffd8cf36b8845
Reviewed-on: https://go-review.googlesource.com/c/go/+/235838
Reviewed-by: Robert Griesemer <gri@golang.org>
Enforces section 14.13 of RFC 2616 so that Content-Length header
values with a sign such as "+5" will be rejected.
Updates #39017
Change-Id: Icce9f00d03c8475fe704b33f9bed9089ff8802f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/234817
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This reverts https://golang.org/cl/232578.
Reason for revert: This commit broke TestScript/mod_load_badchain,
which is causing all longtest builders to fail.
Change-Id: I4a17392ce74ac3a7ad340980556025f669d94b65
Reviewed-on: https://go-review.googlesource.com/c/go/+/235857
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
When run with stdin == /dev/null and stdout/stderr == pipe (i.e., as
os/exec.Command.CombinedOutput), GDB suffers from a bug
(https://sourceware.org/bugzilla/show_bug.cgi?id=26056) that causes
SIGSEGV when sent a SIGWINCH signal.
Package runtime tests TestEINTR and TestSignalDuringExec both send
SIGWINCH signals to the entire process group, thus including GDB if one
of the GDB tests is running in parallel.
TestEINTR only intends its signals for the current process, so it is
changed to do so. TestSignalDuringExec, really does want its signals to
go to children. However, it does not call t.Parallel(), so it won't run
at the same time as GDB tests.
This is a simple fix, but GDB is vulnerable, so we must be careful not
to add new parallel tests that send SIGWINCH to the entire process
group.
Fixes#39021
Change-Id: I803606fb000f08c65c1b10ec554d4ef6819e5dd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/235557
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
CL 228017 added a new help page 'go help buildconstraint' which
summarized the information on build constraints in the go/build
documentation. The summary was almost as long as the go/build
documentation, since there's very little that can be left out.
This CL moves the original go/build documentation to
'go help buildconstraint' to eliminate redundnancy. The text
describing enabled tags is slightly different (targeting command-line
users more than go/build users), but the rest of the documentation is
unchanged.
Fixes#37018
Change-Id: Ic0ed4c6fdae2395dd58852e1600c701247c9c4cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/232981
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
In module mode, 'go get' prints a message for each version query it
resolves. This change groups those messages together near the end of
the output so they aren't mixed with other module "finding" and
"downloading" messages. They'll still be printed before build-related
messages.
Fixes#37982
Change-Id: I107a9f2b2f839e896399df906e20d6fc77f280c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/232578
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
New test case for issue 38068, which deals with build reproducibility:
do a pair of compilations, the first with the concurrent back end
turned on, and the second with -c=1, then check to make sure we get
the same output (using a test case that triggers late inlining into
wrapper methods).
Updates #38068.
Change-Id: I4afaf78898706a66985f09d18f6f6f29876c9017
Reviewed-on: https://go-review.googlesource.com/c/go/+/234417
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
AddCookie properly encodes a cookie and appends it to the Cookie header
field but does not modify or sanitize what the Cookie header field
contains already. If a user manualy sets the Cookie header field to
something not conforming to RFC 6265 then a cookie added via AddCookie
might not be retrievable.
Fixes#38437
Change-Id: I232b64ac489b39bb962fe4f7dbdc2ae44fcc0514
Reviewed-on: https://go-review.googlesource.com/c/go/+/235141
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
fieldInfo.value used to initialize nil anonymous struct fields if they
were encountered. This behavior is wanted when decoding, but not when
encoding. When encoding, the value should never be modified, and these
nil fields should be skipped entirely.
To fix the bug, add a bool argument to the function which tells the
code whether we are encoding or decoding.
Finally, add a couple of tests to cover the edge cases pointed out in
the original issue.
Fixes#27240.
Change-Id: Ic97ae4bfe5f2062c8518e03d1dec07c3875e18f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/196809
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This reverts golang.org/cl/179337.
Reason for revert: broke a few too many reasonably valid Go programs.
The previous behavior was perhaps less consistent, but the docs were
never very clear about when the decoder merges with existing values,
versus replacing existing values altogether.
Fixes#39149.
Change-Id: I1c1d857709b8398969fe421aa962f6b62f91763a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234559
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Previously, when the target (“old”) path passed to os.Symlink was a
“root-relative” Windows path,¹ we would erroneously prepend
destination (“new”) path when determining which path to Stat,
resulting in an invalid path which was then masked by the lack of
error propagation for the Stat call (#39183).
If the link target is a directory (rather than a file), that would
result in the symlink being created without the
SYMBOLIC_LINK_FLAG_DIRECTORY flag, which then fails in os.Open.
¹https://docs.microsoft.com/en-us/windows/win32/fileio/creating-symbolic-links
Updates #39183
Change-Id: I04f179cd2b0c44f984f34ec330acad2408aa3a20
Reviewed-on: https://go-review.googlesource.com/c/go/+/235317
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Android O and newer blocks the dup2 syscall.
Change-Id: Ibca01fc72ef114deeef6c0450a8b81a556ed0530
Reviewed-on: https://go-review.googlesource.com/c/go/+/235537
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When go115ReduceLiveness is true (so we don't emit actual
register maps), use StackMapDontCare consistently for the
register map index, so RegMapValid is always false.
This fixes a compiler crash when doing -live=2 debug print.
Fixes#39251.
Change-Id: Ice087af491fa69c413f8ee59f923b72d592c0643
Reviewed-on: https://go-review.googlesource.com/c/go/+/235418
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The paths for the other "go" commands in this file were fixed in CL 223741,
but this one was missed (and run.bat is not caught by the builders).
Change-Id: Iba1efddc7d2fbe6af39c39d643508decc954bbc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/234758
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
In certain cases the HTTP/2 stack needs to resend a request.
It obtains a fresh body to send by calling req.GetBody.
This call was missing from the path where the HTTP/2
round tripper returns ErrSkipAltProtocol, meaning fall back
to HTTP/1.1. The result was that the HTTP/1.1 fallback
request was sent with no body at all.
This CL changes that code path to rewind the body before
falling back to HTTP/1.1. But rewinding the body is easier
said than done. Some requests have no GetBody function,
meaning the body can't be rewound. If we need to rewind and
can't, that's an error. But if we didn't read anything, we don't
need to rewind. So we have to track whether we read anything,
with a new ReadCloser wrapper. That in turn requires adding
to the couple places that unwrap Body values to look at the
underlying implementation.
This CL adds the new rewinding code in the main retry loop
as well.
The new rewindBody function also takes care of closing the
old body before abandoning it. That was missing in the old
rewind code.
Thanks to Aleksandr Razumov for CL 210123
and to Jun Chen for CL 234358, both of which informed
this CL.
Fixes#32441.
Change-Id: Id183758526c087c6b179ab73cf3b61ed23a2a46a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234894
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
All GDB tests currently ignore non-zero exit statuses. When tests
flakes, we don't even know if GDB exited successfully or not.
Add checks for non-zero exits, which are not expected.
Furthermore, always log the output from GDB. The tests are currently
inconsistent about whether they always log, or only on error.
Updates #39021
Change-Id: I7af1d795fc2fdf58093cb2731d616d4aa44e9996
Reviewed-on: https://go-review.googlesource.com/c/go/+/235282
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When the output file is mmap'd, OutBuf.Close currently munmap the
file but doesn't actually close the file descriptor. This CL
makes it actually close the FD.
Change-Id: I053c5592ae95497228c50ce6a267b3b48f0af6d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/235417
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Uses the `cfg.BuildN` flag to avoid deleting inside the `if cleanCache`
block. Introduces a test in src/cmd/go/testdata/script.
Fixes#39250
Change-Id: I857c441b1d7aa7c68cfd646d6833e6eaca5b18d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/235140
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
If GOPROXY is "", we set it to the default value,
"https://proxy.golang.org,direct". However, if GOPROXY is a non-empty
string that doesn't contain any URLs or keywords, we treat it as
either "off" or "noproxy", which can lead to some strange errors.
This change reports an error for this kind of GOPROXY value.
For #39180
Change-Id: If2e6e39d6f74c708e5ec8f90e9d4880e0e91894f
Reviewed-on: https://go-review.googlesource.com/c/go/+/234857
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This fixes a bug in CL 228777 which disallowed
a MaxPathLen of -1 without IsCA, even though the
x509.Certificate documentation indicates that
MaxPathLen of -1 is considered "unset".
Updates #38216
Change-Id: Ib7240e00408d060f27567be8b820d0eee239256f
Reviewed-on: https://go-review.googlesource.com/c/go/+/235280
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Set linker explicitly to lld because the default does not work on NDK
versions r19c, r20, r20b and r21. NDK 18b (or earlier) based builds
will need to specify -fuse-ld=gold.
Fixes#38838
Change-Id: Ib75f71fb9896b843910f41bd12aa1e36868fa9b3
GitHub-Last-Rev: eeaa171604
GitHub-Pull-Request: golang/go#39217
Reviewed-on: https://go-review.googlesource.com/c/go/+/235017
Reviewed-by: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add two no op instructions following svc on openbsd/arm64 and swi on openbsd/arm.
All except some of the most recent arm64 processors have a speculative execution
flaw that occurs across a syscall boundary, which cannot be mitigated in the
kernel. In order to protect against this leak a speculation barrier needs to be
placed after an svc or swi instruction.
In order to avoid the performance impact of these instructions, the OpenBSD 6.7
kernel returns execution two instructions past the svc or swi call. For now two
hardware no ops are added, which allows syscalls to work with both 6.6 and 6.7.
These should be replaced with real speculation barriers once OpenBSD 6.8 is
released.
Updates #36435
Change-Id: I06153cb0998199242cca8761450e53599c3e7de4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234381
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 235017 is about to change the default Android linker to lld. lld doesn't
support the --compress-debug-sections=zlib-gnu flag, but linkerFlagSupported
doesn't take any alternative linkers specified with -fuse-ld into account.
Updates #38838
Change-Id: I5f7422c06d40dedde2e4b070fc48398e8f822190
Reviewed-on: https://go-review.googlesource.com/c/go/+/235157
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The parallel chatty tests added in CL 229085 fail on the
solaris-amd64-oraclerel builder, because a +NN:NN offset time zone is
used. Allow for the `+` character in the corresponding regex to fix
these tests. Also move the '-' to the end of the character class, so it
is not interpreted as the range 9-T.
Change-Id: Iec9ae82ba45d2490176f274f0dc6812666eae718
Reviewed-on: https://go-review.googlesource.com/c/go/+/234978
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Specifically, this change documents the behavior of Unmarshal when a
SEQUENCE contains trailing elements.
For context Unmarshal treats trailing elements of a SEQUENCE that do not
have matching struct fields as valid, as this is how ASN.1 structures
are typically extended. This can be somewhat confusing as you might
expect those elements to be appended to rest, but rest is really only
for trailing data unrelated to the structure being parsed (i.e. if you
append a second sequence to b, it would be returned in rest).
Fixes#35680
Change-Id: Ia2c68b2f7d8674d09e859b4b7f9aff327da26fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/233537
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
The existing example is needlessly complex.
You have to know that t.Sub returns a Duration
and also have to mentally subtract the two times
to understand what duration should be printed.
Rewrite to focus on just the Duration.String operation.
Change-Id: I00765b6019c07a6ff03022625b556c2b9ba87c09
Reviewed-on: https://go-review.googlesource.com/c/go/+/234893
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
A zombie slot is a slot that is marked, but isn't allocated. This can
indicate a bug in the GC, or a bad use of unsafe.Pointer. Currently,
the sweeper has best-effort detection for zombie slots: if there are
more marked slots than allocated slots, then there must have been a
zombie slot. However, this is imprecise since it only compares totals
and it reports almost no information that may be helpful to debug the
issue.
Add a precise check that compares the mark and allocation bitmaps and
reports detailed information if it detects a zombie slot.
No appreciable effect on performance as measured by the sweet
benchmarks:
name old time/op new time/op delta
BiogoIgor 15.8s ± 2% 15.8s ± 2% ~ (p=0.421 n=24+25)
BiogoKrishna 15.6s ± 2% 15.8s ± 5% ~ (p=0.082 n=22+23)
BleveIndexBatch100 4.90s ± 3% 4.88s ± 2% ~ (p=0.627 n=25+24)
CompileTemplate 204ms ± 1% 205ms ± 0% +0.22% (p=0.010 n=24+23)
CompileUnicode 77.8ms ± 2% 78.0ms ± 1% ~ (p=0.236 n=25+24)
CompileGoTypes 729ms ± 0% 731ms ± 0% +0.26% (p=0.000 n=24+24)
CompileCompiler 3.52s ± 0% 3.52s ± 1% ~ (p=0.152 n=25+25)
CompileSSA 8.06s ± 1% 8.05s ± 0% ~ (p=0.192 n=25+24)
CompileFlate 132ms ± 1% 132ms ± 1% ~ (p=0.373 n=24+24)
CompileGoParser 163ms ± 1% 164ms ± 1% +0.32% (p=0.003 n=24+25)
CompileReflect 453ms ± 1% 455ms ± 1% +0.39% (p=0.000 n=22+22)
CompileTar 181ms ± 1% 181ms ± 1% +0.20% (p=0.029 n=24+21)
CompileXML 244ms ± 1% 244ms ± 1% ~ (p=0.065 n=24+24)
CompileStdCmd 15.8s ± 2% 15.7s ± 2% ~ (p=0.059 n=23+24)
FoglemanFauxGLRenderRotateBoat 13.4s ±11% 12.8s ± 0% ~ (p=0.377 n=25+24)
FoglemanPathTraceRenderGopherIter1 18.6s ± 0% 18.6s ± 0% ~ (p=0.696 n=23+24)
GopherLuaKNucleotide 28.7s ± 4% 28.6s ± 5% ~ (p=0.700 n=25+25)
MarkdownRenderXHTML 250ms ± 1% 248ms ± 1% -1.01% (p=0.000 n=24+24)
[Geo mean] 1.60s 1.60s -0.11%
(https://perf.golang.org/search?q=upload:20200517.6)
For #38702.
Change-Id: I8af1fefd5fbf7b9cb665b98f9c4b73d1d08eea81
Reviewed-on: https://go-review.googlesource.com/c/go/+/234100
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The go/build package uses the "go" tool from the user's environment,
but its tests should not assume that that tool is in any particular
state, let alone appropriate for running the test.
Instead, explicitly use testenv.GoTool, adding it to $PATH in a
TestMain when necessary.
Fixes#39199Fixes#39198
Change-Id: I56618a55ced473e75dd96eeb3a8f7084e2e64d02
Reviewed-on: https://go-review.googlesource.com/c/go/+/234880
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
When a locked M wants to start a new M, it hands off to the template
thread to actually call clone and start the thread. The template thread
is lazily created the first time a thread is locked (or if cgo is in
use).
stoplockedm will release the P (_Pidle), then call handoffp to give the
P to another M. In the case of a pending STW, one of two things can
happen:
1. handoffp starts an M, which does acquirep followed by schedule, which
will finally enter _Pgcstop.
2. handoffp immediately enters _Pgcstop. This only occurs if the P has
no local work, GC work, and no spinning M is required.
If handoffp starts an M, and must create a new M to do so, then newm
will simply queue the M on newmHandoff for the template thread to do the
clone.
When a stop-the-world is required, stopTheWorldWithSema will start the
stop and then wait for all Ps to enter _Pgcstop. If the template thread
is not fully created because startTemplateThread gets stopped, then
another stoplockedm may queue an M that will never get created, and the
handoff P will never leave _Pidle. Thus stopTheWorldWithSema will wait
forever.
A sequence to trigger this hang when STW occurs can be visualized with
two threads:
T1 T2
------------------------------- -----------------------------
LockOSThread LockOSThread
haveTemplateThread == 0
startTemplateThread
haveTemplateThread = 1
newm haveTemplateThread == 1
preempt -> schedule g.m.lockedExt++
gcstopm -> _Pgcstop g.m.lockedg = ...
park g.lockedm = ...
return
... (any code)
preempt -> schedule
stoplockedm
releasep -> _Pidle
handoffp
startm (first 3 handoffp cases)
newm
g.m.lockedExt != 0
Add to newmHandoff, return
park
Note that the P in T2 is stuck sitting in _Pidle. Since the template
thread isn't running, the new M will not be started complete the
transition to _Pgcstop.
To resolve this, we disable preemption around the assignment of
haveTemplateThread and the creation of the template thread in order to
guarantee that if handTemplateThread is set then the template thread
will eventually exist, in the presence of stops.
Fixes#38931
Change-Id: I50535fbbe2f328f47b18e24d9030136719274191
Reviewed-on: https://go-review.googlesource.com/c/go/+/232978
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In #24929, we decided to stream chatty test output. It looks like,
foo_test.go:138: TestFoo/sub-1: hello from subtest 1
foo_test.go:138: TestFoo/sub-2: hello from subtest 2
In this CL, we refactor the output to be grouped by === CONT lines, preserving
the old test-file-before-log-line behavior:
=== CONT TestFoo/sub-1
foo_test.go:138 hello from subtest 1
=== CONT TestFoo/sub-2
foo_test.go:138 hello from subtest 2
This should remove a layer of verbosity from tests, and make it easier to group
together related lines. It also returns to a more familiar format (the
pre-streaming format), whilst still preserving the streaming feature.
Fixes#38458
Change-Id: Iaef94c580d69cdd541b2ef055aa004f50d72d078
Reviewed-on: https://go-review.googlesource.com/c/go/+/229085
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Currently sysmon is not stopped when the world is stopped, which is
in general a difficult thing to do. The result of this is that when
tracing starts and the value of trace.enabled changes, it's possible
for sysmon to fail to emit an event when it really should. This leads to
traces which the execution trace parser deems inconsistent.
Fix this by putting all of sysmon's work behind a new lock sysmonlock.
StartTrace and StopTrace both acquire this lock after stopping the world
but before performing any work in order to ensure sysmon sees the
required state change in tracing. This change is expected to slow down
StartTrace and StopTrace, but will help ensure consistent traces are
generated.
Updates #29707.
Fixes#38794.
Change-Id: I64c58e7c3fd173cd5281ffc208d6db24ff6c0284
Reviewed-on: https://go-review.googlesource.com/c/go/+/234617
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
On darwin, we preallocate file storage space with fcntl
F_ALLOCATEALL in F_PEOFPOSMODE mode. This is specified as
allocating from the physical end of the file. So the size we give
it should be the increment, instead of the total size.
Fixes#39044.
Change-Id: I10c7ee8d51f237b4a7604233ac7abc6f91dcd602
Reviewed-on: https://go-review.googlesource.com/c/go/+/234481
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
When the concurrent back end is not enabled, it is possible to have a
scenario where: we compile a specific inlinable non-pointer-receiver
method T.M, then at some point later on in the compilation we visit a
type that triggers generation of a pointer-receiver wrapper (*T).M,
which then results in an inline of T.M into (*T).M. This introduces
subtle differences in the DWARF as compared with when the concurrent
back end is enabled (in the concurrent case, by the time we run the
SSA back end on T.M is is marked as being inlined, whereas in the
non-current case it is not marked inlined).
As a fix, at the point where we would normally compile a given
function in the xtop list right away, if the function is a method AND
is inlinable AND hasn't been inlined, then delay its compilation until
compileFunctions (so as to make sure that when we do compile it, all
possible inlining has been complete). In addition, make sure that
the abstract function symbol for the inlined function gets recorded
correctly.
Fixes#38068.
Change-Id: I57410ab5658bd4ee5b4b80750518e9b20fd6ba52
Reviewed-on: https://go-review.googlesource.com/c/go/+/234178
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
modfetch.TryProxies ranks errors returned by GOPROXY entries by
usefulness. It returns the error of the highest rank from the last
proxy. Errors from "direct" and "noproxy" are most useful, followed by
errors other than ErrNotExist, followed by ErrNotExist.
This change ranks errUseProxy with ErrNotExist even though it's
reported by "noproxy". There is almost always a more useful message
than "path does not match GOPRIVATE/GONOPROXY".
Fixes#39180
Change-Id: Ifa5b96462d7bf411e6d2d951888465c839d42471
Reviewed-on: https://go-review.googlesource.com/c/go/+/234687
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When copying a stack, we
1. allocate a new stack,
2. adjust pointers pointing to the old stack to pointing to the
new stack.
If the GC is running on another thread concurrently, on a machine
with weak memory model, the GC could observe the adjusted pointer
(e.g. through gp._defer which could be a special heap-to-stack
pointer), but not observe the publish of the new stack span. In
this case, the GC will see the adjusted pointer pointing to an
unallocated span, and throw. Fixing this by adding a publication
barrier between the allocation of the span and adjusting pointers.
One testcase for this is TestDeferHeapAndStack in long mode. It
fails reliably on linux-mips64le-mengzhuo builder without the fix,
and passes reliably after the fix.
Fixes#35541.
Change-Id: I82b09b824fdf14be7336a9ee853f56dec1b13b90
Reviewed-on: https://go-review.googlesource.com/c/go/+/234478
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
On Windows, calling syscall.Open(file, O_CREAT|O_TRUNC, 0) for a file
that already exists would change the file to be read-only.
That is not how the Unix syscall.Open behaves, so avoid it on
Windows by calling CreateFile twice if necessary.
Fixes#38225
Change-Id: I70097fca8863df427cc8a97b9376a9ffc69c6318
Reviewed-on: https://go-review.googlesource.com/c/go/+/234534
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
On Windows, some of gcc command (like msys2 native) output NUL as a file.
Fixes#36000
Change-Id: I02c632fa2d710a011d79f24d5beee4bc57ad994e
Reviewed-on: https://go-review.googlesource.com/c/go/+/233977
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This removes the GOAMD64 environment variable and its documentation.
The value is instead supplied by a compiled-in constant.
Note that function alignment is also dependent on the value of
the (removed) flag; it is 32 for aligned jumps, 16 if not.
When the flag-dependent logic is removed, it will be 32.
Updates #35881.
Change-Id: Ic41c0b9833d2e8a31fa3ce8067d92aa2f165bf72
Reviewed-on: https://go-review.googlesource.com/c/go/+/231600
Reviewed-by: Austin Clements <austin@google.com>
This will hopefully address the occasional "runtime: out of memory"
failures observed on the openbsd-arm-jsing builder:
https://build.golang.org/log/c296d866e5d99ba401b18c1a2ff3e4d480e5238c
Also make the "spin" and "winch" loops concurrent instead of
sequential to cut down the test's running time.
Finally, change Block to coordinate by closing stdin instead of
sending SIGINT. The SIGINT handler wasn't necessarily registered by
the time the signal was sent.
Updates #20400
Updates #39043
Change-Id: Ie12fc75b87e33847dc25a12edb4126db27492da6
Reviewed-on: https://go-review.googlesource.com/c/go/+/234538
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently in (*addrRanges).removeGreaterEqual we use
(addrRange).subtract with a range from specified address to "infinity"
which is supposed to be maxOffAddr. However, maxOffAddr is necessarily
an inclusive bound on the address space, because on many platforms an
exclusive bound would overflow back to 0.
On some platforms like mips and mipsle, the address space is smaller
than what's representable in a pointer, so if there's a range which hits
the top of the address space (such as in the pageAlloc tests), the limit
doesn't overflow, but maxOffAddr is inclusive, so any attempt to prune
this range with (*addrRange).removeGreaterEqual causes a failure, since
the range passed to subtract is contained within the address range which
touches the top of the address space.
Another problem with using subtract here is that addr and
maxOffAddr.addr() may not be in the same segment which could cause
makeAddrRange to panic. While this unlikely to happen, on some platforms
such as Solaris it is possible.
Fix these issues by not using subtract at all. Create a specific
implementation of (addrRange).removeGreaterEqual which side-steps all of
this by not having to worry about the top of the address space at all.
Fixes#39128.
Change-Id: Icd5b587b1a3d32a5681fb76cec4c001401f5756f
Reviewed-on: https://go-review.googlesource.com/c/go/+/234457
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In CL 231958, TempDir was changed to create a new temp directory on
each allocation, on the theory that it is easy to save in a variable
for callers that want the same directory repeatedly. Apply that
transformation here.
Updates #38850
Change-Id: Ibb014095426c33038e0a2c95303579cf95d5c3ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/234582
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We use a single parent directory for all temporary directories
created by a test so they're all kept together.
Fixes#38850
Change-Id: If8edae10c5136efcbcf6fd632487d198b9e3a868
Reviewed-on: https://go-review.googlesource.com/c/go/+/231958
Reviewed-by: Russ Cox <rsc@golang.org>
This was causing issues when fuzzing with
TestMarshalUnmarshal since the test would
occassionally set the version to VersionTLS13,
which would fail when unmarshaling. The check
doesn't add much in practice, and there is no
harm in removing it to de-flake the test.
Fixes#38902
Change-Id: I0906c570e9ed69c85fdd2c15f1b52f9e372c62e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/234486
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
v0.3.0 is a tag on 859b3ef565e2, the version that was already being
used. This change is a no-op, except for letting us use a release
version instead of a pseudo-version.
For #36905
Change-Id: I70b8ce2a3f1451f5602c469501362d7a6a673b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/234002
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
The Plan 9 runtime startup was enabling notes (like Unix signals)
before the gsignal stack was allocated. This left a small window
of time where an interrupt (eg by the parent killing a subprocess
quickly after exec) would cause a null pointer dereference in
sigtramp. This would leave the interrupted process suspended in
'broken' state instead of exiting. We've observed this on the
builders, where it can make a test time out waiting for the broken
process to terminate.
Updates #38772
Change-Id: I54584069fd3109595f06c78724c1f6419e028aab
Reviewed-on: https://go-review.googlesource.com/c/go/+/234397
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
The runtime needs to find the PC of the deferreturn call in a few
places. So for functions that have defer, we record the PC of
deferreturn call in its funcdata.
For very large binaries, the deferreturn call could be made
through a trampoline. The current code of finding deferreturn PC
fails in this case. This CL handles the trampoline as well.
Fixes#39049.
Change-Id: I929be54d6ae436f5294013793217dc2a35f080d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234105
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Found this while deleting the old code. This should be data2.
Change-Id: I1232fac22ef63bb3a3f25a0558537cc371af3bd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/234098
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This part of the test has been flaky despite repeated attempts to fix it,
and it is unclear what exactly it is testing. Remove it.
Fixes#24616.
Change-Id: If7234f99dd3d3e92f15ccb94ee13e75c6da12537
Reviewed-on: https://go-review.googlesource.com/c/go/+/233942
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Don't include SCONST symbols in the symbol table when
NotInSymbolTable is set. This is what the old code (genasmsym)
does.
In fact, SCONST symbol is only emitted by the field tracking
code, and is always NotInSymbolTable. So we should just not
include them at all, or not generate SCONST symbols at all. But
at this late stage I'll just restore the old behavior.
Change-Id: If6843003e16701d45b8c67b2297098a7babdec52
Reviewed-on: https://go-review.googlesource.com/c/go/+/233997
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently maxOffAddr is defined in terms of the whole 64-bit address
space, assuming that it's all supported, by using ^uintptr(0) as the
maximal address in the offset space. In reality, the maximal address in
the offset space is (1<<heapAddrBits)-1 because we don't have more than
that actually available to us on a given platform.
On most platforms this is fine, because arenaBaseOffset is just
connecting two segments of address space, but on AIX we use it as an
actual offset for the starting address of the available address space,
which is limited. This means using ^uintptr(0) as the maximal address in
the offset address space causes wrap-around, especially when we just
want to represent a range approximately like [addr, infinity), which
today we do by using maxOffAddr.
To fix this, we define maxOffAddr more appropriately, in terms of
(1<<heapAddrBits)-1.
This change also redefines arenaBaseOffset to not be the negation of the
virtual address corresponding to address zero in the virtual address
space, but instead directly as the virtual address corresponding to
zero. This matches the existing documentation more closely and makes the
logic around arenaBaseOffset decidedly simpler, especially when trying
to reason about its use on AIX.
Fixes#38966.
Change-Id: I1336e5036a39de846f64cc2d253e8536dee57611
Reviewed-on: https://go-review.googlesource.com/c/go/+/233497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
When tuple generators and selectors are eliminated as part of the
CSE pass we may end up with tuple selectors that are in different
blocks to the tuple generators that they correspond to. This breaks
the invariant that tuple generators and their corresponding
selectors must be in the same block. Therefore after CSE this
situation must be corrected.
Unfortunately the fixup code did not take into account that selectors
could be eliminated by CSE. It assumed that only the tuple generators
could be eliminated. In some situations this meant that it got into
a state where it was replacing references to selectors with references
to dead selectors in the wrong block.
To fix this we move the fixup code after the CSE rewrites have been
applied. This removes any difficult-to-reason-about interactions
with the CSE rewriter.
Fixes#38916.
Change-Id: I2211982dcdba399d03299f0a819945b3eb93b291
Reviewed-on: https://go-review.googlesource.com/c/go/+/233857
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
- Avoid starting subprocesses when the test is already very close to
timing out. The overhead of starting and stopping processes may
cause the test to exceed its deadline even if each individual
process is signaled soon after it is started.
- If a command does not shut down quickly enough after receiving
os.Interrupt, send it os.Kill using the same style of grace period
as in CL 228438.
- Fail the test if a background command whose exit status is not
ignored is left running at the end of the test. We have no reliable
way to distinguish a failure due to the termination signal from an
unexpected failure, and the termination signal varies across
platforms (so may cause failure on one platform but success on
another).
For #38797
Change-Id: I767898cf551dca45579bf01a9d1bb312e12d6193
Reviewed-on: https://go-review.googlesource.com/c/go/+/233526
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
While reviewing CL 228784, I noticed that various filepath.WalkFunc
implementations within cmd/go were dropping non-nil errors.
Those errors turn out to be significant, at least in some cases: for
example, they can cause packages to appear to be missing when any
parent of the directory had the wrong permissions set.
(This also turned up a bug in the existing list_dedup_packages test,
which was accidentally passing a nonexistent directory instead of the
intended duplicate path.)
Change-Id: Ia09a0a33aa7a966d9f132d3747d6c674a5370b2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/232579
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
As per discussion on the accepted proposals, enable these vet checks by
default in the go command. Update corresponding documentation as well.
Updates #32479.
Updates #4483.
Change-Id: Ie93471930c24dbb9bcbf7da5deaf63bc1a97a14f
Reviewed-on: https://go-review.googlesource.com/c/go/+/232660
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
- Don't assume that a process interrupted at 100μs intervals will have
enough remaining time to make progress. (Stop sending signals
in between signal storms to allow the process to quiesce.)
- Don't assume that a child process that spins for 1ms will block long
enough for the parent process to receive signals or make meaningful
progress. (Instead, have the child block indefinitely, and unblock
it explicitly after the signal storm.)
For #39043
Updates #22838
Updates #20400
Change-Id: I85cba23498c346a637e6cfe8684ca0c478562a93
Reviewed-on: https://go-review.googlesource.com/c/go/+/233877
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This updates vendored x/arch/ppc64 to pick up new instructions
and fixes for objdump on ppc64/ppc64le.
Change-Id: I8262e8a2af09057bbd21b39c9fcf37230029cfe8
Reviewed-on: https://go-review.googlesource.com/c/go/+/233364
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Historically we've assumed that we can install all signal handlers
with the SA_RESTART flag set, and let the system restart slow functions
if a signal is received. Therefore, we don't have to worry about EINTR.
This is only partially true, and we've added EINTR checks already for
connect, and open/read on Darwin, and sendfile on Solaris.
Other cases have turned up in #36644, #38033, and #38836.
Also, #20400 points out that when Go code is included in a C program,
the C program may install its own signal handlers without SA_RESTART.
In that case, Go code will see EINTR no matter what it does.
So, go ahead and check for EINTR. We don't check in the syscall package;
people using syscalls directly may want to check for EINTR themselves.
But we do check for EINTR in the higher level APIs in os and net,
and retry the system call if we see it.
This change looks safe, but of course we may be missing some cases
where we need to check for EINTR. As such cases turn up, we can add
tests to runtime/testdata/testprogcgo/eintr.go, and fix the code.
If there are any such cases, their handling after this change will be
no worse than it is today.
For #22838Fixes#20400Fixes#36644Fixes#38033Fixes#38836
Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896
Reviewed-on: https://go-review.googlesource.com/c/go/+/232862
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Taking over Zach's CL 212277. Just cleaned up and added a test.
For a positive, signed integer, an arithmetic right shift of count
(bit-width - 1) equals zero. e.g. int64(22) >> 63 -> 0. This CL makes
prove replace these right shifts with a zero-valued constant.
These shifts may arise in source code explicitly, but can also be
created by the generic rewrite of signed division by a power of 2.
// Signed divide by power of 2.
// n / c = n >> log(c) if n >= 0
// = (n+c-1) >> log(c) if n < 0
// We conditionally add c-1 by adding n>>63>>(64-log(c))
(first shift signed, second shift unsigned).
(Div64 <t> n (Const64 [c])) && isPowerOfTwo(c) ->
(Rsh64x64
(Add64 <t> n (Rsh64Ux64 <t>
(Rsh64x64 <t> n (Const64 <typ.UInt64> [63]))
(Const64 <typ.UInt64> [64-log2(c)])))
(Const64 <typ.UInt64> [log2(c)]))
If n is known to be positive, this rewrite includes an extra Add and 2
extra Rsh. This CL will allow prove to replace one of the extra Rsh with
a 0. That replacement then allows lateopt to remove all the unneccesary
fixups from the generic rewrite.
There is a rewrite rule to handle this case directly:
(Div64 n (Const64 [c])) && isNonNegative(n) && isPowerOfTwo(c) ->
(Rsh64Ux64 n (Const64 <typ.UInt64> [log2(c)]))
But this implementation of isNonNegative really only handles constants
and a few special operations like len/cap. The division could be
handled if the factsTable version of isNonNegative were available.
Unfortunately, the first opt pass happens before prove even has a
chance to deduce the numerator is non-negative, so the generic rewrite
has already fired and created the extra Ops discussed above.
Fixes#36159
By Printf count, this zeroes 137 right shifts when building std and cmd.
Change-Id: Iab486910ac9d7cfb86ace2835456002732b384a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/232857
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
... and 0-31 for 32-bit shifts.
Generally update the docs for ppc64 shift instructions to be
clearer about what they actually do.
This issue is causing problems for the subsequent CL. The shift
amount was <0 and caused the assembler to report an invalid instruction.
Change-Id: I8c708a15e7f71931835e6e543d8db3c716186e52
Reviewed-on: https://go-review.googlesource.com/c/go/+/232858
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Fix the mode parameter to fallocate on Linux which is the operation mode
and not the file mode as with os.OpenFile.
Also handle syscall.EINTR.
Fixes#38950
Change-Id: Ieed20d9ab5c8a49be51c9f9a42b7263f394a5261
Reviewed-on: https://go-review.googlesource.com/c/go/+/232805
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Make use of multi-control values and branch pseudo-instructions to optimise
compiler generated branches.
Change-Id: I7a8bf754db3c2082a390bf6a662ccf18cbcbee39
Reviewed-on: https://go-review.googlesource.com/c/go/+/226400
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Old url 404s because the file no longer exists on master; change it to
point to the android 10 release branch.
Change-Id: If0f8b645f2c746f9fc8bbd68f4d1fe41868493ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/232809
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Only enable broadcast on SOCK_DGRAM and SOCK_RAW sockets, SOCK_STREAM
and others don't support it.
Don't enable SO_BROADCAST on UNIX domain sockets as they don't support it.
This caused failures on WSL which strictly checks setsockopt calls
unlike other OSes which often silently ignore bad options.
Also return error for setsockopt call for SO_BROADCAST on Windows
matching all other platforms but for IPv4 only as it's not supported
on IPv6 as per:
https://docs.microsoft.com/en-us/windows/win32/winsock/socket-optionsFixes#38954
Change-Id: I0503fd1ce96102b17121af548b66b3e9c2bb80d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/232807
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Before this commit, the code declares and assigns "n" with the result of
io.ReadFull() -- but the value is not used. The variable is then reused
later in the function.
This commit removes the first declaration of "n" and declares it closer
to where it is used.
Change-Id: I7ffe19a10f2a563c306bb6fe6562493435b9dc5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/232917
Reviewed-by: Rob Pike <r@golang.org>
testing.M.Run has this bit of code:
if !flag.Parsed() {
flag.Parse()
}
It makes sense, and it's common knowledge for many Go developers that
test flags are automatically parsed by the time tests and benchmarks are
run. However, the docs didn't clarify that. The previous wording only
mentioned that flag.Parse isn't run before TestMain, which doesn't
necessarily mean that it's run afterwards.
Fixes#38952.
Change-Id: I85f7a9dce637a23c5cb9abc485d47415c1a1ca27
Reviewed-on: https://go-review.googlesource.com/c/go/+/232806
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When we decode into a struct, each input key-value may be decoded into
one of the struct's fields. Particularly, existing data isn't dropped,
so that some sub-fields can be decoded into without zeroing all other
data.
However, decoding into a map behaved in the opposite way. Whenever a
key-value was decoded, it completely replaced the previous map element.
If the map contained any non-zero data in that key, it's dropped.
Instead, try to reuse the existing element value if possible. If the map
element type is a pointer, and the value is non-nil, we can decode
directly into it. If it's not a pointer, make a copy and decode into
that copy, as map element values aren't addressable.
This means we have to parse and convert the map element key before the
value, to be able to obtain the existing element value. This is fine,
though. Moreover, reporting errors on the key before the value follows
the input order more closely.
Finally, add a test to explore the four combinations, involving pointer
and non-pointer, and non-zero and zero values. A table-driven test
wasn't used, as each case required different checks, such as checking
that the non-nil pointer case doesn't end up with a different pointer.
Fixes#31924.
Change-Id: I5ca40c9963a98aaf92f26f0b35843c021028dfca
Reviewed-on: https://go-review.googlesource.com/c/go/+/179337
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The BITCON test, isbitcon, assumes 32-bit constants are expanded
repeatedly, i.e. by copying the low 32 bits to high 32 bits,
instead of zero extending. We already do such expansion in
progedit. In con32class when classifying 32-bit constants, we
should use the expanded constant, instead of zero-extending it.
TODO: we could have better encoding for things like ANDW $-1, Rx.
Fixes#38946.
Change-Id: I37d0c95d744834419db5c897fd1f6c187595c926
Reviewed-on: https://go-review.googlesource.com/c/go/+/232984
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Improve the error user experience when users try to set/refer
to unexported fields and methods of struct literals, by directly saying
"cannot refer to unexported field or method"
Fixes#31053
Change-Id: I6fd3caf64b7ca9f9d8ea60b7756875e340792d59
Reviewed-on: https://go-review.googlesource.com/c/go/+/201657
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The recently added function parseFloatPrefix tested the entire
string for correct placement of separators rather than just the
consumed part. The 4-char fix is in readFloat (atof.go:303).
Added more tests. Also added some white space for nicer
grouping of the test cases.
While at it, removed the need for calling testing.Run.
Fixes#38962.
Change-Id: Ifce84f362bb4ede559103f8d535556d3de9325f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/233017
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Omits printing the file:line:column when trying to
open non-existent files
Given:
go tool compile x.go
* Before:
x.go:0: open x.go: no such file or directory
* After:
open x.go: no such file or directory
Reverts the revert in CL 231043 by only fixing the case
of non-existent errors which is what the original bug
was about. The fix for "permission errors" will come later
on when I have bandwidth to investigate the differences
between running with root and why os.Open works for some
builders and not others.
Fixes#36437
Change-Id: I9c8a0981ad708b504bb43990a4105b42266fa41f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230941
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Fix TestFreeBSDNumCPU on newer versions of FreeBSD which have multi line
output from cpuset e.g.
cpuset -g -p 4141
pid 4141 mask: 0, 1, 2, 3, 4, 5, 6, 7, 8
pid 4141 domain policy: first-touch mask: 0, 1
The test now uses just the first line of output.
Fixes#38937Fixes#25924
Change-Id: If082ee6b82120ebde4dc437e58343b3dad69c65f
Reviewed-on: https://go-review.googlesource.com/c/go/+/232801
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The added comment contains some context. The original optimization
assumed that each call to unquoteBytes (or unquote) followed its
corresponding call to rescanLiteral. Otherwise, unquoting a literal
might use d.safeUnquote from another re-scanned literal.
Unfortunately, this assumption is wrong. When decoding {"foo": "bar"}
into a map[T]string where T implements TextUnmarshaler, the sequence of
calls would be as follows:
1) rescanLiteral "foo"
2) unquoteBytes "foo"
3) rescanLiteral "bar"
4) unquoteBytes "foo" (for UnmarshalText)
5) unquoteBytes "bar"
Note that the call to UnmarshalText happens in literalStore, which
repeats the work to unquote the input string literal. But, since that
happens after we've re-scanned "bar", we're using the wrong safeUnquote
field value.
In the added test case, the second string had a non-zero number of safe
bytes, and the first string had none since it was all non-ASCII. Thus,
"safely" unquoting a number of the first string's bytes could cut a rune
in half, and thus mangle the runes.
A rather simple fix, without a full revert, is to only allow one use of
safeUnquote per call to unquoteBytes. Each call to rescanLiteral when
we have a string is soon followed by a call to unquoteBytes, so it's no
longer possible for us to use the wrong index.
Also add a test case from #38126, which is the same underlying bug, but
affecting the ",string" option.
Before the fix, the test would fail, just like in the original two issues:
--- FAIL: TestUnmarshalRescanLiteralMangledUnquote (0.00s)
decode_test.go:2443: Key "开源" does not exist in map: map[开���:12345开源]
decode_test.go:2458: Unmarshal unexpected error: json: invalid use of ,string struct tag, trying to unmarshal "\"aaa\tbbb\"" into string
Fixes#38105.
For #38126.
Change-Id: I761e54924e9a971a4f9eaa70bbf72014bb1476e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/226218
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This change uses the new offAddr type in more parts of the runtime where
we've been implicitly switching from the default address space to a
contiguous view. The purpose of offAddr is to represent addresses in the
contiguous view of the address space, and to make direct computations
between real addresses and offset addresses impossible. This change thus
improves readability in the runtime.
Updates #35788.
Change-Id: I4e1c5fed3ed68aa12f49a42b82eb3f46aba82fc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230718
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently addrRange and addrRanges operate on real addresses. That is,
the addresses they manipulate don't include arenaBaseOffset. When added
to an address, arenaBaseOffset makes the address space appear contiguous
on platforms where the address space is segmented. While this is
generally OK because even those platforms which have a segmented address
space usually don't give addresses in a different segment, today it
causes a mismatch between the scavenger and the rest of the page
allocator. The scavenger scavenges from the highest addresses first, but
only via real address, whereas the page allocator allocates memory in
offset address order.
So this change makes addrRange and addrRanges, i.e. what the scavenger
operates on, use offset addresses. However, lots of the page allocator
relies on an addrRange containing real addresses.
To make this transition less error-prone, this change introduces a new
type, offAddr, whose purpose is to make offset addresses a distinct
type, so any attempt to trivially mix real and offset addresses will
trigger a compilation error.
This change doesn't attempt to use offAddr in all of the runtime; a
follow-up change will look for and catch remaining uses of an offset
address which doesn't use the type.
Updates #35788.
Change-Id: I991d891ac8ace8339ca180daafdf6b261a4d43d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230717
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently the scavenger will reset to the top of the heap every GC. This
means if it scavenges a bunch of memory which doesn't get used again,
it's going to keep re-scanning that memory on subsequent cycles. This
problem is especially bad when it comes to heap spikes: suppose an
application's heap spikes to 2x its steady-state size. The scavenger
will run over the top half of that heap even if the heap shrinks, for
the rest of the application's lifetime.
To fix this, we maintain two numbers: a "free" high watermark, which
represents the highest address freed to the page allocator in that
cycle, and a "scavenged" low watermark, which represents how low of an
address the scavenger got to when scavenging. If the "free" watermark
exceeds the "scavenged" watermark, then we pick the "free" watermark as
the new "top of the heap" for the scavenger when starting the next
scavenger cycle. Otherwise, we have the scavenger pick up where it left
off.
With this mechanism, we only ever re-scan scavenged memory if a random
page gets freed very high up in the heap address space while most of the
action is happening in the lower parts. This case should be exceedingly
unlikely because the page reclaimer walks over the heap from low address
to high addresses, and we use a first-fit address-ordered allocation
policy.
Updates #35788.
Change-Id: Id335603b526ce3a0eb79ef286d1a4e876abc9cab
Reviewed-on: https://go-review.googlesource.com/c/go/+/218997
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
This change removes the concept of s.scavAddr in favor of explicitly
reserving and unreserving address ranges. s.scavAddr has several
problems with raciness that can cause the scavenger to miss updates, or
move it back unnecessarily, forcing future scavenge calls to iterate
over searched address space unnecessarily.
This change achieves this by replacing scavAddr with a second addrRanges
which is cloned from s.inUse at the end of each sweep phase. Ranges from
this second addrRanges are then reserved by scavengers (with the
reservation size proportional to the heap size) who are then able to
safely iterate over those ranges without worry of another scavenger
coming in.
Fixes#35788.
Change-Id: Ief01ae170384174875118742f6c26b2a41cbb66d
Reviewed-on: https://go-review.googlesource.com/c/go/+/208378
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Austin Clements <austin@google.com>
golang.org/cl/193604 fixed one bug when one encodes a string with the
",string" option: if SetEscapeHTML(false) is used, we should not be
using HTML escaping for the inner string encoding. The CL correctly
fixed that.
The CL also tried to speed up this edge case. By avoiding an entire new
call to Marshal, the new Issue34127 benchmark reduced its time/op by
45%, and lowered the allocs/op from 3 to 2.
However, that last optimization wasn't correct:
Since Go 1.2 every string can be marshaled to JSON without error
even if it contains invalid UTF-8 byte sequences. Therefore
there is no need to use Marshal again for the only reason of
enclosing the string in double quotes.
JSON string encoding isn't just about adding quotes and taking care of
invalid UTF-8. We also need to escape some characters, like tabs and
newlines.
The new code failed to do that. The bug resulted in the added test case
failing to roundtrip properly; before our fix here, we'd see an error:
invalid use of ,string struct tag, trying to unmarshal "\"\b\f\n\r\t\"\\\"" into string
If you pay close attention, you'll notice that the special characters
like tab and newline are only encoded once, not twice. When decoding
with the ",string" option, the outer string decode works, but the inner
string decode fails, as we are now decoding a JSON string with unescaped
special characters.
The fix we apply here isn't to go back to Marshal, as that would
re-introduce the bug with SetEscapeHTML(false). Instead, we can use a
new encode state from the pool - it results in minimal performance
impact, and even reduces allocs/op further. The performance impact seems
fair, given that we need to check the entire string for characters that
need to be escaped.
name old time/op new time/op delta
Issue34127-8 89.7ns ± 2% 100.8ns ± 1% +12.27% (p=0.000 n=8+8)
name old alloc/op new alloc/op delta
Issue34127-8 40.0B ± 0% 32.0B ± 0% -20.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
Issue34127-8 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
Instead of adding another standalone test, we convert an existing
"string tag" test to be table-based, and add another test case there.
One test case from the original CL also had to be amended, due to the
same problem - when escaping '<' due to SetEscapeHTML(true), we need to
end up with double escaping, since we're using ",string".
Fixes#38173.
Change-Id: I2b0df9e4f1d3452fff74fe910e189c930dde4b5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/226498
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Since the ConnectionState will now be available during
verification, some code was moved around in order to
initialize and make available as much of the fields on
Conn as possible before the ConnectionState is verified.
Fixes#36736
Change-Id: I0e3efa97565ead7de5c48bb8a87e3ea54fbde140
Reviewed-on: https://go-review.googlesource.com/c/go/+/229122
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Automatically rotate session ticket keys for servers
that don't already have sessionTicketKeys and that
haven't called SetSessionTicketKeys.
Now, session ticket keys will be rotated every 24 hours
with a lifetime of 7 days. This adds a small performance
cost to existing clients that don't provide a session
ticket encrypted with a fresh enough session ticket key,
which would require a full handshake.
Updates #25256
Change-Id: I15b46af7a82aab9a108bceb706bbf66243a1510f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230679
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Per X690 Section 11.6 sort the order of SET of components when generating
DER. This CL makes no changes to Unmarshal, meaning unordered components
will still be accepted, and won't be re-ordered during parsing.
In order to sort the components a new encoder, setEncoder, which is similar
to multiEncoder is added. The functional difference is that setEncoder
encodes each component to a [][]byte, sorts the slice using a sort.Sort
interface, and then writes it out to the destination slice. The ordering
matches the output of OpenSSL.
Fixes#24254
Change-Id: Iff4560f0b8c2dce5aae616ba30226f39c10b972e
GitHub-Last-Rev: e52fc43658
GitHub-Pull-Request: golang/go#38228
Reviewed-on: https://go-review.googlesource.com/c/go/+/226984
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Colons are port separators, so it's risky to allow them in hostnames.
Per the CL 231377 rule, if we at least consider them invalid we will not
apply wildcard processing to them, making behavior a little more
predictable.
We were considering hostnames with colons valid (against spec) because
that meant we'd not ignore them in Common Name. (There was at least
one deployment that was putting colons in Common Name and expecting it
to verify.)
Now that Common Name is ignored by default, those clients will break
again, so it's a good time to drop the exception. Hopefully they moved
to SANs, where invalid hostnames are checked 1:1 (ignoring wildcards)
but still work. (If they didn't, this change means they can't use
GODEBUG=x509ignoreCN=0 to opt back in, but again you don't get to use a
legacy deprecated field AND invalid hostnames.)
Updates #24151
Change-Id: Id44b4fecb2d620480acdfc65fea1473f7abbca7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231381
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trailing dots are not allowed in certificate fields like CN and SANs
(while they are allowed and ignored as inputs to verification APIs).
Move to considering names with trailing dots in certificates as invalid
hostnames.
Following the rule of CL 231378, these invalid names lose wildcard
processing, but can still match if there is a 1:1 match, trailing dot
included, with the VerifyHostname input.
They also become ignored Common Name values regardless of the
GODEBUG=x509ignoreCN=X value, because we have to ignore invalid
hostnames in Common Name for #24151. The error message automatically
accounts for this, and doesn't suggest the environment variable. You
don't get to use a legacy deprecated field AND invalid hostnames.
(While at it, also consider wildcards in VerifyHostname inputs as
invalid hostnames, not that it should change any observed behavior.)
Change-Id: Iecdee8927df50c1d9daf904776b051de9f5e76ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/231380
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Common Name has been deprecated for 20 years, and has horrible
interactions with Name Constraints. The browsers managed to drop it last
year, let's try flicking the switch to disabled by default.
Return helpful errors for things that would get unbroken by flipping the
switch back with the environment variable.
Had to refresh a test certificate that was too old to have SANs.
Updates #24151
Change-Id: I2ab78577fd936ba67969d3417284dbe46e4ae02f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231379
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When the input or SAN dNSNames are not valid hostnames, the specs don't
define what should happen, because this should ideally never happen, so
everything we do is undefined behavior. Browsers get to just return an
error, because browsers can assume that the resolving layer is DNS. We
can't, names can be resolved by anything implementing a Dial function,
and the crypto/x509 APIs can also be used directly without actual
networks in sight.
Trying to process invalid hostnames leads to issues like #27591 where
wildcards glob stuff they aren't expected to, because wildcards are only
defined on hostnames.
Try to rationalize the behavior like this: if both the VerifyHostname
input and the SAN dNSNames are a valid hostname, follow the specs;
otherwise, only accept perfect 1:1 case-insensitive matches (without
wildcards or trailing dot processing).
This should allow us to keep supporting weird names, with less
unexpected side-effects from undefined behavior. Also, it's a rule, even
if completely made up, so something we can reason about and code against.
The commonName field does allow any string, but no specs define how to
process it. Processing it differently from dNSNames would be confusing,
and allowing it to match invalid hostnames is incompatible with Name
Constraint processing (#24151).
This does encourage invalid dNSNames, regrettably, but we need some way
for the standard API to match weird names, and the alternative of
keeping CN alive sounds less appealing.
Fixes#27591
Change-Id: Id2d515f068a17ff796a32b30733abe44ad4f0339
Reviewed-on: https://go-review.googlesource.com/c/go/+/231378
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
If dst slice length is zero in makeslicecopy then the called mallocgc is
using a fast path to only return a pointer to runtime.zerobase.
There may be no heapBits for that address readable by
bulkBarrierPreWriteSrcOnly which will cause a panic.
Protect against this by not calling bulkBarrierPreWriteSrcOnly if
there is nothing to copy. This is the case for all cases where the
length of the destination slice is zero.
runtime.growslice and runtime.typedslicecopy have fast paths that
do not call bulkBarrierPreWrite for zero copy lengths either.
Fixes#38929
Change-Id: I78ece600203a0a8d24de5b6c9eef56f605d44e99
Reviewed-on: https://go-review.googlesource.com/c/go/+/232800
Reviewed-by: Keith Randall <khr@golang.org>
Missed in CL 221790
This is the only remaining use of math.Float64frombits in the .rules
file that isn't already guarded.
Fixes#38880
Change-Id: I11f71e3a48516748d8d2701c6cf6920a7bc9e216
Reviewed-on: https://go-review.googlesource.com/c/go/+/232859
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Currently linearAlloc manages an exclusive "end" address for the top of
its reserved space. While unlikely for a linearAlloc to be allocated
with an "end" address hitting the top of the address space, it is
possible and could lead to overflow.
Avoid overflow by chopping off the last byte from the linearAlloc if
it's bumping up against the top of the address space defensively. In
practice, this means that if 32-bit platforms map the top of the address
space and use the linearAlloc to acquire arenas, the top arena will not
be usable.
Fixes#35954.
Change-Id: I512cddcd34fd1ab15cb6ca92bbf899fc1ef22ff6
Reviewed-on: https://go-review.googlesource.com/c/go/+/231338
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently when checking if we can grow the heap into the current arena,
we do an addition which may overflow. This is particularly likely on
32-bit systems.
Avoid this situation by explicitly checking for overflow, and adding in
some comments about when overflow is possible, when it isn't, and why.
For #35954.
Change-Id: I2d4ecbb1ccbd43da55979cc721f0cd8d1757add2
Reviewed-on: https://go-review.googlesource.com/c/go/+/231337
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I added routines that can acquire/release a particular rank without
acquiring/releasing an associated lock. I added lockRankGscan as a rank
for acquiring/releasing the Gscan bit.
castogscanstatus() and casGtoPreemptScan() are acquires of the Gscan
bit. casfrom_Gscanstatus() is a release of the Gscan bit. casgstatus()
is like an acquire and release of the Gscan bit, since it will wait if
Gscan bit is currently set.
We have a cycle between hchan and Gscan. The acquisition of Gscan and
then hchan only happens in syncadjustsudogs() when the G is suspended,
so the main normal ordering (get hchan, then get Gscan) can't be
happening. So, I added a new rank lockRankHchanLeaf that is used when
acquiring hchan locks in syncadjustsudogs. This ranking is set so no
other locks can be acquired except other hchan locks.
Fixes#38922
Change-Id: I58ce526a74ba856cb42078f7b9901f2832e1d45c
Reviewed-on: https://go-review.googlesource.com/c/go/+/228417
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Fix a minor bug where it should use Textp2 instead of Textp. This
doesn't affect correctness. It just made the pre-allocation less
effective.
Change-Id: Ib3fa8ab3c64037e3582933970d051f278286353b
Reviewed-on: https://go-review.googlesource.com/c/go/+/232837
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
+----------------------------------------------------------------------+
| Hello, if you are reading this and run macOS, please test this code: |
| |
| $ GO111MODULE=on go get golang.org/dl/gotip@latest |
| $ gotip download |
| $ GODEBUG=x509roots=1 gotip test crypto/x509 -v -run TestSystemRoots |
+----------------------------------------------------------------------+
We currently have two code paths to extract system roots on macOS: one
uses cgo to invoke a maze of Security.framework APIs; the other is a
horrible fallback that runs "/usr/bin/security verify-cert" on every
root that has custom policies to check if it's trusted for SSL.
The fallback is not only terrifying because it shells out to a binary,
but also because it lets in certificates that are not trusted roots but
are signed by trusted roots, and because it applies some filters (EKUs
and expiration) only to roots with custom policies, as the others are
not passed to verify-cert. The other code path, of course, requires cgo,
so can't be used when cross-compiling and involves a large ball of C.
It's all a mess, and it broke oh-so-many times (#14514, #16532, #19436,
#20990, #21416, #24437, #24652, #25649, #26073, #27958, #28025, #28092,
#29497, #30471, #30672, #30763, #30889, #32891, #38215, #38365, ...).
Since macOS does not have a stable syscall ABI, we already dynamically
link and invoke libSystem.dylib regardless of cgo availability (#17490).
How that works is that functions in package syscall (like syscall.Open)
take the address of assembly trampolines (like libc_open_trampoline)
that jump to symbols imported with cgo_import_dynamic (like libc_open),
and pass them along with arguments to syscall.syscall (which is
implemented as runtime.syscall_syscall). syscall_syscall informs the
scheduler and profiler, and then uses asmcgocall to switch to a system
stack and invoke runtime.syscall. The latter is an assembly trampoline
that unpacks the Go ABI arguments passed to syscall.syscall, finally
calls the remote function, and puts the return value on the Go stack.
(This last bit is the part that cgo compiles from a C wrapper.)
We can do something similar to link and invoke Security.framework!
The one difference is that runtime.syscall and friends check errors
based on the errno convention, which Security doesn't follow, so I added
runtime.syscallNoErr which just skips interpreting the return value.
We only need a variant with six arguments because the calling convention
is register-based, and extra arguments simply zero out some registers.
That's plumbed through as crypto/x509/internal/macOS.syscall. The rest
of that package is a set of wrappers for Security.framework and Core
Foundation functions, like syscall is for libSystem. In theory, as long
as macOS respects ABI backwards compatibility (a.k.a. as long as
binaries built for a previous OS version keep running) this should be
stable, as the final result is not different from what a C compiler
would make. (One exception might be dictionary key strings, which we
make our own copy of instead of using the dynamic symbol. If they change
the value of those strings things might break. But why would they.)
Finally, I rewrote the crypto/x509 cgo logic in Go using those wrappers.
It works! I tried to make it match 1:1 the old logic, so that
root_darwin_amd64.go can be reviewed by comparing it to
root_cgo_darwin_amd64.go. The only difference is that we do proper error
handling now, and assume that if there is no error the return values are
there, while before we'd just check for nil pointers and move on.
I kept the cgo logic to help with review and testing, but we should
delete it once we are confident the new code works.
The nocgo logic is gone and we shall never speak of it again.
Fixes#32604Fixes#19561Fixes#38365
Awakens Cthulhu
Change-Id: Id850962bad667f71e3af594bdfebbbb1edfbcbb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/227037
Reviewed-by: Katie Hockman <katie@golang.org>
Also encode the certificates in a way that's more
consistent with TLS 1.3 (with a 24 byte length prefix).
Note that this will have an additional performance cost
requiring clients to do a full handshake every 7 days
where previously they were able to use the same ticket
indefinitely.
Updates #25256
Change-Id: Ic4d1ba0d92773c490b33b5f6c1320d557cc7347d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231317
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
We might as well grow the stack at least as large as we'll need for
the frame that is calling morestack. It doesn't help with the
lots-of-small-frames case, but it may help a bit with the
few-big-frames case.
Update #18138
Change-Id: I1f49c97706a70e20b30433cbec99a7901528ea52
Reviewed-on: https://go-review.googlesource.com/c/go/+/225800
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
match:
m = make([]T, x); copy(m, s)
for pointer free T and x==len(s) rewrite to:
m = mallocgc(x*elemsize(T), nil, false); memmove(&m, &s, x*elemsize(T))
otherwise rewrite to:
m = makeslicecopy([]T, x, s)
This avoids memclear and shading of pointers in the newly created slice
before the copy.
With this CL "s" is only be allowed to bev a variable and not a more
complex expression. This restriction could be lifted in future versions
of this optimization when it can be proven that "s" is not referencing "m".
Triggers 450 times during make.bash..
Reduces go binary size by ~8 kbyte.
name old time/op new time/op delta
MakeSliceCopy/mallocmove/Byte 71.1ns ± 1% 65.8ns ± 0% -7.49% (p=0.000 n=10+9)
MakeSliceCopy/mallocmove/Int 71.2ns ± 1% 66.0ns ± 0% -7.27% (p=0.000 n=10+8)
MakeSliceCopy/mallocmove/Ptr 104ns ± 4% 99ns ± 1% -5.13% (p=0.000 n=10+10)
MakeSliceCopy/makecopy/Byte 70.3ns ± 0% 68.0ns ± 0% -3.22% (p=0.000 n=10+9)
MakeSliceCopy/makecopy/Int 70.3ns ± 0% 68.5ns ± 1% -2.59% (p=0.000 n=9+10)
MakeSliceCopy/makecopy/Ptr 102ns ± 0% 99ns ± 1% -2.97% (p=0.000 n=9+9)
MakeSliceCopy/nilappend/Byte 75.4ns ± 0% 74.9ns ± 2% -0.63% (p=0.015 n=9+9)
MakeSliceCopy/nilappend/Int 75.6ns ± 0% 76.4ns ± 3% ~ (p=0.245 n=9+10)
MakeSliceCopy/nilappend/Ptr 107ns ± 0% 108ns ± 1% +0.93% (p=0.005 n=9+10)
Fixes#26252
Change-Id: Iec553dd1fef6ded16197216a472351c8799a8e71
Reviewed-on: https://go-review.googlesource.com/c/go/+/146719
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Speed up repeated HMAC operations with the same key by not recomputing
the first block of the inner and outer hashes in Reset and Sum, saving
two block computations each time.
This is a significant win for applications which hash many small
messages with the same key. In x/crypto/pbkdf2 for example, this
optimization cuts the number of block computations in half, speeding it
up by 25%-40% depending on the hash function.
The hash function needs to implement binary.Marshaler and
binary.Unmarshaler for this optimization to work, so that we can save
and restore its internal state. All hash functions in the standard
library are marshalable (CL 66710) but if the hash isn't marshalable, we
fall back on the old behaviour.
Marshaling the hashes does add a couple unavoidable new allocations, but
this only has to be done once, so the cost is amortized over repeated
uses. To minimize impact to applications which don't (or can't) reuse
hmac objects, marshaling is performed in Reset (rather than in New),
since calling Reset seems like a good indication that the caller intends
to reuse the hmac object later.
I had to add a boolean field to the hmac state to remember if we've
marshaled the hashes or not. This is paid for by removing the size and
blocksize fields, which were basically unused except for some
initialization work in New, and to fulfill the Size and Blocksize
methods. Size and Blocksize can just be forwarded to the underlying
hash, so there doesn't really seem to be any reason to waste space
caching their values.
crypto/hmac benchmarks:
name old time/op new time/op delta
HMAC_Reset/SHA1/1K-2 4.06µs ± 0% 3.77µs ± 0% -7.29% (p=0.000 n=8+10)
HMAC_Reset/SHA1/32-2 1.08µs ± 0% 0.78µs ± 1% -27.67% (p=0.000 n=10+10)
HMAC_Reset/SHA256/1K-2 10.3µs ± 0% 9.4µs ± 0% -9.03% (p=0.000 n=10+10)
HMAC_Reset/SHA256/32-2 2.32µs ± 0% 1.42µs ± 0% -38.87% (p=0.000 n=10+10)
HMAC_Reset/SHA512/1K-2 8.22µs ± 0% 7.04µs ± 0% -14.32% (p=0.000 n=9+9)
HMAC_Reset/SHA512/32-2 3.08µs ± 0% 1.89µs ± 0% -38.54% (p=0.000 n=10+9)
HMAC_New/SHA1/1K-2 4.86µs ± 1% 4.93µs ± 1% +1.30% (p=0.000 n=10+9)
HMAC_New/SHA1/32-2 1.91µs ± 1% 1.95µs ± 1% +1.84% (p=0.000 n=10+9)
HMAC_New/SHA256/1K-2 11.2µs ± 1% 11.2µs ± 0% ~ (p=1.000 n=9+10)
HMAC_New/SHA256/32-2 3.22µs ± 2% 3.19µs ± 2% -1.07% (p=0.018 n=9+10)
HMAC_New/SHA512/1K-2 9.54µs ± 0% 9.66µs ± 1% +1.31% (p=0.000 n=9+10)
HMAC_New/SHA512/32-2 4.37µs ± 1% 4.46µs ± 1% +1.97% (p=0.000 n=10+9)
name old speed new speed delta
HMAC_Reset/SHA1/1K-2 252MB/s ± 0% 272MB/s ± 0% +7.86% (p=0.000 n=8+10)
HMAC_Reset/SHA1/32-2 29.7MB/s ± 0% 41.1MB/s ± 1% +38.26% (p=0.000 n=10+10)
HMAC_Reset/SHA256/1K-2 99.1MB/s ± 0% 108.9MB/s ± 0% +9.93% (p=0.000 n=10+10)
HMAC_Reset/SHA256/32-2 13.8MB/s ± 0% 22.6MB/s ± 0% +63.57% (p=0.000 n=10+10)
HMAC_Reset/SHA512/1K-2 125MB/s ± 0% 145MB/s ± 0% +16.71% (p=0.000 n=9+9)
HMAC_Reset/SHA512/32-2 10.4MB/s ± 0% 16.9MB/s ± 0% +62.69% (p=0.000 n=10+9)
HMAC_New/SHA1/1K-2 211MB/s ± 1% 208MB/s ± 1% -1.29% (p=0.000 n=10+9)
HMAC_New/SHA1/32-2 16.7MB/s ± 1% 16.4MB/s ± 1% -1.81% (p=0.000 n=10+9)
HMAC_New/SHA256/1K-2 91.3MB/s ± 1% 91.5MB/s ± 0% ~ (p=0.950 n=9+10)
HMAC_New/SHA256/32-2 9.94MB/s ± 2% 10.04MB/s ± 2% +1.09% (p=0.021 n=9+10)
HMAC_New/SHA512/1K-2 107MB/s ± 0% 106MB/s ± 1% -1.29% (p=0.000 n=9+10)
HMAC_New/SHA512/32-2 7.32MB/s ± 1% 7.18MB/s ± 1% -1.89% (p=0.000 n=10+9)
name old alloc/op new alloc/op delta
HMAC_Reset/SHA1/1K-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA1/32-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/1K-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/32-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/1K-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/32-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_New/SHA1/1K-2 448B ± 0% 448B ± 0% ~ (all samples are equal)
HMAC_New/SHA1/32-2 448B ± 0% 448B ± 0% ~ (all samples are equal)
HMAC_New/SHA256/1K-2 480B ± 0% 480B ± 0% ~ (all samples are equal)
HMAC_New/SHA256/32-2 480B ± 0% 480B ± 0% ~ (all samples are equal)
HMAC_New/SHA512/1K-2 800B ± 0% 800B ± 0% ~ (all samples are equal)
HMAC_New/SHA512/32-2 800B ± 0% 800B ± 0% ~ (all samples are equal)
name old allocs/op new allocs/op delta
HMAC_Reset/SHA1/1K-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA1/32-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/1K-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/32-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/1K-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/32-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_New/SHA1/1K-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA1/32-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA256/1K-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA256/32-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA512/1K-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA512/32-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
x/crypto/pbkdf2 benchmarks:
name old time/op new time/op delta
HMACSHA1-2 4.63ms ± 0% 3.40ms ± 0% -26.58% (p=0.000 n=10+9)
HMACSHA256-2 9.75ms ± 0% 5.98ms ± 0% -38.62% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
HMACSHA1-2 516B ± 0% 708B ± 0% +37.21% (p=0.000 n=10+10)
HMACSHA256-2 549B ± 0% 772B ± 0% +40.62% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
HMACSHA1-2 8.00 ± 0% 10.00 ± 0% +25.00% (p=0.000 n=10+10)
HMACSHA256-2 8.00 ± 0% 10.00 ± 0% +25.00% (p=0.000 n=10+10)
Fixes#19941
Change-Id: I7077a6f875be68d3da05f7b3664e18514861886f
Reviewed-on: https://go-review.googlesource.com/c/go/+/27458
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
The code writes text sections twice, one with Codeblk, one with
Datblk. The second write shouldn't be there.
May fix#38898.
Change-Id: I4ec70294059ec9aa0fc4cc69a3cd824f5843287b
Reviewed-on: https://go-review.googlesource.com/c/go/+/232661
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reject base 128 encoded integers that aren't using minimal encoding,
specifically if the leading octet of an encoded integer is 0x80. This
only affects parsing of tags and OIDs, both of which expect this
encoding (see X.690 8.1.2.4.2 and 8.19.2).
Fixes#36881
Change-Id: I969cf48ac1fba7e56bac334672806a0784d3e123
GitHub-Last-Rev: fefc03d202
GitHub-Pull-Request: golang/go#38281
Reviewed-on: https://go-review.googlesource.com/c/go/+/227320
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The previous behavior directly contradicted the docs that have been in
place for years:
To unmarshal a JSON array into a slice, Unmarshal resets the
slice length to zero and then appends each element to the slice.
We could use reflect.New to create a new element and reflect.Append to
then append it to the destination slice, but benchmarks have shown that
reflect.Append is very slow compared to the code that manually grows a
slice in this file.
Instead, if we're decoding into an element that came from the original
backing array, zero it before decoding into it. We're going to be using
the CodeDecoder benchmark, as it has a slice of struct pointers that's
decoded very often.
Note that we still reuse existing values from arrays being decoded into,
as the documentation agrees with the existing implementation in that
case:
To unmarshal a JSON array into a Go array, Unmarshal decodes
JSON array elements into corresponding Go array elements.
The numbers with the benchmark as-is might seem catastrophic, but that's
only because the benchmark is decoding into the same variable over and
over again. Since the old decoder was happy to reuse slice elements, it
would save a lot of allocations by not having to zero and re-allocate
said elements:
name old time/op new time/op delta
CodeDecoder-8 10.4ms ± 1% 10.9ms ± 1% +4.41% (p=0.000 n=10+10)
name old speed new speed delta
CodeDecoder-8 186MB/s ± 1% 178MB/s ± 1% -4.23% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.19MB ± 0% 3.59MB ± 0% +64.09% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 76.8k ± 0% 92.7k ± 0% +20.71% (p=0.000 n=10+10)
We can prove this by moving 'var r codeResponse' into the loop, so that
the benchmark no longer reuses the destination pointer. And sure enough,
we no longer see the slow-down caused by the extra allocations:
name old time/op new time/op delta
CodeDecoder-8 10.9ms ± 0% 10.9ms ± 1% -0.37% (p=0.043 n=10+10)
name old speed new speed delta
CodeDecoder-8 177MB/s ± 0% 178MB/s ± 1% +0.37% (p=0.041 n=10+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 3.59MB ± 0% 3.59MB ± 0% ~ (p=0.780 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 92.7k ± 0% 92.7k ± 0% ~ (all equal)
I believe that it's useful to leave the benchmarks as they are now,
because the decoder does reuse memory in some cases. For example,
existing map elements are reused. However, subtle changes like this one
need to be benchmarked carefully.
Finally, add a couple of tests involving both a slice and an array of
structs.
Fixes#21092.
Change-Id: I8b1194f25e723a31abd146fbfe9428ac10c1389d
Reviewed-on: https://go-review.googlesource.com/c/go/+/191783
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The `yield := osyield` line doesn't serve any purpose, it's committed in `2015`, time to delete that line:)
Change-Id: I382d4d32cf320f054f011f3b6684c868cbcb0ff2
GitHub-Last-Rev: 7a0aa25e55
GitHub-Pull-Request: golang/go#36078
Reviewed-on: https://go-review.googlesource.com/c/go/+/210837
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit moves the isSelect bool below the ticket uint32. The
boolean was consuming 8 bytes of the struct. The uint32 was also
consuming 8 bytes, so we can pack isSelect below the uint32 and save 8
bytes. This reduces the sudog struct from 96 bytes to 88 bytes.
Change-Id: If555cdaf2f5eaa125e2590fc4d113dbc99750738
GitHub-Last-Rev: d63b4e086b
GitHub-Pull-Request: golang/go#36552
Reviewed-on: https://go-review.googlesource.com/c/go/+/214677
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Change-Id: I493bb7e5e9a9e1752236dea1e032b317da7f67f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/211560
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL sets positions for errors from cals to load within the load
call itself, similar to how the rest of the code in pkg.go sets
positions right after the error is set on the package.
This allows the code to ensure that we only add positions either for
ImportPathErrors, or if an error was passed into load, and was set
using setLoadPackageDataError. (Though I'm wondering if the call
to setLoadPackageDataError should be done before the call to load).
Fixes#38034
Change-Id: I0748866933b4c1a329954b4b96640bef702a4644
Reviewed-on: https://go-review.googlesource.com/c/go/+/228784
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Being lenient on those has caused enough security issues.
Spun out of CL 231419.
Fixes#38889
Change-Id: Idd3bc6adc22e08a30b3dabb146ce78d4105684cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/232277
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Before going around making changes, surface the current behavior in the
docs as a starting point. No behavior changes.
Change-Id: If8096cedbba7eda37694dbb7f438046d590c3bcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/231377
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
This is a security hardening measure against HTTP request smuggling.
Thank you to ZeddYu for reporting this issue.
We weren't parsing things correctly anyway, allowing "identity" to be
combined with "chunked", and ignoring any Transfer-Encoding header past
the first. This is a delicate security surface that already broke
before, just be strict and don't add complexity to support cases not
observed in the wild (nginx removed "identity" support [1] and multiple
TE header support [2]) and removed by RFC 7230 (see page 81).
It'd probably be good to also drop support for anything other than
"chunked" in outbound TE headers, as "identity" is not a thing anymore,
and we are probably off-spec for anything other than "chunked", but it
should not be a security concern, so leaving it for now. See #38867.
[1]: https://hg.nginx.org/nginx/rev/fe5976aae0e3
[2]: https://hg.nginx.org/nginx/rev/aca005d232ff
Change-Id: If17d0827f9c6167a0b19a158e2bc5844ec803288
Reviewed-on: https://go-review.googlesource.com/c/go/+/231418
Reviewed-by: Katie Hockman <katie@golang.org>
Clients have to reject any HelloRetryRequest message that doesn't lead
to a change in the ClientHello. Instead, we were rejecting any HRR that
didn't select an alternative group, even if it sent a cookie, which
would change the CH.
The good news is that I know of no TLS servers that use or need HRRs
exclusively for cookies (which are mostly useful in DTLS as a way to
verify the source address). The bad news is that we poisoned the
ecosystem as Go 1.12 to 1.14 will reject such HRRs. Oops, hopefully no
one needed this.
No tests because neither Go nor s_server support cookies. This would
presumably get covered once we integrate BoGo.
Fixes#30149
Change-Id: I760fb1ded81148ac3096cf201cbc1e941374b83d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231039
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Following CL 208126, do the same for MIPS.
Change-Id: I95f8fc99a234524119a4d29c7695676dc0ea1025
Reviewed-on: https://go-review.googlesource.com/c/go/+/208217
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
On some architectures, for async preemption the injected call
needs to clobber a register (usually REGTMP) in order to return
to the preempted function. As a consequence, the PC ranges where
REGTMP is live are not preemptible.
The uses of REGTMP are usually generated by the assembler, where
it needs to load or materialize a large constant or offset that
doesn't fit into the instruction. In those cases, REGTMP is not
live at the start of the instruction sequence. Instead of giving
up preemption in those cases, we could preempt it and restart the
sequence when resuming the execution. Basically, this is like
reissuing an interrupted instruction, except that here the
"instruction" is a Prog that consists of multiple machine
instructions. For this to work, we need to generate PC data to
mark the start of the Prog.
Currently this is only done for ARM64.
TODO: the split-stack function prologue is currently not async
preemptible. We could use this mechanism, preempt it and restart
at the function entry.
Change-Id: I37cb282f8e606e7ab6f67b3edfdc6063097b4bd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/208126
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
There is a range of numbers lower than 0x7fff_ffff_ffff_ffff which
cannot be represented by a 64 bit float. We set that to the correct
limit beyond which conversions can happen properly.
It appears that the negative bound check can indeed by correctly handled
by I64TruncF64S. But we use the same limit for consistency.
Fixes#38839
Change-Id: Ib783a22cb331fba7e6955459f41c67f9ceb53461
Reviewed-on: https://go-review.googlesource.com/c/go/+/231837
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Record the caller when Cleanup is called to report it with t.Log
instead of unhelpful line in testing.go.
Fixes#38800
Change-Id: I3136f5d92a0e5a48f8b32a2e13b2521bc91d72d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/232237
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The {AND,OR,XOR}const ops can only take an int32 as an argument.
Make sure that when rewriting a BTx op to one of these, the result
has no high-order bits.
Fixes#38746
Change-Id: Ia7c5f76952329f60974bc033c29a5433610f3b28
Reviewed-on: https://go-review.googlesource.com/c/go/+/231977
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Replaced almost every use of Bytes with FillBytes.
Note that the approved proposal was for
func (*Int) FillBytes(buf []byte)
while this implements
func (*Int) FillBytes(buf []byte) []byte
because the latter was far nicer to use in all callsites.
Fixes#35833
Change-Id: Ia912df123e5d79b763845312ea3d9a8051343c0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230397
Reviewed-by: Robert Griesemer <gri@golang.org>
The Go 1.15 code freeze has just started. This is the time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
Those versions have already gone through code review, and now they
will undergo additional testing during the freeze period.
If there are new issues in these dependencies discovered, we have
the freeze period to deal with that. By the end of the freeze period,
we will have confidence that the Go 1.15 release and the dependency
versions it has selected are robust.
If one of the Go 1.15.x minor releases requires changing code in one of
the vendored packages, we'll be able to do so on top of the versions
that are selected here, and not be forced to use versions that came
from different time periods, or try to jump across multiple untested
versions in a minor release.
The dependency versions that are selected in this commit are:
github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3
github.com/ianlancetaylor/demangle v0.0.0-20200414190113-039b1ae3a340
golang.org/x/arch v0.0.0-20200312215426-ff8b605520f4
golang.org/x/crypto v0.0.0-20200429183012-4b2356b1ed79
golang.org/x/mod v0.2.1-0.20200429172858-859b3ef565e2
golang.org/x/net v0.0.0-20200501053045-e0ff5e5a1de5
golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3
golang.org/x/text v0.3.3-0.20200430171850-afb9336c4530
golang.org/x/tools v0.0.0-20200504152539-33427f1b0364
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
github.com/ianlancetaylor/demangle is considered in scope and updated.
github.com/google/pprof is out of scope and was not updated.
For #36905.
Change-Id: Icb6996eb0df11f16edd9a42e04434012c0336354
Reviewed-on: https://go-review.googlesource.com/c/go/+/231657
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, Value.Addr collapses flagRO, which is a combination of
flagEmbedRO and flagStickyRO, to flagStickyRO. This causes exported
fields of unexported anonymous field from Value.Addr.Elem read only.
This commit fix this by keeping all bits of flagRO from origin
value in Value.Addr. This should be safe due to following reasons:
* Result of Value.Addr is not CanSet because of it is not CanAddr
but not flagRO.
* Addr.Elem get same flagRO as origin, so it should behave same as
origin in CanSet.
Fixes#32772.
Change-Id: I79e086628c0fb6569a50ce63f3b95916f997eda1
GitHub-Last-Rev: 78e280e6d0
GitHub-Pull-Request: golang/go#32787
Reviewed-on: https://go-review.googlesource.com/c/go/+/183937
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
QueryPattern will now look up the current version of a module (if any)
before invoking queryProxy. This changes the interpretation of some
patterns (like "upgrade") and avoids the need to download earlier
versions for earlier versions when the current version is
+incompatible.
Fixes#37574
Change-Id: I4089d6099236493df13a7f88a252b5e5e556d383
Reviewed-on: https://go-review.googlesource.com/c/go/+/231599
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Introduces a few casts, mostly to fix rules that mix int64 and int32
off1 and off2.
Passes
GOARCH=arm64 gotip build -toolexec 'toolstash -cmp' -a std
Change-Id: I1ec75211f3bb8e521dcc5217cf29ab0655a84d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/230840
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 208617 introduced syscall.utf16PtrToString and
internal/syscall/windows.UTF16PtrToString functions.
Original version of CL 208617 did not include syscall.utf16PtrToString
and internal/syscall/windows.UTF16PtrToString max parameter. The
parameter was added by Brad at the request of Ian. Ian said:
"In some cases it seems at least possible that the null terminator is
not present. I think it would be safer if we passed a maximum length
here."
The syscall.utf16PtrToString and
internal/syscall/windows.UTF16PtrToString function are designed to work
with only null terminated strings. So max parameter is superfluous.
This change removes max parameter.
Updates #34972
Change-Id: Ifea65dbd86bca8a08353579c6b9636c6f963d165
Reviewed-on: https://go-review.googlesource.com/c/go/+/228858
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
When forwarding a request, a SingleHostReverseProxy appends the
request's path to the target URL's path. However, if certain path
elements are encoded, (such as %2F for slash in either the request or
target path), simply joining the URL.Path elements is not sufficient,
since the field holds the decoded path.
Since 87a605, the RawPath field was added which holds a decoding
hint for the URL. When joining URL paths, this decoding hint needs
to be taken into consideration.
As an example, if the target URL.Path is /a/b, and URL.RawPath
is /a%2Fb, joining the path with /c should result in /a/b/c
in URL.Path, and /a%2Fb/c in RawPath.
The added joinURLPath function combines the two URL's Paths,
while taking into account escaping, and replaces the previously used
singleJoiningSlash in NewSingleHostReverseProxy.
Fixes#35908
Change-Id: I45886aee548431fe4031883ab1629a41e35f1727
GitHub-Last-Rev: 7be6b8d421
GitHub-Pull-Request: golang/go#36378
Reviewed-on: https://go-review.googlesource.com/c/go/+/213257
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This makes the intent clearer, allows for another ellipsis and will aid
in future rewriting. While here, document boolean loads to explain register
contents.
Change-Id: I933db2813826d88819366191fbbea8fcee5e4dda
Reviewed-on: https://go-review.googlesource.com/c/go/+/230120
Reviewed-by: Keith Randall <khr@golang.org>
Ctty was always handled as a child descriptor, but in some cases
passing a parent descriptor would also work. This depended on
unpredictable details of the implementation. Reject those cases to
avoid confusion.
Also reject setting both Setctty and Foreground, as they use Ctty
in incompatible ways. It's unlikely that any programs set both fields,
as they don't make sense together.
Fixes#29458
Change-Id: Ieba2d625711fd4b82c8e65e1feed02fd1fb25e6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231638
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Normalization of number prefixes and exponents was added in CL 160184
directly in cmd/gofmt. The same behavior change needs to be applied in
the go/format package. This is done by moving the normalization code
into go/printer, behind a new StdFormat mode, which is then re-used
by both cmd/gofmt and go/format.
Note that formatting of Go source code changes over time, so the exact
byte output produced by go/printer may change between versions of Go
when using StdFormat mode. What is guaranteed is that the new formatting
is equivalent Go code.
Clients looking to format Go code with standard formatting consistent
with cmd/gofmt and go/format would need to start using this flag, but
a better alternative is to use the go/format package instead.
Benchstat numbers on go test go/printer -bench=BenchmarkPrint:
name old time/op new time/op delta
Print-8 4.56ms ± 1% 4.57ms ± 0% ~ (p=0.700 n=3+3)
name old alloc/op new alloc/op delta
Print-8 467kB ± 0% 467kB ± 0% ~ (p=1.000 n=3+3)
name old allocs/op new allocs/op delta
Print-8 17.2k ± 0% 17.2k ± 0% ~ (all equal)
That benchmark data doesn't contain any numbers that need to be
normalized. More work needs to be performed when formatting Go code
with numbers, but it is unavoidable to produce standard formatting.
Fixes#37476.
For #37453.
Change-Id: If50bde4035c3ee6e6ff0ece5691f6d3566ffe8d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/231461
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Adding the usage of PCALIGN directive for arm64, and updating some
details on using some directives defined in the textflag.h file.
Change-Id: I43d363e3337939bab69b856831caf06803a292d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/227801
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 231397 is submitted too fast... Expand the comment to make it
a little clearer.
Change-Id: Ica9737aa7b51f97320bab74457388dcab8188370
Reviewed-on: https://go-review.googlesource.com/c/go/+/231597
Reviewed-by: Austin Clements <austin@google.com>
This commit adds a new option to the x86 assembler. If the
GOAMD64 environment variable is set to alignedjumps (the
default) and we're doing a 64 bit build, the assembler will
make sure that neither stand alone nor macro-fused jumps will
end on or cross 32 byte boundaries. To achieve this, functions
are aligned on 32 byte boundaries, rather than 16 bytes, and
jump instructions are padded to ensure that they do not
cross or end on 32 byte boundaries. Jumps are padded
by adding a NOP instruction of the appropriate length before
the jump.
The commit is likely to result in larger binary sizes when
GOAMD64=alignedjumps. On the binaries tested so far, an
increase of between 1.4% and 1.5% has been observed.
Updates #35881
Co-authored-by: David Chase <drchase@google.com>
Change-Id: Ief0722300bc3f987098e4fd92b22b14ad6281d91
Reviewed-on: https://go-review.googlesource.com/c/go/+/219357
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A symbol being reachable doesn't imply its type descriptor is
needed. Don't mark it.
If the type is converted to interface somewhere in the program,
there will be an explicit use of the type descriptor, which
will make it marked.
A println("hello") program before and after
-rwxr-xr-x 1 cherryyz primarygroup 1259824 Apr 30 23:00 hello
-rwxr-xr-x 1 cherryyz primarygroup 1169680 Apr 30 23:10 hello
Updates #38782.
Updates #6853.
Change-Id: I88884c126ce75ba073f1ba059c4b892c87d2ac96
Reviewed-on: https://go-review.googlesource.com/c/go/+/231397
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
(Reland of golang.org/cl/33677.)
This CL adds a UsesCgo config setting to go/types to specify that the
_cgo_gotypes.go file generated by cmd/cgo has been provided as a
source file. The type checker then internally resolves C.bar qualified
identifiers to _Cfoo_bar as appropriate.
It also adds support to srcimporter to automatically run cgo.
Unfortunately, this functionality is not compatible with overriding
OpenFile, because cmd/cgo and gcc will directly open files.
Updates #16623.
Updates #35721.
Change-Id: Ib179d55c8c589916f98ceeae0b9a3e746157253a
Reviewed-on: https://go-review.googlesource.com/c/go/+/231459
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Cgo's initial design for handling "#define foo int*" involved
rewriting "C.foo" to "*_Ctype_int" everywhere. But now that we have
type aliases, we can declare "type _Ctype_foo = *_Ctype_int" once, and
then rewrite "C.foo" to just "_Ctype_foo".
This is important for go/types's UsesCgo mode, where go/types needs to
be able to figure out a type for each C.foo identifier using only the
information written into _cgo_gotypes.go.
Fixes#38649.
Change-Id: Ia0f8c2d82df81efb1be5bc26195ea9154c0af871
Reviewed-on: https://go-review.googlesource.com/c/go/+/230037
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For latest http2 changes.
Which then required updating golang.org/x/sys in cmd too.
Change-Id: I3fac5f3a15f4c9381baaff597873ed0c6209dbac
Reviewed-on: https://go-review.googlesource.com/c/go/+/231457
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Refer to reflect.StringHeader in the godoc comment for type String
instead of reflect.SliceHeader.
Change-Id: I40fc016c7365510a12c41d4ca596f66d2892c3f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/231537
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the dev.link branch we continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker . Since the last merge, more
progress has been made to improve the new linker, with
improvements on both linker speed and memory usage.
This is a clean merge.
Change-Id: I38516d6c4b41021bc61c1b9886e701de5fa2b0f1
Convert the part that uses relocations to use loader.ExtReloc
directly. It still uses sym.Symbols for now, but not sym.Relocs.
This reduces some memory usage: linking cmd/compile with external
linking,
name old allocs/op new allocs/op delta
Loadlibfull_GC 52.2MB ± 0% 13.9MB ± 0% -73.40% (p=0.008 n=5+5)
name old live-B new live-B delta
Loadlibfull_GC 75.5M ± 0% 61.9M ± 0% -18.02% (p=0.008 n=5+5)
Change-Id: I317ecbf516063c42b255b2caba310ea6281342d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/231319
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
For now this will only avoid copying math/big/arith_riscv64.s
Change-Id: Ib236e4bf1a6a758649629268a6f512f307596e74
Reviewed-on: https://go-review.googlesource.com/c/go/+/231298
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reduces binary size by 4K, not counting the http2 changes (in CL
231119) that'll be bundled into this package in the future.
Updates golang/go#38782
Change-Id: Id360348707e076b8310a8f409e412d68dd2394b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/231118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The Ctty field is a child descriptor number when Setctty is set,
but a parent descriptor when Foreground is set. This is absurd
but changing either behavior breaks existing programs.
With this change we at least document how it works.
For #29458
Change-Id: If9cf0a1a1e6ed0d4a4edae5043016d5b4ee3308b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229768
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Implement special case handling and testing to ensure
conformance with the C99 standard annex G.6 Complex arithmetic.
Fixes#29320
Change-Id: Id72eb4c5a35d5a54b4b8690d2f7176ab11028f1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/220689
Reviewed-by: Robert Griesemer <gri@golang.org>
The new package "internal/unsafeheader" depends only on "unsafe", and
provides declarations equivalent to reflect.StringHeader and
reflect.SliceHeader but with Data fields of the proper unsafe.Pointer
type (instead of uintptr).
Unlike the types it replaces, the "internal/unsafeheader" package has
a regression test to ensure that its header types remain equivalent to
the declarations provided by the "reflect" package.
Since "internal/unsafeheader" has almost no dependencies, it can be
used in other low-level packages such as "syscall" and "reflect".
This change is based on the corresponding x/sys change in CL 231177.
Fixes#37805
Updates #19367
Change-Id: I7a6d93ef8dd6e235bcab94e7c47270aad047af31
Reviewed-on: https://go-review.googlesource.com/c/go/+/231223
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now that the loader's internal storage mechanism for symbol alignment
is array-based and not map-based, we can go back to computing symbol
alignment in the parallel-by-section section of dodata.
With this patch plus the previous one, this produces a small
kubelet speedup:
$ benchstat out.devlink.txt out.align.txt
name old time/op new time/op delta
RelinkKubelet 13.3s ± 2% 13.1s ± 2% -1.27% (p=0.000 n=20+20)
RelinkKubelet-WithoutDebug 7.36s ± 5% 7.14s ± 3% -3.00% (p=0.000 n=20+20)
Change-Id: I9eb0e8fea6aeb12f188f499e9031d5a3a23232c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/231221
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Switch the storage mechanism for symbol alignment away from a map and
to a slice of uint8 values per symbol, where value K indicates
alignment 2^K. Intended to help speed up alignment get/set in dodata.
Change-Id: I26416e455c808f697dd0d7f6d2582247ee5c5b40
Reviewed-on: https://go-review.googlesource.com/c/go/+/231220
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Using 'go get x.go' instead of 'go build x.go' or some other
go command is a common mistake. By that mistake, a user gets
a misleading error message about unsuccessful `x.go` domain lookup.
This improvement handles such cases, by validating, whether the
argument hasn't specified version, has .go suffix, and either has
no slashes or such file locally exists. Handled both GOPATH
and GOMOD modes.
Fixes#38478
Change-Id: I583a4ef7f7ca8901deb07ebc811e2b3c0e828fa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/229938
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parseFloatPrefix accepts a string if it has a valid floating-point
number as prefix. Make sure that "infi", "infin", ... etc. are
accepted as valid numbers "inf" with suffix "i", "in", etc. This
is important for parsing complex numbers such as "0+infi".
This change does not affect the correctness of ParseFloat because
ParseFloat rejects strings that contain a suffix after a valid
floating-point number.
Updates #36771.
Change-Id: Ie1693a8ca2f8edf07b57688e0b35751b7100d39d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231237
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, for external relocations, the ExtReloc structure
contains all the fields of the relocation. In fact, many of the
fields are the same with the original relocation. So, instead, we
can just use an index to reference the original relocation and
not expand the fields.
There is one place where we modify relocation type: changing
R_DWARFSECTREF to R_ADDR. Get away with it by changing
downstreams.
It also makes it easier to retrieve the reloc variant.
This reduces some allocation. Linking cmd/compile with external
linking,
name old alloc/op new alloc/op delta
Reloc_GC 34.1MB ± 0% 22.7MB ± 0% -33.30% (p=0.000 n=5+4)
Change-Id: Id08a89ed2aee705296886d3b95014b806a0d55cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/231217
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
In the dev.link branch we continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker . Since the last merge, more
progress has been made to improve the new linker.
This is a clean merge.
Change-Id: Ide5ad6fcec9cede99e9b21c4548929b4ba1f4185
Revise the signature for "relocsym" to reflect the fact that many of
its arguments are invariant: push the invariant args into a struct and
pass the struct by reference.
Add a facility for doing batch allocation of external relocations in
relocsym, so that we don't wind up with wasted space due to the
default "append" behavior.
This produces a small speedup in linking kubelet:
$ benchstat out.devlink.txt out.dodata.txt
name old time/op new time/op delta
RelinkKubelet 14.2s ± 2% 13.8s ± 2% -3.11% (p=0.000 n=19+19)
RelinkKubelet-WithoutDebug 8.02s ± 3% 7.73s ± 3% -3.67% (p=0.000 n=20+20)
Change-Id: I8bc94c366ae792a5b0f23697b8e0108443a7a748
Reviewed-on: https://go-review.googlesource.com/c/go/+/231138
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Under the scavenge lock it's possible to ready a goroutine (or now
injectglist, which has mostly the same effect) which could cause an
unpark trace event to be emitted. If there's no active trace buffer for
the P, then we might acquire the lock. The total order between the two
is correct, but there's no partial order edge between them. Add in the
edge.
Change-Id: I3fc5d86a3b6bdd0b5648181fb76b5ebc90c3d69f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231197
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
This change modifies the semantics of waking the scavenger: rather than
wake on any update to pacing, wake when we know we will have work to do,
that is, when the sweeper is done. The current scavenger runs over the
address space just once per GC cycle, and we want to maximize the chance
that the scavenger observes the most attractive scavengable memory in
that pass (i.e. free memory with the highest address), so the timing is
important. By having the scavenger awaken and reset its search space
when the sweeper is done, we increase the chance that the scavenger will
observe the most attractive scavengable memory, because no more memory
will be freed that GC cycle (so the highest scavengable address should
now be available).
Furthermore, in applications that go idle, this means the background
scavenger will be awoken even if another GC doesn't happen, which isn't
true today.
However, we're unable to wake the scavenger directly from within the
sweeper; waking the scavenger involves modifying timers and readying
goroutines, the latter of which may trigger an allocation today (and the
sweeper may run during allocation!). Instead, we do the following:
1. Set a flag which is checked by sysmon. sysmon will clear the flag and
wake the scavenger.
2. Wake the scavenger unconditionally at sweep termination.
The idea behind this policy is that it gets us close enough to the state
above without having to deal with the complexity of waking the scavenger
in deep parts of the runtime. If the application goes idle and sweeping
finishes (so we don't reach sweep termination), then sysmon will wake
the scavenger. sysmon has a worst-case 20 ms delay in responding to this
signal, which is probably fine if the application is completely idle
anyway, but if the application is actively allocating, then the
proportional sweeper should help ensure that sweeping ends very close to
sweep termination, so sweep termination is a perfectly reasonable time
to wake up the scavenger.
Updates #35788.
Change-Id: I84289b37816a7d595d803c72a71b7f5c59d47e6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/207998
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change adds two bits of logic to the scavenger's pacing. Firstly,
it checks to make sure we scavenged at least one physical page, if we
released a non-zero amount of memory. If we try to release less than one
physical page, most systems will release the whole page, which could
lead to memory corruption down the road, and this is a signal we're in
this situation.
Secondly, the scavenger's pacing logic now checks to see if the time a
scavenging operation takes is measured to be exactly zero or negative.
The exact zero case can happen if time update granularity is too large
to effectively capture the time the scavenging operation took, like on
Windows where the OS timer frequency is generally 1ms. The negative case
should not happen, but we're being defensive (against kernel bugs, bugs
in the runtime, etc.). If either of these cases happen, we fall back to
Go 1.13 behavior: assume the scavenge operation took around 10µs per
physical page. We ignore huge pages in this case because we're in
unknown territory, so we choose to be conservative about pacing (huge
pages could only increase the rate of scavenging).
Currently, the scavenger is broken on Windows because the granularity of
time measurement is around 1 ms, which is too coarse to measure how fast
we're scavenging, so we often end up with a scavenging time of zero,
followed by NaNs and garbage values in the pacing logic, which usually
leads to the scavenger sleeping forever.
Fixes#38617.
Change-Id: Iaaa2a4cbb21338e1258d010f7362ed58b7db1af7
Reviewed-on: https://go-review.googlesource.com/c/go/+/229997
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Austin Clements <austin@google.com>
This CL changes the arm64 TBZ/TBNZ block from using Aux to using
a (typed) AuxInt. The corresponding rules have also been changed
to be typed.
Passes
GOARCH=arm64 gotip build -toolexec 'toolstash -cmp' -a std
Change-Id: I98d0cd2a791948f1db13259c17fb1b9b2807a043
Reviewed-on: https://go-review.googlesource.com/c/go/+/230839
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Tweak doDataSect to reduce symbol sorting overhead, and calculate size
ahead of allocating the ctxt.datap slice. Yields a small speedup
(2-3%) linking kubelet.
Change-Id: I82869f5276caa4bee9f6e6f41da2b240e601ce50
Reviewed-on: https://go-review.googlesource.com/c/go/+/231047
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We call (or will call) ResolveABIAlias in many places. Doing zero
symbol check everytime is annoying. Fold the condition into
ResolveABIAlias.
Change-Id: I10485fe83b9cce2d19b6bd17dc42176f72dae48b
Reviewed-on: https://go-review.googlesource.com/c/go/+/231046
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Adddynrel2 is a function pointer. In dynrelocsym we pass &r to
it, which will cause r to escape. Pass it by value instead.
Linking cmd/compile,
name old alloc/op new alloc/op delta
Dodata_GC 15.8MB ± 0% 5.9MB ± 0% -62.55% (p=0.008 n=5+5)
Change-Id: Ib86005d1026ebaca57777b27ead037e613585f44
Reviewed-on: https://go-review.googlesource.com/c/go/+/231045
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Archreloc2 is a function pointer. It will escape its pointer
arguments. In relocsym, as we pass &r and &rr to Archreloc2, it
causes them to escape, even if Archreloc2 is not actually called.
Instead, pass r by value. loader.Reloc2 is a small structure
which is intended to be passed by value.
For rr, as Archreloc2 will likely return true, we speculatively
add it to extRelocs slice and use that space to pass to
Archreloc2.
Linking cmd/compile,
name old alloc/op new alloc/op delta
Dwarfcompress_GC 110MB ± 0% 24MB ± 0% -78.34% (p=0.008 n=5+5)
Reloc_GC 24.6MB ± 0% 0.0MB ± 0% -100.00% (p=0.029 n=4+4)
Linking cmd/compile using external linking
name old alloc/op new alloc/op delta
Reloc_GC 152MB ± 0% 36MB ± 0% -76.07% (p=0.008 n=5+5)
Change-Id: I1415479e0c17ea9787f9a62453dce00ad9ea792f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231077
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This change adjusts go command to pass -buildmode=pie to cmd/link,
if -buildmode is not explicitly provided.
Fixes#35192
Change-Id: Iec020131e676eb3e9a2df9eea1929b2af2b6df04
Reviewed-on: https://go-review.googlesource.com/c/go/+/230217
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
'go mod verify' checksums one module zip at a time, which is
CPU-intensive on most modern machines with fast disks. As a result, one
can see a CPU bottleneck when running the command on, for example, a
module where 'go list -m all' lists ~440 modules:
$ /usr/bin/time go mod verify
all modules verified
11.47user 0.77system 0:09.41elapsed 130%CPU (0avgtext+0avgdata 24284maxresident)k
0inputs+0outputs (0major+4156minor)pagefaults 0swaps
Instead, verify up to GOMAXPROCS zips at once, which should line up
pretty well with the amount of processors we can use on a machine. The
results below are obtained via 'benchcmd -n 5 GoModVerify go mod verify'
on the same large module.
name old time/op new time/op delta
GoModVerify 9.35s ± 1% 3.03s ± 2% -67.60% (p=0.008 n=5+5)
name old user-time/op new user-time/op delta
GoModVerify 11.2s ± 1% 16.3s ± 3% +45.38% (p=0.008 n=5+5)
name old sys-time/op new sys-time/op delta
GoModVerify 841ms ± 9% 865ms ± 8% ~ (p=0.548 n=5+5)
name old peak-RSS-bytes new peak-RSS-bytes delta
GoModVerify 27.8MB ±13% 50.7MB ±27% +82.01% (p=0.008 n=5+5)
The peak memory usage nearly doubles, and there is some extra overhead,
but it seems clearly worth the tradeoff given that we see a ~3x speedup
on my laptop with 4 physical cores. The vast majority of developer
machines nowadays should have 2-4 cores at least.
No test or benchmark is included; one can benchmark 'go mod verify'
directly, as I did above. The existing tests also cover correctness,
including any data races via -race.
Fixes#38623.
Change-Id: I45d8154687a6f3a6a9fb0e2b13da4190f321246c
Reviewed-on: https://go-review.googlesource.com/c/go/+/229817
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Race builds require C dependencies, but cross-compiled cgo builds are
not always possible, so don't suggest enabling CGO in those cases.
Fixes#37021
Change-Id: I1fd675efc9cef958a926bd63eac8e6858bc59d0a
GitHub-Last-Rev: cbf43c1bbb
GitHub-Pull-Request: golang/go#38670
Reviewed-on: https://go-review.googlesource.com/c/go/+/230202
Reviewed-by: Bryan C. Mills <bcmills@google.com>
A recent change added a title to the HTML coverage report but
neglected to include the package name. Add the package name here.
It's a little trickier than you'd think because there may be multiple
packages and we don't want to parse the files, so we just extract
a directory name from the path of the first file. This will almost
always be right, and has the advantage that it gives a better result
for package main. There are rare cases it will get wrong, but that
will be no hardship.
If this turns out not to be good enough, we can refine it.
Fixes#38609
Change-Id: I2201f6caef906e0b0258b90d7de518879041fe72
Reviewed-on: https://go-review.googlesource.com/c/go/+/230517
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reportedly some Docker images accept the prlimit64 system call,
used by syscall.prlimit, but prohibit the getrlimit and setrlimit
system calls.
Fixes#38604
Change-Id: I91ff9370450b4869098cc8e335bbb7b863060508
Reviewed-on: https://go-review.googlesource.com/c/go/+/230339
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
And remove "2" from some function names.
Change-Id: Ibf1089970d849a42f53976064ceb9ade20bf6eba
Reviewed-on: https://go-review.googlesource.com/c/go/+/231017
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The fields aren't too useful for Go 1.2 and later, but they aren't
actually nil.
Fixes#38754
Change-Id: Ia13a224f623697a00dea8ba0225633e1b9308c9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230940
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 4f7053c87f.
Reason for revert: Newly added test is failing on several builders.
Change-Id: I22dcbfebf2f57735b2f479886bbeb623f95b132f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231043
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Omits printing the file:line:column when trying to open either
* non-existent files
* files without permission
Given:
go tool compile x.go
For either of x.go not existing, or if no read permissions:
* Before:
x.go:0: open x.go: no such file or directory
x.go:0: open x.go: permission denied
* After:
open x.go: no such file or directory
open x.go: permission denied
While here, noticed an oddity with the Linux builders, that appear
to always be running under root, hence the test for permission errors
with 0222 -W-*-W-*-W- can't pass on linux-amd64 builders.
The filed bug is #38608.
Fixes#36437
Change-Id: I9645ef73177c286c99547e3a0f3719fa07b35cb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/229357
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
All five calls to wakep are protected by the same check of nmidle and
nmspinning. Move this check into wakep.
Change-Id: I2094eec211ce551e462e87614578f37f1896ba38
Reviewed-on: https://go-review.googlesource.com/c/go/+/230757
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When I browsed the source code, I saw that there is no corresponding example of this function. I am not sure if there is a need for an increase, this is my first time to submit CL.
Change-Id: Idbf4e1e1ed2995176a76959d561e152263a2fd26
Reviewed-on: https://go-review.googlesource.com/c/go/+/230741
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The switch statement can be statically optimized by the compiler,
whereas similarly optimizing the map index expression would require
additional compiler analysis to detect the map is never mutated.
Updates #10848.
Change-Id: I2fc70d4a34dc545677b99f218b51023c7891bbbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/231041
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, we emit stack maps and register maps at almost every
instruction. This was originally intended to support non-cooperative
preemption, but was only ever used for debug call injection. Now debug
call injection also uses conservative frame scanning. As a result,
stack maps are only needed at call sites and register maps aren't
needed at all except that we happen to also encode unsafe-point
information in the register map PCDATA stream.
This CL reduces stack maps to only appear at calls, and replace full
register maps with just safe/unsafe-point information.
This is all protected by the go115ReduceLiveness feature flag, which
is defined in both runtime and cmd/compile.
This CL significantly reduces binary sizes and also speeds up compiles
and links:
name old exe-bytes new exe-bytes delta
BinGoSize 15.0MB ± 0% 14.1MB ± 0% -5.72%
name old pcln-bytes new pcln-bytes delta
BinGoSize 3.14MB ± 0% 2.48MB ± 0% -21.08%
name old time/op new time/op delta
Template 178ms ± 7% 172ms ±14% -3.59% (p=0.005 n=19+19)
Unicode 71.0ms ±12% 69.8ms ±10% ~ (p=0.126 n=18+18)
GoTypes 655ms ± 8% 615ms ± 8% -6.11% (p=0.000 n=19+19)
Compiler 3.27s ± 6% 3.15s ± 7% -3.69% (p=0.001 n=20+20)
SSA 7.10s ± 5% 6.85s ± 8% -3.53% (p=0.001 n=19+20)
Flate 124ms ±15% 116ms ±22% -6.57% (p=0.024 n=18+19)
GoParser 156ms ±26% 147ms ±34% ~ (p=0.070 n=19+19)
Reflect 406ms ± 9% 387ms ±21% -4.69% (p=0.028 n=19+20)
Tar 163ms ±15% 162ms ±27% ~ (p=0.370 n=19+19)
XML 223ms ±13% 218ms ±14% ~ (p=0.157 n=20+20)
LinkCompiler 503ms ±21% 484ms ±23% ~ (p=0.072 n=20+20)
ExternalLinkCompiler 1.27s ± 7% 1.22s ± 8% -3.85% (p=0.005 n=20+19)
LinkWithoutDebugCompiler 294ms ±17% 273ms ±11% -7.16% (p=0.001 n=19+18)
(https://perf.golang.org/search?q=upload:20200428.8)
The binary size improvement is even slightly better when you include
the CLs leading up to this. Relative to the parent of "cmd/compile:
mark PanicBounds/Extend as calls":
name old exe-bytes new exe-bytes delta
BinGoSize 15.0MB ± 0% 14.1MB ± 0% -6.18%
name old pcln-bytes new pcln-bytes delta
BinGoSize 3.22MB ± 0% 2.48MB ± 0% -22.92%
(https://perf.golang.org/search?q=upload:20200428.9)
For #36365.
Change-Id: I69448e714f2a44430067ca97f6b78e08c0abed27
Reviewed-on: https://go-review.googlesource.com/c/go/+/230544
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We're about to switch to having significantly fewer maps in the
liveness map, so switch from a dense representation to a sparse
representation.
Passes toolstash-check.
For #36365.
Change-Id: Icb17bd6ace17667a280bc5fba4039cae3020a8d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230543
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These are necessarily deeply non-preemptible, so there's no point in
emitting stack maps for them. We already mark them as unsafe points,
so this only affects the runtime, since user code does not emit stack
maps at unsafe points. SSAGenState.PrepareCall also excludes them when
it's sanity checking call stack maps.
Right now this only drops a handful of unnecessary stack maps from the
runtime, but we're about to start emitting stack maps only at calls
for user code, too. At that point, this will matter much more.
For #36365.
Change-Id: Ib3abfedfddc8e724d933a064fa4d573500627990
Reviewed-on: https://go-review.googlesource.com/c/go/+/230542
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The compiler currently conflates whether a Value has a stack map with
whether it's an unsafe point. For the most part, unsafe-points don't
have stack maps, so this is mostly fine, but call instructions can be
both an unsafe-point *and* have a stack map. For example, none of the
instructions in a nosplit function should be preemptible, but calls
must still have stack maps in case the called function grows the stack
or get preempted.
Currently, the compiler can't distinguish this case, so calls in
nosplit functions are marked as safe-points just because they have
stack maps. This is particularly problematic if a nosplit function
calls another nosplit function, since this can introduce a preemption
point where there should be none.
We realized this was a problem for split-stack prologues a while back,
and CL 207349 changed the encoding of unsafe-points to use the
register map index instead of the stack map index so we could record
both a stack map and an unsafe-point at the same instruction. But this
was never extended into the compiler.
This CL fixes this problem in the compiler. We make LivenessIndex
slightly more abstract by separating unsafe-point marks from stack and
register map indexes. We map this to the PCDATA encoding later when
producing Progs. This isn't enough to fix the whole problem for
nosplit functions, because obj still adds prologues and marks those as
preemptible, but it's a step in the right direction.
I checked this CL by comparing maps before and after this change in
the runtime and net/http. In net/http, unsafe-points match exactly; at
anything that isn't an unsafe-point, both the stack and register maps
are unchanged by this CL. In the runtime, at every point that was a
safe-point before this change, the stack maps agree (and mostly the
runtime doesn't have register maps at all now). In both, all CALLs
(except write barrier calls) have stack maps.
For #36365.
Change-Id: I066628938b02e78be5c81a6614295bcf7cc566c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/230541
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, this function conflates two (easily conflated!) concepts:
whether a Value is a safe-point and whether it has a stack map. In
particular, call Values may not be a safe-point, but may need a stack
map anyway in case the called function grows the stack.
Hence, rename this function to "hasStackMap", since that's really what
it represents.
For #36365.
Change-Id: I89839de0be8db3be3f0d3a7fb5fcf0b0b6ebc98a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230540
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
PanicBounds and PanicExtend are lowered to runtime calls (with a
non-Go ABI), but are not currently marked as calls. Since liveness
analysis only emits stack maps at calls in the runtime, this means
these panic call sites in the runtime won't get a stack map. These
almost immediately turn into throws in the runtime, but there's still
a chance they'll try to grow the stack first, which would lead to a
different panic.
To fix this, mark these operations as calls.
Outside the runtime, we currently emit stack maps for everything that
isn't an unsafe-point, so these panic calls get stack maps by default.
However, we're about to move to emitting stack maps only at call
sites, at which point this will start to matter outside the runtime as
well.
I confirmed that this has no effect on anything but PCDATA/FUNCDATA in
runtime and net/http.
For #36365.
Change-Id: Ic5bb463fd152cc320c815dc04cf62005261ae169
Reviewed-on: https://go-review.googlesource.com/c/go/+/230539
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
A debugger can inject a call at almost any PC, which causes
significant complications with stack scanning and growth. Currently,
the runtime solves this using precise stack maps and register maps at
nearly all PCs, but these extra maps require roughly 5% of the binary.
These extra maps were originally considered worth this space because
they were intended to be used for non-cooperative preemption, but are
now used only for debug call injection.
This CL switches from using precise maps to instead using conservative
frame scanning, much like how non-cooperative preemption works. When a
call is injected, the runtime flushes all potential pointer registers
to the stack, and then treats that frame as well as the interrupted
frame conservatively.
The limitation of conservative frame scanning is that we cannot grow
the goroutine stack. That's doable because the previous CL switched to
performing debug calls on a new goroutine, where they are free to grow
the stack.
With this CL, there are no remaining uses of precise register maps
(though we still use the unsafe-point information that's encoded in
the register map PCDATA stream), and stack maps are only used at call
sites.
For #36365.
Change-Id: Ie217b6711f3741ccc437552d8ff88f961a73cee0
Reviewed-on: https://go-review.googlesource.com/c/go/+/229300
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, when a debugger injects a call, that call happens on the
goroutine where the debugger injected it. However, this requires
significant runtime complexity that we're about to remove.
To prepare for this, this CL switches to a different approach that
leaves the interrupted goroutine parked and runs the debug call on a
new goroutine. When the debug call returns, it resumes the original
goroutine.
This should be essentially transparent to debuggers. It follows the
exact same call injection protocol and ensures the whole protocol
executes indivisibly on a single OS thread. The only difference is
that the current G and stack now change part way through the protocol.
For #36365.
Change-Id: I68463bfd73cbee06cfc49999606410a59dd8f653
Reviewed-on: https://go-review.googlesource.com/c/go/+/229299
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, newproc1 allocates, initializes, and schedules a new
goroutine. We're about to change debug call injection in a way that
will need to create a new goroutine without immediately scheduling it.
To prepare for that, make scheduling the responsibility of newproc1's
caller. Currently, there's exactly one caller (newproc), so this
simply shifts that responsibility.
For #36365.
Change-Id: Idacd06b63e738982e840fe995d891bfd377ce23b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229298
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Rusage.Maxrss is in bytes on Darwin but in KiB on Linux. Fix this
discrepancy so it's always in bytes.
Change-Id: Ic714abc3276566b8fe5e30565072092212610854
Reviewed-on: https://go-review.googlesource.com/c/go/+/230979
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
golang.org/cl/229763 removed the documentation of requirements of
the function passed to FieldsFunc. The current implementation does
not require functions to return consistent results but this had not
been the case for previous implementations.
Add the requirement for consistent results back to the documentation
to allow for future implementations to be more allocation efficient
for an output with more than 32 fields. This is possible with a two
pass algorithm first determining the number of fields used to allocate
the output slice and then splitting the input into fields.
While at it align the documentation of bytes.FieldsFunc with
strings.FieldFunc.
Fixes#38630
Change-Id: Iabbf9ca3dff0daa41f4ec930a21a3dd98e19f122
Reviewed-on: https://go-review.googlesource.com/c/go/+/230797
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When then go command is run with -trimpath, it will now use
-fdebug-prefix-map when invoking the C compiler (if supported) to
replace the source root directory with a dummy root directory.
This should prevent source directories from appearing either literally
or in compressed DWARF in linked binaries.
Updates #36072
Change-Id: Iedd08d5e886f81e981f11248a1be4ed4f58bdd29
Reviewed-on: https://go-review.googlesource.com/c/go/+/212101
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This allows writing
// F does a thing.
//go:noinline
func F()
without the //go:noinline or other directive (such as //line)
ending up looking like extra words in the doc comment.
Fixes#37974.
Change-Id: Ic738d72802cc2fa448f7633915e7126d2f76d8ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/224737
Reviewed-by: Robert Griesemer <gri@golang.org>
When parsing go.mod files found via file-path replacements, it's safer to
use lockedfile.Read instead of ioutil.ReadFile, in case of overwriting by
other concurrent go commands.
Change-Id: I7dcac3bb5ada84bee1eb634b39f813c461ef103a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230838
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Previously, non-standard attributes in Name.Names were being
omitted when printed using Name.String(). Now, any non-standard
attributes that would not already be printed in Name.String()
are being added temporarily to Name.ExtraNames to be printed.
Fixes#33094Fixes#23069
Change-Id: Id9829c20968e16db7194549f69c0eb5985044944
Reviewed-on: https://go-review.googlesource.com/c/go/+/229864
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Per suggestion in CL 202578, this CL drops the purego build tag used
within this package.
Change-Id: I33626c73d6602e321528544ee601741f7e634c1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/230677
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Adds a few instructions to ppc64enc.s that were missing from the
previous update.
Change-Id: Ieafce39e905cdf4da3bfb00fdd5a39ab28089cb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/230437
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Once the symbol is compressed, we will not need the uncompressed
symbol content. Free its memory.
Linking cmd/compile,
name old live-B new live-B delta
Dwarfcompress_GC 42.7M ± 0% 37.9M ± 0% -11.31% (p=0.008 n=5+5)
Change-Id: Ib6cc73832946d158ff4f5b4f31be9c35ba7cf103
Reviewed-on: https://go-review.googlesource.com/c/go/+/230859
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Non functional, but required by the majority of the architectures.
Change-Id: I57601016c28ce665a9d434e283a1db8bded9b133
Reviewed-on: https://go-review.googlesource.com/c/go/+/230858
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
After LoadFull, we'll be using sym.Symbols mostly. We still need
the loader information for symbol index mappings and name
lookups, but not much else. Free some memory.
Linking cmd/compile,
name old time/op new time/op delta
Loadlibfull_GC 44.5M ± 0% 35.8M ± 0% -19.66% (p=0.008 n=5+5)
Archive_GC 46.4M ± 0% 37.6M ± 0% -18.89% (p=0.008 n=5+5)
Linking cmd/compile with external linking,
name old time/op new time/op delta
Loadlibfull_GC 82.5M ± 0% 57.4M ± 0% -30.41% (p=0.008 n=5+5)
Archive_GC 86.8M ± 0% 61.7M ± 0% -28.90% (p=0.008 n=5+5)
Change-Id: I6859d488172ef8968918b86de527fbfed6832ebf
Reviewed-on: https://go-review.googlesource.com/c/go/+/230300
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
When applying relocations, we need to resolve ABI aliases.
relocsym does that. Architecture-specific archreloc also needs to
do that. The old code doesn't do that since ABI aliases are
resolved in loadlibfull, or, in the old linker, in a much earlier
stage. We don't do this in the new linker, as we want to avoid
mutating relocations.
While here, move R_CONST and R_GOTOFF handling to generic code.
They appear on several architectures and the handling are same.
Should fix 386-clang and *bsd-386 builds.
Change-Id: I6681c94f0327555d6cf329d0a518c88848773671
Reviewed-on: https://go-review.googlesource.com/c/go/+/230857
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Support external linking for the new reloc pass as well, and
enable it on AMD64 and 386.
Change-Id: Ia71aec3d7c14e9d661e0748d2e988f29f220d1e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230308
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Remove 'Sub' field from sym.Symbol, replacing uses (those downstream of
loadlibfull) with loader method calls.
NB: removing the Outer field will have to wait for now; it is accessed
in archreloc methods that don't have access to link ctxt or loader
currently.
Change-Id: I2abe5906fc169c64b2ab7d5ad213619bea5a17c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/230617
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This updates the PPC64.rules file to use the MOD instructions
that are available in power9. Prior to power9 this is done
using a longer sequence with multiply and divide.
Included in this change is removal of the REM* opcode variations
that set the CC or OV bits since their settings are based
on the DIV and are not appropriate for the REM.
Change-Id: Iceed9ce33e128e1911c15592ee674276ce8ba3fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/229761
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Prior to this commit, NewXxx could panic when passed an image.Rectangle
with one of width or height being negative. But it might not panic if
both were negative, because (bpp * w * h) could still be positive. After
this commit, it will panic if both are negative.
With overflow, NewXxx might not have panicked if (bpp * w * h), the
length passed to "make([]uint8, length)", was still non-negative (after
truncation), but even if w and h were valid (non-negative), the overall
byte slice wasn't long enough. Iterating over the pixels would possibly
panic later with index out of bounds. This change moves the panic
earlier, closer to where the mistake is.
Change-Id: I011feb2d53515fc3f0fe72bb6c23b3953772c577
Reviewed-on: https://go-review.googlesource.com/c/go/+/230220
Reviewed-by: Rob Pike <r@golang.org>
Convert some optimizations rules to strongly-typed versions. Similar to
CL 230338, this CL only converts rules that need no additional changes
(i.e: only need to change '->' to '=>').
This CL covers the rules from line 800 - 1219.
Passes toolstash-check
Change-Id: I94181a809fa38918b78301f1c0c680b7a8ab552f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230738
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Only enabled for AMD64 when internal linking for now.
Change-Id: I2aa9ee47c0f7413ea7bbcdd31b8317c14220bba3
Reviewed-on: https://go-review.googlesource.com/c/go/+/230302
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Previously, looking up only IPv4 or IPv6 addresses was only possible
with DefaultResolver via ResolveIPAddr. Add this functionality to the
Resolver type with a new method, LookupIP. This largely brings Resolver
functionally to parity with the global functions. The name LookupIP is
used over ResolveIPAddr to be consistent with the other Resolver
methods.
There are two main benefits to (*Resolver).LookupIP over
(*Resolver).LookupHost. First is an ergonomic benefit. Wanting a
specific family of address is common enough to justify a method, evident
by the existence of ResolveIPAddr. Second, this opens the possibility of
not performing unnecessary DNS requests when only a specific family of
addresses are needed. This optimization is left to follow up work.
Updates #30452
Change-Id: I241f61019588022a39738f8920b0ddba900cecdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/228641
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allocate the runcases slice on the stack if the number
of select cases is small (up to 4).
Found while looking at production profiles of common
proto based RPC server framework code in Google which do
not have a large number of cases.
name old time/op new time/op delta
Select/1 147ns ± 2% 120ns ± 6% -18.32% (p=0.000 n=7+10)
Select/4 316ns ± 5% 249ns ± 2% -21.23% (p=0.000 n=10+10)
Select/8 516ns ± 3% 515ns ± 3% ~ (p=0.858 n=10+9)
name old alloc/op new alloc/op delta
Select/1 96.0B ± 0% 64.0B ± 0% -33.33% (p=0.000 n=10+10)
Select/4 336B ± 0% 208B ± 0% -38.10% (p=0.000 n=10+10)
Select/8 672B ± 0% 672B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
Select/1 4.00 ± 0% 3.00 ± 0% -25.00% (p=0.000 n=10+10)
Select/4 7.00 ± 0% 6.00 ± 0% -14.29% (p=0.000 n=10+10)
Select/8 11.0 ± 0% 11.0 ± 0% ~ (all equal)
Change-Id: I1687e74fc8e86606a27f03fa8a561bcfb68775d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/230657
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Opening a connection with Connect should still create a derived
context with a timeout because some clients will not use a timeout
and the connection pool may open a connection asynchronously.
Likewise, if a connection close makes a network operation it should
provide some type of sane timeout for the operation.
Fixes#38185
Change-Id: I9b7ce2996c81c486170dcc84b12672a99610fa27
Reviewed-on: https://go-review.googlesource.com/c/go/+/230438
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
CL 220499 started marking readonly syms as SRODATA earlier,
so we can use that in the writebarrier pass now.
Passes toolstash-check.
Change-Id: Ic4d49714b8bffbe03c8e9a75ca96df4475bae732
Reviewed-on: https://go-review.googlesource.com/c/go/+/230559
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Remove the "ElfSym" and "LocalElfSym" fields from sym.Symbol,
replacing uses with loader method calls as needed.
Change-Id: I3828f13203ece2bdc03eeb09ab37a5c94e21a726
Reviewed-on: https://go-review.googlesource.com/c/go/+/230462
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Remove sym.Symbol 'Unit' field, replacing accesses to the field with
calls into the loader instead.
Change-Id: Ia1abd4c3d93036705dd624a49cb3d9cbe6a5188b
Reviewed-on: https://go-review.googlesource.com/c/go/+/230307
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Add new get/set methods to the loader for recording the ELF symbol
index for a given loader symbol. These are map-based, since it is
expected that many/most symbols will not need an ELF symbol table
entry.
Change-Id: I1102c3637775515ccc6650118e8b059468a2c3ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/230461
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove the 'Gotype' field from sym.Symbol, as it is now no longer
used. Store the loader.Sym for a symbol as a field in sym.Symbol
("SymIdx"). Then remove sym.Symbol 'File' field, and replace the field
accesses in question with calls into the loader instead.
Change-Id: I01c5504425006b8d3fe77fac2b69a86e198c7a5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230304
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
None of the users of genasmsym are doing anything with the Gotype
field of sym.Symbol, so remove that param from the callback function.
Change-Id: Ie902c4cdbcc6b68d353daf5ce21a99012161a946
Reviewed-on: https://go-review.googlesource.com/c/go/+/230545
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The span set data structure may leak blocks due to a race in the logic
to check whether it's safe to free a block. The simplest example of this
race is between two poppers:
1. Popper A claims slot spanSetEntries-2.
2. Popper B claims slot spanSetEntries-1.
3. Popper A gets descheduled before it subtracts from block.used.
4. Popper B subtracts from block.used, sees that claimed
spanSetEntries-1, but also that block.used != 0, so it returns.
5. Popper A comes back and subtracts from block.used, but it didn't
claim spanSetEntries-1 so it also returns.
The spine is left with a stale block pointer and the block later gets
overwritten by pushes, never to be re-used again.
The problem here is that we designate the claimer of slot
spanSetEntries-1 to be the one who frees the block, but that may not be
the thread that actually does the last subtraction from block.used.
Fixing this problem is tricky, and the fundamental problem there is that
block.used is not stable: it may be observed to be zero, but that
doesn't necessarily mean you're the last popper!
Do something simpler: keep a counter of how many pops have happened to a
given block instead of block.used. This counter monotonically increases
when a pop is _completely done_. Because this counter is monotonically
increasing, and only increases when a popper is done, then we know for
sure whichever popper is the last to increase it (i.e. its value is
spanSetBlockEntries) is also the last popper in the block. Because the
race described above still exists, the last popper may not be the one
which claimed the last slot in the block, but we know for certain nobody
else is popping from that block anymore so we can safely free it.
Finally, because pops serialize with pushes to the same slot, we need
not worry about concurrent pushers at all.
Updates #37487.
Change-Id: I6697219372774c8ca7d8ee6895eaa230a64ce9e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make Wasm more like other architectures, writing data sections to
heap in Asmb instead of Asmb2. Then we can remove the
copy-on-write logic in applying relocations.
Change-Id: I26d5315ea9fba032fe4bdb9b5c7fe483611c4373
Reviewed-on: https://go-review.googlesource.com/c/go/+/230465
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently mcaches are flushed to mcentral after a bunch of memstats have
already been read. This is not safe (in the sense that it doesn't ensure
consisent memstats) since memstats may in general change when mcentral
data structures are manipulated.
Note that prior to the new mcentral implementation this was not a
problem because mcentral operations happened to never modify certain
memstats. As of the new mcentral implementation, we might for example
persistentalloc when uncaching a span, which would change memstats. This
can cause a skew between the value of sys (which currently is calculated
before mcaches are flushed) and the value of gc_sys and other_sys.
Fix this by moving mcache flushing to the very top of updatememstats.
Also leave a comment explaining that this must be done first, in
general, because mcentrals make no guarantee that they will not
influence memstats (and doing so would be unnecessarily restrictive).
Fixes#38712.
Change-Id: I15bacb313c54a46e380a945a71bb75db67169c1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/230498
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
This change is just the mechanical work of moving the wavefront past
address.
Change-Id: I519ec49fa8ba50760c7d23fc084fcd3bb0544546
Reviewed-on: https://go-review.googlesource.com/c/go/+/229700
Run-TryBot: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a second attempt at CL 230024, with
cmd/go/testdata/script/mod_retention.txt updated to perform a
version-independent comparison on the 'go' version added to a go.mod
file that lacks one.
Fixes#38708
Change-Id: I15dcd83b51ed5ec57946b419bcbaec41e85a46f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/230382
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Robert Griesemer
Keith Randall
Dmitri Shuralyov
Ian Lance Taylor
Baokun Lee
Matthew Dempsky
Gawen Arab
Rodolfo Carvalho
Michael Munday
Bryan C. Mills
Roland Shoemaker
Brad Fitzpatrick
Vee Zhang
eric fang
Xiangdong Ji
Russ Cox
Austin Clements
Filippo Valsorda
Tobias Klauser
Katie Hockman
joshuabezaleel
David Chase
Cherry Zhang
Than McIntosh
fanzha02
Andrew G. Morgan
Jean de Klerk
David Golden
Dan Scales
Richard Miller
Jay Conrod
Daniel Nephin
Richard Musiol
Elias Naur
Yasser Abdolmaleki
Paschalis Tsilias
Brad Erickson
Michael Pratt
Alberto Donizetti
Volker Dobler
Daniel Martí
Paul D. Weber
Joel Sing
matsuyoshi
Michael Anthony Knyszek
Yasuhiro Matsumoto
Michael Hudson-Doyle
Roger Peppe
Josh Bleecher Snyder
smasher164
Lynn Boger
Hajime Hoshi
Steven Hartland
Povilas Versockas
Hana (Hyang-Ah) Kim
Ayan George
Emmanuel T Odeke
pj
Kevin Burke
Evgeniy Kulikov
Martin Möhrmann
Andrew Ekstedt
徐志强
Aaron Patterson
Meng Zhuo
Shengyu Zhang
Michael Matloob
Agniva De Sarker
Cuong Manh Le
Alex Gaynor
Michał Łowicki
geedchin
Kezhu Wang
Alex Brainman
kakulisen
Daniel Kumor
Mark Ryan
Gerrit Code Review
cncal
Brian Kessler
Anton Kuklin
Ruixin Bao
Keyan Pishdadian
Rob Pike
Jeremy Faller
Nigel Tao
Ian Gudger
Daniel Theophanes