Robert Griesemer [Thu, 15 Jun 2023 23:09:52 +0000 (16:09 -0700)]
spec: update section on type inference for Go 1.21
The new section describes type inference as the problem
of solving a set of type equations for bound type parameters.
The next CL will update the section on unification to match
the new inference approach.
Change-Id: I2cb49bfb588ccc82d645343034096a82b7d602e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/503920
TryBot-Bypass: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Michael Pratt [Wed, 7 Jun 2023 20:57:34 +0000 (16:57 -0400)]
runtime: call wakep in gosched
goschedImpl transitions the current goroutine from _Grunning to
_Grunnable and places it on the global run queue before calling into
schedule.
It does _not_ call wakep after adding the global run queue. I believe
the intuition behind skipping wakep is that since we are immediately
calling the scheduler so we don't need to wake anything to run this
work. Unfortunately, this intuition is not correct, as it breaks
coordination with spinning Ms [1].
Consider this example scenario:
Initial conditions:
M0: Running P0, G0
M1: Spinning, holding P1 and looking for work
Timeline:
M1: Fails to find work; drops P
M0: newproc adds G1 to P0 runq
M0: does not wakep because there is a spinning M
M1: clear mp.spinning, decrement sched.nmspinning (now in "delicate dance")
M1: check sched.runqsize -> no global runq work
M0: gosched preempts G0; adds G0 to global runq
M0: does not wakep because gosched doesn't wakep
M0: schedules G1 from P0 runq
M1: check P0 runq -> no work
M1: no work -> park
G0 is stranded on the global runq with no M/P looking to run it. This is
a loss of work conservation.
As a result, G0 will have unbounded* scheduling delay, only getting
scheduled when G1 yields. Even once G1 yields, we still won't start
another P, so both G0 and G1 will switch back and forth sharing one P
when they should start another.
*The caveat to this is that today sysmon will preempt G1 after 10ms,
effectively capping the scheduling delay to 10ms, but not solving the P
underutilization problem. Sysmon's behavior here is theoretically
unnecessary, as our work conservation guarantee should allow sysmon to
avoid preemption if there are any idle Ps. Issue #60693 tracks changing
this behavior and the challenges involved.
[1] It would be OK if we unconditionally entered the scheduler as a
spinning M ourselves, as that would require schedule to call wakep when
it finds work in case there is more work.
Fixes #55160.
Change-Id: I2f44001239564b56ea30212553ab557051d22588
Reviewed-on: https://go-review.googlesource.com/c/go/+/501976 Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Michael Pratt [Wed, 7 Jun 2023 15:38:26 +0000 (11:38 -0400)]
runtime: check global runq during "delicate dance"
When a thread transitions to spinning to non-spinning it must recheck
all sources of work because other threads may submit new work but skip
wakep because they see a spinning thread.
However, since the beginning of time (CL 7314062) we do not check the
global run queue, only the local per-P run queues.
The global run queue is checked just above the spinning checks while
dropping the P. I am unsure what the purpose of this check is. It
appears to simply be opportunistic since sched.lock is already held
there in order to drop the P. It is not sufficient to synchronize with
threads adding work because it occurs before decrementing
sched.nmspinning, which is what threads us to decide to wake a thread.
Resolve this by adding an explicit global run queue check alongside the
local per-P run queue checks.
Almost nothing happens between dropped sched.lock after dropping the P
and relocking sched.lock: just clearing mp.spinning and decrementing
sched.nmspinning. Thus it may be better to just hold sched.lock for this
entire period, but this is a larger change that I would prefer to avoid
in the freeze and backports.
For #55160.
Change-Id: Ifd88b5a4c561c063cedcfcfe1dd8ae04202d9666
Reviewed-on: https://go-review.googlesource.com/c/go/+/501975
Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Wed, 12 Jul 2023 20:54:32 +0000 (16:54 -0400)]
cmd/link: pass flags to external linker in deterministic order
Currently we may pass C linker flags in nondeterministic order,
as on ELf systems we pass --export-dynamic-symbol for symbols from
a map. This is usally not a big problem because even if the flags
are passed in nondeterministic order the resulting binary is
probably still deterministic. This CL makes it pass them in a
deterministic order to be extra sure. This also helps build
systems where e.g. there is a build cache for the C linking action.
Change-Id: I930524dd2c3387f49d62be7ad2cef937cb2c2238
Reviewed-on: https://go-review.googlesource.com/c/go/+/509215 Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Cherry Mui [Tue, 18 Jul 2023 20:23:27 +0000 (16:23 -0400)]
cmd/go: attach PGO profile for test dependencies
When running "go test" including a main package which has a PGO
profile, we currently build the package being tested and its
dependencies with PGO, but we failed to attach the profile to
test-only dependencies. If a package is (transitively) imported
by both the package being tested and the test, the PGO version
and the non-PGO version of the package are both linked into the
binary, causing link-time error.
This CL fixes this by attaching the PGO profile to dependencies of
the test.
Cherry Mui [Thu, 8 Jun 2023 02:28:20 +0000 (22:28 -0400)]
cmd/internal/obj: print relocation type by name in -S output
The compiler/assembler's -S output prints relocation type
numerically, which is hard to understand. Every time I need to
count the relocation type constants to figure out which relocation
it actually is. Print the symbolic name instead.
Change-Id: I4866873bbae8b3dc0ee212609cb00280f9164243
Reviewed-on: https://go-review.googlesource.com/c/go/+/501856
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Cherry Mui [Thu, 8 Jun 2023 16:19:54 +0000 (12:19 -0400)]
cmd/link: handle dynamic import variables on Darwin
Currently, on darwin, we only support cgo_dynamic_import for
functions, but not variables, as we don't need it before.
mach_task_self_ is a variable defined in the system library, which
can be used to e.g. access the process's memory mappings via the
mach API. The C header defines a macro mach_task_self(), which
refers to the variable. To use mach_task_self_ (in pure-Go
programs) we need to access it in Go.
This CL handles cgo_dynamic_import for variables in the linker,
loading its address via the GOT. (Currently only on Darwin, as
we only need it there.)
For #50891.
Change-Id: Idf64fa88ba2f2381443a1ed0b42b14b581843493
Reviewed-on: https://go-review.googlesource.com/c/go/+/501855
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Bryan C. Mills [Fri, 30 Jun 2023 16:47:06 +0000 (12:47 -0400)]
runtime/pprof: use testenv.Command in tests instead of exec.Command
If the test is about to time out, testenv.Command sends SIGQUIT to the
child process. The runtime's SIGQUIT goroutine dump should help us to
determine whether the hangs observed in TestCPUProfileWithFork are a
symptom of #60108 or a separate bug.
qmuntal [Mon, 26 Jun 2023 13:10:30 +0000 (15:10 +0200)]
net: remove sysSocket fallback for Windows 7
`syscall.ForkLock` is not used in `syscall.StartProcess` since
CL 288297, so using it in `sysSocket` makes no sense. This CL
goes a little bit further and removes the `sysSocket` fallback for
Windows 7, since it is not supported since go 1.21 (#57003).
qmuntal [Tue, 27 Jun 2023 14:54:44 +0000 (16:54 +0200)]
internal/fuzz: pass handle to temporary file instead of the path
This CL partially reverts CL 297034. Inheritable handles are not
inherited by all workers thanks to using AdditionalInheritedHandles,
which explicitly specifies which handles to inherit by each worker.
This CL doesn't fix any bug, it's more of a cleanup, but also makes
the code more robust and more similar to its Unix counterpart.
Ian Lance Taylor [Tue, 23 May 2023 00:55:43 +0000 (17:55 -0700)]
cmd/go: don't collect package CGOLDFLAGS when using gccgo
They are already collected via cmd/cgo.
The gccgo_link_c test is tweaked to do real linking as with this
change the cgo ldflags are not fully reflected in go build -n output,
since they now only come from the built archive.
Fixes #60287
Change-Id: Id433435fe8aeb9571327bf936e52a37f400cef4c
Reviewed-on: https://go-review.googlesource.com/c/go/+/497117
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Andrey Bokhanko <andreybokhanko@gmail.com>
Ian Lance Taylor [Thu, 8 Jun 2023 20:11:31 +0000 (13:11 -0700)]
runtime: use unsafe.{String,StringData} in arena test
Change-Id: Ia567b163efe7b323694c15abcf0cef0effc6ff6a
Reviewed-on: https://go-review.googlesource.com/c/go/+/501995 Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
slices: refactor DeleteFunc to improve code readability
Reuse IndexFunc function to avoid confusing subscript indexing, and to reduce code nesting depth.
Change-Id: I309416ebf928071f71054433e078f0fda802fba8
GitHub-Last-Rev: af54738bda7f27afda5f92496363c0a68493c369
GitHub-Pull-Request: golang/go#61154
Reviewed-on: https://go-review.googlesource.com/c/go/+/507635
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
crypto/internal/boring,crypto/sha1: remove cmd_go_bootstrap build tag
Since CL 402595, the Go compiler no longer uses any package under
crypto, so there is no need to explicitly exclude boring from the
go bootstrap build.
os: remove executable bits from os.OpenFile example
The mode used in the os.OpenFile example (0755) has all executable bits set.
I suspect that this ill-advised usage propagates to other codebases (by means of people carelessly copying the usage example), which is why I suggest modifying the example.
Change-Id: Ic36c8b41974f3fe00471822c2414e36b4e5dc1bc
GitHub-Last-Rev: 638f3beefe8926c8e5c2c4ab9a1a5899e55de892
GitHub-Pull-Request: golang/go#61384
Reviewed-on: https://go-review.googlesource.com/c/go/+/510135 Reviewed-by: Rob Pike <r@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rob Pike <r@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Philip Silva [Mon, 10 Jul 2023 20:22:15 +0000 (22:22 +0200)]
internal/bytealg: use generic IndexByte on plan9/amd64
Use generic implementation of IndexByte/IndexByteString
on plan9/amd64 since the assembly implementation
uses SSE instructions which are classified as floating
point instructions and cannot be used in a note handler.
A similar issue was fixed in CL 100577.
This fixes runtime.TestBreakpoint.
Fixes #61087.
Change-Id: Id0c085e47da449be405ea04ab9b93518c4e2fde8
Reviewed-on: https://go-review.googlesource.com/c/go/+/508400 Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David du Colombier <0intro@gmail.com>
Damien Neil [Tue, 11 Jul 2023 22:34:09 +0000 (15:34 -0700)]
net/url: document requirements for IPv6 addresses in URL.Host
When the host subcomponent of a URL is an IPv6 address, it must be
surrounded by square brackets to prevent colons in the address
from being interpreted as the port: "[fe80::1]:80".
Document this requirement.
Fixes #61093
Fixes #61276
Change-Id: Iaf411b9dc211fd876468d6e2e94ff672ba0d329d
Reviewed-on: https://go-review.googlesource.com/c/go/+/508976
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Cherry Mui <cherryyz@google.com>
Daniel Martí [Fri, 7 Jul 2023 09:17:28 +0000 (11:17 +0200)]
net: remove unused error result from newRawConn
It's currently always nil, and the code gets generally less verbose.
Change-Id: Id4f5f9ac6eac0218dda34b8bd5ef41c633cfaf2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/508396
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Wed, 19 Jul 2023 23:09:35 +0000 (16:09 -0700)]
runtime: adjust netpollWaiters after goroutines are ready
The runtime was adjusting netpollWaiters before the waiting
goroutines were marked as ready. This could cause the scheduler
to report a deadlock because there were no goroutines ready to run.
Keeping netpollWaiters non-zero ensures that at least one goroutine
will call netpoll(-1) from findRunnable.
This does mean that if a program has network activity for a while
and then never has it again, and also has no timers, then we can leave
an M stranded in a call to netpoll from which it will never return.
At least this won't be a common case. And it's not new; this has been
a potential problem for some time.
Fixes #61454
Change-Id: I17c7f891c2bb1262fda12c6929664e64686463c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/511455
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
cmd/asm, cmd/internal/obj: generate proper atomic ops for riscv64
Go's memory model closely follows the approach C++ concurrency memory
model (https://go.dev/ref/mem) and Go atomic "has the same semantics as C++'s
sequentially consistent atomics".
Meanwhile according to RISCV manual A.6 "Mappings from C/C++ primitives to RISC-V primitives".
C/C++ atomic operations (memory_order_acq_rel) should be map to "amo<op>.{w|d}.aqrl"
LR/SC (memory_order_acq_rel) should map to "lr.{w|d}.aq; <op>; sc.{w|d}.rl"
runtime: ensure stack is aligned in _rt0_amd64_windows_lib
The Windows DLL loader may call a DLL entry point, in our case
_rt0_amd64_windows_lib, with a stack that is
not 16-byte aligned. In theory, it shouldn't, but under some
circumstances, it does (see below how to reproduce it).
Having an unaligned stack can, and probably will, cause problems
down the line, for example if a movaps instruction tries to store
a value in an unaligned address it throws an Access Violation exception
(code 0xc0000005).
I managed to consistently reproduce this issue by loading a Go DLL into
a C program that has the Page Heap Verification diagnostic enabled [1].
Change-Id: Id447d57d43b12c3748267295928d45a089549340
Reviewed-on: https://go-review.googlesource.com/c/go/+/507815 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org> Reviewed-by: Meidan Li <limeidan@loongson.cn>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Robert Griesemer [Tue, 18 Jul 2023 23:21:45 +0000 (16:21 -0700)]
spec: clarify prose in rule for clear built-in
Per feedback on #56351.
For #56351.
Change-Id: I63dd1713a1efe4d7180d932dbd8e1510cbb32e90
Reviewed-on: https://go-review.googlesource.com/c/go/+/510935
TryBot-Bypass: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Ian Lance Taylor [Wed, 12 Jul 2023 00:42:53 +0000 (17:42 -0700)]
runtime/coverage: use unsafe.Slice, not reflect.SliceHeader
Change-Id: I59c4757df83c12b4c8b85cdd523552c5e5e7bf95
Reviewed-on: https://go-review.googlesource.com/c/go/+/508977 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Ian Lance Taylor [Fri, 16 Jun 2023 00:41:19 +0000 (17:41 -0700)]
runtime: decrement netpollWaiters in netpollunblock
We used to decrement it in netpollgoready, but that missed
the common case of a descriptor becoming ready due to I/O.
All calls to netpollgoready go through netpollunblock,
so this shouldn't miss any decrements we missed before.
Fixes #60782
Change-Id: Ideefefa1ac96ca38e09fe2dd5d595c5dd7883237
Reviewed-on: https://go-review.googlesource.com/c/go/+/503923
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
It is conventional to use `err` for error variables, so change the example to use `err` instead of `e`.
Change-Id: I53bc3c5384fe608b322a55c564e9aee228b43329
GitHub-Last-Rev: 3e2ed84eefad7104b952bc6eab1c3b0af6f8f80e
GitHub-Pull-Request: golang/go#61375
Reviewed-on: https://go-review.googlesource.com/c/go/+/510075 Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Rob Pike <r@golang.org> Reviewed-by: Allen Li <ayatane@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
cmd/go: fix tests when go.env sets GOPROXY=direct GOSUMDB=off
Tested locally by changing GOROOT/go.env. At some point perhaps we
should also set up a builder that runs with some common expected
modifications to go.env
(such as GOTOOLCHAIN=local GOPROXY=direct GOSUMDB=off).
Will Roden [Wed, 12 Jul 2023 22:23:09 +0000 (17:23 -0500)]
log/slog: fix issue with concurrent writes
This causes instances commonHandler created by withAttrs or withGroup to
share a mutex with their parent preventing concurrent writes to their
shared writer.
Fixes #61321
Change-Id: Ieec225e88ad51c84b41bad6c409fac48c90320b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/509196 Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
cmd/covdata: format package comment for 'go doc' rendering
Due to an errant newline, both 'go doc' and pkg.go.dev currently
interpret the long comment in cmd/covdata/doc.go as a file comment
instead of package documentation.
Removing the errant newline caused 'go doc' to render the comment, but
it does not strip out the interior '//' tokens from the '/* … */'
block.
Removing those tokens and fixing up indentation seems to give
satisfactory rendering.
Johan Brandhorst-Satzkorn [Fri, 30 Jun 2023 20:48:10 +0000 (13:48 -0700)]
bytes: remove builders check from compare test
TestCompareBytes already took into account the -short
testing flag, however, only if not run on one of the Go builders.
This extra condition is no longer necessary as we have much
better longtest coverage than we did when the check was added.
Fixes #61071
Change-Id: I0fc716f4e7baa04019ee00608f223f27c931edcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/507416
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Bypass: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
cui fliter [Wed, 12 Jul 2023 11:11:05 +0000 (19:11 +0800)]
all: remove duplicate word and fix comment
Change-Id: I3302b94a47f384ec2519d08af50b3b5725c5b42a
Reviewed-on: https://go-review.googlesource.com/c/go/+/508995 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: shuang cui <imcusg@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Dan Kortschak [Wed, 12 Jul 2023 12:56:57 +0000 (22:26 +0930)]
all: fix typos and remove repeated words
Change-Id: I5f06a4ef1d827eb0fe32a8d98444142108b0d573
Reviewed-on: https://go-review.googlesource.com/c/go/+/508996
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@google.com>
I'm not sure why the relnote tool did not fill in a TODO for that
change; one was requested in
http://go.dev/cl/c/go/+/463177/3#message-87065dffb06e196fba9a325fefb32f16b41b6b15.
Jonathan Amsterdam [Fri, 7 Jul 2023 13:55:56 +0000 (09:55 -0400)]
testing/slogtest: check for no group with empty record
As #61067 pointed out, slog did not properly handle empty groups.
https://go.dev/cl/508436 dealt with most cases inside slog itself,
but handlers must still do a check on their own. Namely, a handler
must not output a group created by WithGroup unless the Record
has attributes.
This change adds a test to slogtest to check that case.
Fixes #61227.
Change-Id: Ibc065b6e5f6e199a41bce8332ea8c7f9d8373392
Reviewed-on: https://go-review.googlesource.com/c/go/+/508438 Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Also fix a (minor) double-Close error in Serve that was exposed by the
test fix.
Serve accepts a net.Listener, which produces net.Conn instances.
The documentation for net.Conn requires its methods to be safe for
concurrent use, so most implementations likely allow Close to be
called multiple times as a side effect of making it safe to call
concurrently with other methods. However, the net.Conn interface is a
superset of the io.Closer interface, io.Closer explicitly leaves the
behavior of multiple Close calls undefined, and net.Conn does not
explicitly document a stricter requirement.
Perhaps more importantly, the test for the fcgi package calls
unexported functions that accept an io.ReadWriteCloser (not a
net.Conn), and at least one of the test-helper ReadWriteCloser
implementations expects Close to be called only once.
The goroutine leaks were exposed by a racy arbitrary timeout reported
in #61271. Fixing the goroutine leak exposed the double-Close error:
one of the leaked goroutines was blocked on reading from an unclosed
pipe. Closing the pipe (to unblock the goroutine) triggered the second
Close call.
Fixes #61271.
Change-Id: I5cfac8870e4bb4f13adeee48910d165dbd4b76fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/508815
Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
os/exec: ignore context.Canceled errors in TestConcurrentExec
We cancel the Context to unblock the test as soon as all of the "exit"
processes have completed. If that happens to occur before all of the
"hang" processes have started, the Start calls may fail with
context.Canceled.
Since those errors are possible in normal operation of the test,
ignore them.
time: increase arbitrary upper bound in TestReset to 10s
The previous upper bound was around 0.375 s, which is empirically
too short on a slow, heavily-loaded builder. Since the test doesn't
seem to depend on the actual duration in any meaningful way, let's
make it several orders of magnitude larger.
Fixes #61266.
Change-Id: I6dde5e174966ee385db67e3cb87334f463c7e597
Reviewed-on: https://go-review.googlesource.com/c/go/+/508695
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Thanks to andrius4669 for having reported this issue to me.
It makes sense to have this GODEBUG setting not to have to modify
applications to use MPTCP (if available). It can then be useful to
estimate the impact in case we want to switch from opt-in to opt-out
later.
The MPTCP E2E test has been modified to make sure we can enable MPTCP
either via the source code like it was already the case before or with
this environment variable:
GODEBUG=multipathtcp=1
The documentation has been adapted accordingly.
I don't know if it is too late for Go 1.21 but I had to put a version in
the documentation. The modification is small, the risk seems low and
this was supposed to be there from the beginning according to Russ Cox's
specifications. It can also be backported or only be present in the
future v1.22 if it is easier.
Note: I didn't re-open #56539 or open a new one. It is not clear to me
what I should do in this case.
Fixes #56539
Change-Id: I9201f4dc0b99e3643075a34c7032a95528c48fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/507375 Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Bryan C. Mills [Fri, 30 Jun 2023 15:50:47 +0000 (11:50 -0400)]
syscall: serialize locks on ForkLock on platforms where forkExecPipe is not atomic
In CL 421441, we changed syscall to allow concurrent calls to
forkExec.
On platforms that support the pipe2 syscall that is the right
behavior, because pipe2 atomically opens the pipe with CLOEXEC already
set.
However, on platforms that do not support pipe2 (currently aix and
darwin), syscall.forkExecPipe is not atomic, and the pipes do not
initially have CLOEXEC set. If two calls to forkExec proceed
concurrently, a pipe intended for one child process can be
accidentally inherited by the other. If the process is long-lived, the
pipe can be held open unexpectedly and prevent the parent process from
reaching EOF reading the child's status from the pipe.
os: support reading empty root directories on Windows
GetFileInformationByHandleEx can return `ERROR_FILE_NOT_FOUND` when no
files were found in a root directory, as per MS-FSA 2.1.5.6.3 [1].
This error code should not be treated as an error, but rather as an
indication that no files were found, in which case `readdir` should
return an empty slice.
This CL doesn't add any test as it is difficult to trigger this error
code. Empty root directories created using Windows utilities such as
`net use` always report at least the optional `.` and `..` entries.
A reproducer is provided in #61159, but it requires WinFSP to be
installed.
Robert Findley [Fri, 7 Jul 2023 15:20:16 +0000 (11:20 -0400)]
go/types, types2: do not mutate arguments in NewChecker
CL 507975 resulted in new data races (as reported in #61212), because
the pkg argument to NewChecker was mutated.
Fix this by deferring the recording of the goVersion in pkg until type
checking is actually initiated via a call to Checker.Files.
Additionally, modify types2/check.go to bring it in sync with the
changes in go/types/check.go, and generate the new version_test.go from
the types2 file.
Also move parsing the version into checkFiles, for simplicity.
Fixes #61212
Change-Id: I15edb6c2cff3085622fe7c6a3b0dab531d27bd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/508439
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Jonathan Amsterdam [Thu, 6 Jul 2023 16:30:39 +0000 (12:30 -0400)]
log/slog: handle recursively empty groups
Handlers should not display empty groups.
A group with no attributes is certainly empty. But we also want to
consider a group to be empty if all its attributes are empty groups.
The built-in handlers did not handle this second case properly.
This CL fixes that.
There are two places in the implementation that we need to consider.
For Values of KindGroup, we change the GroupValue constructor to omit
Attrs that are empty groups. A Group is then empty if and only if it
has no Attrs. This avoids a recursive check for emptiness.
It does require allocation, but that doesn't worry us because Group
values should be relatively rare.
For groups established by WithGroup, we avoid opening such groups
unless the Record contains non-empty groups. As we did for values, we
avoid adding empty groups to records in the first place, so we only
need to check that the record has at least one Attr.
We are doing extra work, so we need to make sure we aren't slowing
things down unduly. Benchmarks before and after this change show
minimal differences.
Fixes #61067.
Change-Id: I684c7ca834bbf69210516faecae04ee548846fb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/508436
Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
os: do not skip directory entries with zero inodes on wasip1
When building programs to GOOS=wasip1, the program does not have the
guarantees that the underlying directories will come from a file system
where a zero inode value indicates that the entry was deleted but not
yet removed from the directory. The host runtime may be running on
windows or may be exposing virtual user-space file systems that do not
have the concept of inodes. In those setup, we assume that the host
runtime is in charge of dealing with edge cases such as skipping
directory entries with zero inodes when needed, and the guest
application should trust the list of entries that it sees;
therefore, we disable skipping over zero inodes on wasip1.
Change-Id: I99aa562441cdb4182965f270af054cf3cf7f8f20
Reviewed-on: https://go-review.googlesource.com/c/go/+/507915 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Ian Lance Taylor [Mon, 3 Jul 2023 12:05:55 +0000 (05:05 -0700)]
net: only build cgo_stub.go on unix or wasip1
We were building it for Windows, although Windows code never calls
any of these functions. When using -tags netgo that cause a multiple
definition of cgoAvailable.
Fixes #61153
Change-Id: Ib9e1de7720a8c0dacd6f12002917bf305dfa5405
Reviewed-on: https://go-review.googlesource.com/c/go/+/507655 Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
go/types: record Config.GoVersion for reporting in Package.GoVersion method
Clients of go/types, such as analyzers, may need to know which
specific Go version a package is written for. Record that information
in the Package and expose it using the new GoVersion method.
Update parseGoVersion to handle the new Go versions that may
be passed around starting in Go 1.21.0: versions like "go1.21.0"
and "go1.21rc2". This is not strictly necessary today, but it adds some
valuable future-proofing.
While we are here, change NewChecker from panicking on invalid
version to saving an error for returning later from Files.
Go versions are now likely to be coming from a variety of sources,
not just hard-coded in calls to NewChecker, making a panic
inappropriate.
Adds an optional close quote in the expected log message regex for TestConnections to prevent failing when the source filepath is surrounded in quotes due to it containing one or more spaces.
Michael Munday [Fri, 30 Jun 2023 23:01:26 +0000 (00:01 +0100)]
math: fix portable FMA when x*y < 0 and x*y == -z
When x*y == -z the portable implementation of FMA copied the sign
bit from x*y into the result. This meant that when x*y == -z and
x*y < 0 the result was -0 which is incorrect.
Fixes #61130.
Change-Id: Ib93a568b7bdb9031e2aedfa1bdfa9bddde90851d
Reviewed-on: https://go-review.googlesource.com/c/go/+/507376 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Michael Munday <mike.munday@lowrisc.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Joedian Reid <joedian@golang.org>
zikaeroh [Fri, 23 Jun 2023 00:09:21 +0000 (17:09 -0700)]
database/sql: prevent internal context error from being returned from Rows.Err()
CL 497675 modified Rows such that context errors are propagated through
Rows.Err(). This caused an issue where calling Close meant that an
internal cancellation error would (eventually) be returned from Err:
1. A caller makes a query using a cancellable context.
2. initContextClose sees that either the query context or the
transaction context can be canceled, so will need to spawn a
goroutine to capture their errors.
3. initContextClose derives a context from the query context via
WithCancel and sets rs.cancel.
4. When a user calls Close, rs.cancel is called. awaitDone's ctx is
cancelled, which is good, since we don't want it to hang forever.
5. This internal cancellation (after CL 497675) has its error saved on
contextDone.
6. Later, calling Err will return the error in contextDone if present.
This leads to a race condition depending on how quickly Err is called
after Close.
The docs for Close and Err state that calling Close should have no
affect on the return result for Err. So, a potential fix is to ensure
that awaitDone does not save the error when the cancellation comes from
a Close via rs.cancel.
This CL does that, using a new context not derived from the query
context, whose error is ignored as the query context's error used to be
before the original bugfix.
The included test fails before the CL, and passes afterward.
Michael Anthony Knyszek [Fri, 9 Jun 2023 19:14:55 +0000 (19:14 +0000)]
runtime,runtime/metrics: clarify OS stack metrics
There are some subtle details here about measuring OS stacks in cgo
programs. There's also an expectation about magnitude in the MemStats
docs that isn't in the runtime/metrics docs. Fix both.
Fixes #54396.
Change-Id: I6b60a62a4a304e6688e7ab4d511d66193fc25321
Reviewed-on: https://go-review.googlesource.com/c/go/+/502156
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Oleksandr Redko [Thu, 29 Jun 2023 13:06:19 +0000 (16:06 +0300)]
os, syscall: update unreachable link about =C: envs
Change-Id: I185dec133599f9c69fda7563697bbc33e433fb78
Reviewed-on: https://go-review.googlesource.com/c/go/+/507135
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Chris O'Hara [Sat, 3 Jun 2023 01:51:02 +0000 (11:51 +1000)]
net: enable pure Go resolver for wasip1
Top-level functions in the net package that only read files,
for example LookupPort(...), or LookupIP(host) where host resides
in /etc/hosts, now work on wasip1.
If the application has the ability to create sockets (for example,
when using a sockets extension to WASI preview 1), it's now
possible to do name resolution by passing a custom Dial function
to a Resolver instance.
Change-Id: I923886f67e336820bc89f09ea1855387c8dac61a
Reviewed-on: https://go-review.googlesource.com/c/go/+/500579
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Randy Reddig <ydnar@shaderlab.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Chris O'Hara [Sat, 3 Jun 2023 01:49:48 +0000 (11:49 +1000)]
syscall: stub Getrlimit on wasip1
This is a prerequisite to enabling the pure Go resolver for
wasip1.
Change-Id: Iecd8a18ce4c9eb69a697d29930bedb7175b4f0ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/500577
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Thu, 29 Jun 2023 16:48:52 +0000 (09:48 -0700)]
cmd/compile/internal/types2: make TestIssue43124 match the go/types version
Replace the (flaky) types2.TestIssue43124 with the code of the
(stable) go/types version of this test.
While at it, replace a handful of syntax.Pos{} with the equivalent
nopos, to further reduce differences between the two versions of
the issues_test.go file.
For #61064.
Change-Id: I69f3e4627a48c9928e335d67736cb875ba3835fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/507215
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Damien Neil [Wed, 28 Jun 2023 20:20:08 +0000 (13:20 -0700)]
net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
Chris O'Hara [Wed, 14 Jun 2023 02:16:38 +0000 (12:16 +1000)]
runtime: run wasip1 tests with wazero
The latest wazero release supports non-blocking I/O and pre-opened
sockets. Unmask the relevant wasip1 tests so that there are multiple
WebAssembly runtimes exercising these code paths.
Change-Id: I8506ab35186f98fde2cd3ce84634d5fcb7b053f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/503595
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Ben Hoyt [Thu, 22 Jun 2023 12:35:45 +0000 (00:35 +1200)]
slices, maps: add examples; doc comment fixes
There are currently no examples in the new slices and maps package, so
add some. This adds examples for most functions in the slices package
except the very obvious ones, and adds examples for the DeleteFunc and
EqualFunc functions in the maps package.
Also clarify/correct a few doc comments:
* EqualFunc takes an "equality" function, not a "comparison" function
* It's confusing for Delete and DeleteFunc to say they "do not create a
new slice", as they do return a new slice. They already say they
"return the modified slice" which is enough.
* Similar for Compact, and mention that it returns the modified slice
(and say why)
* Note that CompactFunc keeps the first element in equal runs
* Say what cmp is in SortStableFunc and IsSortedFunc
* Say that MinFunc and MaxFunc return the first value
Change-Id: I59c7bb1c7cabc4986d81018a5aaf5b712d3310f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/505095 Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Mark Ryan [Fri, 2 Jun 2023 11:13:29 +0000 (13:13 +0200)]
internal/bytealg: fix alignment code in equal_riscv64.s
The riscv64 implementation of equal has an optimization that is
applied when both pointers share the same alignment but that alignment
is not 8 bytes. In this case it tries to align both pointers to an 8 byte boundaries,
by individually comparing the first few bytes of each buffer. Unfortunately,
the existing code is incorrect. It adjusts the pointers by the wrong number
of bytes resulting, in most cases, in pointers that are not 8 byte aligned.
This commit fixes the issue by individually comparing the first
(8 - (pointer & 7)) bytes of each buffer rather than the first
(pointer & 7) bytes.
This particular optimization is not covered by any of the existing
benchmarks so a new benchmark, BenchmarkEqualBothUnaligned,
is provided. The benchmark tests the case where both pointers have
the same alignment but may not be 8 byte aligned. Results of the
new benchmark along with some of the existing benchmarks generated on
a SiFive HiFive Unmatched A00 with 16GB of RAM running Ubuntu 23.04
are presented below.