Ian Lance Taylor [Fri, 21 Jul 2023 18:14:41 +0000 (11:14 -0700)]
runtime: consider core PID in gdb test
Add the PID to the core file name if the current system uses it
when generating core files.
Fixes #61487
Change-Id: I3b53a6c850c754795c8022921160f03c588d4c91
Reviewed-on: https://go-review.googlesource.com/c/go/+/511659
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 20 Jul 2023 23:36:08 +0000 (16:36 -0700)]
spec: fix a couple of minor mistakes in type inference section
Change-Id: I9cdb301163b67add39928c8fc7df2b7f3893f45e
Reviewed-on: https://go-review.googlesource.com/c/go/+/511836 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Cherry Mui [Fri, 5 May 2023 17:31:05 +0000 (13:31 -0400)]
cmd/link: allow enabling DWARF with -s
The -s flag is to documented to disable symbol table, not DWARF
(which is the -w flag). However, due to a bug (#15166), -s was
made to also disable DWARF. That bug can be fixed without
disabling DWARF. So do that, and make it possible to enable DWARF
with -s.
Since -s has been disabling DWARF for quite some time, and users
who use -s may want to suppress all symbol information, as DWARF
also contains symbol information, we keep the current behavior,
having -s continue to disable DWARF by default. But we allow
enabling DWARF by specifying -w=0 (or false).
In summary, this is the behavior now:
-s no symbol table, no DWARF
-w has symbol table, no DWARF
-s -w no symbol table, no DWARF (same as -s)
-s -w=0 no symbol table, has DWARF
Change-Id: I1883f0aa3618abccfd735d104d983f7f531813d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/492984
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Cherry Mui [Fri, 5 May 2023 22:52:39 +0000 (18:52 -0400)]
cmd/link: rationalize -s and -w flags with Mach-O external linking
Currently, on Mach-O in external linking mode, the handling of -s
and -w flags are a bit mixed: neither flag disables the symbol
table, and both flags disable DWARF.
This CL makes it do what is documented: -s disables symbol table,
and -w disables DWARF. For the Darwin system linker, the -s flag
(strip symbol table) is obsolete. So we strip it afterwards. We
already use the strip command to strip the debug STAB symbols if
we need to combine DWARF. With this CL we'll use an additional
flag to strip more symbols. And we now also use strip if -s is
specified and we don't need to combine DWARF.
Change-Id: I9bed24fd388f2bd5b0ffa4ec2db46a4a2f6b1016
Reviewed-on: https://go-review.googlesource.com/c/go/+/493136 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 5 May 2023 21:04:45 +0000 (17:04 -0400)]
cmd/link: suppress symbol table on Mach-O when -s
Currently, on Mach-O, we don't strip the symbol table even the -s
flag is set. This CL makes it suppress the symbol table, as
documented.
On Mach-O, even with -s, we still need to keep symbols that are
dynamically exported or referenced symbol. Otherwise the dynamic
linker cannot resolve them and the binary doesn't run.
(Interestingly, for a PIE binary it is okay to strip the symbol
table entirely. We keep the dynamic symbols for consistency. And
this is also in consistent with what the system "strip" command
does.)
Change-Id: I39c572553fe0215ae3bdf5349bf2bab7205fbdc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/492744 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Thu, 20 Jul 2023 18:21:07 +0000 (14:21 -0400)]
cmd/cgo/internal/testshared: skip in short mode and not on builders
We already skip testcarchive, testcshared, and testplugin in short
mode and not on builders. The shared build mode is not more
supported than the c-archive, c-shared, and plugin build modes. No
need to run it everywhere by default.
Updates #61025.
Change-Id: I6a06e04c1a1dc78f0f85456320d128bd67277915
Reviewed-on: https://go-review.googlesource.com/c/go/+/511696
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
encoding/base64: reduce the overflow risk when computing encode/decode length
Change-Id: I0a55cdc38ae496e2070f0b9ef317a41f82352afd
GitHub-Last-Rev: c19527a26b0778cbb4548f49e1e365102709f068
GitHub-Pull-Request: golang/go#61407
Reviewed-on: https://go-review.googlesource.com/c/go/+/510635 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>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
This extra newline causes pkg.go.dev and gopls to only show the bottom
half of this comment; I'm pretty sure this entire thing is meant to be
in the docs.
Change-Id: I5bbf081fb2072d9d773d5a995bc3693dc44f65ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/511855
Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Dmitri Shuralyov [Thu, 20 Jul 2023 14:23:47 +0000 (10:23 -0400)]
cmd/dist: apply timeout scale even if timeout isn't overridden
The timeout field is documented as being available so that it's possible
to override timeout by setting a non-zero value. If it's left at zero,
we don't need to override the default go test timeout, but we still need
to apply the timeout scale whenever it's something other than 1.
Fixes (via backport) #61468.
Change-Id: I63634e9b3ef8c4ec7f334b5a6b4bf3cad121355c
Reviewed-on: https://go-review.googlesource.com/c/go/+/511567
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Most of the benchmarks are unaffected. The ones with significant reductions
are precisely for the functions that were forwarded.
This CL has to move some things around to avoid a circular dependency
between sort and slices. Since sort depends on slices now, nothing in
slices can depend on sort - not even in tests.
Fixes #61180
Change-Id: Ic0e5f519863d96a139fada08aefb1bcdf4c7a9a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/508135
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Mauri de Souza Meneguzzo [Thu, 20 Jul 2023 23:14:03 +0000 (23:14 +0000)]
reflect: panic on recv channel close
It is possible to call reflect.ValueOf(ch).Close() on a recv-only channel,
while close(ch) is a compile-time error. Following the same reflect
semantics as send and recv this should result in a panic.
Fixes #61445
Change-Id: I2a9ee8f45963593a37bd6df4643dd64fb322f9f9
GitHub-Last-Rev: fe2d5e09f5bb5536ac25d1606cf3744fb7a0a4a9
GitHub-Pull-Request: golang/go#61453
Reviewed-on: https://go-review.googlesource.com/c/go/+/511295
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Cuong Manh Le [Fri, 23 Jun 2023 06:20:33 +0000 (13:20 +0700)]
runtime/cgo: reduce runtime init done check using atomic
Every call from C to Go does acquire a mutex to check whether Go runtime
has been fully initialized. This often does not matter, because the lock
is held only briefly. However, with code that does a lot of parallel
calls from C to Go could cause heavy contention on the mutex.
Since this is an initialization guard, we can double check with atomic
operation to provide a fast path in case the initialization is done.
With this CL, program in #60961 reduces from ~2.7s to ~1.8s.
Fixes #60961
Change-Id: Iba4cabbee3c9bc646e70ef7eb074212ba63fdc04
Reviewed-on: https://go-review.googlesource.com/c/go/+/505455 Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Mon, 27 Mar 2023 15:47:18 +0000 (15:47 +0000)]
runtime: fix debug non-concurrent sweep mode after activeSweep changes
Currently the GC creates a sweepLocker before restarting the world at
the end of the mark phase, so that it can safely flush mcaches without
the runtime incorrectly concluding that sweeping is done before that
happens.
However, with GODEBUG=gcstoptheworld=2, where sweeping happens during
that STW phase, creating that sweepLocker will fail, since the runtime
will conclude that sweeping is in fact complete (all the queues will be
drained). The problem however is that gcSweep, which does the
non-concurrent sweeping, doesn't actually flush mcaches.
In essence, this failure to create a sweepLocker is indicating a real
issue: sweeping is marked as complete, but we haven't flush the mcaches
yet!
The fix to this is to flush mcaches in gcSweep when in a non-concurrent
sweep. Now that gcSweep actually completes a full sweep, it's safe to
ignore a failure to create a sweepLocker (and in fact, it *must* fail).
While we're here, let's also remove _ConcurrentSweep, the debug flag.
There's already an alias for it called concurrentSweep, and there's only
one use of it in gcSweep.
Lastly, add a dist test for the GODEBUG=gcstoptheworld=2 mode.
Fixes #53885.
Change-Id: I8a1e5b8f362ed8abd03f76e4950d3211f145ab1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/479517
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Thu, 15 Jun 2023 23:56:45 +0000 (16:56 -0700)]
time: document that AfterFunc Timer's C field is nil
Fixes #60811
Change-Id: Ica947a4789e71826284f9f6e41c298baa3d033e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/503922
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
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>