The full truth seems too complicated to write in this method's doc, so
I'm going with a simple half truth.

The full truth is that Fd returns the descriptor in blocking mode,
because that is historically how it worked, and existing programs
would be surprised if the descriptor is suddenly non-blocking. On Unix
systems whether a file is non-blocking or not is a property of the
underlying file description, not of a particular file descriptor, so
changing the returned descriptor to blocking mode also changes the
existing File to blocking mode. Blocking mode works fine, althoug I/O
operations now take up a thread. SetDeadline and friends rely on the
runtime poller, and the runtime poller only works if the descriptor is
non-blocking. So it's correct that calling Fd disables SetDeadline.
The other half of the truth is that if the program is willing to work
with a non-blocking descriptor, it could call
syscall.SetNonblock(descriptor, true) to change the descriptor, and
the original File, to non-blocking mode. At that point SetDeadline
would start working again. I tried to write that in a way that is
short and comprehensible but failed. Since deadlines mostly work on
pipes, and there isn't much reason to call Fd on a pipe, and few
people use SetDeadline, I decided to punt.

Fixes #22934

Change-Id: I2e49e036f0bcf71f5365193831696f9e4120527c
Reviewed-on: https://go-review.googlesource.com/81636Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Fixes #22958

Change-Id: Ib078a5f6e1105a2afca77c6d9a05f65ddf5d9010
Reviewed-on: https://go-review.googlesource.com/81435
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>

That Parse doesn't parse ("foo.com/path" or "foo.com:443/path") has
become something of a FAQ.

Updates #19779
Updates #21415
Updates #22955

Change-Id: Ib68efddb67f59b1374e8ed94effd4a326988dee7
Reviewed-on: https://go-review.googlesource.com/81436Reviewed-by: Ian Lance Taylor <iant@golang.org>

Simply choose an extremely unlikely path name in the test is fine.

Fixes #21013.

Change-Id: I56c0a1986b5ef5d618c7fe2b14701f584fe81c37
Reviewed-on: https://go-review.googlesource.com/81578
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

MAP_ANON is the deprecated but more portable spelling of
MAP_ANONYMOUS. Use MAP_ANON to un-break the Darwin 10.10 builder.

Updates #22930.

Change-Id: Iedd6232b94390b3b2a7423c45cdcb25c1a5b3323
Reviewed-on: https://go-review.googlesource.com/81615
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Fixes #21724

Change-Id: I92571bf228781b17fdf012a2fb52a597c877cefe
Reviewed-on: https://go-review.googlesource.com/81576
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>

The test binaries accept -timeout=0 to mean no timeout,
but then the backup timer in cmd/go kills the test after 1 minute.
Make cmd/go understand this special case and change
behavior accordingly.

Fixes #14780.

Change-Id: I66bf517173a4ad21d53a5ee88d163f04b8929fb6
Reviewed-on: https://go-review.googlesource.com/81499
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Also be clear that go test output is not suitable for piping into test2json.

Fixes #22710.
Fixes #22789.

Change-Id: I3d850c8a2288be7f9a27d638bbf847cb8707dcce
Reviewed-on: https://go-review.googlesource.com/81555
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

1. Apply JSON conversion when -bench is in use.
2. Apply JSON conversion to "no test files" result.
3. Apply JSON conversion to test case-ending SKIP status.

Fixes #22769.
Fixes #22790.

Change-Id: I67ad656fc58bacae8c51d23b1e6d543cad190f08
Reviewed-on: https://go-review.googlesource.com/81535
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

The cover variable indices could vary from build to build,
but they were not included in the build ID hash, so that
reusing the previously built package was not safe.
Make the indices no longer vary from build to build,
so that caching is safe.

Fixes #22652.

Change-Id: Ie26d73c648aadd285f97e0bf39619cabc3da54f2
Reviewed-on: https://go-review.googlesource.com/81515
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

