In Go 1.15 we switched the default linking mode for PIE on
Linux/AMD64 and Linux/ARM64 to internal linking. Clarify that
the previous behavior (external linking) can still be used with
a flag.
Fixes#40719.
Change-Id: Ib7042622bc91e1b1aa31f520990d03b5eb6c56bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/248199
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In the dev.link branch we have continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker . Since the last merge (May 1st
2020), more progress has been made to improve the new linker, with
improvements on both linker speed and memory usage.
Fixes#40703.
Change-Id: I9924ea88d981845c3a40ec8c25820120fc21c003
This change propagates context into PackagesForErrors and Do for
the purpose of tracing, and calls trace.StartSpan on PackagesForErrors
and Do, so that the trace now shows the broad outline of where
the "Loading" and "Execution" phases are in the build.
Updates #38714
Change-Id: Ib9a7cf7030210f68f76663d1c8a7461e0a226611
Reviewed-on: https://go-review.googlesource.com/c/go/+/238541
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
One small step to start propagating the context in
cmd/go for tracing purposes.
Updates #38714
Change-Id: Ibb6debeb9233f84d55f0e81244487355cbe7b82c
Reviewed-on: https://go-review.googlesource.com/c/go/+/237684
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If cmd/go is provided with a -debug-trace=<file> option, cmd/go will write an
execution trace to that file.
Updates #38714
Change-Id: I3e6521343902c08266a0292f4280298a3bf8b725
Reviewed-on: https://go-review.googlesource.com/c/go/+/237683
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The ViewerEvent, ViewerData and ViewerFrame structs are moved into
cmd/internal/traceviewer, and renamed Event, Data, and Frame.
The structs are the same, except for the following: A definition
for the JSON "bp" field that's defined in the trace format, but
missing in the structs has been added. Also, the Tid and Pid fields
on Event have been renamed TID and PID to better match Go style.
Finally, the footer field on ViewerData, which hasn't been used
for a while, has been removed.
This CL is in preparation for the usage of these structs by cmd/go's
tracing functionality.
Updates #38714
Change-Id: I345f23617b96d4629b876ae717f89d56a67e05a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/239098
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This template is based on CL 220278 and previous ones like it.
Include Compiler and Linker sections proactively, they can be
removed if they don't end up being needed for Go 1.16.
Use two spaces of indentation for TODOs to set a better precedent
for the final text that will take its place.
'relnote -html' does not report any changes at this time.
For #40700.
Change-Id: I096b0ce0d33aaaa6fae9c91c0d2dfb89b9c5e94c
Reviewed-on: https://go-review.googlesource.com/c/go/+/248198
Reviewed-by: Carlos Amedee <carlos@golang.org>
This is the start of the Go 1.16 development cycle, so update the
Version value accordingly. It represents the Go 1.x version that
will soon open up for development (and eventually become released).
Historically, we used to bump this at an arbitrary time throughout
the development cycle, but it's better to be more predictable about
updating it. The start of a development cycle should be the most
appropriate time: it clearly marks the boundary between 1.15 and
1.16 development, and doing it early can help catch issues in other
tooling. See issue #38704 for more background.
There is no longer a need to update the list of Go versions in
src/go/build/doc.go because it does not exist as of CL 232981.
For #40705.
Updates #38704.
Updates #37018.
Change-Id: Id8ee733b5e79c53b6cd03509c6560614d8743833
Reviewed-on: https://go-review.googlesource.com/c/go/+/248038
Reviewed-by: Carlos Amedee <carlos@golang.org>
Rewrite part of cmd/pack to use the cmd/internal/archive package.
Change-Id: Ia7688810d3ea4d0277056870091f59cf09cffcad
Reviewed-on: https://go-review.googlesource.com/c/go/+/247917
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Combine objfile2.go into objfile.go.
objfile.go has a lot of code for DWARF generation. Move them to
dwarf.go.
Change-Id: I2a27c672e9e9b8eea35d5e0a71433dcc80b7afa4
Reviewed-on: https://go-review.googlesource.com/c/go/+/247918
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
CL 245485 introduced a map for used files in a function. When
numbering symbols, make sure we traverse the files in
deterministic order.
Should fix longtest builders.
Change-Id: I1006bc5425116ab40e33a61e8f5acd1bdb4abad9
Reviewed-on: https://go-review.googlesource.com/c/go/+/247997
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Read Go object files using cmd/internal/goobj2 package directly,
instead of using cmd/internal/goobj as an intermediate layer.
Now cmd/internal/archive is only about reading archives.
Change-Id: Ifecb217fb26c16c26fc1bbc3fba0ed44710020ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/246443
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
See golang.org/cl/240657, which reverted the original change to fix the
regression reported in golang.org/issue/39427.
Updates #37419.
Change-Id: I39fbaa0b028ee00856cffea38879a631f540f057
Reviewed-on: https://go-review.googlesource.com/c/go/+/247718
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rename cmd/internal/goobj package to cmd/internal/archive. This
is in preparation of a refactoring of object and archive file
reading packages.
With this CL, the cmd/internal/archive contains logic about
reading Go object files. This will be moved to other places in
later CLs.
Change-Id: Ided7287492a4766183d6e49be840a7f361504d1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/246442
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This update was created using the updatecontrib command:
go get golang.org/x/build/cmd/updatecontrib
cd gotip
updatecontrib
With manual changes based on publicly available information
to canonicalize letter case and formatting for a few names.
For #12042.
Change-Id: I66dc5ee28d9a64bc9d150e72d136d8f71e50373b
Reviewed-on: https://go-review.googlesource.com/c/go/+/247767
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Currently, nanotime1 (and walltime1) is not reentrant, in that it
sets m.vdsoSP at entry and clears it at exit. If a signal lands
in between, and nanotime1 is called from the signal handler, it
will clear m.vdsoSP while we are still in nanotime1. If (in the
unlikely event) it is signaled again, m.vdsoSP will be wrong,
which may cause the stack unwinding code to crash.
This CL makes it reentrant, by saving/restoring the previous
vdsoPC and vdsoSP, instead of setting it to 0 at exit.
TODO: have some way to test?
Change-Id: I9ee53b251f1d8a5a489c71d4b4c0df1dee70c3e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/246763
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In order to prevent renumbering of filenames in pclntab generation, use
the per-package file list (previously only used for DWARF generation) as
file-indices. This is the largest step to eliminate renumbering of
filenames in pclntab.
Note, this is probably not the final state of the file table within the
object file. In this form, the linker loads all filenames for all
objects. I'll move to storing the filenames as regular string
symbols,and defaulting all string symbols to using the larger hash value
to make generation of pcln simplest, and most memory friendly.
Change-Id: I23daafa3f4b4535076e23100200ae0e7163aafe0
Reviewed-on: https://go-review.googlesource.com/c/go/+/245485
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Add a new PPC64-only linker test that does a build with the
-debugppc64textsize debugging option (selecting a lower the threshold
for text section splitting) to verify that no bugs have been
introduced in the linker code that manages this process.
Change-Id: Iea3f16a04c894d528eab2cb52f1ec1d75a2770cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/241499
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Add a new debugging command line option (-debugppc64textsize=N) that
forces the start of a new text section after ".text" hits N bytes as
opposed to the architected limit of 2^26. This is intended to enable
testing of the linker code paths that handle multiple .text sections
on PPC64 without resorting to building giant applications.
Updates #20492.
Change-Id: I74ab7fd1e412e9124de5bd0d8d248c5e73225ae3
Reviewed-on: https://go-review.googlesource.com/c/go/+/241073
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The code in the the linker's genelfsym() routine was not properly
including runtime.text.%d marker symbols that are emitted on PPC64
when a very large text section is split into chunks. This bug was
introduced in CL 233338 when portions of asmb2() were converted
from sym.Symbol to loader.Sym usage.
Change-Id: Idfed944c41e1805f78f35be67bcdd18bdefd7819
Reviewed-on: https://go-review.googlesource.com/c/go/+/241498
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This preallocation is way too large, and showed up in the metrics. Just
remove it all together.
Change-Id: Ib4646b63cd0a903656ada244f15e977cde2a2c4c
Reviewed-on: https://go-review.googlesource.com/c/go/+/247177
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This CL ensures that ReadUvarint consumes only a limited
amount of input (instead of an unbounded amount).
On some inputs, ReadUvarint could read an arbitrary number
of bytes before deciding to return an overflow error.
After this CL, ReadUvarint returns that same overflow
error sooner, after reading at most MaxVarintLen64 bytes.
Fix authored by Robert Griesemer and Filippo Valsorda.
Thanks to Diederik Loerakker, Jonny Rhea, Raúl Kripalani,
and Preston Van Loon for reporting this.
Fixes#40618
Fixes CVE-2020-16845
Change-Id: Ie0cb15972f14c38b7cf7af84c45c4ce54909bb8f
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/812099
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/247120
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
After golang.org/cl/228784 setLoadPackageDataError tries to decide whether an
error is caused by an imported package or an importing package by examining the
error itself to decide. Ideally, the errors themselves would belong to a
specific interface or some other property to make it unambiguous that they
were import errors. Since they don't, setLoadPackageDataError just checked
for nogoerrors and classified all other errors as import errors. But
it missed scanner errors which are also "caused" by the imported
package.
Fixes#40544
Change-Id: I39159bfdc286bee73697decd07b8aa9451f2db06
Reviewed-on: https://go-review.googlesource.com/c/go/+/246717
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Use the original *Request in the reqCanceler map, not the transient
wrapper created to handle body rewinding.
Change the key of reqCanceler to a struct{*Request}, to make it more
difficult to accidentally use the wrong request as the key.
Fixes#40453.
Change-Id: I4e61ee9ff2c794fb4c920a3a66c9a0458693d757
Reviewed-on: https://go-review.googlesource.com/c/go/+/245357
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
On AIX, container symbols are handled in a weird way (unlike
other platforms): the outer symbol needs to have size (but still
no data), and the inner symbols must not be in the symbol table
(otherwise it overlaps with the outer symbol, which the system
linker doesn't like).
As of CL 241598, pclntab becomes a container symbol. We need to
follow the rule above for AIX.
Fix AIX build.
Change-Id: Ie2515a4cabbd8cf3f6d3868643a28f64ca3365a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/246479
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently, at compile time, for each itab symbol, we create an
"itablink" symbol which holds solely the address of the itab
symbol. At link time, all the itablink symbols are grouped
together to form the itablinks slice.
This CL removes the itablink symbols, and directly generate the
itablinks slice in the linker. This removes a number of symbols,
which are dupOK and generally have long names. And also removes
a special handling of itablink symbols in the deadcode pass which
iterates through all symbols.
Change-Id: I475c3c8899e9fbeec9abc7647b1e4a69aa5c3c5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/245901
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The hash maps are used to deduplicate hashed symbols. Once we
loaded all the symbols, we no longer need the hash maps. Drop
them.
Linking cmd/compile,
name old live-B new live-B delta
Loadlib_GC 13.1M ± 0% 11.3M ± 0% -13.62% (p=0.008 n=5+5)
Change-Id: I4bb1f84e1111a56d9e777cd6a68f7d974b60e321
Reviewed-on: https://go-review.googlesource.com/c/go/+/245721
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Extend the content-addressable symbol mechanism to itab symbols.
Itab symbols require global uniqueness (as at run time we compare
pointers), so it needs to be reliably deduplicated. Currently the
content hash depends on symbol name expansion, so we can only do
this when all Go packages are built with know package paths. Fall
back to checking names if any Go package is built with unknown
package path.
Change-Id: Icf5e8873755050c20e5fc6549f6de1c883254c89
Reviewed-on: https://go-review.googlesource.com/c/go/+/245719
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The type descriptor symbol of a defined (named) type (and pointer
to it) is defined only in the package that defines the type. It
is not dupOK, unlike other type descriptors. So it can be
referenced by index. Currently it is referenced by name for
cross-package references, because the index is not exported and
so not known to the referencing package.
This CL passes the index through the export data, so the symbol
can be referenced by index, and does not need to be looked up by
name. This also makes such symbol references consistent: it is
referenced by index within the defining package and also cross-
package, which makes it easier for content hashing (in later CLs).
One complication is that we need to set flags on referenced
symbols (specifically, the UsedInIface flag). Before, they are
non-package refs, which naturally carry flags in the object file.
For indexed refs, we currently don't put their flags in the
object file. Introduce a new block for this.
Change-Id: I8126f8e318ac4e6609eb2ac136201fd6c264c256
Reviewed-on: https://go-review.googlesource.com/c/go/+/245718
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Not used yet, but add the compilation unit for a function to func.
Change-Id: I7c43fa9f1da044ca63bab030062519771b9f4418
Reviewed-on: https://go-review.googlesource.com/c/go/+/244547
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Leaving creation of the funcID till the linker requires the linker to
load the function and file names into memory. Moving these into the
compiler/assembler prevents this.
This work is a step towards moving all func metadata into the compiler.
Change-Id: Iebffdc5a909adbd03ac263fde3f4c3d492fb1eac
Reviewed-on: https://go-review.googlesource.com/c/go/+/244024
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
The .syso test also fails for ppc64le. Not sure why. For now, just
disable the test for that architecture. The test really only needs to
run on a single builder of any arch.
Change-Id: I346cdc01ada09d43c4c504fbc30be806f59d5422
Reviewed-on: https://go-review.googlesource.com/c/go/+/246358
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We can't depend on getauxval because it only exists in glibc >= 2.16.
Tsan has been updated to avoid that dependency
(https://reviews.llvm.org/D84859). This CL rebuilds the affected
.syso files, and adds a test to make sure we don't regress.
Fixes#37485
Change-Id: I891f54d28ec0d7da50a8df1adadc76dd6e7ab3e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/246258
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
For dupOK symbols, their attributes should be OR'd. Most of the
attributes are expected to be set consistently across multiple
definitions, but UsedInIface must be OR'd, and for alignment we
need to pick the largest one. Currently the attributes are not
always OR'd, depending on addSym returning true or false. This
doesn't cause any real problem, but it would be a problem if we
make type descriptor symbols content-addressable.
This CL removes the second result of addSym, and lets preloadSyms
always set the attributes. Also removes the alignment handling on
addSym, handles it in preloadSyms only.
Change-Id: I06b3f0adb733f6681956ea9ef54736baa86ae7bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/245720
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Consider this test package:
package p
// enum E { E0 };
// union U { long x; };
// void f(enum E e, union U* up) {}
import "C"
func f() {
C.f(C.enum_E(C.E0), (*C.union_U)(nil))
}
In Go 1.14, cgo translated this to (omitting irrelevant details):
type _Ctype_union_U [8]byte
func f() {
_Cfunc_f(uint32(_Ciconst_E0), (*[8]byte)(nil))
}
func _Cfunc_f(p0 uint32, p1 *[8]byte) (r1 _Ctype_void) { ... }
Notably, _Ctype_union_U was declared as a defined type, but uses were
being rewritten into uses of the underlying type, which matched how
_Cfunc_f was declared.
After CL 230037, cgo started consistently rewriting "C.foo" type
expressions as "_Ctype_foo", which caused it to start emitting:
type _Ctype_enum_E uint32
type _Ctype_union_U [8]byte
func f() {
_Cfunc_f(_Ctype_enum_E(_Ciconst_E0), (*_Ctype_union_U)(nil))
}
// _Cfunc_f unchanged
Of course, this fails to type-check because _Ctype_enum_E and
_Ctype_union_U are defined types.
This CL changes cgo to emit:
type _Ctype_enum_E = uint32
type _Ctype_union_U = [8]byte
// f unchanged since CL 230037
// _Cfunc_f still unchanged
It would probably be better to fix this in (*typeConv).loadType so
that cgo generated code uses the _Ctype_foo aliases too. But as it
wouldn't have any effect on actual compilation, it's not worth the
risk of touching it at this point in the release cycle.
Updates #39537.
Fixes#40494.
Change-Id: I88269660b40aeda80a9a9433777601a781b48ac0
Reviewed-on: https://go-review.googlesource.com/c/go/+/246057
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently pageAlloc.find attempts to find a better estimate for the
first free page in the heap, even if the space its looking for isn't
necessarily going to be the first free page in the heap (e.g. if npages
>= 2). However, in doing so it has the potential to return a searchAddr
candidate that doesn't actually correspond to mapped memory, but this
candidate might still be adopted. As a result, pageAlloc.alloc's fast
path may look at unmapped summary memory and segfault. This case is rare
on most operating systems since the heap is kept fairly contiguous, so
the chance that the candidate searchAddr discovered is unmapped is
fairly low. Even so, this is totally possible and outside the user's
control when it happens (in fact, it's likely to happen consistently for
a given user on a given system).
Fix this problem by ensuring that our candidate always points to mapped
memory. We do this by looking at mheap's arenas structure first. If it
turns out our candidate doesn't correspond to mapped memory, then we
look at inUse to round up the searchAddr to the next mapped address.
While we're here, clean up some documentation related to searchAddr.
Fixes#40191.
Change-Id: I759efec78987e4a8fde466ae45aabbaa3d9d4214
Reviewed-on: https://go-review.googlesource.com/c/go/+/242680
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We're reworking pclntab generation in the linker, and with that we're
moving FuncID generation in to the compiler. Determining the FuncID is
done by a lookup on the package.function name; therefore, we need the
package whenever we make the TEXT symbols.
Change-Id: I805445ffbf2f895f06ce3a91fb09126d012bf86e
Reviewed-on: https://go-review.googlesource.com/c/go/+/245318
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Move the function names out of runtime.pclntab_old, creating
runtime.funcnametab. There is an unfortunate artifact in this change in
that calculating the funcID still requires loading the name. Future work
will likely pull this out and put it into the object file Funcs.
ls -l cmd/compile (darwin):
before: 18524016
after: 18519952
The difference in size can be attributed to alignment in pclntab_old.
Change-Id: Ibcbb230d4632178f8fcd0667165f5335786381f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/243223
Reviewed-by: Austin Clements <austin@google.com>
