mp.isExtraInC is intended to indicate that this M has no Go frames at
all; it is entirely executing in C.
If there was a cgocallback to Go and then a cgocall to C, such that the
leaf frames are C, that is fine. e.g., traceback can handle this fine
with SetCgoTraceback (or by simply skipping the C frames).
However, we currently mismanage isExtraInC, unconditionally setting it
on return from cgocallback. This means that if there are two levels of
cgocallback, we end up running Go code with isExtraInC set.
1. C-created thread calls into Go function 1 (via cgocallback).
2. Go function 1 calls into C function 1 (via cgocall).
3. C function 1 calls into Go function 2 (via cgocallback).
4. Go function 2 returns back to C function 1 (returning via the remainder of cgocallback).
5. C function 1 returns back to Go function 1 (returning via the remainder of cgocall).
6. Go function 1 is now running with mp.isExtraInC == true.
The fix is simple; only set isExtraInC on return from cgocallback if
there are no more Go frames. There can't be more Go frames unless there
is an active cgocall out of the Go frames.
For #72870.
Fixes#72871.
Cq-Include-Trybots: luci.golang.try:go1.23-linux-amd64-longtest
Change-Id: I6a6a636c4e7ba75a29639d7036c5af3738033467
Reviewed-on: https://go-review.googlesource.com/c/go/+/658035
Reviewed-by: Cherry Mui <cherryyz@google.com>
Commit-Queue: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 577bb3d0ce)
Reviewed-on: https://go-review.googlesource.com/c/go/+/658055
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
The existing code for recover from deferrangefunc was broken in
several ways.
1. the code following a deferrangefunc call did not check the return
value for an out-of-band value indicating "return now" (i.e., recover
was called)
2. the returned value was delivered using a bespoke ABI that happened
to match on register-ABI platforms, but not on older stack-based
ABI.
3. the returned value was the wrong width (1 word versus 2) and
type/value(integer 1, not a pointer to anything) for deferrangefunc's
any-typed return value (in practice, the OOB value check could catch
this, but still, it's sketchy).
This -- using the deferreturn lookup method already in place for
open-coded defers -- turned out to be a much-less-ugly way of
obtaining the desired transfer of control for recover().
TODO: we also could do this for regular defer, and delete some code.
Fixes#71839
Change-Id: If7d7ea789ad4320821aab3b443759a7d71647ff0
Reviewed-on: https://go-review.googlesource.com/c/go/+/650476
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/651496
Currently, at a cgo callback where there is already a Go frame on
the stack (i.e. C->Go->C->Go), we require that at the inner Go
callback the SP is within the g0's stack bounds set by a previous
callback. This is to prevent that the C code switches stack while
having a Go frame on the stack, which we don't really support. But
this could also happen when we cannot get accurate stack bounds,
e.g. when pthread_getattr_np is not available. Since the stack
bounds are just estimates based on the current SP, if there are
multiple C->Go callbacks with various stack depth, it is possible
that the SP of a later callback falls out of a previous call's
estimate. This leads to runtime throw in a seemingly reasonable
program.
This CL changes it to save the old g0 stack bounds at cgocallback,
update the bounds, and restore the old bounds at return. So each
callback will get its own stack bounds based on the current SP,
and when it returns, the outer callback has the its old stack
bounds restored.
Also, at a cgo callback when there is no Go frame on the stack,
we currently always get new stack bounds. We do this because if
we can only get estimated bounds based on the SP, and the stack
depth varies a lot between two C->Go calls, the previous
estimates may be off and we fall out or nearly fall out of the
previous bounds. But this causes a performance problem: the
pthread API to get accurate stack bounds (pthread_getattr_np) is
very slow when called on the main thread. Getting the stack bounds
every time significantly slows down repeated C->Go calls on the
main thread.
This CL fixes it by "caching" the stack bounds if they are
accurate. I.e. at the second time Go calls into C, if the previous
stack bounds are accurate, and the current SP is in bounds, we can
be sure it is the same stack and we don't need to update the bounds.
This avoids the repeated calls to pthread_getattr_np. If we cannot
get the accurate bounds, we continue to update the stack bounds
based on the SP, and that operation is very cheap.
On a Linux/AMD64 machine with glibc:
name old time/op new time/op delta
CgoCallbackMainThread-8 96.4µs ± 3% 0.1µs ± 2% -99.92% (p=0.000 n=10+9)
Updates #68285.
Updates #68587.
Fixes#69988.
Change-Id: I3422badd5ad8ff63e1a733152d05fb7a44d5d435
Reviewed-on: https://go-review.googlesource.com/c/go/+/600296
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit 76a8409eb8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/635775
The failures in #70288 are consistent with and strongly imply
stack corruption during fault handling, and debug prints show
that the Go code run during fault handling is running about
300 bytes above the bottom of the goroutine stack.
That should be okay, but that implies the DLL code that called
Go's handler was running near the bottom of the stack too,
and maybe it called other deeper things before or after the
Go handler and smashed the stack that way.
stackSystem is already 4096 bytes on amd64;
making it match that on 386 makes the flaky failures go away.
It's a little unsatisfying not to be able to say exactly what is
overflowing the stack, but the circumstantial evidence is
very strong that it's Windows.
For #70288.
Fixes#70475.
Change-Id: Ife89385873d5e5062a71629dbfee40825edefa49
Reviewed-on: https://go-review.googlesource.com/c/go/+/627375
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 7eeb0a188e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/632196
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Veronica Silina <veronicasilina@google.com>
getOrAddWeakHandle is very careful about keeping its input alive across
the operation, but not very careful about keeping the heap-allocated
handle it creates alive. In fact, there's a window in this function
where it is *only* visible via the special. Specifically, the window of
time between when the handle is stored in the special and when the
special actually becomes visible to the GC.
(If we fail to add the special because it already exists, that case is
fine. We don't even use the same handle value, but the one we obtain
from the attached GC-visible special, *and* we return that value, so it
remains live.)
For #70455.
Fixes#70469.
Change-Id: Iadaff0cfb93bcaf61ba2b05be7fa0519c481de82
Reviewed-on: https://go-review.googlesource.com/c/go/+/630316
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Currently it's possible for weak->strong conversions to create more GC
work during mark termination. When a weak->strong conversion happens
during the mark phase, we need to mark the newly-strong pointer, since
it may now be the only pointer to that object. In other words, the
object could be white.
But queueing new white objects creates GC work, and if this happens
during mark termination, we could end up violating mark termination
invariants. In the parlance of the mark termination algorithm, the
weak->strong conversion is a non-monotonic source of GC work, unlike the
write barriers (which will eventually only see black objects).
This change fixes the problem by forcing weak->strong conversions to
block during mark termination. We can do this efficiently by setting a
global flag before the ragged barrier that is checked at each
weak->strong conversion. If the flag is set, then the conversions block.
The ragged barrier ensures that all Ps have observed the flag and that
any weak->strong conversions which completed before the ragged barrier
have their newly-minted strong pointers visible in GC work queues if
necessary. We later unset the flag and wake all the blocked goroutines
during the mark termination STW.
There are a few subtleties that we need to account for. For one, it's
possible that a goroutine which blocked in a weak->strong conversion
wakes up only to find it's mark termination time again, so we need to
recheck the global flag on wake. We should also stay non-preemptible
while performing the check, so that if the check *does* appear as true,
it cannot switch back to false while we're actively trying to block. If
it switches to false while we try to block, then we'll be stuck in the
queue until the following GC.
All-in-all, this CL is more complicated than I would have liked, but
it's the only idea so far that is clearly correct to me at a high level.
This change adds a test which is somewhat invasive as it manipulates
mark termination, but hopefully that infrastructure will be useful for
debugging, fixing, and regression testing mark termination whenever we
do fix it.
For #69803.
Fixes#70323.
Change-Id: Ie314e6fd357c9e2a07a9be21f217f75f7aba8c4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/623615
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 80d306da50)
Reviewed-on: https://go-review.googlesource.com/c/go/+/627615
TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Fix a regression introduced in CL 598515 causing runtime.MutexProfile
stack traces to omit their root frames.
In most cases this was merely causing the `runtime.goexit` frame to go
missing. But in the case of runtime._LostContendedRuntimeLock, an empty
stack trace was being produced.
Add a test that catches this regression by checking for a stack trace
with the `runtime.goexit` frame.
Also fix a separate problem in expandFrame that could cause
out-of-bounds panics when profstackdepth is set to a value below 32.
There is no test for this fix because profstackdepth can't be changed at
runtime right now.
Fixes#69865
Change-Id: I1600fe62548ea84981df0916d25072c3ddf1ea1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/611615
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit c64ca8c6ef)
Reviewed-on: https://go-review.googlesource.com/c/go/+/621276
Reviewed-by: Cherry Mui <cherryyz@google.com>
CL 622235 would fix#70000 while resulting in one extra sendfile(2) system
call when sendfile(2) returns (>0, EAGAIN).
That's also why I left sendfile_bsd.go behind, and didn't make it line up
with other two implementations: sendfile_linux.go and sendfile_solaris.go.
Unlike sendfile(2)'s on Linux and Solaris that always return (0, EAGAIN),
sendfile(2)'s on *BSD and macOS may return (>0, EAGAIN) when using a socket
marked for non-blocking I/O. In that case, the current code will try to re-call
sendfile(2) immediately, which will most likely get us a (0, EAGAIN).
After that, it goes to `dstFD.pd.waitWrite(dstFD.isFile)` below,
which should have been done in the first place.
Thus, the real problem that leads to #70000 is that the old code doesn't handle
the special case of sendfile(2) sending the exact number of bytes the caller requested.
Fixes#70000Fixes#70020
Change-Id: I6073d6b9feb58b3d7e114ec21e4e80d9727bca66
Reviewed-on: https://go-review.googlesource.com/c/go/+/622255
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/622697
The BSD implementation of poll.SendFile incorrectly halted
copying after succesfully writing one full chunk of data.
Adjust the copy loop to match the Linux and Solaris
implementations.
In testing, empirically macOS appears to sometimes return
EAGAIN from sendfile after successfully copying a full
chunk. Add a check to all implementations to return nil
after successfully copying all data if the last sendfile
call returns EAGAIN.
For #70000
For #70020
Change-Id: I57ba649491fc078c7330310b23e1cfd85135c8ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/622235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit bd388c0216)
Reviewed-on: https://go-review.googlesource.com/c/go/+/622696
Currently, on Mach-O, the Go linker doesn't generate LC_UUID in
internal linking mode. This causes some macOS system tools unable
to track the binary, as well as in some cases the binary unable
to access local network on macOS 15.
This CL makes the linker start generate LC_UUID. Currently, the
UUID is generated if the -B flag is specified. And we'll make it
generate UUID by default in a later CL. The -B flag is currently
for generating GNU build ID on ELF, which is a similar concept to
Mach-O's UUID. Instead of introducing another flag, we just use
the same flag and the same setting. Specifically, "-B gobuildid"
will generate a UUID based on the Go build ID.
Updates #68678.
Fixes#69992.
Cq-Include-Trybots: luci.golang.try:go1.23-darwin-amd64_14,go1.23-darwin-arm64_13
Change-Id: I90089a78ba144110bf06c1c6836daf2d737ff10a
Reviewed-on: https://go-review.googlesource.com/c/go/+/618595
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Than McIntosh <thanm@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 20ed603118)
Reviewed-on: https://go-review.googlesource.com/c/go/+/622595
Reviewed-by: Michael Pratt <mpratt@google.com>
This change switches isSending to be an atomic.Int32 instead of an
atomic.Uint8. The Int32 version is managed as a counter, which is
something that we couldn't do with Uint8 without adding a new intrinsic
which may not be available on all architectures.
That is, instead of only being able to support 8 concurrent timer
firings on the same timer because we only have 8 independent bits to set
for each concurrent timer firing, we can now have 2^31-1 concurrent
timer firings before running into any issues. Like the fact that each
bit-set was matched with a clear, here we match increments with
decrements to indicate that we're in the "sending on a channel" critical
section in the timer code, so we can report the correct result back on
Stop or Reset.
We choose an Int32 instead of a Uint32 because it's easier to check for
obviously bad values (negative values are always bad) and 2^31-1
concurrent timer firings should be enough for anyone.
Previously, we avoided anything bigger than a Uint8 because we could
pack it into some padding in the runtime.timer struct. But it turns out
that the type that actually matters, runtime.timeTimer, is exactly 96
bytes in size. This means its in the next size class up in the 112 byte
size class because of an allocation header. We thus have some free space
to work with. This change increases the size of this struct from 96
bytes to 104 bytes.
(I'm not sure if runtime.timer is often allocated directly, but if it
is, we get lucky in the same way too. It's exactly 80 bytes in size,
which means its in the 96-byte size class, leaving us with some space to
work with.)
Fixes#69978
For #69969.
Related to #69880 and #69312 and #69882.
Change-Id: I9fd59cb6a69365c62971d1f225490a65c58f3e77
Cq-Include-Trybots: luci.golang.try:go1.23-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/621616
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 6a49f81edc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/621856
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Inside Google we have seen issues with QEMU user mode failing to wake a
parent waitid when this child exits with SYS_EXIT. This bug appears to
not affect SYS_EXIT_GROUP.
It is currently unclear if this is a general QEMU or specific to
Google's configuration, but SYS_EXIT and SYS_EXIT_GROUP are semantically
equivalent here, so we can use the latter here in case this is a general
QEMU bug.
For #68976.
For #69259.
Change-Id: I34e51088c9a6b7493a060e2a719a3cc4a3d54aa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/617417
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 47a9935920)
Reviewed-on: https://go-review.googlesource.com/c/go/+/617716
clone(CLONE_PIDFD) was added in Linux 5.2 and pidfd_open was added in
Linux 5.3. Thus our feature check for pidfd_open should be sufficient to
ensure that clone(CLONE_PIDFD) works.
Unfortuantely, some alternative Linux implementations may not follow
this strict ordering. For example, QEMU 7.2 (Dec 2022) added pidfd_open,
but clone(CLONE_PIDFD) was only added in QEMU 8.0 (Apr 2023).
Debian bookworm provides QEMU 7.2 by default.
For #68976.
Fixes#69259.
Change-Id: Ie3f3dc51f0cd76944871bf98690abf59f68fd7bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/592078
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 7a5fc9b34d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/612218
I've done some more testing of the new isSending field.
I'm not able to get more than 2 bits set. That said,
with this change it's significantly less likely to have even
2 bits set. The idea here is to clear the bit before possibly
locking the channel we are sending the value on, thus avoiding
some delay and some serialization.
For #69312
For #69333
Change-Id: I8b5f167f162bbcbcbf7ea47305967f349b62b0f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/617596
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
On Arch Linux with gdb version 15.1, the test for TestGdbAutotmpTypes print
the following output,
----
~/src/go/src/runtime
$ go test -run=TestGdbAutotmpTypes -v
=== RUN TestGdbAutotmpTypes
=== PAUSE TestGdbAutotmpTypes
=== CONT TestGdbAutotmpTypes
runtime-gdb_test.go:78: gdb version 15.1
runtime-gdb_test.go:570: gdb output:
Loading Go Runtime support.
Target 'exec' cannot support this command.
Breakpoint 1 at 0x46e416: file /tmp/TestGdbAutotmpTypes750485513/001/main.go, line 8.
This GDB supports auto-downloading debuginfo from the following URLs:
<https://debuginfod.archlinux.org>
Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
Debuginfod has been disabled.
To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
[New LWP 355373]
[New LWP 355374]
[New LWP 355375]
[New LWP 355376]
Thread 1 "a.exe" hit Breakpoint 1, main.main () at /tmp/TestGdbAutotmpTypes750485513/001/main.go:8
8 func main() {
9 var iface interface{} = map[string]astruct{}
All types matching regular expression "astruct":
File runtime:
[]main.astruct
bucket<string,main.astruct>
hash<string,main.astruct>
main.astruct
typedef hash<string,main.astruct> * map[string]main.astruct;
typedef noalg.[8]main.astruct noalg.[8]main.astruct;
noalg.map.bucket[string]main.astruct
runtime-gdb_test.go:587: could not find []main.astruct; in 'info typrs astruct' output
!!! FAIL
exit status 1
FAIL runtime 0.273s
$
----
In the back trace for "File runtime", each output lines does not end with
";" anymore, while in test we check the string with it.
While at it, print the expected string with "%q" instead of "%s" for
better error message.
For #67089Fixes#69746
Change-Id: If6019ee68c0d8e495c920f98568741462c7d0fd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/598135
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit ff695ca2e3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/617455
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
The timer code is careful to ensure that if stop/reset is called
while a timer is being run, we cancel the run. However, the code
failed to ensure that in that case stop/reset returned true,
meaning that the timer had been stopped. In the racing case
stop/reset could see that t.when had been set to zero,
and return false, even though the timer had not and never would fire.
Fix this by tracking whether a timer run is in progress,
and using that to reliably detect that the run was cancelled,
meaning that stop/reset should return true.
For #69312Fixes#69333
Change-Id: I78e870063eb96650638f12c056e32c931417c84a
Reviewed-on: https://go-review.googlesource.com/c/go/+/611496
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 2ebaff4890)
Reviewed-on: https://go-review.googlesource.com/c/go/+/616096
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
There's a bug in the weak-to-strong conversion in that creating the
*only* strong pointer to some weakly-held object during the mark phase
may result in that object not being properly marked.
The exact mechanism for this is that the new strong pointer will always
point to a white object (because it was only weakly referenced up until
this point) and it can then be stored in a blackened stack, hiding it
from the garbage collector.
This "hide a white pointer in the stack" problem is pretty much exactly
what the Yuasa part of the hybrid write barrier is trying to catch, so
we need to do the same thing the write barrier would do: shade the
pointer.
Added a test and confirmed that it fails with high probability if the
pointer shading is missing.
For #69210.
Fixes#69240.
Change-Id: Iaae64ae95ea7e975c2f2c3d4d1960e74e1bd1c3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/610396
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 79fd633632)
Reviewed-on: https://go-review.googlesource.com/c/go/+/610696
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
In rare situations, like during same-sized grows, the source map for
maps.Clone may be overloaded (has more than 6.5 entries per
bucket). This causes the runtime to allocate a larger bucket array for
the destination map than for the source map. The maps.Clone code
walks off the end of the source array if it is smaller than the
destination array.
This is a pretty simple fix, ensuring that the destination bucket
array is never longer than the source bucket array. Maybe a better fix
is to make the Clone code handle shorter source arrays correctly, but
this fix is deliberately simple to reduce the risk of backporting this
fix.
Fixes#69156
Change-Id: I824c93d1db690999f25a3c43b2816fc28ace7509
Reviewed-on: https://go-review.googlesource.com/c/go/+/610377
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@google.com>
The current implementation has a panic when the database is closed
concurrently with a new connection attempt.
connRequestSet.CloseAndRemoveAll sets connRequestSet.s to a nil slice.
If this happens between calls to connRequestSet.Add and
connRequestSet.Delete, there is a panic when trying to write to the nil
slice. This is sequence is likely to occur in DB.conn, where the mutex
is released between calls to db.connRequests.Add and
db.connRequests.Delete
This change updates connRequestSet.CloseAndRemoveAll to set the curIdx
to -1 for all pending requests before setting its internal slice to nil.
CloseAndRemoveAll already iterates the full slice to close all the request
channels. It seems appropriate to set curIdx to -1 before deleting the
slice for 3 reasons:
1. connRequestSet.deleteIndex also sets curIdx to -1
2. curIdx will not be relevant to anything after the slice is set to nil
3. connRequestSet.Delete already checks for negative indices
For #68949Fixes#69041
Change-Id: I6b7ebc5a71b67322908271d13865fa12f2469b87
GitHub-Last-Rev: 7d2669155b
GitHub-Pull-Request: golang/go#68953
Reviewed-on: https://go-review.googlesource.com/c/go/+/607238
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
(cherry picked from commit 08707d66c3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/609255
As of CL 580255, the runtime tracks the frame pointer (or base pointer,
bp) when entering syscalls, so that we can use fpTracebackPCs on
goroutines that are sitting in syscalls. That CL mostly got things
right, but missed one very subtle detail.
When calling from Go->C->Go, the goroutine stack performing the calls
when returning to Go is free to move around in memory due to growth,
shrinking, etc. But upon returning back to C, it needs to restore
gp.syscall*, including gp.syscallsp and gp.syscallbp. The way syscallsp
currently gets updated is automagically: it's stored as an
unsafe.Pointer on the stack so that it shows up in a stack map. If the
stack ever moves, it'll get updated correctly. But gp.syscallbp isn't
saved to the stack as an unsafe.Pointer, but rather as a uintptr, so it
never gets updated! As a result, in rare circumstances, fpTracebackPCs
can correctly try to use gp.syscallbp as the starting point for the
traceback, but the value is stale.
This change fixes the problem by just storing gp.syscallbp to the stack
on cgocallback as an unsafe.Pointer, like gp.syscallsp. It also adds a
comment documenting this subtlety; the lack of explanation for the
unsafe.Pointer type on syscallsp meant this detail was missed -- let's
not miss it again in the future.
Now, we have a fix, what about a test? Unfortunately, testing this is
going to be incredibly annoying because the circumstances under which
gp.syscallbp are actually used for traceback are non-deterministic and
hard to arrange, especially from within testprogcgo where we don't have
export_test.go and can't reach into the runtime.
So, instead, add a gp.syscallbp check to reentersyscall and
entersyscallblock that mirrors the gp.syscallbp consistency check. This
probably causes some miniscule slowdown to the syscall path, but it'll
catch the issue without having to actually perform a traceback.
For #69085.
Fixes#69087.
Change-Id: Iaf771758f1666024b854f5fbe2b2c63cbe35b201
Reviewed-on: https://go-review.googlesource.com/c/go/+/608775
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 54fe0fd43f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/608835
Change the rules for how //go:build "file versions" are applied: instead
of considering whether a file version is an upgrade or downgrade from
the -lang version, always use max(fileVersion, go1.21). This prevents
file versions from downgrading the version below go1.21. Before Go 1.21
the //go:build version did not have the meaning of setting the file's
langage version.
This fixes an issue that was appearing in GOPATH builds: Go 1.23.0
started providing -lang versions to the compiler in GOPATH mode (among
other places) which it wasn't doing before, and it set -lang to the
toolchain version (1.23). Because the -lang version was greater than
go1.21, language version used to compile the file would be set to the
//go:build file version. //go:build file versions below 1.21 could cause
files that could previously build to stop building.
For example, take a Go file with a //go:build line specifying go1.10.
If that file used a 1.18 feature, that use would compile fine with a Go
1.22 toolchain. But it would produce an error when compiling with the
1.23.0 toolchain because it set the language version to 1.10 and
disallowed the 1.18 feature. This breaks backwards compatibility: when
the build tag was added, it did not have the meaning of restricting the
language version.
For #68658Fixes#69094
Change-Id: I6cedda81a55bcccffaa3501eef9e2be6541b6ece
Reviewed-on: https://go-review.googlesource.com/c/go/+/607955
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
(cherry picked from commit aeac0b6cbf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/608935
Currently the first thing Make does it get the abi.Type of its argument,
and uses abi.TypeOf to do it. However, this has a problem for interface
types, since the type of the value stored in the interface value will
bleed through. This is a classic reflection mistake.
Fix this by implementing and using a generic TypeFor which matches
reflect.TypeFor. This gets the type of the type parameter, which is far
less ambiguous and error-prone.
For #68990.
Fixes#68992.
Change-Id: Idd8d9a1095ef017e9cd7c7779314f7d4034f01a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/607355
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 755c18ecdf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/607435
Reviewed-by: Ian Lance Taylor <iant@google.com>
It appears that some builders (notably, linux-arm) have some additional
security software installed, which apparently reads the files created by
tests. As a result, test file atime is changed, making the test fail
like these:
=== RUN TestChtimesOmit
...
os_test.go:1475: atime mismatch, got: "2024-07-30 18:42:03.450932494 +0000 UTC", want: "2024-07-30 18:42:02.450932494 +0000 UTC"
=== RUN TestChtimes
...
os_test.go:1539: AccessTime didn't go backwards; was=2024-07-31 20:45:53.390326147 +0000 UTC, after=2024-07-31 20:45:53.394326118 +0000 UTC
According to inode(7), atime is changed when more than 0 bytes are read
from the file. So, one possible solution to these flakes is to make the
test files empty, so no one can read more than 0 bytes from them.
For #68687
For #68663Fixes#68812
Change-Id: Ib9234567883ef7b16ff8811e3360cd26c2d6bdab
Reviewed-on: https://go-review.googlesource.com/c/go/+/604315
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Kirill Kolyshkin <kolyshkin@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 84266e1469)
Reviewed-on: https://go-review.googlesource.com/c/go/+/604196
Only honor //go:build language version downgrades if the version
specified is 1.21 or greater. Before 1.21 the version in //go:build
lines didn't have the meaning of setting the file's language version.
This fixes an issue that was appearing in GOPATH builds: Go 1.23 started
providing -lang versions to the compiler in GOPATH mode (among other
places) which it wasn't doing before.
For example, take a go file with a //go:build line specifying go1.10.
If that file used a 1.18 feature, that use would compile fine with a Go
1.22 toolchain. But, before this change, it would produce an error when
compiling with the 1.23 toolchain because it set the language version to
1.10 and disallowed the 1.18 feature. This breaks backwards
compatibility: when the build tag was added, it did not have the meaning
of restricting the language version.
Fixes#68658
Change-Id: I4ac2b45a981cd019183d52ba324ba8f0fed93a8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/603895
Reviewed-by: Robert Griesemer <gri@google.com>
Commit-Queue: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Matloob <matloob@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/604935
When GOEXPERIMENT=aliastypeparams is set, type aliases may have type
parameters. The compiler export data doesn't export that type parameter
information yet, which leads to an index-out-of-bounds panic when a
client package imports a package with a general type alias and then
refers to one of the missing type parameters.
This CL detects this specific case and panics with a more informative
panic message explaining the shortcoming. The change is only in effect
if the respective GOEXPERIMENT is enabled.
Manually tested. No test addded since this is just a temporary fix
(Go 1.24 will have a complete implementation), and because the existing
testing framework doesn't easily support testing that a compilation
panics.
Together with @taking and input from @rfindley.
For #68526.
Change-Id: I24737b153a7e2f9b705cd29a5b70b2b9e808dffc
Reviewed-on: https://go-review.googlesource.com/c/go/+/601035
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Merge List:
+ 2024-07-15 8f1ec59bdb strings: re-introduce noescape wrapper
+ 2024-07-15 5d36bc18d5 net/http: document io.Seeker requirement for fs.FS arguments
+ 2024-07-12 071b8d51c1 cmd: vendor golang.org/x/telemetry@268b4a8ec2d7
+ 2024-07-12 4e77872d16 go/types: fix assertion failure when range over int is not permitted
+ 2024-07-12 8bc32ab6b1 os: clean up after TestIssue60181
+ 2024-07-11 b31e9a63a4 unsafe: say "functions like syscall.Syscall", not only Syscall
+ 2024-07-11 a71bb570d0 all: make struct comments match struct names
+ 2024-07-11 611f18c4e9 strings: more cross-references in docstrings
+ 2024-07-11 08a6e080ca database/sql/driver: fix name in comment
+ 2024-07-11 dfaaa91f05 os: clarify that IsNotExist, IsExist, IsPermission and IsTimeout work with nil errors
+ 2024-07-10 5881d857c5 crypto/tls: add support for -expect-no-hrr to bogo_shim_test
+ 2024-07-10 b3040679ad math: remove riscv64 assembly implementations of rounding
+ 2024-07-10 70e453b436 context: handle nil values for valueCtx.String()
+ 2024-07-09 183a40db6d runtime: avoid multiple records with identical stacks from MutexProfile
+ 2024-07-09 e89e880eac crypto/tls: add support for -reject-alpn and -decline-alpn flags to bogo_shim_test
+ 2024-07-09 73186ba002 crypto/internal/cryptotest: add common tests for the hash.Hash interface
+ 2024-07-08 87ec2c959c testing: remove call to os.Exit in documentation for TestMain
+ 2024-07-08 6d89b38ed8 unsafe: clarify when String bytes can be modified
+ 2024-07-07 5565462a86 cmd/dist: remove iter,slices,maps test on GOEXPERIMENT=rangefunc
+ 2024-07-07 b43d6c57de io: add test for Pipe constructor allocations
+ 2024-07-07 d0146bd85b os/exec: only use cachedLookExtensions if Cmd.Path is unmodified
+ 2024-07-05 ad77cefeb2 cmd/compile: correct RewriteMultiValueCall fatal message
+ 2024-07-05 be152920b9 cmd/compile: fix ICE when compiling global a, b = f()
+ 2024-07-03 82c14346d8 cmd/link: don't disable memory profiling when pprof.WriteHeapProfile is used
+ 2024-07-03 7d19d508a9 cmd/cgo: read CGO_LDFLAGS environment variable
+ 2024-07-03 5f50b1e3bf cmd/compile: fix mis-compilation when switching over channels
+ 2024-07-03 71f9dbb1e4 cmd/compile: emit error message on mismatch import path
+ 2024-07-03 148755a27b cmd/link: document -checklinkname option
+ 2024-07-02 f12ac5be70 time: fix time zone parsing when format includes time zone seconds
+ 2024-07-02 09aeb6e33a os: add TODO about removing test exception
+ 2024-07-01 94982a0782 cmd/go/internal/workcmd: remove a potentially confusing statement
+ 2024-07-01 f71c00b616 cmd/link: align .reloc block starts by 32 bits for PE target
+ 2024-07-01 d3c93f2f00 cmd/go: update go clean help message
+ 2024-07-01 cdbf5f2f7e sync: refer to Locker interface methods in RWMutex.RLocker doc
+ 2024-07-01 c33144c47c runtime: fix nil pointer in TestGoroutineParallelism2 when offline
+ 2024-06-28 82c371a307 cmd/compile: drop internal range-over-func vars from DWARF output
+ 2024-06-28 773767def0 net/http: avoid appending an existing trailing slash to path again
+ 2024-06-28 7f90b960a9 cmd/compile: don't elide zero extension on top of signed values
+ 2024-06-27 ea537cca31 cmd/go/internal/help: add documentation for language version downgrading
+ 2024-06-27 b0927fdd49 slices: update docs for All, Backward, Values
+ 2024-06-26 5a18e79687 cmd/link: don't skip code sign even if dsymutil didn't generate a file
+ 2024-06-26 5f319b7507 cmd/link: don't let dsymutil delete our temp directory
+ 2024-06-26 a2e90be996 os: rewrite TestChtimesWithZeroTimes
+ 2024-06-25 90bcc552c0 crypto/tls: apply QUIC session event flag to QUICResumeSession events
+ 2024-06-25 b1fd047508 cmd/internal/obj/arm64: fix return with register
+ 2024-06-25 b3b4556c24 cmd/compile: update README to reflect dead code elimination changes
+ 2024-06-24 68315bc8ce cmd: run go mod tidy after CL 593684
+ 2024-06-24 f214a76075 cmd/vendor: vendor x/telemetry@38a4430
+ 2024-06-24 29b1a6765f net/http: document that Request.Clone does not deep copy Body
+ 2024-06-24 cf54a3d114 crypto/tls: replay test recordings without network
+ 2024-06-24 b98803e8e5 os: TestChtimes: separate hasNoatime
+ 2024-06-24 0def9d5c02 cmd/internal/obj/arm64: Enable arm64 assembler tests for cross-compiler builds
+ 2024-06-24 085cf0fcdc net/netip: add test that Compare and reflect.DeepEqual match
+ 2024-06-24 740043f516 net/netip: unexport fields of addrDetail
+ 2024-06-23 e8ee1dc4f9 cmd/link/internal/ld: handle "\r" in MinGW "--print-prog-name" output
+ 2024-06-22 44f1870666 cmd/link: handle dynamic import variables on Darwin in plugin mode
+ 2024-06-21 0af2148fdc cmd: vendor golang.org/x/telemetry@a740542
+ 2024-06-21 cb3b34349b doc/next: delete
+ 2024-06-21 d79c350916 cmd/internal: separate counter package from telemetry package
+ 2024-06-21 52ce25b44e cmd/vendor: pull in golang.org/x/telemetry@b4de734
+ 2024-06-21 fed2c11d67 iter: minor doc comment updates
+ 2024-06-21 d73a8a206a cmd/cgo: fail on v, err := C.fn when fn is a builtin function
+ 2024-06-21 1b4f1dc95d os: improve newFile, rm newDir
+ 2024-06-21 72e2220b50 encoding/json: clarify the map's key type for Unmarshal
+ 2024-06-21 e9a306e004 types2, go/types: correct NewTypeParam documentation
+ 2024-06-21 6fea409424 text/template/parse: fix handling of assignment/declaration in PipeNode.String
+ 2024-06-21 d67839f58a crypto/tls: add support for -expect-version to bogo_shim_test
+ 2024-06-21 201129414f sync/atomic: correct result names for Or methods
+ 2024-06-21 20b79fd577 time: provide non-default metric for asynctimerchan
+ 2024-06-20 9d33956503 internal/godebugs: fix old value for httpservecontentkeepheaders
+ 2024-06-20 477ad7dd51 cmd/compile: support generic alias type
+ 2024-06-18 4f77a83589 internal/syscall/unix: fix UTIME_OMIT for dragonfly
Change-Id: I3864b03b8c377e8fe82014eee96dc7b77aea64e2
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.