Robert Griesemer [Tue, 12 Mar 2024 16:31:06 +0000 (09:31 -0700)]
[release-branch.go1.21] go/types, types2: don't do version checks for embedded types of imported interfaces
[This is a re-apply of CL 571075]
Imported interfaces don't have position information for embedded types.
When computing the type set of such interfaces, doing a version check
may fail because it will rely on the Go version of the current package.
We must not do a version check for features of types from imported
packages - those types have already been typechecked and are "correct".
The version check code does look at packages to avoid such incorrect
version checks, but we don't have the package information available
in an interface type (divorced from its object).
Instead, for now rely on the fact that imported interfaces don't have
position information for embedded types: if the position is unknown,
don't do a version check.
We may want to assert that positions are known in all other cases,
but since this is an older release, don't add such additional changes
to avoid introducing other bugs.
Fixes #66326.
Updates #66064.
Change-Id: I158cf51aa382f85d612ab958ba4b591de1c5fdb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/574736 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cherry Mui [Wed, 13 Mar 2024 18:35:29 +0000 (18:35 +0000)]
[release-branch.go1.21] Revert "go/types, types2: don't do version checks for embedded types of imported interfaces"
This reverts CL 571075.
Reason for revert: We might want to do a security-only minor release. Back off the release branch to a clean state from the previous minor release. Sorry for the inconvenience.
Change-Id: Ifc8c7e00e6faea3aa547b883eed44180ddb447de
Reviewed-on: https://go-review.googlesource.com/c/go/+/571355
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Tue, 12 Mar 2024 16:31:06 +0000 (09:31 -0700)]
[release-branch.go1.21] go/types, types2: don't do version checks for embedded types of imported interfaces
Imported interfaces don't have position information for embedded types.
When computing the type set of such interfaces, doing a version check
may fail because it will rely on the Go version of the current package.
We must not do a version check for features of types from imported
packages - those types have already been typechecked and are "correct".
The version check code does look at packages to avoid such incorrect
version checks, but we don't have the package information available
in an interface type (divorced from its object).
Instead, for now rely on the fact that imported interfaces don't have
position information for embedded types: if the position is unknown,
don't do a version check.
We may want to assert that positions are known in all other cases,
but since this is an older release, don't add such additional changes
to avoid introducing other bugs.
Fixes #66064.
Change-Id: I773d57e5410c3d4a911ab3e018b3233c2972b3c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/571075 Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com>
Damien Neil [Tue, 16 Jan 2024 23:37:52 +0000 (15:37 -0800)]
[release-branch.go1.21] net/textproto, mime/multipart: avoid unbounded read in MIME header
mime/multipart.Reader.ReadForm allows specifying the maximum amount
of memory that will be consumed by the form. While this limit is
correctly applied to the parsed form data structure, it was not
being applied to individual header lines in a form.
For example, when presented with a form containing a header line
that never ends, ReadForm will continue to read the line until it
runs out of memory.
Limit the amount of data consumed when reading a header.
Fixes CVE-2023-45290
Fixes #65389
For #65383
Change-Id: I7f9264d25752009e95f6b2c80e3d76aaf321d658
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2134435 Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2173776 Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569240
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
Damien Neil [Thu, 11 Jan 2024 19:31:57 +0000 (11:31 -0800)]
[release-branch.go1.21] net/http, net/http/cookiejar: avoid subdomain matches on IPv6 zones
When deciding whether to forward cookies or sensitive headers
across a redirect, do not attempt to interpret an IPv6 address
as a domain name.
Avoids a case where a maliciously-crafted redirect to an
IPv6 address with a scoped addressing zone could be
misinterpreted as a within-domain redirect. For example,
we could interpret "::1%.www.example.com" as a subdomain
of "www.example.com".
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Fixes CVE-2023-45289
Fixes #65385
For #65065
Change-Id: I8f463f59f0e700c8a18733d2b264a8bcb3a19599
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2131938 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2173775 Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569239 Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Roland Shoemaker [Thu, 18 Jan 2024 20:51:13 +0000 (12:51 -0800)]
[release-branch.go1.21] crypto/x509: make sure pub key is non-nil before interface conversion
alreadyInChain assumes all keys fit a interface which contains the
Equal method (which they do), but this ignores that certificates may
have a nil key when PublicKeyAlgorithm is UnknownPublicKeyAlgorithm. In
this case alreadyInChain panics.
Check that the key is non-nil as part of considerCandidate (we are never
going to build a chain containing UnknownPublicKeyAlgorithm anyway).
For #65390
Fixes #65392
Fixes CVE-2024-24783
Change-Id: Ibdccc0a487e3368b6812be35daad2512220243f3
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2137282 Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2173774 Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569238
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
Roland Shoemaker [Thu, 15 Feb 2024 01:18:36 +0000 (17:18 -0800)]
[release-branch.go1.21] html/template: escape additional tokens in MarshalJSON errors
Escape "</script" and "<!--" in errors returned from MarshalJSON errors
when attempting to marshal types in script blocks. This prevents any
user controlled content from prematurely terminating the script block.
Updates #65697
Fixes #65968
Change-Id: Icf0e26c54ea7d9c1deed0bff11b6506c99ddef1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/564196
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit ccbc725f2d678255df1bd326fa511a492aa3a0aa)
Reviewed-on: https://go-review.googlesource.com/c/go/+/567515 Reviewed-by: Carlos Amedee <carlos@golang.org>
Bryan C. Mills [Tue, 12 Dec 2023 21:26:45 +0000 (16:26 -0500)]
[release-branch.go1.21] internal/syscall/windows: fix the signature of SetFileInformationByHandle
Also fix its call site in internal/poll to pass the length of the
actual buffer instead of an unrelated variable, and update the
definition of FILE_BASIC_INFO to match the documented field types
and add padding that is empirically needed on the 386 architecture.
Passing a pointer to a Go-allocated buffer as type uintptr violates
the unsafe.Pointer conversion rules, which allow such a conversion
only in the call expression itself for a call to syscall.Syscall or
equivalent. That can allow the buffer to be corrupted arbitrarily if
the Go runtime happens to garbage-collect it while the call to
SetFileInformationByHandle is in progress.
The Microsoft documentation for SetFileInformationByHandle specifies
its third argument type as LPVOID, which corresponds to Go's
unsafe.Pointer, not uintptr.
Fixes #65882.
Updates #58933.
Change-Id: If577b57adea9922f5fcca55e46030c703d8f035c
Cq-Include-Trybots: luci.golang.try:go1.21-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/549256
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
(cherry picked from commit a7097243e462802f3055da9f843013e450a75092)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566155 Reviewed-by: Bryan Mills <bcmills@google.com>
Roland Shoemaker [Wed, 10 Jan 2024 19:02:14 +0000 (11:02 -0800)]
[release-branch.go1.21] net/mail: properly handle special characters in phrase and obs-phrase
Fixes a couple of misalignments with RFC 5322 which introduce
significant diffs between (mostly) conformant parsers.
This change reverts the changes made in CL50911, which allowed certain
special RFC 5322 characters to appear unquoted in the "phrase" syntax.
It is unclear why this change was made in the first place, and created
a divergence from comformant parsers. In particular this resulted in
treating comments in display names incorrectly.
Additionally properly handle trailing malformed comments in the group
syntax.
For #65083
Fixes #65848
Change-Id: I00dddc044c6ae3381154e43236632604c390f672
Reviewed-on: https://go-review.googlesource.com/c/go/+/555596 Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/566195 Reviewed-by: Carlos Amedee <carlos@golang.org>
Michael Matloob [Tue, 20 Feb 2024 21:13:33 +0000 (16:13 -0500)]
[release-branch.go1.21] cmd/go/internal/modcmd: correctly filter out main modules in verify
This change fixes a bug where we incorrectly filtered out the main
modules from the beginning of the build list before verifying them. We
made the assumption that the first MainModules.Len() entries of the
build list were the main modules, but now it can contain the go and
toolchain version entries, so removing the first MainModules.Len()
entries could leave main module names in the build list if any of
their names sorted after the string 'go'.
For #62663
Fixes #65851
Change-Id: I35ab6857a556f58d306303322afe24c48fc8b38f
Reviewed-on: https://go-review.googlesource.com/c/go/+/565378 Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 3f60da7944352d7f2d4a04830c0612457643016c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/565755
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Fri, 1 Sep 2023 19:52:48 +0000 (12:52 -0700)]
[release-branch.go1.21] runtime: don't let the tests leave core files behind
Also add a check that we didn't leave any core files behind.
For #65476.
Fixes #65478.
Change-Id: I30444ef43ad1a8cc1cacd3b75280f2128e104939
Reviewed-on: https://go-review.googlesource.com/c/go/+/525175 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit cffdfe8d2cabbe874bceaeed1eba92cc567be6db)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560896
Michael Anthony Knyszek [Thu, 25 Jan 2024 17:23:15 +0000 (17:23 +0000)]
[release-branch.go1.21] internal/testenv: support the LUCI mobile builders in tests
This change updates the testenv tests to correctly match on future LUCI
builder names for mobile builders. This isn't a problem today because
those haven't been set up yet, but the builder names are structured and
it's clear where the modifiers will appear. Might as well set them up
now.
For #65473.
Fixes #65475.
Change-Id: I244b88a62a90312c0f3ff2360527d58531070362
Reviewed-on: https://go-review.googlesource.com/c/go/+/558597 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 5c7c24ce827b10982245951f6c2b1bbf0abc5aae)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560895
Michael Anthony Knyszek [Thu, 25 Jan 2024 17:13:11 +0000 (17:13 +0000)]
[release-branch.go1.21] internal/testenv: allow "-noopt" anywhere in builder name in test
testenv's TestHasGoBuild test is supposed to allow noopt builders to not
have go build, but the pattern match is failing on the LUCI builders
where a test shard might have an additional "-test_only" suffix in the
builder name. Furthermore, in the LUCI world, "run mods" (the builder
type suffixes) are supposed to be well-defined and composable, so it
doesn't make sense to restrict "-noopt" to the builder suffix anyway.
This change modifies the test to allow "-noopt" to appear anywhere in
the builder name when checking if it's running on a noopt builder.
For #65470.
Fixes #65472.
Change-Id: I393818e3e8e452c7b0927cbc65726d552aa8ff8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/558596
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit 93f0c0b25e7ea8ae7c2d5df32c6083cfabf2ffc2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560518
Keith Randall [Mon, 30 Oct 2023 20:23:00 +0000 (13:23 -0700)]
[release-branch.go1.21] cmd/internal/obj/arm64: fix frame pointer restore in epilogue
For leaf but nonzero-frame functions.
Currently we're not restoring it properly. We also need to restore
it before popping the stack frame, so that the frame won't get
clobbered by a signal handler in the meantime.
For #63830
Fixes #65449
Needs a test, but I'm not at all sure how we would actually do that. Leaving for inspiration.
Change-Id: I273a25f2a838f05a959c810145cccc5428eaf164
Reviewed-on: https://go-review.googlesource.com/c/go/+/538635 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Eric Fang <eric.fang@arm.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit c9888bdfe2cdd85d14d7d184863d2e8bc42ff73c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560735
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Filippo Valsorda [Fri, 26 Jan 2024 22:22:45 +0000 (23:22 +0100)]
[release-branch.go1.21] Revert "crypto/internal/boring: upgrade module to fips-20220613" +1
This reverts CL 553855 ("crypto/internal/boring: upgrade module to
fips-20220613") and CL 553856 ("crypto/tls: align FIPS-only mode with
BoringSSL policy").
Robert Griesemer [Wed, 10 Jan 2024 23:49:33 +0000 (15:49 -0800)]
[release-branch.go1.21] go/types, types2: don't lose position info of interface embeddings
Accurate position information for embedded types in interfaces is
crucial to identify the corresponding source file, and with that
the Go language version associated with that file. (The position
information is also important for proper error messages.)
Before this CL, the position information for embedded types was
discarded after type set computation, in the assumption that it
was not needed anymore. However, substitutions that update the
interface may lead to repeated type set computations which then
won't have the correct position information.
This CL does preserve the position information for embedded
types until the end of type checking (cleanup phase), and also
copy the position information during a substitution of the
interface.
The respective bug (#64759) doesn't seem to appear in 1.22 (most
likely because it's hidden by some of the changes made with respect
to the file version logic), but the existing code is still wrong.
The backport of this code to 1.21 and 1.20 fixes the issue in those
releases.
For #64759.
Fixes #65053.
Change-Id: I80f4004c9d79cb02eac6739c324c477706615102
Reviewed-on: https://go-review.googlesource.com/c/go/+/555296
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/555415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Pratt [Wed, 13 Dec 2023 20:07:28 +0000 (15:07 -0500)]
[release-branch.go1.21] runtime: properly model rwmutex in lock ranking
(This cherry-pick combines CL 549536 and the follow-up fix CL 555055.)
Currently, lock ranking doesn't really try to model rwmutex. It records
the internal locks rLock and wLock, but in a subpar fashion:
1. wLock is held from lock to unlock, so it works OK, but it conflates
write locks of all rwmutexes as rwmutexW, rather than allowing
different rwmutexes to have different rankings.
2. rLock is an internal implementation detail that is only taken when
there is contention in rlock. As as result, the reader lock path is
almost never checked.
Add proper modeling. rwmutexR and rwmutexW remain as the ranks of the
internal locks, which have their own ordering. The new init method is
passed the ranks of the higher level lock that this represents, just
like lockInit for mutex.
execW ordered before MALLOC captures the case from #64722. i.e., there
can be allocation between BeforeFork and AfterFork.
For #64722.
Fixes #64761.
------
runtime: replace rwmutexR/W with per-rwmutex lock rank
CL 549536 intended to decouple the internal implementation of rwmutex
from the semantic meaning of an rwmutex read/write lock in the static
lock ranking.
Unfortunately, it was not thought through well enough. The internals
were represented with the rwmutexR and rwmutexW lock ranks. The idea was
that the internal lock ranks need not model the higher-level ordering,
since those have separate rankings. That is incorrect; rwmutexW is held
for the duration of a write lock, so it must be ranked before any lock
taken while any write lock is held, which is precisely what we were
trying to avoid.
This is visible in violations like:
0 : execW 11 0x0
1 : rwmutexW 51 0x111d9c8
2 : fin 30 0x111d3a0
fatal error: lock ordering problem
execW < fin is modeled, but rwmutexW < fin is missing.
Fix this by eliminating the rwmutexR/W lock ranks shared across
different types of rwmutex. Instead require users to define an
additional "internal" lock rank to represent the implementation details
of rwmutex.rLock. We can avoid an additional "internal" lock rank for
rwmutex.wLock because the existing writeRank has the same semantics for
semantic and internal locking. i.e., writeRank is held for the duration
of a write lock, which is exactly how rwmutex.wLock is used, so we can
use writeRank directly on wLock.
For #64722.
Cq-Include-Trybots: luci.golang.try:go1.21-linux-amd64-staticlockranking
Change-Id: I23335b28faa42fb04f1bc9da02fdf54d1616cd28
Reviewed-on: https://go-review.googlesource.com/c/go/+/549536 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 9b4b3e5acca2dabe107fa2c3ed963097d78a4562)
(cherry picked from commit dcbe77246922fe7ef41f07df228f47a37803f360)
Reviewed-on: https://go-review.googlesource.com/c/go/+/554976
Michael Pratt [Wed, 25 Oct 2023 15:40:56 +0000 (11:40 -0400)]
[release-branch.go1.21] runtime: clear g0 stack bounds in dropm
After CL 527715, needm uses callbackUpdateSystemStack to set the stack
bounds for g0 on an M from the extra M list. Since
callbackUpdateSystemStack is also used for recursive cgocallback, it
does nothing if the stack is already in bounds.
Currently, the stack bounds in an extra M may contain stale bounds from
a previous thread that used this M and then returned it to the extra
list in dropm.
Typically a new thread will not have an overlapping stack with an old
thread, but because the old thread has exited there is a small chance
that the C memory allocator will allocate the new thread's stack
partially or fully overlapping with the old thread's stack.
If this occurs, then callbackUpdateSystemStack will not update the stack
bounds. If in addition, the overlap is partial such that SP on
cgocallback is close to the recorded stack lower bound, then Go may
quickly "overflow" the stack and crash with "morestack on g0".
Fix this by clearing the stack bounds in dropm, which ensures that
callbackUpdateSystemStack will unconditionally update the bounds in
needm.
For #62440.
Fixes #63209.
Change-Id: Ic9e2052c2090dd679ed716d1a23a86d66cbcada7
Reviewed-on: https://go-review.googlesource.com/c/go/+/537695 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
(cherry picked from commit 1af424c196584cd0b05e559c2740f046d1f32042)
Reviewed-on: https://go-review.googlesource.com/c/go/+/549495
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
[This is a redo of CL 525455 with the test fixed on darwin by defining
_XOPEN_SOURCE, and disabled with android, musl, and openbsd, which do
not provide getcontext.]
Since CL 495855, Ms are cached for C threads calling into Go, including
the stack bounds of the system stack.
Some C libraries (e.g., coroutine libraries) do manual stack management
and may change stacks between calls to Go on the same thread.
Changing the stack if there is more Go up the stack would be
problematic. But if the calls are completely independent there is no
particular reason for Go to care about the changing stack boundary.
Thus, this CL allows the stack bounds to change in such cases. The
primary downside here (besides additional complexity) is that normal
systems that do not manipulate the stack may not notice unintentional
stack corruption as quickly as before.
Note that callbackUpdateSystemStack is written to be usable for the
initial setup in needm as well as updating the stack in cgocallbackg.
For #62440.
For #62130.
For #63209.
Change-Id: I0fe0134f865932bbaff1fc0da377c35c013bd768
Reviewed-on: https://go-review.googlesource.com/c/go/+/527715
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 4f9fe6d50965020053ab80bf115f08070ce97f33)
(cherry picked from commit e8ba0579e2913f96c65b96e0696d64ff5f1599c5)
(cherry picked from commit a843991fdd079c931d4e98c0a17c9ac6dc254fe8)
(cherry picked from commit d110d7c42dd8025465153e4008ba807f1e69b359)
Reviewed-on: https://go-review.googlesource.com/c/go/+/530480
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
Cherry Mui [Fri, 29 Sep 2023 18:00:04 +0000 (14:00 -0400)]
[release-branch.go1.21] runtime: don't print "unexpected SPWRITE" when printing traceback
The system stack often starts with a stack transition function
like "systemstack" or "mcall", which is marked as SPWRITE. When
unwinding a system stack for printing, we want the traceback stop
at the stack switching frame, but not print the "unexpected
SPWRITE" message.
Previously before CL 525835, we don't print the "unexpected
SPWRITE" message if unwindPrintErrors is set, i.e. printing a
stack trace. This CL restores this behavior.
Another possibility is not printing the message only on the system
stack. We don't expect a stack transition function to appear in a
user G.
Fixes #64935.
Change-Id: I173e89ead2cd4fbf1f0f8cca225f28718b5baebe
Reviewed-on: https://go-review.googlesource.com/c/go/+/531815
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 15a274b621b3654e9b4962a27c9d14ea51645b6c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/553476 Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
David Chase [Thu, 14 Dec 2023 19:20:12 +0000 (14:20 -0500)]
[release-branch.go1.21] runtime: add race annotations in IncNonDefault
Also use CompareAndSwap to make the code actually less racy.
Added a test which will be meaningful when run under the race
detector (tested it -race with broken fix in runtime, it failed).
This backport incorporates the correction in CL 551856,
using racereleasemerge instead of racerelease.
Fixes #64757
Change-Id: I5972e08901d1adc8ba74858edad7eba91be1b0ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/549796
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 3313bbb4055f38f53cd43c6c5782a229f445f230)
Reviewed-on: https://go-review.googlesource.com/c/go/+/550236
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Keith Randall [Fri, 1 Dec 2023 06:59:04 +0000 (22:59 -0800)]
[release-branch.go1.21] maps: fix aliasing problems with Clone
Make sure to alloc+copy large keys and values instead of aliasing them,
when they might be updated by a future assignment.
Fixes #64475
Change-Id: Ie2226a81cf3897e4e2ee24472f2966d397ace53f
Reviewed-on: https://go-review.googlesource.com/c/go/+/546515 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
(cherry picked from commit 16d3040a84be821d801b75bd1a3d8ab4cc89ee36)
Reviewed-on: https://go-review.googlesource.com/c/go/+/547375
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Michael Anthony Knyszek [Mon, 27 Nov 2023 22:27:32 +0000 (22:27 +0000)]
[release-branch.go1.21] runtime: put ReadMemStats debug assertions behind a double-check mode
ReadMemStats has a few assertions it makes about the consistency of the
stats it's about to produce. Specifically, how those stats line up with
runtime-internal stats. These checks are generally useful, but crashing
just because some stats are wrong is a heavy price to pay.
For a long time this wasn't a problem, but very recently it became a
real problem. It turns out that there's real benign skew that can happen
wherein sysmon (which doesn't synchronize with a STW) generates a trace
event when tracing is enabled, and may mutate some stats while
ReadMemStats is running its checks.
Fix this by synchronizing with both sysmon and the tracer. This is a bit
heavy-handed, but better that than false positives.
Also, put the checks behind a debug mode. We want to reduce the risk of
backporting this change, and again, it's not great to crash just because
user-facing stats are off. Still, enable this debug mode during the
runtime tests so we don't lose quite as much coverage from disabling
these checks by default.
For #64401.
Fixes #64410.
Change-Id: I9adb3e5c7161d207648d07373a11da8a5f0fda9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/545277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
(cherry picked from commit b2efd1de97402ec4b8fb4e9e0ec29c8e49e8e200)
Reviewed-on: https://go-review.googlesource.com/c/go/+/545557
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com>
Michael Anthony Knyszek [Tue, 5 Dec 2023 17:31:34 +0000 (17:31 +0000)]
[release-branch.go1.21] runtime: add the disablethp GODEBUG setting
Go 1.21.1 and Go 1.22 have ceased working around an issue with Linux
kernel defaults for transparent huge pages that can result in excessive
memory overheads. (https://bugzilla.kernel.org/show_bug.cgi?id=93111)
Many Linux distributions disable huge pages altogether these days, so
this problem isn't quite as far-reaching as it used to be. Also, the
problem only affects Go programs with very particular memory usage
patterns.
That being said, because the runtime used to actively deal with this
problem (but with some unpredictable behavior), it's preventing users
that don't have a lot of control over their execution environment from
upgrading to Go beyond Go 1.20.
This change adds a GODEBUG to smooth over the transition. The GODEBUG
setting disables transparent huge pages for all heap memory on Linux,
which is much more predictable than restoring the old behavior.
For #64332.
Fixes #64561.
Change-Id: I73b1894337f0f0b1a5a17b90da1221e118e0b145
Reviewed-on: https://go-review.googlesource.com/c/go/+/547475 Reviewed-by: Michael Pratt <mpratt@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 c915215af65897cc9cffed75630cbfbc6b2462cc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/547636 Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Tolya Korniltsev [Mon, 4 Dec 2023 10:53:29 +0000 (17:53 +0700)]
[release-branch.go1.21] runtime/pprof: fix generics function names
profileBuilder is using Frame->Function as key for checking if we already
emitted a function. However for generics functions it has dots there [...],
so sometimes for different functions with different generics types,
the profileBuilder emits wrong functions.
For #64528
For #64609
Change-Id: I8b39245e0b18f4288ce758c912c6748f87cba39a
Reviewed-on: https://go-review.googlesource.com/c/go/+/546815 Reviewed-by: Cherry Mui <cherryyz@google.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 20a03fc7130d8d99b513071c7e413b436ea649a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/549535
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Michael Pratt [Wed, 1 Nov 2023 21:06:08 +0000 (17:06 -0400)]
[release-branch.go1.21] os/signal: remove go t.Run from TestNohup
Since CL 226138, TestNohup has a bit of a strange construction: it wants
to run the "uncaught" subtests in parallel with each other, and the
"nohup" subtests in parallel with each other, but also needs join
between "uncaught" and "nohop" so it can Stop notifying for SIGHUP.
It achieves this by doing `go t.Run` with a WaitGroup rather than using
`t.Parallel` in the subtest (which would make `t.Run` return immediately).
However, this makes things more difficult to understand than necessary.
As noted on https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks,
a second layer of subtest can be used to join parallel subtests.
Switch to this form, which makes the test simpler to follow
(particularly the cleanup that goes with "uncaught").
For #63799.
For #63911.
Change-Id: Ibfce0f439508a7cfca848c7ccfd136c9c453ad8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/538899
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit 5622a4b2054664edcdd64974b9df73b440aedfae)
Reviewed-on: https://go-review.googlesource.com/c/go/+/546021
Matthew Dempsky [Tue, 5 Dec 2023 20:56:04 +0000 (12:56 -0800)]
[release-branch.go1.21] cmd/compile: fix escape analysis of string min/max
When I was plumbing min/max support through the compiler, I was
thinking mostly about numeric argument types. As a result, I forgot
that escape analysis would need to be aware that min/max can operate
on string values, which contain pointers.
Updates #64565.
Fixes #64567.
Change-Id: I36127ce5a2da942401910fa0f9de922726c9f94d
Reviewed-on: https://go-review.googlesource.com/c/go/+/547715 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 34416d7f6f93cd6562636e311c362ebe421f1a4c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/547757 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
Keith Randall [Thu, 30 Nov 2023 18:04:16 +0000 (10:04 -0800)]
[release-branch.go1.21] cmd/compile: fix memcombine pass for big endian, > 1 byte elements
The shift amounts were wrong in this case, leading to miscompilation
of load combining.
Also the store combining was not triggering when it should.
Fixes #64472
Change-Id: Iaeb08972c5fc1d6f628800334789c6af7216e87b
Reviewed-on: https://go-review.googlesource.com/c/go/+/546355 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/546356
Damien Neil [Tue, 7 Nov 2023 18:47:56 +0000 (10:47 -0800)]
[release-branch.go1.21] net/http: limit chunked data overhead
The chunked transfer encoding adds some overhead to
the content transferred. When writing one byte per
chunk, for example, there are five bytes of overhead
per byte of data transferred: "1\r\nX\r\n" to send "X".
Chunks may include "chunk extensions",
which we skip over and do not use.
For example: "1;chunk extension here\r\nX\r\n".
A malicious sender can use chunk extensions to add
about 4k of overhead per byte of data.
(The maximum chunk header line size we will accept.)
Track the amount of overhead read in chunked data,
and produce an error if it seems excessive.
Updates #64433
Fixes #64435
Fixes CVE-2023-39326
Change-Id: I40f8d70eb6f9575fb43f506eb19132ccedafcf39
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2076135 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit 3473ae72ee66c60744665a24b2fde143e8964d4f)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2095408
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/547356 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Jorropo [Sun, 5 Nov 2023 21:40:01 +0000 (22:40 +0100)]
[release-branch.go1.21] cmd/compile: fix findIndVar so it does not match disjointed loop headers
Fix #63984
parseIndVar, prove and maybe more are on the assumption that the loop header
is a single block. This can be wrong, ensure we don't match theses cases we
don't know how to handle.
In the future we could update them so that they know how to handle such cases
but theses cases seems rare so I don't think the value would be really high.
We could also run a loop canonicalization pass first which could handle this.
The repro case looks weird because I massaged it so it would crash with the
previous compiler.
Change-Id: I4aa8afae9e90a17fa1085832250fc1139c97faa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/539977 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 8b4e1259d0e82c8fe38a1456f997a4e9d63573a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/540535 Reviewed-by: Jorropo <jorropo.pgm@gmail.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Fri, 10 Nov 2023 21:23:38 +0000 (21:23 +0000)]
[release-branch.go1.21] runtime: call enableMetadataHugePages and its callees on the systemstack
These functions acquire the heap lock. If they're not called on the
systemstack, a stack growth could cause a self-deadlock since stack
growth may allocate memory from the page heap.
This has been a problem for a while. If this is what's plaguing the
ppc64 port right now, it's very surprising (and probably just
coincidental) that it's showing up now.
For #64050.
For #64062.
For #64067.
Fixes #64073.
Change-Id: I2b95dc134d17be63b9fe8f7a3370fe5b5438682f
Reviewed-on: https://go-review.googlesource.com/c/go/+/541635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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> Reviewed-by: Paul Murphy <murp@ibm.com>
(cherry picked from commit 5f08b4479930af266d4a84c1533b320ed75edba7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/541955 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Roland Shoemaker [Wed, 18 Oct 2023 15:25:08 +0000 (11:25 -0400)]
[release-branch.go1.21] crypto/rand,runtime: switch RtlGenRandom for ProcessPrng
RtlGenRandom is a semi-undocumented API, also known as
SystemFunction036, which we use to generate random data on Windows.
It's definition, in cryptbase.dll, is an opaque wrapper for the
documented API ProcessPrng. Instead of using RtlGenRandom, switch to
using ProcessPrng, since the former is simply a wrapper for the latter,
there should be no practical change on the user side, other than a minor
change in the DLLs we load.
Updates #53192
Fixes #64413
Change-Id: Ie6891bf97b1d47f5368cccbe92f374dba2c2672a
Reviewed-on: https://go-review.googlesource.com/c/go/+/536235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
(cherry picked from commit 693def151adff1af707d82d28f55dba81ceb08e1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/545355
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Damien Neil [Thu, 9 Nov 2023 17:53:44 +0000 (09:53 -0800)]
[release-branch.go1.21] path/filepath: consider \\?\c: as a volume on Windows
While fixing several bugs in path handling on Windows,
beginning with \\?\.
Prior to #540277, VolumeName considered the first path component
after the \\?\ prefix to be part of the volume name.
After, it considered only the \\? prefix to be the volume name.
Restore the previous behavior.
For #64028.
Fixes #64041.
Change-Id: I6523789e61776342800bd607fb3f29d496257e68
Reviewed-on: https://go-review.googlesource.com/c/go/+/541175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
(cherry picked from commit eda42f7c60adab26ed1a340414c726c4bf46b1f7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/541521 Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Bryan C. Mills [Thu, 26 Oct 2023 16:06:04 +0000 (12:06 -0400)]
[release-branch.go1.21] os: report IO_REPARSE_TAG_DEDUP files as regular in Stat and Lstat
Prior to CL 460595, Lstat reported most reparse points as regular
files. However, reparse points can in general implement unusual
behaviors (consider IO_REPARSE_TAG_AF_UNIX or IO_REPARSE_TAG_LX_CHR),
and Windows allows arbitrary user-defined reparse points, so in
general we must not assume that an unrecognized reparse tag represents
a regular file; in CL 460595, we began marking them as irregular.
As it turns out, the Data Deduplication service on Windows Server runs
an Optimization job that turns regular files into reparse files with
the tag IO_REPARSE_TAG_DEDUP. Those files still behave more-or-less
like regular files, in that they have well-defined sizes and support
random-access reads and writes, so most programs can treat them as
regular files without difficulty. However, they are still reparse
files: as a result, on servers with the Data Deduplication service
enabled, files could arbitrarily change from “regular” to “irregular”
without explicit user intervention.
Since dedup files are converted in the background and otherwise behave
like regular files, this change adds a special case to report DEDUP
reparse points as regular.
Fixes #63764.
Updates #63429.
No test because to my knowledge we don't have any Windows builders
that have the deduplication service enabled, nor do we have a way to
reliably guarantee the existence of an IO_REPARSE_TAG_DEDUP file.
(In theory we could add a builder with the service enabled on a
specific volume, write a test that encodes knowledge of that volume,
and use the GO_BUILDER_NAME environment variable to run that test only
on the specially-configured builders. However, I don't currently have
the bandwidth to reconfigure the builders in this way, and given the
simplicity of the change I think it is unlikely to regress
accidentally.)
Change-Id: I649e7ef0b67e3939a980339ce7ec6a20b31b23a1
Cq-Include-Trybots: luci.golang.try:go1.21-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/538218 Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Keith Randall [Wed, 25 Oct 2023 20:35:13 +0000 (13:35 -0700)]
[release-branch.go1.21] cmd/compile: ensure pointer arithmetic happens after the nil check
Have nil checks return a pointer that is known non-nil. Users of
that pointer can use the result, ensuring that they are ordered
after the nil check itself.
The order dependence goes away after scheduling, when we've fixed
an order. At that point we move uses back to the original pointer
so it doesn't change regalloc any.
This prevents pointer arithmetic on nil from being spilled to the
stack and then observed by a stack scan.
Fixes #63743
Change-Id: I1a5fa4f2e6d9000d672792b4f90dfc1b7b67f6ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/537775 Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
(cherry picked from commit 962ccbef91057f91518443b648e02fc3afe8c764)
Reviewed-on: https://go-review.googlesource.com/c/go/+/538717
Auto-Submit: Heschi Kreinick <heschi@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Dmitri Shuralyov [Thu, 2 Nov 2023 18:36:30 +0000 (14:36 -0400)]
[release-branch.go1.21] syscall: copy rlimit.go's build constraint to rlimit_test.go
Tests in rlimit_test.go exist to test the behavior of automatically
bumping RLIMIT_NOFILE on Unix implemented in rlimit.go (issue #46279),
with darwin-specific behavior split out into rlimit_darwin.go and
the rest left empty in rlimit_stub.go.
Since the behavior happens only on Unix, it doesn't make sense to test
it on other platforms. Copy rlimit.go's 'unix' build constraint to
rlimit_test.go to accomplish that.
Leave out the simplification of the build constraint in rlimit_stub.go
so that this CL remains a test-only fix.
In particular, this fixes a problem where TestOpenFileLimit was
failing in some environments when testing the wasip1/wasm port.
The RLIMIT_NOFILE bumping behavior isn't implemented there, so
the test was testing the environment and not the Go project.
Updates #46279.
For #61116.
Fixes #63994.
Change-Id: Ic993f9cfc021d4cda4fe3d7fed8e2e180f78a2ca
Cq-Include-Trybots: luci.golang.try:go1.21-wasip1-wasm_wasmtime
Reviewed-on: https://go-review.googlesource.com/c/go/+/539435 Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
(cherry picked from commit b7cbcf0c274a0e9f9703468c8ea1d511efe90c5e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/540615 Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Damien Neil [Fri, 1 Sep 2023 18:17:19 +0000 (11:17 -0700)]
[release-branch.go1.21] path/filepath: fix various issues in parsing Windows paths
On Windows, A root local device path is a path which begins with
\\?\ or \??\. A root local device path accesses the DosDevices
object directory, and permits access to any file or device on the
system. For example \??\C:\foo is equivalent to common C:\foo.
The Clean, IsAbs, IsLocal, and VolumeName functions did not
recognize root local device paths beginning with \??\.
Clean could convert a rooted path such as \a\..\??\b into
the root local device path \??\b. It will now convert this
path into .\??\b.
IsAbs now correctly reports paths beginning with \??\
as absolute.
IsLocal now correctly reports paths beginning with \??\
as non-local.
VolumeName now reports the \??\ prefix as a volume name.
Join(`\`, `??`, `b`) could convert a seemingly innocent
sequence of path elements into the root local device path
\??\b. It will now convert this to \.\??\b.
In addition, the IsLocal function did not correctly
detect reserved names in some cases:
- reserved names followed by spaces, such as "COM1 ".
- "COM" or "LPT" followed by a superscript 1, 2, or 3.
IsLocal now correctly reports these names as non-local.
For #63713
Fixes #63715
Fixes CVE-2023-45283
Fixes CVE-2023-45284
Change-Id: I446674a58977adfa54de7267d716ac23ab496c54
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2040691 Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2072596 Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/540276
Auto-Submit: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cherry Mui [Sat, 7 Oct 2023 00:53:27 +0000 (20:53 -0400)]
[release-branch.go1.21] runtime/cgo: avoid taking the address of crosscall2 in code
Currently, set_crosscall2 takes the address of crosscall2 without
using the GOT, which, on some architectures, results in a
PC-relative relocation (e.g. R_AARCH64_ADR_PREL_PG_HI21 on ARM64)
to the crosscall2 symbol. But crosscall2 is dynamically exported,
so the C linker thinks it may bind to a symbol from a different
DSO. Some C linker may not like a PC-relative relocation to such a
symbol. Using a local trampoline to avoid taking the address of a
dynamically exported symbol.
It may be possible to not dynamically export crosscall2. But this
CL is safer for backport. Later we may remove the trampolines
after unexport crosscall2, if they are not needed.
Fixes #63509.
Updates #62556.
Change-Id: Id28457f65ef121d3f87d8189803abc65ed453283
Reviewed-on: https://go-review.googlesource.com/c/go/+/533535
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 872d7181f4084461441787c70ffd1354314987af)
Reviewed-on: https://go-review.googlesource.com/c/go/+/534915 Reviewed-by: David Chase <drchase@google.com>
Matthew Dempsky [Sat, 9 Sep 2023 01:04:31 +0000 (18:04 -0700)]
[release-branch.go1.21] cmd/compile/internal/typecheck: fix closure field naming
When creating the struct type to hold variables captured by a function
literal, we currently reuse the captured variable names as fields.
However, there's no particular reason to do this: these struct types
aren't visible to users, and it adds extra complexity in making sure
fields belong to the correct packages.
Further, it turns out we were getting that subtly wrong. If two
function literals from different packages capture variables with
identical names starting with an uppercase letter (and in the same
order and with corresponding identical types) end up in the same
function (e.g., due to inlining), then we could end up creating
closure struct types that are "different" (i.e., not types.Identical)
yet end up with equal LinkString representations (which violates
LinkString's contract).
The easy fix is to just always use simple, exported, generated field
names in the struct. This should allow further struct reuse across
packages too, and shrink binary sizes slightly.
For #62498.
Fixes #62545.
Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51
Reviewed-on: https://go-review.googlesource.com/c/go/+/527135
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit e3ce3126212115808bc248bdc9ad92c0a46436fe)
Reviewed-on: https://go-review.googlesource.com/c/go/+/534916 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This has caused performance issues in production environments.
MADV_COLLAPSE can go into direct reclaim, but we call it with the heap
lock held. This means that the process could end up stalled fairly
quickly if just one allocating goroutine ends up in the madvise call, at
least until the madvise(MADV_COLLAPSE) call returns. A similar issue
occurred with madvise(MADV_HUGEPAGE), because that could go into direct
reclaim on any page fault for MADV_HUGEPAGE-marked memory.
My understanding was that the calls to madvise(MADV_COLLAPSE) were
fairly rare, and it's "best-effort" nature prevented it from going into
direct reclaim often, but this was wrong. It tends to be fairly
heavyweight even when it doesn't end up in direct reclaim, and it's
almost certainly not worth it.
Disable it until further notice and let the kernel fully dictate
hugepage policy. The updated scavenger policy is still more hugepage
friendly by delaying scavening until hugepages are no longer densely
packed, so we don't lose all that much.
The Sweet benchmarks show a minimal difference. A couple less realistic
benchmarks seem to slow down a bit; they might just be getting unlucky
with what the kernel decides to back with a huge page. Some benchmarks
on the other hand improve. Overall, it's a wash.
Robert Griesemer [Mon, 2 Oct 2023 22:47:08 +0000 (15:47 -0700)]
[release-branch.go1.21] go/types, types2: don't implicitly modify an argument function's type
See the comment in the (very small) fix for a detailed description.
Use the opportunity to introduce a generic clone function which may
be useful elsewhere.
Fixes #63339.
Change-Id: Ic63c6b8bc443011b1a201908254f7d062e1aec71
Reviewed-on: https://go-review.googlesource.com/c/go/+/532157
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/531998
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Than McIntosh [Fri, 29 Sep 2023 18:19:17 +0000 (14:19 -0400)]
[release-branch.go1.21] cmd/link: split text sections for arm 32-bit
This CL is a roll-forward (tweaked slightly) of CL 467715, which
turned on text section splitting for GOARCH=arm. The intent is to
avoid recurrent problems with external linking where there is a
disagreement between the Go linker and the external linker over
whether a given branch will reach. In the past our approach has been
to tweak the reachability calculations slightly to try to work around
potential linker problems, but this hasn't proven to be very robust;
section splitting seems to offer a better long term fix.
Updates #58425.
Fixes #63317.
Change-Id: I7372d41abce84097906a3d0805b6b9c486f345d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/531795 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 1e690409206ff97330b5a91517d453fc5129bab2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/532096
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Dmitri Shuralyov [Tue, 10 Oct 2023 17:37:01 +0000 (13:37 -0400)]
[release-branch.go1.21] all: tidy dependency versioning after release
Done with:
go get golang.org/x/net@internal-branch.go1.21-vendor
go mod tidy
go mod vendor
go generate net/http # zero diff since CL 534235 already did this
For #63417.
For #63427.
For CVE-2023-39325.
Change-Id: Ib258e0d8165760a1082e02c2f4c5ce7d2a3c3c90
Reviewed-on: https://go-review.googlesource.com/c/go/+/534415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Ian Lance Taylor [Wed, 20 Sep 2023 23:16:29 +0000 (16:16 -0700)]
[release-branch.go1.21] cmd/compile: use absolute file name in isCgo check
For #23672
Updates #63211
Fixes #63214
Fixes CVE-2023-39323
Change-Id: I4586a69e1b2560036afec29d53e53cf25e6c7352
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2032884 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit 9b19e751918dd218035811b1ef83a8c2693b864a)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2037958 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/533215 Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Thu, 3 Aug 2023 20:53:52 +0000 (20:53 +0000)]
[release-branch.go1.21] runtime: always lock OS thread in debugcall
Right now debuggers like Delve rely on the new goroutine created to run
a debugcall function to run on the same thread it started on, up until
it hits itself with a SIGINT as part of the debugcall protocol.
That's all well and good, except debugCallWrap1 isn't particularly
careful about not growing the stack. For example, if the new goroutine
happens to have a stale preempt flag, then it's possible a stack growth
will cause a roundtrip into the scheduler, possibly causing the
goroutine to switch to another thread.
Previous attempts to just be more careful around debugCallWrap1 were
helpful, but insufficient. This change takes everything a step further
and always locks the debug call goroutine and the new goroutine it
creates to the OS thread.
For #61732.
Fixes #62509.
Change-Id: I038f3a4df30072833e27e6a5a1ec01806a32891f
Reviewed-on: https://go-review.googlesource.com/c/go/+/515637
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit d9a4b24a1775c4c5baa4ce3005cb5af61346198e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/526576
Cherry Mui [Fri, 8 Sep 2023 16:14:30 +0000 (12:14 -0400)]
[release-branch.go1.21] runtime: increase g0 stack size in non-cgo case
Currently, for non-cgo programs, the g0 stack size is 8 KiB on
most platforms. With PGO which could cause aggressive inlining in
the runtime, the runtime stack frames are larger and could
overflow the 8 KiB g0 stack. Increase it to 16 KiB. This is only
one per OS thread, so it shouldn't increase memory use much.
Updates #62120.
Updates #62489.
Fixes #62537.
Change-Id: I565b154517021f1fd849424dafc3f0f26a755cac
Reviewed-on: https://go-review.googlesource.com/c/go/+/526995 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit c6d550a6683cebb2a11d7fa91823edf7db1d58a5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/527055
Cherry Mui [Mon, 10 Jul 2023 22:50:06 +0000 (18:50 -0400)]
[release-branch.go1.21] cmd/link: suppress -bind_at_load deprecation warning for ld-prime
ld-prime emits a deprecation warning for -bind_at_load. The flag
is needed for plugins to not deadlock (#38824) when linking with
older darwin linker. It is supposedly not needed with newer linker
when chained fixups are used. For now, we always pass it, and
suppress the warning.
Updates #61229.
For #62598.
Change-Id: I4b8a6f864a460c40dc38adbb533f664f7fd5343c
Reviewed-on: https://go-review.googlesource.com/c/go/+/508696 Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 040dbf9c181a0e3ea9f7bd3ebe3f75acdc878aaf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/527817
Cherry Mui [Tue, 12 Sep 2023 19:15:12 +0000 (15:15 -0400)]
[release-branch.go1.21] cmd/link: force old Apple linker in plugin mode
There are some bugs in Apple's new linker that causes plugins to
be built incorrectly. And the bugs probably will not be fixed when
Xcode 15 is released (some time soon). Force old Apple linker to
work around.
Updates #61229.
For #62598.
Change-Id: I01ba5caadec6dc14f8c85dd02f78c1ed2e8b7d4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/527815
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Than McIntosh [Wed, 6 Sep 2023 14:15:37 +0000 (10:15 -0400)]
[release-branch.go1.21] cmd/link: avoid deadcode of global map vars for programs using plugins
If a program imports the plugin package, the mechanisms in place for
detecting and deleting unused global map variables are no longer safe,
since it's possibly for a given global map var to be unreferenced in
the main program but referenced by a plugin. This patch changes the
linker to test for plugin use and to avoid removing any unused global
map variables if the main program could possibly load up a plugin.
Fixes #62505.
Updates #62430.
Change-Id: Ie00b18b681cb0d259e3c859ac947ade5778cd6c8
(cherry picked from commit 660620dd45dc11f2d889add79bedf2dc771c7d04)
Reviewed-on: https://go-review.googlesource.com/c/go/+/526575 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
[release-branch.go1.21] cmd/go: in TestScript/gotoolchain_path, remove the user's PATH
This test checks a behavior of GOTOOLCHAIN when an appropriate
toolchain is found in PATH. That requires it to exclude any suitable
toolchain binaries from the user's $PATH, which may otherwise
interfere.
Fixes #62711.
Updates #62709.
Change-Id: Ie9161e52d33a65be0b5265cb49e9f2bc8473e057
Reviewed-on: https://go-review.googlesource.com/c/go/+/529217
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 30886b1b1ea0ff0282419e4deefdd35a51b2c9fc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/529435
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
In the existing implementation, all /gc/scan/* metrics are
always equal to 0 due to the dependency on gcStatDep not being
set. This leads to gcStatAggregate always containing zeros, and
always reporting 0 for those metrics.
Also, add a test to ensure that /gc/scan/* metrics are not empty.
For #62477.
Fixes #62478.
Change-Id: I67497347d50ed5c3ce1719a18714c062ec938cab
Reviewed-on: https://go-review.googlesource.com/c/go/+/526116 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
[release-branch.go1.21] runtime: ignore SPWrite on innermost traceback frame
Prior to CL 458218, gentraceback ignored the SPWrite function flag on
the innermost frame when doing a precise traceback on the assumption
that precise tracebacks could only be started from the morestack
prologue, and that meant that the innermost function could not have
modified SP yet.
CL 458218 rearranged this logic a bit and unintentionally lost this
particular case. As a result, if traceback starts in an assembly
function that modifies SP (either as a result of stack growth or stack
scanning during a GC preemption), traceback stop at the SPWrite
function and then crash with "traceback did not unwind completely".
Fix this by restoring the earlier special case for when the innermost
frame is SPWrite.
This is a fairly minimal change that should be easy to backport. I
think a more robust change would be to encode this per-PC in the
spdelta table, so it would be clear that we're unwinding from the
morestack prologue and wouldn't rely on a complicated and potentially
fragile set of conditions.
Fixes #62464.
Change-Id: I34f38157631890d33a79d0bd32e32c0fcc2574e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/526100
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Roland Shoemaker [Thu, 3 Aug 2023 19:28:28 +0000 (12:28 -0700)]
[release-branch.go1.21] html/template: properly handle special tags within the script context
The HTML specification has incredibly complex rules for how to handle
"<!--", "<script", and "</script" when they appear within literals in
the script context. Rather than attempting to apply these restrictions
(which require a significantly more complex state machine) we apply
the workaround suggested in section 4.12.1.3 of the HTML specification [1].
More precisely, when "<!--", "<script", and "</script" appear within
literals (strings and regular expressions, ignoring comments since we
already elide their content) we replace the "<" with "\x3C". This avoids
the unintuitive behavior that using these tags within literals can cause,
by simply preventing the rendered content from triggering it. This may
break some correct usages of these tags, but on balance is more likely
to prevent XSS attacks where users are unknowingly either closing or not
closing the script blocks where they think they are.
Thanks to Takeshi Kaneko (GMO Cybersecurity by Ierae, Inc.) for
reporting this issue.
Roland Shoemaker [Thu, 3 Aug 2023 19:24:13 +0000 (12:24 -0700)]
[release-branch.go1.21] html/template: support HTML-like comments in script contexts
Per Appendix B.1.1 of the ECMAScript specification, support HTML-like
comments in script contexts. Also per section 12.5, support hashbang
comments. This brings our parsing in-line with how browsers treat these
comment types.
Thanks to Takeshi Kaneko (GMO Cybersecurity by Ierae, Inc.) for
reporting this issue.
Fixes #62196
Fixes #62396
Fixes CVE-2023-39318
Change-Id: Id512702c5de3ae46cf648e268cb10e1eb392a181
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1976593
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2014618
Reviewed-on: https://go-review.googlesource.com/c/go/+/526096
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
If GOTOOLCHAIN="path" or "auto", the go command uses exec.LookPath to
search for it in order to allow toolchains to refer to local-only
toolchain variants (such as toolchains built from enterprise- or
distro-patched source). However, those toolchains should only be
resolved from $PATH, not relative to the working directory of the
command.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Fixes #62198.
Fixes #62394.
Fixes CVE-2023-39320.
Change-Id: I247c7acea95d737362dd0475e9fc8515430d0fcc
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1996318
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit e41c0a55d45e9a9acbc5d7c1143ea4fff8fb9283)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2014013 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/526095 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
haruyama480 [Fri, 25 Aug 2023 06:14:35 +0000 (15:14 +0900)]
[release-branch.go1.21] net/http: revert "support streaming POST content in wasm"
CL 458395 added support for streaming POST content in Wasm.
Unfortunately, this breaks requests to servers that only support HTTP/1.1.
Revert the change until a suitable fallback or opt-in strategy can be decided.
For #61889.
Fixes #62328.
Change-Id: If53a77e1890132063b39abde867d34515d4ac2af
Reviewed-on: https://go-review.googlesource.com/c/go/+/522955
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/524855
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Commit-Queue: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Nick Ripley [Fri, 4 Aug 2023 21:31:43 +0000 (17:31 -0400)]
[release-branch.go1.21] runtime: restore caller's frame pointer when recovering from panic
When recovering from a panic, restore the caller's frame pointer before
returning control to the caller. Otherwise, if the function proceeds to
run more deferred calls before returning, the deferred functions will
get invalid frame pointers pointing to an address lower in the stack.
This can cause frame pointer unwinding to crash, such as if an execution
trace event is recorded during the deferred call on architectures which
support frame pointer unwinding.
Original CL by Nick Ripley, includes fix from CL 523697, and includes a
test update from CL 524315.
This CL also deviates from the original fix by doing some extra
computation to figure out the fp from the sp, since we don't have the fp
immediately available to us in `recovery` on the Go 1.21 branch, and it
would probably be complicated to plumb that through its caller.
For #61766
Fixes #62046
Change-Id: I5a99ca4f909f6b6e209a330d595d1c99987d4359
Reviewed-on: https://go-review.googlesource.com/c/go/+/523698
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Mon, 7 Aug 2023 19:09:59 +0000 (19:09 +0000)]
[release-branch.go1.21] runtime: avoid MADV_HUGEPAGE for heap memory
Currently the runtime marks all new memory as MADV_HUGEPAGE on Linux and
manages its hugepage eligibility status. Unfortunately, the default
THP behavior on most Linux distros is that MADV_HUGEPAGE blocks while
the kernel eagerly reclaims and compacts memory to allocate a hugepage.
This direct reclaim and compaction is unbounded, and may result in
significant application thread stalls. In really bad cases, this can
exceed 100s of ms or even seconds.
Really all we want is to undo MADV_NOHUGEPAGE marks and let the default
Linux paging behavior take over, but the only way to unmark a region as
MADV_NOHUGEPAGE is to also mark it MADV_HUGEPAGE.
The overall strategy of trying to keep hugepages for the heap unbroken
however is sound. So instead let's use the new shiny MADV_COLLAPSE if it
exists.
MADV_COLLAPSE makes a best-effort synchronous attempt at collapsing the
physical memory backing a memory region into a hugepage. We'll use
MADV_COLLAPSE where we would've used MADV_HUGEPAGE, and stop using
MADV_NOHUGEPAGE altogether.
Because MADV_COLLAPSE is synchronous, it's also important to not
re-collapse huge pages if the huge pages are likely part of some large
allocation. Although in many cases it's advantageous to back these
allocations with hugepages because they're contiguous, eagerly
collapsing every hugepage means having to page in at least part of the
large allocation.
However, because we won't use MADV_NOHUGEPAGE anymore, we'll no longer
handle the fact that khugepaged might come in and back some memory we
returned to the OS with a hugepage. I've come to the conclusion that
this is basically unavoidable without a new madvise flag and that it's
just not a good default. If this change lands, advice about Linux huge
page settings will be added to the GC guide.
Verified that this change doesn't regress Sweet, at least not on my
machine with:
/sys/kernel/mm/transparent_hugepage/enabled [always or madvise]
/sys/kernel/mm/transparent_hugepage/defrag [madvise]
/sys/kernel/mm/transparent_hugepage/khugepaged/max_ptes_none [0 or 511]
Unfortunately, this workaround means that we only get forced hugepages
on Linux 6.1+.
For #61718.
Fixes #62329.
Change-Id: I7f4a7ba397847de29f800a99f9cb66cb2720a533
Reviewed-on: https://go-review.googlesource.com/c/go/+/516795 Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 9f9bb26880388c5bead158e9eca3be4b3a9bd2af)
Reviewed-on: https://go-review.googlesource.com/c/go/+/523655
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Robert Griesemer [Mon, 21 Aug 2023 22:50:18 +0000 (15:50 -0700)]
[release-branch.go1.21] go/types, types2: remove order dependency in inference involving channels
In inexact unification, when a named type matches against an inferred
unnamed type, we change the previously inferred type to the named type.
This preserves the type name and assignability.
We have to do the same thing when encountering a directional channel:
a bidirectional channel can always be assigned to a directional channel
but not the other way around. Thus, if we see a directional channel, we
must choose the directional channel.
This CL extends the previously existing logic for named types to
directional channels and also makes the code conditional on inexact
unification. The latter is an optimization - if unification is exact,
type differences don't exist and updating an already inferred type has
no effect.
Fixes #62205.
Change-Id: I807e3b9f9ab363f9ed848bdb18b2577b1d680ea7
Reviewed-on: https://go-review.googlesource.com/c/go/+/524256
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Bryan C. Mills [Tue, 22 Aug 2023 21:33:50 +0000 (17:33 -0400)]
[release-branch.go1.21] cmd/go: retry ETXTBSY errors when running test binaries
An ETXTBSY error when starting a test binary is almost certainly
caused by the race reported in #22315. That race will resolve quickly
on its own, so we should just retry the command instead of reporting a
spurious failure.
Fixes #62222.
Updates #62221.
Change-Id: I408f3eaa7ab5d7efbc7a2b1c8bea3dbc459fc794
Reviewed-on: https://go-review.googlesource.com/c/go/+/522015
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 4dc2564933146efc411efad16b662589306744d1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/522176
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Damien Neil [Thu, 24 Aug 2023 16:57:58 +0000 (09:57 -0700)]
[release-branch.go1.21] crypto/tls: QUIC: fix panics when processing post-handshake messages
The check for fragmentary post-handshake messages in QUICConn.HandleData
was reversed, resulting in a potential panic when HandleData receives
a partial message.
In addition, HandleData wasn't checking the size of buffered
post-handshake messages. Produce an error when a post-handshake
message is larger than maxHandshake.
TestQUICConnectionState was using an onHandleCryptoData hook
in runTestQUICConnection that was never being called.
(I think it was inadvertently removed at some point while
the CL was in review.) Fix this test while making the hook
more general.
For #62266
Fixes #62290
Change-Id: I210b70634e50beb456ab3977eb11272b8724c241
Reviewed-on: https://go-review.googlesource.com/c/go/+/522595
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Marten Seemann <martenseemann@gmail.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
(cherry picked from commit e92c0f846c54d88f479b1c48f0dbc001d2ff53e9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/523039
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Bryan C. Mills [Fri, 18 Aug 2023 19:38:44 +0000 (15:38 -0400)]
[release-branch.go1.21] cmd/go: find GOROOT using os.Executable when installed to GOROOT/bin/GOOS_GOARCH
When running make.bash in a cross-compiled configuration
(for example, GOARCH different from GOHOSTARCH), cmd/go
is installed to GOROOT/bin/GOOS_GOARCH instead of GOROOT/bin.
That means that we need to look for GOROOT in both ../.. and ../../..,
not just the former.
xuri [Wed, 9 Aug 2023 06:32:12 +0000 (06:32 +0000)]
[release-branch.go1.21] encoding/xml: overriding by empty namespace when no new name declaration
The unmarshal and marshal XML text should be consistent if not modified deserialize variable.
For #61881
Fixes #62051
Change-Id: I475f7b05211b618685597d3ff20b97e3bbeaf8f8
GitHub-Last-Rev: 6831c770c384831798cb1c6dc4674e5d4caa5e3c
GitHub-Pull-Request: golang/go#58401
Reviewed-on: https://go-review.googlesource.com/c/go/+/522316
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Andy Pan [Fri, 18 Aug 2023 05:39:57 +0000 (13:39 +0800)]
[release-branch.go1.21] encoding/gob: prevent panic from index out of range in Decoder.typeString
I believe this bug is introduced by CL 460543 which optimizes the allocations
by changing the type of `idToType` from map to slice, but didn't update the
access code in `Decoder.typeString` that is safe for map but not for slice.
For #62117
Fixes #62154
Change-Id: I0f2e4cc2f34c54dada1f83458ba512a6fde6dcbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/520757
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
(cherry picked from commit ba626ac327f45a6d9d211fddd5b48e321fa0702a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/521156
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Bryan C. Mills [Mon, 21 Aug 2023 16:55:43 +0000 (12:55 -0400)]
[release-branch.go1.21] context: fix synchronization in ExampleAfterFunc_cond
Condition variables are subtle and error-prone, and this example
demonstrates exactly the sorts of problems that they introduce.
Unfortunately, we're stuck with them for the foreseeable future.
As previously implemented, this example was racy: since the callback
passed to context.AfterFunc did not lock the mutex before calling
Broadcast, it was possible for the Broadcast to occur before the
goroutine was parked in the call to Wait, causing in a missed wakeup
resulting in deadlock.
The example also had a more insidious problem: it was not safe for
multiple goroutines to call waitOnCond concurrently, but the whole
point of using a sync.Cond is generally to synchronize concurrent
goroutines. waitOnCond must use Broadcast to ensure that it wakes up
the target goroutine, but the use of Broadcast in this way would
produce spurious wakeups for all of the other goroutines waiting on
the same condition variable. Since waitOnCond did not recheck the
condition in a loop, those spurious wakeups would cause waitOnCond
to spuriously return even if its own ctx was not yet done.
Fixing the aforementioned bugs exposes a final problem, inherent to
the use of condition variables in this way. This one is a performance
problem: for N concurrent calls to waitOnCond, the resulting CPU cost
is at least O(N²). This problem cannot be addressed without either
reintroducing one of the above bugs or abandoning sync.Cond in the
example entirely. Given that this example was already published in Go
1.21, I worry that Go users may think that it is appropriate to use a
sync.Cond in conjunction with context.AfterFunc, so I have chosen to
retain the Cond-based example and document its pitfalls instead of
removing or replacing it entirely.
I described this class of bugs and performance issues — and suggested
some channel-based alternatives — in my GopherCon 2018 talk,
“Rethinking Classical Concurrency Patterns”. The section on condition
variables starts on slide 37. (https://youtu.be/5zXAHh5tJqQ?t=679)
David Chase [Tue, 15 Aug 2023 21:45:50 +0000 (17:45 -0400)]
[release-branch.go1.21] cmd/compile: in expandCalls, move all arg marshalling into call block
For aggregate-typed arguments passed to a call, expandCalls
decomposed them into parts in the same block where the value
was created. This is not necessarily the call block, and in
the case where stores are involved, can change the memory
leaving that block, and getting that right is problematic.
Instead, do all the expanding in the same block as the call,
which avoids the problems of (1) not being able to reorder
loads/stores across a block boundary to conform to memory
order and (2) (incorrectly, not) exposing the new memory to
consumers in other blocks. Putting it all in the same block
as the call allows reordering, and the call creates its own
new memory (which is already dealt with correctly).
Fixes #62057.
Updates #61992.
Change-Id: Icc7918f0d2dd3c480cc7f496cdcd78edeca7f297
Reviewed-on: https://go-review.googlesource.com/c/go/+/519276 Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit e72ecc6a6becab9ca895c0108047db4723394296)
Reviewed-on: https://go-review.googlesource.com/c/go/+/520058
Bryan Mills [Wed, 16 Aug 2023 17:23:10 +0000 (17:23 +0000)]
[release-branch.go1.21] Revert "os: use handle based APIs to read directories on windows"
This reverts CL 452995.
Reason for revert: caused os.File.ReadDir to fail on
filesystems that do not support FILE_ID_BOTH_DIR_INFO.
This is an alternative to a fix-forward change in CL 518196.
Since the original change was mostly a performance improvement,
reverting to the previous implementation seems less risky than
backporting a larger fix.