- 13 Nov, 2018 10 commits
-
-
Russ Cox authored
I see no reason Plan 9 should be special cased. A directory named go.mod is not useful on any system. Followup to CL 129804. Change-Id: I9cc91b5934b17650bfdb07370aa73aeae445968c Reviewed-on: https://go-review.googlesource.com/c/149337 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
-
Than McIntosh authored
The code to implement new-style gccgo name mangling had a recipe that didn't quite match that of the compiler (incorrect handling for '.'). This showed up as a failure in the gotools cgo test if the directory containing the test run included a "." character. [This is a copy of https://golang.org/cl/147917]. Change-Id: Ia94728ecead879c8d223eb6cee6c102a8af1c86e Reviewed-on: https://go-review.googlesource.com/c/147937Reviewed-by: Cherry Zhang <cherryyz@google.com>
-
Russ Cox authored
This reverts CL 144137. Reason for revert: The justification for the original commit was that golint said so, but golint is wrong. The code reads more clearly the original way. Change-Id: I960f286ed66fec67aabd953e7b69993f60b00bca Reviewed-on: https://go-review.googlesource.com/c/149339Reviewed-by: Russ Cox <rsc@golang.org>
-
Martin Garton authored
Since Reader.Peek potentially reads from the underlying io.Reader, discarding previous buffers, UnreadRune and UnreadByte cannot necessarily work. Change Peek to invalidate the unread buffers in all cases (as allowed according to the documentation) and thus prevent hiding bugs in the caller. (This change was previoiusly merged and then reverted due concern about being too close to a release) Fixes #18556 Change-Id: I9027d75aa834d4b27703f37711ba25de04d89f3c GitHub-Last-Rev: 917ef1e51131d734f92efc946a0ab5ca4ff69be6 GitHub-Pull-Request: golang/go#28768 Reviewed-on: https://go-review.googlesource.com/c/149297 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
Followup to CL 129779 but also some other minor tweaks. Change-Id: Id71455d8a14f5e33f82c942c9e892da56c49d17c Reviewed-on: https://go-review.googlesource.com/c/149257 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
The form of runtime.Version is not guaranteed to be helpful. Do not suggest it. (The suggestion was added in CL 136215.) Change-Id: I3227d2e66b6ce860b7e62d7ba531c18fb173823c Reviewed-on: https://go-review.googlesource.com/c/149258 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
CL 145577 added the part about io.ReadFull to read len(p) but it should be next to the existing sentence about not reading len(p) bytes. Change-Id: Idfa037c59a3085d44d5da6129188473db0e96d23 Reviewed-on: https://go-review.googlesource.com/c/148903 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
-
Brad Fitzpatrick authored
Fixes #26937 Change-Id: I6cdc1bad4cf476cd2ea1462b53444eccd8841e14 Reviewed-on: https://go-review.googlesource.com/c/146437 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
-
Robert Griesemer authored
Adjusted spec to explicitly define the string length as the number of bytes of the string; the prose now matches the prose for arrays. Made analogous change for slices. Fixes #28736. Change-Id: I47cab321c87de0a4c482f5466b819b2cc8993fd1 Reviewed-on: https://go-review.googlesource.com/c/149077Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
-
Ian Lance Taylor authored
Change-Id: Iad10d0a2dbc8e12e9f776c6cfb34070f584fd439 Reviewed-on: https://go-review.googlesource.com/c/149057 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
-
- 12 Nov, 2018 30 commits
-
-
Emmanuel T Odeke authored
To avoid any cancelation of the parent context from affecting lookupGroup operations, Resolver.LookupIPAddr previously used an entirely new context created from context.Background(). However, this meant that all the values in the parent context with which LookupIPAddr was invoked were dropped. This change provides a custom context implementation that only preserves values of the parent context by composing context.Background() and the parent context. It only falls back to the parent context to perform value lookups if the parent context has not yet expired. This context is never canceled, and has no deadlines. Fixes #28600 Change-Id: If2f570caa26c65bad638b7102c35c79d5e429fea Reviewed-on: https://go-review.googlesource.com/c/148698 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Filippo Valsorda authored
The Config does not own the memory pointed to by the Certificate slice. Instead, opportunistically use Certificate.Leaf and let the application set it if it desires the performance gain. This is a partial rollback of CL 107627. See the linked issue for the full explanation. Fixes #28744 Change-Id: I33ce9e6712e3f87939d9d0932a06d24e48ba4567 Reviewed-on: https://go-review.googlesource.com/c/149098Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Ali Rizvi-Santiago authored
This adds the includes for errno.h to the windows stubs for runtime/cgo so that "errno" is properly declared. Due to "errno" not being properly declared, the compiler is forced to assume it's an external which leaves it up to the linker. This is an issue in some implementations as errno might be a macro which results in an unresolved symbol error during linking. runtime/cgo/gcc_libinit_windows.c: added include runtime/cgo/gcc_windows_386.c: added include runtime/cgo/gcc_windows_amd64.c: added include Change-Id: I77167d02f7409462979135efc55cf50bbc6bd363 GitHub-Last-Rev: 90da06ee3cbec3f51c6d31185868bb70341ce9d3 GitHub-Pull-Request: golang/go#28747 Reviewed-on: https://go-review.googlesource.com/c/149118 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Austin Clements authored
SSA lowering can create PFUNC ONAME nodes when compiling method calls. Since we generally initialize the node's Sym to a func when we set its class to PFUNC, we did this here, too. Unfortunately, since SSA compilation is concurrent, this can cause a race if two function compilations try to initialize the same symbol. Luckily, we don't need to do this at all, since we're actually just wrapping an ONAME node around an existing Sym that's already marked as a function symbol. Fixes the linux-amd64-racecompile builder, which was broken by CL 147158. Updates #27539. Change-Id: I8ddfce6e66a08ce53998c5bfa6f5a423c1ffc1eb Reviewed-on: https://go-review.googlesource.com/c/149158 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
-
Austin Clements authored
We create the "init" symbol and mark it as a function before compiling to SSA because SSA can initialize this symbol, but it turns out we do it slightly too late. peekitabs, at least, can also create the "init" LSym. Move this initialization to just after type-checking. Fixes the linux-amd64-ssacheck and the android-arm64-wiko-fever builders. Updates #27539. Change-Id: If145952c79d39f75c93b24e35e67fe026dd08329 Reviewed-on: https://go-review.googlesource.com/c/149137 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
-
Austin Clements authored
This fixes the linux-amd64-longtest builder, which was broken by CL 147160. Updates #27539. Change-Id: If6e69581ef503bba2449ec9bacaa31f34f59beb1 Reviewed-on: https://go-review.googlesource.com/c/149157Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Austin Clements authored
This implements compiler and linker support for separating the function calling ABI into two ABIs: a stable and an internal ABI. At the moment, the two ABIs are identical, but we'll be able to evolve the internal ABI without breaking existing assembly code that depends on the stable ABI for calling to and from Go. The Go compiler generates internal ABI symbols for all Go functions. It uses the symabis information produced by the assembler to create ABI wrappers whenever it encounters a body-less Go function that's defined in assembly or a Go function that's referenced from assembly. Since the two ABIs are currently identical, for the moment this is implemented using "ABI alias" symbols, which are just forwarding references to the native ABI symbol for a function. This way there's no actual code involved in the ABI wrapper, which is good because we're not deriving any benefit from it right now. Once the ABIs diverge, we can eliminate ABI aliases. The linker represents these different ABIs internally as different versions of the same symbol. This way, the linker keeps us honest, since every symbol definition and reference also specifies its version. The linker is responsible for resolving ABI aliases. Fixes #27539. Change-Id: I197c52ec9f8fc435db8f7a4259029b20f6d65e95 Reviewed-on: https://go-review.googlesource.com/c/147160 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
-
Austin Clements authored
Currently, if a symbol is only defined under one ABI and referenced under another ABI, you simply get a "relocation target X not defined". This is confusing because it seems like the symbol is defined. This CL enhances the error message in this case to be "relocation target X not defined for <ABI> (but is defined for <ABI>)". For #27539. Change-Id: If857a1882c3fe9af5346797d5295ca1fe50ae565 Reviewed-on: https://go-review.googlesource.com/c/147159 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Austin Clements authored
In order to mark the obj.LSyms produced by the compiler with the correct ABI, we need to know which types.Syms refer to function symbols. This CL adds a flag to types.Syms to mark symbols for functions, and sets this flag everywhere we create a PFUNC-class node, and in the one place where we directly create function symbols without always wrapping them in a PFUNC node (methodSym). We'll use this information to construct obj.LSyms with correct ABI information. For #27539. Change-Id: Ie3ac8bf3da013e449e78f6ca85546a055f275463 Reviewed-on: https://go-review.googlesource.com/c/147158 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
-
Austin Clements authored
This repurposes the "version" field of a symbol reference in the Go object file format to be an ABI field. Currently, this is just 0 or 1 depending on whether the symbol is static (the linker turns it into a different internal version number), so it's already only tenuously a symbol version. We change this to be -1 for static symbols and otherwise by the ABI number. This also adds a separate list of ABI alias symbols to be recorded in the object file. The ABI aliases must be a separate list and not just part of the symbol definitions because it's possible to have a symbol defined in one package and the alias "defined" in a different package. For example, this can happen if a symbol is defined in assembly in one package and stubbed in a different package. The stub triggers the generation of the ABI alias, but in a different package from the definition. For #27539. Change-Id: I015c9fe54690c027de6ef77e22b5585976a01587 Reviewed-on: https://go-review.googlesource.com/c/147157 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
-
Austin Clements authored
This extends cmd/go's symabis support to collect known cross-package uses of runtime symbols from other "basically runtime" packages in std. This avoids having to declare a large number of ABI0 symbols in the runtime for a small number of known cross-package references. For cmd/dist, we use a simpler but less efficient approach and tell the compiler to generate ABI wrappers for everything. Change-Id: Ifaed94efdcff42e7345ab11b4d2fb880fb1a24e8 Reviewed-on: https://go-review.googlesource.com/c/147257 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Austin Clements authored
For #27539. Change-Id: I0e27f142224e820205fb0e65ad03be7eba93da14 Reviewed-on: https://go-review.googlesource.com/c/146999 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Austin Clements authored
This is a little clearer, and we're about to need the .s file list in one more place, so this will cut down on duplication. Change-Id: I4da8bf03a0469fb97565b0841c40d505657b574e Reviewed-on: https://go-review.googlesource.com/c/146998 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Austin Clements authored
This doesn't yet do anything with this information. For #27539. Change-Id: Ia12c905812aa1ed425eedd6ab2f55ec75d81c0ce Reviewed-on: https://go-review.googlesource.com/c/147099 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Zhang <cherryyz@google.com>
-
Austin Clements authored
This adds a -symabis flag that runs the assembler in a special mode that outputs symbol definition and reference ABIs rather than assembling the code. This uses a fast and somewhat lax parser because the go_asm.h definitions may not be available. For #27539. Change-Id: I248ba0ebab7cc75dcb2a90e82a82eb445da7e88e Reviewed-on: https://go-review.googlesource.com/c/147098 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Zhang <cherryyz@google.com>
-
Austin Clements authored
Currently cmd/asm's Parser.line both consumes a line of assembly from the lexer and assembles it. This CL separates these two steps so that the line parser can be reused for purposes other than generating a Prog stream. For #27539. Updates #17544. Change-Id: I452c9a2112fbcc1c94bf909efc0d1fcc71014812 Reviewed-on: https://go-review.googlesource.com/c/147097 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
-
Filippo Valsorda authored
To disable TLS 1.3, simply remove VersionTLS13 from supportedVersions, as tested by TestEscapeRoute, and amend documentation. To make it opt-in, revert the change to (*Config).supportedVersions from this CL. I did not have the heart to implement the early data skipping feature when I realized that it did not offer a choice between two abstraction-breaking options, but demanded them both (look for handshake type in case of HelloRetryRequest, trial decryption otherwise). It's a lot of complexity for an apparently small gain, but if anyone has strong opinions about it let me know. Note that in TLS 1.3 alerts are encrypted, so the close_notify peeking to return (n > 0, io.EOF) from Read doesn't work. If we are lucky, those servers that unexpectedly close connections after serving a single request will have stopped (maybe thanks to H/2) before they got updated to TLS 1.3. Relatedly, session tickets are now provisioned on the client first Read instead of at Handshake time, because they are, well, post-handshake messages. If this proves to be a problem we might try to peek at them. Doubled the tests that cover logic that's different in TLS 1.3. The benchmarks for TLS 1.2 compared to be0f3c28 (before TLS 1.3 and its refactors, after CL 142817 changed them to use real connections) show little movement. name old time/op new time/op delta HandshakeServer/RSA-8 795µs ± 1% 798µs ± 1% ~ (p=0.057 n=10+18) HandshakeServer/ECDHE-P256-RSA-8 903µs ± 0% 909µs ± 1% +0.68% (p=0.000 n=8+17) HandshakeServer/ECDHE-P256-ECDSA-P256-8 198µs ± 0% 204µs ± 1% +3.24% (p=0.000 n=9+18) HandshakeServer/ECDHE-X25519-ECDSA-P256-8 202µs ± 3% 208µs ± 1% +2.98% (p=0.000 n=9+20) HandshakeServer/ECDHE-P521-ECDSA-P521-8 15.5ms ± 1% 15.9ms ± 2% +2.49% (p=0.000 n=10+20) Throughput/MaxPacket/1MB-8 5.81ms ±23% 6.14ms ±44% ~ (p=0.605 n=8+18) Throughput/MaxPacket/2MB-8 8.91ms ±22% 8.74ms ±33% ~ (p=0.498 n=9+19) Throughput/MaxPacket/4MB-8 12.8ms ± 3% 14.0ms ±10% +9.74% (p=0.000 n=10+17) Throughput/MaxPacket/8MB-8 25.1ms ± 7% 24.6ms ±16% ~ (p=0.129 n=9+19) Throughput/MaxPacket/16MB-8 46.3ms ± 4% 45.9ms ±12% ~ (p=0.340 n=9+20) Throughput/MaxPacket/32MB-8 88.5ms ± 4% 86.0ms ± 4% -2.82% (p=0.004 n=10+20) Throughput/MaxPacket/64MB-8 173ms ± 2% 167ms ± 7% -3.42% (p=0.001 n=10+19) Throughput/DynamicPacket/1MB-8 5.88ms ± 4% 6.59ms ±64% ~ (p=0.232 n=9+18) Throughput/DynamicPacket/2MB-8 9.08ms ±12% 8.73ms ±21% ~ (p=0.408 n=10+18) Throughput/DynamicPacket/4MB-8 14.2ms ± 5% 14.0ms ±11% ~ (p=0.188 n=9+19) Throughput/DynamicPacket/8MB-8 25.1ms ± 6% 24.0ms ± 7% -4.39% (p=0.000 n=10+18) Throughput/DynamicPacket/16MB-8 45.6ms ± 3% 43.3ms ± 1% -5.22% (p=0.000 n=10+8) Throughput/DynamicPacket/32MB-8 88.4ms ± 3% 84.8ms ± 2% -4.06% (p=0.000 n=10+10) Throughput/DynamicPacket/64MB-8 175ms ± 3% 167ms ± 2% -4.63% (p=0.000 n=10+10) Latency/MaxPacket/200kbps-8 694ms ± 0% 694ms ± 0% -0.02% (p=0.000 n=9+9) Latency/MaxPacket/500kbps-8 279ms ± 0% 279ms ± 0% -0.09% (p=0.000 n=10+10) Latency/MaxPacket/1000kbps-8 140ms ± 0% 140ms ± 0% -0.15% (p=0.000 n=10+9) Latency/MaxPacket/2000kbps-8 71.1ms ± 0% 71.0ms ± 0% -0.09% (p=0.001 n=8+9) Latency/MaxPacket/5000kbps-8 30.5ms ± 6% 30.1ms ± 6% ~ (p=0.905 n=10+9) Latency/DynamicPacket/200kbps-8 134ms ± 0% 134ms ± 0% ~ (p=0.796 n=9+9) Latency/DynamicPacket/500kbps-8 54.8ms ± 0% 54.7ms ± 0% -0.18% (p=0.000 n=8+10) Latency/DynamicPacket/1000kbps-8 28.5ms ± 0% 29.1ms ± 8% ~ (p=0.173 n=8+10) Latency/DynamicPacket/2000kbps-8 15.3ms ± 6% 15.9ms ±10% ~ (p=0.905 n=9+10) Latency/DynamicPacket/5000kbps-8 9.14ms ±21% 9.65ms ±82% ~ (p=0.529 n=10+10) name old speed new speed delta Throughput/MaxPacket/1MB-8 175MB/s ±13% 167MB/s ±64% ~ (p=0.646 n=7+20) Throughput/MaxPacket/2MB-8 241MB/s ±25% 241MB/s ±40% ~ (p=0.660 n=9+20) Throughput/MaxPacket/4MB-8 328MB/s ± 3% 300MB/s ± 9% -8.70% (p=0.000 n=10+17) Throughput/MaxPacket/8MB-8 335MB/s ± 7% 340MB/s ±17% ~ (p=0.212 n=9+20) Throughput/MaxPacket/16MB-8 363MB/s ± 4% 367MB/s ±11% ~ (p=0.340 n=9+20) Throughput/MaxPacket/32MB-8 379MB/s ± 4% 390MB/s ± 4% +2.93% (p=0.004 n=10+20) Throughput/MaxPacket/64MB-8 388MB/s ± 2% 401MB/s ± 7% +3.25% (p=0.004 n=10+20) Throughput/DynamicPacket/1MB-8 178MB/s ± 4% 157MB/s ±73% ~ (p=0.127 n=9+20) Throughput/DynamicPacket/2MB-8 232MB/s ±11% 243MB/s ±18% ~ (p=0.415 n=10+18) Throughput/DynamicPacket/4MB-8 296MB/s ± 5% 299MB/s ±15% ~ (p=0.295 n=9+20) Throughput/DynamicPacket/8MB-8 334MB/s ± 6% 350MB/s ± 7% +4.58% (p=0.000 n=10+18) Throughput/DynamicPacket/16MB-8 368MB/s ± 3% 388MB/s ± 1% +5.48% (p=0.000 n=10+8) Throughput/DynamicPacket/32MB-8 380MB/s ± 3% 396MB/s ± 2% +4.20% (p=0.000 n=10+10) Throughput/DynamicPacket/64MB-8 384MB/s ± 3% 403MB/s ± 2% +4.83% (p=0.000 n=10+10) Comparing TLS 1.2 and TLS 1.3 at tip shows a slight (~5-10%) slowdown of handshakes, which might be worth looking at next cycle, but the latency improvements are expected to overshadow that. name old time/op new time/op delta HandshakeServer/ECDHE-P256-RSA-8 909µs ± 1% 963µs ± 0% +5.87% (p=0.000 n=17+18) HandshakeServer/ECDHE-P256-ECDSA-P256-8 204µs ± 1% 225µs ± 2% +10.20% (p=0.000 n=18+20) HandshakeServer/ECDHE-X25519-ECDSA-P256-8 208µs ± 1% 230µs ± 2% +10.35% (p=0.000 n=20+18) HandshakeServer/ECDHE-P521-ECDSA-P521-8 15.9ms ± 2% 15.9ms ± 1% ~ (p=0.444 n=20+19) Throughput/MaxPacket/1MB-8 6.14ms ±44% 7.07ms ±46% ~ (p=0.057 n=18+19) Throughput/MaxPacket/2MB-8 8.74ms ±33% 8.61ms ± 9% ~ (p=0.552 n=19+17) Throughput/MaxPacket/4MB-8 14.0ms ±10% 14.1ms ±12% ~ (p=0.707 n=17+20) Throughput/MaxPacket/8MB-8 24.6ms ±16% 25.6ms ±14% ~ (p=0.107 n=19+20) Throughput/MaxPacket/16MB-8 45.9ms ±12% 44.7ms ± 6% ~ (p=0.607 n=20+19) Throughput/MaxPacket/32MB-8 86.0ms ± 4% 87.9ms ± 8% ~ (p=0.113 n=20+19) Throughput/MaxPacket/64MB-8 167ms ± 7% 169ms ± 2% +1.26% (p=0.011 n=19+19) Throughput/DynamicPacket/1MB-8 6.59ms ±64% 6.79ms ±43% ~ (p=0.480 n=18+19) Throughput/DynamicPacket/2MB-8 8.73ms ±21% 9.58ms ±13% +9.71% (p=0.006 n=18+20) Throughput/DynamicPacket/4MB-8 14.0ms ±11% 13.9ms ±10% ~ (p=0.687 n=19+20) Throughput/DynamicPacket/8MB-8 24.0ms ± 7% 24.6ms ± 8% +2.36% (p=0.045 n=18+17) Throughput/DynamicPacket/16MB-8 43.3ms ± 1% 44.3ms ± 2% +2.48% (p=0.001 n=8+9) Throughput/DynamicPacket/32MB-8 84.8ms ± 2% 86.7ms ± 2% +2.27% (p=0.000 n=10+10) Throughput/DynamicPacket/64MB-8 167ms ± 2% 170ms ± 3% +1.89% (p=0.005 n=10+10) Latency/MaxPacket/200kbps-8 694ms ± 0% 699ms ± 0% +0.65% (p=0.000 n=9+10) Latency/MaxPacket/500kbps-8 279ms ± 0% 280ms ± 0% +0.68% (p=0.000 n=10+10) Latency/MaxPacket/1000kbps-8 140ms ± 0% 141ms ± 0% +0.59% (p=0.000 n=9+9) Latency/MaxPacket/2000kbps-8 71.0ms ± 0% 71.3ms ± 0% +0.42% (p=0.000 n=9+9) Latency/MaxPacket/5000kbps-8 30.1ms ± 6% 30.7ms ±10% +1.93% (p=0.019 n=9+9) Latency/DynamicPacket/200kbps-8 134ms ± 0% 138ms ± 0% +3.22% (p=0.000 n=9+10) Latency/DynamicPacket/500kbps-8 54.7ms ± 0% 56.3ms ± 0% +3.03% (p=0.000 n=10+8) Latency/DynamicPacket/1000kbps-8 29.1ms ± 8% 29.1ms ± 0% ~ (p=0.173 n=10+8) Latency/DynamicPacket/2000kbps-8 15.9ms ±10% 16.4ms ±36% ~ (p=0.633 n=10+8) Latency/DynamicPacket/5000kbps-8 9.65ms ±82% 8.32ms ± 8% ~ (p=0.573 n=10+8) name old speed new speed delta Throughput/MaxPacket/1MB-8 167MB/s ±64% 155MB/s ±55% ~ (p=0.224 n=20+19) Throughput/MaxPacket/2MB-8 241MB/s ±40% 244MB/s ± 9% ~ (p=0.407 n=20+17) Throughput/MaxPacket/4MB-8 300MB/s ± 9% 298MB/s ±11% ~ (p=0.707 n=17+20) Throughput/MaxPacket/8MB-8 340MB/s ±17% 330MB/s ±13% ~ (p=0.201 n=20+20) Throughput/MaxPacket/16MB-8 367MB/s ±11% 375MB/s ± 5% ~ (p=0.607 n=20+19) Throughput/MaxPacket/32MB-8 390MB/s ± 4% 382MB/s ± 8% ~ (p=0.113 n=20+19) Throughput/MaxPacket/64MB-8 401MB/s ± 7% 397MB/s ± 2% -0.96% (p=0.030 n=20+19) Throughput/DynamicPacket/1MB-8 157MB/s ±73% 156MB/s ±39% ~ (p=0.738 n=20+20) Throughput/DynamicPacket/2MB-8 243MB/s ±18% 220MB/s ±14% -9.65% (p=0.006 n=18+20) Throughput/DynamicPacket/4MB-8 299MB/s ±15% 303MB/s ± 9% ~ (p=0.512 n=20+20) Throughput/DynamicPacket/8MB-8 350MB/s ± 7% 342MB/s ± 8% -2.27% (p=0.045 n=18+17) Throughput/DynamicPacket/16MB-8 388MB/s ± 1% 378MB/s ± 2% -2.41% (p=0.001 n=8+9) Throughput/DynamicPacket/32MB-8 396MB/s ± 2% 387MB/s ± 2% -2.21% (p=0.000 n=10+10) Throughput/DynamicPacket/64MB-8 403MB/s ± 2% 396MB/s ± 3% -1.84% (p=0.005 n=10+10) Fixes #9671 Change-Id: Ieb57c5140eb2c083b8be0d42b240cd2eeec0dcf6 Reviewed-on: https://go-review.googlesource.com/c/147638 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Fix a couple overlooked ConnectionState fields noticed by net/http tests, and add a test in crypto/tls. Spun off CL 147638 to keep that one cleanly about enabling TLS 1.3. Change-Id: I9a6c2e68d64518a44be2a5d7b0b7b8d78c98c95d Reviewed-on: https://go-review.googlesource.com/c/148900 Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Filippo Valsorda authored
TLS_FALLBACK_SCSV is extremely fragile in the presence of sparse supported_version, but gave it the best try I could. Set the server random canaries but don't check them yet, waiting for the browsers to clear the way of misbehaving middleboxes. Updates #9671 Change-Id: Ie55efdec671d639cf1e716acef0c5f103e91a7ce Reviewed-on: https://go-review.googlesource.com/c/147617Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Note that the SignatureSchemes passed to GetClientCertificate in TLS 1.2 are now filtered by the requested certificate type. This feels like an improvement anyway, and the full list can be surfaced as well when support for signature_algorithms_cert is added, which actually matches the semantics of the CertificateRequest signature_algorithms in TLS 1.2. Also, note a subtle behavior change in server side resumption: if a certificate is requested but not required, and the resumed session did not include one, it used not to invoke VerifyPeerCertificate. However, if the resumed session did include a certificate, it would. (If a certificate was required but not in the session, the session is rejected in checkForResumption.) This inconsistency could be unexpected, even dangerous, so now VerifyPeerCertificate is always invoked. Still not consistent with the client behavior, which does not ever invoke VerifyPeerCertificate on resumption, but it felt too surprising to entirely change either. Updates #9671 Change-Id: Ib2b0dbc30e659208dca3ac07d6c687a407d7aaaf Reviewed-on: https://go-review.googlesource.com/c/147599Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Added some assertions to testHandshake, but avoided checking the error of one of the Close() because the one that would lose the race would write the closeNotify to a connection closed on the other side which is broken on js/wasm (#28650). Moved that Close() after the chan sync to ensure it happens second. Accepting a ticket with client certificates when NoClientCert is configured is probably not a problem, and we could hide them to avoid confusing the application, but the current behavior is to skip the ticket, and I'd rather keep behavior changes to a minimum. Updates #9671 Change-Id: I93b56e44ddfe3d48c2bef52c83285ba2f46f297a Reviewed-on: https://go-review.googlesource.com/c/147445Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Also check original certificate validity when resuming TLS 1.0–1.2. Will refuse to resume a session if the certificate is expired or if the original connection had InsecureSkipVerify and the resumed one doesn't. Support only PSK+DHE to protect forward secrecy even with lack of a strong session ticket rotation story. Tested with NSS because s_server does not provide any way of getting the same session ticket key across invocations. Will self-test like TLS 1.0–1.2 once server side is implemented. Incorporates CL 128477 by @santoshankr. Fixes #24919 Updates #9671 Change-Id: Id3eaa5b6c77544a1357668bf9ff255f3420ecc34 Reviewed-on: https://go-review.googlesource.com/c/147420Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Looks like the introduction of CCS records in the client second flight gave time to s_server to send NewSessionTicket messages in between the client application data and close_notify. There seems to be no way of turning NewSessionTicket messages off, neither by not sending a psk_key_exchange_modes extension, nor by command line flag. Interleaving the client write like that tickled an issue akin to #18701: on Windows, the client reaches Close() before the last record is drained from the send buffer, the kernel notices and resets the connection, cutting short the last flow. There is no good way of synchronizing this, so we sleep for a RTT before calling close, like in CL 75210. Sigh. Updates #9671 Change-Id: I44dc1cca17b373695b5a18c2741f218af2990bd1 Reviewed-on: https://go-review.googlesource.com/c/147419 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Since TLS 1.3 delivers handshake messages (including KeyUpdate) after the handshake, the want argument to readRecord had became almost pointless: it only meant something when set to recordTypeChangeCipherSpec. Replaced it with a bool to reflect that, and added two shorthands to avoid anonymous bools in calls. Took the occasion to simplify and formalize the invariants of readRecord. The maxConsecutiveEmptyRecords loop became useless when readRecord started retrying on any non-advancing record in CL 145297. Replaced panics with errors, because failure is better than undefined behavior, but contained failure is better than a DoS vulnerability. For example, I suspect the panic at the top of readRecord was reachable from handleRenegotiation, which calls readHandshake with handshakeComplete false. Thankfully it was not a panic in 1.11, and it's allowed now. Removed Client-TLSv13-RenegotiationRejected because OpenSSL isn't actually willing to ask for renegotiation over TLS 1.3, the expected error was due to NewSessionTicket messages, which didn't break the rest of the tests because they stop too soon. Updates #9671 Change-Id: I297a81bde5c8020a962a92891b70d6d70b90f5e3 Reviewed-on: https://go-review.googlesource.com/c/147418 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
-
Filippo Valsorda authored
Also, add support for the SSLKEYLOGFILE environment variable to the tests, to simplify debugging of unexpected failures. Updates #9671 Change-Id: I20a34a5824f083da93097b793d51e796d6eb302b Reviewed-on: https://go-review.googlesource.com/c/147417 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
-
Austin Clements authored
We're going to use the linker's symbol versions to track ABIs. Currently, version 0 is used for global symbols and version > 0 is used for file-local symbols. This CL reserves versions 0 to 9 for global symbols with ABIs and uses version 10 and up for file-local symbols. To make this clean, it also introduces a method on Symbol for querying whether it's file-local. For #27539. Change-Id: Id3bc7369268f35128b14318a62e86335181a80e5 Reviewed-on: https://go-review.googlesource.com/c/146859 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
-
Austin Clements authored
The compiler passes a lot of DWARF metadata about functions to the linker via symbols whose names are derived from the function's own symbol name. We look up these symbols in several places. This is about to get slightly more complex as we introduce ABIs as symbol versions, so abstract this lookup pattern into a helper function. For #27539. Change-Id: Ic71f6b5dc6608a5a5f5f515808981e6d6f5d728e Reviewed-on: https://go-review.googlesource.com/c/146858 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
-
Austin Clements authored
Currently, TestPCLine manually invokes asm and link on its test data. Once we introduce symbol ABIs this is going to become problematic because the test program defines main.main and main.init in assembly so they use ABI0, but the runtime expects to find them with the internal ABI. There are various ways we could solve this. This CL moves main.main and main.init into Go code and switches to using "go build" to compile and link the test binary. This has the added advantage of simplifying this test. For #27539. Change-Id: I4c0cf6467f7a39e6b1500eca6ad2620b5ef2b73c Reviewed-on: https://go-review.googlesource.com/c/146857 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Austin Clements authored
There are three cases where we don't currently have the visibility to get the ABIs of runtime symbols right, which this CL fixes: 1. For Go functions referenced from non-Go code in other packages. This is runtime.morestackc (which is referenced from function prologues) and a few syscall symbols. For these we need to generate ABI0 wrappers, so this CL adds dummy calls in the assembly code to force wrapper generation. There are many other cross-package references to runtime and runtime/internal/atomic, but these are handled specially by cmd/go. 2. For calls generated by the compiler to runtime Go functions, there are a few symbols that aren't declared in builtins.go because we've never needed their type information before. Now we at least need their ABI information, so these are added to builtins.go. 3. For calls generated by the compiler to runtime assembly functions, the compiler is going to assume the internal ABI is available, so we add Go stubs to the runtime to trigger wrapper generation. For these we're probably going to want to provide internal ABI definitions directly in the assembly for performance, but for now the ABIs are the same so it doesn't matter. For #27539. Change-Id: I9c224e7408d2ef4dd9b0e4c9d7e962ddfe111245 Reviewed-on: https://go-review.googlesource.com/c/146822 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 authored
The symbol runtime._cgo_panic_internal is defined both as a function in package runtime and as a (linknamed) variable in package runtime/cgo. Since we're introducing function ABIs, this is going to cause problems with resolving the ABI-marked function symbol with the unmarked data symbol. It's also confusing. Fix this by declaring runtime._cgo_panic_internal as a function in runtime/cgo as well and extracting the PC from the function object. For #27539. Change-Id: I148a458a600cf9e57791cf4cbe92e79bddbf58d4 Reviewed-on: https://go-review.googlesource.com/c/146821 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-