]> Cypherpunks repositories - gostls13.git/log
gostls13.git
4 years agocmd/buildid: move and reuse duplicated HashToString code to cmd/internal/buildid...
Mikhail Fesenko [Wed, 28 Oct 2020 20:35:23 +0000 (20:35 +0000)]
cmd/buildid: move and reuse duplicated HashToString code to cmd/internal/buildid/buildid

Change-Id: I1e1ac770d4aac12d7d7ec57ef95f77a3e14a678c
GitHub-Last-Rev: c01db4346eb08ffe0c1953892fb4222764048e30
GitHub-Pull-Request: golang/go#42052
Reviewed-on: https://go-review.googlesource.com/c/go/+/263418
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>

4 years agonet/url: improve performance for resolvePath
imxyb [Wed, 28 Oct 2020 21:26:04 +0000 (21:26 +0000)]
net/url: improve performance for resolvePath

benchmark compare results:

benchmark                   old ns/op     new ns/op     delta
BenchmarkResolvePath-12     297           141           -52.53%

benchmark                   old allocs     new allocs     delta
BenchmarkResolvePath-12     5              3              -40.00%

benchmark                   old bytes     new bytes     delta
BenchmarkResolvePath-12     181           24            -86.74%

Change-Id: Ia69e9fb36abb5930ed49217b5219be62b57ec429
GitHub-Last-Rev: e16dd9f7415178120f67f472bf45a2b006e00a93
GitHub-Pull-Request: golang/go#42180
Reviewed-on: https://go-review.googlesource.com/c/go/+/264817
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

4 years agocmd/go: use internal/testenv instead of computing canRun and skipExternal ad-hoc
Bryan C. Mills [Wed, 28 Oct 2020 14:44:24 +0000 (10:44 -0400)]
cmd/go: use internal/testenv instead of computing canRun and skipExternal ad-hoc

Fixes #42223

Change-Id: Icf9bb61d48f0a6c7fd6f74e80e333a4837aa52ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/265781
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agodoc: mention Trust+1 in contribution guide
Ian Lance Taylor [Mon, 5 Oct 2020 23:00:08 +0000 (16:00 -0700)]
doc: mention Trust+1 in contribution guide

For #40699

Change-Id: If753a073488880433ae3319dcf2a2dfaa887fd0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/259737
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Russ Cox <rsc@golang.org>
4 years agocmd/compile: rename mergeSymTyped to mergeSym
Alberto Donizetti [Wed, 28 Oct 2020 09:10:55 +0000 (10:10 +0100)]
cmd/compile: rename mergeSymTyped to mergeSym

Also make canMergeSym take Syms instead of interface{}

Change-Id: I4926a1fc586aa90e198249d67e5b520404b40869
Reviewed-on: https://go-review.googlesource.com/c/go/+/265817
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
4 years agocmd/compile: delete log2, switch to log64
Alberto Donizetti [Tue, 27 Oct 2020 10:30:14 +0000 (11:30 +0100)]
cmd/compile: delete log2, switch to log64

rewrite.go has two identical functions log2 and log64; the former has
been there for a while, while the latter was added together with
log{8,16,32} for use in typed rules.

This change deletes log2 and switches to using log64 everywhere.

Change-Id: I759b878814e4c115a5fa470274f22477738d69ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/265457
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
4 years agoruntime: add defs for openbsd/mips64
Joel Sing [Tue, 25 Aug 2020 07:53:08 +0000 (17:53 +1000)]
runtime: add defs for openbsd/mips64

Update #40995

Change-Id: I6963ead1a7c4520092361cce80edb17010e7f436
Reviewed-on: https://go-review.googlesource.com/c/go/+/250579
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
4 years agoruntime: remove new g register (X27) from preempt save/restore
Joel Sing [Tue, 27 Oct 2020 12:10:13 +0000 (23:10 +1100)]
runtime: remove new g register (X27) from preempt save/restore

The g register is now in X27 (previously X4, which collided with TP usage). Remove
X27 from preempt save/restore.

Change-Id: I9dd38ec3a8222fa0710757463769dbfac8ae7d20
Reviewed-on: https://go-review.googlesource.com/c/go/+/265517
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agoruntime: try to elide timer stealing if P has no timers
Michael Pratt [Mon, 5 Oct 2020 22:12:35 +0000 (18:12 -0400)]
runtime: try to elide timer stealing if P has no timers

Following golang.org/cl/259578, findrunnable still must touch every
other P in checkTimers in order to look for timers to steal. This scales
poorly with GOMAXPROCS and potentially performs poorly by pulling remote
Ps into cache.

Add timerpMask, a bitmask that tracks whether each P may have any timers
on its timer heap.

Ideally we would update this field on any timer add / remove to always
keep it up to date. Unfortunately, updating a shared global structure is
antithetical to sharding timers by P, and doing so approximately doubles
the cost of addtimer / deltimer in microbenchmarks.

Instead we only (potentially) clear the mask when the P goes idle. This
covers the best case of avoiding looking at a P _at all_ when it is idle
and has no timers. See the comment on updateTimerPMask for more details
on the trade-off. Future CLs may be able to expand cases we can avoid
looking at the timers.

Note that the addition of idlepMask to p.init is a no-op. The zero value
of the mask is the correct init value so it is not necessary, but it is
included for clarity.

Benchmark results from WakeupParallel/syscall/pair/race/1ms (see
golang.org/cl/228577). Note that these are on top of golang.org/cl/259578:

name                        old msec           new msec   delta
Perf-task-clock-8           244 ± 4%           246 ± 4%     ~     (p=0.841 n=5+5)
Perf-task-clock-16          247 ±11%           252 ± 4%     ~     (p=1.000 n=5+5)
Perf-task-clock-32          270 ± 1%           268 ± 2%     ~     (p=0.548 n=5+5)
Perf-task-clock-64          302 ± 3%           296 ± 1%     ~     (p=0.222 n=5+5)
Perf-task-clock-128         358 ± 3%           352 ± 2%     ~     (p=0.310 n=5+5)
Perf-task-clock-256         483 ± 3%           458 ± 1%   -5.16%  (p=0.008 n=5+5)
Perf-task-clock-512         663 ± 1%           612 ± 4%   -7.61%  (p=0.008 n=5+5)
Perf-task-clock-1024      1.06k ± 1%         0.95k ± 2%  -10.24%  (p=0.008 n=5+5)

Updates #28808
Updates #18237

Change-Id: I4239cd89f21ad16dfbbef58d81981da48acd0605
Reviewed-on: https://go-review.googlesource.com/c/go/+/264477
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Trust: Michael Pratt <mpratt@google.com>

4 years agoRevert "cmd/compile: split exported/non-exported methods for interface type"
Cuong Manh Le [Wed, 21 Oct 2020 17:25:17 +0000 (00:25 +0700)]
Revert "cmd/compile: split exported/non-exported methods for interface type"

This reverts commit 8f26b57f9afc238bdecb9b7030bc2f4364093885.

Reason for revert: break a bunch of code, include standard library.

Fixes #42123

Change-Id: Ife90ecbafd2cb395623d1db555fbfc9c1b0098e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/264026
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
4 years agoos: do not use procfs for os.Executable in dragonfly
Antonio Huete Jimenez [Wed, 28 Oct 2020 11:44:26 +0000 (11:44 +0000)]
os: do not use procfs for os.Executable in dragonfly

  procfs(5) is not always mounted in DragonFly BSD, for example during
  the binary package build with synth. os.Executable() consumers
  will then fail, we've spotted this when trying to build tinygo:

    [...]

    copying source files
    ./build/tinygo build-builtins -target=armv6m-none-eabi [...]
    panic: could not get executable path: readlink /proc/curproc/file:
    no such file or directory

    [...]

  Use KERN_PROC_PATHNAME as FreeBSD does.

Change-Id: Ic65bea02cd0309fb24dec8ba8d2b151d1acde67b
GitHub-Last-Rev: 083120a43b3158bb45d7e1a66fa32e3335a2d407
GitHub-Pull-Request: golang/go#36826
Reviewed-on: https://go-review.googlesource.com/c/go/+/216622
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Brad Fitzpatrick <bradfitz@golang.org>

4 years agodatabase/sql: fix tx stmt deadlock when rollback
Tzu-Chiao Yeh [Mon, 24 Aug 2020 14:04:17 +0000 (22:04 +0800)]
database/sql: fix tx stmt deadlock when rollback

Tx acquires tx.closemu W-lock and then acquires stmt.closemu.W-lock
to fully close the transaction and associated prepared statement.
Stmt query and execution run in reverse ways - acquires
stmt.closemu.R-lock and then acquires tx.closemu.R-lock to grab tx
connection, which may cause deadlock.

Prevent the lock is held around tx.closePrepared to ensure no
deadlock happens.

Fixes #40985

Change-Id: If53909822b87bce11861a6e3035ecb9476d2cd17
Reviewed-on: https://go-review.googlesource.com/c/go/+/250178
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

4 years agocmd/go/internal/modfetch: drop gopkg.in/russross/blackfriday.v2 from TestCodeRepoVersions
Tobias Klauser [Wed, 28 Oct 2020 12:44:53 +0000 (13:44 +0100)]
cmd/go/internal/modfetch: drop gopkg.in/russross/blackfriday.v2 from TestCodeRepoVersions

Follow-up for CL 265819.