For Go 1.10, works around a go/types bug that can't typecheck
a corner-case type cycle. Once we are confident that bugs like
this are gone from go/types then we can stop ignoring these
failures.

For #22890.

Change-Id: I38da57e01a0636323e1af4484c30871786125df3
Reviewed-on: https://go-review.googlesource.com/81500
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Earlier versions of Go were not very picky about leading spaces
in the -gcflags values. Make the new pattern-enhanced parser
equally lax.

Fixes #22943.

Change-Id: I5cf4d3e81412e895a4b52af325853ed48d0b73f4
Reviewed-on: https://go-review.googlesource.com/81498
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

It's not safe to do p+x with unsafe if that would point past the
end of the object. (Valid in C, not safe in Go.)
Pass a "whySafe" reason (compiled away) to explain at each
call site why it's safe.

Fixes #21733.

Change-Id: I5da8c25bde66f5c9beac232f2135dcab8e8bf3b1
Reviewed-on: https://go-review.googlesource.com/80738Reviewed-by: Austin Clements <austin@google.com>

The number of threads in syscall presented by execution tracer's
trace view includes not only the threads calling system calls on behalf
of user created goroutines, but also those running on behalf of system
goroutines.

When the number of such system goroutines was small, the graph was
useful when examining where a program was saturating the CPU.
But as more and more system goroutines are invloved the graph became
less useful for the purpose - for example, after golang.org/cl/34784,
the timer goroutines dominate in the graph with large P
because the runtime creates per-P timer goroutines.

This change excludes the threads in syscall on behalf of runtime (system
goroutines) from the visualization. Alternatively, I could visualize the
count of such threads in a separate counter but in the same graph.
Given that many other debug endpoints (e.g. /debug/pprof/goroutine) hide
the system goroutines, including them in the same graph can confuse users.

Update #22574

Change-Id: If758cd6b9ed0596fde9a471e846b93246580b9d5
Reviewed-on: https://go-review.googlesource.com/81315Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Minor.

Makes reading failing runtime test stacktraces easier (by having fewer
goroutines to read) on machines where these gdb tests wouldn't have
ever run anyway.

Change-Id: I3fab0667e017f20ef3bf96a8cc4cfcc614d25b5c
Reviewed-on: https://go-review.googlesource.com/81575Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

This adds logging for the expected duration of a growStack, plus
progress information on the growStack that timed out.

Updates #19381.

Change-Id: Ic358f8350f499ff22dd213b658aece7d1aa62675
Reviewed-on: https://go-review.googlesource.com/81556
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Per the decision for #14844, index expressions that are non-constant
shifts where the LHS operand is representable as an int are now valid.

Fixes #21693.

Change-Id: Ifafad2c0c65975e0200ce7e28d1db210e0eacd9d
Reviewed-on: https://go-review.googlesource.com/81277Reviewed-by: Matthew Dempsky <mdempsky@google.com>

I think of "sending" a signal as calling kill, but sigsend is involved
in handling a signal and, specifically delivering it to the internal
signal queue. The term "delivery" is already used in
signalWaitUntilIdle, so this CL also uses it in the documentation for
sigsend.

Change-Id: I86e171f247f525ece884a680bace616fa9a3c7bd
Reviewed-on: https://go-review.googlesource.com/81235Reviewed-by: Ian Lance Taylor <iant@golang.org>

