In interface-to-concrete comparisons, we are short circuiting on the interface
value's dynamic type before evaluating the concrete expression for side effects,
causing concrete expression won't panic at runtime, while it should.
To fix it, evaluating the RHS of comparison before we do the short-circuit.
We also want to prioritize panics in the LHS over the RHS, so evaluating
the LHS too.
Fixes#32187
Change-Id: I15b58a523491b7fd1856b8fdb9ba0cba5d11ebb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/178817
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The only place set releasetime to negative is in runtime.selectgo
(when blockprofilerate greater than zero), so we are safe in compiler
code.
But scasetype must keep in sync with runtime/select.go scase struct, so
releasetime must be int64.
Change-Id: I39ea944f5f2872452d3ffd57f7604d51e0d2590a
Reviewed-on: https://go-review.googlesource.com/c/go/+/179799
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Drops support for old escape analysis pass. Subsequent, separate CL
will remove dead code.
While here, fix a minor error in fmt.go: it was still looking for
esc.go's NodeEscState in n.Opt() rather than escape.go's EscLocation.
But this only affected debug diagnostics printed during escape
analysis itself.
Change-Id: I62512e1b31c75ba0577550a5fd7824abc3159ed5
Reviewed-on: https://go-review.googlesource.com/c/go/+/187597
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing condition is long and repetitive. Using select/case with
multiple values in the expression list is more concise and clearer.
Change-Id: I43f8abcf958e433468728f1d89ff1436332b29da
Reviewed-on: https://go-review.googlesource.com/c/go/+/188519
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current implementation allows multiple calls `Close` and `CloseWithError` in every side of the pipe, as a result, the original error can be overwritten.
This CL fixes this behavior adding an error existence check on `atomicError` type
and keeping the first error still available.
Fixes#24283
Change-Id: Iefe8f758aeb775309424365f8177511062514150
GitHub-Last-Rev: b559540d7a
GitHub-Pull-Request: golang/go#33239
Reviewed-on: https://go-review.googlesource.com/c/go/+/187197
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current implementation panics on nil certificates,
so introduce a nil check and early return true if both
are nil, false if only one is.
Fixes#28743
Change-Id: I71b0dee3e505d3ad562a4470ccc22c3a2579bc52
Reviewed-on: https://go-review.googlesource.com/c/go/+/167118
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Include references in the package-level comment block, expand
the obscure IRO acronym, and add a reference for "the standard
(cryptographic) assumptions".
Fixes#33589
Change-Id: I76c3b0a2f7258b3ab4bf1c8e7681c5d159720a20
GitHub-Last-Rev: 30d5a1e2fb
GitHub-Pull-Request: golang/go#33723
Reviewed-on: https://go-review.googlesource.com/c/go/+/190840
Reviewed-by: Filippo Valsorda <filippo@golang.org>
On linux/riscv64, the renameat syscall no longer exists and has been
superseded by renameat2. Thus we'll have to use Renameat2 to implement
Renameat on linux/riscv64 for #27532. Prepare for this by moving the
Renameat definition to the GOARCH specific files.
Follow CL 157899 which did the same for golang.org/x/sys/unix
Change-Id: I9670213cc3987df48fee962ddee36915a7785560
Reviewed-on: https://go-review.googlesource.com/c/go/+/192077
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
math/big.Int Cmp method does not have a fast path for the case if x and y are the same.
Fixes#30856
Change-Id: Ia9a5b5f72db9d73af1b13ed6ac39ecff87d10393
Reviewed-on: https://go-review.googlesource.com/c/go/+/178957
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Stack object symbol is named as <FunctionName>.stkobj. If the
function itself is not DUPOK, its stack object symbol should only
be defined in the package where the function is defined,
therefore no duplicates.
If in the future we change the stack object symbol to
content-hash naming, as other gcdata symbols, we can mark it as
DUPOK.
Change-Id: I5aee96578940e2f76e7115d96cd2716021672c03
Reviewed-on: https://go-review.googlesource.com/c/go/+/191437
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The net/url error in question now quotes the URL, so update the expected
output string.
While at it, also update a comment in httputil, though that doesn't
affect any test.
Fixes#33910.
Change-Id: I0981f528b24337c2952ef60c0db3b7ff72d72110
Reviewed-on: https://go-review.googlesource.com/c/go/+/192078
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Current implementation doesn't always make it obvious what the exact
problem with the URL is, so this makes it clearer by consistently quoting
the invalid URL, as is the norm in other parsing implementations, eg.:
strconv.Atoi(" 123") returns an error: parsing " 123": invalid syntax
Updates #29261
Change-Id: Icc6bff8b4a4584677c0f769992823e6e1e0d397d
GitHub-Last-Rev: 648b9d93fe
GitHub-Pull-Request: golang/go#29384
Reviewed-on: https://go-review.googlesource.com/c/go/+/185117
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Per the code review guidelines: "Words in names that are
initialisms or acronyms have a consistent case."
Change-Id: I347b02d2f48455f2cbbc040191ba197e3e8f23fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/191970
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Production profiling shows ~15% of url.Parse time being spend in the overhead
of calling strings.IndexByte through strings.Index instead of calling
strings.IndexByte directly.
name old time/op new time/op delta
Split 15.5ns ± 2% 10.7ns ± 3% -30.98% (p=0.000 n=20+19)
Change-Id: Ie25dd4afa93539a1335a91ab2a4a367f97bd3df0
Reviewed-on: https://go-review.googlesource.com/c/go/+/178877
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
golang.org/cl/143180 changed gc.eqtype to types.Identical, but gc.eqtype
is still mentioned in some comments. Remove them and update comments to
mention new functions instead.
Change-Id: I6c5eece5221f524556ee55db80de0e4bdfaf166e
Reviewed-on: https://go-review.googlesource.com/c/go/+/191357
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Extend the optimization introduced in CL 141118 to the wasm architecture.
And for reference, the rules trigger 212 times while building std and cmd
$GOOS=js GOARCH=wasm gotip build std cmd
$grep -E "Wasm.rules:44(1|2|3|4)" rulelog | wc -l
212
Updates #26498
Change-Id: I153684a2b98589ae812b42268da08b65679e09d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/185477
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Richard Musiol <neelance@gmail.com>
When an invalid URL was passed to DumpRequestOut, it would directly return
without gracefully shutting down the reader goroutine.
So we create a channel and signal the reader goroutine to exit
if an error occurs during roundtrip.
Fixes#32571
Change-Id: I8c2970f1601e599f3d1ebfed298faf5f5716fc2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/182037
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Use the shiftIsBounded function to generate more efficient
Shift instructions.
Updates #25167
Change-Id: Id350f8462dc3a7ed3bfed0bcbea2860b8f40048a
Reviewed-on: https://go-review.googlesource.com/c/go/+/182558
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Richard Musiol <neelance@gmail.com>
This CL optimizes math.bits.TrailingZeros16 on 386 with
a pair of BSFL and ORL instrcutions.
The case TrailingZeros16-4 of the benchmark test in
math/bits shows big improvement.
name old time/op new time/op delta
TrailingZeros16-4 1.55ns ± 1% 0.87ns ± 1% -43.87% (p=0.000 n=50+49)
Change-Id: Ia899975b0e46f45dcd20223b713ed632bc32740b
Reviewed-on: https://go-review.googlesource.com/c/go/+/189277
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This CL performs the branchelim optimization on WASM with its
select instruction. And the total size of pkg/js_wasm decreased
about 80KB by this optimization.
Change-Id: I868eb146120a1cac5c4609c8e9ddb07e4da8a1d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/190957
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Richard Musiol <neelance@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This patch supports the EL0 and EL1 system registers used in MRS/MSR
instructions. This patch refactors the assembler code, allowing the
assembler to read system register information from the automatically
generated sysRegEnc.go file and move existing declared system registers
to the sysRegEnc.go file.
This patch adds 431 system registers, it is worth noting that the number
of special registers is initialized to less than 1024 in the list7.go file.
This CL also adds some test cases to test the newly added system registers.
The test cases are contributed by Dianhong Xu <Dianhong.Xu@arm.com>
Change-Id: Ic09a937eaaeefe82bd08b5dd726808f8ff6cebf6
Reviewed-on: https://go-review.googlesource.com/c/go/+/189577
Reviewed-by: Ben Shi <powerman1st@163.com>
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We never used it, might as well get rid of it.
Change-Id: I5c23c93e90173bff9ac1fc1b8ae1e2025215d6eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/191938
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
SSLv3 has been irreparably broken since the POODLE attack 5 years ago
and RFC 7568 (f.k.a. draft-ietf-tls-sslv3-diediedie) prohibits its use
in no uncertain terms.
As announced in the Go 1.13 release notes, remove support for it
entirely in Go 1.14.
Updates #32716
Change-Id: Id653557961d8f75f484a01e6afd2e104a4ccceaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/191976
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
'git ls-remote' started recognizing the '--' separator at some point
after 2.7.4, but git defaults to version 2.7.4 on Ubuntu 16.04 LTS,
which remains supported by Ubuntu until April 2021.
We added '--' tokens to most VCS commands as a defensive measure in
CL 181237, but it isn't strictly necessary here because the 'scheme'
argument to our template is chosen from a predefined list: we can
safely drop it to retain compatibility.
Fixes#33836
Updates #26746
Change-Id: Ibb53366b95f8029b587e0b7646a439330d759ac7
Reviewed-on: https://go-review.googlesource.com/c/go/+/191978
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
The TestParseErrors test function was not strict with unwanted errors
received from url.Parse(). It was not failing in such cases, now it does
Fixes#33646
Updates #29098
Change-Id: I069521093e2bff8b1fcd41ffd3f9799f3108bc61
GitHub-Last-Rev: e6844c57f9
GitHub-Pull-Request: golang/go#33876
Reviewed-on: https://go-review.googlesource.com/c/go/+/191966
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Because the Node AST represents references to declared objects (e.g.,
variables, packages, types, constants) by directly pointing to the
referred object, we don't have use-position info for these objects.
For switch statements with duplicate cases, we report back where the
first duplicate value appeared. However, due to the AST
representation, if the value was a declared constant, we mistakenly
reported the constant declaration position as the previous case
position.
This CL reports back against the 'case' keyword's position instead, if
there's no more precise information available to us.
It also refactors code to emit the same "previous at" error message
for duplicate values in map literals.
Thanks to Emmanuel Odeke for the test case.
Fixes#33460.
Change-Id: Iec69542ccd4aad594dde8df02d1b880a422c5622
Reviewed-on: https://go-review.googlesource.com/c/go/+/188901
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use efaceOf to safely convert from *interface{} to *_eface, and to
make it clearer what the pointer arithmetic is computing.
Incidentally, remove a spurious unsafe.Pointer->*uint8->unsafe.Pointer
round trip conversion in newproc.
No behavior change.
Change-Id: I2ad9d791d35d8bd008ef43b03dad1589713c5fd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/190457
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
I'm trying to keep the code changes minimal for backporting to Go 1.13,
so it is still possible for a handful of entries to leak,
but the leaks are now O(1) instead of O(N) in the steady state.
Longer-term, I think it would be a good idea to coalesce idleMu with
connsPerHostMu and clear entries out of both queues as soon as their
goroutines are done waiting.
Fixes#33849Fixes#33850
Change-Id: Ia66bc64671eb1014369f2d3a01debfc023b44281
Reviewed-on: https://go-review.googlesource.com/c/go/+/191964
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The assembly output for x & c == c, where c is power of 2:
MOVQ "".set+8(SP), AX
ANDQ $8, AX
CMPQ AX, $8
SETEQ "".~r2+24(SP)
With optimization using bitset:
MOVQ "".set+8(SP), AX
BTL $3, AX
SETCS "".~r2+24(SP)
output less than 1 instruction.
However, there is no speed improvement:
name old time/op new time/op delta
AllBitSet-8 0.35ns ± 0% 0.35ns ± 0% ~ (all equal)
Fixes#31904
Change-Id: I5dca4e410bf45716ed2145e3473979ec997e35d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/175957
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The decoder called this function to check numbers being decoded into a
json.Number. However, these can't be quoted as strings, so the tokenizer
has already verified they are valid JSON numbers.
Verified this by adding a test with such an input. As expected, it
produces a syntax error, not the fmt.Errorf - that line could never
execute.
Since the only remaining non-test caller of isvalidnumber is in
encode.go, move the function there.
This change should slightly reduce the amount of work when decoding into
json.Number, though that isn't very common nor part of any current
benchmarks.
Change-Id: I67a1723deb3d18d5b542d6dd35f3ae56a43f23eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/184817
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There is an unnecessary lower operation in isJSType.
Simple logic fix can improve tiny performance.
name old time/op new time/op delta
isJSType-8 152ns ± 0% 58ns ± 7% -61.82% (p=0.001 n=6+8)
name old alloc/op new alloc/op delta
isJSType-8 32.0B ± 0% 0.0B -100.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
isJSType-8 1.00 ± 0% 0.00 -100.00% (p=0.000 n=8+8)
Change-Id: I281aadf1677d4377920c9649af206381189a27e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/177118
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This reverts https://golang.org/cl/185080.
Reason for revert: some new changes are erroring again, so this broke the builders.
Change-Id: I28da16da98b90cefbb47173d31bbbb56e43062d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/191781
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
state and ssafn both have their own Fatalf, so use them instead of
global Fatalf.
Updates #19683
Change-Id: Ie02a961d4285ab0a3f3b8d889a5b498d926ed567
Reviewed-on: https://go-review.googlesource.com/c/go/+/188539
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
First, add cpu and memory profiling flags, as these are useful to see
where rulegen is spending its time. It now takes many seconds to run on
a recent laptop, so we have to keep an eye on what it's doing.
Second, stop writing '_ = var' lines to keep imports and variables used
at all times. Now that rulegen removes all such unused names, they're
unnecessary.
To perform the removal, lean on go/types to first detect what names are
unused. We can configure it to give us all the type-checking errors in a
file, so we can collect all "declared but not used" errors in a single
pass.
We then use astutil.Apply to remove the relevant nodes based on the line
information from each unused error. This allows us to apply the changes
without having to do extra parser+printer roundtrips to plaintext, which
are far too expensive.
We need to do multiple such passes, as removing an unused variable
declaration might then make another declaration unused. Two passes are
enough to clean every file at the moment, so add a limit of three passes
for now to avoid eating cpu uncontrollably by accident.
The resulting performance of the changes above is a ~30% loss across the
table, since go/types is fairly expensive. The numbers were obtained
with 'benchcmd Rulegen go run *.go', which involves compiling rulegen
itself, but that seems reflective of how the program is used.
name old time/op new time/op delta
Rulegen 5.61s ± 0% 7.36s ± 0% +31.17% (p=0.016 n=5+4)
name old user-time/op new user-time/op delta
Rulegen 7.20s ± 1% 9.92s ± 1% +37.76% (p=0.016 n=5+4)
name old sys-time/op new sys-time/op delta
Rulegen 135ms ±19% 169ms ±17% +25.66% (p=0.032 n=5+5)
name old peak-RSS-bytes new peak-RSS-bytes delta
Rulegen 71.0MB ± 2% 85.6MB ± 2% +20.56% (p=0.008 n=5+5)
We can live with a bit more resource usage, but the time/op getting
close to 10s isn't good. To win that back, introduce concurrency in
main.go. This further increases resource usage a bit, but the real time
on this quad-core laptop is greatly reduced. The final benchstat is as
follows:
name old time/op new time/op delta
Rulegen 5.61s ± 0% 3.97s ± 1% -29.26% (p=0.008 n=5+5)
name old user-time/op new user-time/op delta
Rulegen 7.20s ± 1% 13.91s ± 1% +93.09% (p=0.008 n=5+5)
name old sys-time/op new sys-time/op delta
Rulegen 135ms ±19% 269ms ± 9% +99.17% (p=0.008 n=5+5)
name old peak-RSS-bytes new peak-RSS-bytes delta
Rulegen 71.0MB ± 2% 226.3MB ± 1% +218.72% (p=0.008 n=5+5)
It might be possible to reduce the cpu or memory usage in the future,
such as configuring go/types to do less work, or taking shortcuts to
avoid having to run it many times. For now, ~2x cpu and ~4x memory usage
seems like a fair trade for a faster and better rulegen.
Finally, we can remove the old code that tried to remove some unused
variables in a hacky and unmaintainable way.
Change-Id: Iff9e83e3f253babf5a1bd48cc993033b8550cee6
Reviewed-on: https://go-review.googlesource.com/c/go/+/189798
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This removes a special case that was added to fix issue #10956, but that
was never actually effective. The code in the test case still fails to
read, so perhaps the zip64 support added in CL 6463050 inadvertently
caught this particular case.
It's possible that the original theorized bug still exists, but I'm not
convinced it was ever fixed.
Update #28700
Change-Id: I4854de616364510f64a6def30b308686563f8dbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/179757
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Template.New calls t.init, which allocates several items that
are immediately rewritten by copy, so avoid the call to New
Change-Id: I16c7cb001bbcd14cf547c1a2db2734a2f8214e7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/182757
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The TestParseErrors test function was not strict with unwanted errors
received from url.Parse(). It was not failing in such cases, now it does.
Change-Id: I18a26a68c1136f5c762989a76e04b47e33dd35f1
GitHub-Last-Rev: c33f9842f7
GitHub-Pull-Request: golang/go#32954
Reviewed-on: https://go-review.googlesource.com/c/go/+/185080
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TransmitFile does not allow for number of bytes that can be
transmitted to be larger than 2147483646. See
https://docs.microsoft.com/en-us/windows/win32/api/mswsock/nf-mswsock-transmitfile
for details. So adjust sendFile accordingly.
No test added, because this would require creating large file
(more than 2GB file).
Fixes#33193.
Change-Id: I82e0cb104d112264e4ea363bb20b6d02ac30b38e
Reviewed-on: https://go-review.googlesource.com/c/go/+/187037
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
After Go.1.10+ strings.Builder is known as more efficient in
concatenating and building strings than bytes.Buffer.
In this CL,
there is a minor logic fix for getting advantage of strings.builder.
name old time/op new time/op delta
DefinedTemplate-8 543ns ± 3% 512ns ± 2% -5.73% (p=0.000 n=8+8)
name old alloc/op new alloc/op delta
DefinedTemplate-8 192B ± 0% 160B ± 0% -16.67% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
DefinedTemplate-8 5.00 ± 0% 5.00 ± 0% ~ (all equal)
Change-Id: Icda0054d146e6c5e32ed8a4d13221bb6850d31b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/175261
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add brief descriptions of why one might implement
an Is or As method.
Fixes#33364.
Change-Id: I81a091bf564c654ddb9ef3997e780451a01efb7a
Reviewed-on: https://go-review.googlesource.com/c/go/+/191338
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Because TestUnmarshal actually allocates a new value to decode into
using ptr's pointer type, any existing data is thrown away. This was
harmless in alomst all of the test cases, minus the "overwriting of
data" ones added in 2015 in CL 12209.
I spotted that nothing covered decoding a JSON array with few elements
into a slice which already had many elements. I initially assumed that
the code was buggy or that some code could be removed, when in fact
there simply wasn't any code covering the edge case.
Move those two tests to TestPrefilled, which already served a very
similar purpose. Remove the map case, as TestPrefilled already has
plenty of prefilled map cases. Moreover, we no longer reset an entire
map when decoding, as per the godoc:
To unmarshal a JSON object into a map, Unmarshal first
establishes a map to use. If the map is nil, Unmarshal allocates
a new map. Otherwise Unmarshal reuses the existing map, keeping
existing entries.
Finally, to ensure that ptr is used correctly in the future, make
TestUnmarshal error if it's anything other than a pointer to a zero
value. That is, the only correct use should be new(type). Don't rename
the ptr field, as that would be extremely noisy and cause unwanted merge
conflicts.
Change-Id: I41e3ecfeae42d877ac5443a6bd622ac3d6c8120c
Reviewed-on: https://go-review.googlesource.com/c/go/+/185738
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
rulegen.go produces plaintext Go code directly, which was fine for a
while. However, that's started being a bottleneck for making code
generation more complex, as we can only generate code directly one line
at a time.
Some workarounds were used, like multiple layers of buffers to generate
chunks of code, to then use strings.Contains to see whether variables
need to be defined or not. However, that's error-prone, verbose, and
difficult to work with.
A better approach is to generate an intermediate syntax tree in memory,
which we can inspect and modify easily. For example, we could run a
number of "passes" on the syntax tree before writing to disk, such as
removing unused variables, simplifying logic, or moving declarations
closer to their uses.
This is the first step in that direction, without changing any of the
generated code. We didn't use go/ast directly, as it's too complex for
our needs. In particular, we only need a few kinds of simple statements,
but we do want to support arbitrary expressions. As such, define a
simple set of statement structs, and add thin layers for printer.Fprint
and ast.Inspect.
A nice side effect of this change, besides removing some buffers and
string handling, is that we can now avoid passing so many parameters
around. And, while we add over a hundred lines of code, the tricky
pieces of code are now a bit simpler to follow.
While at it, apply some cleanups, such as replacing isVariable with
token.IsIdentifier, and consistently using log.Fatalf.
Follow-up CLs will start improving the generated code, also simplifying
the rulegen code itself. I've added some TODOs for the low-hanging fruit
that I intend to work on right after.
Updates #30810.
Change-Id: Ic371c192b29c85dfc4a001be7fbcbeec85facc9d
Reviewed-on: https://go-review.googlesource.com/c/go/+/177539
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
HTTP is an initialism, not an acronym, where you pronounce each letter as a
word. It's "an H", not "a H".
Running `find src/net/http -type f | xargs grep -n 'an HTTP' | wc -l` shows
that the "an HTTP" form is used 67 times across the `net/http` package.
Furthermore, `find src/net/http -type f | xargs grep -n 'a HTTP' | wc -l`
yields only 4 results.
Change-Id: I219c292a9e2c9bf7a009dbfe82ea8b15874685e9
GitHub-Last-Rev: 6ebd095023
GitHub-Pull-Request: golang/go#33810
Reviewed-on: https://go-review.googlesource.com/c/go/+/191700
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
After golang.org/cl/33895, function adjustctx can not be inlined,
cost 82 exceeds budget 80
Change-Id: Ie559ed80ea2c251add940a99f11b2983f6cbddbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/187977
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Array accesses with index types smaller than the machine word size may
involve a sign or zero extension of the index value before bounds
checking. Currently, this defeats prove because the facts about the
original index value don't flow through the sign/zero extension.
This CL fixes this by looking back through value-preserving sign/zero
extensions when adding facts via Update and, where appropriate, applying
the same facts using the pre-extension value. This fix is enhanced by
also looking back through value-preserving extensions within
ft.isNonNegative to infer whether the extended value is known to be
non-negative. Without this additional isNonNegative enhancement, this
logic is rendered significantly less effective by the limitation
discussed in the next paragraph.
In Update, the application of facts to pre-extension values is limited
to cases where the domain of the new fact is consistent with the type of
the pre-extension value. There may be cases where this cross-domain
passing of facts is valid, but distinguishing them from the invalid
cases is difficult for me to reason about and to implement.
Assessing which cases to allow requires details about the context and
inferences behind the fact being applied which are not available
within Update. Additional difficulty arises from the fact that the SSA
does not curently differentiate extensions added by the compiler for
indexing operations, extensions added by the compiler for implicit
conversions, or explicit extensions from the source.
Examples of some cases that would need to be filtered correctly for
cross-domain facts:
(1) A uint8 is zero-extended to int for indexing (a value-preserving
zeroExt). When, if ever, can signed domain facts learned about the int be
applied to the uint8?
(2) An int8 is sign-extended to int16 (value-preserving) for an equality
comparison. Equality comparison facts are currently always learned in both
the signed and unsigned domains. When, if ever, can the unsigned facts
learned about the int16, from the int16 != int16 comparison, be applied
to the original int8?
This is an alternative to CL 122695 and CL 174309. Compared to CL 122695,
this CL differs in that the facts added about the pre-extension value will
pass through the Update method, where additional inferences are processed
(e.g. fence-post implications, see #29964). CL 174309 is limited to bounds
checks, so is narrower in application, and makes the code harder to read.
Fixes#26292.
Fixes#29964.
Fixes#15074
Removes 238 bounds checks from std/cmd.
Change-Id: I1f87c32ee672bfb8be397b27eab7a4c2f304893f
Reviewed-on: https://go-review.googlesource.com/c/go/+/174704
Run-TryBot: Zach Jones <zachj1@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
directlyAssignable invoked rtype.Name() just to compare its result
to empty string. We really only need to check whether rtype has
name. It can be done much cheaper, by checking tflagNamed.
Benchmark: https://play.golang.org/p/V2BzESPuf2w
name old time/op new time/op delta
DirectlyAssignable-12 32.7ns ± 6% 6.6ns ± 6% -79.80% (p=0.008 n=5+5)
Fixes#32186
Change-Id: I1a2a167dbfddf319fba3015cb6a011bf010f99a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/178518
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Don't skip closing parentheses of any kind after a missing
expression. They are likely part of the lexical construct
enclosing the expression.
Fixes#33386.
Change-Id: Ic0abc2037ec339a345ec357ccc724b7ad2a64c00
Reviewed-on: https://go-review.googlesource.com/c/go/+/188502
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Some constants were added above the initial copyright blurb, and then
later a new copyright blurb was added on top of that. So we wound up
with two header sections, one of which contained a useful comment that
became obscured. This commit fixes up that mistake.
Change-Id: I8b9b8c34495cdceae959e151e8ccdee3137f6ca4
Reviewed-on: https://go-review.googlesource.com/c/go/+/191841
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Quietly drop duplicate methods inherited from embedded interfaces if
they have an identical signature to existing methods.
Updates #6977.
Change-Id: I144151cb7d99695f12b555c0db56207993c56284
Reviewed-on: https://go-review.googlesource.com/c/go/+/187519
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Move checkdupfields call into expandiface, and inline/simplify offmod.
More prep work for implementing #6977.
Change-Id: I958ae87f761ec25a8fa7298a2a3019eeca5b25ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/187518
Reviewed-by: Robert Griesemer <gri@golang.org>
Allows avoiding the Type.Fields call, which affects prevents
checkdupfields from being called at the more natural point during
dowidth.
Change-Id: I724789c860e7fffba1e8e876e2d74dcfba85d75c
Reviewed-on: https://go-review.googlesource.com/c/go/+/187517
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The stack of delayed actions is grown by pushing a new action
on top with Checker.later. Checker.processDelayed processes
all actions above a top watermark and then resets the stack
to top.
Until now, pushed actions above the watermark were processed
in LIFO order. This change processes them in FIFO order, which
seems more natural (if an action A was delayed before an action
B, A should be processed before B for that stack segment).
(With this change, Checker.later could be used instead of
Checker.atEnd to postpone interface method type comparison
and then the specific example in issue #33656 does type-check.
However, in general we want interface method type comparisons
to run after all interfaces are completed. With Checker.later
we may still end up mixing interface completions and interface
method type comparisons in ways leading to other errors for
sufficiently convoluted code.)
Also, move Checker.processDelayed from resolver.go to check.go.
Change-Id: Id31254605e6944c490eab410553fff907630cc64
Reviewed-on: https://go-review.googlesource.com/c/go/+/191458
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Introduce a new list of final actions that is executed at the
end of type checking and use it to collect method type comparisons
and also map key checks.
Fixes#33656.
Change-Id: Ia77a35a45a9d7eaa7fc3e9e19f41f32dcd6ef9d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/191418
Reviewed-by: Alan Donovan <adonovan@google.com>
When creating a new interface via the exported API calls, a shared
empty and completed Interface value is returned if there are no
methods or embedded interfaces. This is a minor optimization and
matches the internal behavior when creating empty interfaces.
Since calling Interface.Complete is idempotent, and since there
are no other legitimate ways to create Interface values externally
but via NewInterface/NewInterfaceType calls, and completed Interfaces
are considered "immutable", this change is not expected to affect
clients. The only observable behavior that changed is the string
value for empty interfaces created via the above API calls; those
empty interfaces now don't show "incomplete" anymore even before
Interface.Complete is called. Except in special test cases, this
behavior is unlikely to affect clients.
Change-Id: Idf7f2cd112241c5b81a43b4544bbe3f2e003d8d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/191417
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Quietly drop duplicate methods from embedded interfaces
if they have an identical signature to existing methods.
Instead of adjusting the prior syntax-based only method set
computation where methods don't have signature information
(and thus where de-duplication according to the new rules
would have been somewhat tricky to get right), this change
completely rewrites interface method set computation, taking
a page from the cmd/compiler's implementation. In a first
pass, when type-checking interfaces, explicit methods and
embedded interfaces are collected, but the interfaces are
not "expanded", that is the final method set computation
is done lazily, either when needed for method lookup, or
at the end of type-checking.
While this is a substantial rewrite, it allows us to get
rid of the separate (duplicate and delicate) syntactical
method set computation and generally simplifies checking
of interface types significantly. A few (esoteric) test
cases now have slightly different error messages but all
tests that are accepted by cmd/compile are also accepted
by go/types.
(This is a replacement for golang.org/cl/190258.)
Updates #6977.
Change-Id: Ic8b9321374ab4f617498d97c12871b69d1119735
Reviewed-on: https://go-review.googlesource.com/c/go/+/191257
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Specifying -halt in `go test -run Check$ -halt` causes a panic
upon encountering the first error. The stack trace is useful to
determine what code path issued the error.
Change-Id: I2e17e0014ba87505b01786980b98565f468065bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/190257
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The doc comments of both ServerContextKey and LocalAddrContextKey both suggest that context.WithValue can be used to access (get) properties of the server or connection. This PR fixes those comments to refer to Context.Value instead.
Change-Id: I4ed383ef97ba1951f90c555243007469cfc18d4d
GitHub-Last-Rev: 05bc3acf82
GitHub-Pull-Request: golang/go#33833
Reviewed-on: https://go-review.googlesource.com/c/go/+/191838
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 739123c3a3.
Reason for revert: broke Windows and Plan 9 builders
Fixes#33828
Change-Id: I1d85c81549b1b34924fdd0ade8bf9406e5cf6555
Reviewed-on: https://go-review.googlesource.com/c/go/+/191742
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Update vendored x/arch repo to pick up the fix of issue #33802.
This is done with the following commands:
$ cd $GOROOT/src/cmd
$ go get -d golang.org/x/arch@latest
go: finding golang.org/x/arch latest
go: downloading golang.org/x/arch v0.0.0-20190815191158-8a70ba74b3a1
go: extracting golang.org/x/arch v0.0.0-20190815191158-8a70ba74b3a1
$ go mod tidy
$ go mod vendor
Fixes#33802.
Change-Id: I0a44f1d83d6f573124cea1f099378b1c851f3feb
Reviewed-on: https://go-review.googlesource.com/c/go/+/191619
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The -m flag is removed in Go 1.13. -d should be used instead.
Change-Id: Ia53764748309f16cb231e5ac6770400a73804484
Reviewed-on: https://go-review.googlesource.com/c/go/+/191621
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Update golang.org/x/net to v0.0.0-20190813141303-74dc4d7220e7 to import
the following security fix.
commit 74dc4d7220e7acc4e100824340f3e66577424772
Author: Filippo Valsorda <filippo@golang.org>
Date: Sun Aug 11 02:12:18 2019 -0400
http2: limit number of control frames in server send queue
An attacker could cause servers to queue an unlimited number of PING
ACKs or RST_STREAM frames by soliciting them and not reading them, until
the program runs out of memory.
Limit control frames in the queue to a few thousands (matching the limit
imposed by other vendors) by counting as they enter and exit the scheduler,
so the protection will work with any WriteScheduler.
Once the limit is exceeded, close the connection, as we have no way to
communicate with the peer.
Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
This change was generated with cmd/go and cmd/bundle:
$ go get -u golang.org/x/net
$ go mod tidy
$ go mod vendor
$ go generate net/http
Fixes CVE-2019-9512 and CVE-2019-9514
Fixes#33606
Change-Id: I464baf96175006aa101d65d3b0f6494f28a626ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/190137
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
- Add doc to syscall.Errno (and syscall.ErrorString for plan9).
- Mention under `syscall` in release notes.
Fixes#33436.
Change-Id: I032ffebaa76ed67eb9d748e7645ca73f26144ea0
Reviewed-on: https://go-review.googlesource.com/c/go/+/191337
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As of Go 1.13rc1, TimeoutHandler supports the Flusher and Pusher interfaces and
this change corrects its documentation to say that.
Fixes#33769
Updates #29193
Change-Id: Ia0523f7f2e3dc1f8f0b68950b85a7bf81c4abe60
GitHub-Last-Rev: 5310d2c960
GitHub-Pull-Request: golang/go#33770
Reviewed-on: https://go-review.googlesource.com/c/go/+/191237
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts CL 151157.
CL 151157 introduced a crash when decoding into ",string" fields. It
came with a moderate speedup, so at this stage of the release cycle
let's just revert it, and reapply it in Go 1.14 with the fix in CL 190659.
Also applied the test cases from CL 190659.
Updates #33728
Change-Id: Ie46e2bc15224b251888580daf6b79d5865f3878e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190909
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This CL makes the go command understand that
GOSUMDB=sum.golang.google.cn should connect
to that domain but expect to find a checksum database
signed by sum.golang.org there.
The host sum.golang.google.cn is not yet completely
configured; we hope it will be available in a few weeks.
Change-Id: Ie0fc4323f0c7084dda59bd3b45fc406717fa16d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/191137
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
The docs refer to "the last two paragraphs", but in fact should refer to
the first two of the previous three paragraphs. Moved up the out of place
paragraph.
Updates #14295
Change-Id: I066da7a665bc6754d246782b941af214a385017a
Reviewed-on: https://go-review.googlesource.com/c/go/+/190839
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently test build fails with:
$ go test -tags=gofuzz encoding/json
encoding/json/fuzz.go:36:4: Println call has possible formatting directive %s
FAIL encoding/json [build failed]
Change-Id: I23aef44a421ed0e7bcf48b74ac5a8c6768a4841b
Reviewed-on: https://go-review.googlesource.com/c/go/+/190698
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 131196 optimized Time.Sub, but was reverted because
it incorrectly computed the nanoseconds in some edge cases.
This CL adds a test case to enforce the correct behavior
so that a future optimization does not break this again.
Updates #17858
Updates #33677
Change-Id: I596d8302ca6bf721cf7ca11cc6f939639fcbdd43
Reviewed-on: https://go-review.googlesource.com/c/go/+/190524
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 162337 changed go/ast to better handle block comments,
but was reverted because it introduced an off-by-one bug.
This CL adds a test case to enforce the correct behavior
so that future changes do not break this again.
Updates #18929
Updates #33538
Change-Id: I2d25c139d007f8db1091b7a48b1dd20c584e2699
Reviewed-on: https://go-review.googlesource.com/c/go/+/190523
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This reverts commit CL 131196 because there is a bug
in the calculation of nanoseconds.
Fixes#33677
Change-Id: Ic8e94c547ee29b8aeda1b9a5cb9764dbf47b14b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/190497
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
When calling a function obtained from reflect.Value.Method (or
MethodByName), we copy the arguments from the caller frame, which
does not include the receiver, to a new frame to call the actual
method, which does include the receiver. Here we need to align
the first (non-receiver) argument. As the receiver is pointer
sized, it is generally naturally aligned, except on amd64p32,
where the argument can have larger alignment, and this aligning
becomes necessary.
Fixes#33628.
Change-Id: I5bea0e20173f06d1602c5666d4f334e3d0de5c1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190297
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When Host is not valid per RFC 3986, the behavior of Hostname and Port
was wildly unpredictable, to the point that Host could have a suffix
that didn't appear in neither Hostname nor Port.
This is a security issue when applications are applying checks to Host
and expecting them to be meaningful for the contents of Hostname.
To reduce disruption, this change only aims to guarantee the following
two security-relevant invariants.
* Host is either Hostname or [Hostname] with Port empty, or
Hostname:Port or [Hostname]:Port.
* Port is only decimals.
The second invariant is the one that's most likely to cause disruption,
but I believe it's important, as it's conceivable an application might
do a suffix check on Host and expect it to be meaningful for the
contents of Hostname (if the suffix is not a valid port).
There are three ways to ensure it.
1) Reject invalid ports in Parse. Note that non-numeric ports are
already rejected if and only if the host starts with "[".
2) Consider non-numeric ports as part of Hostname, not Port.
3) Allow non-numeric ports, and hope they only flow down to net/http,
which will reject them (#14353).
This change adopts both 1 and 2. We could do only the latter, but then
these invalid hosts would flow past port checks, like in
http_test.TestTransportRejectsAlphaPort. Non-numeric ports weren't fully
supported anyway, because they were rejected after IPv6 literals, so
this restores consistency. We could do only the former, but at this
point 2) is free and might help with manually constructed Host values
(or if we get something wrong in Parse).
Note that net.SplitHostPort and net.Dial explicitly accept service names
in place of port numbers, but this is an URL package, and RFC 3986,
Section 3.2.3, clearly specifies ports as a number in decimal.
net/http uses a mix of net.SplitHostPort and url.Parse that would
deserve looking into, but in general it seems that it will still accept
service names in Addr fields as they are passed to net.Listen, while
rejecting them in URLs, which feels correct.
This leaves a number of invalid URLs to reject, which however are not
security relevant once the two invariants above hold, so can be done in
Go 1.14: IPv6 literals without brackets (#31024), invalid IPv6 literals,
hostnames with invalid characters, and more.
Tested with 200M executions of go-fuzz and the following Fuzz function.
u, err := url.Parse(string(data))
if err != nil {
return 0
}
h := u.Hostname()
p := u.Port()
switch u.Host {
case h + ":" + p:
return 1
case "[" + h + "]:" + p:
return 1
case h:
fallthrough
case "[" + h + "]":
if p != "" {
panic("unexpected Port()")
}
return 1
}
panic("Host is not a variant of [Hostname]:Port")
Fixes CVE-2019-14809
Updates #29098
Change-Id: I7ef40823dab28f29511329fa2d5a7fb10c3ec895
Reviewed-on: https://go-review.googlesource.com/c/go/+/189258
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The bundle included changes from a commit after the one referred to by
the go.mod, probably due to cmd/bundle using the GOPATH source.
Identified with the new go/packages based cmd/bundle from CL 189818.
$ go get golang.org/x/net@461777fb6f
$ go mod tidy
$ go mod vendor
$ go generate net/http # with CL 189818
Also, updated the socks_bundle.go generate command to drop obsolete
options and match h2_bundle.go. It caused no output changes.
Updates #32031
Change-Id: I0322d4e842dbfdad749455111072ca4872a62ad4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189897
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
There is real (albeit generated) code that exceeds the limit.
Fixes#33555
Change-Id: I668e85825d3d2a471970e869abe63f3492213cc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/189697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
If we don't know whether a path is a module path or a package path,
previously we would first try a module query for it, then fall back to
a package query.
If we are using a sequence of proxies with fallback (as will be the
default in Go 1.13), and the path is not actually a module path, that
initial module query will fail against the first proxy, then
immediately fall back to the next proxy in the sequence — even if the
query could have been satisfied by some other (prefix) module
available from the first proxy.
Instead, we now query the requested path as only one kind of path.
If we query it as a package path but it turns out to only exist as a
module, we can detect that as a PackageNotInModuleError with an
appropriate module path — we do not need to issue a second query to
classify it.
Fixes#31785
Change-Id: I581d44279196e41d1fed27ec25489e75d62654e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/189517
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
modload.ListModules now wraps errors as module.ModuleError as
appropriate. The resulting errors always include the module path and
will include the version, if known.
'go mod download' no longer ignores errors reported by ListModules.
Previously, it started requesting module info, go.mod, and zip. Those
requests would fail, overwriting the original failure. They were
usually less descriptive.
'go mod download' with a module not in the build list (and no version
query) is now an error. Previously, this was silently ignored.
Fixes#30743
Change-Id: Icee8c1c6c5240de135a8b6ba42d6bbcdb757cdac
Reviewed-on: https://go-review.googlesource.com/c/go/+/189323
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This reverts CL 162337.
Reason for revert: this introduces a regression
Fixes#33538
Updates #18929
Change-Id: Ib2320a840c6d3ec7912e8f414e933d04fbf11ab4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189379
Reviewed-by: Robert Griesemer <gri@golang.org>
Document goroutine label inheritance. Goroutine labels are copied upon
goroutine creation and there is a test enforcing this, but it was not
mentioned in the docstrings for `Do` or `SetGoroutineLabels`.
Add notes to both of those functions' docstrings so it's clear that one
does not need to set labels as soon as a new goroutine is spawned if
they want to propagate tags.
Updates #32223
Updates #23458
Change-Id: Idfa33031af0104b884b03ca855ac82b98500c8b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189317
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In modload.Import, confirm that the import path does not start with
"cmd/" before calling QueryPackage, which returns a less helpful
error.
In load.loadPackageData, don't wrap errors with "unknown import path".
The wrapped error should always include the import path, and it's also
repeated in the PackageError wrapper.
Fixes#31031
Change-Id: I071efa22e3842c62831d096f888a8006811fe724
Reviewed-on: https://go-review.googlesource.com/c/go/+/189157
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This mirrors the ELF fix in CL 188957. TestScript/version failed on
darwin after that change.
Fixes#31861
Change-Id: I4ce953ebec8dd5fa47e26d373c59d7e290b75a34
Reviewed-on: https://go-review.googlesource.com/c/go/+/189159
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Explain wrapping and how to use Is and As in the package doc.
Explain "chain" in Is and As.
Updates #33364.
Change-Id: Ic06362106dbd129e33dd47e63176ee5355492086
Reviewed-on: https://go-review.googlesource.com/c/go/+/188737
Reviewed-by: Rob Pike <r@golang.org>
CL 188817 improved the error message for a nonexistent version in a dependency.
This CL locks in that improvement in a regression test.
Fixes#33474
Change-Id: I6246b4995adee966f24eaebe491d35830aea8370
Reviewed-on: https://go-review.googlesource.com/c/go/+/188977
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Based on original fix from Mark Glines in golang.org/cl/186737
Fixes#31861
Change-Id: Ibd583a3aa8f8b8eefade998aa2ac757b55140937
Reviewed-on: https://go-review.googlesource.com/c/go/+/188957
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The two methods act the same, so make their documentation similar so
that people don't think they act differently.
Change-Id: If224692ef50870faf855d789380a614d1e724132
Reviewed-on: https://go-review.googlesource.com/c/go/+/188137
Reviewed-by: Rob Pike <r@golang.org>
CL #163058 moves interpretation of platform-specific errors to the
syscall package. Package syscall errors implement an Is method which
os.IsPermission etc. consult. This results in an unintended semantic
change to the os package predicate functions: The following program
now prints 'true' where it used to print 'false':
package main
import "os"
type myError struct{ error }
func (e myError) Is(target error) bool { return target == os.ErrPermission }
func main() { println(os.IsPermission(myError{})) }
Change the os package error predicate functions to only examine syscall
errors, avoiding this semantic change.
This CL does retain one minor semantic change: On Plan9, os.IsPermission
used to return true for any error with text containing the string
"permission denied". It now only returns true for a syscall.ErrorString
containing that text.
Change-Id: I6b512b1de6ced46c2f1cc8d264fa2495ae7bf9f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/188817
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The compiler can crash if the compiled code tries to
unconditionally read from a nil pointer. This should cause
the generated binary to panic, not the compiler.
Fixes#33438
Change-Id: Ic8fa89646d6968e2cc4e27da0ad9286662f8bc49
Reviewed-on: https://go-review.googlesource.com/c/go/+/188760
Reviewed-by: Austin Clements <austin@google.com>
It is unclear whether the current definition of os.IsTimeout is
desirable or not. Drop ErrTimeout for now so we can consider adding it
(or some other error) in a future release with a corrected definition.
Fixes#33411
Change-Id: I8b880da7d22afc343a08339eb5f0efd1075ecafe
Reviewed-on: https://go-review.googlesource.com/c/go/+/188758
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There's a race here with fork/exec, enable the close-on-exec flag
for the new file descriptor.
Fixes#33405
Change-Id: If95bae97a52b7026a930bb3427e47bae3b0032ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/188537
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As discussed in
https://github.com/golang/go/issues/32463#issuecomment-506833421
the classification of deadline-based timeouts as "temporary" errors is a
historical accident. I/O timeouts used to be duration-based, so they
really were temporary--retrying a timed-out operation could succeed. Now
that they're deadline-based, timeouts aren't temporary unless you reset
the deadline.
Drop ErrTemporary from Go 1.13, since its definition is wrong. We'll
consider putting it back in Go 1.14 with a clear definition and
deprecate net.OpError.Temporary.
Fixes#32463
Change-Id: I70cda664590d8872541e17409a5780da76920891
Reviewed-on: https://go-review.googlesource.com/c/go/+/188398
Reviewed-by: Jonathan Amsterdam <jba@google.com>
We shouldn't mask to desired registers if we haven't masked out all the
forbidden registers yet. In this path we haven't masked out the nospill
registers yet. If the resulting mask contains only nospill registers, then
allocReg fails.
This can only happen on resultNotInArgs-marked instructions, which exist
only on the ARM64, MIPS, MIPS64, and PPC64 ports.
Maybe there's a better way to handle resultNotInArgs instructions.
But for 1.13, this is a low-risk fix.
Fixes#33355
Change-Id: I1082f78f798d1371bde65c58cc265540480e4fa4
Reviewed-on: https://go-review.googlesource.com/c/go/+/188178
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Also add other gccgo options.
This ports CL 45695 and CL 48592 from the gofrontend repo to the gc repo.
CL 45695 (partial entry, other parts out of date and not ported):
cmd/go: gccgo: consistent results
Pass the -fdebug-prefix-map and -gno-record-gcc-switches compiler
options to gccgo to generate consistent results.
CL 48592:
cmd/go: use gccSupportsFlag for -fsplit-stack
Don't assume that all (or only) 386/amd64 compilers support
-fsplit-stack.
Fixes#33108
Change-Id: I61f9e5a67e4fb059f26750e97621d27afa566ec2
Reviewed-on: https://go-review.googlesource.com/c/go/+/187824
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Request.PostForm gets populated with form data for PATCH, POST, or PUT
http verbs.
Change-Id: I33065aa78a8470c4e9490aac830aa6f5963c61cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/187821
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
ioutil.WriteFile always truncates the destination file to 0 before
writing, which is inappropriate for unsynchronized, idempotent,
fixed-size files such as the cache entry files here.
Instead, truncate the file only after writing it, so that a second
write will never (even temporarily!) remove the contents of a
preceding write.
Fixes#29667
Change-Id: I16a53ce79d8a23d23580511cb6abd062f54b65ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/188157
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Currently when we coalesce memory we make a sysHugePage call
(MADV_HUGEPAGE) to ensure freed and coalesced huge pages are treated as
such so the scavenger's assumptions about performance are more in line
with reality.
Unfortunately we do it way too often because we do it if there was any
change to the huge page count for the span we're coalescing into, not
taking into account that it could coalesce with its neighbors and not
actually create a new huge page.
This change makes it so that it only calls sysHugePage if the original
huge page counts between the span to be coalesced into and its neighbors
do not add up (i.e. a new huge page was created due to alignment). Calls
to sysHugePage will now happen much less frequently, as intended.
Updates #32828.
Change-Id: Ia175919cb79b730a658250425f97189e27d7fda3
Reviewed-on: https://go-review.googlesource.com/c/go/+/186926
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change adds physHugePageShift which is defined such that
1 << physHugePageShift == physHugePageSize. The purpose of this variable
is to avoid doing expensive divisions in key functions, such as
(*mspan).hugePages.
This change also does a sweep of any place we might do a division or mod
operation with physHugePageSize and turns it into bit shifts and other
bitwise operations.
Finally, this change adds a check to mallocinit which ensures that
physHugePageSize is always a power of two. osinit might choose to ignore
non-powers-of-two for the value and replace it with zero, but mallocinit
will fail if it's not a power of two (or zero). It also derives
physHugePageShift from physHugePageSize.
This change helps improve the performance of most applications because
of how often (*mspan).hugePages is called.
Updates #32828.
Change-Id: I1a6db113d52d563f59ae8fd4f0e130858859e68f
Reviewed-on: https://go-review.googlesource.com/c/go/+/186598
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This is intended to help clear up confusion around the usage of the
Title and ToTitle functions. It includes a link to define title case
to distinguish it from upper case. It also includes an additional
example for the ToTitle function to showcase the difference in behavior
between it and the Title function.
Fixes#33302
Change-Id: I44e62962fb04d0d22966a39eda3a2d16de7a2291
Reviewed-on: https://go-review.googlesource.com/c/go/+/187825
Reviewed-by: Rob Pike <r@golang.org>
Overflow of the comparison caused very large (>=1<<32) allocations to
sometimes not get sampled at all. Use uintptr so the comparison will
never overflow.
Fixes#33342
Tested on the example in 33342. I don't want to check a test in that
needs that much memory, however.
Change-Id: I51fe77a9117affed8094da93c0bc5f445ac2d3d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/188017
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Document that *os.File is subject to resource limits
for concurrent operations. We aren't documenting
a specific number of concurrent operations because that
number is OS/system dependent. This limit comes from:
internal/poll/fd_mutex.go
where we use 20 bits to count locks.
Fixes#32544
Change-Id: I7d305d4aaba5b2dbc6f1ab8c447117fde5e31a66
Reviewed-on: https://go-review.googlesource.com/c/go/+/181841
Reviewed-by: Rob Pike <r@golang.org>
This reverts CL 176098.
Reason for revert: added complexity, but did not completely fix the
underlying problem. A complete solution would not be worth the
complexity, and as a partial solution this is probably not worth the
complexity either.
Updates #31859
Change-Id: Ifd34c292fd1b811c60afe3c339e5edd3f37190c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/186817
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>
Currently, GODEBUG=sbrk=1 mode aligns allocations by their type's
alignment. You would think this would be the right thing to do, but
because 64-bit fields are only 4-byte aligned right now (see #599),
this can cause a 64-bit field of an allocated object to be 4-byte
aligned, but not 8-byte aligned. If there is an atomic access to that
unaligned 64-bit field, it will crash.
This doesn't happen in normal allocation mode because the
size-segregated allocation and the current size classes will cause any
types larger than 8 bytes to be 8 byte aligned.
We fix this by making sbrk=1 mode use alignment based on the type's
size rather than its declared alignment. This matches how the tiny
allocator aligns allocations.
This was tested with
GOARCH=386 GODEBUG=sbrk=1 go test sync/atomic
This crashes with an unaligned access before this change, and passes
with this change.
This should be reverted when/if we fix#599.
Fixes#33159.
Change-Id: Ifc52c72c6b99c5d370476685271baa43ad907565
Reviewed-on: https://go-review.googlesource.com/c/go/+/186919
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
'go test' uses the Root field to determine the set of files that
invalidate test results, and there is no other sensible meaning of
“root” for code within a module.
Fixes#29111
Change-Id: Icf1be90a26d22665613e42cb968087b63c36e74c
Reviewed-on: https://go-review.googlesource.com/c/go/+/154100
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The go command invokes each tool with -V=full to discover its version
to compute a tool id. For release versions (that don't include the
word "devel"), the go command only used the third word in
the output (e.g., "go1.13"), ignoring any toolchain experiments that
followed. With this change, the go command will use whole version line
in the tool id for release versions.
Also, when -V=full is set and there are non-default experiments,
experiments are no longer printed twice.
Fixes#33091
Change-Id: I19b96f939c7e2fbc5d8befe3659156ee4b58daef
Reviewed-on: https://go-review.googlesource.com/c/go/+/186200
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
In Go1.12 and below, the logic in ReverseProxy.ServeHTTP would always
allocate request.Header even if it were not present in the incoming request.
CL 174324 added http.Request.Clone and re-factors ReverseProxy.ServeHTTP
to use the new Clone method. However, the new Clone logic is not equivalent
to the former logic. We preserve former semantics by explicitly allocating
the Header map if nil.
Fixes#33142
Change-Id: I356f94a915dd9779584ce3fe31e56e5474b9ad37
Reviewed-on: https://go-review.googlesource.com/c/go/+/186437
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Changes "was loaded as" to "was required as". This is slightly more
precise, since it hints at a requirement edge in the module version
graph.
Updates #28489
Change-Id: I636268c33f1ea9858c214fe275f271538186ed6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/186377
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
With gccgo, if we generate getg inlined, the backend may cache
the address of the TLS variable, which will become invalid after
a thread switch.
Currently there is no known bug for this. But if we didn't
implement this carefully, we may get subtle bugs. This CL adds a
test that will fail loudly if this is wrong. (See also
https://go.googlesource.com/gofrontend/+/refs/heads/master/libgo/runtime/proc.c#333
and an incorrect attempt CL 185337.)
Note: at least on Linux/AMD64, even with an incorrect
implementation, this only fails if the test is compiled with
-fPIC, which is not the default setting for gccgo test suite. So
some manual work is needed. Maybe we could extend the test suite
to run the runtime test with more settings (e.g. PIC and static).
Change-Id: I459a3b4c31f09b9785c0eca19b7756f80e8ef54c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186357
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Do not allow a pseudo-version derived from a canonical tag to refer to
the same revision as the tag itself. It's unnecessary (because
canonical tags already have a total ordering) and confusing (the
pseudo-version appears to come after the tag, but actually refers to
the exact same revision).
Updates #32879
Updates #27173
Change-Id: I02befedbe89c8819bdd93e470783ce63fc813193
Reviewed-on: https://go-review.googlesource.com/c/go/+/184720
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