Given the -pre tag added recently, a new stable version is likely
tagged soon. This would break TestCodeRepoVersions on the longtest
builders again. Since the other test cases in codeRepoVersionsTests
already provide enough coverage, drop gopkg.in/russross/blackfriday.v2
to avoid breaking TestCodeRepoVersions once the release happens.

Updates #28856

Change-Id: If86a637b5e47f59faf9048fc1cbbae6e8f1dcc53
Reviewed-on: https://go-review.googlesource.com/c/go/+/265917
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agoruntime: add edge lockRankSysmon -> lockRankRwmutexR
Dan Scales [Tue, 27 Oct 2020 22:52:42 +0000 (15:52 -0700)]
runtime: add edge lockRankSysmon -> lockRankRwmutexR

Sysmon can actually get the RW lock execLock while holding the sysmon
lock (if no M is available), so there is an edge from lockRankSysmon to
lockRankRwmutexR. The stack trace is sysmon() [gets sched.sysmonlock] ->
startm() -> newm() -> newm1() -> execLock.runlock() [gets
execLock.rLock]

Change-Id: I9658659ba3899afb5219114d66b989abd50540db
Reviewed-on: https://go-review.googlesource.com/c/go/+/265721
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
4 years agocmd/link: enable internal linking by default on darwin/arm64
Cherry Zhang [Sun, 25 Oct 2020 00:58:38 +0000 (20:58 -0400)]
cmd/link: enable internal linking by default on darwin/arm64

With previous CLs, internal linking without cgo should work well.
Enable it by default. And stop always requiring cgo.

Enable tests that were previously disabled due to the lack of
internal linking.

Updates #38485.

Change-Id: I45125b9c263fd21d6847aa6b14ecaea3a2989b29
Reviewed-on: https://go-review.googlesource.com/c/go/+/265121
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
4 years agoruntime: always enable async preemption on darwin/arm64
Cherry Zhang [Sat, 24 Oct 2020 20:34:12 +0000 (16:34 -0400)]
runtime: always enable async preemption on darwin/arm64

Now that we have the G register saved, we can enable asynchronous
preemption for pure Go programs on darwin/arm64.

Updates #38485, #36365.

Change-Id: Ic654fa4dce369efe289b38d59cf1a184b358fe9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/265120
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agoruntime: save/restore g unconditionally on darwin/arm64
Cherry Zhang [Sat, 24 Oct 2020 20:29:26 +0000 (16:29 -0400)]
runtime: save/restore g unconditionally on darwin/arm64

Now that we always have TLS set up, we can always save the G
register, regardless of whether cgo is used. This makes pure Go
programs signal-safe.

Updates #38485.

Change-Id: Icbc69acf0e2a5652fbcbbd074258a1a5efe87f1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/265119
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
4 years agoruntime: set up TLS without cgo on darwin/arm64
Cherry Zhang [Sat, 24 Oct 2020 17:14:36 +0000 (13:14 -0400)]
runtime: set up TLS without cgo on darwin/arm64

Currently, on darwin/arm64 we set up TLS using cgo. TLS is not
set for pure Go programs. As we use libc for syscalls on darwin,
we need to save the G register before the libc call. Otherwise it
is not signal-safe, as a signal may land during the execution of
a libc function, where the G register may be clobbered.

This CL initializes TLS in Go, by calling the pthread functions
directly without cgo. This makes it possible to save the G
register to TLS in pure Go programs (done in a later CL).

Inspired by Elias's CL 209197. Write the logic in Go instead of
assembly.

Updates #38485, #35853.

Change-Id: I257ba2a411ad387b2f4d50d10129d37fec7a226e
Reviewed-on: https://go-review.googlesource.com/c/go/+/265118
Trust: Cherry Zhang <cherryyz@google.com>
Trust: Elias Naur <mail@eliasnaur.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agocmd/go/internal/modfetch: update expected tags for TestCodeRepoVersions
Tobias Klauser [Wed, 28 Oct 2020 11:07:31 +0000 (12:07 +0100)]
cmd/go/internal/modfetch: update expected tags for TestCodeRepoVersions

Updates #28856

Change-Id: I89c564cefd7c5777904bc00c74617dab693373bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/265819
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agocmd/compile,cmd/internal/obj/riscv,runtime: use Duff's devices on riscv64
Michał Derkacz [Sat, 13 Jun 2020 22:06:24 +0000 (00:06 +0200)]
cmd/compile,cmd/internal/obj/riscv,runtime: use Duff's devices on riscv64

Implement runtime.duffzero and runtime.duffcopy for riscv64.
Use obj.ADUFFZERO/obj.ADUFFCOPY for medium size, word aligned
zeroing/moving.

Change-Id: I42ec622055630c94cb77e286d8d33dbe7c9f846c
Reviewed-on: https://go-review.googlesource.com/c/go/+/237797
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
4 years agocmd/compile: optimize generated struct/array equality code
Keith Randall [Wed, 16 Sep 2020 17:13:37 +0000 (10:13 -0700)]
cmd/compile: optimize generated struct/array equality code

Use a standard "not-equal" label that we can jump to when we
detect that the arguments are not equal. This prevents the
recombination that was noticed in #39428.

Fixes #39428

Change-Id: Ib7c6b3539f4f6046043fd7148f940fcdcab70427
Reviewed-on: https://go-review.googlesource.com/c/go/+/255317
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
4 years agoruntime: move TestNeedmDeadlock to crash_cgo_test.go
Ian Lance Taylor [Wed, 28 Oct 2020 04:05:13 +0000 (21:05 -0700)]
runtime: move TestNeedmDeadlock to crash_cgo_test.go

It requires cgo. Also, skip the test on windows and plan9.

For #42207

Change-Id: I8522773f93bc3f9826506a41a08b86a083262e31
Reviewed-on: https://go-review.googlesource.com/c/go/+/265778
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
4 years agocmd/compile: make sure field offset is aligned for structure splitting
David Chase [Mon, 26 Oct 2020 23:06:39 +0000 (19:06 -0400)]
cmd/compile: make sure field offset is aligned for structure splitting

Always use the aligned form -- the old code sometimes could
at least nominally use a misaligned field in an SSA-able struct,
even if not actually.

Fixes #42181.

Change-Id: Ibdce0985f9349da70921a37423054b85ee4200d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/265277
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: David Chase <drchase@google.com>

4 years agocmd/link: remove all constants of elf
Meng Zhuo [Tue, 27 Oct 2020 06:18:13 +0000 (14:18 +0800)]
cmd/link: remove all constants of elf

Use debug/elf instead.

Related:
CL 252478
CL 265317

Change-Id: If63b0458d9a6e825b40616bfb7a5a2c2e32402b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/265318
Trust: Meng Zhuo <mzh@golangcn.org>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agoruntime: block signals in needm before allocating M
Ian Lance Taylor [Tue, 27 Oct 2020 23:09:40 +0000 (16:09 -0700)]
runtime: block signals in needm before allocating M

Otherwise, if a signal occurs just after we allocated the M,
we can deadlock if the signal handler needs to allocate an M
itself.

Fixes #42207

Change-Id: I76f44547f419e8b1c14cbf49bf602c6e645d8c14
Reviewed-on: https://go-review.googlesource.com/c/go/+/265759
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
4 years agocmd/go: add -include to cgo whitelist
Dave Pifke [Thu, 2 Jul 2020 00:48:37 +0000 (00:48 +0000)]
cmd/go: add -include to cgo whitelist

Fixes #39988.

Change-Id: Ia6f5b73e6508f27e3badbcbd29dbeadffd55a932
Reviewed-on: https://go-review.googlesource.com/c/go/+/240739
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agoruntime: handle signal 34 for musl setgid
George Tsilias [Thu, 4 Jun 2020 20:11:56 +0000 (23:11 +0300)]
runtime: handle signal 34 for musl setgid

It has been observed that setgid hangs when using cgo with musl.
This fix ensures that signal 34 gets handled in an appropriate way,
like signal 33 when using glibc.

Fixes #39343

Change-Id: I89565663e2c361f62cbccfe80aaedf290bd58d57
Reviewed-on: https://go-review.googlesource.com/c/go/+/236518
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agocmd/dist: use GOOS=ios for iOS simulator test
Cherry Zhang [Sun, 25 Oct 2020 02:02:50 +0000 (22:02 -0400)]
cmd/dist: use GOOS=ios for iOS simulator test

Updates #42100.

Change-Id: Ib59432bea99a9dd6e25fbd759b31e423dc250f42
Reviewed-on: https://go-review.googlesource.com/c/go/+/265197
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agotime: fix LoadLocationFromTZData with slim tzdata
Christopher Hlubek [Mon, 26 Oct 2020 12:44:44 +0000 (13:44 +0100)]
time: fix LoadLocationFromTZData with slim tzdata

The extend information of a time zone file with last transition < now
could result in a wrong cached zone because it used the zone of the
last transition.

This could lead to wrong zones in systems with slim zoneinfo.

Fixes #42216

Change-Id: I7c57c35b5cfa58482ac7925b5d86618c52f5444d
Reviewed-on: https://go-review.googlesource.com/c/go/+/264939
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agocrypto/tls: document the ClientAuthType consts
Roland Shoemaker [Wed, 21 Oct 2020 17:59:22 +0000 (10:59 -0700)]
crypto/tls: document the ClientAuthType consts

Fixes #34023

