Changkun Ou [Mon, 14 Sep 2020 08:24:59 +0000 (10:24 +0200)]
testing: fix panicking tests hang if Cleanup calls FailNow
Previously, it was impossible to call FailNow in a Cleanup.
Because it can terminate a panicking goroutine and cause its
parent hangs on t.signal channel. This CL sends the signal
in a deferred call to prevent the hang.
Fixes #41355
Change-Id: I4552d3a7ea763ef86817bf9b50c0e37fb34bf20f
Reviewed-on: https://go-review.googlesource.com/c/go/+/254637 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
cmd/api: omit outside dependencies when listing the packages in "std"
As of CL 251159, when 'go list -deps std' is run within GOROOT/src, it
treats the vendored external dependencies as real module dependencies,
not standard-library "vendor/" packages (which still exist in that
case, but are treated as distinct packages outside the "std" module).
Fixes #41358
Updates #30241
Change-Id: Ic23eae9829d90e74a340d49ca9052e9191597410
Reviewed-on: https://go-review.googlesource.com/c/go/+/254738
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Martin Möhrmann [Mon, 13 Jul 2020 16:12:20 +0000 (18:12 +0200)]
runtime: add 24 byte allocation size class
This CL introduces a 24 byte allocation size class which
fits 3 pointers on 64 bit and 6 pointers on 32 bit architectures.
Notably this new size class fits a slice header on 64 bit
architectures exactly while previously a 32 byte size class
would have been used for allocating a slice header on the heap.
The main complexity added with this CL is that heapBitsSetType
needs to handle objects that aren't 16-byte aligned but contain
more than a single pointer on 64-bit architectures.
Due to having a non 16 byte aligned size class on 32 bit a
h.shift of 2 is now possible which means a heap bitmap byte might
only be partially written. Due to this already having been
possible on 64 bit before the heap bitmap code only needed
minor adjustments for 32 bit doublecheck code paths.
Note that this CL changes the slice capacity allocated by append
for slice growth to a target capacity of 17 to 24 bytes.
On 64 bit architectures the capacity of the slice returned by
append([]byte{}, make([]byte, 24)...)) is 32 bytes before and
24 bytes after this CL. Depending on allocation patterns of the
specific Go program this can increase the number of total
alloctions as subsequent appends to the slice can trigger slice
growth earlier than before. On the other side if the slice is
never appended to again above its capacity this will lower heap
usage by 8 bytes.
This CL changes the set of size classes reported in the
runtime.MemStats.BySize array due to it being limited to a
total of 61 size classes. The new 24 byte size class is now
included and the 20480 byte size class is not included anymore.
Change-Id: I9ecf2a10024513abef8fbfbe519e44e0b29b6167
Reviewed-on: https://go-review.googlesource.com/c/go/+/242258
Trust: Martin Möhrmann <moehrmann@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Change-Id: I1f67085a1e7c40f60282c2fea7104fb44a01e310
Reviewed-on: https://go-review.googlesource.com/c/go/+/254739
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
This CL ensures that responses served via CGI and FastCGI
have a Content-Type header based on the content of the
response if not explicitly set by handlers.
If the implementers of the handler did not explicitly
specify a Content-Type both CGI implementations would default
to "text/html", potentially causing cross-site scripting.
Thanks to RedTeam Pentesting GmbH for reporting this.
Dominic Della Valle [Sun, 13 Sep 2020 17:03:48 +0000 (17:03 +0000)]
make.bat: fix compare between GOROOT and srcdir paths, when either contains whitespace.
CL 96455 brings CL 57753 to Windows
However, a path comparison within it was left unquoted.
If the Go source directory resides in a path containing whitespace,
the interpreter will compare against the first portion of the path string,
and treat the remainder as an expression.
This patch amends that.
For example, consider the path
`C:\Users\Dominic Della Valle\Projects\Go\goroot\src`
Issuing `make.bat` will print out `'Della' is not recognized as an internal or external command, operable program or batch file.` before proceeding.
Change-Id: Ifcec159baeec940c29c61aa721c64c13c6fd8c14
GitHub-Last-Rev: 809ddbb4dbc80d834f8108ca44c2826016d78d1c
GitHub-Pull-Request: golang/go#41319
Reviewed-on: https://go-review.googlesource.com/c/go/+/253898
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Giovanni Bajo <rasky@develer.com>
Trust: Giovanni Bajo <rasky@develer.com>
Daniel Martí [Sat, 15 Aug 2020 14:20:50 +0000 (16:20 +0200)]
cmd/go: avoid flag.FlagSet.VisitAll at init time
We want to error early if GOFLAGS contains any flag that isn't known to
any cmd/go command. Thus, at init time we would recursively use VisitAll
on each of the flagsets to populate a map of all registered flags.
This was unfortunate, as populating said map constituted a whole 5% of
the run-time of 'go env GOARCH'. This is because VisitAll is pretty
expensive; it copies all the maps from the flagset's map to a slice,
sorts the slice, then does one callback per flag.
First, this was a bit wasteful. We only ever needed to query the
knownFlag map if GOFLAGS wasn't empty. If it's empty, there's no work to
do, thus we can skip the map populating work.
Second and most important, we don't actually need the map at all. A
flag.FlagSet already has a Lookup method, so we can simply recursively
call those methods for each flag in GOFLAGS. Add a hasFlag func to make
that evident.
This mechanism is different; its upfront cost is none, but it will
likely mean a handful of map lookups for each flag in GOFLAGS. However,
that tradeoff is worth it; we don't expect GOFLAGS to contain thousands
of flags. The most likely scenario is less than a dozen flags, in which
case constructing a "unified" map is not at all a net win.
One possible reason the previous mechanism was that way could be
AddKnownFlag. Thankfully, the one and only use of that API was removed
last year when Bryan cleaned up flag parsing in cmd/go.
The wins for the existing benchmark with an empty GOFLAGS are
significant:
name old time/op new time/op delta
ExecGoEnv-8 575µs ± 1% 549µs ± 2% -4.44% (p=0.000 n=7+8)
name old sys-time/op new sys-time/op delta
ExecGoEnv-8 1.69ms ± 1% 1.68ms ± 2% ~ (p=0.281 n=7+8)
name old user-time/op new user-time/op delta
ExecGoEnv-8 1.80ms ± 1% 1.66ms ± 2% -8.09% (p=0.000 n=7+8)
To prove that a relatively large number of GOFLAGS isn't getting
noticeably slower, we measured that as well, via benchcmd and GOFLAGS
containing 50 valid flags:
GOFLAGS=$(yes -- -race | sed 50q) benchcmd -n 500 GoEnvGOFLAGS go env GOARCH
And the result, while noisy, shows no noticeable difference (note that
it measures 3ms instead of 0.6ms since it's sequential):
name old time/op new time/op delta
GoEnvGOFLAGS 3.04ms ±32% 3.03ms ±35% ~ (p=0.156 n=487+481)
Finally, we've improved the existing Go benchmark. Now it's parallel,
and it also reports sys-time and user-time, which are useful metrics.
Change-Id: I9b4551415cedf2f819eb184a02324b8bd919e2bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/248757 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jay Conrod [Fri, 11 Sep 2020 13:31:30 +0000 (09:31 -0400)]
cmd/go: make 'go mod download' update go.sum after downloads are complete
'go mod download' calls WriteGoMod once via modload.ListModules when
it loads the build list. This saves sums for go.mod files needed by
MVS, but the write occurs before any zip files are downloaded.
With this change, 'go mod download' calls WriteGoMod again (and thus,
modfetch.WriteGoSum) after downloading and verifying module zip files,
so the sums of the zip files will be saved, too.
Fixes #41341
Change-Id: I7d56754aa255256ed45fd93cb154c2e6ea5f45a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/254357
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
cmd/link: put read-only data in __DATA_CONST segment
On darwin, we put read-only data in __TEXT segment on AMD64 in
exe (non-PIE) buildmode, and in __DATA on everywhere else. This
is not ideal: things in __DATA segment are not read-only, and
being mapped R/W may use more run-time resources.
In fact, newer darwin systems support a __DATA_CONST segment,
which the dynamic linker will map it read-only after applying
relocations. Use that.
Fixes #38830.
Change-Id: Ic281e6c6ca8ef5fec4bb7c5b71c50dd5393e78ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/253919 Reviewed-by: Than McIntosh <thanm@google.com>
Jay Conrod [Wed, 9 Sep 2020 20:41:55 +0000 (16:41 -0400)]
cmd/go: refactor modload.Import for better -mod=readonly errors
When -mod=readonly is set, Import will now allow imports from
replacements without explicit requirements. With -mod=mod, this would
add a new requirement but does not trigger a module lookup, so it's
determinisitic.
Before reporting an error for an unknown import with -mod=readonly,
check whether the import is valid. If there's a typo in the import,
that's more relevant.
For #40728
Change-Id: I05e138ff76ba3d0eb2e3010c15589fa363deb8d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/253745
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Jay Conrod [Wed, 9 Sep 2020 20:35:56 +0000 (16:35 -0400)]
cmd/go: refactor -mod flag parsing
Keep track of whether the -mod flag was set explicitly. When
-mod=readonly is the default, we'll want to adjust our error messages
if it's set explicitly.
Also, register the -mod, -modcacherw, and -modfile flags in functions
in internal/base instead of internal/work. 'go mod' commands that
don't load packages shouldn't depend on internal/work.
For #40728
Change-Id: I272aea9e19908ba37e151baac4ea8630e90f241f
Reviewed-on: https://go-review.googlesource.com/c/go/+/253744
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
cmd/link: record only the first occurance in Reachparent graph
In the deadcode pass, a type symbol may be marked twice, one
without UsedInIface, one with. For the second time, don't
update the Reachparent graph, so it only records the path of
the first time the symbol is reached. This ensures the
Reachparent graph is acyclic.
TODO: add a test. (This only affects GOEXPERIMENT=fieldtrack)
eric fang [Wed, 13 May 2020 06:38:39 +0000 (06:38 +0000)]
unicode/utf8: refactor benchmarks for FullRune function
BenchmarkFullASCIIRune tests the performance of function utf8.FullRune,
which will be inlined in BenchmarkFullASCIIRune. Since the return value
of FullRune is not referenced, it will be removed as dead code.
This CL makes the FullRune functions return value referenced by a global
variable to avoid this point. In addition, this CL adds one more benchmark
to cover more code paths, and puts them together as sub benchmarks of
BenchmarkFullRune.
Change-Id: I6e79f4c087adf70e351498a4b58d7482dcd1ec4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/233979
Run-TryBot: eric fang <eric.fang@arm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Junchen Li [Mon, 31 Aug 2020 05:32:33 +0000 (13:32 +0800)]
cmd/asm: add more SIMD instructions on arm64
This CL adds USHLL, USHLL2, UZP1, UZP2, and BIF instructions requested
by #40725. And since UXTL* are aliases of USHLL*, this CL also merges
them into one case.
fanzha02 [Thu, 20 Aug 2020 09:02:18 +0000 (17:02 +0800)]
cmd/internal/obj/arm64: enable some SIMD instructions
Enable VBSL, VBIT, VCMTST, VUXTL VUXTL2 and FMOVQ SIMD
instructions required by the issue #40725. And FMOVQ
instrucion is used to move a large constant to a Vn
register.
I found the control flow of this function a bit tricky to reason about
due to nesting and interaction between conditions and iteration. This
change factors out a helper function that can return early instead of
mixing conditionals and 'continue' statements.
Also remove the (unused) ModuleUsedDirectly function.
For #36460
Change-Id: I60a2a5a1b32989e5a17a14e1a8c858b280cda8f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/251998
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
cmd/go/internal/modload: rename LoadBuildList and BuildList
With lazy loading, the “build list” can be refined as packages are loaded.
Rename functions that return the build list to more precisely describe
the set of modules returned by the call.
Also eliminate a redundant call to LoadBuildList (right before
ListModules, which itself begins with the same call).
For #36460
Change-Id: I0fc4f9dd7602e0df5e166e329ee5d516d810ca53
Reviewed-on: https://go-review.googlesource.com/c/go/+/249878
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Bryan C. Mills [Fri, 13 Mar 2020 20:32:37 +0000 (16:32 -0400)]
cmd/go/internal/modload: implement the "all" pattern for lazy loading
The new semantics of the "all" package pattern can be implemented
without actually changing module loading per se. This change
implements those semantics, so that the change can be decoupled from
the changes to the module requirement graph.
For #36460
Change-Id: I0ee8b17afa8b728dc470a42a540fcc01764a4442
Reviewed-on: https://go-review.googlesource.com/c/go/+/240623
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Due to a bug in CL 173017, if QueryPackages found multiple candidates
for the given package and *at least* one of those candidates was not
available to add, we would reject *all* such candidates — even those
that were still viable.
Now, we return the first viable candidate, and only return an error if
*no* candidate is viable given the current build list.
Fixes #41113
Change-Id: Idb2e77244be7c0f5dd511efb142c3059925d7336
Reviewed-on: https://go-review.googlesource.com/c/go/+/251446
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Steven Hartland [Mon, 31 Aug 2020 20:37:40 +0000 (21:37 +0100)]
net/http: fix data race due to writeLoop goroutine left running
Fix a data race for clients that mutate requests after receiving a
response error which is caused by the writeLoop goroutine left
running, this can be seen on cancelled requests.
Fixes #37669
Change-Id: Ia4743c6b8abde3a7503de362cc6a3782e19e7f60
Reviewed-on: https://go-review.googlesource.com/c/go/+/251858 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills [Fri, 28 Aug 2020 22:19:22 +0000 (18:19 -0400)]
cmd/go/internal/modload: rework import resolution
modload.Import previously performed two otherwise-separable tasks:
1. Identify which module in the build list contains the requested
package.
2. If no such module exists, search available modules to try to find
the missing package.
This change splits those two tasks into two separate unexported
functions, and reports import-resolution errors by attaching them to
the package rather than emitting them directly to stderr. That allows
'list' to report the errors, but 'list -e' to ignore them.
With the two tasks now separate, it will be easier to avoid the
overhead of resolving missing packages during lazy loading if we
discover that some existing dependency needs to be promoted to the top
level (potentially altering the main module's selected versions, and
thus suppling packages that were previously missing).
For #36460
Updates #26909
Change-Id: I32bd853b266d7cd231d1f45f92b0650d95c4bcbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/251445
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
forceStdVendor was a special-case mechanism to allow Go contributors
to use vendored dependencies by default when working in GOROOT/src.
As of Go 1.14,¹ the 'go' command uses vendored dependencies by default
within all modules, so the 'std' and 'cmd' modules no longer need to
be special cases, and we can remove this special-case code.
¹ https://golang.org/doc/go1.14#vendor
Updates #33848
Updates #30241
Change-Id: Ib2fb5841c253113b17fa86a086ce85a22ac3d121
Reviewed-on: https://go-review.googlesource.com/c/go/+/251159
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
cmd/go/internal/modload: track which packages are in 'all' during loading
If the user requests the 'all' pattern in addition to explicit roots
outside of 'all', we should not load the transitive dependencies of
those explicit roots as if they were *in* 'all'. Without the '-test'
flag, we should not load test dependencies of any package outside of
'all'. Even *with* the '-test' flag, we should only load test
dependencies of the requested roots, not test dependencies of other
packages that happen to be imported by those roots.
More precise tracking of membership in 'all' will be important when we
implement lazy loading, because membership in 'all' determines which
module dependencies we will record in the main module's go.mod file.
This change also reduces reliance on global state, factors out the
loading process into several smaller functions, and sets us up to
reuse the 'go mod vendor' version of the 'all' pattern for lazy
loading.
For #36460
Fixes #40799
Change-Id: I5ca21c86a860daee1316f732cea131a331d8ddf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/240505
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Ian Lance Taylor [Wed, 9 Sep 2020 17:40:11 +0000 (10:40 -0700)]
runtime: ignore SIGPROF if profiling disable for thread
This avoids a deadlock on prof.signalLock between setcpuprofilerate
and cpuprof.add if a SIGPROF is delivered to the thread between the
call to setThreadCPUProfiler and acquiring prof.signalLock.
Fixes #41014
Change-Id: Ie825e8594f93a19fb1a6320ed640f4e631553596
Reviewed-on: https://go-review.googlesource.com/c/go/+/253758
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Michael Anthony Knyszek [Wed, 9 Sep 2020 16:52:18 +0000 (16:52 +0000)]
runtime: fix ReadMemStatsSlow's and CheckScavengedBits' chunk iteration
Both ReadMemStatsSlow and CheckScavengedBits iterate over the page
allocator's chunks but don't actually check if they exist. During the
development process the chunks index became sparse, so now this was a
possibility. If the runtime tests' heap is sparse we might end up
segfaulting in either one of these functions, though this will generally
be very rare.
The pattern here to return nil for a nonexistent chunk is also useful
elsewhere, so this change introduces tryChunkOf which won't throw, but
might return nil. It also updates the documentation of chunkOf.
Fixes #41296.
Change-Id: Id5ae0ca3234480de1724fdf2e3677eeedcf76fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/253777
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Cherry Zhang [Tue, 11 Aug 2020 23:57:50 +0000 (19:57 -0400)]
cmd/link: pre-resolve package reference
Pre-resolve package index references, so it doesn't need to do a
map lookup in every cross-package symbol reference resolution. It
increases the memory usage very slightly (O(# imported packages)).
Jay Conrod [Fri, 28 Aug 2020 19:05:44 +0000 (15:05 -0400)]
cmd/go: make 'go get' preserve sums for content of new requirements
This preserves zip sums when 'go get' is run on a module that does not
have a package in the root directory. The zip must be fetched to
determine whether the package should be loaded, so we already load and
verify the sum.
Note that 'go mod tidy' may still remove these sums, since they
aren't needed to load packages.
Fixes #41103
Change-Id: I78f10a25f0392461fdc98518a7c92a38ee3233c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/251880
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
It appears the machoCalcStart function is meant to align the
segment, but it doesn't. Replace it with an actual alignment
calculation. Also, use the alignment from the configuration,
instead of hardcode.
With this fix we could enable DWARF combining on macOS ARM64.
Change-Id: I19ec771b77d752b83a54c53b6ee65af78a31b8ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/253558 Reviewed-by: Than McIntosh <thanm@google.com>
cmd/link: rewrite some code without using reflection
In Mach-O DWARF combining, some code was written using reflection,
so it could support both 32-bit and 64-bit Mach-O files without
duplicating code. We no longer support 32-bit darwin platforms
now. 32-bit support can go. Rewrite it with direct field access,
for 64-bit only.
Cherry Zhang [Wed, 26 Aug 2020 22:19:03 +0000 (18:19 -0400)]
cmd/link: strip STAB (symbolic debugging) symbols on darwin
On darwin, with external linking, the system linker produces STAB
(symbolic debugging) symbols in the binary's symbol table. These
include paths of the intermediate object files, like
<tmpdir>/go.o, which changes from run to run, making the build
non-reproducible.
Since we run dsymutil to produce debug info and combine them
back into the binary, we don't need those STAB symbols anymore.
Strip them after running dsymutil.
If DWARF is not enabled, we don't run dsymutil. We can pass
"-Wl,-S" to let the system linker not generate those symbols.
While here, also make it more consistent about DWARF combining.
Currently we only do DWARF combining on macOS/AMD64, when DWARF
is enabled. On ARM64, we run dsymutil, but then throw the result
away. This CL changes it to not run dsymutil (and strip) on
ARM64.
TODO: add a test. We don't do it here as it fails on some
(non-darwin) platforms.
Fixes #40979.
Change-Id: If770f7828cdb858857d6079e0585bf067f8f7a92
Reviewed-on: https://go-review.googlesource.com/c/go/+/250944
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
fanzha02 [Thu, 20 Aug 2020 10:39:46 +0000 (18:39 +0800)]
cmd/asm: fix the error of checking the post-index offset of VLD[1-4]R instructions of arm64
The post-index offset of VLD[1-4]R instructions is decided by the
"size" field not "Q" field, the current assembler uses "Q" fileld
to check the correctness of post-index offset which is not correct.
This patch fixes it.
Alberto Donizetti [Mon, 27 Jul 2020 12:42:06 +0000 (14:42 +0200)]
go/ast: note that in BasicLit CHARs and STRINGs are quoted
The Value field of ast.BasicLit is a string field holding the literal
string. For CHARs and STRINGs, the BasicLit.Value literal includes
quotes, so to use the value in practise one will often need to Unquote
it.
Since this is a common gotcha (I've been bitten by this a few times),
document it, and suggest the use of the strconv.Unquote functions.
Fixes #39590
Change-Id: Ie3e13f5a2a71bb1b59e03bc5b3a16d8e2e7c01d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/244960 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Michael Fraenkel [Sat, 27 Jun 2020 19:31:34 +0000 (13:31 -0600)]
net/http: mark http/2 connections active
On Server.Shutdown, all idle connections are closed.
A caveat for new connections is that they are marked idle
after 5 seconds.
Previously new HTTP/2 connections were marked New, and after 5 seconds,
they would then become idle. With this change, we now mark HTTP/2
connections as Active to allow the proper shutdown sequence to occur.
Ainar Garipov [Wed, 26 Aug 2020 20:55:09 +0000 (23:55 +0300)]
doc/go1.16: reformat the minor changes section as a definition list
Change the section to use <dl>, <dt>, and <dd> tags to match
previous documents.
Change-Id: Ide0bea698a84ed6b61b364ef9e2f3801ebb8d4d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/250897 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Changkun Ou [Fri, 28 Aug 2020 10:13:37 +0000 (12:13 +0200)]
testing: make TempDir idempotent for both Cleanup and Benchmark
Ensures that calling TempDir() in either of Cleanup or Benchmark
doesn't cause test failures which were previously caused by the
created directory having been deleted after the first run, yet
we prevented the recreation of the directory due to our selection
of concurrency primitive sync.Once. This change recreates the
temporary directory if it doesn't exist, regardless of how
many times Cleanup and Benchmark are invoked.
Benjamin Barenblat [Thu, 3 Sep 2020 20:36:38 +0000 (16:36 -0400)]
runtime: opportunistically rotate map key seed
When clearing a map, reinitialize the hash seed with random data. This
makes it more difficult for attackers to trigger pathological
performance via repeated hash collisions.
The extra reinitialization causes no statistically significant slowdown:
cmd/link: pass darwin/amd64-specific flags only on AMD64
The linker assumed macOS is AMD64 (and 386 in the past). It
passes darwin/amd64-specific flags to the external linker when
building for macOS. They don't work for ARM64-based macOS. So
only pass them on AMD64.
Disable DWARF combining for macOS ARM64 for now. The generated
binary doesn't run. (TODO: fix.)
fanzha02 [Thu, 27 Aug 2020 09:34:59 +0000 (17:34 +0800)]
cmd/compile: store the comparison pseudo-ops of arm64 conditional instructions in AuxInt
The current implementation stores the comparison pseudo-ops of arm64
conditional instructions (CSEL/CSEL0) in Aux, this patch modifies it
and stores it in AuxInt, which can avoid the allocation.
cmd/go: define an asm macro for GOEXPERIMENT=regabi
This defines a macro for the regabi GOEXPERIMENT when assembling
runtime assembly code.
In general, assembly code will be shielded from the calling convention
change, but there is a small amount of runtime assembly that is going
to have to change. By defining a macro, we can easily make the small
necessary changes. The other option is to use build tags, but that
would require duplicating nontrivial amounts of unaffected code,
leading to potential divergence issues. (And unlike Go code, assembly
code can't depend on the compiler optimizing away branches on a
feature constant.) We consider the macro preferable, especially since
this is expected to be temporary as we transition to the new calling
convention.
This is the "feature flag" for the register calling convention work
(though since this work is expected to extend over a few releases,
it's not version-prefixed). This will let us develop the register
calling convention on the main branch while maintaining an easy toggle
between the old and new ABIs.
Cuong Manh Le [Wed, 19 Aug 2020 09:05:02 +0000 (16:05 +0700)]
cmd/compile: do not declare func nodes
The primary responsibility of declare() to associate a symbol (Sym) with
a declaration (Node), so "oldname" will work. Function literals are
anonymous, so their symbols does not need to be declared.
Passes toolstash-check.
Change-Id: I739b1054e3953e85fbd74a99148b9cfd7e5a57eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/249078
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Dan Kortschak [Wed, 2 Sep 2020 22:57:34 +0000 (08:27 +0930)]
cmd/go: add -Wl,-Bsymbolic-functions to cgo flags whitelist
Closes #41199
Change-Id: Iab69358e8c39e6d2b2797c7ce750df63aa7e96b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/252698
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
net/http: reject negative suffix-length Range:bytes=--N with 416 status code
Fixes the file server to reject requests of the form:
"Range": "bytes=--N"
where "-N" is a negative suffix-length as designated by the
grammar in RFC 7233 Section 2.1, "Byte-Ranges", which specifies
that suffix-length MUST be of the form 1*DIGIT aka a non-negative digit.
Thus requests such as:
"Range": "bytes=--2"
will be rejected with a "416 Range Not Satisfiable" response.
Bryan C. Mills [Wed, 19 Aug 2020 02:47:12 +0000 (22:47 -0400)]
testing: flush test summaries to stdout atomically when streaming output
While debugging #40771, I realized that the chatty printer should only
ever print to a single io.Writer (normally os.Stdout). The other
Writer implementations in the chain write to local buffers, but if we
wrote a test's output to a local buffer, then we did *not* write it to
stdout and we should not store it as the most recently logged test.
Because the chatty printer should only ever print to one place, it
shouldn't receive an io.Writer as an argument — rather, it shouldn't
be used at all for destinations other than the main output stream.
On the other hand, when we flush the output buffer to stdout in the
top-level flushToParent call, it is important that we not allow some
other test's output to intrude between the test summary header and the
remainder of the test's output. cmd/test2json doesn't know how to
parse such an intrusion, and it's confusing to humans too.
No test because I couldn't reproduce the user-reported error without
modifying the testing package. (This behavior seems to be very
sensitive to output size and/or goroutine scheduling.)
Fixes #40771
Updates #38458
Change-Id: Ic19bf1d535672b096ba1c8583a3b74aab6d6d766
Reviewed-on: https://go-review.googlesource.com/c/go/+/249026
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
chainhelen [Fri, 21 Aug 2020 16:44:52 +0000 (16:44 +0000)]
runtime: fix panic if newstack at runtime.acquireLockRank
Process may crash becaues acquireLockRank and releaseLockRank may
be called in nosplit context. With optimizations and inlining
disabled, these functions won't get inlined or have their morestack
calls eliminated.
Nosplit is not strictly required for lockWithRank, unlockWithRank
and lockWithRankMayAcquire, just keep consistency with lockrank_on.go
here.
cgo effectively prepends -I${SRCDIR} to the header include path of all
preambles it processes, so when an #include <> matches a header file
both in the source directory and also another include directory, the
local copy will be used in preference.
This behaviour is surprising but unfortunately also longstanding and
relied upon by packages in the wild, so the best we can do is to
document it.
Fixes #41059
Change-Id: If6d2818294b2bd94ea0fe5fd6ce77e54b3e167a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/251758 Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/go: simplify and document lazy-loading test cases
I decided to add package and module diagrams to the test cases to make
them easier to follow.
While adding those diagrams, I noticed some strong similarities among
a couple of the graphs, so I consolidated those cases (and deleted the
redundant tests).
For #36460
Change-Id: Id6cd04fc871379b83851c2d1af89ea9296a0f3e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/251997
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Michael Matloob [Tue, 1 Sep 2020 19:41:36 +0000 (15:41 -0400)]
cmd/go: remove TestScript/get_unicode and an internal get test case
That test tested that import paths with non-ASCII unicode paths
were allowed by the Go command. Remove this test case because
golang.org/cl/251878 removes that support.
Also rewrite a test case in TestRepoRootForImportPath in the test
for cmd/go/internal/get to reflect that unicode directory names are now
disallowed.
Updates #29101
Change-Id: I669e220facd04fc82ccd05dd08e8f1ff4d48b1fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/252297
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Changkun Ou [Mon, 31 Aug 2020 18:54:17 +0000 (20:54 +0200)]
testing: fail Example tests that invoke runtime.Goexit
Previously, if an example test invoked runtime.Goexit, it would
pass yet hang until a timeout, while regular tests that invoke
runtime.Goexit do fail. This change removes that inconsistent
behavior and makes such example tests fail, and panic with an
indication of having invoked runtime.Goexit.
Michael Matloob [Mon, 31 Aug 2020 20:12:35 +0000 (16:12 -0400)]
cmd/go/internal/get: disallow non-ASCII unicode letters from import paths
The copy of CheckImportPath in path.go and the regular expression for github
repos in vcsPaths together allow import paths with unicode letters with import
paths. These all come from github repos with non-ASCII unicode letters
with paths in directories. This mainly shows up in GOPATH mode, but could
also show up in Module mode when getting a module in GOPROXY=direct mode.
We expect there to not be any significant affected users of this change--
an investingation of github repos that would produce import paths that
would comply with the copy CheckImportPaths that's being removed, but not
modload.CheckImportPaths only surfaced a handful of cases, all of which
seemed to be small test or demonstation repos. But this CL is being
submitted early in the cycle so that it can be backed out if need be.
Updates #29101
Change-Id: I719df4af5b318e1330e90d8a0bffe5bb8d816f4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/251878
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>
Michael Matloob [Mon, 31 Aug 2020 20:29:13 +0000 (16:29 -0400)]
cmd: update vendored golang.org/x/mod
This pulls in golang.org/cl/250920 which rejects Windows shortnames as
path components in module.CheckImportPath (as is already done in
cmd/go/internal/get's copy of CheckImportPath). This will allow us to replace
the copy of CheckImportPath with the original.
This also pulls in golang.org/cl/250919 which rejects + in CheckPath and
CheckImportPath, and golang.org/cl/235597, which adds methods to the zip
package for gorelease, but shouldn't affect cmd.
This change also updates the cmd/go test case TestScript/mod_bad_filenames
to reflect that golang.org/x/mod/zip error messages now include filenames
for bad file names that can't be included in zip archives.
Updates #29101
Change-Id: I7f654325dc33b19bc9c6f77a56546747add5a47f
Reviewed-on: https://go-review.googlesource.com/c/go/+/251877
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>
Paul Forgey [Tue, 1 Sep 2020 00:38:01 +0000 (00:38 +0000)]
net/http: refactor ResponseWriter.ReadFrom to permit splice on Linux
Rather than probe and guess if sendfile will work inside ResponseWriter.ReadFrom(src),
this change fixes the underlying issue of starting to respond before src is readable
We'll no longer send a status OK if a header has not yet been written and reading
from src is destined to fail. This small change implicitly takes care of the need for
the server to sniff the response body to determine the Content-Type.
This allows splice to work on Linux when src is a socket or any non-regular file that's spliceable.
The extra read of 512 bytes may raise an objection, and that's fair, but
we're already swapping some syscall prep work for another and a read of
512 probably will not impact the overall performance. For shorter
bodies, there's likely less setup time. A little initial slop is not too
unusual in zero copy network code, and sometimes actually helps.
Victor Michel [Sun, 30 Aug 2020 20:43:39 +0000 (20:43 +0000)]
debug/elf: support relocations relative to sections with non-zero addresses
commit 72ec930fa70c20ce69b21bf32a7916c04c2e9c2f added basic support for
relocations, but assumed that the symbol value would be 0, likely because
.debug_info always has address == 0 in the ELF section headers.
CL 195679 added further support for relocations, but explicitly encoded
the original assumption that section addresses would be 0.
This change removes that assumption: all relocations will now be
properly computed based on the target symbol value even when that symbol
is a section with a non-zero address.
Typically, sections that are part of a LOAD program segment have
non-zero addresses. For example, .debug_ranges relocations could be
relative to .text, which usually has an address > 0.
Fixes #40879
Change-Id: Ib0a616bb8b05d6c96d179b03ca33a10946fc5d59
GitHub-Last-Rev: 4200de732641995f3a4958a13a5c78f65b7eae50
GitHub-Pull-Request: golang/go#41038
Reviewed-on: https://go-review.googlesource.com/c/go/+/250559
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
David Finkel [Sat, 25 Apr 2020 17:32:06 +0000 (13:32 -0400)]
cmd/compile: adjust some AMD64 rewrite rules to use typed aux fields
Remove an extra int32-representable check when deciding to use an int32
constant as an immediate value.
Comment out a broken optimization that relies on MaxUint32 being
representable by a signed int32. It never triggers and when fixed, the
signedness of the auxint prevents other optimization passes from
handling it properly, thus causing segfaults in the runtime.
Remove a couple offset representable in 32-bits checks on 32-bit aux
vals.
toolstash-check clean
Change-Id: I148b53403fde523c90d692cb90e412460664b439
Reviewed-on: https://go-review.googlesource.com/c/go/+/230458 Reviewed-by: Keith Randall <khr@golang.org>
Keith Randall [Fri, 28 Aug 2020 17:10:32 +0000 (17:10 +0000)]
cmd/compile,cmd/asm: simplify recording of branch targets, take 2
We currently use two fields to store the targets of branches.
Some phases use p.To.Val, some use p.Pcond. Rewrite so that
every branch instruction uses p.To.Val.
p.From.Val is also used in rare instances.
Introduce a Pool link for use by arm/arm64, instead of
repurposing Pcond.
This is a cleanup CL in preparation for some stack frame CLs.