- 12 Nov, 2018 28 commits
-
-
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>
-
Austin Clements authored
The internal/bytealg package defines several symbols in the runtime, bytes, and strings packages in assembly, and the runtime package defines symbols in reflect and sync/atomic. Currently, there's no corresponding Go prototype for these symbols in the defining package. We're going to start depending on Go prototypes in the same package as their assembly definitions in order to provide ABI wrappers. Plus, these are good documentation and colocate type information with definitions, which could be useful for vet if it learned a little about linkname. This CL adds linknamed Go prototypes for all pushed symbols in internal/bytealg and runtime. For #27539. Change-Id: I9b0c12d935a75bb6af46b6761180d451c00f11b8 Reviewed-on: https://go-review.googlesource.com/c/146820 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
Currently, package runtime contains the definition of reflect.call, even though it's just a jump to runtime.reflectcall. This "push" symbol is confusing, since it's not clear where the definition of reflect.call comes from when you're in the reflect package. Replace this with a "pull" symbol: the runtime now defines only runtime.reflectcall and package reflect uses a go:linkname to access this symbol directly. This makes it clear where reflect.call is coming from without any spooky action at a distance and eliminates all of the definitions of reflect.call in the runtime. Change-Id: I3ec73cd394efe9df8d3061a57c73aece2e7048dd Reviewed-on: https://go-review.googlesource.com/c/148657 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
- 11 Nov, 2018 4 commits
-
-
Ainar Garipov authored
This cleanup was proposed in CL 148937. The branch is already ended with a continue, so remove continues from subbranches and use an else-if. Change-Id: Iaf6eb57afc84e25862f99a342f5824e315bcdcb7 Reviewed-on: https://go-review.googlesource.com/c/148922Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Michael Anthony Knyszek authored
This change fixes incorrect formatting in mheap.go (the result of my previous heap scavenging changes) and map_test.go. Change-Id: I2963687504abdc4f0cdf2f0c558174b3bc0ed2df Reviewed-on: https://go-review.googlesource.com/c/148977 Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Josh Bleecher Snyder authored
We have an existing optimization that recognizes memory moves of the form A -> B -> C and converts them into A -> C, in the hopes that the store to B will be end up being dead and thus eliminated. However, when A, B, and C are large types, the front end sometimes emits VarDef ops for the moves. This change adds an optimization to match that pattern. This required changing an old compiler test. The test assumed that a temporary was required to deal with a large return value. With this optimization in place, that temporary ended up being eliminated. Triggers 649 times during 'go build -a std cmd'. Cuts 16k off cmd/go. name old object-bytes new object-bytes delta Template 507kB ± 0% 507kB ± 0% -0.15% (p=0.008 n=5+5) Unicode 225kB ± 0% 225kB ± 0% ~ (all equal) GoTypes 1.85MB ± 0% 1.85MB ± 0% ~ (all equal) Flate 328kB ± 0% 328kB ± 0% ~ (all equal) GoParser 402kB ± 0% 402kB ± 0% -0.00% (p=0.008 n=5+5) Reflect 1.41MB ± 0% 1.41MB ± 0% -0.20% (p=0.008 n=5+5) Tar 458kB ± 0% 458kB ± 0% ~ (all equal) XML 601kB ± 0% 599kB ± 0% -0.21% (p=0.008 n=5+5) Change-Id: I9b5f25c8663a0b772ad1ee51fa61f74b74d26dd3 Reviewed-on: https://go-review.googlesource.com/c/143479 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Munday <mike.munday@ibm.com>
-
Ainar Garipov authored
These issues were found by the new vet's nilness check. The variables were already checked against nil, so remove extra checks. Change-Id: Ie252ccfcc755f3d06f691f354bf13d5a623fe17b Reviewed-on: https://go-review.googlesource.com/c/148937Reviewed-by: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
- 10 Nov, 2018 7 commits
-
-
Nikhil Benesch authored
When a goroutine enters a syscall, its M unwires from its P to allow the P to be retaken by another M if the syscall is slow. The M retains a reference to its old P, however, so that if its old P has not been retaken when the syscall returns, it can quickly reacquire that P. The implementation, however, was confusing, as it left the reference to the potentially-retaken P in m.p, which implied that the P was still wired. Make the code clearer by enforcing the invariant that m.p is never stale. entersyscall now moves m.p to m.oldp and sets m.p to 0; exitsyscall does the reverse, provided m.oldp has not been retaken. With this scheme in place, the issue described in #27660 (assertion failures in the race detector) would have resulted in a clean segfault instead of silently corrupting memory. Change-Id: Ib3e03623ebed4f410e852a716919fe4538858f0a Reviewed-on: https://go-review.googlesource.com/c/148899 Run-TryBot: Dmitry Vyukov <dvyukov@google.com> Reviewed-by: Dmitry Vyukov <dvyukov@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Josh Bleecher Snyder authored
Some of the Ops got re-ordered with OSLICEHEADER. Re-generate. Change-Id: I8ec3f5056537dd0b18cac2d4267453451ade2b00 Reviewed-on: https://go-review.googlesource.com/c/148824 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Martin Möhrmann <martisch@uos.de>
-
Josh Bleecher Snyder authored
n.Type and n.Left.Type are used heavily. Give them useful names. We generate the type word frequently. Make it a closure. (We don't want to generate it up front, since there are some code paths that don't need it, and generating it has side-effects.) Simplify and document the final call construction. Follow-up to address feedback on CL 147360. Change-Id: I251134a55cf80d8b1676280a345d150f2288c09a Reviewed-on: https://go-review.googlesource.com/c/147538 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Martin Möhrmann <moehrmann@google.com>
-
Richard Musiol authored
With this change, callbacks returned by syscall/js.NewCallback get executed synchronously. This is necessary for the APIs of many JavaScript libraries. A callback triggered during a call from Go to JavaScript gets executed on the same goroutine. A callback triggered by JavaScript's event loop gets executed on an extra goroutine. Fixes #26045 Fixes #27441 Change-Id: I591b9e85ab851cef0c746c18eba95fb02ea9e85b Reviewed-on: https://go-review.googlesource.com/c/142004Reviewed-by: Cherry Zhang <cherryyz@google.com> Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Martin Möhrmann authored
Create a new node for OSLICEHEADER nodes to ensure typechecks are applied. Add nil checks for OSLICEHEADER type and pointer parameters for better error messages when these are not set. Improve formatting of OSLICEHEADER nodes in compiler error messages. Change-Id: Idea8f41bb4beb636f0e1fc381ff8d79b1d44fbae Reviewed-on: https://go-review.googlesource.com/c/146997 Run-TryBot: Martin Möhrmann <moehrmann@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
-
Ian Lance Taylor authored
It can be used to set the Go language version used by the module. RELNOTES=yes Updates #28221 Change-Id: Ief0dd185c01195a17be20dff8627c80943c436e7 Reviewed-on: https://go-review.googlesource.com/c/147282 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 authored
When creating a go.mod file, add a go statement mentioning the current Go version. We can be reasonably confident that the current version is able to build the module. This is as described in the language transition proposal at https://golang.org/issue/28221. Updates #28221 Change-Id: I70a99b3a53f4b6c0288da07473c5a71bb28cd86f Reviewed-on: https://go-review.googlesource.com/c/147281 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
-
- 09 Nov, 2018 1 commit
-
-
Ainar Garipov authored
This issue was found by the new vet's nilness check. _defer was already checked against nil, so don't check it again. Change-Id: I78725eaec7234b262b3c941e06441ca57f82bdd9 Reviewed-on: https://go-review.googlesource.com/c/148917 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-