Change-Id: Ib7552a8873a79a91e8d971f906c6d7283da7a80c
Reviewed-on: https://go-review.googlesource.com/c/go/+/264027
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
4 years agoruntime: don't always adjust timers
Ian Lance Taylor [Wed, 30 Sep 2020 00:01:33 +0000 (17:01 -0700)]
runtime: don't always adjust timers

Some programs have a lot of timers that they adjust both forward and
backward in time. This can cause a large number of timerModifiedEarlier
timers. In practice these timers are used for I/O deadlines and are
rarely reached. The effect is that the runtime spends a lot of time
in adjusttimers making sure that there are no timerModifiedEarlier
timers, but the effort is wasted because none of the adjusted timers
are near the top of the timer heap anyhow.

Avoid much of this extra work by keeping track of the earliest known
timerModifiedEarlier timer. This lets us skip adjusttimers if we know
that none of the timers will be ready to run anyhow. We will still
eventually run it, when we reach the deadline of the earliest known
timerModifiedEarlier, although in practice that timer has likely
been removed. When we do run adjusttimers, we will reset all of the
timerModifiedEarlier timers, and clear our notion of when we need
to run adjusttimers again.

This effect should be to significantly reduce the number of times we
walk through the timer list in adjusttimers.

Fixes #41699

Change-Id: I38eb2be611fb34e3017bb33d0a9ed40d75fb414f
Reviewed-on: https://go-review.googlesource.com/c/go/+/258303
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
4 years agocmd/compile: print pointers to go:notinheap types without converting to unsafe.Pointer
Keith Randall [Tue, 27 Oct 2020 21:15:00 +0000 (14:15 -0700)]
cmd/compile: print pointers to go:notinheap types without converting to unsafe.Pointer

Pretty minor concern, but after auditing the compiler/runtime for
conversions from pointers to go:notinheap types to unsafe.Pointer,
this is the only remaining one I found.

Update #42076

Change-Id: I81d5b893c9ada2fc19a51c2559262f2e9ff71c35
Reviewed-on: https://go-review.googlesource.com/c/go/+/265757
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
4 years agocmd/compile, runtime: store pointers to go:notinheap types indirectly
Keith Randall [Thu, 22 Oct 2020 23:37:19 +0000 (16:37 -0700)]
cmd/compile, runtime: store pointers to go:notinheap types indirectly

pointers to go:notinheap types should be treated as scalars. That
means they shouldn't be stored directly in interfaces, or directly
in reflect.Value.ptr.

Also be sure to use uintpr to compare such pointers in reflect.DeepEqual.

Fixes #42076

Change-Id: I53735f6d434e9c3108d4940bd1bae14c61ef2a74
Reviewed-on: https://go-review.googlesource.com/c/go/+/264480
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agocmd/compile: fix storeType to handle pointers to go:notinheap types
Keith Randall [Thu, 22 Oct 2020 20:11:16 +0000 (13:11 -0700)]
cmd/compile: fix storeType to handle pointers to go:notinheap types

storeType splits compound stores up into a scalar parts and a pointer parts.
The scalar part happens unconditionally, and the pointer part happens
under the guard of a write barrier check.

Types which are declared as pointers, but are represented as scalars because
they might have "bad" values, were not handled correctly here. They ended
up not getting stored in either set.

Fixes #42032

Change-Id: I46f6600075c0c370e640b807066247237f93c7ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/264300
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agogo/internal/gccgoimporter: support notinheap annotation
Ian Lance Taylor [Tue, 27 Oct 2020 19:59:54 +0000 (12:59 -0700)]
go/internal/gccgoimporter: support notinheap annotation

The gofrontend has started emitting a notinheap annotation for types
marked go:notinheap.

For #41761

Change-Id: Ic8f7ffc32dbfe98ec09b3d835957f1be8e6c1208
Reviewed-on: https://go-review.googlesource.com/c/go/+/265702
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
4 years agodoc/go1.16: document BuildID in 'go list -export'
Daniel Martí [Thu, 22 Oct 2020 15:30:26 +0000 (16:30 +0100)]
doc/go1.16: document BuildID in 'go list -export'

This corresponds to the feature in https://golang.org/cl/263542, since
this can be a noteworthy change to people writing tools to inspect Go
builds.

Also amend the wording to clarify that build IDs are for an entire
compiled package, not just their export data or object file.

Change-Id: I2eb295492807d5d2997a35e5e2371914cb3ad3a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/264158
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
4 years agocmd/compile: replace int32(b2i(x)) with b2i32(x) in rules
Alberto Donizetti [Tue, 27 Oct 2020 13:04:10 +0000 (14:04 +0100)]
cmd/compile: replace int32(b2i(x)) with b2i32(x) in rules

Change-Id: I7fbb0c1ead6e29a7445c8ab43f7050947597f3e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/265497
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
4 years agocmd/compile: delete isPowerOfTwo, switch to isPowerOfTwo64
Alberto Donizetti [Tue, 27 Oct 2020 10:03:21 +0000 (11:03 +0100)]
cmd/compile: delete isPowerOfTwo, switch to isPowerOfTwo64

rewrite.go has two identical functions isPowerOfTwo and
isPowerOfTwo64; the former has been there for a while, while the
latter was added together with isPowerOfTwo{8,16,32} for use in typed
rules.

This change deletes isPowerOfTwo and switch to using isPowerOfTwo64
everywhere.

Change-Id: If26c94565d2393fac6f0ba117ee7ee2fc915f7cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/265417
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
4 years agocmd/compile: clean up ValAndOff funcs after untyped aux removal
Alberto Donizetti [Tue, 27 Oct 2020 08:38:52 +0000 (09:38 +0100)]
cmd/compile: clean up ValAndOff funcs after untyped aux removal

Changes:
- makeValAndOff is deleted in favour of MakeValAndOff{32,64}
- canAdd is renamed to canAdd64 to uniform with existing canAdd32
- addOffset{32,64} is simplified by directly using MakeValAndOff{32,64}
- ValAndOff.Int64 is removed

Change-Id: Ic01db7fa31ddfe0aaaf1d1d77af823d48a7bee84
Reviewed-on: https://go-review.googlesource.com/c/go/+/265357
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>

4 years agocmd/compile: remove support for untyped ssa rules
Alberto Donizetti [Sat, 24 Oct 2020 15:27:52 +0000 (17:27 +0200)]
cmd/compile: remove support for untyped ssa rules

This change removes support in rulegen for untyped -> ssa rules.

Change-Id: I202018e191fc74f027243351bc8cf96145f2482c
Reviewed-on: https://go-review.googlesource.com/c/go/+/264679
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>

4 years agoruntime: add 2-byte and 8-byte sub-benchmarks for memmove load/store
Heisenberg [Tue, 8 Sep 2020 06:31:39 +0000 (14:31 +0800)]
runtime: add 2-byte and 8-byte sub-benchmarks for memmove load/store

Change-Id: I6389d7efe90836b6ece44d2e75053d1ad9f35d08
Reviewed-on: https://go-review.googlesource.com/c/go/+/253417
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Keith Randall <khr@golang.org>
4 years agonet/http: use exponential backoff for polling in Server.Shutdown
Dan Peterson [Thu, 22 Oct 2020 22:25:56 +0000 (19:25 -0300)]
net/http: use exponential backoff for polling in Server.Shutdown

Instead of always polling 500ms, start with an interval of 1ms and
exponentially back off to at most 500ms. 10% jitter is added to each
interval.

This makes Shutdown more responsive when connections and listeners
close quickly.

Also removes the need for the polling interval to be changed in tests
since if tests' connections and listeners close quickly Shutdown will
also return quickly.

Fixes #42156

Change-Id: I5e59844a2980c09adebff57ae8b58817965e6db4
Reviewed-on: https://go-review.googlesource.com/c/go/+/264479
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Bryan C. Mills <bcmills@google.com>

4 years agocmd/compile: combine more 32 bit shift and mask operations on ppc64
Paul E. Murphy [Fri, 23 Oct 2020 17:12:34 +0000 (12:12 -0500)]
cmd/compile: combine more 32 bit shift and mask operations on ppc64

Combine (AND m (SRWconst x)) or (SRWconst (AND m x)) when mask m is
and the shift value produce constant which can be encoded into an
RLWINM instruction.

Combine (CLRLSLDI (SRWconst x)) if the combining of the underling rotate
masks produces a constant which can be encoded into RLWINM.

Likewise for (SLDconst (SRWconst x)) and (CLRLSDI (RLWINM x)).

Combine rotate word + and operations which can be encoded as a single
RLWINM/RLWNM instruction.

The most notable performance improvements arise from the crypto
benchmarks below (GOARCH=power8 on a ppc64le/linux):

pkg:golang.org/x/crypto/blowfish goos:linux goarch:ppc64le
ExpandKeyWithSalt                               52.2µs ± 0%    47.5µs ± 0%  -8.88%
ExpandKey                                       44.4µs ± 0%    40.3µs ± 0%  -9.15%

pkg:golang.org/x/crypto/ssh/internal/bcrypt_pbkdf goos:linux goarch:ppc64le
Key                                             57.6ms ± 0%    52.3ms ± 0%  -9.13%

pkg:golang.org/x/crypto/bcrypt goos:linux goarch:ppc64le
Equal                                           90.9ms ± 0%    82.6ms ± 0%  -9.13%
DefaultCost                                     91.0ms ± 0%    82.7ms ± 0%  -9.12%

Change-Id: I59a0ca29face38f4ab46e37124c32906f216c4ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/260798
Run-TryBot: Carlos Eduardo Seo <carlos.seo@linaro.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Carlos Eduardo Seo <carlos.seo@linaro.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>

