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>
Bryan C. Mills [Fri, 18 Nov 2022 22:35:45 +0000 (17:35 -0500)]
cmd/go: prune more dependencies in 'go get'
Prior to this change, 'go get' pulled in every version of each module
whose path is explicitly listed in the go.mod file. When graph pruning
is enabled (that is, when the main module is at 'go 1.17' or higher),
that pulled in transitive dependencies of older-than-selected versions
of dependencies, which are normally pruned out by other 'go' commands
(including 'go mod tidy' and 'go mod graph').
To make matters worse, different parts of `go get` were making
different assumptions about which kinds of conflicts would be
reported: the modget package assumed that any conflict is necessarily
due to some explicit constraint, but 'go get' was imposing an
additional constraint that modules could not be incidentally upgraded
in the course of a downgrade. When that additional constraint failed,
the modload package reported the failure as though it were a real
(caller-supplied) constraint, confusing the caller (which couldn't
identify any specific package or argument that caused the failure).
This change fixes both of those problems by replacing the
modload.EditRequirements algorithm with a different one.
The new algorithm is, roughly, as follows.
1. Propose a list of “root requirements” to be written to the updated
go.mod file.
2. Load the module graph from those requirements mostly as usual, but
if any root is upgraded due to transitive dependencies, retain the
original roots and the paths leading from those roots to the
upgrades. (This forms an “extended graph”, in which we can trace a
path from to each version that appears in the graph starting at one
or more of the original roots.)
3. Identify which roots caused any module path to be upgraded above
its passed-in version constraint. For each such root, either report
an unresolvable conflict (if the root itself is constrained to a
specific version) or identify an updated version to propose: either
a downgrade to the next-highest version, or an upgrade to the
actually-selected version of the root (if that version is allowed).
To avoid looping forever or devolving into an NP-complete search,
we never propose a version that was already rejected previously,
regardless of what other roots were present alongside it at the
time.
4. If the version of any root was changed, repeat from (1).
This algorithm is guaranteed to terminate, because there are finitely
many root versions and we permanently reject at least one each time we
downgrade its path to a lower version.
In addition, this change implements support for the '-v' flag to log
more information about version changes at each iteration.
Ian Lance Taylor [Mon, 15 May 2023 23:34:36 +0000 (16:34 -0700)]
runtime: consistently define fcntl
Clean up and consolidate on a single consistent definition of fcntl,
which takes three int32 arguments and returns either a positive result
or a negative errno value.
Change-Id: Id9505492712db4b0aab469c6bd15e4fce3c9ff6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495075
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Keith Randall [Tue, 16 May 2023 22:16:06 +0000 (15:16 -0700)]
slices: for Insert and Replace, grow slices like append does
At least when we're inserting/replacing near the end of a slice, when
we have to grow it use the same multiplicative growth factor that the
runtime uses for append.
Before this CL, we would grow the slice one page (8192 bytes) at a time
for large slices. This would cause O(n^2) work when appending near the
end should only take O(n) work.
This doesn't fix the problem if you insert/replace near the start of the
array, but maybe that doesn't need fixing because it is O(n^2) anyway.
Fixes #60134
Change-Id: If05376bc512ab839769180e5ce4cb929f47363b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/495296
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Robert Griesemer [Tue, 9 May 2023 19:38:40 +0000 (12:38 -0700)]
go/types, types2: permit partially instantiated functions as function arguments
This CL changes Checker.genericExprList such that it collects partially
instantiated generic functions together with their (partial) type
argument (and corresponding) expression lists, instead of trying to
infer the missing type arguments in place or to report an error.
Special care is being taken to explictly record expression types where
needed (because we can't use one of the usual expr evaluators which
takes care of that), or to track the correct instance expression for
later recording with Checker.arguments.
The resulting generic expression list is passed to Checker.arguments
which is changed to accept explicit partial type argument (and
corresponding) expression lists. The provided type arguments are fed
into type inference, matching up with their respective type parameters
(which were collected already, before this CL). If type inference is
successful, the instantiated functions are recorded as needed.
For now, the type argument expression lists are collected and passed
along but not yet used. We may use them eventually for better error
reporting.
Fixes #59958.
For #59338.
Change-Id: I26db47ef3546e64553da49d62b23cd3ef9e2b549
Reviewed-on: https://go-review.googlesource.com/c/go/+/494116 Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 16 May 2023 18:34:14 +0000 (11:34 -0700)]
go/types, types2: remove superfluous argument test in Checker.arguments
There's only two places which call Checker.arguments: Checker.callExpr
and Checker.builtin. Both ensure that the passed argument list doesn't
contain type expressions, so we don't need that extra check at the start
of Checker.arguments.
The remaining check causes Checker.arguments to exit early if any of
the passed arguments is invalid. This reduces the number of reported
errors in rare cases but is executed all the time.
If the extra errors are a problem, it would be better to not call
Checker.arguments in the first place, or only do the extra check
before Checker.arguments reports an error.
Removing this code for now. Removes a long-standing TODO.
Change-Id: Ief654b680eb6b6a768bb1b4c621d3c8169953f17
Reviewed-on: https://go-review.googlesource.com/c/go/+/495395
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Jakub Ciolek [Mon, 6 Feb 2023 06:18:53 +0000 (07:18 +0100)]
cmd/compile: call phiElimValue from removePhiArg
With the exception of the shortcircuit pass, removePhiArg is always unconditionally followed by phiElimValue.
Move the phiElimValue inside removePhiArg.
Resolves a TODO.
See CL 357964 for more info.
Change-Id: I8460b35864f4cd7301ba86fc3dce08ec8041da7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/465435 Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
With CL 408826, CL 413474, etc. reflect.ValueOf no longer
unconditionally escapes its argument, allowing a Value's content
to be allocated on the stack.
Change-Id: I3a0af85c11e2fd0df42b056095565f0ce5548886
Reviewed-on: https://go-review.googlesource.com/c/go/+/494657
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Austin Clements <austin@google.com>
In the types1 universe, we only need to represent value types. For
interfaces, this means we only need to worry about pure interfaces. A
pure interface can embed a union type, but the overall union must be
equivalent to "any".
In go.dev/cl/458619, we changed the types1 reader to return "any", but
to incorporate a consistency check to make sure this is valid.
Unfortunately, a pure interface can actually still reference impure
interfaces, and in general this is hard to check precisely without
reimplementing a lot of types2 data structures and logic into types1.
We haven't had any other reports of this check failing since 1.20, so
it seems simplest to just suppress for now.
Fixes #60117.
Change-Id: I5053faafe2d1068c6d438b2193347546bf5330cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/495455
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
David Chase [Wed, 10 May 2023 23:29:18 +0000 (19:29 -0400)]
cmd/compile: add more information to the bisect-verbose report
running on cmd/compile/internal/testdata/inlines now shows:
```
--- change set #1 (enabling changes causes failure)
b/b.go:16:6: loop variable i now per-iteration (loop inlined into b/b.go:10)
b/b.go:16:6: loop variable i now per-iteration
./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into a/a.go:18)
./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into ./main.go:37)
./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into ./main.go:38)
---
```
and
```
--- change set #2 (enabling changes causes failure)
./main.go:27:6: loop variable i now per-iteration
./main.go:27:6: loop variable i now per-iteration (loop inlined into ./main.go:35)
---
```
Still unsure about the utility of mentioning the inlined occurrence, but better
than mysteriously repeating the line over and over again.
Change-Id: I357f5d419ab4928fa316f4612eec3b75e7f8ac34
Reviewed-on: https://go-review.googlesource.com/c/go/+/494296
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
Paul E. Murphy [Mon, 1 May 2023 15:06:22 +0000 (10:06 -0500)]
cmd/link/internal/ppc64: link ELFv2 objects built with -mcpu=power10
Specifically, objects built via cgo using CGO_CFLAGS="-O2 -g -mcpu=power10".
These use new relocations defined by ELFv2 1.5, and the R_PPC64_REL24_NOTOC
relocation. These objects contain functions which may not use a TOC
pointer requiring the insertion of trampolines to use correctly.
The relocation targets of these ELFv2 objects may also contain non-zero
values. Clear the relocated bits before setting them.
Extra care is taken if GOPPC64 < power10. The R_PPC64_REL24_NOTOC reloc
existed prior to ELFv2 1.5. The presence of this relocation itself does
not imply a power10 target. Generate power8 compatible stubs if
GOPPC64 < power10.
Jonathan Amsterdam [Wed, 10 May 2023 00:56:09 +0000 (20:56 -0400)]
log/slog: create prefix buffer earlier
It's possible that the replacement for a built-in attribute is a Group.
That would cause a nil pointer exception because the handleState.prefix
field isn't set until later, in appendNonBuiltIns.
So create the prefix field earlier, at the start of commonHandler.handle.
Once we do this, we can simplify the code by creating and freeing the
prefix in newHandleState.
Along the way I discovered a line that wasn't being tested:
state.prefix.WriteString(h.groupPrefix)
so I modified an existing test case to cover it.
Change-Id: Ib385e3c13451017cb093389fd5a1647d53e610bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/494037
Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Austin Clements [Mon, 15 May 2023 18:53:45 +0000 (14:53 -0400)]
cmd/dist: introduce test variants
This introduces the concept of test variants in dist, which are
different configurations of the same package. The variant of a test is
a short string summarizing the configuration.
The "variant name" of a test is either the package name if the variant
is empty, or package:variant if not. Currently this isn't used for
anything, but soon we'll use this as the Package field of the test
JSON output so that we can disambiguate output from differently
configured runs of the same test package, and naturally flow this
through to any test result viewer.
The long-term plan is to use variant names as dist's own test names
and eliminate the ad hoc names it has right now. Unfortunately, the
build coordinator is aware of many of the ad hoc dist test names, so
some more work is needed to get to that point. This CL keeps almost
all test names the same, with the exception of tests registered by
registerCgoTests, where we regularize test names a bit using variants
to avoid some unnecessary complexity (I believe nothing depends on the
names of these tests).
Austin Clements [Thu, 11 May 2023 19:12:45 +0000 (15:12 -0400)]
cmd/dist: put cgo tests under a "Testing cgo" heading
Currently the cgo tests mostly use their package name as a heading,
which means we get a large number of test sections that each have a
single test package in them.
Unify them all under "Testing cgo" to reduce output noise.
This leaves just the cmd/api test without a heading, so we give it a
heading and require that all tests have a heading.
Austin Clements [Mon, 15 May 2023 18:24:00 +0000 (14:24 -0400)]
cmd/dist: refactor adding to the test list into a method
Currently, there are four places that add distTests to the
tester.tests list. That means we're already missing a few name
uniqueness checks, and we're about to start enforcing some more
requirements on tests that would be nice to have in one place. Hence,
to prepare for this, this CL refactors the process of adding to the
tester.tests list into a method. That also means we can trivially use
a map to check name uniqueness rather than an n^2 slice search.
eric fang [Fri, 5 May 2023 01:07:25 +0000 (01:07 +0000)]
cmd/compile: enhance tighten pass for memory values
[This is a roll-forward of CL 458755, which was reverted due to make.bash
being broken on GOAMD64=v3. But it turned out that the problem was caused
by wrong bswap/load rewrite rules, and it was fixed in CL 492616.]
This CL enhances the tighten pass. Previously if a value has memory arg,
then the tighten pass won't move it, actually if the memory state is
consistent among definition and use block, we can move the value. This
CL optimizes this case. This is useful for the following situation:
b1:
x = load(...mem)
if(...) goto b2 else b3
b2:
use(x)
b3:
some_op_not_use_x
For the micro-benchmark mentioned in #56620, the performance improvement
is about 15%.
There's no noticeable performance change in the go1 benchmark.
Fixes #56620
Change-Id: I36ea68bed384986cd3ae81cb9e6efe84bb213adc
Reviewed-on: https://go-review.googlesource.com/c/go/+/492895 Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
Ian Lance Taylor [Mon, 15 May 2023 20:53:21 +0000 (13:53 -0700)]
os, syscall: permit setting mtime to Unix 0 on Windows
This edge case was accidentally broken by CL 219638.
Change-Id: I673b3b580fbe379a04f8650cf5969fe9bce83691
Reviewed-on: https://go-review.googlesource.com/c/go/+/495036 Reviewed-by: Cherry Mui <cherryyz@google.com>
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>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Lynn Boger [Fri, 17 Mar 2023 20:22:31 +0000 (15:22 -0500)]
cmd/compile: update rules to generate more prefixed instructions
This modifies some existing rules to allow more prefixed instructions
to be generated when using GOPPC64=power10. Some rules also check
if PCRel is available, which is currently supported for linux/ppc64le
and linux/ppc64 (internal linking only).
Prior to p10, DS-offset loads and stores had a 16 bit size limit for
the offset field. If the offset of the data for load or store was
beyond this range then an indexed load or store would be selected by
the rules.
In p10 the assembler can generate prefixed instructions in this case,
but does not if an indexed instruction was selected during the lowering
pass.
This allows many more cases to use prefixed loads or stores, reducing
function sizes and improving performance in some cases where the code
change happens in key loops.
For example in strconv BenchmarkAppendQuoteRune before:
Damien Neil [Tue, 18 Apr 2023 15:50:02 +0000 (08:50 -0700)]
net/http: handle WriteHeader(101) as a non-informational header
Prior to Go 1.19 adding support for sending 1xx informational headers
with ResponseWriter.WriteHeader, WriteHeader(101) would send a 101
status and disable further writes to the response. This behavior
was not documented, but is intentional: Writing to the response
body explicitly checks to see if a 101 status has been sent before
writing.
Restore the pre-1.19 behavior when writing a 101 Switching Protocols
header: The header is sent, no subsequent headers are sent, and
subsequent writes to the response body fail.
For #59564
Change-Id: I72c116f88405b1ef5067b510f8c7cff0b36951ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/485775 Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Laurent Senta [Fri, 10 Mar 2023 12:33:59 +0000 (12:33 +0000)]
net/http: do not force the Content-Length header if nilled
According to the ResponseWriter documentation:
To suppress automatic response headers (such as "Date"), set
their value to nil.
In some cases, this documentation is incorrect: chunkWriter writes
a Content-Length header even if the value was set to nil. Meaning
there is no way to suppress this header.
This patch replaces the empty string comparison with a call to
`header.has` which takes into account nil values as expected.
This is similar to the way we handle the "Date" header.
Austin Clements [Mon, 8 May 2023 15:52:48 +0000 (11:52 -0400)]
cmd/cgo: enable test with non-sensible build tag
The build tag on this file is currently unsatisfiable. It was clearly
supposed to be "linux || freebsd || openbsd", but the test doesn't
actually compile on FreeBSD or OpenBSD because they don't define
SYS_gettid. Change the build tag to just "linux".
qmuntal [Fri, 5 May 2023 16:17:18 +0000 (18:17 +0200)]
os, syscall: support ill-formed UTF-16 strings on Windows
Windows UTF-16 strings can contain unpaired surrogates, which can't be
decoded into a valid UTF-8 string. This file defines a set of functions
that can be used to encode and decode potentially ill-formed UTF-16
strings by using the
[the WTF-8 encoding](https://simonsapin.github.io/wtf-8/).
WTF-8 is a strict superset of UTF-8, i.e. any string that is
well-formed in UTF-8 is also well-formed in WTF-8 and the content
is unchanged. Also, the conversion never fails and is lossless.
The benefit of using WTF-8 instead of UTF-8 when decoding a UTF-16
string is that the conversion is lossless even for ill-formed
UTF-16 strings. This property allows to read an ill-formed UTF-16
string, convert it to a Go string, and convert it back to the same
original UTF-16 string.
James Bartlett [Wed, 3 May 2023 18:27:05 +0000 (18:27 +0000)]
cmd/link: fix checks for supported linker flags with relative paths.
The existing way of checking for supported linker flags causes false negatives
when there are relative paths passed to go tool link. This fixes the issue by
calling the external linker in the current working directory, instead of
in a temporary directory.
Fixes #59952
Change-Id: I173bb8b44902f30dacefde1c202586f87667ab70
Reviewed-on: https://go-review.googlesource.com/c/go/+/491796
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Dmitri Shuralyov [Fri, 12 May 2023 17:00:45 +0000 (13:00 -0400)]
cmd/dist: use registerStdTestSpecially for normal Go tests only
It was my oversight in CL 463276 to skip registerStdTestSpecially
packages in the race bench test registration loop. Package testdir
has no benchmarks and doesn't need to be skipped. (And if it had
benchmarks, it's very unlikely they'd need any special handling.)
By now there are more cmd/cgo/internal/... packages that are registered
specially, and there isn't a need for their few benchmarks not to be
used for the purpose of race bench tests. If the 3 benchmarks in
cmd/cgo/internal/test were to require something special, then we can
add it to a new registerRaceBenchTestSpecially map with a comment, and
do register them specially in registerTests instead of forgetting to.
This restores the automatic 'go_test_bench:cmd/cgo/internal/test'
registration and reduces prevalence of registerStdTestSpecially a bit.
For #37486.
For #56844.
Change-Id: I1791fe5bf94cb4b4e0859c5fff4e7a3d5a23723e
Reviewed-on: https://go-review.googlesource.com/c/go/+/494656 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Cherry Mui [Mon, 10 Oct 2022 22:05:16 +0000 (18:05 -0400)]
test: add escape test for reflect.Value operations
With CL 408826 reflect.Value does not always escape. We need to
make sure Value operations does (or does not) escape the Value
correctly. This CL adds a test.
There are still a few unfortunate cases, where some Value
operations escape more than necessary (comparing to a non-reflect
version of the code), but hard to fix. These are mostly that a
Value would escape conditionally (mostly on the type of the Value),
but currently we don't have a good way to express that.
Change-Id: I9fdfc7584670aa09c5a01f6b2803f2043aaddb65
Reviewed-on: https://go-review.googlesource.com/c/go/+/441938
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Ian Lance Taylor [Fri, 17 Mar 2023 20:30:22 +0000 (13:30 -0700)]
os, runtime: remove unused implementations of os.sigpipe
Clean up instances that are unused since CL 6450058.
Change-Id: I0e9ae28cfa83fcc8abda8f5eca9c7dfc2c1c4ad1
Reviewed-on: https://go-review.googlesource.com/c/go/+/477396 Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
Cherry Mui [Fri, 7 Oct 2022 22:11:10 +0000 (18:11 -0400)]
reflect: make Value.IsZero not escape
With CL 408826 reflect.Value not always escape. IsZero still
escapes the Value because in some cases it passes the Value
pointer to the equal function, which is function pointer. Equal
functions are compiler generated and never escapes, but the escape
analysis doesn't know. Add noescape to help.
Change-Id: Ica397c2be77cac9e8a46d03d70bac385b0aa9e82
Reviewed-on: https://go-review.googlesource.com/c/go/+/441937 Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 17 Jun 2022 17:06:16 +0000 (13:06 -0400)]
reflect: do not escape Value.Type
Types are either static (for compiler-created types) or heap
allocated and always reachable (for reflection-created types, held
in the central map). So there is no need to escape types.
With CL 408826 reflect.Value does not always escape. Some functions
that escapes Value.typ would make the Value escape without this CL.
Had to add a special case for the inliner to keep (*Value).Type
still inlineable.
Change-Id: I7c14d35fd26328347b509a06eb5bd1534d40775f
Reviewed-on: https://go-review.googlesource.com/c/go/+/413474 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 27 May 2022 19:44:40 +0000 (15:44 -0400)]
reflect: allow Value be stack allocated
Currently, reflect.ValueOf forces the referenced object to be heap
allocated. This CL makes it possible to be stack allocated. We
need to be careful to make sure the compiler's escape analysis can
do the right thing, e.g. channel send, map assignment, unsafe
pointer conversions.
Tests will be added in a later CL.
CL 408827 might help ensure the correctness.
Change-Id: I8663651370c7c8108584902235062dd2b3f65954
Reviewed-on: https://go-review.googlesource.com/c/go/+/408826
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Michael Pratt [Fri, 12 May 2023 15:54:47 +0000 (11:54 -0400)]
cmd/compile: remove post-inlining PGO graph dump
The RedirectEdges logic is fragile and not quite complete (doesn't
update in-edges), which adds overhead to maintaining this package.
In my opinion, the post-inlining graph doesn't provide as much value as
the pre-inlining graph. Even the latter I am not convinced should be in
the compiler rather than an external tool, but it is comparatively
easier to maintain.
Drop it for now. Perhaps we'll want it back in the future for tracking
follow-up optimizations, but for now keep things simple.
Change-Id: I3133a2eb97893a14a6770547f96a3f1796798d17
Reviewed-on: https://go-review.googlesource.com/c/go/+/494655
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Michael Pratt [Fri, 12 May 2023 14:33:14 +0000 (10:33 -0400)]
cmd/compile/internal/pgo: remove node weights from IRNode
Actual PGO operation doesn't use these weights at all. They are
theoretically used when printing a dot graph for debugging, but that
doesn't actually work because these weights are always zero.
These fields are initialized by looking for a NodeMap entry with key
{CallerName: sym, CalleeName: "", CallSiteOffset: 0}. These entries will
never exist, as we never put entries in NodeMap without CalleeName.
Since they aren't really used and don't work, just remove them entirely,
which offers nice simplification.
This leaves IRNode with just a single field. I keep the type around as a
future CL will make the *ir.Func optional, allowing nodes with a name
but no IR.
Change-Id: I1646654cad1d0779ce071042768ffad2a7e6ff49
Reviewed-on: https://go-review.googlesource.com/c/go/+/494616
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Carlos Amedee [Mon, 8 May 2023 15:17:59 +0000 (11:17 -0400)]
time: update windows zoneinfo_abbrs
The primary branch of the github.com/unicode-org/cldr repository
has changed to main instead of master. This changes the branch used
to download the Windows zone file.
Ran:
go generate time
For #58113
Change-Id: Idb3dcaf44fc52d4b6abfed5a3ca6cd6f745dc3f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/493477 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Auto-Submit: Carlos Amedee <carlos@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Carlos Amedee <carlos@golang.org>
Austin Clements [Fri, 12 May 2023 19:19:23 +0000 (15:19 -0400)]
cmd/cgo/internal/testcarchive: build on all platforms
This test package uses syscall.SIGSEGV and syscall.SIGPIPE, which are
defined on most, but not all platforms. Normally this test runs as
part of dist test, which only registers this test on platforms that
support c-archive build mode, which includes all platforms that define
these signals. But this doesn't help if you're just trying to type
check everything in cmd.
Add build constraints so that this package type checks on all
platforms.
Austin Clements [Fri, 12 May 2023 19:19:23 +0000 (15:19 -0400)]
cmd/cgo/internal/testsanitizers: build on all platforms
This test package uses the Pdeathsig field of syscall.SysProcAttr,
which is only available on a few platforms. Currently, dist test
checks for compatible platforms and only registers it as part of
all.bash on platforms where it can build. But this doesn't help if
you're just trying to type check everything in cmd.
Make this package pass type checking by moving the condition from dist
into build tags on the test package itself.
Mateusz Poliwczak [Tue, 2 May 2023 18:40:56 +0000 (18:40 +0000)]
net: add tests for forceGoDNS and forceCgoDNS
There was a bug in forceCgoDNS (CL 479416), it was fixed by CL 487196, so
add a test case for it.
Change-Id: I2010374451ef236dc2898d9e9ea006eb8b40d02e
GitHub-Last-Rev: 34a84fad33404c66c3ee20cb63803214c42e991d
GitHub-Pull-Request: golang/go#59922
Reviewed-on: https://go-review.googlesource.com/c/go/+/491255 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@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> Reviewed-by: Ian Lance Taylor <iant@google.com>
Dmitri Shuralyov [Sun, 7 May 2023 02:52:06 +0000 (22:52 -0400)]
cmd/internal/bootstrap_test: update TestExperimentToolID for Go 1.21
This test is configured to run only when explicitly requested due to
being costly. Apply two updates so it can run on the toolchain today:
- overlay GOROOT/lib for zoneinfo.zip (similarly to CL 462279)
- stop expecting framepointer to be listed in the GOEXPERIMENT
section of the compiler version (see CL 49252 and CL 249857)
I checked if by now there's another test that would report a problem
if the fix made in CL 186200 had regressed. Running all.bash locally
with GO_TEST_SHORT=0 GO_BUILDER_NAME=darwin-arm64-longtest passed ok,
while this manual test did catch the problem.
Also simplify the test implementation while here so it's less different
from TestRepeatBootstrap.
For #33091.
Change-Id: I14eea18c19c2e8996bcba31c80e03dcf679f56ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/493475 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Zeke Lu [Tue, 9 May 2023 23:25:50 +0000 (23:25 +0000)]
cmd/go/internal/modload: reject the -modfile flag in workspace mode
Currently, in workspace mode, the -modfile flag affects all the modules
listed in the go.work file. This is not desirable most of the time. And
when it results in an error, the error message does not help.
For example, when there are more than one modules listed in the go.work
file, running "go list -m -modfile=path/to/go.mod" gives this error:
go: module example.com/foo appears multiple times in workspace
This change reject -modfile flag explicitly with this error message:
go: -modfile cannot be used in workspace mode
While at here, correct some typos in the modload package.