Currently, when we minit on a thread that already has an alternate
signal stack (e.g., because the M was an extram being used for a cgo
callback, or to handle a signal on a C thread, or because the
platform's libc always allocates a signal stack like on Android), we
simply drop the Go-allocated gsignal stack on the floor.

This is a problem for Ms on the extram list because those Ms may later
be reused for a different thread that may not have its own alternate
signal stack. On tip, this manifests as a crash in sigaltstack because
we clear the gsignal stack bounds in unminit and later try to use
those cleared bounds when we re-minit that M. On 1.9 and earlier, we
didn't clear the bounds, so this manifests as running more than one
signal handler on the same signal stack, which could lead to arbitrary
memory corruption.

This CL fixes this problem by saving the Go-allocated gsignal stack in
a new field in the m struct when overwriting it with a system-provided
signal stack, and then restoring the original gsignal stack in
unminit.

This CL is designed to be easy to back-port to 1.9. It won't quite
cherry-pick cleanly, but it should be sufficient to simply ignore the
change in mexit (which didn't exist in 1.9).

Now that we always have a place to stash the original signal stack in
the m struct, there are some simplifications we can make to the signal
stack handling. We'll do those in a later CL.

Fixes #22930.

Change-Id: I55c5a6dd9d97532f131146afdef0b216e1433054
Reviewed-on: https://go-review.googlesource.com/81476
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

If package strings has a particular set of gcflags, then the strings_test
pseudo-package built as part of the test binary should inherit the same flags.

Fixes #22831.

Change-Id: I0e896b6c0f1063454300b7323f577feffbd6650b
Reviewed-on: https://go-review.googlesource.com/81496
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

If the build of the test binary failed, the go command correctly
avoided running the binary, but the -x output indicated otherwise.

Fixes #22659.

Change-Id: Ib4d262bf1735f057c994a45fc23c499d4ebe3246
Reviewed-on: https://go-review.googlesource.com/81495
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

The CL introducing merged handling of cover profiles
did not correctly account for the fact that the file name argument
to -coverprofile is required to be interpreted relative to
the -outputdir argument.

Fixes #22804.

Change-Id: I804774013c12187313b8fd2044302978bdbb6697
Reviewed-on: https://go-review.googlesource.com/81455
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

The code that generates the list of DWARF variables for a function
(params and autos) will emit a "no-location" entry in the DWARF for a
user var that appears in the original pre-optimization version of the
function but is no longer around when optimization is complete. The
intent is that if a GDB user types "print foo" (where foo has been
optimized out), the response will be "<optimized out>" as opposed to
"there is no such variable 'foo'). This change fixes said code to
include vars on the autom list for the function, to insure that the
type symbol for the variable makes it to the linker.

Fixes #22941.

Change-Id: Id29f1f39d68fbb798602dfd6728603040624fc41
Reviewed-on: https://go-review.googlesource.com/81415
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

Values must not be copied after the first use.

Using noCopy makes vet complain about copies
even before the first use, which is incorrect
and very frustrating.

Drop it.

Fixes #21504.

Change-Id: Icd3a5ac3fe11e84525b998e848ed18a5d996f45a
Reviewed-on: https://go-review.googlesource.com/80836
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Change-Id: Ibf227dcfefa179b1c3378476bcd17100b1b1c01e
Reviewed-on: https://go-review.googlesource.com/81375
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Call is meant to mirror the language semantics, which allow:

	var r io.ReadWriter
	f := func(io.Reader){}
	f(r)

even though the conversion from io.ReadWriter to io.Reader is
being applied to a nil interface. This is different from an explicit
conversion:

	_ = r.(io.Reader)
	f(r.(io.Reader))

Both of those lines panic, but the implicit conversion does not.

By using E2I, which is the implementation of the explicit conversion,
the reflect.Call equivalent of f(r) was inadvertently panicking.
Avoid the panic.

Fixes #22143.

Change-Id: I6b2f5b808e0cd3b89ae8bc75881e307bf1c25558
Reviewed-on: https://go-review.googlesource.com/80736
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

The ssa backend is aggressive about placing constants and
certain other values in the Entry block.  It's implausible
that the original line numbers for these constants makes
any sort of sense when it appears to a user stepping in a
debugger, and they're also not that useful in dumps since
entry-block instructions tend to be constants (i.e.,
unlikely to be the cause of a crash).

Therefore, use src.NoXPos for any values that are explicitly
inserted into a function's entry block.

Passes all tests, including ssa/debug_test.go with both
gdb and a fairly recent dlv.  Hand-verified that it solves
the reported problem; constructed a test that reproduced
a problem, and fixed it.

Modified test harness to allow injection of slightly more
interesting inputs.

Fixes #22558.

Change-Id: I4476927067846bc4366da7793d2375c111694c55
Reviewed-on: https://go-review.googlesource.com/81215
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

Fixes #19293

Change-Id: I35f2f786e2e3972eda21ba5a948433bfcd621269
Reviewed-on: https://go-review.googlesource.com/81355Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Updates http2 to x/net git rev 894f8ed58 for:

    http2: fix flake in net/http's TestCloseIdleConnections_h2
    https://golang.org/cl/80139

    http2: fix leak in activeRes by removing activeRes
    https://golang.org/cl/80137

Fixes #22413
Fixes #21543

Change-Id: Ic8ea20f8ddae2fde17884ed045f9fa7058a4bd23
Reviewed-on: https://go-review.googlesource.com/81276
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>

This reverts commit 08f19bbd.

Reason for revert:
The changed transformation takes effect on a larger set
of code snippets than expected.

For example, this:
    func foo() {

        // Comment
        bar()

    }
becomes:
    func foo() {
        // Comment
        bar()

    }

This is an unintended consequence.

Change-Id: Ifca88d6267dab8a8170791f7205124712bf8ace8
Reviewed-on: https://go-review.googlesource.com/81335Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Fixes #6007

Change-Id: I239a1699122e086e907ac1f18b1c86a650e1438a
Reviewed-on: https://go-review.googlesource.com/81135
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

GOEXPERIMENT is only set during make.bash, so checking the environment
variable isn't effectual. Instead, check the values exposed by objabi.

These experiments look potentially safe, but it seems too late in the
release cycle to try to assuage that. The one exception is frame
pointer experiment, which is trivially safe: it just amounts to
incrementing some stack offsets by PtrSize.

Fixes #22223.

Change-Id: I46dc7c54b1347143d02d6b9635038230cda6d164
Reviewed-on: https://go-review.googlesource.com/80760Reviewed-by: Russ Cox <rsc@golang.org>

It's not safe (it crashes), and it's also useless: if you run
multiple benchmarks in parallel you will not get reliable
timing results from any of them.

Fixes #18603.

Change-Id: I00e5a72f7c98151543cf7d5573c38383276e391a
Reviewed-on: https://go-review.googlesource.com/80841Reviewed-by: Ian Lance Taylor <iant@golang.org>

Reading the mem stats before our own allocations
avoids cluttering memory stats with our recent garbage.

Fixes #20565.

Change-Id: I3b0046c8300dca83cea24013ffebc32b2ae7f742
Reviewed-on: https://go-review.googlesource.com/80739Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

Lock in fix for #22738, submitted in CL 78031.

Fixes #22738.

Change-Id: I6896feb158569e3f12fa7055387cbd7caad29ef4
Reviewed-on: https://go-review.googlesource.com/80635
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>

The build tags already prevent the tests from being run on windows or
plan9, so there is no need to check GOOS again.

Change-Id: I74d3c3b7756d9c50f6e5fd4c3e8b0db618fdebbb
Reviewed-on: https://go-review.googlesource.com/81295
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Otherwise we may delay the delivery of these signals for an arbitrary
length of time. We are already careful to not block signals that the
program has asked to see.

Also make sure that we don't miss a signal delivery if a thread
decides to stop for a while while executing the signal handler.

Also clean up the TestAtomicStop output a little bit.

Fixes #21433

Change-Id: Ic0c1a4eaf7eba80d1abc1e9537570bf4687c2434
Reviewed-on: https://go-review.googlesource.com/79581
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

Change-Id: I850d961e0444f8d34284e994aee183afba35eaa7
Reviewed-on: https://go-review.googlesource.com/79597Reviewed-by: Ian Lance Taylor <iant@golang.org>

This is inspired by
https://blog.cloudflare.com/go-dont-collect-my-garbage/
This CL adds allocation tracking and parallelizes p256-related benchmarks.
Amount of allocations can be significantly reduced by marking amd64 asm
functions as noescape. This exposes a bug in p256MovCond:
PANDN with memory argument will fault if memory is not aligned, so they
are replaced with MOVDQU (which is ok with unaligned memory) and
register version of PANDN.

Results on 88-thread machine (2x 22 cores) below:
crypto/elliptic:
name               old time/op    new time/op    delta
BaseMultP256-88      1.50µs ±11%    1.19µs ± 5%  -20.20%  (p=0.000 n=10+10)
ScalarMultP256-88    5.47µs ± 5%    3.63µs ±10%  -33.66%  (p=0.000 n=9+10)

name               old alloc/op   new alloc/op   delta
BaseMultP256-88        800B ± 0%      288B ± 0%  -64.00%  (p=0.000 n=10+10)
ScalarMultP256-88    2.59kB ± 0%    0.26kB ± 0%  -90.12%  (p=0.000 n=10+10)

name               old allocs/op  new allocs/op  delta
BaseMultP256-88        13.0 ± 0%       6.0 ± 0%  -53.85%  (p=0.000 n=10+10)
ScalarMultP256-88      16.0 ± 0%       5.0 ± 0%  -68.75%  (p=0.000 n=10+10)

crypto/ecdsa:
name              old time/op    new time/op    delta
SignP256-88         8.63µs ±37%    7.55µs ±38%     ~     (p=0.393 n=10+10)
VerifyP256-88       13.9µs ± 8%     7.0µs ± 7%  -49.29%  (p=0.000 n=10+9)
KeyGeneration-88    2.77µs ±11%    2.34µs ±11%  -15.57%  (p=0.000 n=10+10)

name              old alloc/op   new alloc/op   delta
SignP256-88         4.14kB ± 1%    2.98kB ± 2%  -27.94%  (p=0.000 n=10+10)
VerifyP256-88       4.47kB ± 0%    0.99kB ± 0%  -77.84%  (p=0.000 n=9+10)
KeyGeneration-88    1.21kB ± 0%    0.69kB ± 0%  -42.78%  (p=0.000 n=10+10)

name              old allocs/op  new allocs/op  delta
SignP256-88           47.0 ± 0%      34.0 ± 0%  -27.66%  (p=0.000 n=10+10)
VerifyP256-88         38.0 ± 0%      17.0 ± 0%  -55.26%  (p=0.000 n=10+10)
KeyGeneration-88      20.0 ± 0%      13.0 ± 0%  -35.00%  (p=0.000 n=10+10)

On machine with only 4 cores, results are much less impressive:
around 2% performance gain.

Change-Id: I8a2f8168f83d27ad9ace1b4b1a1e11cb83edf717
Reviewed-on: https://go-review.googlesource.com/80757
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

Dim performance has regressed by 14% vs 1.9 on amd64.
Current pure go version of Dim is faster and,
what is even more important for performance, is inlinable, so
instead of tweaking asm implementation, just remove it.
I had to update BenchmarkDim, because it was simply reloading
constant(answer) in a loop.
Perf data below:

name   old time/op  new time/op  delta
Dim-6  6.79ns ± 0%  1.60ns ± 1%  -76.39%  (p=0.000 n=7+10)

If I modify benchmark to be the same as in this CL results are even better:

name   old time/op  new time/op  delta
Dim-6  10.2ns ± 0%   1.6ns ± 1%  -84.27%  (p=0.000 n=8+10)

Updates #21913

Change-Id: I00e23c8affc293531e1d9f0e0e49f3a525634f53
Reviewed-on: https://go-review.googlesource.com/80695
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>