4 years agocmd/compile: remove isLiteral
Cuong Manh Le [Tue, 27 Oct 2020 10:08:57 +0000 (17:08 +0700)]
cmd/compile: remove isLiteral

It has duplicated logic with "n.isGoConst".

Passes toolstash-check.

Change-Id: I5bf871ef81c7188ca09dae29c7ff55b3a254d972
Reviewed-on: https://go-review.googlesource.com/c/go/+/265437
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
4 years agomisc/cgo/test: test C.enum_*
HowJMay [Tue, 27 Oct 2020 17:03:48 +0000 (17:03 +0000)]
misc/cgo/test: test C.enum_*

Allocate a C enum object, and test if it can be assigned a value
successfully.

For #39537

Change-Id: I7b5482112486440b9d99f2ee4051328d87f45dca
GitHub-Last-Rev: 81890f40acc5589563ec1206fe119873fb46dc1b
GitHub-Pull-Request: golang/go#39977
Reviewed-on: https://go-review.googlesource.com/c/go/+/240697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

4 years agoruntime: reduce timer latency
Chris Hines [Fri, 1 May 2020 21:04:36 +0000 (17:04 -0400)]
runtime: reduce timer latency

Change the scheduler to treat expired timers with the same approach it
uses to steal runnable G's.

Previously the scheduler ignored timers on P's not marked for
preemption. That had the downside that any G's waiting on those expired
timers starved until the G running on their P completed or was
preempted. That could take as long as 20ms if sysmon was in a 10ms
wake up cycle.

In addition, a spinning P that ignored an expired timer and found no
other work would stop despite there being available work, missing the
opportunity for greater parallelism.

With this change the scheduler no longer ignores timers on
non-preemptable P's or relies on sysmon as a backstop to start threads
when timers expire. Instead it wakes an idle P, if needed, when
creating a new timer because it cannot predict if the current P will
have a scheduling opportunity before the new timer expires. The P it
wakes will determine how long to sleep and block on the netpoller for
the required time, potentially stealing the new timer when it wakes.

This change also eliminates a race between a spinning P transitioning
to idle concurrently with timer creation using the same pattern used
for submission of new goroutines in the same window.

Benchmark analysis:

CL 232199, which was included in Go 1.15 improved timer latency over Go
1.14 by allowing P's to steal timers from P's not marked for preemption.
The benchmarks added in this CL measure that improvement in the
ParallelTimerLatency benchmark seen below. However, Go 1.15 still relies
on sysmon to notice expired timers in some situations and sysmon can
sleep for up to 10ms before waking to check timers. This CL fixes that
shortcoming with modest regression on other benchmarks.

name \ avg-late-ns                                        go14.time.bench  go15.time.bench  fix.time.bench
ParallelTimerLatency-8                                         17.3M ± 3%        7.9M ± 0%       0.2M ± 3%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=1-8        53.4k ±23%       50.7k ±31%     252.4k ± 9%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=2-8         204k ±14%         90k ±58%       188k ±12%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=3-8        1.17M ± 0%       0.11M ± 5%      0.11M ± 2%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=4-8        1.81M ±44%       0.10M ± 4%      0.10M ± 2%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=5-8        2.28M ±66%       0.09M ±13%      0.08M ±21%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=6-8        2.84M ±85%       0.07M ±15%      0.07M ±18%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=7-8        2.13M ±27%       0.06M ± 4%      0.06M ± 9%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=8-8        2.63M ± 6%       0.06M ±11%      0.06M ± 9%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=9-8        3.32M ±17%       0.06M ±16%      0.07M ±14%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=10-8       8.46M ±20%       4.37M ±21%      5.03M ±23%
StaggeredTickerLatency/work-dur=2ms/tickers-per-P=1-8          1.02M ± 1%       0.20M ± 2%      0.20M ± 2%

name \ max-late-ns                                        go14.time.bench  go15.time.bench  fix.time.bench
ParallelTimerLatency-8                                         18.3M ± 1%        8.2M ± 0%       0.5M ±12%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=1-8         141k ±19%        127k ±19%      1129k ± 3%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=2-8        2.78M ± 4%       1.23M ±15%      1.26M ± 5%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=3-8        6.05M ± 5%       0.67M ±56%      0.81M ±33%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=4-8        7.93M ±20%       0.71M ±46%      0.76M ±41%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=5-8        9.41M ±30%       0.92M ±23%      0.81M ±44%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=6-8        10.8M ±42%        0.8M ±41%       0.8M ±30%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=7-8        9.62M ±24%       0.77M ±38%      0.88M ±27%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=8-8        10.6M ±10%        0.8M ±32%       0.7M ±27%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=9-8        11.9M ±36%        0.6M ±46%       0.8M ±38%
StaggeredTickerLatency/work-dur=300µs/tickers-per-P=10-8       36.8M ±21%       24.7M ±21%      27.5M ±16%
StaggeredTickerLatency/work-dur=2ms/tickers-per-P=1-8          2.12M ± 2%       1.02M ±11%      1.03M ± 7%

Other time benchmarks:
name \ time/op          go14.time.bench  go15.time.bench  fix.time.bench
AfterFunc-8                  137µs ± 4%       123µs ± 4%      131µs ± 2%
After-8                      212µs ± 3%       195µs ± 4%      204µs ± 7%
Stop-8                       165µs ± 6%       156µs ± 2%      151µs ±12%
SimultaneousAfterFunc-8      260µs ± 3%       248µs ± 3%      284µs ± 2%
StartStop-8                 65.8µs ± 9%      64.4µs ± 7%     67.3µs ±15%
Reset-8                     13.6µs ± 2%       9.6µs ± 2%      9.1µs ± 4%
Sleep-8                      307µs ± 4%       306µs ± 3%      320µs ± 2%
Ticker-8                    53.0µs ± 5%      54.5µs ± 5%     57.0µs ±11%
TickerReset-8                                9.24µs ± 2%     9.51µs ± 3%
TickerResetNaive-8                            149µs ± 5%      145µs ± 5%

Fixes #38860
Updates #25471
Updates #27707

Change-Id: If52680509b0f3b66dbd1d0c13fa574bd2d0bbd57
Reviewed-on: https://go-review.googlesource.com/c/go/+/232298
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Trust: Ian Lance Taylor <iant@golang.org>

4 years agocmd/dist: fix build failure of misc/cgo/test on arm64
xd [Wed, 14 Oct 2020 18:02:49 +0000 (11:02 -0700)]
cmd/dist: fix build failure of misc/cgo/test on arm64

misc/cgo/test fails in 'dist test' on arm64 if the C compiler is of GCC-9.4 or
above and its 'outline atomics' feature is enabled, since the internal linking
hasn't yet supported "__attribute__((constructor))" and also mis-handles hidden
visibility.

This change addresses the problem by skipping the internal linking cases of
misc/cgo/test on linux/arm64. It fixes 'dist test' failure only, user is expected to
pass a GCC option '-mno-outline-atomics' via CGO_CFLAGS if running into the same
problem when building cgo programs using internal linking.

Updates #39466

Change-Id: I57f9e85fca881e5fd2dae6c1b4446bce9e0c1975
Reviewed-on: https://go-review.googlesource.com/c/go/+/262357
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>

4 years agocmd/go/internal/fsys: add Glob
Russ Cox [Fri, 23 Oct 2020 01:33:02 +0000 (21:33 -0400)]
cmd/go/internal/fsys: add Glob

Glob is needed for //go:embed processing.

Also change TestReadDir to be deterministic
and print more output about failures.

Change-Id: Ie22a9c5b32bda753579ff98cec1d28e3244c4e06
Reviewed-on: https://go-review.googlesource.com/c/go/+/264538
Trust: Russ Cox <rsc@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
4 years agomime: look up mime types on Arch Linux
Andy Williams [Fri, 5 Jun 2020 10:18:47 +0000 (10:18 +0000)]
mime: look up mime types on Arch Linux

Some systems use "httpd" directory structure instead of "apache"

Change-Id: I77600baf356f0c0c3359b331505b0426112daebb
GitHub-Last-Rev: c64766f88cc28899d3d387e3eebfa4fcc3eef808
GitHub-Pull-Request: golang/go#39416
Reviewed-on: https://go-review.googlesource.com/c/go/+/236677
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
4 years agocmd/go/internal/imports: make Tags and AnyTags safe for concurrent use
Bryan C. Mills [Tue, 27 Oct 2020 07:09:26 +0000 (03:09 -0400)]
cmd/go/internal/imports: make Tags and AnyTags safe for concurrent use

AnyTags turned up as a data race while running 'go test -race cmd/go'.
I'm not sure how long the race has been present.

==================
WARNING: DATA RACE
Read at 0x000001141ec0 by goroutine 8:
  cmd/go/internal/imports.AnyTags()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/imports/tags.go:45 +0x10e
  cmd/go/internal/modload.QueryPattern.func2()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:539 +0x11d
  cmd/go/internal/modload.QueryPattern.func4.1()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:607 +0x3db
  cmd/go/internal/modload.queryPrefixModules.func1()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:677 +0xa7

Previous write at 0x000001141ec0 by goroutine 7:
  cmd/go/internal/imports.AnyTags()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/imports/tags.go:46 +0x26b
  cmd/go/internal/modload.QueryPattern.func2()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:539 +0x11d
  cmd/go/internal/modload.QueryPattern.func4.1()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:607 +0x3db
  cmd/go/internal/modload.queryPrefixModules.func1()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:677 +0xa7

