cmd/go/internal/robustio: extend filesystem workarounds to darwin platforms
The macOS filesystem seems to have gotten significantly flakier as of
macOS 10.14, so this causes frequently flakes in the 10.14 builders.
We have no reason to believe that it will be fixed any time soon, so
rather than trying to detect the specific macOS version, we'll apply
the same workarounds that we use on Windows: classifying (and
retrying) the errors known to indicate flakiness and relaxing the
success criteria for renameio.TestConcurrentReadsAndWrites.
Fixes #33041
Change-Id: I74d8c15677951d7a0df0d4ebf6ea03e43eebddf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197517
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Jeremy Faller [Thu, 29 Aug 2019 20:35:50 +0000 (16:35 -0400)]
cmd/compile: remove isStmt symbol from FuncInfo
As promised in CL 188238, removing the obsolete symbol.
Here are the latest stats. This is baselined at "e53edafb66" with only
these changes applied, run on magna.cam. The linker looks straight
better (in memory and speed).
There is still a change I'm working on walking the progs to generate the
debug_lines data in the compiler. That will likely result in a compiler
speedup.
Michael Munday [Fri, 13 Sep 2019 12:28:49 +0000 (13:28 +0100)]
cmd/compile: use numeric condition code masks on s390x
Prior to this CL conditional branches on s390x always used an
extended mnemonic such as BNE, BLT and so on to represent branch
instructions with different condition code masks. This CL adds
support for numeric condition code masks to the s390x SSA backend
so that we can encode the condition under which a Block's
successor is chosen as a field in that Block rather than in its
type.
This change will be useful as we come to add support for combined
compare-and-branch instructions. Rather than trying to add extended
mnemonics for every possible combination of mask and compare-and-
branch instruction we can instead use a single mnemonic for each
instruction.
Change-Id: Idb7458f187b50906877d683695c291dff5279553
Reviewed-on: https://go-review.googlesource.com/c/go/+/197178 Reviewed-by: Keith Randall <khr@golang.org>
internal/poll: make SendFile work with large files on Windows
CL 192518 was a minimal simplification to get sendfile
on Windows to work with chunked files, but as I had mentioned,
I would add even more improvements.
This CL improves it by:
* If the reader is not an *io.LimitedReader, since the underlying
reader is anyways an *os.File, we fallback and stat that
file to determine the file size and then also invoke the chunked
sendFile on the underlying reader. This issue existed even
before the prior CL.
* Extracting the chunked TransmitFile logic and moving it directly
into internal/poll.SendFile.
Thus if the callers of net.sendFile don't use *io.LimitedReader,
but have a huge file (>2GiB), we can still invoke the chunked
internal/poll.SendFile on it directly.
The test case is not included in this patch as it requires
creating a 3GiB file, but that if anyone wants to view it, they
can find it at
https://go-review.googlesource.com/c/go/+/194218/13/src/net/sendfile_windows_test.go
Dan Scales [Tue, 24 Sep 2019 00:46:38 +0000 (17:46 -0700)]
misc, runtime, test: extra tests and benchmarks for defer
Add a bunch of extra tests and benchmarks for defer, in preparation for new
low-cost (open-coded) implementation of defers (see #34481),
- New file defer_test.go that tests a bunch more unusual defer scenarios,
including things that might have problems for open-coded defers.
- Additions to callers_test.go actually verifying what the stack trace looks like
for various panic or panic-recover scenarios.
- Additions to crash_test.go testing several more crash scenarios involving
recursive panics.
- New benchmark in runtime_test.go measuring speed of panic-recover
- New CGo benchmark in cgo_test.go calling from Go to C back to Go that
shows defer overhead
Updates #34481
Change-Id: I423523f3e05fc0229d4277dd00073289a5526188
Reviewed-on: https://go-review.googlesource.com/c/go/+/197017
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Michael Munday [Thu, 15 Aug 2019 19:43:46 +0000 (20:43 +0100)]
cmd/asm: add masked branch and conditional load instructions to s390x
The branch-relative-on-condition (BRC) instruction allows us to use
an immediate to specify under what conditions the branch is taken.
For example, `BRC $7, L1` is equivalent to `BNE L1`. It is sometimes
useful to specify branches in this way when either we don't have
an extended mnemonic for a particular mask value or we want to
generate the condition code mask programmatically.
The new load-on-condition (LOCR and LOCGR) and compare-and-branch
(CRJ, CGRJ, CLRJ, CLGRJ, CIJ, CGIJ, CLIJ and CLGIJ) instructions
provide the same flexibility for conditional loads and combined
compare and branch instructions.
Michael Anthony Knyszek [Tue, 25 Jun 2019 19:06:57 +0000 (19:06 +0000)]
runtime: scavenge on growth instead of inline with allocation
Inline scavenging causes significant performance regressions in tail
latency for k8s and has relatively little benefit for RSS footprint.
We disabled inline scavenging in Go 1.12.5 (CL 174102) as well, but
we thought other changes in Go 1.13 had mitigated the issues with
inline scavenging. Apparently we were wrong.
This CL switches back to only doing foreground scavenging on heap
growth, rather than doing it when allocation tries to allocate from
scavenged space.
Fixes #32828.
Change-Id: I1f5df44046091f0b4f89fec73c2cde98bf9448cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/183857
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Austin Clements [Mon, 12 Aug 2019 18:54:28 +0000 (14:54 -0400)]
runtime: grow the heap incrementally
Currently, we map and grow the heap a whole arena (64MB) at a time.
Unfortunately, in order to fix #32828, we need to switch from
scavenging inline with allocation back to scavenging on heap growth,
but heap-growth scavenging happens in large jumps because we grow the
heap in large jumps.
In order to prepare for better heap-growth scavenging, this CL
separates mapping more space for the heap from actually "growing" it
(tracking the new space with spans). Instead, growing the heap keeps
track of the "current arena" it's growing into. It track that with new
spans as needed, and only maps more arena space when the current arena
is inadequate. The effect to the user is the same, but this will let
us scavenge on much smaller increments of heap growth.
There are two slightly subtleties to this change:
1. If an allocation requires mapping a new arena and that new arena
isn't contiguous with the current arena, we don't want to lose the
unused space in the current arena, so we have to immediately track
that with a span.
2. The mapped space must be accounted as released and idle, even
though it isn't actually tracked in a span.
For #32828, since this makes heap-growth scavenging far more
effective, especially at small heap sizes. For example, this change is
necessary for TestPhysicalMemoryUtilization to pass once we remove
inline scavenging.
Change-Id: I300e74a0534062467e4ce91cdc3508e5ef9aa73a
Reviewed-on: https://go-review.googlesource.com/c/go/+/189957
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 3 Sep 2019 19:54:32 +0000 (19:54 +0000)]
runtime: redefine scavenge goal in terms of heap_inuse
This change makes it so that the scavenge goal is defined primarily in
terms of heap_inuse at the end of the last GC rather than next_gc. The
reason behind this change is that next_gc doesn't take into account
fragmentation, and we can fall into situation where the scavenger thinks
it should have work to do but there's no free and unscavenged memory
available.
In order to ensure the scavenge goal still tracks next_gc, we multiply
heap_inuse by the ratio between the current heap goal and the last heap
goal, which describes whether the heap is growing or shrinking, and by
how much.
Finally, this change updates the documentation for scavenging and
elaborates on why the scavenge goal is defined the way it is.
Fixes #34048.
Updates #32828.
Change-Id: I8deaf87620b5dc12a40ab8a90bf27932868610da
Reviewed-on: https://go-review.googlesource.com/c/go/+/193040
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Keith Randall <khr@golang.org>
I'm also not sure why it ever worked; it looks like it is writing
the arguments to racecallback in the wrong place (the race detector
itself probably still works, it would just have trouble symbolizing
any resulting race report).
At a meta-level, we should really add a ppc64le/race builder.
Otherwise this code will rot, as evidenced by the rot this CL fixes :)
When the -json flag is passed to go mod download,
the sumdb error is embedded in the json Error field.
Other errors for the same command behave this way as
well such as module not found. The fix is done by changing
base.Fatalf into proper error returns.
Fixes #34485
Change-Id: I2727a5c70c7ab03988cad8661894d0f8ec71a768
Reviewed-on: https://go-review.googlesource.com/c/go/+/197062
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Matthew Dempsky [Fri, 20 Sep 2019 22:31:13 +0000 (15:31 -0700)]
cmd/compile: optimize escape graph construction and walking
This CL implements several optimizations for the escape analysis flow
graph:
1. Instead of recognizing heapLoc specially within Escape.outlives,
set heapLoc.escapes = true and recognize any location with escapes
set. This allows us to skip adding edges from the heap to escaped
variables in two cases:
1a. In newLoc, if the location is for a variable or allocation too
large to fit on the stack.
1b. During walkOne, if we discover that an object's address flows
somewhere that naturally outlives it.
2. When recording edges in Escape.flow, if x escapes and we're adding
an edge like "x = &y", we can simply mark that y escapes too.
3. During walkOne, if we reach a location that's marked as escaping,
we can skip visiting it again: we've either already walked from it, or
it's in queue to be walked from again.
On average, reduces the number of visited locations by 15%. Reduces
time spent in escape analysis for particularly hairy packages like
runtime and gc by about 8%. Reduces escape.go's TODO count by 22%.
Matthew Dempsky [Fri, 20 Sep 2019 22:27:14 +0000 (15:27 -0700)]
cmd/compile: use proper work queue for escape graph walking
The old escape analysis code used to repeatedly walk the entire flow
graph until it reached a fixed point. With escape.go, I wanted to
avoid this if possible, so I structured the walking code with two
constraints:
1. Always walk from the heap location last.
2. If an object escapes, ensure it has flow edge to the heap location.
This works, but it precludes some graph construction
optimizations. E.g., if there's an assignment "heap = &x", then we can
immediately tell that 'x' escapes without needing to visit it during
the graph walk. Similarly, if there's a later assignment "x = &y", we
could immediately tell that 'y' escapes too. However, the natural way
to implement this optimization ends up violating the constraints
above.
Further, the constraints above don't guarantee that the 'transient'
flag is handled correctly. Today I think that's handled correctly
because of the order that locations happen to be constructed and
visited based on the AST, but I've felt uneasy about it for a little
while.
This CL changes walkAll to use a proper work queue (technically a work
stack) to track locations that need to be visited, and allows walkOne
to request that a location be re-visited.
Jeremy Faller [Thu, 22 Aug 2019 16:18:28 +0000 (12:18 -0400)]
cmd/compile: update object file format for DWARF file table
In CL 188317, we generate the debug_lines in the compiler, and created a
new symbol to hold the line table. Here we modify the object file format
to output the file table.
Matthew Dempsky [Wed, 25 Sep 2019 06:56:50 +0000 (23:56 -0700)]
cmd/compile: use underlying OCOMPLIT's position for OPTRLIT
Currently, when we create an OPTRLIT node, it defaults to the
OCOMPLIT's final element's position. But it improves error messages to
use the OCOMPLIT's own position instead.
Matthew Dempsky [Wed, 25 Sep 2019 07:14:58 +0000 (00:14 -0700)]
cmd/compile: remove -s flag
This is better handled by tools like cmd/gofmt, which can
automatically rewrite the source code and already supports a syntactic
version of this simplification. (go/types can be used if
type-sensitive simplification is actually necessary.)
Daniel Martí [Tue, 24 Sep 2019 12:11:36 +0000 (13:11 +0100)]
text/template: don't evaluate '.' as a float64
When using a '.' constant literal as a reflect.Value variadic argument,
idealConstant would incorrectly result in a float64. This is because
rune literals can be represented as a float64, and contain a period,
which tricked the logic into thinking the literal must have been a
floating point number.
This also happened with other characters that can be part of a floating
point number, such as 'e' or 'P'.
To fix these edge cases, exit the case sooner if the literal was a rune,
since that should always go to the int case instead.
Finally, add test cases that verify that they behave properly. These
would error before, since eq would receive a mix of int and float64,
which aren't comparable.
Fixes #34483.
Change-Id: Icfcb7803bfa0cf317a1d1adacacad3d69a57eb42
Reviewed-on: https://go-review.googlesource.com/c/go/+/196808
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tom Payne <tom@airmap.com> Reviewed-by: Rob Pike <r@golang.org>
In CL 192980, I tend to think that canSSAType can be used as replacement
for isfat. It is not the truth as @khr points me out that isfat has very
different purpose.
So this CL adds documentation for isfat, also remove outdated TODO.
Change-Id: I15954d638759bd9f6b28a6aa04c1a51129d9ae7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196499
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Jeremy Faller [Wed, 31 Jul 2019 14:33:11 +0000 (10:33 -0400)]
cmd/compile: generate debug_lines in compiler
This is mostly a copy-paste jobs from the linker to generate the debug
information in the compiler instead of the linker. The new data is
inserted into the debug line numbers symbol defined in CL 188238.
Generating the debug information BEFORE deadcode results in one subtle
difference, and that is that the state machine needs to be reset at the
end of every function's debug line table. The reasoning is that
generating the table AFTER dead code allows the producer and consumer of
the table to agree on the state of the state machine, and since these
blocks will (eventually) be concatenated in the linker, we don't KNOW
the state of the state machine unless we reset it. So,
generateDebugLinesSymbol resets the state machine at the end of every
function.
Right now, we don't do anything with this line information, or the file
table -- we just populate the symbols.
Jeremy Faller [Fri, 9 Aug 2019 15:36:03 +0000 (11:36 -0400)]
cmd/link: add notion of multiple compilation units per package
As we move the debug_line generation into the compiler, we need to
upgrade the notion of compilationUnit to not just be on a per package
basis. That won't be the case as it will be impossible for all
compilationUnits to have the same set of files names used to build the
debug_lines table. (For example, assembled files in a package don't know
about any files but themselves, so the debug_lines table could only
reference themseves. As such, we need to break the 1:1 relationship
between compUnit and package.)
cmd/go: suppress errors in package-to-module queries if the package is already found
In CL 173017, I changed the package-to-module query logic to query all
possible module paths in parallel in order to reduce latency. (For
long package paths, most such paths will not exist and will fail with
little overhead.)
The module resolution algorithm treats various kinds of non-existence
as “soft errors”, to be reported only if package resolution fails, but
treats any remaining errors as hard errors that should fail the query.
Unfortunately, that interacted badly with the +incompatible version
validation added in CL 181881, causing a regression in the 'direct'
fetch path for modules using the “major branch” layout¹ with a post-v1
version on the repository's default branch. Because we did not
interpret a mismatched module path as “no such module”, a go.mod file
specifying the path 'example.com/foo/v2' would cause the search for
module 'example.com/foo' to error out. (That regression was not caught
ahead of time due to a lack of test coverage for 'go get' on a package
within a /vN module.)
The promotion of hard errors during parallel search also made the 'go'
command less tolerant of servers that advertise 'go-import' tags for
nonexistent repositories. CL 194561 mitigated that problem for HTTP
servers that return code 404 or 410 for a nonexistent repository, but
unfortunately a few servers in common use (notably GitLab and
pre-1.9.3 releases of Gitea) do not.
This change mitigates both of those failure modes by ignoring
“miscellaneous” errors from shorter module paths if the requested
package pattern was successfully matched against a module with a
longer path.
Martin Möhrmann [Mon, 9 Sep 2019 15:50:35 +0000 (17:50 +0200)]
compile: prefer an AND instead of SHR+SHL instructions
On modern 64bit CPUs a SHR, SHL or AND instruction take 1 cycle to execute.
A pair of shifts that operate on the same register will take 2 cycles
and needs to wait for the input register value to be available.
Large constants used to mask the high bits of a register with an AND
instruction can not be encoded as an immediate in the AND instruction
on amd64 and therefore need to be loaded into a register with a MOV
instruction.
However that MOV instruction is not dependent on the output register and
on many CPUs does not compete with the AND or shift instructions for
execution ports.
Using a pair of shifts to mask high bits instead of an AND to mask high
bits of a register has a shorter encoding and uses one less general
purpose register but is slower due to taking one clock cycle longer
if there is no register pressure that would make the AND variant need to
generate a spill.
For example the instructions emitted for (x & 1 << 63) before this CL are: 48c1ea3f SHRQ $0x3f, DX 48c1e23f SHLQ $0x3f, DX
after this CL the instructions are the same as GCC and LLVM use: 48b80000000000000080 MOVQ $0x8000000000000000, AX
4821d0 ANDQ DX, AX
Some platforms such as arm64 already have SSA optimization rules to fuse
two shift instructions back into an AND.
Removing the general rule to rewrite AND to SHR+SHL speeds up this benchmark:
var GlobalU uint
func BenchmarkAndHighBits(b *testing.B) {
x := uint(0)
for i := 0; i < b.N; i++ {
x &= 1 << 63
}
GlobalU = x
}
amd64/darwin on Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz:
name old time/op new time/op delta
AndHighBits-4 0.61ns ± 6% 0.42ns ± 6% -31.42% (p=0.000 n=25+25):
'go run run.go -all_codegen -v codegen' passes with following adjustments:
ARM64: The BFXIL pattern ((x << lc) >> rc | y & ac) needed adjustment
since ORshiftRL generation fusing '>> rc' and '|' interferes
with matching ((x << lc) >> rc) to generate UBFX. Previously
ORshiftLL was created first using the shifts generated for (y & ac).
S390X: Add rules for abs and copysign to match use of AND instead of SHIFTs.
cmd/go/internal/modfetch/codehost: work around an apparent bug in 'git fetch --unshallow'
When 'git fetch' is passed the '--unshallow' flag, it assumes that the
local and remote refs are equal.¹ However, we were fetching an
expanded set of refs explicitly in the same command, violating that
assumption.
Now we first expand the set of refs, then unshallow the repo in a
separate fetch. Empirically, this seems to work, whereas the opposite
order does not.
Eli Bendersky [Thu, 1 Aug 2019 22:39:40 +0000 (15:39 -0700)]
cmd/gofmt: fix computation of function header size
Function sizes are computed to determine whether a function
can be kept on one line or should be split to several lines. Part of the
computation is the function header from the FUNC token and until the
opening { token.
Prior to this change, the function header size used distance from the
original source position of the current token, which led to issues when
the source between FUNC and the original source position was rewritten
(such as whitespace being collapsed). Now we take the current output
position into account, so that header size represents the reformatted
source rather than the original source.
The following files in the Go repository are reformatted with this
change:
In both cases the reformatting is minor and seems to be correct given
the heuristic to single-line functions longer than 100 columns to
multiple lines.
Gregory Man [Sun, 22 Sep 2019 10:30:45 +0000 (13:30 +0300)]
cmd/go: allow -I= and -I$SYSROOT in cgo CFLAGS
Current checkFlags() didn't allow any not safe charactars in arguments.
In GCC "=" in arguments will be replaced with sysroot prefix, and used
by users to work with different SDK versions.
This CL allow to use "=" and $SYSROOT with -I argument.
Fixes #34449
Change-Id: I3d8b2b9d13251e454ea18e9d34a94b87c373c7b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/196783
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Daniel Martí [Sat, 29 Jun 2019 15:57:16 +0000 (17:57 +0200)]
go/printer: never print a newline before the returned results
Otherwise, if one ends up with a "return result" where the two nodes are
in separate lines, the printer would incorrectly print a naked return:
return
result
The fix is simple - by not telling exprList what the previous position
is, it never adds a leading linebreak. This is the same mechanism used
for identifier lists and values, so it seems appropriate.
All other exprList calls that can produce a leading linebreak don't seem
buggy, because closing tokens such as parentheses and colons are needed
to finish the statement.
Verified that the test failed before the patch as well:
Matthew Dempsky [Thu, 19 Sep 2019 23:55:56 +0000 (16:55 -0700)]
cmd/compile: clean up escape graph construction
OTYPESW and ORANGE were manually creating locations and flows around
them, which are relatively low-level graph construction primitives.
This CL changes them to use holes like the rest of the code.
Also, introduce "later" as an abstraction for assignment flows that
don't happen right away, and which need to prevent expressions from
being marked as "transient" (e.g., in ODEFER and ORANGE).
There's no behavior change here, but this does reduce the number of
newLoc call sites, which should help with restoring -m=2 diagnostics.
Jeremy Faller [Tue, 30 Jul 2019 21:48:11 +0000 (17:48 -0400)]
cmd/compile: add new symbol for debug line numbers
This is broken out from: CL 187117
This new symbol will be populated by the compiler and contain debug line
information that's currently generated in the linker. One might say it's
sad to create a new symbol, but this symbol will replace the isStmt
symbols.
This fixes a regression introduced with CL 192937. That change
was intended to fix a problem in arm and arm64 but also added
code to change the behavior in ppc64 and ppc64le even though the
error never occurred there. The change to function sigFetchG
assumes that the register holding 'g' could be clobbered by
vdso code when in fact 'g' is in R30 and that is nonvolatile
in the 64-bit PowerPC ELF ABI so would not be clobbered in vdso code.
So if this happens somehow the path it takes is incorrect,
falling through to a call to badsignal which doesn't seem right.
This regression caused intermittent hangs on the builder dashboard
for ppc64, and can be reproduced consistently when running os/signal
TestStress on some ppc64 systems.
I mentioned this problem is issue #34391 because I thought it was
related to another problem described there.
net: use case-insensitive host string comparison in TestLookupGmailNS
Some nameservers alter the case of NS records they return, e.g.
ns2.google.COm. or ns2.google.coM. Change TestLookupGmailNS to account
for this possibility by comparing host names in lower case.
Keith Randall [Mon, 16 Sep 2019 21:38:12 +0000 (14:38 -0700)]
runtime: allow the Go runtime to return multiple stack frames for a single PC
Upgrade the thread sanitizer to handle mid-stack inlining correctly.
We can now return multiple stack frames for each pc that the thread sanitizer
gives us to symbolize.
To fix #33309, we still need to modify the tsan library with its portion
of this fix, rebuild the .syso files on all supported archs, and check
them into runtime/race.
Daniel Martí [Thu, 19 Sep 2019 14:31:03 +0000 (15:31 +0100)]
cmd/compile: reduce rulegen's output by 200 KiB
First, renove unnecessary "// cond:" lines from the generated files.
This shaves off about ~7k lines.
Second, join "if cond { break }" statements via "||", which allows us to
deduplicate a large number of them. This shaves off another ~25k lines.
This change is not for readability or simplicity; but rather, to avoid
unnecessary verbosity that makes the generated files larger. All in all,
git reports that the generated files overall weigh ~200KiB less, or
about 2.7% less.
While at it, add a -trace flag to rulegen.
Updates #33644.
Change-Id: I3fac0290a6066070cc62400bf970a4ae0929470a
Reviewed-on: https://go-review.googlesource.com/c/go/+/196498
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Andrew Medvedev [Sat, 21 Sep 2019 17:18:47 +0000 (20:18 +0300)]
crypto/x509: give type hint in error message in marshalPublicKey
Currently if type of public key is unsupported, error message is "only
RSA and ECDSA public keys supported". After adding Ed25519 this message
is no longer correct.
Moreover, it is superfluous because documentation for
MarshalPKIXPublicKey, CreateCertificateRequest and CreateCertificate
already lists supported public key types.
This CL removes unnecessary details from error message.
It also adds reporting the type of unsupported key, which helps
debugging cases when struct (instead of a pointer) to otherwise correct
public key is given.
net: close correct file descriptor when netpoll registration fails
In (*netFD).accept, if initializing the *netFD associated with the
new connection fails, the listen FD is closed, rather than the FD
associated with the new connection. Close the correct FD instead.
Fixes #34392
Change-Id: I7bf3469d661e6d30cbd4b12f5f5fd330a81a541b
Reviewed-on: https://go-review.googlesource.com/c/go/+/196778
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 19 Sep 2019 21:58:10 +0000 (14:58 -0700)]
go/types: don't clone interface methods when embedding them
https://golang.org/cl/191257 significantly changed (and simplified)
the computation of interface method sets with embedded interfaces.
Specifically, when adding methods from an embedded interface, those
method objects (Func Objects) were cloned so that they could have a
different source position (the embedding position rather than the
original method position) for better error messages.
This causes problems for code that depends on the identity of method
objects that represent the same method, embedded or not.
This CL avoids the cloning. Instead, while computing the method set
of an interface, a position map is carried along that tracks
embedding positions. The map is not needed anymore after type-
checking.
Updates #34421.
Change-Id: I8ce188136c76fa70fba686711167db29a049f46d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196561 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Andrew Medvedev [Sat, 21 Sep 2019 12:47:34 +0000 (12:47 +0000)]
strings, bytes: clarify usage of EqualFolds
This clarifies meaning of "case folding" Unicode equality with more familiar "case insensitive" wording.
For case folding properties see ftp://ftp.unicode.org/Public/UNIDATA/CaseFolding.txt.
Martin Möhrmann [Mon, 9 Sep 2019 15:50:35 +0000 (17:50 +0200)]
compile: prefer an AND instead of SHR+SHL instructions
On modern 64bit CPUs a SHR, SHL or AND instruction take 1 cycle to execute.
A pair of shifts that operate on the same register will take 2 cycles
and needs to wait for the input register value to be available.
Large constants used to mask the high bits of a register with an AND
instruction can not be encoded as an immediate in the AND instruction
on amd64 and therefore need to be loaded into a register with a MOV
instruction.
However that MOV instruction is not dependent on the output register and
on many CPUs does not compete with the AND or shift instructions for
execution ports.
Using a pair of shifts to mask high bits instead of an AND to mask high
bits of a register has a shorter encoding and uses one less general
purpose register but is slower due to taking one clock cycle longer
if there is no register pressure that would make the AND variant need to
generate a spill.
For example the instructions emitted for (x & 1 << 63) before this CL are: 48c1ea3f SHRQ $0x3f, DX 48c1e23f SHLQ $0x3f, DX
after this CL the instructions are the same as GCC and LLVM use: 48b80000000000000080 MOVQ $0x8000000000000000, AX
4821d0 ANDQ DX, AX
Some platforms such as arm64 already have SSA optimization rules to fuse
two shift instructions back into an AND.
Removing the general rule to rewrite AND to SHR+SHL speeds up this benchmark:
var GlobalU uint
func BenchmarkAndHighBits(b *testing.B) {
x := uint(0)
for i := 0; i < b.N; i++ {
x &= 1 << 63
}
GlobalU = x
}
amd64/darwin on Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz:
name old time/op new time/op delta
AndHighBits-4 0.61ns ± 6% 0.42ns ± 6% -31.42% (p=0.000 n=25+25):
'go run run.go -all_codegen -v codegen' passes with following adjustments:
ARM64: The BFXIL pattern ((x << lc) >> rc | y & ac) needed adjustment
since ORshiftRL generation fusing '>> rc' and '|' interferes
with matching ((x << lc) >> rc) to generate UBFX. Previously
ORshiftLL was created first using the shifts generated for (y & ac).
S390X: Add rules for abs and copysign to match use of AND instead of SHIFTs.
cmd/compile: optimize ssa if blocks for wasm architecture
Check for the next block and accordingly place the successor blocks.
This saves an additional jump instruction if the next block is any one
of the successor blocks.
While at it, inline the logic of goToBlock.
Reduces the size of pkg/js_wasm by 264 bytes.
Change-Id: I671ac4322e6edcb0d7e590dcca27e074268068d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/195204
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Richard Musiol <neelance@gmail.com>
(*buf).string previously manually searched
through its underlying byte slice until we
encountered a '0'. This change instead uses
bytes.IndexByte that results in a speed up:
$ benchstat before.txt after.txt
name old time/op new time/op delta
BufString-8 257ns ± 1% 174ns ± 1% -32.37% (p=0.000 n=9+8)
name old speed new speed delta
BufString-8 495MB/s ± 1% 732MB/s ± 1% +47.76% (p=0.000 n=10+8)
name old alloc/op new alloc/op delta
BufString-8 162B ± 0% 162B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
BufString-8 3.00 ± 0% 3.00 ± 0% ~ (all equal)
Ian Lance Taylor [Mon, 16 Sep 2019 19:35:15 +0000 (12:35 -0700)]
runtime: avoid overflow in markrootBlock
In a position independent executable the data or BSS may be located
close to the end of memory. If it is placed closer than
rootBlockBytes, then the calculations in markrootBlock would overflow,
and the test that ensures that n is not larger than n0 would fail.
This would then cause scanblock to scan data that it shouldn't,
using an effectively random ptrmask, leading to program crashes.
No test because the only way to test it is to build a PIE and convince
the kernel to put the data section near the end of memory, and I don't
know how to do that. Or perhaps we could use a linker script, but that
is painful.
The new code is algebraically identical to the original code, but
avoids the potential overflow of b+rootBlockBytes.
Change-Id: Ieb4e5465174bb762b063d2491caeaa745017345e
Reviewed-on: https://go-review.googlesource.com/c/go/+/195717
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Jay Conrod [Thu, 19 Sep 2019 16:58:02 +0000 (12:58 -0400)]
cmd/go: don't construct module version info if there are import errors
A precondition of modload.PackageBuildInfo is that its path and deps
arguments correspond to paths that have been loaded successfully with
modload.ImportPaths or one of the Load functions. load.Package.load
should not call PackageBuildInfo if there were any errors resolving
imports.
Fixes #34393
Change-Id: I107514f1c535885330ff266c85d3981b71b31c2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196520
Run-TryBot: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Richard Musiol [Thu, 12 Sep 2019 19:05:45 +0000 (21:05 +0200)]
cmd/compile: add 32 bit float registers/variables on wasm
Before this change, wasm only used float variables with a size of 64 bit
and applied rounding to 32 bit precision where necessary. This change
adds proper 32 bit float variables.
cmd/go/internal/modload: use a structured error for 'ambiguous import'
This consolidates the construction of 'ambiguous import' errors to a
single location, ensuring consistency, and lays the groundwork for
automatic resolution in the future.
While we're at it, change "found" to "found package" to try to make
the cause of the error clearer.
Updates #32128
Updates #27899
Change-Id: I14a93593320e5c60d20b0eb686d0d5355763c30c
Reviewed-on: https://go-review.googlesource.com/c/go/+/196298
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
cmd/go: change the gccgo 'package m' regression test to run in GOPATH mode
This test is failing in the builders due to the deployed versions of
gccgo not supporting module mode. However, the bug reproduced in
GOPATH mode too, so that mode should be fine for a regression test.
Updates #34358
Change-Id: I954132a96849e80e8783d4de10389fcab7b14af2
Reviewed-on: https://go-review.googlesource.com/c/go/+/196518
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Than McIntosh [Wed, 18 Sep 2019 19:49:25 +0000 (15:49 -0400)]
cmd/link: enhance linker's dwarf test
Couple of changes to the linker's dwarf test, including:
- add some code to the DWARF tests inlining coverage to verify the
call_file attribute attached to inlined routine DIEs. If function
main.F is inlined into function main.G, we want to see that the
call_file attribute in the inlined routine DIE for main.F is the
same file as that reported for main.G.
- fix a glitch with the way the DW_AT_decl_file attribute was
being checked. The previous code relied on hard-coded indices
into the line table files table, which is very brittle (since
there is no requirement that files be ordered in any specific
way). Instead, add machinery to look up the actual file string
via the line table reader.
Change-Id: I44e71c69b6e676238cf4b805e7170de17b50939f
Reviewed-on: https://go-review.googlesource.com/c/go/+/196517
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org>
cmd/go/internal/mvs: recompute build list in Reqs before minimizing
modload.MinReqs was passing modload.buildList to mvs.Reqs explicitly,
apparently as an optimization. However, we do not always have the
invariant that modload.buildList is complete: in particular, 'go mod
tidy' begins by reducing modload.buildList to only the set of modules
that provide packages to the build, which may be substantially smaller
than the final build list.
Other operations, such as 'go mod graph', do not load the entire
import graph, and therefore call Reqs with the unreduced build list.
Since Reqs retains modules according to a post-order traversal of the
list, an incomplete list may produce a different traversal order — and
therefore a different minimal solution, when multiple minimal
solutions exist. That caused 'go mod tidy' to produce different output
from other 'go' subcommands when certain patterns of dependencies are
present.
Since passing in the build list is only an optimization anyway, remove
the parameter and recompute the actual (complete) list at the
beginning of mvs.Reqs itself. That way, it is guaranteed to be
complete and in canonical order.
Fixes #34086
Change-Id: I3101bb81a1853c4a5e773010da3e44d2d90a570c
Reviewed-on: https://go-review.googlesource.com/c/go/+/193397
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Jason A. Donenfeld [Wed, 18 Sep 2019 06:10:07 +0000 (00:10 -0600)]
syscall: avoid memory corruption in mksyscall_windows.go with *bool parameters
Windows type PBOOL is a pointer to a 4 byte value, where 0 means false
and not-0 means true. That means we should use uint32 here, not bool,
since Go bools can be 1 byte. Since a *bool is never a "real" valid
Windows type, converting on both in and out is probably sufficient,
since *bool shouldn't ever be used as something with significance for
its particular address.
Updates: #34364
Change-Id: I4c1b91cd9a39d91e23dae6f894b9a49f7fba2c0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/196122
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Gert Cuykens [Thu, 27 Jun 2019 01:04:21 +0000 (03:04 +0200)]
cmd/doc: add option to output a clean one-line symbol representation
Currently there is no way for go doc to output a clean
one-line symbol representation of types, functions, vars
and consts without documentation lines or other text lines
added.
For example `go doc fmt` has a huge introduction so if you
pass that to grep or fzf to search a symbol let say scan
`go doc fmt | grep scan` you get way to many false
positives.
Added a `-short` flag to be able to do
`go doc -short fmt | grep scan` instead which will result in
just the symbols you are looking for.
func Fscan(r io.Reader, a ...interface{}) (n int, err error)
func Fscanf(r io.Reader, format string, a ...interface{}) (n int, err error)
func Fscanln(r io.Reader, a ...interface{}) (n int, err error)
func Sscan(str string, a ...interface{}) (n int, err error)
func Sscanf(str string, format string, a ...interface{}) (n int, err error)
func Sscanln(str string, a ...interface{}) (n int, err error)
Fixes #32597
Change-Id: I77a73838adc512c8d1490f5a82075de6b0462a31
Reviewed-on: https://go-review.googlesource.com/c/go/+/184017
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
cmd/go/internal/modload: do not prune the module root when walking directories
When walking filesystem paths to locate packages, we normally prune
out subdirectories with names beginning with ".", "_", or equal to
"testdata". However, we should not prune out such a directory if it is
at or above the module root, since its name is not part of the package
path.
Fixes #28481
Updates #27852
Change-Id: Ice82b1f908afaab50f5592f6c38ca6a0fe911edf
Reviewed-on: https://go-review.googlesource.com/c/go/+/196297
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current msanwrite() segfaults during libpreinit
when built with -msan on arm64. The cause is msancall()
in runtime/msan_arm64.s called by msanwrite() assumes
that it is always called with a valid g, leading to a
segfult.
Liz Rice [Wed, 20 Jun 2018 19:33:37 +0000 (20:33 +0100)]
regexp: add more examples for Regexp methods
Since I first started on this CL, most of the methods have had examples
added by other folks, so this is now one new example, and additions to
two existing examples for extra clarity.
The issue has a comment about not necessarily having examples for all
methods, but I recall finding this package pretty confusing when I first
used it, and having concrete examples would have really helped me
navigate all the different options. There are more
String methods with examples now, but I think seeing how the byte-slice
methods work could also be helpful to explain the differences.
Updates #21450
Change-Id: I27b4eeb634fb8ab59f791c0961cce79a67889826
Reviewed-on: https://go-review.googlesource.com/c/go/+/120145 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jean de Klerk [Thu, 20 Jun 2019 00:02:09 +0000 (18:02 -0600)]
reflect: give type hints in error messages
Currently, if you call various reflect methods you might get a panic with a
message like, "reflect: Field of non-struct type". Sometimes it's easy to
grok what's going on, but other times you need to laboriously go perform
reflect.ValueOf(myType).Kind().
This CL just adds that detail to the error message, saving debuggers the
extra step and making the error message more clear.
Change-Id: I7e0c211a3001e6b217b828cbcf50518080b5cb1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/183097 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Wed, 18 Sep 2019 15:33:54 +0000 (16:33 +0100)]
cmd/compile: parallelize one more bit of rulegen
'go tool trace' pointed at an obvious inefficiency; roughly the first
fifth of the program's life was CPU-heavy and making use of only one CPU
core at a time.
This was due to genOp being run before genLower. We did make genLower
use goroutines to parallelize the work between architectures, but we
didn't make genOp run in parallel too.
Do that. To avoid having two layers of goroutines, simply fire off all
goroutines from the main function, and inline genLower, since it now
becomes just two lines of code.
Overall, this shaves another ~300ms from 'go run *.go' on my laptop.
name old time/op new time/op delta
Rulegen 2.04s ± 2% 1.76s ± 2% -13.93% (p=0.008 n=5+5)
name old user-time/op new user-time/op delta
Rulegen 9.04s ± 1% 9.25s ± 1% +2.37% (p=0.008 n=5+5)
name old sys-time/op new sys-time/op delta
Rulegen 235ms ±14% 245ms ±16% ~ (p=0.690 n=5+5)
name old peak-RSS-bytes new peak-RSS-bytes delta
Rulegen 179MB ± 1% 190MB ± 2% +6.21% (p=0.008 n=5+5)
Austin Clements [Wed, 21 Aug 2019 20:21:06 +0000 (16:21 -0400)]
debug/dwarf: expose file name table from line table reader
Currently, the line table reader keeps the file name table internal.
However, there are various attributes like AttrDeclFile and
AttrCallFile whose value is an index into this table. Hence, in order
to interpret these attributes, we need access to the file name table.
This CL adds a method to LineReader that exposes the file table of the
current compilation unit in order to allow consumers to interpret
attributes that index into this table.
Change-Id: I6b64b815f23b3b0695036ddabe1a67c3954867dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/192699
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Wed, 21 Aug 2019 19:05:26 +0000 (15:05 -0400)]
debug/dwarf: expose CU byte order
Currently, dwarf.Reader exposes the current compilation unit's address
size, but doesn't expose its byte order. Both are important for
decoding many attributes. For example, location descriptions include
addresses that are encoded in native form for the CU.
This CL exposes the byte order of the compilation unit in the same way
we already expose its address size, which makes it possible to decode
attributes containing native addresses.
Change-Id: I92f156818fe92b049d1dfc1613816bb1689cfadf
Reviewed-on: https://go-review.googlesource.com/c/go/+/192698
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tom Thorogood [Sat, 14 Sep 2019 01:16:20 +0000 (01:16 +0000)]
net/http: fix HTTP/2 idle pool tracing
CL 140357 caused HTTP/2 connections to be put in the idle pool, but
failed to properly guard the trace.GotConn call in getConn. dialConn
returns a minimal persistConn with conn == nil for HTTP/2 connections.
This persistConn was then returned from queueForIdleConn and caused the
httptrace.GotConnInfo passed into GotConn to have a nil Conn field.
HTTP/2 connections call GotConn themselves so leave it for HTTP/2 to call
GotConn as is done directly below.
Than McIntosh [Wed, 18 Sep 2019 18:07:55 +0000 (14:07 -0400)]
cmd/go: fix buglet in alternate gccgo debug_modinfo recipe
Fix bug in previous CL 171768 -- with Go 1.13 the proper entry point
to call is runtime.setmodinfo, not runtime..z2fdebug.setmodinfo (this
changed when we moved from 1.12). [ Unclear why trybots and runs of
all.bash didn't catch this, but hand testing made it apparent. ]
Updates #30344.
Change-Id: I91f47bd0c279ad2d84875051be582818b13735b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/196237
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Wed, 18 Sep 2019 16:13:46 +0000 (17:13 +0100)]
all: remove trailing whitespace from HTML files
I noticed lots of trailing whitespace in one of cmd/trace's HTML files.
While at it, remove a few others from still-maintained files. Leave old
documents alone, such as doc/devel/weekly.html.
Than McIntosh [Fri, 12 Apr 2019 13:47:43 +0000 (09:47 -0400)]
cmd/go: use alternate debug_modinfo recipe for gccgo
Use a different recipe for capturing debug modinfo if we're compiling
with the gccgo toolchain, to avoid applying a go:linkname directive to
a variable (not supported by gccgo).
Fixes #30344.
Change-Id: I9ce3d42c3bbb809fd68b140f56f9bbe3406c351b
Reviewed-on: https://go-review.googlesource.com/c/go/+/171768 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
cmd/asm,cmd/compile: clean up isel codegen on ppc64x
This cleans up the isel code generation in ssa for ppc64x.
Current there is no isel op and the isel code is only
generated from pseudo ops in ppc64/ssa.go, and only using
operands with values 0 or 1. When the isel is generated,
there is always a load of 1 into the temp register before it.
This change implements the isel op so it can be used in PPC64.rules,
and can recognize operand values other than 0 or 1. This also
eliminates the forced load of 1, so it will be loaded only if
needed.
This will make the isel code generation consistent with other ops,
and allow future rule changes that can take advantage of having
a more general purpose isel rule.
Change-Id: I363e1dbd3f7f5dfecb53187ad51cce409a8d1f8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/195057
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>