Roland Shoemaker [Tue, 28 Feb 2023 21:23:43 +0000 (13:23 -0800)]
[release-branch.go1.20] crypto/x509: fix system root tests + darwin intermediate handling
On Windows, replace tests which rely on a root that expired last year.
On Darwin fix an test which wasn't testing the expected behavior, and
fix the behavior which was broken.
Updates #58791
Fixes #58811
Change-Id: I771175b9e123b8bb0e4efdf58cc2bb93aa94fbae
Reviewed-on: https://go-review.googlesource.com/c/go/+/472295
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
(cherry picked from commit bb8f9a6ae66d742cb67b4ad444179905a537de00)
Reviewed-on: https://go-review.googlesource.com/c/go/+/472616
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This test is flaky, apparently due to a typo'd operator in CL 21447
that causes it to compare “same port OR IP” instead of
“same port AND IP”.
If we merely fixed the comparison, the test would hopefully stop being
flaky itself, but we would still be left with another problem:
repeatedly dialing a port that we believe to be unused can interfere
with other tests, which may open the previously-unused port and then
attempt a single Dial and expect it to succeed. Arbitrary other Dial
calls for that port may cause the wrong connection to be accepted,
leading to spurious test failures.
Moreover, the test can be extremely expensive for the amount of data
we hope to get from it, depending on the system's port-reuse
algorithms and dial implementations. It is already scaled back by up
to 1000x on a huge number of platforms due to latency, and may even be
ineffective on those platforms because of the arbitrary 1ms Dial
timeout. And the incremental value from it is quite low, too: it tests
the workaround for what is arguably a bug in the Linux kernel, which
ought to be fixed (and tested) upstream instead of worked around in
every open-source project that dials local ports.
Instead of trying to deflake this test, let's just get rid of it.
Updates #18290.
Fixes #58717.
Change-Id: I8a58b93d67916a33741c9ab29ef99c49c46b32c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/460657
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
(cherry picked from commit e08642cae18460778ba3f7808c91cbf6d9ee9f67)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471155 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Kir Kolyshkin [Thu, 16 Feb 2023 00:47:40 +0000 (16:47 -0800)]
[release-branch.go1.20] syscall: Faccessat: check for CAP_DAC_OVERRIDE on Linux
CL 416115 added using faccessat2(2) from syscall.Faccessat on Linux
(which is the only true way to implement AT_EACCESS flag handing),
if available. If not available, it uses some heuristics to mimic the
kernel behavior, mostly taken from glibc (see CL 126415).
Next, CL 414824 added using the above call (via unix.Eaccess) to
exec.LookPath in order to check if the binary can really be executed.
As a result, in a very specific scenario, described below,
syscall.Faccessat (and thus exec.LookPath) mistakenly tells that the
binary can not be executed, while in reality it can be. This makes
this bug a regression in Go 1.20.
This scenario involves all these conditions:
- no faccessat2 support available (i.e. either Linux kernel < 5.8,
or a seccomp set up to disable faccessat2);
- the current user is not root (i.e. geteuid() != 0);
- CAP_DAC_OVERRIDE capability is set for the current process;
- the file to be executed does not have executable permission
bit set for either the current EUID or EGID;
- the file to be executed have at least one executable bit set.
Unfortunately, this set of conditions was observed in the wild -- a
container run as a non-root user with the binary file owned by root with
executable permission set for a user only [1]. Essentially it means it
is not as rare as it may seem.
Now, CAP_DAC_OVERRIDE essentially makes the kernel bypass most of the
checks, so execve(2) and friends work the same was as for root user,
i.e. if at least one executable bit it set, the permission to execute
is granted (see generic_permission() function in the Linux kernel).
Modify the code to check for CAP_DAC_OVERRIDE and mimic the kernel
behavior for permission checks.
Change-Id: I82a7e757ab3fd3d0193690a65c3b48fee46ff067
Reviewed-on: https://go-review.googlesource.com/c/go/+/468735 Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 031401a7905a38498fc399fc10cd0c1e885f7fc9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/469956
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Roland Shoemaker [Tue, 31 Jan 2023 17:14:16 +0000 (09:14 -0800)]
[release-branch.go1.20] crypto/ecdh: explicitly reject mismatched curves in ECDH
Return an explicit error when PrivateKey.ECDH is called with a PublicKey
which uses a different Curve. Also document this requirement, even
though it is perhaps obvious.
Updates #58131.
Fixes #58498.
Change-Id: I739181a3f1283bed14fb5ee7eb78658b854d28d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/464335 Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
(cherry picked from commit 67d8916d551d22f5376e0be71d3922c9d63eaa6a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471602 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Matthew Dempsky [Tue, 14 Feb 2023 23:13:50 +0000 (15:13 -0800)]
[release-branch.go1.20] cmd/compile/internal/noder: correct positions for synthetic closures
When inlining functions that contain function literals, we need to be
careful about position information. The OCLOSURE node should use the
inline-adjusted position, but the ODCLFUNC and its body should use the
original positions.
However, the same problem can arise with certain generic constructs,
which require the compiler to synthesize function literals to insert
dictionary arguments.
go.dev/cl/425395 fixed the issue with user-written function literals
in a somewhat kludgy way; this CL extends the same solution to
synthetic function literals.
This is all quite subtle and the solutions aren't terribly robust, so
longer term it's probably desirable to revisit how we track inlining
context for positions. But for now, this seems to be the least bad
solution, esp. for backporting to 1.20.
Updates #54625.
Fixes #58531.
Change-Id: Icc43a70dbb11a0e665cbc9e6a64ef274ad8253d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/468415
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
(cherry picked from commit 873c14cec730ee278848f7cc58d2b4d89ab52288)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471677
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Than McIntosh [Fri, 17 Feb 2023 19:51:05 +0000 (14:51 -0500)]
[release-branch.go1.20] cmd/link: better fix for arm32 trampgen problem with duff routines
This patch provides a fix for a problem linking large arm32 binaries
with external linking, specifically R_CALLARM relocations against
runtime.duff* routines being flagged by the external linker as not
reaching.
What appears to be happening in the bug in question is that the Go
linker and the external linker are using slightly different recipes to
decide whether a given R_CALLARM relocation will "fit" (e.g. will not
require a trampoline). The Go linker is taking into account the addend
on the call reloc (which for calls to runtime.duffcopy or
runtime.duffzero is nonzero), whereas the external linker appears to
be ignoring the addend.
Let's say that the distance between ABC (start address of
runtime.duffcopy) and XYZ (start of MyFunc) is just over the
architected 24-bit maximum displacement for an R_CALLARM (let's say
that ABC-XYZ is just over the architected limit by some small value,
say 36). Because we're calling into runtime.duffcopy at offset 848,
however, the relocation does in fact fit, but if the external linker
isn't taking into account the addend (assuming that all calls target
the first instruction of the called routine), then we'll get a
"doesn't fit" error from the linker.
To work around this problem, revise the ARM trampoline generation code
in the Go linker that computes the trampoline threshold to ignore the
addend on R_CALLARM relocations, so as to harmonize the two linkers.
Fixes #58503.
Updates #58428.
Updates #58425.
Change-Id: I56e580c05b7b47bbe8edf5532a1770bbd700fbe5
Reviewed-on: https://go-review.googlesource.com/c/go/+/469275
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
(cherry picked from commit 0b5affb193ed559f2cb646e0324827f261e2e767)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471597
Unlike the rest of nistec, the P-256 assembly doesn't use complete
addition formulas, meaning that p256PointAdd[Affine]Asm won't return the
correct value if the two inputs are equal.
This was (undocumentedly) ignored in the scalar multiplication loops
because as long as the input point is not the identity and the scalar is
lower than the order of the group, the addition inputs can't be the same.
As part of the math/big rewrite, we went however from always reducing
the scalar to only checking its length, under the incorrect assumption
that the scalar multiplication loop didn't require reduction.
Added a reduction, and while at it added it in P256OrdInverse, too, to
enforce a universal reduction invariant on p256OrdElement values.
Note that if the input point is the infinity, the code currently still
relies on undefined behavior, but that's easily tested to behave
acceptably, and will be addressed in a future CL.
Updates #58647
Fixes #58720
Fixes CVE-2023-24532
(Filed with the "safe APIs like complete addition formulas are good" dept.)
Change-Id: I7b2c75238440e6852be2710fad66ff1fdc4e2b24
Reviewed-on: https://go-review.googlesource.com/c/go/+/471255
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 203e59ad41bd288e1d92b6f617c2f55e70d3c8e3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471695 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Than McIntosh [Wed, 8 Feb 2023 20:06:08 +0000 (15:06 -0500)]
[release-branch.go1.20] cmd/internal/cov: fix misuse of bufio.Reader.Read in read helper
Fix a misuse of bufio.Reader.Read in the helper class
cmd/internal/cov.MReader; the MReader method in question should have
been using io.ReadFull (passing the bufio.Reader) instead of directly
calling Read.
Using the Read method instead of io.ReadFull will result in a "short"
read when processing a specific subset of counter data files, e.g.
those that are short enough to not trigger the mmap-based scheme we
use for larger files, but also with a large args section (something
large enough to exceed the default 4k buffer size used by
bufio.Reader).
Along the way, add some additional defered Close() calls for files
opened by the CovDataReader.visitPod, to enure we don't leave any open
file descriptor following a call to CovDataReader.Visit.
Fixes #58427.
Updates #58411.
Change-Id: Iea48dc25c0081be1ade29f3a633df02a681fd941
Reviewed-on: https://go-review.googlesource.com/c/go/+/466677
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit a7fe9ada10c02a7ea61b2909ef7db151d290073f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/468536 Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Wed, 4 Jan 2023 05:20:58 +0000 (05:20 +0000)]
[release-branch.go1.20] runtime: check for overflow in sweep assist
The sweep assist computation is intentionally racy for performance,
since the specifics of sweep assist aren't super sensitive to error.
However, if overflow occurs when computing the live heap delta, we can
end up with a massive sweep target that causes the sweep assist to sweep
until sweep termination, causing severe latency issues. In fact, because
heapLive doesn't always increase monotonically then anything that
flushes mcaches will cause _all_ allocating goroutines to inevitably get
stuck in sweeping.
Consider the following scenario:
1. SetGCPercent is called, updating sweepHeapLiveBasis to heapLive.
2. Very shortly after, ReadMemStats is called, flushing mcaches and
decreasing heapLive below the value sweepHeapLiveBasis was set to.
3. Every allocating goroutine goes to refill its mcache, calls into
deductSweepCredit for sweep assist, and gets stuck sweeping until
the sweep phase ends.
Fix this by just checking for overflow in the delta live heap calculation
and if it would overflow, pick a small delta live heap. This probably
means that no sweeping will happen at all, but that's OK. This is a
transient state and the runtime will recover as soon as heapLive
increases again.
Note that deductSweepCredit doesn't check overflow on other operations
but that's OK: those operations are signed and extremely unlikely to
overflow. The subtraction targeted by this CL is only a problem because
it's unsigned. An alternative fix would be to make the operation signed,
but being explicit about the overflow situation seems worthwhile.
For #57523.
Fixes #58536.
Change-Id: Ib18f71f53468e913548aac6e5358830c72ef0215
Reviewed-on: https://go-review.googlesource.com/c/go/+/468375 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Update golang.org/x/net to the tip of internal-branch.go1.20-vendor to
include CL 468336.
The contents of that CL were already merged into this branch in CL
468122, so this CL just brings go.mod back in line to matching the
actual vendored content.
For #58356
For #57855
Change-Id: I6ee9483077630c11c725927f38f6b69a784106db
Reviewed-on: https://go-review.googlesource.com/c/go/+/468302
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Roland Shoemaker [Wed, 14 Dec 2022 17:43:16 +0000 (09:43 -0800)]
[release-branch.go1.20] crypto/tls: replace all usages of BytesOrPanic
Message marshalling makes use of BytesOrPanic a lot, under the
assumption that it will never panic. This assumption was incorrect, and
specifically crafted handshakes could trigger panics. Rather than just
surgically replacing the usages of BytesOrPanic in paths that could
panic, replace all usages of it with proper error returns in case there
are other ways of triggering panics which we didn't find.
In one specific case, the tree routed by expandLabel, we replace the
usage of BytesOrPanic, but retain a panic. This function already
explicitly panicked elsewhere, and returning an error from it becomes
rather painful because it requires changing a large number of APIs.
The marshalling is unlikely to ever panic, as the inputs are all either
fixed length, or already limited to the sizes required. If it were to
panic, it'd likely only be during development. A close inspection shows
no paths for a user to cause a panic currently.
This patches ends up being rather large, since it requires routing
errors back through functions which previously had no error returns.
Where possible I've tried to use helpers that reduce the verbosity
of frequently repeated stanzas, and to make the diffs as minimal as
possible.
Thanks to Marten Seemann for reporting this issue.
Updates #58001
Fixes #58359
Fixes CVE-2022-41724
Change-Id: Ieb55867ef0a3e1e867b33f09421932510cb58851
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1679436 Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 1d4e6ca9454f6cf81d30c5361146fb5988f1b5f6)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728205 Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468121 Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Damien Neil [Wed, 25 Jan 2023 17:27:01 +0000 (09:27 -0800)]
[release-branch.go1.20] mime/multipart: limit memory/inode consumption of ReadForm
Reader.ReadForm is documented as storing "up to maxMemory bytes + 10MB"
in memory. Parsed forms can consume substantially more memory than
this limit, since ReadForm does not account for map entry overhead
and MIME headers.
In addition, while the amount of disk memory consumed by ReadForm can
be constrained by limiting the size of the parsed input, ReadForm will
create one temporary file per form part stored on disk, potentially
consuming a large number of inodes.
Update ReadForm's memory accounting to include part names,
MIME headers, and map entry overhead.
Update ReadForm to store all on-disk file parts in a single
temporary file.
Files returned by FileHeader.Open are documented as having a concrete
type of *os.File when a file is stored on disk. The change to use a
single temporary file for all parts means that this is no longer the
case when a form contains more than a single file part stored on disk.
The previous behavior of storing each file part in a separate disk
file may be reenabled with GODEBUG=multipartfiles=distinct.
Update Reader.NextPart and Reader.NextRawPart to set a 10MiB cap
on the size of MIME headers.
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
Updates #58006
Fixes #58363
Fixes CVE-2022-41725
Change-Id: Ibd780a6c4c83ac8bcfd3cbe344f042e9940f2eab
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1714276 Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
(cherry picked from commit 7d0da0029bfbe3228cc5216ced8c7b3184eb517d)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728950 Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468120
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Damien Neil [Tue, 13 Dec 2022 00:43:37 +0000 (16:43 -0800)]
[release-branch.go1.20] path/filepath: do not Clean("a/../c:/b") into c:\b on Windows
Do not permit Clean to convert a relative path into one starting
with a drive reference. This change causes Clean to insert a .
path element at the start of a path when the original path does not
start with a volume name, and the first path element would contain
a colon.
This may introduce a spurious but harmless . path element under
some circumstances. For example, Clean("a/../b:/../c") becomes `.\c`.
This reverts CL 401595, since the change here supersedes the one
in that CL.
Thanks to RyotaK (https://twitter.com/ryotkak) for reporting this issue.
Updates #57274
Fixes #57276
Fixes CVE-2022-41722
Change-Id: I837446285a03aa74c79d7642720e01f354c2ca17
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1675249 Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
(cherry picked from commit 8ca37f4813ef2f64600c92b83f17c9f3ca6c03a5)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728944
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468119 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Frederic Branczyk [Wed, 8 Feb 2023 17:59:27 +0000 (17:59 +0000)]
[release-branch.go1.20] cmd/compile/internal/pgo: fix hard-coded PGO sample data position
This patch detects at which index position profiling samples that have
the value-type samples count are, instead of the previously hard-coded
position of index 1. Runtime generated profiles always generate CPU
profiling data with the 0 index being CPU nanoseconds, and samples count
at index 1, which is why this previously hasn't come up.
This is a redo of CL 465135, now allowing empty profiles. Note that
preprocessProfileGraph will already cause pgo.New to return nil for
empty profiles.
Bryan C. Mills [Wed, 8 Feb 2023 19:36:47 +0000 (14:36 -0500)]
[release-branch.go1.20] cmd/go/internal/test: refresh flagdefs.go and fix test
The tests for cmd/go/internal/test were not running at all due to a
missed call to m.Run in TestMain. That masked a missing vet analyzer
("timeformat") and a missed update to the generator script in
CL 355452.
Fixes #58421.
Updates #58415.
Change-Id: I7b0315952967ca07a866cdaa5903478b2873eb7a
Reviewed-on: https://go-review.googlesource.com/c/go/+/466635
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
(cherry picked from commit 910f041ff0cdf90dbcd3bd22a272b9b7205a5add)
Reviewed-on: https://go-review.googlesource.com/c/go/+/466855
Bryan C. Mills [Thu, 9 Feb 2023 21:37:51 +0000 (16:37 -0500)]
[release-branch.go1.20] cmd/go: remove tests that assume lack of new versions of external modules
In general it seems ok to assume that an open-source module that did
exist will continue to do so — after all, users of open-source modules
already do that all the time. However, we should not assume that those
modules do not publish new versions — that's really up to their
maintainers to decide.
Two existing tests did make that assumption for the module
gopkg.in/natefinch/lumberjack.v2. Let's remove those two tests.
If we need to replace them at some point, we can replace them with
hermetic test-only modules (#54503) or perhaps modules owned by the Go
project.
Russ Cox [Wed, 8 Feb 2023 19:02:55 +0000 (14:02 -0500)]
[release-branch.go1.20] runtime: skip darwin osinit_hack on ios
Darwin needs the osinit_hack call to fix some bugs in the Apple libc
that surface when Go programs call exec. On iOS, the functions that
osinit_hack uses are not available, so signing fails. But on iOS exec
is also unavailable, so the hack is not needed. Disable it there,
which makes signing work again.
Fixes #58323.
Fixes #58419.
Change-Id: I3f1472f852bb36c06854fe1f14aa27ad450c5945
Reviewed-on: https://go-review.googlesource.com/c/go/+/466516
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Dave Anderson <danderson@tailscale.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/467316
Russ Cox [Wed, 1 Feb 2023 17:15:08 +0000 (12:15 -0500)]
[release-branch.go1.20] cmd/link: keep go.buildinfo even with --gc-sections
If you use an external linker with --gc-sections, nothing refers
to .go.buildinfo, so the section is deleted, which in turns makes
'go version' fail on the binary. It is important for vulnerability
scanning and the like to be able to run 'go version' on any binary.
Fix this by inserting a reference to .go.buildinfo from the rodata
section, which will not be GC'ed.
In the types1 universe under the unified frontend, we never need to
worry about type parameter constraints, so we only see pure
interfaces. However, we might still see interfaces that contain union
types, because of interfaces like "interface{ any | int }" (equivalent
to just "any").
We can handle these without needing to actually represent type unions
within types1 by simply mapping any union to "any".
Fixes #58413.
Change-Id: I5e4efcf0339edbb01f4035c54fb6fb1f9ddc0c65
Reviewed-on: https://go-review.googlesource.com/c/go/+/458619
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit a7de684e1b6f460aae7d4dbf2568cb21130ec520)
Reviewed-on: https://go-review.googlesource.com/c/go/+/466435 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: David Chase <drchase@google.com>
Cuong Manh Le [Sun, 5 Feb 2023 07:33:32 +0000 (14:33 +0700)]
[release-branch.go1.20] cmd/compile: fix inline static init arguments substitued tree
Blank node must be ignored when building arguments substitued tree.
Otherwise, it could be used to replace other blank node in left hand
side of an assignment, causing an invalid IR node.
After staticAssignInlinedCall, the assignment becomes:
var _ = S1{s2: S2{}.Make()}
and the arg substitued tree is "map[*ir.Name]ir.Node{_: S1{}}". Now,
when doing static assignment, if there is any assignment to blank node,
for example:
_ := S2{}
That blank node will be replaced with "S1{}":
S1{} := S2{}
So constructing an invalid IR which causes the ICE.
Fixes #58335
Change-Id: I21b48357f669a7e02a7eb4325246aadc31f78fb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/465098
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/466275 Reviewed-by: Than McIntosh <thanm@google.com>
Cuong Manh Le [Sat, 4 Feb 2023 05:00:20 +0000 (12:00 +0700)]
[release-branch.go1.20] cmd/compile: remove constant arithmetic overflows during typecheck
Since go1.19, these errors are already reported by types2 for any user's
Go code. Compiler generated code, which looks like constant expression
should be evaluated as non-constant semantic, which allows overflows.
Fixes #58319
Change-Id: I6f0049a69bdb0a8d0d7a0db49c7badaa92598ea2
Reviewed-on: https://go-review.googlesource.com/c/go/+/466676 Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Than McIntosh [Fri, 20 Jan 2023 19:03:43 +0000 (14:03 -0500)]
[release-branch.go1.20] internal/coverage/decodemeta: fix coding error in func literal handling
Fix a coding error in coverage meta-data decoding in the method
decodemeta.CoverageMetaDataDecoder.ReadFunc. The code was not
unconditionally assigning the "function literal" field of the
coverage.FuncDesc object passed in, resulting in bad values depending
on what the state of the field happened to be in the object.
Fixes #57942.
Change-Id: I6dfd7d7f7af6004f05c622f9a7116e9f6018cf4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/462955
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 620399ef0d1390a8f7e7061a45d5304ed087889a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/463418 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Than McIntosh [Thu, 19 Jan 2023 15:16:07 +0000 (10:16 -0500)]
[release-branch.go1.20] runtime/coverage: avoid non-test coverage profiles in test report helper
When walking through the set of coverage data files generated from a
"go test -cover" run, it's possible to encounter pods (clumps of data
files) that were generated by a run from an instrumented Go tool (for
example, cmd/compile). Add a guard to the test reporting code to
ensure that it only processes files created by the currently running
test.
Fixes #57924.
Change-Id: I1bb7dce88305e1088162e3cb1df628486ecee1c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/462756 Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
(cherry picked from commit cf70d37967b8447af8305e02ef534c4c5f42d49c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/463417
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Changkun Ou [Wed, 28 Dec 2022 14:04:58 +0000 (15:04 +0100)]
[release-branch.go1.20] sync: document memory model for Swap/CompareAnd{Swap,Delete} in Map
CL 381316 documented the memory model of Map's APIs. However, the newly
introduced Swap, CompareAndSwap, and CompareAndDelete are missing from
this documentation as CL 399094 did not add this info.
This CL specifies the defined read/write operations of the new Map APIs.
For #51972
Change-Id: I519a04040a0b429a3f978823a183cd62e42c90ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/459715
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Changkun Ou <mail@changkun.de>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit f07910bd577f73b81e4f7117c7cfdf9cf7579028)
Reviewed-on: https://go-review.googlesource.com/c/go/+/463416
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Changkun Ou <mail@changkun.de>
Joe Tsai [Wed, 18 Jan 2023 18:22:54 +0000 (13:22 -0500)]
[release-branch.go1.20] time: revert strict parsing of RFC 3339
CL 444277 fixed Time.UnmarshalText and Time.UnmarshalJSON to properly
unmarshal timestamps according to RFC 3339 instead of according
to Go's bespoke time syntax that is a superset of RFC 3339.
However, this change seems to have broken an AWS S3 unit test
that relies on parsing timestamps with single digit hours.
It is unclear whether S3 emits these timestamps in production or
whether this is simply a testing artifact that has been cargo culted
across many code bases. Either way, disable strict parsing for now
and re-enable later with better GODEBUG support.
Updates #54580
Change-Id: Icced2c7f9a6b2fc06bbd9c7e90f90edce24c2306
Reviewed-on: https://go-review.googlesource.com/c/go/+/462286 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/462675 Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Matthew Dempsky [Tue, 17 Jan 2023 18:59:25 +0000 (10:59 -0800)]
[release-branch.go1.20] cmd/compile: fix static init inlining for hidden node fields
Unified IR added several new IR fields for holding *runtime._type
expressions. To avoid throwing off any frontend semantics
(particularly inlining cost heuristics), they were marked as
`mknode:"-"` so that code wouldn't visit them.
Unfortunately, this has a bad interaction with the static init
inlining optimization, because the latter relies on ir.EditChildren to
substitute all parameters. This potentially includes dictionary
parameters, which can appear within the new RType fields.
This CL adds a new ir.EditChildrenWithHidden function that also edits
these fields, and switches staticinit to use it. Longer term, we
should unhide the RType fields so that ir.EditChildren visits them
normally, but that's scarier so late in the release cycle.
Updates #57778.
Updates #57854.
Change-Id: I98c1e8cf366156dc0c81a0cb79029cc5e59c476f
Reviewed-on: https://go-review.googlesource.com/c/go/+/461686
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
(cherry picked from commit 9f2fbedf010d59c3ecaa8c25b07a5f68fcb2e3d5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/462535
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
We need to avoid nospill registers at this point in regalloc.
Make sure that we don't restrict our register set to avoid registers
desired by other instructions, if the resulting set includes only
nospill registers.
Fixes #57846
Change-Id: I05478e4513c484755dc2e8621d73dac868e45a27
Reviewed-on: https://go-review.googlesource.com/c/go/+/461685 Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
fangguizhen [Tue, 17 Jan 2023 16:37:42 +0000 (16:37 +0000)]
strings: remove redundant symbols
Change-Id: Ie3fe0274288d6cb6303acdcec1340c480e5c0b20
GitHub-Last-Rev: ce9d44619e970b1319fbccf3aace1ddf719bcec1
GitHub-Pull-Request: golang/go#57848
Reviewed-on: https://go-review.googlesource.com/c/go/+/462277
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Than McIntosh [Fri, 13 Jan 2023 21:46:49 +0000 (16:46 -0500)]
cmd/go: include coverage build flags for "go list"
This patch ensures that the go command's "list" subcommand accepts
coverage-related build options, which were incorrectly left out when
"go build -cover" was rolled out. This is needed in order to do things
like check the staleness of an installed cover-instrumented target.
Filippo Valsorda [Wed, 4 Jan 2023 20:51:50 +0000 (21:51 +0100)]
crypto/x509: clarify that CheckSignatureFrom and CheckSignature are low-level APIs
In particular, CheckSignatureFrom just can't check the path length
limit, because it might be enforced above the parent.
We don't need to document the supported signature algorithms for
CheckSignatureFrom, since we document at the constants in what contexts
they are allowed and not. That does leave CheckSignature ambiguous,
though, because that function doesn't have an explicit context.
Russ Cox [Sat, 14 Jan 2023 20:08:49 +0000 (15:08 -0500)]
cmd/go: document GODEBUG=installgoroot=all
At the moment the only documentation is the release notes,
but everything mentioned in the release notes should have
proper documentation separate from them.
Russ Cox [Tue, 17 Jan 2023 14:21:43 +0000 (09:21 -0500)]
doc/go1.20: remove mention of arena goexperiment
The arena goexperiment contains code used inside Google in very
limited use cases that we will maintain, but the discussion on #51317
identified serious problems with the very idea of adding arenas to the
standard library. In particular the concept tends to infect many other
APIs in the name of efficiency, a bit like sync.Pool except more
publicly visible.
It is unclear when, if ever, we will pick up the idea and try to push
it forward into a public API, but it's not going to happen any time
soon, and we don't want users to start depending on it: it's a true
experiment and may be changed or deleted without warning.
The arena text in the release notes makes them seem more official
and supported than they really are, and we've already seen a couple
blog posts based on that erroneous belief. Delete the text to try to
set expectations better.
Archana R [Wed, 11 Jan 2023 17:45:28 +0000 (11:45 -0600)]
runtime: fix performance regression in morestack_noctxt on ppc64
In the fix for 54332 the MOVD R1, R1 instruction was added to
morestack_noctxt function to set the SPWRITE bit. However, the
instruction MOVD R1, R1 results in or r1,r1,r1 which is a special
instruction on ppc64 architecture as it changes the thread priority
and can negatively impact performance in some cases.
More details on such similar nops can be found in Power ISA v3.1
Book II on Power ISA Virtual Environment architecture in the chapter
on Program Priority Registers and Or instructions.
Replacing this by OR R0, R1 has the same affect on setting SPWRITE as
needed by the first fix but does not affect thread priority and
hence does not cause the degradation in performance
Carlos Amedee [Wed, 11 Jan 2023 23:01:17 +0000 (18:01 -0500)]
[release-branch.go1.20] all: merge master (245e95d) into release-branch.go1.20
Merge List:
+ 2023-01-11 245e95dfab go/types, types2: don't look up fields or methods when expecting a type
+ 2023-01-11 18625d9bec runtime: fix incorrect comment
+ 2023-01-11 6ad27161f8 cmd/compile: better error message for when a type is in a constraint but not the type set
+ 2023-01-10 76d39ae349 cmd/link, runtime: Apple libc atfork workaround take 3
+ 2023-01-10 0a0de0fc42 runtime: revert use of __fork to work around Apple atfork bugs
+ 2023-01-10 82f09b75ca os/exec: avoid leaking an exec.Cmd in TestWaitInterrupt
+ 2023-01-09 0202ad0b3a cmd/compile: prevent IsNewObject from taking quadratic time
+ 2023-01-09 64519baf38 cmd/compile/internal/pgo: add hint to missing start_line error
+ 2023-01-09 376076f3c6 runtime: skip TestCgoPprofCallback in short mode, don't run in parallel
+ 2023-01-09 0bbd67e52f runtime/pprof: document possibility of empty stacks
+ 2023-01-09 d9f23cfe78 runtime/pprof: improve output of TestLabelSystemstack
+ 2023-01-09 8232a09e3e sync/atomic: fix the note of atomic.Store
+ 2023-01-09 841c3eb166 all: fix typos in go file comments
+ 2023-01-06 f721fa3be9 syscall: skip TestUseCgroupFD if cgroupfs not mounted
+ 2023-01-06 76ec919237 net: fix typo in hosts.go
+ 2023-01-06 660d4815ea cmd/compile: describe how Go maps to wasm implementation
+ 2023-01-05 119f679a3b crypto/tls: fix typo in cacheEntry godoc
+ 2023-01-05 d50ea217f6 cmd/cover: fix problems with "go test -covermode=atomic sync/atomic"
+ 2023-01-04 bae7d772e8 doc/go1.20: fix links to new strings functions
+ 2023-01-04 4e7c838483 crypto/internal/boring: add dev.boringcrypto README.md text
+ 2023-01-04 46e3d9d12a cmd/compile: use "satisfies" (not "implements") for constraint errors
+ 2023-01-04 79cdecc852 cmd/gofmt: fix a typo in a comment
+ 2023-01-03 9955a7e9bb README.vendor: minor updates
+ 2023-01-03 d03231d9ce doc/go1.20: fix http.ResponseController example
+ 2023-01-03 cdc73f0679 .github: suggest using private browsing in pkgsite template
Robert Findley [Wed, 11 Jan 2023 19:41:03 +0000 (14:41 -0500)]
go/types, types2: don't look up fields or methods when expecting a type
As we have seen many times, the type checker must be careful to avoid
accessing named type information before the type is fully set up. We
need a more systematic solution to this problem, but for now avoid one
case that causes a crash: checking a selector expression on an
incomplete type when a type expression is expected.
For golang/go#57522
Change-Id: I7ed31b859cca263276e3a0647d1f1b49670023a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/461577
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Robert Griesemer [Wed, 4 Jan 2023 01:05:53 +0000 (17:05 -0800)]
cmd/compile: better error message for when a type is in a constraint but not the type set
While at it, also remove the word "constraint" in the detail explanation
of an unsatisfied constraint.
Fixes #57500.
Change-Id: I55dae1694de2cfdb434aeba9d4a3530af7aca8f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/460455 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Russ Cox [Wed, 4 Jan 2023 14:21:14 +0000 (09:21 -0500)]
cmd/link, runtime: Apple libc atfork workaround take 3
CL 451735 worked around bugs in Apple's atfork handlers by calling
notify_is_valid_token and xpc_atfork_child at startup, so that init
code that wouldn't be safe in the child process would be warmed up in
the parent process instead, but xpc_atfork_child broke use of the xpc
library in Go programs, and xpc is internally used by various macOS
frameworks (#57263).
CL 459175 reverted that change, and then CL 459176 tried a new
approach: use __fork, which doesn't call any of the atfork handlers at all.
That worked, but an Apple engineer reviewing the change in private
email suggests that since __fork is not public API, it should be avoided.
The same engineer (with access to the source code for the xpc library)
suggests that the breakage in #57263 is caused by xpc_atfork_child
marking the library as unusable, expecting an imminent call to exec,
and that calling xpc_date_create_from_current instead would do the
necessary initialization without marking xpc as unusable.
CL 460475 reverted that change, to prepare for this one.
This CL goes back to the original “call functions to warm things up”
approach, replacing xpc_atfork_child with xpc_date_create_from_current.
The CL also updates cmd/link to use OS and SDK version 10.13.0 for
x86 macOS binaries, up from 10.9.0, also suggested by the Apple engineer.
Combined with the two warmup calls, this makes the fork hangs go away.
The minimum macOS version has been 10.13 High Sierra since Go 1.17,
so there should be no problem with writing that in the binaries too.
Russ Cox [Wed, 4 Jan 2023 14:18:02 +0000 (09:18 -0500)]
runtime: revert use of __fork to work around Apple atfork bugs
An Apple engineer suggests that since __fork is not public API,
it would be better to use a different fix. With the benefit of source code,
they suggest using xpc_date_create_from_current instead of
xpc_atfork_child. The latter sets some flags that disable certain
functionality for the remainder of the process lifetime (expecting exec),
while the former should do the necessary setup.
Reverting the __fork fix in order to prepare a clean fix based
on CL 451735 using xpc_date_create_from_current.
Bryan C. Mills [Tue, 10 Jan 2023 16:29:25 +0000 (11:29 -0500)]
os/exec: avoid leaking an exec.Cmd in TestWaitInterrupt
In CL 436655 I added a GODEBUG setting to this test process to verify
that Wait is eventually called for every exec.Cmd before it becomes
unreachable. However, the cmdHang test helpers in
TestWaitInterrupt/Exit-hang and TestWaitInterrupt/SIGKILL-hang
intentially leak a subprocess in order to simulate a leaky third-party
program, as Go users might encounter in practical use.
To avoid tripping over the leak check, we call Wait on the leaked
subprocess in a background goroutine. Since we expect the process
running cmdHang to exit before its subprocess does, the call to Wait
should have no effect beyond suppressing the leak check.
Keith Randall [Mon, 9 Jan 2023 17:49:32 +0000 (09:49 -0800)]
cmd/compile: prevent IsNewObject from taking quadratic time
As part of IsNewObject, we need to go from the SelectN[0] use of
a call to the SelectN[1] use of a call. The current code does this
by just looking through the block. If the block is very large,
this ends up taking quadratic time.
Instead, prepopulate a map from call -> SelectN[1] user of that call.
That lets us find the SelectN[1] user in constant time.
Fixes #57657
Change-Id: Ie2e0b660af5c080314f4f17ba2838510a1147f9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/461080
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Michael Pratt [Mon, 9 Jan 2023 18:59:54 +0000 (13:59 -0500)]
cmd/compile/internal/pgo: add hint to missing start_line error
Profiles only began adding Function.start_line in 1.20. If it is
missing, add a hint to the error message that they may need to profile a
build of the application built with a newer version of the toolchain.
Technically profiles are not required to come from Go itself (e.g., they
could be converted from perf), but in practice they most likely are.
Fixes #57674.
Change-Id: I87eca126d3fed0cff94bbb8dd748bd4652f88b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/461195
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Ian Lance Taylor [Thu, 5 Jan 2023 02:03:46 +0000 (18:03 -0800)]
runtime: skip TestCgoPprofCallback in short mode, don't run in parallel
Fixes #54778
Change-Id: If9aef0c06b993ef2aedbeea9452297ee9f11fa06
Reviewed-on: https://go-review.googlesource.com/c/go/+/460461 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Austin Clements [Thu, 5 Jan 2023 14:08:25 +0000 (09:08 -0500)]
runtime/pprof: document possibility of empty stacks
I spent quite a while determining the cause of empty stacks in
profiles and reasoning out why this is okay. There isn't a great place
to record this knowledge, but a documentation comment on
appendLocsForStack is better than nothing.
Updates #51550.
Change-Id: I2eefc6ea31f1af885885c3d96199319f45edb4ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/460695 Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Austin Clements [Wed, 4 Jan 2023 17:31:23 +0000 (12:31 -0500)]
runtime/pprof: improve output of TestLabelSystemstack
The current output of TestLabelSystemstack is a bit cryptic. This CL
improves various messages and hopefully simplifies the logic in the
test.
Simplifying the logic leads to three changes in possible outcomes,
which I verified by running the logic before and after this change
through all 2^4 possibilities (https://go.dev/play/p/bnfb-OQCT4j):
1. If a sample both must be labeled and must not be labeled, the test
now reports that explicitly rather than giving other confusing output.
2. If a sample must not be labeled but is, the current logic will
print two identical error messages. The new logic prints only one.
3. If the test finds no frames at all that it recognizes, but the
sample is labeled, it will currently print a confusing "Sample labeled
got true want false" message. The new logic prints nothing. We've seen
this triggered by empty stacks in profiles.
Fixes #51550. This bug was caused by case 3 above, where it was
triggered by a profile label on an empty stack. It's valid for empty
stacks to appear in a profile if we sample a goroutine just as it's
exiting (and that goroutine may have a profile label), so the test
shouldn't fail in this case.
Change-Id: I1593ec4ac33eced5bb89572a3ba7623e56f2fb3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/460516
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I56281eda3b178968fbf104de1f71316c1feac64f
GitHub-Last-Rev: e91c7cee340fadfa32b0c1773e4e5cd1ca567638
GitHub-Pull-Request: golang/go#57669
Reviewed-on: https://go-review.googlesource.com/c/go/+/460767
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Shengjing Zhu [Thu, 5 Jan 2023 07:00:25 +0000 (15:00 +0800)]
syscall: skip TestUseCgroupFD if cgroupfs not mounted
When building in schroot (for building Debian package),
the cgroupfs is not mounted in schroot (only sysfs is mounted).
Change-Id: Ieddac7b2f334d58d832f5d84d81af812badf3720
Reviewed-on: https://go-review.googlesource.com/c/go/+/460539
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Change-Id: I656ea210d8ef6eaa85245cb8f463f6b4fd67e1a2
GitHub-Last-Rev: 5a93045add2f2a6885b46377c784f665ab465cfd
GitHub-Pull-Request: golang/go#57633
Reviewed-on: https://go-review.googlesource.com/c/go/+/460756 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Than McIntosh [Thu, 22 Dec 2022 19:10:12 +0000 (14:10 -0500)]
cmd/cover: fix problems with "go test -covermode=atomic sync/atomic"
This patch fixes an elderly bug with "go test -covermode=atomic
sync/atomic". Change the cover tool to avoid adding an import of
sync/atomic when processing "sync/atomic" itself in atomic mode;
instead make direct calls to AddUint32/StoreUint32. In addition,
change the go command to avoid injecting an artificial import of
"sync/atomic" for sync/atomic itself.
Robert Griesemer [Wed, 4 Jan 2023 00:01:49 +0000 (16:01 -0800)]
cmd/compile: use "satisfies" (not "implements") for constraint errors
Per the latest spec, we distinguish between interface implementation
and constraint satisfaction. Use the verb "satisfy" when reporting
an error about failing constraint satisfaction.
This CL only changes error messages. It has no impact on correct code.
Fixes #57564.
Change-Id: I6dfb3b2093c2e04fe5566628315fb5f6bd709f17
Reviewed-on: https://go-review.googlesource.com/c/go/+/460396 Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Roland Illig [Wed, 4 Jan 2023 06:59:03 +0000 (06:59 +0000)]
cmd/gofmt: fix a typo in a comment
Change-Id: I34b2b9f9b70e39c872d5edbbda4de0fe330211f5
GitHub-Last-Rev: 723e36e11ee011c54d3ffc8125d0477c8eed25a0
GitHub-Pull-Request: golang/go#57566
Reviewed-on: https://go-review.googlesource.com/c/go/+/460457
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Dan Peterson [Mon, 5 Sep 2022 17:18:40 +0000 (14:18 -0300)]
.github: suggest using private browsing in pkgsite template
I opened #54872 without considering my browser's extensions and one of
them ended up being the cause of my issue. This seems to be a common
error.
To help that, add a suggestion to use a private/incognito tab/window
to the pkgsite issue template when reproducing issues. This probably
would have been enough for me to figure things out before opening my
issue.
Updates #54872, #47213
Change-Id: Ic61a3462cb902c91554cf9432aaae1222c6a991e
Reviewed-on: https://go-review.googlesource.com/c/go/+/427962
Run-TryBot: Dan Peterson <danp@danp.net> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Xiao Cui [Thu, 29 Dec 2022 00:35:13 +0000 (19:35 -0500)]
doc/go1.20: fix typos
Change-Id: Ie2e583cba9e9bec7d642e323e77fb2d9b05dc7bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/459780 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Robert Griesemer [Wed, 28 Dec 2022 22:21:50 +0000 (14:21 -0800)]
go/types, types2: use strict comparability for type set intersection
Fixes #57486.
Change-Id: I4b71199a724718886ce6d7a49e96a9ebdcbcf737
Reviewed-on: https://go-review.googlesource.com/c/go/+/459816
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Tue, 27 Dec 2022 22:29:28 +0000 (17:29 -0500)]
misc/cgo/testsanitizers: run libfuzzer tests in temp directory
The libFuzzer generated binary by default writes failure input
into the current directory. Set cmd.Dir to the temporary directory
so it won't write to GOROOT when running the test.
Change-Id: I3e4ce7e3f845be5c9f09511c36e7a9a396eafad2
Reviewed-on: https://go-review.googlesource.com/c/go/+/459556 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Andrey Bozhko [Wed, 21 Dec 2022 22:24:05 +0000 (22:24 +0000)]
testing: rephrase the sentence about naming test files
This updates the explanation about
naming test files to be a little more clear.
Fixes #57389
Change-Id: I9b3a8e2dae5e3ad398b55624e183809b7d90864c
GitHub-Last-Rev: 9ff81b74b201ca032fd373424aa02a3fedaec008
GitHub-Pull-Request: golang/go#57399
Reviewed-on: https://go-review.googlesource.com/c/go/+/458435 Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Fahad King <fahadking750@gmail.com> Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
Cherry Mui [Wed, 21 Dec 2022 21:35:57 +0000 (16:35 -0500)]
cmd/link, runtime: use a different section for Go libfuzzer counters
Currently in libfuzzer mode, we put our counters in section
__sancov_cntrs. When linking with C/C++ code that also has fuzzer
counters, apparently the C linker combines our counters and their
counters and registers them together. But in the Go runtime we
also have code to register our counters. So the Go counters ended
up registered twice, causing problems.
Since we already have code to register our counters, put them in
a Go-specific section so it won't be combined with the C counters.
Fixes #57449.
Change-Id: If3d41735124e7e301572d4b7aecf7d057ac134c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/459055 Reviewed-by: Nicolas Hillegeer <aktau@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Russ Cox [Thu, 22 Dec 2022 14:44:56 +0000 (09:44 -0500)]
runtime: call __fork instead of fork on darwin
Issues #33565 and #56784 were caused by hangs in the child process
after fork, while it ran atfork handlers that ran into slow paths that
didn't work in the child.
CL 451735 worked around those two issues by calling a couple functions
at startup to try to warm up those child paths. That mostly worked,
but it broke programs using cgo with certain macOS frameworks (#57263).
CL 459175 reverted CL 451735.
This CL introduces a different fix: bypass the atfork child handlers
entirely. For a general fork call where the child and parent are both
meant to keep executing the original program, atfork handlers can be
necessary to fix any state that would otherwise be tied to the parent
process. But Go only uses fork as preparation for exec, and it takes
care to limit what it attempts to do in the child between the fork and
exec. In particular it doesn't use any of the things that the macOS
atfork handlers are trying to fix up (malloc, xpc, others). So we can
use the low-level fork system call (__fork) instead of the
atfork-wrapped one.
The full list of functions that can be called in a child after fork in
exec_libc2.go is:
I disassembled all of these while attached to a hung exec.test binary
and confirmed that nearly all of them are making direct kernel calls,
not using anything that the atfork handler needs to fix up.
The exceptions are ioctl, fcntl, and exit.
The ioctl and fcntl implementations do some extra work around the
kernel call but don't call any other functions, so they should still
be OK. (If not, we could use __ioctl and __fcntl instead, but without
a good reason, we should keep using the standard entry points.)
The exit implementation calls atexit handlers. That is almost
certainly inappropriate in a failed fork child, so this CL changes
that call to __exit on darwin. To avoid making unnecessary changes at
this point in the release cycle, this CL leaves OpenBSD calling plain
exit, even though that is probably a bug in the OpenBSD port
(filed #57446).
Fixes #33565.
Fixes #56784.
Fixes #57263.
Change-Id: I26812c26a72bdd7fcf72ec41899ba11cf6b9c4ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/459176 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Russ Cox [Thu, 22 Dec 2022 14:21:03 +0000 (09:21 -0500)]
runtime: revert Apple libc atfork workaround
Revert CL 451735 (1f4394a0c926), which fixed #33565 and #56784
but also introduced #57263.
I have a different fix to apply instead. Since the first fix was
never backported, it will be easiest to backport the new fix
if the new fix is done in a separate CL from the revert.
Cherry Mui [Tue, 20 Dec 2022 22:26:18 +0000 (17:26 -0500)]
runtime/internal/startlinetest: work around shared buildmode linking issue
The runtime/internal/startlinetest package contains a call to a
function defined in runtime_test. Generally this is fine as this
package is only linked in for runtime_test. Except that for "go
install -buildmode=shared std", which include all packages in std,
including this test-only internal package. In this mode, the
caller is included in the linking but the callee is not, causing
linking error. Work around it by calling
runtime_test.callerStartLine via a function pointer. The function
pointer is only set in runtime_test. In the shared std build, the
function pointer will not be set, and this is fine.
Fixes #57334.
Change-Id: I7d871c50ce6599c6ea2802cf6e14bb749deab220
Reviewed-on: https://go-review.googlesource.com/c/go/+/458696
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Cherry Mui [Tue, 20 Dec 2022 22:19:09 +0000 (17:19 -0500)]
runtime/race: add build tag to internal amd64vN packages
Only one of the runtime/race/internal/amd64vN packages should be
included in a build. Generally this is true because the
runtime/race package would import only one of them depending on
the build configuration. But for "go install -buildmode=shared std"
it includes all Go packages in std, which includes both, which
then causes link-time failure due to duplicated symbols. To avoid
this, we add build tags to the internal packages, so, depending on
the build configuation, only one package would contain buildable
go files therefore be included in the build.
For #57334.
Change-Id: I52ddc3a40e16c7d04b4dd861e9689918d27e8509
Reviewed-on: https://go-review.googlesource.com/c/go/+/458695
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Tobias Klauser [Wed, 21 Dec 2022 12:27:04 +0000 (13:27 +0100)]
doc/go1.20: fix typo
Change-Id: Ia0ce728ca18eefd835220b2076c4aa8ba00cda6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/458815
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: David Chase <drchase@google.com>
Than McIntosh [Wed, 21 Dec 2022 16:03:16 +0000 (11:03 -0500)]
runtime/coverage: add missing file close in test support helper
The processPod() helper (invoked by processCoverTestDir, which is in
turn called by _testmain.go) was opening and reading counter data
files, but never closing them. Add a call to close the files after
they have been read.
Fixes #57407.
Change-Id: If9a489f92e4bab72c5b2df8697e14420a6f7b8f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/458835 Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>