Goroutine 8 (running) created at:
  cmd/go/internal/modload.queryPrefixModules()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:676 +0x284
  cmd/go/internal/modload.QueryPattern.func4()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:624 +0x2e4
  cmd/go/internal/modfetch.TryProxies()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modfetch/proxy.go:220 +0x107
  cmd/go/internal/modload.QueryPattern()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:590 +0x69e
  cmd/go/internal/work.installOutsideModule()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/work/build.go:744 +0x4b0
  cmd/go/internal/work.runInstall()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/work/build.go:556 +0x217
  main.main()
      /usr/local/google/home/bcmills/go/src/cmd/go/main.go:194 +0xb94

Goroutine 7 (finished) created at:
  cmd/go/internal/modload.queryPrefixModules()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:676 +0x284
  cmd/go/internal/modload.QueryPattern.func4()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:624 +0x2e4
  cmd/go/internal/modfetch.TryProxies()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modfetch/proxy.go:220 +0x107
  cmd/go/internal/modload.QueryPattern()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/modload/query.go:590 +0x69e
  cmd/go/internal/work.installOutsideModule()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/work/build.go:744 +0x4b0
  cmd/go/internal/work.runInstall()
      /usr/local/google/home/bcmills/go/src/cmd/go/internal/work/build.go:556 +0x217
  main.main()
      /usr/local/google/home/bcmills/go/src/cmd/go/main.go:194 +0xb94
==================

Change-Id: Id394978fd6ea0c30614caf8f90ee4f8e2d272843
Reviewed-on: https://go-review.googlesource.com/c/go/+/265278
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
4 years agodatabase/sql: fix comment on DB.stop()
Ayzat Sadykov [Sat, 24 Oct 2020 22:11:51 +0000 (22:11 +0000)]
database/sql: fix comment on DB.stop()

Previously, 2 goroutines were created in OpenDB and a comment in the DB.close() field indicated that they were canceled. Later, session Resetter () was removed, but the comment remained the same. This commit just fixes this message

Change-Id: Ie81026f51d7770e9cf8004818154021f626fb2e8
GitHub-Last-Rev: 38b338a0d1cd713d71fa547aa842d395e6d75484
GitHub-Pull-Request: golang/go#42191
Reviewed-on: https://go-review.googlesource.com/c/go/+/264838
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>

4 years agocmd/go: fix bug introduced in CL 264537
Russ Cox [Tue, 27 Oct 2020 14:41:25 +0000 (10:41 -0400)]
cmd/go: fix bug introduced in CL 264537

Shadowing bug noted after submit by Tom Thorogood.

Change-Id: I5f40cc3863dcd7dba5469f8530e9d0460e7c3e7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/265537
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agocmd/compile: eliminate unnecessary sign/zero extension for riscv64
Joel Sing [Sat, 24 Oct 2020 14:34:17 +0000 (01:34 +1100)]
cmd/compile: eliminate unnecessary sign/zero extension for riscv64

Add additional rules to eliminate unnecessary sign/zero extension for riscv64.
Also where possible, replace an extension following a load with a different typed
load. This removes almost another 8,000 instructions from the go binary.

Of particular note, change Eq16/Eq8/Neq16/Neq8 to zero extend each value before
subtraction, rather than zero extending after subtraction. While this appears to
double the number of zero extensions, it often lets us completely eliminate them
as the load can already be performed in a properly typed manner.

As an example, prior to this change runtime.memequal16 was:

0000000000013028 <runtime.memequal16>:
   13028:       00813183                ld      gp,8(sp)
   1302c:       00019183                lh      gp,0(gp)
   13030:       01013283                ld      t0,16(sp)
   13034:       00029283                lh      t0,0(t0)
   13038:       405181b3                sub     gp,gp,t0
   1303c:       03019193                slli    gp,gp,0x30
   13040:       0301d193                srli    gp,gp,0x30
   13044:       0011b193                seqz    gp,gp
   13048:       00310c23                sb      gp,24(sp)
   1304c:       00008067                ret

Whereas it now becomes:

0000000000012fa8 <runtime.memequal16>:
   12fa8:       00813183                ld      gp,8(sp)
   12fac:       0001d183                lhu     gp,0(gp)
   12fb0:       01013283                ld      t0,16(sp)
   12fb4:       0002d283                lhu     t0,0(t0)
   12fb8:       405181b3                sub     gp,gp,t0
   12fbc:       0011b193                seqz    gp,gp
   12fc0:       00310c23                sb      gp,24(sp)
   12fc4:       00008067                ret

Change-Id: I16321feb18381241cab121c0097a126104c56c2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/264659
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
4 years agocrypto/rand: generate random numbers using RtlGenRandom on Windows
Jason A. Donenfeld [Thu, 5 Dec 2019 17:48:21 +0000 (18:48 +0100)]
crypto/rand: generate random numbers using RtlGenRandom on Windows

CryptGenRandom appears to be unfavorable these days, whereas the classic
RtlGenRandom is still going strong.

This commit also moves the warnBlocked function into rand_unix, rather
than rand, because it's now only used on unix.

Fixes #33542

Change-Id: I5c02a5917572f54079d627972401efb6e1ce4057
Reviewed-on: https://go-review.googlesource.com/c/go/+/210057
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>

4 years agocmd/compile: use MOV pseudo-instructions for sign/zero extension
Joel Sing [Sat, 24 Oct 2020 13:32:23 +0000 (00:32 +1100)]
cmd/compile: use MOV pseudo-instructions for sign/zero extension

Rather than handling sign and zero extension via rules, defer to the assembler
and use MOV pseudo-instructions. The instruction can also be omitted where the
type and size is already correct. This change results in more than 6,000
instructions being removed from the go binary (in part due to omitted
instructions, in part due to MOVBU having a more efficient implementation in
the assembler than what is used in the current ZeroExt8to{16,32,64} rules).

This will also allow for further rewriting to remove redundant sign/zero
extension.

Change-Id: I05e42fd9f09f40a69948be7de772cce8946c8744
Reviewed-on: https://go-review.googlesource.com/c/go/+/264658
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
4 years agocmd/go/internal/modload: embed PackageOpts in loaderParams
Bryan C. Mills [Tue, 13 Oct 2020 14:58:13 +0000 (10:58 -0400)]
cmd/go/internal/modload: embed PackageOpts in loaderParams

Instead of duplicating PackageOpts fields in the loaderParams struct,
embed the PackageOpts directly. Many of the fields are duplicated, and
further fields that would also be duplicated will be added in
subsequent changes.

For #36460

Change-Id: I3b0770d162e901d23ec1643183eb07c413d51e0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/263138
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
4 years agoRevert "cmd/link: remove all constants of elf"
Meng Zhuo [Tue, 27 Oct 2020 05:13:24 +0000 (05:13 +0000)]
Revert "cmd/link: remove all constants of elf"

This reverts CL 252478.

Reason for revert: debug/Elfhdr has no Flags fields, some other CLs has removed it.

Change-Id: Ie199ac29f382c56aaf37a2e8338f2dafe6e79297
Reviewed-on: https://go-review.googlesource.com/c/go/+/265317
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Meng Zhuo <mzh@golangcn.org>

4 years agocmd/link: remove all constants of elf
Meng Zhuo [Wed, 23 Sep 2020 16:22:05 +0000 (00:22 +0800)]
cmd/link: remove all constants of elf

Use debug/elf instead.

Change-Id: Ia6580648b6440e4a352f5c5ed59ac4d1c95e0175
Reviewed-on: https://go-review.googlesource.com/c/go/+/252478
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
4 years agocmd/link,cmd/internal/obj/riscv: add TLS support for linux/riscv64
Joel Sing [Tue, 19 May 2020 08:55:10 +0000 (18:55 +1000)]
cmd/link,cmd/internal/obj/riscv: add TLS support for linux/riscv64

Add support for Thread Local Storage (TLS) for linux/riscv64 with external
linking, using the initial-exec model.

Update #36641

Change-Id: I3106ef9a29cde73215830b00deff43dbec1c76e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/263478
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
4 years agocmd/compile,cmd/internal/sys: enable additional build modes on linux/riscv64
Joel Sing [Tue, 2 Jun 2020 13:34:06 +0000 (23:34 +1000)]
cmd/compile,cmd/internal/sys: enable additional build modes on linux/riscv64

Enable c-archive, c-shared, shared and pie build modes for linux/riscv64.

Change-Id: I15a8a51b84dbbb82a5b6592aec84a7f09f0cc37f
Reviewed-on: https://go-review.googlesource.com/c/go/+/263457
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agocmd/compile: defer lowering OANDNOT until SSA
Cuong Manh Le [Sat, 24 Oct 2020 03:40:09 +0000 (10:40 +0700)]
cmd/compile: defer lowering OANDNOT until SSA

Currently, "x &^ y" gets rewriten into "x & ^y" during walk. It adds
unnecessary complexity to other parts, which must aware about this.

Instead, we can just implement "&^" in the conversion to SSA, so "&^"
can be handled like other binary operators.

However, this CL does not pass toolstash-check. It seems that implements
"&^" in the conversion to SSA causes registers allocation change.

With the parent:

