Ian Lance Taylor [Fri, 19 May 2023 22:09:58 +0000 (15:09 -0700)]
net, os: net.Conn.File.Fd should return a blocking descriptor
Historically net.Conn.File.Fd has returned a descriptor in blocking mode.
That was broken by CL 495079, which changed the behavior for os.OpenFile
and os.NewFile without intending to affect net.Conn.File.Fd.
Use a hidden os entry point to preserve the historical behavior,
to ensure backward compatibility.
Change-Id: I8d14b9296070ddd52bb8940cb88c6a8b2dc28c27
Reviewed-on: https://go-review.googlesource.com/c/go/+/496080
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Wed, 17 May 2023 22:22:25 +0000 (15:22 -0700)]
net: ignore more errors in TestDialCancel
TestDialCancel assumes that packets sent to the private IP addresses
198.18.0.254 and 2001:2::254 will be routed to /dev/null.
Not all systems are configured that way. We already ignore one
error case in the test; ignore a couple more than have appeared
on the builders. The test is still valid as long as some builders
discard the packets as expected.
Fixes #52579
Fixes #57364
Change-Id: Ibe9ed73b8b3b498623f1d18203dadf9207a0467e
Reviewed-on: https://go-review.googlesource.com/c/go/+/496037 Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Sat, 20 May 2023 00:44:35 +0000 (17:44 -0700)]
runtime: consolidate on a single closeonexec definition
Now that we implement fcntl on all Unix systems, we can
write closeonexec that uses it. This lets us remove a bunch
of assembler code.
Change-Id: If35591df535ccfc67292086a9492f0a8920e3681
Reviewed-on: https://go-review.googlesource.com/c/go/+/496081 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Sean Liao [Sun, 12 Mar 2023 02:34:04 +0000 (10:34 +0800)]
cmd/go: update help for empty environment variables
Fixes #50335
Change-Id: I44b9dc6afa8c70b5cc8c79fb3ebddc3f45d3cef8
Reviewed-on: https://go-review.googlesource.com/c/go/+/475695
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Fri, 19 May 2023 04:13:03 +0000 (21:13 -0700)]
runtime: change fcntl to return two values
Separate the result and the errno value, rather than assuming
that the result can never be negative.
Change-Id: Ib01a70a3d46285aa77e95371cdde74e1504e7c12
Reviewed-on: https://go-review.googlesource.com/c/go/+/496416
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Russ Cox [Thu, 11 May 2023 18:32:56 +0000 (14:32 -0400)]
cmd/go: implement GOTOOLCHAIN=auto
The documentation is yet to be written (more work in the go
command remains first). This CL implements the toolchain
selection described in
https://go.dev/design/57001-gotoolchain#the-and-lines-in-in-the-work-module
with these changes based on the issue discussion:
1. GOTOOLCHAIN=auto looks for a go1.19.1 binary in $PATH
and if found uses it instead of downloading Go 1.19.1 as a module.
2. GOTOOLCHAIN=path is like GOTOOLCHAIN=auto, with
downloading disabled.
3. GOTOOLCHAIN=auto+version and GOTOOLCHAIN=path+version
set a different minimum version of Go to use during the version
selection. The default is to use the newer of what's on the go line
or the current toolchain. If you are have Go 1.22 installed locally
and want to switch to a minimum of Go 1.25 with go.mod files
allowed to bump even further, you would set GOTOOLCHAIN=auto+go1.25.
The minimum is also important when there is no go.mod involved,
such as when you write a tiny x.go program and run "go run x.go".
That would get Go 1.25 in this example, instead of falling back to
the local Go 1.22.
Change-Id: I6c7519fea68daefa641f25130cdd9803dc8aae22
GitHub-Last-Rev: a29d890d6f32fd5a1ecef84d012b8447b406e2e2
GitHub-Pull-Request: golang/go#60222
Reviewed-on: https://go-review.googlesource.com/c/go/+/495155
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Jabar Asadi <jasadi@d2iq.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Sean Liao [Fri, 19 May 2023 18:39:29 +0000 (19:39 +0100)]
net/http/pprof: document query params
Fixes #59452
Change-Id: Ia0b5a03565f663190c480ef9e26309fa85ff192c
Reviewed-on: https://go-review.googlesource.com/c/go/+/496144 Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Sven Anderson [Thu, 4 May 2023 22:15:07 +0000 (00:15 +0200)]
runtime: improve Pinner with gcBits
This change replaces the statically sized pinnerBits with gcBits
based ones, that are copied in each GC cycle if they exist. The
pinnerBits now include a second bit per object, that indicates if a
pinner counter for multi-pins exists, in order to avoid unnecessary
specials iterations.
This is a follow-up to CL 367296.
Change-Id: I82e38cecd535e18c3b3ae54b5cc67d3aeeaafcfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/493275 Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Dmitri Shuralyov [Thu, 18 May 2023 15:11:55 +0000 (11:11 -0400)]
cmd/dist: use "pkg[:variant]" as dist test name
The work to add the -json flag to the 'dist test' command also cleaned
how dist tests are tracked and registered. By now, a pair of (import
path, variant) strings is sufficient to uniquely identify every dist
test that exists. Some of the custom dist test names have been improved
along the way. And since the names are already changing a little anyway,
we use this opportunity to make them more uniform and predictable.
The mapping from the old dist test names to the new is as follows:
- "go_test:pkg" → "pkg" (this is the most common case)
- "go_test_bench:pkg" → "pkg:racebench"
- all other custom names are now called "pkg:variant", where variant
is a description of their test configuration and pkg is the import
path of the Go package under test
CL 495016 introduced test variants and used variant names for rewriting
the Package field in JSON events, and now that same name starts to also
be used as the dist test name.
Like previously done in CL 494496, registering a test variant involving
multiple Go packages creates a "pkg:variant" dist test name for each.
In the future we may combine their 'go test' invocation purely as an
optimization.
We can do this with the support of CL 496190 that keeps the coordinator
happy and capable of working with both new and old names.
In the end, all dist tests now have a consistent "pkg[:variant]" name.
For #37486.
For #59990.
Change-Id: I7eb02a42792a9831a2f3eeab583ff635d24269e8 Co-authored-by: Austin Clements <austin@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/496181
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Austin Clements [Fri, 19 May 2023 20:58:37 +0000 (16:58 -0400)]
cmd/dist: delete moved_goroot test
This test is largely obviated by the goroot_executable and
list_goroot_symlink cmd/go script tests. It's the last user of several
special-case features in cmd/dist and runs only under a fairly
constrained set of conditions (including only running on builders, not
locally). Delete it.
Change-Id: Icc744e3f9f04813bfd0cad2ef3e88e42617ecf5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/496519 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Michael Anthony Knyszek [Thu, 18 May 2023 14:21:05 +0000 (14:21 +0000)]
runtime: make trace.lock not reentrant
Currently trace.lock is reentrant in a few cases. AFAICT, this was
necessary a long time ago when the trace reader would goparkunlock, and
might flush a trace buffer while parking the goroutine. Today, that's no
longer true, since that always happens without the trace.lock held.
However, traceReadCPU does still rely on this behavior, since it could
get called either with trace.lock held, or without it held. The silver
lining here is that it doesn't *need* trace.lock to be held, so the
trace reader can just drop the lock to call traceReadCPU (this is
probably also nice for letting other goroutines flush while the trace
reader is reading from the CPU log).
Michael Anthony Knyszek [Wed, 17 May 2023 14:22:55 +0000 (14:22 +0000)]
runtime: replace raw traceEv with traceBlockReason in gopark
This change adds traceBlockReason which leaks fewer implementation
details of the tracer to the runtime. Currently, gopark is called with
an explicit trace event, but this leaks details about trace internals
throughout the runtime.
This change will make it easier to change out the trace implementation.
Change-Id: Id633e1704d2c8838c6abd1214d9695537c4ac7db
Reviewed-on: https://go-review.googlesource.com/c/go/+/494185
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Austin Clements [Fri, 19 May 2023 13:32:22 +0000 (09:32 -0400)]
cmd/dist: flush incomplete lines in -json mode
Currently, if a test prints an incomplete line and then exits, in JSON
mode, the filter we use to rewrite Package lines will keep the last
incomplete line in an internal buffer and never print it. In theory
this should never happen anyway because the test should only write
JSON to stdout, but we try pretty hard to pass through any non-JSON,
so it seems inconsistent to swallow incomplete lines.
Fix this by adding a testJSONFilter.Flush method and calling it in the
right places. Unfortunately this is a bit tricky because the filter is
constructed pretty far from where we run the exec.Cmd, so we return
the flush function through the various layers in order to route it to
the place where we call Cmd.Run.
Keith Randall [Tue, 2 May 2023 17:37:00 +0000 (17:37 +0000)]
cmd/compile: constant-fold loads from constant dictionaries and types
Retrying the original CL with a small modification. The original CL
did not handle the case of reading an itab out of a dictionary
correctly. When we read an itab out of a dictionary, we must treat
the type inside that itab as maybe being put in an interface.
Original CL: 486895
Revert CL: 490156
Change-Id: Id2dc1699d184cd8c63dac83986a70b60b4e6cbd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/491495 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Fri, 12 May 2023 21:13:06 +0000 (21:13 +0000)]
runtime: formalize the trace clock
Currently the trace clock is cputicks() with comments sprinkled in
different places as to which clock to use. Since the execution tracer
redesign will use a different clock, it seems like a good time to clean
that up.
Also, rename the start/end timestamps to be more readable (i.e.
startTime vs. timeStart).
Change-Id: If43533eddd0e5f68885bb75cdbadb38da42e7584
Reviewed-on: https://go-review.googlesource.com/c/go/+/494775 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Thu, 18 May 2023 23:58:05 +0000 (16:58 -0700)]
cmp: new package
The new cmp package provides types and functions related to
comparing ordered values.
For #59488
Change-Id: I43f4b2e6036f63b87c2152672d2b6fa18235cbeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/496356
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Eli Bendersky <eliben@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
miller [Thu, 18 May 2023 09:17:42 +0000 (10:17 +0100)]
internal/poll: handle SetDeadline to time.Now() in Plan 9
The implementation of SetDeadline in Plan 9 begins by calculating
d = the offset of the requested deadline from time.Now(). If d > 0,
a timer is set to interrupt future I/O. If d < 0, the channel is
flagged to prevent future I/O and any current I/O is cancelled.
But if d = 0, nothing happens and the deadline isn't set.
The d = 0 case should be handled the same as d < 0.
Fixes #60282
Fixes #52896
Change-Id: Id8167db3604db1c129d99376fa78a3da75417d20
Reviewed-on: https://go-review.googlesource.com/c/go/+/496137 Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: David du Colombier <0intro@gmail.com>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Michael Anthony Knyszek [Thu, 11 May 2023 21:09:10 +0000 (21:09 +0000)]
runtime: emit STW events for all pauses, not just those for the GC
Currently STW events are only emitted for GC STWs. There's little reason
why the trace can't contain events for every STW: they're rare so don't
take up much space in the trace, yet being able to see when the world
was stopped is often critical to debugging certain latency issues,
especially when they stem from user-level APIs.
This change adds new "kinds" to the EvGCSTWStart event, renames the
GCSTW events to just "STW," and lets the parser deal with unknown STW
kinds for future backwards compatibility.
But, this change must break trace compatibility, so it bumps the trace
version to Go 1.21.
This change also includes a small cleanup in the trace command, which
previously checked for STW events when deciding whether user tasks
overlapped with a GC. Looking at the source, I don't see a way for STW
events to ever enter the stream that that code looks at, so that
condition has been deleted.
Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/494495 Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Fri, 19 May 2023 15:56:07 +0000 (15:56 +0000)]
runtime: fix lockrank ordering for pinner implementation
The new Pinner API's implementation imposes some partial-orders that are
safe but previously did not exist between a mspanSpecial, mheapSpecial,
and mheap. Fix that up in the lock ranking.
For #46787.
Change-Id: I51cc8f7f069240caeb44d749bed43515634f4814
Reviewed-on: https://go-review.googlesource.com/c/go/+/496193
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Sven Anderson [Sun, 28 Nov 2021 04:05:16 +0000 (13:05 +0900)]
runtime: implement Pinner API for object pinning
Some C APIs require the use or structures that contain pointers to
buffers (iovec, io_uring, ...). The pointer passing rules would
require that these buffers are allocated in C memory and to process
this data with Go libraries it would need to be copied.
In order to provide a zero-copy way to use these C APIs, this CL
implements a Pinner API that allows to pin Go objects, which
guarantees that the garbage collector does not move these objects
while pinned. This allows to relax the pointer passing rules so that
pinned pointers can be stored in C allocated memory or can be
contained in Go memory that is passed to C functions.
The Pin() method accepts pointers to objects of any type and
unsafe.Pointer. Slices and arrays can be pinned by calling Pin()
with the pointer to the first element. Pinning of maps is not
supported.
If the GC collects unreachable Pinner holding pinned objects it
panics. If Pin() is called with the other non-pointer types it
panics as well.
Performance considerations: This change has no impact on execution
time on existing code, because checks are only done in code paths,
that would panic otherwise. The memory footprint on existing code is
one pointer per memory span.
Fixes: #46787 Signed-off-by: Sven Anderson <sven@anderson.de>
Change-Id: I110031fe789b92277ae45a9455624687bd1c54f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/367296
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 3 Jan 2023 20:39:23 +0000 (20:39 +0000)]
runtime: make the memory limit heap goal headroom proportional
Currently if GOGC=off and GOMEMLIMIT is set, then the synchronous
scavenger is likely to work fairly often to maintain the limit, since
the heap goal goes right up to the edge of the memory limit (minus a
fixed 1 MiB of headroom).
If the application's allocation rate is high, and page-level
fragmentation is high, then most allocations will scavenge.
This change mitigates this problem by adding a proportional component
to constant headroom added to the memory-limit-based heap goal. This
means the runtime will have much more headroom before fragmentation
forces memory to be eagerly scavenged.
The proportional headroom in this case is 3%, or ~30 MiB for a 1 GiB
heap. This technically will increase GC frequency in the GOGC=off case
by a tiny amount, but will likely have a positive impact on both
allocation throughput and latency that outweighs this difference.
I wrote a small program to reproduce this issue and confirmed that the
issue is resolved by this patch:
This value of 3% is chosen as it seems to be a inflection point in this
particular small program. 2% still resulted in quite a bit of eager
scavenging work. I confirmed this results in a GC frequency increase of
about 3%.
This choice is still somewhat arbitrary because the program is
arbitrary, so perhaps worth revisiting in the future. Still, it should
help a good number of programs.
Fixes #57069.
Change-Id: Icb9829db0dfefb4fe42a0cabc5aa8d35970dd7d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/460375 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Bryan C. Mills [Fri, 3 Mar 2023 14:48:19 +0000 (14:48 +0000)]
cmd/go: add a GODEBUG to limit the number of concurrent network connections
I implemented this in order to debug connection failures on a
new-to-me VM development environment that uses Cloud NAT. It doesn't
directly fix the bug, but perhaps folks will find it useful to
diagnose port-exhaustion-related flakiness in other environments.
Austin Clements [Fri, 19 May 2023 01:46:55 +0000 (21:46 -0400)]
cmd/cgo/internal/testcarchive: fix nocgo and no-c-archive builds
CL 495918 enabled testcarchive much more widely and added many dynamic
test skips. CL 495855 added TestDeepStack before these dynamic skips
were in. Unfortunately, the two CLs don't logically commute, so when
CL 495918 landed, it broke at least nocgo builders and platforms that
don't support c-archive builds. Fix this by adding the necessary skips
to TestDeepStack.
Austin Clements [Thu, 18 May 2023 21:32:05 +0000 (17:32 -0400)]
cmd/cgo/internal/testsovar: merge into testso
The test driver for testso and testsovar are literally identical, and
only the testdata code is different between the two test packages.
Merge them into a single test package with two tests that share a
driver.
Change-Id: I3f107a6aba345c0dd58606c10e3ac8eee33b33c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/496315
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Austin Clements [Thu, 18 May 2023 20:26:13 +0000 (16:26 -0400)]
cmd/cgo/internal/testtls: cleanup and support more arches
Currently, this test only enabled on non-Darwin UNIX platforms because
it uses the non-standard _thread attribute for thread-local storage.
C11 introduced a standard way to declare something thread-local, so
this CL takes advantage of that to generalize the test to Darwin and
Windows.
Austin Clements [Wed, 17 May 2023 20:02:06 +0000 (16:02 -0400)]
cmd/dist: drop remaining conditions on default cgo tests
Currently, dist registers cmd/cgo/internal{test,testtls,testnocgo}
specially, so they're opted out of "go test std cmd". It has to
register these test packages to run in various non-default build
configurations, but at this point they can also run with the default
build configuration (and for test and testtls, we intentionally want
to test them in the default configuration; this is pointless but
harmless for testnocgo). Hence, this CL drops the special registration
of their default build configurations from registerCgoTests and lets
them be registered as part of registerStdTests.
Austin Clements [Wed, 17 May 2023 19:59:47 +0000 (15:59 -0400)]
cmd/cgo/internal/testnocgo: always run in default configuration
This test is actually intended to test that we can build in -static
mode even without any cgo. That means it's quite harmless to run in
the default build configuration (in addition to running with various
other build configurations).
Austin Clements [Wed, 17 May 2023 19:10:51 +0000 (15:10 -0400)]
cmd/dist: don't pass -linkmode=auto
This is the default value of this flag, so passing it clutters up
debugging output. This also makes it clearer which tests are running
with a default configuration.
Austin Clements [Wed, 10 May 2023 18:28:30 +0000 (14:28 -0400)]
cmd/dist: let several cgo tests run as regular cmd tests
Several cgo tests no longer have any special conditions, so they can
just be normal cmd tests. This brings dist's "go_test:.*" tests much
closer to what "go test std cmd" runs.
cmd/dist: refine test conditions and enable more cgo tests on Android, iOS
This CL moves many cgo test conditions out of dist and into the tests
themselves, now that they can use the testenv.Must* helpers.
This refines a lot of the conditions, which happens to have the effect
of enabling many tests on Android and iOS that are disabled by
too-coarse GOOS checks in dist today.
Austin Clements [Wed, 10 May 2023 17:55:10 +0000 (13:55 -0400)]
cmd/dist: enable more cgo tests if GO_GCFLAGS != ""
Currently, we have several tests disabled if GO_GCFLAGS is non-empty.
Long ago, this was critical because many of these tests use "go
install" with no -gcflags and would thus overwrite std packages in
GOROOT built with -gcflags=$GO_GCFLAGS. Now these packages all live in
the build cache, so this is no longer a concern.
The other reason for this (the reason given in the code comment), is
that these tests will rebuild significant portions of std without
flags. While this is still theoretically true, there are many tests
that run "go build" with no -gcflags, so these tests don't contribute
much overall.
Empirically, on my linux/amd64 host, running these tests at all grows
the Go build cache by 14%, from 1.899 GB to 2.165 GB. When building
with GO_GCFLAGS="-N -l" (the only use case on the builders), enabling
them grows the Go build cache by 18%, from 1.424 GB to 1.684 GB. This
is only a 4 percentage point difference, and still results in a build
cache that's smaller than the default build
Given all this, there's little reason to carry the complexity of
disabling these tests when GO_GCFLAGS != "". Removing this condition
is a step toward running these as regular cmd tests.
Oleksandr Redko [Thu, 18 May 2023 07:53:28 +0000 (10:53 +0300)]
os: remove unnecessary return after t.Fatal
Change-Id: Ibddf36431abb799d8f9288d6e17159ce1538d62e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495879 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Heschi Kreinick [Thu, 18 May 2023 21:44:16 +0000 (17:44 -0400)]
doc: add TODOs to go1.21
Generated with relnote. I did my best to put things where they made
sense, but some weren't obvious, like the Unicode upgrade and backward
compatibility stuff.
cmd/go: make TestScript/gotoolchain more realistic
- Build the fake go1.999testpath binary from Go source instead of
special-casing a fake command on Windows.
- Skip the part of the test that uses shell scripts served from the
test GOPROXY if /bin/sh is not present.
This makes the test more expensive, but also more realistic: notably,
it does not require test hooks to determine whether to run a real or
fake binary.
Robert Griesemer [Tue, 16 May 2023 21:15:07 +0000 (14:15 -0700)]
go/types, types2: remove argument "getter" use from Checker.builtins (cleanup)
Check all arguments for validity once, in the beginning.
Conservatively replace arg(x, i) calls with *x = args[i].
Use y (2nd arguments) directly, w/o copying.
Remove unnecessary copies and slice creations in append.
Change-Id: I1e2891cba9658f5b3cdf897e81db2f690a99b16b
Reviewed-on: https://go-review.googlesource.com/c/go/+/495515
TryBot-Result: Gopher Robot <gobot@golang.org>
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>
Austin Clements [Thu, 18 May 2023 15:19:54 +0000 (11:19 -0400)]
cmd/cgo/internal/test: fix TestThreadLock
This test was introduced in CL 18882, but only recently enabled as of
CL 493603. It's intended to check that we don't move executing C code
between threads when it re-enters Go, but it has always contained a
flake. Go *can* preempt between the Go call to gettid and the C call
to gettid and move the goroutine to another thread because there's no
C code on the stack during the Go call to gettid. This will cause the
test to fail.
Fix this by making both gettid calls in C, with a re-entry to Go
between them.
Bryan C. Mills [Thu, 18 May 2023 12:20:54 +0000 (08:20 -0400)]
cmd/gofmt: fix a data race in TestPermissions
The asynchronous call to processFile is synchronized by the call to
GetExitCode. We can't safely access errBuf until then, because
processFile may still be writing to it.
This is diagnosed by 'go test -race cmd/gofmt', but only the
darwin-amd64-race builder caught it because the other "-race" builders
apparently all run as root (see #10719).
Tobias Klauser [Wed, 17 May 2023 07:57:37 +0000 (09:57 +0200)]
os: set File.appendMode in NewFile if file was opened with O_APPEND
To allow skipping the use of the copy_file_range syscall on Linux which
isn't supported for destination files opened with O_APPEND, see comment
in (*File).readFrom and
https://man7.org/linux/man-pages/man2/copy_file_range.2.html#ERRORS
erifan01 [Wed, 17 May 2023 04:01:07 +0000 (12:01 +0800)]
cmd/asm: remove unsupported opcodes MOVNP and STLP for arm64
ARM64 doesn't have MOVNP/MOVNPW and STLP/STLPW instructions, which are
currently useless instructions as well. This CL deletes them. At the
same time this CL sorts the opcodes by name, which looks cleaner.
Change-Id: I25cfb636b23356ba0a50cba527a8c85b3f7e2ee4
Reviewed-on: https://go-review.googlesource.com/c/go/+/495695 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Wed, 17 May 2023 19:26:05 +0000 (12:26 -0700)]
cmd/compile: enable more lenient type inference for untyped arguments
This enables the implementation for proposal #58671, which is
a likely accept. By enabling it early we get a bit extra soak
time for this feature. The change can be reverted trivially, if
need be.
For #58671.
Change-Id: Id6c27515e45ff79f4f1d2fc1706f3f672ccdd1ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/495955
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Wed, 17 May 2023 16:01:15 +0000 (12:01 -0400)]
runtime/cgo: store M for C-created thread in pthread key
This reapplies CL 485500, with a fix drafted in CL 492987 incorporated.
CL 485500 is reverted due to #60004 and #60007. #60004 is fixed in
CL 492743. #60007 is fixed in CL 492987 (incorporated in this CL).
[Original CL 485500 description]
This reapplies CL 481061, with the followup fixes in CL 482975, CL 485315, and
CL 485316 incorporated.
CL 481061, by doujiang24 <doujiang24@gmail.com>, speed up C to Go
calls by binding the M to the C thread. See below for its
description.
CL 482975 is a followup fix to a C declaration in testprogcgo.
CL 485315 is a followup fix for x_cgo_getstackbound on Illumos.
CL 485316 is a followup cleanup for ppc64 assembly.
CL 479915 passed the G to _cgo_getstackbound for direct updates to
gp.stack.lo. A G can be reused on a new thread after the previous thread
exited. This could trigger the C TSAN race detector because it couldn't
see the synchronization in Go (lockextra) preventing the same G from
being used on multiple threads at the same time.
We work around this by passing the address of a stack variable to
_cgo_getstackbound rather than the G. The stack is generally unique per
thread, so TSAN won't see the same address from multiple threads. Even
if stacks are reused across threads by pthread, C TSAN should see the
synchonization in the stack allocator.
A regression test is added to misc/cgo/testsanitizer.
[Original CL 481061 description]
This reapplies CL 392854, with the followup fixes in CL 479255,
CL 479915, and CL 481057 incorporated.
CL 392854, by doujiang24 <doujiang24@gmail.com>, speed up C to Go
calls by binding the M to the C thread. See below for its
description.
CL 479255 is a followup fix for a small bug in ARM assembly code.
CL 479915 is another followup fix to address C to Go calls after
the C code uses some stack, but that CL is also buggy.
CL 481057, by Michael Knyszek, is a followup fix for a memory leak
bug of CL 479915.
[Original CL 392854 description]
In a C thread, it's necessary to acquire an extra M by using needm while invoking a Go function from C. But, needm and dropm are heavy costs due to the signal-related syscalls.
So, we change to not dropm while returning back to C, which means binding the extra M to the C thread until it exits, to avoid needm and dropm on each C to Go call.
Instead, we only dropm while the C thread exits, so the extra M won't leak.
When invoking a Go function from C:
Allocate a pthread variable using pthread_key_create, only once per shared object, and register a thread-exit-time destructor.
And store the g0 of the current m into the thread-specified value of the pthread key, only once per C thread, so that the destructor will put the extra M back onto the extra M list while the C thread exits.
When returning back to C:
Skip dropm in cgocallback, when the pthread variable has been created, so that the extra M will be reused the next time invoke a Go function from C.
This is purely a performance optimization. The old version, in which needm & dropm happen on each cgo call, is still correct too, and we have to keep the old version on systems with cgo but without pthreads, like Windows.
This optimization is significant, and the specific value depends on the OS system and CPU, but in general, it can be considered as 10x faster, for a simple Go function call from a C thread.
For the newly added BenchmarkCGoInCThread, some benchmark results:
1. it's 28x faster, from 3395 ns/op to 121 ns/op, in darwin OS & Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
2. it's 6.5x faster, from 1495 ns/op to 230 ns/op, in Linux OS & Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz
[CL 479915 description]
Currently, when C calls into Go the first time, we grab an M
using needm, which sets m.g0's stack bounds using the SP. We don't
know how big the stack is, so we simply assume 32K. Previously,
when the Go function returns to C, we drop the M, and the next
time C calls into Go, we put a new stack bound on the g0 based on
the current SP. After CL 392854, we don't drop the M, and the next
time C calls into Go, we reuse the same g0, without recomputing
the stack bounds. If the C code uses quite a bit of stack space
before calling into Go, the SP may be well below the 32K stack
bound we assumed, so the runtime thinks the g0 stack overflows.
This CL makes needm get a more accurate stack bound from
pthread. (In some platforms this may still be a guess as we don't
know exactly where we are in the C stack), but it is probably
better than simply assuming 32K.
[CL 492987 description]
On the first call into Go from a C thread, currently we set the g0
stack's high bound imprecisely based on the SP. With CL 485500, we
keep the M and don't recompute the stack bounds when it calls into
Go again. If the first call is made when the C thread uses some
deep stack, but a subsequent call is made with a shallower stack,
the SP may be above g0.stack.hi.
This is usually okay as we don't check usually stack.hi. One place
where we do check for stack.hi is in the signal handler, in
adjustSignalStack. In particular, C TSAN delivers signals on the
g0 stack (instead of the usual signal stack). If the SP is above
g0.stack.hi, we don't see it is on the g0 stack, and throws.
This CL makes it get an accurate stack upper bound with the
pthread API (on the platforms where it is available).
Also add some debug print for the "handler not on signal stack"
throw.
Ian Lance Taylor [Tue, 16 May 2023 17:18:54 +0000 (10:18 -0700)]
cmd/gofmt: try to write original data on rewrite failure
When gofmt needs to rewrite a file, it first copies it into a backup.
If the rewrite fails, it used to rename the backup to the original.
However, if for some reason the file is owned by some other user,
and if the rewrite fails because gofmt doesn't have permission to
write to the file, then renaming the backup file will change
the file owner. This CL changes gofmt so that if it fails to rewrite
a file, it tries to write the original contents. If writing the original
content fails, it reports the problem to the user referring to the
backup file, rather than trying a rename.
Also create the backup file with the correct permissions,
to avoid a tiny gap when some process might get write access to the
file contents that it shouldn't have. (This tiny gap only applies to
files that are not formatted correctly, and have read-only permission,
and are in a directory with write permission.)
Fixes #60225
Change-Id: Ic16dd0c85cf416d6b2345e0650d5e64413360847
Reviewed-on: https://go-review.googlesource.com/c/go/+/495316
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Tue, 16 May 2023 04:50:51 +0000 (21:50 -0700)]
os: if descriptor is non-blocking, retain that in Fd method
For #58408
Fixes #60211
Change-Id: I30f5678b46e15121865b19d1c0f82698493fad4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495079
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Patryk Chelmecki [Wed, 17 May 2023 20:48:28 +0000 (20:48 +0000)]
crypto/x509: fix certificate validation with FQDN on Windows
Currently certificate verification on Windows fails if the provided dns name ends with a dot (which means it is a Fully Qualified Domain Name). The certificates according to RFC 6066 (https://www.rfc-editor.org/rfc/rfc6066#section-3) do not contain that ending dot. Go uses CertVerifyCertificateChainPolicy Windows system call with CERT_CHAIN_POLICY_SSL option for verification of the certificates. That call fails if the specified domain name contains the dot at the end.
Examples of other open source codebases that use the same system call and trim the trailing dot before executing it:
MongoDb - https://github.com/mongodb/mongo/blob/master/src/mongo/util/net/ssl_manager_windows.cpp#L1777
Dot Net - https://github.com/dotnet/runtime/blob/v7.0.5/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs#L52
Change-Id: I5db558eb277cf00f5401ec0ffc96c935023ad100
GitHub-Last-Rev: cc69ab9be35f79a93279bd618912a3fd6aaa9f88
GitHub-Pull-Request: golang/go#59846
Reviewed-on: https://go-review.googlesource.com/c/go/+/489135
Run-TryBot: Roland Shoemaker <roland@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Patryk Chełmecki <patchelmecki@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Ian Lance Taylor [Tue, 16 May 2023 23:03:38 +0000 (16:03 -0700)]
runtime: publish netpoll info after incrementing fdseq
I think there is a theoretical possibility of a mistake before this CL.
pollCache.free would increment fdseq, but would not update atomicInfo.
The epoll code could compare to fdseq before the increment, but suspend
before calling setEventErr. The pollCache could get reallocated,
and pollOpen could clear eventErr. Then the setEventErr could continue
and set it again. Then pollOpen could call publishInfo.
Avoid this rather remote possibility by calling publishInfo after
incrementing fdseq. That ensures that delayed setEventErr will not
modify the eventErr flag.
Fixes #60133
Change-Id: I69e336535312544690821c9fd53f3023ff15b80c
Reviewed-on: https://go-review.googlesource.com/c/go/+/495297
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Austin Clements [Thu, 11 May 2023 20:20:36 +0000 (16:20 -0400)]
cmd/dist: add -json flag
This enables JSON output for all tests run by dist.
Most the complexity here is that, in order to disambiguate JSON
records from different package variants, we have to rewrite the JSON
stream on the fly to include variant information. We do this by
rewriting the Package field to be pkg:variant so existing CI systems
will naturally pick up the disambiguated test name.
Fixes #37486.
Change-Id: I0094e5e27b3a02ffc108534b8258c699ed8c3b87
Reviewed-on: https://go-review.googlesource.com/c/go/+/494958
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Austin Clements [Wed, 17 May 2023 15:54:45 +0000 (11:54 -0400)]
cmd/dist: refactor rtPreFunc to print skips in only one place
Currently, all uses of rtPreFunc are to print a message and skip a
test. When we move to JSON, the logic to just "print a message" is
going to be more complicated, so refactor this so the function returns
the skip message and we print it in just one place. We also rename the
option to rtSkipFunc to better represent what we use it for.
For #37486.
Change-Id: Ibd537064fa646a956a1c0f85a5d8c6febd098dde
Reviewed-on: https://go-review.googlesource.com/c/go/+/495856 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Austin Clements [Fri, 12 May 2023 18:57:39 +0000 (14:57 -0400)]
cmd/dist: make it possible to filter output of background commands
Many of the commands dist test executes are "background" commands run
by a work queue system. The work queue allows it to run commands in
parallel, but still serialize their output. Currently, the work queue
system assumes that exec.Cmd.Stdout and Stderr will be nil and that it
can take complete control over them.
We're about to inject output filters on many of these commands, so we
need a way to interpose on Stdout and Stderr. This CL rearranges
responsibilities in the work queue system to make that possible. Now,
the thing enqueuing the work item is responsible to constructing the
Cmd to write its output to work.out. There's only one place that
constructs work objects (there used to be many more), so that's
relatively easy, and sets us up to add filters.
For #37486.
Change-Id: I55ab71ddd456a12fdbf676bb49f698fc08a5689b
Reviewed-on: https://go-review.googlesource.com/c/go/+/494957
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Austin Clements [Fri, 12 May 2023 19:04:21 +0000 (15:04 -0400)]
cmd/dist: simplify work list functions
There are no uses of addCmd, so delete it. The only use of bgDirCmd is
dirCmd, so inline it. Now the only function that interacts with the
work queue is registerTest and dist's "background commands" are used
exclusively in goTest.bgCommand and registerTest (which calls
goTest.bgCommand).
For #37486.
Change-Id: Iebbb24cf9dbee45f3975fe9504d858493e1cd947
Reviewed-on: https://go-review.googlesource.com/c/go/+/494956 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Dmitri Shuralyov [Fri, 12 May 2023 12:55:49 +0000 (08:55 -0400)]
cmd: update vendored golang.org/x/mod
Pull in CL 492990. This teaches 'go mod tidy' and other go subcommands
that write go.mod files to use semantic sort for exclude blocks, gated
on said files declaring Go version 1.21 or higher.
go get golang.org/x/mod@e7bea8f1d64f # includes CL 492990
go mod tidy
go mod vendor
Fixes #60028.
Change-Id: Ia9342dcc23cd68de068a70657b59c25f69afa381
Reviewed-on: https://go-review.googlesource.com/c/go/+/494578 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
This change wraps the errors from the CharsetReader function so the caller can distinguish different error conditions.
Context: I have an XML file with an unknown encoding which I like to handle separately. I like to use the CharsetReader for this but the error type has not been forwarded.
Change-Id: I6739a0dee04ec376cd20536be2806ce7f50c5213
GitHub-Last-Rev: ada9dd510f9a5b7f8c9473f6864077e0ed6898bd
GitHub-Pull-Request: golang/go#60199
Reviewed-on: https://go-review.googlesource.com/c/go/+/494897 Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Michael Anthony Knyszek [Fri, 12 May 2023 18:55:25 +0000 (18:55 +0000)]
runtime: replace sysBlockTraced with tracedSyscallEnter
sysBlockTraced is a subtle and confusing flag.
Currently, it's only used in one place: a condition around whether to
traceGoSysExit when a goroutine is about to start running. That condition
looks like "gp.syscallsp != 0 && gp.trace.sysBlockTraced".
In every case but one, "gp.syscallsp != 0" is equivalent to
"gp.trace.sysBlockTraced."
That one case is where a goroutine is running without a P and racing
with trace start (world is stopped). It switches itself back to
_Grunnable from _Gsyscall before the trace start goroutine notices, such
that the trace start goroutine fails to emit a EvGoInSyscall event for
it (EvGoInSyscall or EvGoSysBlock must precede any EvGoSysExit event).
sysBlockTraced is set unconditionally on every syscall entry and the
trace start goroutine clears it if there was no EvGoInSyscall event
emitted (i.e. did not observe _Gsyscall on the goroutine). That way when
the goroutine-without-a-P wakes up and gets scheduled, it only emits
EvGoSysExit if the flag is set, i.e. trace start didn't _clear_ the
flag.
What makes this confusing is the fact that the flag is set
unconditionally and the code relies on it being *cleared*. Really, all
it's trying to communicate is whether the tracer is aware of a
goroutine's syscall at the point where a goroutine that lost its P
during a syscall is trying to run again.
Therefore, we can replace this flag with a less subtle one:
tracedSyscallEnter. It is set when GoSysCall is traced, indicating on
the goroutine that the tracer is aware of the syscall. Later, if
traceGoSysExit is called, the tracer knows its safe to emit an event
because the tracer is aware of the syscall.
This flag is then also set at trace start, when it emits EvGoInSyscall,
which again, lets the goroutine know the tracer is aware of its syscall.
The flag is cleared by GoSysExit to indicate that the tracer is no
longer aware of any syscalls on the goroutine. It's also cleared by
trace start. This is necessary because a syscall may have been started
while a trace was stopping. If the GoSysExit isn't emitted (because it
races with the trace end STW) then the flag will be left set at the
start of the next trace period, which will result in an erroneous
GoSysExit. Instead, the flag is cleared in the same way sysBlockTraced
is today: if the tracer doesn't notice the goroutine is in a syscall, it
makes that explicit to the goroutine.
A more direct flag to use here would be one that explicitly indicates
whether EvGoInSyscall or EvGoSysBlock specifically were already emitted
for a goroutine. The reason why we don't just do this is because setting
the flag when EvGoSysBlock is emitted would be racy: EvGoSysBlock is
emitted by whatever thread is stealing the P out from under the
syscalling goroutine, so it would need to synchronize with the goroutine
its emitting the event for.
The end result of all this is that the new flag can be managed entirely
within trace.go, hiding another implementation detail about the tracer.
Tested with `stress ./trace.test -test.run="TestTraceStressStartStop"`
which was occasionally failing before the CL in which sysBlockTraced was
added (CL 9132). I also confirmed also that this test is still sensitive
to `EvGoSysExit` by removing the one use of sysBlockTraced. The result
is about a 5% error rate. If there is something very subtly wrong about
how this CL emits `EvGoSysExit`, I would expect to see it as a test
failure. Instead:
53m55s: 200434 runs so far, 0 failures
Change-Id: If1d24ee6b6926eec7e90cdb66039a5abac819d9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/494715
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Cherry Mui [Tue, 16 May 2023 23:52:41 +0000 (19:52 -0400)]
cmd/compile: don't inline from norace packages in race mode
In race mode (or other instrumentation mode), if the caller is in
a regular package and the callee is in a norace (or noinstrument)
package, don't inline. Otherwise, when the caller is instumented
it will also affect the inlined callee.
An example is sync.(*Mutex).Unlock, which is typically not inlined
but with PGO it can be inlined into a regular function, which is
then get instrumented. But the rest of the sync package, in
particular, the Lock function is not instrumented, causing the
race detector to signal false race.
Change-Id: Ia78bb602c6da63a34ec2909b9a82646bf20873f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/495595
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Fri, 12 May 2023 21:39:42 +0000 (14:39 -0700)]
internal/abi: update type name in comment
method -> Method
For #59670
Change-Id: I78e0410f3d5094aa12b2f3ccd6735fec0d696190
Reviewed-on: https://go-review.googlesource.com/c/go/+/494795 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>