Alan Donovan [Thu, 8 Nov 2018 15:43:40 +0000 (10:43 -0500)]
cmd/go: vet: revert $GOVETTOOL env var, restore -vettool flag
The environment variable is no longer necessary as we now plan to
transition to the new vet by replacing it in a single step,
and we really don't want to add more environment variables.
Fixes #28636
Change-Id: Ib85e5c0d61213b7b9f6a53d9376fec29525df971
Reviewed-on: https://go-review.googlesource.com/c/148497
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Mon, 5 Nov 2018 03:23:08 +0000 (19:23 -0800)]
cmd/compile: encourage inlining of functions with single-call bodies
This is a simple tweak to allow a bit more mid-stack inlining.
In cases like this:
func f() {
g()
}
We'd really like to inline f into its callers. It can't hurt.
We implement this optimization by making calls a bit cheaper, enough
to afford a single call in the function body, but not 2.
The remaining budget allows for some argument modification, or perhaps
a wrapping conditional:
func f(x int) {
g(x, 0)
}
func f(x int) {
if x > 0 {
g()
}
}
Filippo Valsorda [Tue, 6 Nov 2018 02:00:52 +0000 (21:00 -0500)]
crypto/tls: remove a forgotten note to future self
Now, this is embarrassing. While preparing CL 142818, I noticed a
possible vulnerability in the existing code which I was rewriting. I
took a note to go back and assess if it was indeed an issue, and in case
start the security release process. The note unintentionally slipped
into the commit. Fortunately, there was no vulnerability.
What caught my eye was that I had fixed the calculation of the minimum
encrypted payload length from
roundUp(explicitIVLen+macSize+1, blockSize)
to (using the same variable names)
explicitIVLen + roundUp(macSize+1, blockSize)
The explicit nonce sits outside of the encrypted payload, so it should
not be part of the value rounded up to the CBC block size.
You can see that for some values of the above, the old result could be
lower than the correct value. An unexpectedly short payload might cause
a panic during decryption (a DoS vulnerability) or even more serious
issues due to the constant time code that follows it (see for example
Yet Another Padding Oracle in OpenSSL CBC Ciphersuites [1]).
In practice, explicitIVLen is either zero or equal to blockSize, so it
does not change the amount of rounding up necessary and the two
formulations happen to be identical. Nothing to see here.
It looked more suspicious than it is in part due to the fact that the
explicitIVLen definition moved farther into hc.explicitNonceLen() and
changed name from IV (which suggests a block length) to nonce (which
doesn't necessarily). But anyway it was never meant to surface or be
noted, except it slipped, so here we are for a boring explanation.
Ian Lance Taylor [Fri, 2 Nov 2018 23:00:33 +0000 (16:00 -0700)]
cmd/go: pass go language version to cmd/compile
Pass the Go language version specified in the go.mod file to
cmd/compile.
Also, change the behavior when the go.mod file requests a Go version
that is later than the current one. Previously cmd/go would give a
fatal error in this situation. With this change it attempts the
compilation, and if (and only if) the compilation fails it adds a note
saying that the requested Go version is newer than the known version.
This is as described in https://golang.org/issue/28221.
Updates #28221.
Change-Id: I46803813e7872d4a418a3fd5299880be3b73a971
Reviewed-on: https://go-review.googlesource.com/c/147278
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Fri, 2 Nov 2018 22:42:15 +0000 (15:42 -0700)]
cmd/go: add cmpenv command to testing script language
This will be used by later commits in this sequence.
Updates #28221
Change-Id: I2b22b9f88a0183636cde9509606f03f079eb33f1
Reviewed-on: https://go-review.googlesource.com/c/147277
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Mon, 24 Sep 2018 14:13:36 +0000 (07:13 -0700)]
syscall: implement syscalls on Darwin using libSystem
There are still some references to the bare Syscall functions
in the stdlib. I will root those out in a following CL.
(This CL is big enough as it is.)
Most are in vendor directories:
Robert Griesemer [Mon, 5 Nov 2018 23:51:11 +0000 (15:51 -0800)]
go/types: avoid certain problems with recursive alias type declarations
It is possible to create certain recursive type declarations involving
alias types which cause the type-checker to produce an (invalid) type
for the alias because it is not yet available. By type-checking alias
declarations in a 2nd phase, the problem is mitigated a bit since it
requires more convoluted alias declarations for the problem to appear.
Also re-enable testing of fixedbugs/issue27232.go again (which was the
original cause for this change).
Updates #28576.
Change-Id: If6f9656a95262e6575b01c4a003094d41551564b
Reviewed-on: https://go-review.googlesource.com/c/147597 Reviewed-by: Alan Donovan <adonovan@google.com>
Also update misc/nacl/testzip.proto to include new testdata.
Change-Id: If41590be9f395a591056e89a417b589c4ba71b1a
Reviewed-on: https://go-review.googlesource.com/c/147979
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
David Chase [Tue, 6 Nov 2018 21:42:13 +0000 (16:42 -0500)]
cmd/compile: update TestNexting golden file for Delve
This change updates the expected output of the delve debugging session
in the TestNexting internal/ssa test, aligning it with the changes
introduced in CL 147360 and earlier.
Change-Id: I1cc788d02433624a36f4690f24201569d765e5d3
Reviewed-on: https://go-review.googlesource.com/c/147998
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Alberto Donizetti [Tue, 6 Nov 2018 21:08:30 +0000 (22:08 +0100)]
cmd/compile: update TestNexting golden file
This change updates the expected output of the gdb debugging session
in the TestNexting internal/ssa test, aligning it with the changes
introduced in CL 147360.
Fixes the longtest builder.
Change-Id: I5b5c22e1cf5e205967ff8359dc6c1485c815428e
Reviewed-on: https://go-review.googlesource.com/c/147957
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Thu, 25 Oct 2018 12:42:46 +0000 (13:42 +0100)]
cmd/go: make 'go test -h' print two lines
Like every other command's -h flag. To achieve this, pass the command's
usage function to the cmdflag package, since that package is used by
multiple commands and cannot directly access *base.Command.
This also lets us get rid of testFlag1 and testFlag2, and instead have
contiguous raw strings for the test and testflag help docs.
Fixes #26999.
Change-Id: I2ebd66835ee61fa83270816a01fa312425224bb3
Reviewed-on: https://go-review.googlesource.com/c/144558
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Alberto Donizetti [Tue, 6 Nov 2018 18:43:55 +0000 (19:43 +0100)]
cmd/compile: add new format to known_formats
This change fixes a TestFormat failure in fmt_test by adding a
recently introduced new known format (%q for syntax.Error).
Fixes #28621
Change-Id: I026ec88c334549a957a692c1652a860c57e23dae
Reviewed-on: https://go-review.googlesource.com/c/147837 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Fri, 2 Nov 2018 20:51:14 +0000 (16:51 -0400)]
cmd/asm: rename R18 to R18_PLATFORM on ARM64
In ARM64 ABI, R18 is the "platform register", the use of which is
OS specific. The OS could choose to reserve this register. In
practice, it seems fine to use R18 on Linux but not on darwin (iOS).
Rename R18 to R18_PLATFORM to prevent accidental use. There is no
R18 usage within the standard library (besides tests, which are
updated).
This change adds the vet-lite command (the future cmd/vet) and all its
dependencies from x/tools, but not its tests and their dependencies.
It was created with these commands:
I feel sure I am holding govendor wrong. Please advise.
A followup CL will make cmd/vet behave like vet-lite, initially just
for users that opt in, and soon after for all users, at which point
cmd/vet will be replaced in its entirety by a copy of vet-lite's small
main.go.
In the meantime, anyone can try the new tool using these commands:
$ go build cmd/vendor/golang.org/x/tools/go/analysis/cmd/vet-lite
$ export GOVETTOOL=$(which vet-lite)
$ go vet your/project/...
Lynn Boger [Fri, 5 Oct 2018 18:21:39 +0000 (14:21 -0400)]
runtime: improve performance of memclr, memmove on ppc64x
This improves the asm implementations for memmove and memclr on
ppc64x through use of vsx loads and stores when size is >= 32 bytes.
For memclr, dcbz is used when the size is >= 512 and aligned to 128.
Clément Chigot [Fri, 2 Nov 2018 12:14:07 +0000 (13:14 +0100)]
cmd/compile/internal/gc: update cgo_import_dynamic for AIX
On AIX, cmd/link needs two information in order to generate a dynamic
import, the library and its object needed. Currently, cmd/link isn't
able to retrieve this object only with the name of the library.
Therefore, the library pattern in cgo_import_dynamic must be
"lib.a/obj.o".
Change-Id: Ib8b8aaa9807c9fa6af46ece4e312d58073ed6ec1
Reviewed-on: https://go-review.googlesource.com/c/146957
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Raghavendra Nagaraj [Tue, 6 Nov 2018 09:02:03 +0000 (09:02 +0000)]
reflect: fix StructOf panics from too many methods in embedded fields
Previously we panicked if the number of methods present for an embedded
field was >= 32. This change removes that limit and now StructOf
dynamically calls itself to create space for the number of methods.
Fixes #25402
Change-Id: I3b1deb119796d25f7e6eee1cdb126327b49a0b5e
GitHub-Last-Rev: 16da71ad6b23563f3ed26f1914adf41e3d42de69
GitHub-Pull-Request: golang/go#26865
Reviewed-on: https://go-review.googlesource.com/c/128479
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
We collapse OpOffPtrs during generic rewrites.
However, we also use disjoint at the same time.
Instead of waiting for all OpOffPtrs to be collapsed
before the disjointness rules can kick in,
burrow through all OpOffPtrs immediately.
Change-Id: I60d0a70a9b4605b1817db7c4aab0c0d789651c90
Reviewed-on: https://go-review.googlesource.com/c/145206
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Michael Munday <mike.munday@ibm.com> Reviewed-by: Keith Randall <khr@golang.org>
convT2E16 and other specialized type-to-interface routines
accept a type/itab argument and return a complete interface value.
However, we know enough in the routine to do without the type.
And the caller can construct the interface value using the type.
Doing so shrinks the call sites of ten of the specialized convT2x routines.
It also lets us unify the empty and non-empty interface routines.
Prior to this change, since f and g have identical signatures,
they would share a single generated func type.
types.SubstAny makes a shallow type copy, even after instantiation,
f and g share a single generated Result type.
So if you instantiate f with any=T, call dowidth,
instantiate g with any=U, and call dowidth,
and if sizeof(T) != sizeof(U),
then the Offset of the result for f is now wrong.
I don't believe this happens at all right now, but it bit me hard when
experimenting with some other compiler changes.
And it's hard to debug. It results in rare stack corruption, causing
problems far from the actual source of the problem.
To fix this, change SubstAny to make deep copies of TSTRUCTs.
Michael Anthony Knyszek [Mon, 5 Nov 2018 19:26:25 +0000 (19:26 +0000)]
runtime: clean up MSpan* MCache* MCentral* in docs
This change cleans up references to MSpan, MCache, and MCentral in the
docs via a bunch of sed invocations to better reflect the Go names for
the equivalent structures (i.e. mspan, mcache, mcentral) and their
methods (i.e. MSpan_Sweep -> mspan.sweep).
Austin Clements [Mon, 5 Nov 2018 17:36:42 +0000 (12:36 -0500)]
runtime: deflake TestTracebackAncestors
TestTracebackAncestors has a ~0.1% chance of failing with more
goroutines in the traceback than expected. This happens because
there's a window between each goroutine starting its child and that
goroutine actually exiting. The test captures its own stack trace
after everything is "done", but if this happens during that window, it
will include the goroutine that's in the process of being torn down.
Here's an example of such a failure:
https://build.golang.org/log/fad10d0625295eb79fa879f53b8b32b9d0596af8
This CL fixes this by recording the goroutines that are expected to
exit and removing them from the stack trace. With this fix, this test
passed 15,000 times with no failures.
Change-Id: I71e7c6282987a15e8b74188b9c585aa2ca97cbcd
Reviewed-on: https://go-review.googlesource.com/c/147517
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Yury Smolsky [Tue, 30 Oct 2018 22:19:35 +0000 (00:19 +0200)]
go/doc: inspect function signature for building playground examples
This documentation example was broken:
https://golang.org/pkg/image/png/#example_Decode.
It did not have the "io" package imported,
The package was referenced in the result type of the function.
The "playExample" function did not inspect
the result types of declared functions.
This CL adds inspecting of parameters and result types of functions.
Robert Griesemer [Sat, 3 Nov 2018 06:28:26 +0000 (23:28 -0700)]
cmd/compile: reintroduce work-around for cyclic alias declarations
This change re-introduces (temporarily) a work-around for recursive
alias type declarations, originally in https://golang.org/cl/35831/
(intended as fix for #18640). The work-around was removed later
for a more comprehensive cycle detection check. That check
contained a subtle error which made the code appear to work,
while in fact creating incorrect types internally. See #25838
for details.
By re-introducing the original work-around, we eliminate problems
with many simple recursive type declarations involving aliases;
specifically cases such as #27232 and #27267. However, the more
general problem remains.
This CL also fixes the subtle error (incorrect variable use when
analyzing a type cycle) mentioned above and now issues a fatal
error with a reference to the relevant issue (rather than crashing
later during the compilation). While not great, this is better
than the current status. The long-term solution will need to
address these cycles (see #25838).
As a consequence, several old test cases are not accepted anymore
by the compiler since they happened to work accidentally only.
This CL disables parts or all code of those test cases. The issues
are: #18640, #23823, and #24939.
One of the new test cases (fixedbugs/issue27232.go) exposed a
go/types issue. The test case is excluded from the go/types test
suite and an issue was filed (#28576).
Michael Anthony Knyszek [Wed, 17 Oct 2018 20:16:45 +0000 (20:16 +0000)]
runtime: fix stale comments about mheap and mspan
As of 07e738e all spans are allocated out of a treap, and not just
large spans or spans for large objects. Also, now we have a separate
treap for spans that have been scavenged.
Austin Clements [Thu, 17 Aug 2017 22:11:01 +0000 (18:11 -0400)]
cmd/trace: display p99.9, p99 and p95 MUT
This uses the mutator utilization distribution to compute the p99.9,
p99, and p95 mutator utilization topograph lines and display them
along with the MMU.
Change-Id: I8c7e0ec326aa4bc00619ec7562854253f01cc802
Reviewed-on: https://go-review.googlesource.com/c/60800
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
The current MMU analysis considers all Ps together, so if, for
example, one of four Ps is blocked, mutator utilization is 75%.
However, this is less useful for understanding the impact on
individual goroutines because that one blocked goroutine could be
blocked for a very long time, but we still appear to have good
utilization.
Hence, this introduces a new flag that does a "per-P" analysis where
the utilization of each P is considered independently. The MMU is then
the combination of the MMU for each P's utilization function.
Change-Id: Id67b980d4d82b511d28300cdf92ccbb5ae8f0c78
Reviewed-on: https://go-review.googlesource.com/c/60797
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
cmd/trace: list and link to worst mutator utilization windows
This adds the ability to click a point on the MMU graph to show a list
of the worst 10 mutator utilization windows of the selected size. This
list in turn links to the trace viewer to drill down on specifically
what happened in each specific window.
Change-Id: Ic1b72d8b37fbf2212211c513cf36b34788b30133
Reviewed-on: https://go-review.googlesource.com/c/60795
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Tue, 8 Aug 2017 22:15:32 +0000 (18:15 -0400)]
internal/trace: use banding to optimize MMU computation
This further optimizes MMU construction by first computing a
low-resolution summary of the utilization curve. This "band" summary
lets us compute the worst-possible window starting in each of these
low-resolution bands (even without knowing where in the band the
window falls). This in turn lets us compute precise minimum mutator
utilization only in the worst low-resolution bands until we can show
that any remaining bands can't possibly contain a worse window.
This slows down MMU construction for small traces, but these are
reasonably fast to compute either way. For large traces (e.g.,
150,000+ utilization changes) it's significantly faster.
Change-Id: Ie66454e71f3fb06be3f6173b6d91ad75c61bda48
Reviewed-on: https://go-review.googlesource.com/c/60792
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Austin Clements [Mon, 28 Aug 2017 16:29:07 +0000 (12:29 -0400)]
internal/trace: use MU slope to optimize MMU
This commit speeds up MMU construction by ~10X (and reduces the number
of windows considered by ~20X) by using an observation about the
maximum slope of the windowed mutator utilization function to advance
the window time in jumps if the window's current mean mutator
utilization is much larger than the current minimum.
Change-Id: If3cba5da0c4adc37b568740f940793e491e96a51
Reviewed-on: https://go-review.googlesource.com/c/60791
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This adds an endpoint to the trace tool that plots the minimum mutator
utilization curve using information on mark assists and GC pauses from
the trace.
This commit implements a fairly straightforward O(nm) algorithm for
computing the MMU (and tests against an even more direct but slower
algorithm). Future commits will extend and optimize this algorithm.
This should be useful for debugging and understanding mutator
utilization issues like #14951, #14812, #18155. #18534, #21107,
particularly once follow-up CLs add trace cross-referencing.
Change-Id: Ic2866869e7da1e6c56ba3e809abbcb2eb9c4923a
Reviewed-on: https://go-review.googlesource.com/c/60790
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Tobias Klauser [Mon, 5 Nov 2018 11:39:11 +0000 (12:39 +0100)]
runtime/internal/sys: regenerate zgoos_*.go files
zgoos_aix.go is missing GoosJs, the order of GoosAndroid and GoosAix is
mixed up in all files and GoosHurd was added after CL 146023 introduced
GOOS=hurd.
Change-Id: I7e2f5a15645272e9020cfca86e44c364fc072a2b
Reviewed-on: https://go-review.googlesource.com/c/147397
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Elias Naur [Sun, 4 Nov 2018 08:36:25 +0000 (09:36 +0100)]
runtime: avoid arm64 8.1 atomics on Android
The kernel on some Samsung S9+ models reports support for arm64 8.1
atomics, but in reality only some of the cores support them. Go
programs scheduled to cores without support will crash with SIGILL.
This change unconditionally disables the optimization on Android.
A better fix is to precisely detect the offending chipset.
Fixes #28431
Change-Id: I35a1273e5660603824d30ebef2ce7e429241bf1f
Reviewed-on: https://go-review.googlesource.com/c/147377
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Keith Randall [Sun, 4 Nov 2018 05:57:52 +0000 (22:57 -0700)]
strings: lower running time of TestCompareStrings
At each comparison, we're making a copy of the whole string.
Instead, use unsafe to share the string backing store with a []byte.
It reduces the test time from ~4sec to ~1sec on my machine
(darwin/amd64). Some builders were having much more trouble with this
test (>3min), it may help more there.
The Fatalf mechanism already prints "compiler internal error:"
when reporting an error. There's no need to have "internal error"
in the error message passed to Fatalf calls. Removed them.
Fixes #28575.
Change-Id: I12b1bea37bc839780f257c27ef9e2005bf334925
Reviewed-on: https://go-review.googlesource.com/c/147287
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Austin Clements [Mon, 29 Oct 2018 22:27:51 +0000 (18:27 -0400)]
cmd/internal/obj: don't dedup symbols in WriteObjFile
Currently, WriteObjFile deduplicates symbols by name. This is a
strange and unexpected place to do this. But, worse, there's no
checking that it's reasonable to deduplicate two symbols, so this
makes it incredibly easy to mask errors involving duplicate symbols.
Dealing with duplicate symbols is better left to the linker. We're
also about to introduce multiple symbols with the same name but
different ABIs/versions, which would make this deduplication more
complicated. We just removed the only part of the compiler that
actually depended on this behavior.
This CL removes symbol deduplication from WriteObjFile, since it is no
longer needed.
Austin Clements [Mon, 29 Oct 2018 22:21:00 +0000 (18:21 -0400)]
cmd/compile: avoid duplicate GC bitmap symbols
Currently, liveness produces a distinct obj.LSym for each GC bitmap
for each function. These are then named by content hash and only
ultimately deduplicated by WriteObjFile.
For various reasons (see next commit), we want to remove this
deduplication behavior from WriteObjFile. Furthermore, it's
inefficient to produce these duplicate symbols in the first place.
GC bitmaps are the only source of duplicate symbols in the compiler.
This commit eliminates these duplicate symbols by declaring them in
the Ctxt symbol hash just like every other obj.LSym. As a result, all
GC bitmaps with the same content now refer to the same obj.LSym.
The next commit will remove deduplication from WriteObjFile.
Cherry Zhang [Fri, 2 Nov 2018 16:03:31 +0000 (12:03 -0400)]
cmd/internal/obj/arm64: fix encoding of 32-bit negated logical instructions
32-bit negated logical instructions (BICW, ORNW, EONW) with
constants were mis-encoded, because they were missing in the
cases where we handle 32-bit logical instructions. This CL
adds the missing cases.
Fixes #28548
Change-Id: I3d6acde7d3b72bb7d3d5d00a9df698a72c806ad5
Reviewed-on: https://go-review.googlesource.com/c/147077
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Ben Shi <powerman1st@163.com> Reviewed-by: Ben Shi <powerman1st@163.com>
Carl Mastrangelo [Fri, 26 Oct 2018 05:03:43 +0000 (22:03 -0700)]
net/http: speed up ServeMux matching
Scanning through all path patterns is not necessary, since the
paths do not change frequently. Instead, maintain a sorted list
of path prefixes and return the first match.
name old time/op new time/op delta
ServerMatch-12 134ns ± 3% 17ns ± 4% -86.95% (p=0.000 n=19+20)
Brad Fitzpatrick [Fri, 2 Nov 2018 15:18:43 +0000 (15:18 +0000)]
all: use "reports whether" consistently in the few places that didn't
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns true if")
This CL also replaces 4 uses of "iff" with the same "reports whether"
wording, which doesn't lose any meaning, and will prevent people from
sending typo fixes when they don't realize it's "if and only if". In
the past I think we've had the typo CLs updated to just say "reports
whether". So do them all at once.
(Inspired by the addition of another "returns true if" in CL 146938
in fd_plan9.go)
Change-Id: Ided502237f5ab0d25cb625dbab12529c361a8b9f
Reviewed-on: https://go-review.googlesource.com/c/147037 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 1 Nov 2018 20:46:50 +0000 (13:46 -0700)]
cmd/compile/internal/gc: add tracing support to debug type checking
The compiler must first be built with the constant enableTrace set
to true (typecheck.go). After that, the -t flag becomes available
which enables tracing output of type-checking functions.
With enableTrace == false, the tracing code becomes dead code
and won't affect the compiler.
Typical output might look like this:
path/y.go:4:6: typecheck 0xc00033e180 DCLTYPE <node DCLTYPE> tc=0
path/y.go:4:6: . typecheck1 0xc00033e180 DCLTYPE <node DCLTYPE> tc=2
path/y.go:4:6: . . typecheck 0xc000331a40 TYPE T tc=1
path/y.go:4:6: . . . typecheck1 0xc000331a40 TYPE T tc=2
path/y.go:4:6: . . . . typecheckdef 0xc000331a40 TYPE T tc=2
path/y.go:4:6: . . . . => 0xc000331a40 TYPE T tc=2 type=*T
path/y.go:4:6: . . . => 0xc000331a40 TYPE T tc=2 type=*T
path/y.go:4:6: . . => 0xc000331a40 TYPE T tc=1 type=*T
path/y.go:4:6: . => 0xc00033e180 DCLTYPE <node DCLTYPE> tc=2 type=<T>
path/y.go:4:6: => 0xc00033e180 DCLTYPE <node DCLTYPE> tc=1 type=<T>
Disabled by default.
Change-Id: Ifd8385290d1cf0d3fc5e8468b2f4ab84e8eff338
Reviewed-on: https://go-review.googlesource.com/c/146782 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Filippo Valsorda [Fri, 2 Nov 2018 04:57:30 +0000 (00:57 -0400)]
crypto/tls: implement TLS 1.3 server handshake (base)
Implement a basic TLS 1.3 server handshake, only enabled if explicitly
requested with MaxVersion.
This CL intentionally leaves for future CLs:
- PSK modes and resumption
- client authentication
- compatibility mode ChangeCipherSpecs
- early data skipping
- post-handshake messages
- downgrade protection
- KeyLogWriter support
- TLS_FALLBACK_SCSV processing
It also leaves a few areas up for a wider refactor (maybe in Go 1.13):
- the certificate selection logic can be significantly improved,
including supporting and surfacing signature_algorithms_cert, but
this isn't new in TLS 1.3 (see comment in processClientHello)
- handshake_server_tls13.go can be dried up and broken into more
meaningful, smaller functions, but it felt premature to do before
PSK and client auth support
- the monstrous ClientHello equality check in doHelloRetryRequest can
get both cleaner and more complete with collaboration from the
parsing layer, which can come at the same time as extension
duplicates detection
Filippo Valsorda [Wed, 31 Oct 2018 16:14:51 +0000 (12:14 -0400)]
crypto/tls: advertise and accept rsa_pss_rsae signature algorithms
crypto/x509 already supports PSS signatures (with rsaEncryption OID),
and crypto/tls support was added in CL 79736. Advertise support for the
algorithms and accept them as a peer.
Note that this is about PSS signatures from regular RSA public keys.
RSA-PSS only public keys (with RSASSA-PSS OID) are supported in neither
crypto/tls nor crypto/x509. See RFC 8446, Section 4.2.3.
testdata/Server-TLSv12-ClientAuthRequested* got modified because the
CertificateRequest carries the supported signature algorithms.
The net/smtp tests changed because 512 bits keys are too small for PSS.
Based on Peter Wu's CL 79738, who did all the actual work in CL 79736.
Filippo Valsorda [Wed, 31 Oct 2018 13:34:10 +0000 (09:34 -0400)]
crypto/tls: implement TLS 1.3 version negotiation
RFC 8446 recommends using the supported_versions extension to negotiate
lower versions as well, so begin by implementing it to negotiate the
currently supported versions.
Note that pickTLSVersion was incorrectly negotiating the ServerHello
version down on the client. If the server had illegally sent a version
higher than the ClientHello version, the client would have just
downgraded it, hopefully failing later in the handshake.
In TestGetConfigForClient, we were hitting the record version check
because the server would select TLS 1.1, the handshake would fail on the
client which required TLS 1.2, which would then send a TLS 1.0 record
header on its fatal alert (not having negotiated a version), while the
server would expect a TLS 1.1 header at that point. Now, the client gets
to communicate the minimum version through the extension and the
handshake fails on the server.
Note that there is significant code duplication due to extensions with
the same format appearing in different messages in TLS 1.3. This will be
cleaned up in a future refactor once CL 145317 is merged.
Enforcing the presence/absence of each extension in each message is left
to the upper layer, based on both protocol version and extensions
advertised in CH and CR. Duplicated extensions and unknown extensions in
SH, EE, HRR, and CT will be tightened up in a future CL.
The TLS 1.2 CertificateStatus message was restricted to accepting only
type OCSP as any other type (none of which are specified so far) would
have to be negotiated.
Updates #9671
Change-Id: I7c42394c5cc0af01faa84b9b9f25fdc6e7cfbb9e
Reviewed-on: https://go-review.googlesource.com/c/145477 Reviewed-by: Adam Langley <agl@golang.org>
Hajime Hoshi [Fri, 2 Nov 2018 17:19:47 +0000 (02:19 +0900)]
syscall: add Syscall18 on Windows
There are some OpenGL functions that take more than 15 arguments.
This CL adds Syscall18 to enable to call such functions on Windows
via syscall functions.
Fixes #28434
Change-Id: Ic7e37dda9cadf4516183e98166bfc52844ad2bbe
Reviewed-on: https://go-review.googlesource.com/c/147117 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Brad Fitzpatrick [Fri, 2 Nov 2018 15:47:40 +0000 (15:47 +0000)]
runtime: look up runtime env variables case insensitively on Windows
Fixes #28557
Change-Id: Ifca958b78e8c62fbc66515e693f528d799e8e84b
Reviewed-on: https://go-review.googlesource.com/c/147039 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Fri, 2 Nov 2018 09:55:12 +0000 (10:55 +0100)]
all: fix tests for older versions of AIX 7.2
This commit fixes tests which fail on some versions of AIX 7.2 due
to internal bugs.
getsockname isn't working properly with unix networks.
Timezone files aren't returning a correct output.
Change-Id: I4ff15683912be62ab86dfbeeb63b73513404d086
Reviewed-on: https://go-review.googlesource.com/c/146940
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Fri, 2 Nov 2018 09:02:38 +0000 (10:02 +0100)]
internal/poll, os/exec, runtime: replace PollDescriptor by IsPollDescriptor
This commit changes poll.PollDescriptor by poll.IsPollDescriptor. This
is needed for OS like AIX which have more than one FD using inside their
netpoll implementation.
Change-Id: I49e12a8d74045c501e19fdd8527cf166a3c64850
Reviewed-on: https://go-review.googlesource.com/c/146938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitry Vyukov [Wed, 31 Oct 2018 14:07:57 +0000 (15:07 +0100)]
runtime: avoid runtimeNano call on a common netpoll path
runtimeNano is slower than nanotime, so pass the duration
to runtime_pollSetDeadline as is. netpoll can add nanotime itself.
Arguably a bit simpler because, say, a negative duration
clearly represents already expired timer, no need to compare to
nanotime again.
This may also fix an obscure corner case when a deadline in past
which happens to be nanotime 0 is confused with no deadline at all,
which are radically different things.
Also don't compute any durations and times if Time is zero
(currently we first compute everything and then reset d back to 0,
which is wasteful).
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.1µs ± 0% 17.0µs ± 0% ~ (p=0.421 n=5+5)
SetReadDeadline-6 230ns ± 0% 205ns ± 1% -10.63% (p=0.008 n=5+5)
Change-Id: I2aad699270289a5b9ead68f5e44ec4ec6d96baa0
Reviewed-on: https://go-review.googlesource.com/c/146344 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Dmitry Vyukov [Wed, 31 Oct 2018 16:56:14 +0000 (17:56 +0100)]
runtime: execute memory barrier conditionally when changing netpoll timers
We only need the memory barrier in poll_runtime_pollSetDeadline only
when one of the timers has fired, which is not the expected case.
Memory barrier can be somewhat expensive on some archs,
so execute it only if one of the timers has in fact fired.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.1µs ± 0% +0.35% (p=0.032 n=5+5)
SetReadDeadline-6 232ns ± 0% 230ns ± 0% -1.03% (p=0.000 n=4+5)
Update #25729
Change-Id: Ifce6f505b9e7ba3717bad8f454077a2e94ea6e75
Reviewed-on: https://go-review.googlesource.com/c/146343 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitry Vyukov [Wed, 31 Oct 2018 16:36:51 +0000 (17:36 +0100)]
time: speed up Since and Until
time.now is somewhat expensive (much more expensive than nanotime),
in the common case when Time has monotonic time we don't actually
need to call time.now in Since/Until as we can do calculation
based purely on monotonic times.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.1µs ± 1% ~ (p=0.151 n=5+5)
SetReadDeadline-6 261ns ± 0% 234ns ± 1% -10.35% (p=0.008 n=5+5)
Benchmark that only calls Until:
benchmark old ns/op new ns/op delta
BenchmarkUntil 54.0 29.5 -45.37%
Update #25729
Change-Id: I5ac5af3eb1fe9f583cf79299f10b84501b1a0d7d
Reviewed-on: https://go-review.googlesource.com/c/146341
Run-TryBot: Dmitry Vyukov <dvyukov@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Dmitry Vyukov [Wed, 31 Oct 2018 16:27:16 +0000 (17:27 +0100)]
runtime, time: refactor startNano handling
Move startNano from runtime to time package.
In preparation for a subsequent change that speeds up Since and Until.
This also makes code simpler as we have less assembly as the result,
monotonic time handling is better localized in time package.
This changes values returned from nanotime on windows
(it does not account for startNano anymore), current comments state
that it's important, but it's unclear how it can be important
since no other OS does this.
Update #25729
Change-Id: I2275d57b7b5ed8fd0d53eb0f19d55a86136cc555
Reviewed-on: https://go-review.googlesource.com/c/146340 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitry Vyukov [Wed, 31 Oct 2018 16:03:35 +0000 (17:03 +0100)]
runtime: add and use modtimer in netpoll
Currently when netpoll deadline is incrementally prolonged,
we delete and re-add timer each time.
Add modtimer function that does both and use it when we need
to modify an existing netpoll timer to avoid unnecessary lock/unlock.
Change-Id: I08b89dbbc1785dd180e967a37b0aa23b0c4613a8
Reviewed-on: https://go-review.googlesource.com/c/146339 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitry Vyukov [Wed, 31 Oct 2018 15:18:36 +0000 (16:18 +0100)]
runtime: don't recreate netpoll timers if they don't change
Currently we always delete both read and write timers and then
add them again. However, if user setups read and write deadline
separately, then we don't need to touch the other one.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.2µs ± 0% 17.2µs ± 0% ~ (p=0.310 n=5+5)
SetReadDeadline-6 319ns ± 1% 274ns ± 2% -13.94% (p=0.008 n=5+5)
Update #25729
Change-Id: I4c869c3083521de6d0cd6ca99a7609d4dd84b4e4
Reviewed-on: https://go-review.googlesource.com/c/146338 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitry Vyukov [Wed, 31 Oct 2018 10:31:31 +0000 (11:31 +0100)]
runtime: don't wake timeproc needlessly
It's not always necessary to wake timerproc even if we add
a new timer to the top of the heap. Since we don't wake and
reset timerproc when we remove timers, it still can be sleeping
with shorter timeout. It such case it's more profitable to let it
sleep and then update timeout when it wakes on its own rather than
proactively wake it, let it update timeout and go to sleep again.
name old time/op new time/op delta
TCP4OneShotTimeout-6 18.6µs ± 1% 17.2µs ± 0% -7.66% (p=0.008 n=5+5)
SetReadDeadline-6 562ns ± 5% 319ns ± 1% -43.27% (p=0.008 n=5+5)
Update #25729
Change-Id: Iec8eacb8563dbc574a82358b3bac7ac479c16826
Reviewed-on: https://go-review.googlesource.com/c/146337 Reviewed-by: Ian Lance Taylor <iant@golang.org>