obj: 00212 (.../src/runtime/complex.go:47)    MOVQ    X0, AX
obj: 00213 (.../src/runtime/complex.go:47)    BTRQ    $63, AX
obj: 00214 (.../src/runtime/complex.go:47)    MOVQ    "".n(SP), CX
obj: 00215 (.../src/runtime/complex.go:47)    MOVQ    $-9223372036854775808, DX
obj: 00216 (.../src/runtime/complex.go:47)    ANDQ    DX, CX
obj: 00217 (.../src/runtime/complex.go:47)    ORQ AX, CX

With this CL:

obj: 00212 (.../src/runtime/complex.go:47)    MOVQ    X0, AX
obj: 00213 (.../src/runtime/complex.go:47)    BTRQ    $63, AX
obj: 00214 (.../src/runtime/complex.go:47)    MOVQ    $-9223372036854775808, CX
obj: 00215 (.../src/runtime/complex.go:47)    MOVQ    "".n(SP), DX
obj: 00216 (.../src/runtime/complex.go:47)    ANDQ    CX, DX
obj: 00217 (.../src/runtime/complex.go:47)    ORQ AX, DX

Change-Id: I80acf8496a91be4804fb7ef3df04c19baae2754c
Reviewed-on: https://go-review.googlesource.com/c/go/+/264660
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
4 years agotest: add index bounds check elided with "&^"
Cuong Manh Le [Mon, 26 Oct 2020 04:28:02 +0000 (11:28 +0700)]
test: add index bounds check elided with "&^"

For follow up CL, which will defer lowering OANDNOT until SSA.

Change-Id: I5a988d0b8f0ae664580f08b123811b2a31ef55c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/265040
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
4 years agoruntime: implement addrRanges.findSucc with a binary search
Michael Anthony Knyszek [Tue, 14 Jul 2020 21:45:16 +0000 (21:45 +0000)]
runtime: implement addrRanges.findSucc with a binary search

This change modifies addrRanges.findSucc to more efficiently find the
successor range in an addrRanges by using a binary search to narrow down
large addrRanges and iterate over no more than 8 addrRanges.

This change makes the runtime more robust against systems that may
aggressively randomize the address space mappings it gives the runtime
(e.g. Fuchsia).

For #40191.

Change-Id: If529df2abd2edb1b1496d8690ddd284ecd7138c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/242679
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agocmd/compile: port last ARM rules to typed
Alberto Donizetti [Sat, 24 Oct 2020 17:07:14 +0000 (19:07 +0200)]
cmd/compile: port last ARM rules to typed

Passes

  GOARCH=arm gotip build -toolexec 'toolstash -cmp' -a std

Change-Id: I4a1cace82c5d957774ea20572406af276f02bf97
Reviewed-on: https://go-review.googlesource.com/c/go/+/264680
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
4 years agoruntime: implement dumpmemstats in terms of readmemstats_m
Michael Anthony Knyszek [Thu, 17 Sep 2020 21:19:28 +0000 (21:19 +0000)]
runtime: implement dumpmemstats in terms of readmemstats_m

Since MemStats is now populated directly and some values are derived,
avoid duplicating the logic by instead populating the heap dump directly
from MemStats (external version) instead of memstats (runtime internal
version).

Change-Id: I0bec96bfa02d2ffd1b56475779c124a760e64238
Reviewed-on: https://go-review.googlesource.com/c/go/+/255817
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime,runtime/metrics: export goroutine count as a metric
Michael Anthony Knyszek [Fri, 7 Aug 2020 16:37:29 +0000 (16:37 +0000)]
runtime,runtime/metrics: export goroutine count as a metric

For #37112.

Change-Id: I994dfe848605b95ef6aec24f53869e929247e987
Reviewed-on: https://go-review.googlesource.com/c/go/+/247049
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime,runtime/metrics: add metric for distribution of GC pauses
Michael Anthony Knyszek [Thu, 6 Aug 2020 21:59:13 +0000 (21:59 +0000)]
runtime,runtime/metrics: add metric for distribution of GC pauses

For #37112.

Change-Id: Ibb0425c9c582ae3da3b2662d5bbe830d7df9079c
Reviewed-on: https://go-review.googlesource.com/c/go/+/247047
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: add timeHistogram type
Michael Anthony Knyszek [Thu, 6 Aug 2020 20:36:49 +0000 (20:36 +0000)]
runtime: add timeHistogram type

This change adds a concurrent HDR time histogram to the runtime with
tests. It also adds a function to generate boundaries for use by the
metrics package.

For #37112.

Change-Id: Ifbef8ddce8e3a965a0dcd58ccd4915c282ae2098
Reviewed-on: https://go-review.googlesource.com/c/go/+/247046
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime,runtime/metrics: add object size distribution metrics
Michael Anthony Knyszek [Thu, 6 Aug 2020 19:04:46 +0000 (19:04 +0000)]
runtime,runtime/metrics: add object size distribution metrics

This change adds metrics for the distribution of objects allocated and
freed by size, mirroring MemStats' BySize field.

For #37112.

Change-Id: Ibaf1812da93598b37265ec97abc6669c1a5efcbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/247045
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agocmd/go,cmd/compile,sync: remove special import case in cmd/go
Austin Clements [Mon, 26 Oct 2020 18:32:13 +0000 (14:32 -0400)]
cmd/go,cmd/compile,sync: remove special import case in cmd/go

CL 253748 introduced a special case in cmd/go to allow sync to import
runtime/internal/atomic. Besides introducing unnecessary complexity
into cmd/go, this breaks other packages (like gopls) that understand
how imports work, but don't understand this special case.

Fix this by using the more standard linkname-based approach to pull
the necessary functions from runtime/internal/atomic into sync. Since
these are compiler intrinsics, we also have to tell the compiler that
the linknamed symbols are intrinsics to get this optimization in sync.

Fixes #42196.

Change-Id: I1f91498c255c91583950886a89c3c9adc39a32f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/265124
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Paul Murphy <murp@ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agoruntime: make sysMemStats' methods nosplit
Michael Anthony Knyszek [Mon, 26 Oct 2020 19:35:23 +0000 (19:35 +0000)]
runtime: make sysMemStats' methods nosplit

sysMemStats are updated early on in runtime initialization, so
triggering a stack growth would be bad. Mark them nosplit.

Thank you so much to cherryyz@google.com for finding this fix!

Fixes #42218.

Change-Id: Ic62db76e6a4f829355d7eaabed1727c51adfbd0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/265157
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

4 years agocmd/internal/obj/{arm,s390x}: make return jump print nicer
Cherry Zhang [Thu, 22 Oct 2020 01:29:43 +0000 (21:29 -0400)]
cmd/internal/obj/{arm,s390x}: make return jump print nicer

When a function with non-zero frame size makes a return jump
(RET target), it assembles to, conceptually,

MOV (SP), LR
ADD $framesize, SP
JMP target

We did not clear some fields in the first instruction's Prog.To,
causing it printed like (on ARM)

MOVW.P 4(R13), (R14)(R14)(REG)

Clear the fields to make it print nicer.

Change-Id: I180901aeea41f1ff287d7c6034a6d69005927744
Reviewed-on: https://go-review.googlesource.com/c/go/+/264343
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
4 years agoruntime,runtime/metrics: add heap goal and GC cycle metrics
Michael Anthony Knyszek [Thu, 6 Aug 2020 16:47:58 +0000 (16:47 +0000)]
runtime,runtime/metrics: add heap goal and GC cycle metrics

This change adds three new metrics: the heap goal, GC cycle count, and
forced GC count. These metrics are identical to their MemStats
counterparts.

For #37112.

Change-Id: I5a5e8dd550c0d646e5dcdbdf38274895e27cdd88
Reviewed-on: https://go-review.googlesource.com/c/go/+/247044
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime,runtime/metrics: add heap object count metric
Michael Anthony Knyszek [Thu, 6 Aug 2020 15:44:27 +0000 (15:44 +0000)]
runtime,runtime/metrics: add heap object count metric

For #37112.

Change-Id: Idd3dd5c84215ddd1ab05c2e76e848aa0a4d40fb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/247043
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: add readMetrics latency benchmark
Michael Anthony Knyszek [Wed, 5 Aug 2020 23:10:46 +0000 (23:10 +0000)]
runtime: add readMetrics latency benchmark

This change adds a new benchmark to the runtime tests for measuring the
latency of the new metrics implementation, based on the
ReadMemStats latency benchmark. readMetrics will have more metrics added
to it in the future, and this benchmark will serve as a way to measure
the cost of adding additional metrics.

Change-Id: Ib05e3ed4afa49a70863fc0c418eab35b72263e24
Reviewed-on: https://go-review.googlesource.com/c/go/+/247042
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime,runtime/metrics: add memory metrics
Michael Anthony Knyszek [Wed, 1 Jul 2020 16:02:42 +0000 (16:02 +0000)]
runtime,runtime/metrics: add memory metrics

This change adds support for a variety of runtime memory metrics and
contains the base implementation of Read for the runtime/metrics
package, which lives in the runtime.

It also adds testing infrastructure for the metrics package, and a bunch
of format and documentation tests.

For #37112.

Change-Id: I16a2c4781eeeb2de0abcb045c15105f1210e2d8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/247041
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>

4 years agoruntime: move malloc stats into consistentHeapStats
Michael Anthony Knyszek [Tue, 4 Aug 2020 17:29:03 +0000 (17:29 +0000)]
runtime: move malloc stats into consistentHeapStats

