Change-Id: Ie3fe0274288d6cb6303acdcec1340c480e5c0b20
GitHub-Last-Rev: ce9d44619e
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>
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.
Fixes#57785.
Change-Id: I140732ff1e6b83cd9c453701bb8199b333fc0f2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/462116
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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.
Change-Id: I4c107440a93f60bc0de07df2b7efeb1a4a766da0
Reviewed-on: https://go-review.googlesource.com/c/go/+/460537
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
At the moment the only documentation is the release notes,
but everything mentioned in the release notes should have
proper documentation separate from them.
Change-Id: I9885962f6c6d947039b0be59b608385781479271
Reviewed-on: https://go-review.googlesource.com/c/go/+/462196
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
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.
Change-Id: I4f6e328ac470a9cd410f5f722d0769ef62d5e5ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/462355
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Eli Bendersky <eliben@google.com>
These are mentioned in the release notes but not the actual doc comments.
Nothing should exist only in release notes.
Change-Id: I8d10f25a2c9b2677231929ba3f393af9034b777b
Reviewed-on: https://go-review.googlesource.com/c/go/+/462195
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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
Hash65536-64 2.81GB/s ±10% 16.69GB/s ± 0% +494.44%
Fixes#57741
Change-Id: Ib912e3716c6afd277994d6c1c5b2891f82225d50
Reviewed-on: https://go-review.googlesource.com/c/go/+/461597
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Auto-Submit: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
This reverts CL 448360 and adds a regression test for #57754.
Reason for revert: broke 'go list' in Debian's distribution of the Go toolchain
Fixes#57754.
Updates #50807.
Change-Id: I3e8b9126294c55f21572774b549fb28f29eded0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/461959
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Remove a superfluous go.mod file in one of the testdata subdirs; test
runs ok without it, no need for it to be there (can confuse tooling).
Change-Id: I3c43dd8ca557fdd32ce2f84cdb2427326a2dd35e
Reviewed-on: https://go-review.googlesource.com/c/go/+/461945
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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>
Fix incorrect comment for the runtime package.
Change-Id: Iab889eff0e9c622afbed959d32b8b5f0ed0bfebf
GitHub-Last-Rev: e9587868db
GitHub-Pull-Request: golang/go#57731
Reviewed-on: https://go-review.googlesource.com/c/go/+/461498
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Keith Randall <khr@google.com>
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>
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.
Fixes#33565.
Fixes#56784.
Fixes#57263.
Fixes#57577.
Change-Id: I20769d9daa1fe9ea930f8009481335f8a14dc21b
Reviewed-on: https://go-review.googlesource.com/c/go/+/460476
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
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.
This reverts commit c61d322d5f.
Change-Id: I2da293ff537237ffd2d40ad756d827c95c84635b
Reviewed-on: https://go-review.googlesource.com/c/go/+/460475
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
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.
Fixes#57596.
Updates #52580.
Updates #50436.
Change-Id: Ia4b88ea47fc6b605c27ca6d9d7669c874867a900
Reviewed-on: https://go-review.googlesource.com/c/go/+/460998
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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>
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>
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>
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>
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>
These typos were found by executing grep, aspell, sort, and uniq in
a pipe and searching the resulting list manually for possible typos.
grep -r --include '*.go' -E '^// .*$' . | aspell list | sort | uniq
Change-Id: I56281eda3b178968fbf104de1f71316c1feac64f
GitHub-Last-Rev: e91c7cee34
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>
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>
cannonical -> canonical
Change-Id: I656ea210d8ef6eaa85245cb8f463f6b4fd67e1a2
GitHub-Last-Rev: 5a93045add
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>
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.
Fixes#57445.
Change-Id: I8c8fbd0bcf26c8a8607d4806046f826296508c74
Reviewed-on: https://go-review.googlesource.com/c/go/+/459335
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Links under strings package were linking to the bytes versions of the functions.
Change-Id: If6ebe37fede8e417f8683695783aa767bc01e9c7
GitHub-Last-Rev: 8849285122
GitHub-Pull-Request: golang/go#57579
Reviewed-on: https://go-review.googlesource.com/c/go/+/460458
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Add the text from dev.boringcrypto's README making clear what
this code is and that it is unsupported.
Change-Id: Ie49e8ccff10436f5d27ed422f159b4899193c9a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/460515
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
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>
Change-Id: I34b2b9f9b70e39c872d5edbbda4de0fe330211f5
GitHub-Last-Rev: 723e36e11e
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>
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>
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>
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>
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>
Apparently we don't have tests for libfuzzer mode. Add some tests.
Updates #57449.
Change-Id: I813da3e71c6d6f15db31914b248db220b0b7041e
Reviewed-on: https://go-review.googlesource.com/c/go/+/459555
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
This updates the explanation about
naming test files to be a little more clear.
Fixes#57389
Change-Id: I9b3a8e2dae5e3ad398b55624e183809b7d90864c
GitHub-Last-Rev: 9ff81b74b2
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>
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>
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:
- ptrace
- setsid
- setpgid
- getpid
- ioctl
- chroot
- setgroups
- setgid
- setuid
- chdir
- dup2
- fcntl
- close
- execve
- write
- exit
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>
Revert CL 451735 (1f4394a0c9), 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.
Change-Id: I6c8ea3a46e542ee4702675bbc058e29ccd2723e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/459175
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Test that "go install -buildmode=shared std" works.
For #57334.
Change-Id: I465a07cf2e9035995916ef9940b4c1eeba998099
Reviewed-on: https://go-review.googlesource.com/c/go/+/459056
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim Scharfenort <timscharfenort89@googlemail.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
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>
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>
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>
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>