This change moves the mcache-local malloc stats into the
consistentHeapStats structure so the malloc stats can be managed
consistently with the memory stats. The one exception here is
tinyAllocs for which moving that into the global stats would incur
several atomic writes on the fast path. Microbenchmarks for just one CPU
core have shown a 50% loss in throughput. Since tiny allocation counnt
isn't exposed anyway and is always blindly added to both allocs and
frees, let that stay inconsistent and flush the tiny allocation count
every so often.

Change-Id: I2a4b75f209c0e659b9c0db081a3287bf227c10ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/247039
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: replace some memstats with consistent stats
Michael Anthony Knyszek [Mon, 3 Aug 2020 20:35:40 +0000 (20:35 +0000)]
runtime: replace some memstats with consistent stats

This change replaces stacks_inuse, gcWorkBufInUse and
gcProgPtrScalarBitsInUse with their corresponding consistent stats. It
also adds checks to make sure the rest of the sharded stats line up with
existing stats in updatememstats.

Change-Id: I17d0bd181aedb5c55e09c8dff18cef5b2a3a14e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/247038
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: add consistent heap statistics
Michael Anthony Knyszek [Mon, 3 Aug 2020 20:11:04 +0000 (20:11 +0000)]
runtime: add consistent heap statistics

This change adds a global set of heap statistics which are similar
to existing memory statistics. The purpose of these new statistics
is to be able to read them and get a consistent result without stopping
the world. The goal is to eventually replace as many of the existing
memstats statistics with the sharded ones as possible.

The consistent memory statistics use a tailor-made synchronization
mechanism to allow writers (allocators) to proceed with minimal
synchronization by using a sequence counter and a global generation
counter to determine which set of statistics to update. Readers
increment the global generation counter to effectively grab a snapshot
of the statistics, and then iterate over all Ps using the sequence
counter to ensure that they may safely read the snapshotted statistics.
To keep statistics fresh, the reader also has a responsibility to merge
sets of statistics.

These consistent statistics are computed, but otherwise unused for now.
Upcoming changes will integrate them with the rest of the codebase and
will begin to phase out existing statistics.

Change-Id: I637a11f2439e2049d7dccb8650c5d82500733ca5
Reviewed-on: https://go-review.googlesource.com/c/go/+/247037
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime/metrics: add package interface
Michael Anthony Knyszek [Tue, 14 Apr 2020 21:06:26 +0000 (21:06 +0000)]
runtime/metrics: add package interface

This change creates the runtime/metrics package and adds the initial
interface as laid out in the design document.

For #37112.

Change-Id: I202dcee08ab008dd63bf96f7a4162f5b5f813637
Reviewed-on: https://go-review.googlesource.com/c/go/+/247040
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: add helper for getting an mcache in allocation contexts
Michael Anthony Knyszek [Mon, 3 Aug 2020 20:08:25 +0000 (20:08 +0000)]
runtime: add helper for getting an mcache in allocation contexts

This change adds a function getMCache which returns the current P's
mcache if it's available, and otherwise tries to get mcache0 if we're
bootstrapping. This function will come in handy as we need to replicate
this behavior in multiple places in future changes.

Change-Id: I536073d6f6dc6c6390269e613ead9f8bcb6e7f98
Reviewed-on: https://go-review.googlesource.com/c/go/+/246976
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: remove memstats.heap_alloc
Michael Anthony Knyszek [Mon, 3 Aug 2020 19:31:23 +0000 (19:31 +0000)]
runtime: remove memstats.heap_alloc

memstats.heap_alloc is 100% a duplicate and unnecessary copy of
memstats.alloc which exists because MemStats used to be populated from
memstats via a memmove.

Change-Id: I995489f61be39786e573b8494a8ab6d4ea8bed9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/246975
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: remove memstats.heap_idle
Michael Anthony Knyszek [Mon, 3 Aug 2020 19:27:59 +0000 (19:27 +0000)]
runtime: remove memstats.heap_idle

This statistic is updated in many places but for MemStats may be
computed from existing statistics. Specifically by definition
heap_idle = heap_sys - heap_inuse since heap_sys is all memory allocated
from the OS for use in the heap minus memory used for non-heap purposes.
heap_idle is almost the same (since it explicitly includes memory that
*could* be used for non-heap purposes) but also doesn't include memory
that's actually used to hold heap objects.

Although it has some utility as a sanity check, it complicates
accounting and we want fewer, orthogonal statistics for upcoming metrics
changes, so just drop it.

Change-Id: I40af54a38e335f43249f6e218f35088bfd4380d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/246974
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: break down memstats.gc_sys
Michael Anthony Knyszek [Mon, 3 Aug 2020 19:23:30 +0000 (19:23 +0000)]
runtime: break down memstats.gc_sys

This change breaks apart gc_sys into three distinct pieces. Two of those
pieces are pieces which come from heap_sys since they're allocated from
the page heap. The rest comes from memory mapped from e.g.
persistentalloc which better fits the purpose of a sysMemStat. Also,
rename gc_sys to gcMiscSys.

Change-Id: I098789170052511e7b31edbcdc9a53e5c24573f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/246973
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: copy in MemStats fields explicitly
Michael Anthony Knyszek [Fri, 31 Jul 2020 21:32:26 +0000 (21:32 +0000)]
runtime: copy in MemStats fields explicitly

Currently MemStats is populated via an unsafe memmove from memstats, but
this places unnecessary structural restrictions on memstats, is annoying
to reason about, and tightly couples the two. Instead, just populate the
fields of MemStats explicitly.

Change-Id: I96f6a64326b1a91d4084e7b30169a4bbe6a331f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/246972
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: delineate which memstats are system stats with a type
Michael Anthony Knyszek [Wed, 29 Jul 2020 20:25:05 +0000 (20:25 +0000)]
runtime: delineate which memstats are system stats with a type

This change modifies the type of several mstats fields to be a new type:
sysMemStat. This type has the same structure as the fields used to have.

The purpose of this change is to make it very clear which stats may be
used in various functions for accounting (usually the platform-specific
sys* functions, but there are others). Currently there's an implicit
understanding that the *uint64 value passed to these functions is some
kind of statistic whose value is atomically managed. This understanding
isn't inherently problematic, but we're about to change how some stats
(which currently use mSysStatInc and mSysStatDec) work, so we want to
make it very clear what the various requirements are around "sysStat".

This change also removes mSysStatInc and mSysStatDec in favor of a
method on sysMemStat. Note that those two functions were originally
written the way they were because atomic 64-bit adds required a valid G
on ARM, but this hasn't been the case for a very long time (since
golang.org/cl/14204, but even before then it wasn't clear if mutexes
required a valid G anymore). Today we implement 64-bit adds on ARM with
a spinlock table.

Change-Id: I4e9b37cf14afc2ae20cf736e874eb0064af086d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/246971
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: make the span allocation purpose more explicit
Michael Anthony Knyszek [Wed, 29 Jul 2020 19:00:37 +0000 (19:00 +0000)]
runtime: make the span allocation purpose more explicit

This change modifies mheap's span allocation API to have each caller
declare a purpose, defined as a new enum called spanAllocType.

The purpose behind this change is two-fold:
1. Tight control over who gets to allocate heap memory is, generally
   speaking, a good thing. Every codepath that allocates heap memory
   places additional implicit restrictions on the allocator. A notable
   example of a restriction is work bufs coming from heap memory: write
   barriers are not allowed in allocation paths because then we could
   have a situation where the allocator calls into the allocator.
2. Memory statistic updating is explicit. Instead of passing an opaque
   pointer for statistic updating, which places restrictions on how that
   statistic may be updated, we use the spanAllocType to determine which
   statistic to update and how.

We also take this opportunity to group all the statistic updating code
together, which should make the accounting code a little easier to
follow.

Change-Id: Ic0b0898959ba2a776f67122f0e36c9d7d60e3085
Reviewed-on: https://go-review.googlesource.com/c/go/+/246970
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: rename mcache fields to match Go style
Michael Anthony Knyszek [Fri, 24 Jul 2020 19:58:31 +0000 (19:58 +0000)]
runtime: rename mcache fields to match Go style

This change renames a bunch of malloc statistics stored in the mcache
that are all named with the "local_" prefix. It also renames largeAlloc
to allocLarge to prevent a naming conflict, and next_sample because it
would be the last mcache field with the old C naming style.

Change-Id: I29695cb83b397a435ede7e9ad5c3c9be72767ea3
Reviewed-on: https://go-review.googlesource.com/c/go/+/246969
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: flush local_scan directly and more often
Michael Anthony Knyszek [Thu, 23 Jul 2020 22:36:58 +0000 (22:36 +0000)]
runtime: flush local_scan directly and more often

Now that local_scan is the last mcache-based statistic that is flushed
by purgecachedstats, and heap_scan and gcController.revise may be
interacted with concurrently, we don't need to flush heap_scan at
arbitrary locations where the heap is locked, and we don't need
purgecachedstats and cachestats anymore. Instead, we can flush
local_scan at the same time we update heap_live in refill, so the two
updates may share the same revise call.

Clean up unused functions, remove code that would cause the heap to get
locked in the allocSpan when it didn't need to (other than to flush
local_scan), and flush local_scan explicitly in a few important places.
Notably we need to flush local_scan whenever we flush the other stats,
but it doesn't need to be donated anywhere, so have releaseAll do the
flushing. Also, we need to flush local_scan before we set heap_scan at
the end of a GC, which was previously handled by cachestats. Just do so
explicitly -- it's not much code and it becomes a lot more clear why we
need to do so.

Change-Id: I35ac081784df7744d515479896a41d530653692d
Reviewed-on: https://go-review.googlesource.com/c/go/+/246968
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: don't flush local_tinyallocs
Michael Anthony Knyszek [Thu, 23 Jul 2020 22:16:46 +0000 (22:16 +0000)]
runtime: don't flush local_tinyallocs

This change makes local_tinyallocs work like the rest of the malloc
stats and doesn't flush local_tinyallocs, instead making that the
source-of-truth.

Change-Id: I3e6cb5f1b3d086e432ce7d456895511a48e3617a
Reviewed-on: https://go-review.googlesource.com/c/go/+/246967
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: remove mcentral.nmalloc and add mcache.local_nsmallalloc
Michael Anthony Knyszek [Thu, 23 Jul 2020 22:07:44 +0000 (22:07 +0000)]
runtime: remove mcentral.nmalloc and add mcache.local_nsmallalloc

This change removes mcentral.nmalloc and adds mcache.local_nsmallalloc
which fulfills the same role but may be accessed non-atomically. It also
moves responsibility for updating heap_live and local_nsmallalloc into
mcache functions.

As a result of this change, mcache is now the sole source-of-truth for
malloc stats. It is also solely responsible for updating heap_live and
performing the various operations required as a result of updating
heap_live. The overall improvement here is in code organization:
previously malloc stats were fairly scattered, and now they have one
single home, and nearly all the required manipulations exist in a single
file.

Change-Id: I7e93fa297c1debf17e3f2a0d68aeed28a9c6af00
Reviewed-on: https://go-review.googlesource.com/c/go/+/246966
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: make nlargealloc and largealloc mcache fields
Michael Anthony Knyszek [Thu, 23 Jul 2020 21:10:29 +0000 (21:10 +0000)]
runtime: make nlargealloc and largealloc mcache fields

This change makes nlargealloc and largealloc into mcache fields just
like nlargefree and largefree. These local fields become the new
source-of-truth. This change also moves the accounting for these fields
out of allocSpan (which is an inappropriate place for it -- this
accounting generally happens much closer to the point of allocation) and
into largeAlloc. This move is partially possible now that we can call
gcController.revise at that point.

Furthermore, this change moves largeAlloc into mcache.go and makes it a
method of mcache. While there's a little bit of a mismatch here because
largeAlloc barely interacts with the mcache, it helps solidify the
mcache as the first allocation layer and provides a clear place to
aggregate and manage statistics.

Change-Id: I37b5e648710733bb4c04430b71e96700e438587a
Reviewed-on: https://go-review.googlesource.com/c/go/+/246965
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: make distributed/local malloc stats the source-of-truth
Michael Anthony Knyszek [Thu, 23 Jul 2020 21:02:05 +0000 (21:02 +0000)]
runtime: make distributed/local malloc stats the source-of-truth

This change makes it so that various local malloc stats (excluding
heap_scan and local_tinyallocs) are no longer written first to mheap
fields but are instead accessed directly from each mcache.

This change is part of a move toward having stats be distributed, and
cleaning up some old code related to the stats.

Note that because there's no central source-of-truth, when an mcache
dies, it must donate its stats to another mcache. It's always safe to
donate to the mcache for the 0th P, so do that.

Change-Id: I2556093dbc27357cb9621c9b97671f3c00aa1173
Reviewed-on: https://go-review.googlesource.com/c/go/+/246964
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: access the assist ratio atomically
Michael Anthony Knyszek [Thu, 23 Jul 2020 20:48:06 +0000 (20:48 +0000)]
runtime: access the assist ratio atomically

This change makes it so that the GC assist ratio (the pair of
gcControllerState fields assistBytesPerWork and assistWorkPerByte) is
updated atomically. Note that the pair of fields are not updated
together atomically, but that's OK. The code here was already racy for
some time and in practice the assist ratio moves very slowly.

The purpose of this change is so that we can document
gcController.revise to be safe for concurrent use, which will be useful
in further changes.

Change-Id: Ie25d630207c88e4f85f2b8953f6a0051ebf1b4ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/246963
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: make next_gc atomically accessed
Michael Anthony Knyszek [Thu, 23 Jul 2020 20:24:56 +0000 (20:24 +0000)]
runtime: make next_gc atomically accessed

next_gc is mostly updated only during a STW, but may occasionally be
updated by calls to e.g. debug.SetGCPercent. In this case the update is
supposed to be protected by the heap lock, but in reality it's accessed
by gcController.revise which may be called without the heap lock held
(despite its documentation, which will be updated in a later change).

Change the synchronization policy on next_gc so that it's atomically
accessed when the world is not stopped to aid in making revise safe for
concurrent use.

Change-Id: I79657a72f91563f3241aaeda66e8a7757d399529
Reviewed-on: https://go-review.googlesource.com/c/go/+/246962
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: load gcControllerState.scanWork atomically in revise
Michael Anthony Knyszek [Thu, 23 Jul 2020 20:17:40 +0000 (20:17 +0000)]
runtime: load gcControllerState.scanWork atomically in revise

gcControllerState.scanWork's docs state that it must be accessed
atomically during a GC cycle, but gcControllerState.revise does not do
this (even when called with the heap lock held).

This change makes it so that gcControllerState.revise accesses scanWork
atomically and explicitly.

Note that we don't update gcControllerState.revise's erroneous doc
comment here because this change isn't about revise's guarantees, just
about heap_scan. The comment is updated in a later change.

Change-Id: Iafc3ad214e517190bfd8a219896d23da19f7659d
Reviewed-on: https://go-review.googlesource.com/c/go/+/246961
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agoruntime: define and enforce synchronization on heap_scan
Michael Anthony Knyszek [Thu, 23 Jul 2020 20:13:49 +0000 (20:13 +0000)]
runtime: define and enforce synchronization on heap_scan

Currently heap_scan is mostly protected by the heap lock, but
gcControllerState.revise sometimes accesses it without a lock. In an
effort to make gcControllerState.revise callable from more contexts (and
have its synchronization guarantees actually respected), make heap_scan
atomically read from and written to, unless the world is stopped.

Note that we don't update gcControllerState.revise's erroneous doc
comment here because this change isn't about revise's guarantees, just
about heap_scan. The comment is updated in a later change.

Change-Id: Iddbbeb954767c704c2bd1d221f36e6c4fc9948a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/246960
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
4 years agocmd/go/internal/fsys: rewrite non-idiomatic if statements
Russ Cox [Fri, 23 Oct 2020 00:55:38 +0000 (20:55 -0400)]
cmd/go/internal/fsys: rewrite non-idiomatic if statements

https://golang.org/doc/effective_go.html#if

Change-Id: I4d868e05c7827638f45b3b06d8762f5a298d56f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/264537
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
4 years agoruntime: fix sub-uintptr-sized Windows callback arguments
Austin Clements [Sat, 17 Oct 2020 22:42:03 +0000 (18:42 -0400)]
runtime: fix sub-uintptr-sized Windows callback arguments

The Windows callback support accepts Go functions with arguments that
are uintptr-sized or smaller. However, it doesn't implement smaller
arguments correctly. It assumes the Windows arguments layout is
equivalent to the Go argument layout. This is often true, but because
Windows C ABIs pad arguments to word size, while Go packs arguments,
the layout is different if there are multiple sub-word-size arguments
in a row. For example, a function with two uint16 arguments will have
a two-word C argument frame, but only a 4 byte Go argument frame.
There are also subtleties surrounding floating-point register
arguments that it doesn't handle correctly.

To fix this, when constructing a callback, we examine the Go
function's signature to construct a mapping between the C argument
frame and the Go argument frame. When the callback is invoked, we use
this mapping to build the Go argument frame and copy the result back.

This adds several test cases to TestStdcallAndCDeclCallbacks that
exercise more complex function signatures. These all fail with the
current code, but work with this CL.

In addition to fixing these callback types, this is also a step toward
the Go register ABI (#40724), which is going to make the ABI
translation more complex.

Change-Id: I19fb1681b659d9fd528ffd5e88912bebb95da052
Reviewed-on: https://go-review.googlesource.com/c/go/+/263271
Trust: Austin Clements <austin@google.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
4 years agoruntime: tidy Windows callback test
Austin Clements [Sat, 17 Oct 2020 02:22:20 +0000 (22:22 -0400)]
runtime: tidy Windows callback test

This simplifies the systematic test of Windows callbacks with
different signatures and prepares it for expanded coverage of function
signatures.

It now returns a result from the Go function and threads it back
through C. This simplifies things, but also previously the code could
have succeeded by simply not calling the callbacks at all (though
other tests would have caught that).

It bundles together the C function description and the Go function
it's intended to call. Now the test source generation and the test
running both loop over a single slice of test functions.

Since the C function and Go function are now bundled, it generates the
C function by reflectively inspecting the signature of the Go
function. For the moment, we keep the same test suite, which is
entirely functions with "uintptr" arguments, but we'll expand this
shortly.

It now use sub-tests. This way tests automatically get useful
diagnostic labels in failures and the tests don't have to catch panics
on their own.

It eliminates the DLL function argument. I honestly couldn't figure
out what the point of this was, and it added what appeared to be an
unnecessary loop level to the tests.

Change-Id: I120dfd4785057cc2c392bd2c821302f276bd128e
Reviewed-on: https://go-review.googlesource.com/c/go/+/263270
Trust: Austin Clements <austin@google.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>