This CL marks some darwin assembly functions as NOFRAME to avoid relying
on the implicit amd64 NOFRAME heuristic, where NOSPLIT functions
without stack were also marked as NOFRAME.
Change-Id: I797f3909bcf7f7aad304e4ede820c884231e54f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/460235
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
runtime·cbctxts has been unused since CL 258938, but it was left over.
Change-Id: I374ad26e668a36994e41f5d17593b33090bdc644
Reviewed-on: https://go-review.googlesource.com/c/go/+/463119
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This CL adds the NOFRAME flag to runtime·rt0_go, which should
had been added in CL 459395 but didn't and broke fix windows-amd64-2008.
Change-Id: I4583f2034bf114e1f7aaddef9ba505f53536c3eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/463120
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Update race_windows_amd64.syso to latest tsan (V3) runtime.
This version of the runtime depends on libsynchronization.a, so to
use this syso, you need to also be using a sufficiently up to date
version of GCC (notably GCC 5.1, installed on the Go windows builders
right now, does not include this library).
Updates #48231.
Updates #35006.
Fixes#49761.
Change-Id: Ia1e2b1d8fe7e2c99728150734935a2c522006caa
Reviewed-on: https://go-review.googlesource.com/c/go/+/420197
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Replace deprecated // +build lines by their respective //go:build line
counterpart. Also remove build constraints implied by file name or type.
Change-Id: I8d18cd40071ca28d7654da8f0d22841f43729ca6
Reviewed-on: https://go-review.googlesource.com/c/go/+/460538
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL marks non-leaf nosplit assembly functions as NOFRAME to avoid
relying on the implicit amd64 NOFRAME heuristic, where NOSPLIT functions
without stack were also marked as NOFRAME.
Updates #57302
Updates #40044
Change-Id: Ia4d26f8420dcf2b54528969ffbf40a73f1315d61
Reviewed-on: https://go-review.googlesource.com/c/go/+/459395
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL removes badsignal2 function, as it is unused on Windows.
badsignal2 was originally intended to abort the process when
an exception was raised on a non-Go thread, following the same approach
as Linux and others.
Since it was added, back on https://golang.org/cl/5797068, it has caused
several issues on Windows, see #8224 and #50877. That's because we can't
know wether the signal is bad or not, as our trap might not be at the
end of the exception handler chain.
To fix those issues, https://golang.org/cl/104200046 and CL 442896
stopped calling badsignal2, and CL 458135 removed one last incorrect
call on amd64 and 386.
Change-Id: I5bd31ee2672118ae0f1a2c8b46a1bb0f4893a011
Reviewed-on: https://go-review.googlesource.com/c/go/+/463116
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL factors out part of the Windows sigtramp implementation, which
was duplicated in all four architectures. The new common code is
implemented in Go rather than in assembly, which will make Windows
error handling easier to reason and maintain.
While here, implement the control flow guard workaround on
windows/386, which almost comes for free.
Change-Id: I0bf38c28c54793225126e161bd95527a62de05e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/458135
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This CL updates TestVectoredHandlerDontCrashOnLibrary so it can run on
windows/386 and windows/arm64. It still can't run on windows/arm as
it does not support c-shared buildmode (see #43800).
Change-Id: Id1577687e165e77d27633c632634ecf86e6e9d6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/463117
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This CL redesign how we get the TLS pointer on windows/i386.
It applies the same changes as done in CL 431775 for windows/amd64.
We were previously reading it from the [TEB] arbitrary data slot,
located at 0x14(FS), which can only hold 1 TLS pointer.
With this CL, we will read the TLS pointer from the TEB TLS slot array,
located at 0xE10(GS). The TLS slot array can hold multiple
TLS pointers, up to 64, so multiple Go runtimes running on the
same thread can coexists with different TLS.
Each new TLS slot has to be allocated via [TlsAlloc],
which returns the slot index. This index can then be used to get the
slot offset from GS with the following formula: 0xE10 + index*4.
The slot index is fixed per Go runtime, so we can store it
in runtime.tls_g and use it latter on to read/update the TLS pointer.
Loading the TLS pointer requires the following asm instructions:
MOVQ runtime.tls_g, AX
MOVQ AX(FS), AX
Notice that this approach will now be implemented in all the supported
windows arches.
[TEB]: https://en.wikipedia.org/wiki/Win32_Thread_Information_Block
[TlsAlloc]: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-tlsalloc
Change-Id: If4550b0d44694ee6480d4093b851f4991a088b32
Reviewed-on: https://go-review.googlesource.com/c/go/+/454675
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Previously TryBot-tested with bucket bits = 4.
Also tested locally with bucket bits = 5.
This makes it much easier to change the size of map
buckets, and hopefully provides pointers to all the
code that in some way depends on details of map layout.
Change-Id: I9f6669d1eadd02f182d0bc3f959dc5f385fa1683
Reviewed-on: https://go-review.googlesource.com/c/go/+/462115
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Change-Id: Ibffe46bad7d30df9380ba18d49eeb6782406a1aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/463115
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
When walking through the set of coverage data files generated from a
"go test -cover" run, it's possible to encounter pods (clumps of data
files) that were generated by a run from an instrumented Go tool (for
example, cmd/compile). Add a guard to the test reporting code to
ensure that it only processes files created by the currently running
test.
Fixes#57924.
Change-Id: I1bb7dce88305e1088162e3cb1df628486ecee1c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/462756
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Currently runtime.Breakpoint generates SIGSEGV in s390x.
The solution to this is add new asm instruction BRRK of
type FORMAT_E for the breakpoint exception.
Fixes#52103
Change-Id: I8358a56e428849a5d28d5ade141e1d7310bee084
Reviewed-on: https://go-review.googlesource.com/c/go/+/457456
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
The number 104 appears to date back to the
first implementation of split stacks in
https://go.googlesource.com/go/+/b987f7a757f53f460973622a36eebb696f9b5060.
That change introduces a 104 byte stack guard.
it doesn't makes any sense today.
Change-Id: I73069f6d1a827653af63e616f0119fbac809882e
GitHub-Last-Rev: bcf9000590
GitHub-Pull-Request: golang/go#56594
Reviewed-on: https://go-review.googlesource.com/c/go/+/448036
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
This is the second round to look for spelling mistakes. This time the
manual sifting of the result list was made easier by filtering out
capitalized and camelcase words.
grep -r --include '*.go' -E '^// .*$' . | aspell list | grep -E -x '[A-Za-z]{1}[a-z]*' | sort | uniq
This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.
Change-Id: Ie8a2092aaa7e1f051aa90f03dbaf2b9aaf5664a9
GitHub-Last-Rev: fc2bd6e0c5
GitHub-Pull-Request: golang/go#57737
Reviewed-on: https://go-review.googlesource.com/c/go/+/461595
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@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Allow GODEBUG users to report how many times a setting
resulted in non-default behavior.
Record non-default-behaviors for all existing GODEBUGs.
Also rework tests to ensure that runtime is in sync with runtime/metrics.All,
and generate docs mechanically from metrics.All.
For #56986.
Change-Id: Iefa1213e2a5c3f19ea16cd53298c487952ef05a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/453618
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
func f() {
defer func() {
if err := recover(); err != nil {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
}
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
Instead you have to write something like:
func f() {
done := false
defer func() {
if err := recover(); !done {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
done = true
}
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for #56986.
Fixes#25448.
Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
These tests were only run on GOARCH=amd64, but the rationale given in
CL 11858043 was GC precision on 32-bit platforms. Today, we have far
more 64-bit platforms than just amd64, and I believe that GC precision
on 32-bit platforms has been substantially improved as well.
The GOARCH restriction seems unnecessary.
Updates #57166.
Updates #5368.
Change-Id: I45c608b6fa721012792c96d4ed94a6d772b90210
Reviewed-on: https://go-review.googlesource.com/c/go/+/456120
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Austin Clements <austin@google.com>
In the fix for 54332 the MOVD R1, R1 instruction was added to
morestack_noctxt function to set the SPWRITE bit. However, the
instruction MOVD R1, R1 results in or r1,r1,r1 which is a special
instruction on ppc64 architecture as it changes the thread priority
and can negatively impact performance in some cases.
More details on such similar nops can be found in Power ISA v3.1
Book II on Power ISA Virtual Environment architecture in the chapter
on Program Priority Registers and Or instructions.
Replacing this by OR R0, R1 has the same affect on setting SPWRITE as
needed by the first fix but does not affect thread priority and
hence does not cause the degradation in performance
Hash65536-64 2.81GB/s ±10% 16.69GB/s ± 0% +494.44%
Fixes#57741
Change-Id: Ib912e3716c6afd277994d6c1c5b2891f82225d50
Reviewed-on: https://go-review.googlesource.com/c/go/+/461597
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Auto-Submit: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Fix incorrect comment for the runtime package.
Change-Id: Iab889eff0e9c622afbed959d32b8b5f0ed0bfebf
GitHub-Last-Rev: e9587868db
GitHub-Pull-Request: golang/go#57731
Reviewed-on: https://go-review.googlesource.com/c/go/+/461498
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Keith Randall <khr@google.com>
CL 451735 worked around bugs in Apple's atfork handlers by calling
notify_is_valid_token and xpc_atfork_child at startup, so that init
code that wouldn't be safe in the child process would be warmed up in
the parent process instead, but xpc_atfork_child broke use of the xpc
library in Go programs, and xpc is internally used by various macOS
frameworks (#57263).
CL 459175 reverted that change, and then CL 459176 tried a new
approach: use __fork, which doesn't call any of the atfork handlers at all.
That worked, but an Apple engineer reviewing the change in private
email suggests that since __fork is not public API, it should be avoided.
The same engineer (with access to the source code for the xpc library)
suggests that the breakage in #57263 is caused by xpc_atfork_child
marking the library as unusable, expecting an imminent call to exec,
and that calling xpc_date_create_from_current instead would do the
necessary initialization without marking xpc as unusable.
CL 460475 reverted that change, to prepare for this one.
This CL goes back to the original “call functions to warm things up”
approach, replacing xpc_atfork_child with xpc_date_create_from_current.
The CL also updates cmd/link to use OS and SDK version 10.13.0 for
x86 macOS binaries, up from 10.9.0, also suggested by the Apple engineer.
Combined with the two warmup calls, this makes the fork hangs go away.
The minimum macOS version has been 10.13 High Sierra since Go 1.17,
so there should be no problem with writing that in the binaries too.
Fixes#33565.
Fixes#56784.
Fixes#57263.
Fixes#57577.
Change-Id: I20769d9daa1fe9ea930f8009481335f8a14dc21b
Reviewed-on: https://go-review.googlesource.com/c/go/+/460476
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Fixes#54778
Change-Id: If9aef0c06b993ef2aedbeea9452297ee9f11fa06
Reviewed-on: https://go-review.googlesource.com/c/go/+/460461
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
I spent quite a while determining the cause of empty stacks in
profiles and reasoning out why this is okay. There isn't a great place
to record this knowledge, but a documentation comment on
appendLocsForStack is better than nothing.
Updates #51550.
Change-Id: I2eefc6ea31f1af885885c3d96199319f45edb4ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/460695
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
The current output of TestLabelSystemstack is a bit cryptic. This CL
improves various messages and hopefully simplifies the logic in the
test.
Simplifying the logic leads to three changes in possible outcomes,
which I verified by running the logic before and after this change
through all 2^4 possibilities (https://go.dev/play/p/bnfb-OQCT4j):
1. If a sample both must be labeled and must not be labeled, the test
now reports that explicitly rather than giving other confusing output.
2. If a sample must not be labeled but is, the current logic will
print two identical error messages. The new logic prints only one.
3. If the test finds no frames at all that it recognizes, but the
sample is labeled, it will currently print a confusing "Sample labeled
got true want false" message. The new logic prints nothing. We've seen
this triggered by empty stacks in profiles.
Fixes#51550. This bug was caused by case 3 above, where it was
triggered by a profile label on an empty stack. It's valid for empty
stacks to appear in a profile if we sample a goroutine just as it's
exiting (and that goroutine may have a profile label), so the test
shouldn't fail in this case.
Change-Id: I1593ec4ac33eced5bb89572a3ba7623e56f2fb3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/460516
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently in libfuzzer mode, we put our counters in section
__sancov_cntrs. When linking with C/C++ code that also has fuzzer
counters, apparently the C linker combines our counters and their
counters and registers them together. But in the Go runtime we
also have code to register our counters. So the Go counters ended
up registered twice, causing problems.
Since we already have code to register our counters, put them in
a Go-specific section so it won't be combined with the C counters.
Fixes#57449.
Change-Id: If3d41735124e7e301572d4b7aecf7d057ac134c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/459055
Reviewed-by: Nicolas Hillegeer <aktau@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Revert CL 451735 (1f4394a0c9), which fixed#33565 and #56784
but also introduced #57263.
I have a different fix to apply instead. Since the first fix was
never backported, it will be easiest to backport the new fix
if the new fix is done in a separate CL from the revert.
Change-Id: I6c8ea3a46e542ee4702675bbc058e29ccd2723e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/459175
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
The runtime/internal/startlinetest package contains a call to a
function defined in runtime_test. Generally this is fine as this
package is only linked in for runtime_test. Except that for "go
install -buildmode=shared std", which include all packages in std,
including this test-only internal package. In this mode, the
caller is included in the linking but the callee is not, causing
linking error. Work around it by calling
runtime_test.callerStartLine via a function pointer. The function
pointer is only set in runtime_test. In the shared std build, the
function pointer will not be set, and this is fine.
Fixes#57334.
Change-Id: I7d871c50ce6599c6ea2802cf6e14bb749deab220
Reviewed-on: https://go-review.googlesource.com/c/go/+/458696
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Only one of the runtime/race/internal/amd64vN packages should be
included in a build. Generally this is true because the
runtime/race package would import only one of them depending on
the build configuration. But for "go install -buildmode=shared std"
it includes all Go packages in std, which includes both, which
then causes link-time failure due to duplicated symbols. To avoid
this, we add build tags to the internal packages, so, depending on
the build configuation, only one package would contain buildable
go files therefore be included in the build.
For #57334.
Change-Id: I52ddc3a40e16c7d04b4dd861e9689918d27e8509
Reviewed-on: https://go-review.googlesource.com/c/go/+/458695
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The processPod() helper (invoked by processCoverTestDir, which is in
turn called by _testmain.go) was opening and reading counter data
files, but never closing them. Add a call to close the files after
they have been read.
Fixes#57407.
Change-Id: If9a489f92e4bab72c5b2df8697e14420a6f7b8f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/458835
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The function LoweredAtomicCas32 is implemented using the LL-SC instruction pair
on loong64, mips64x, riscv64. However,the LL instruction on loong64, mips64x,
riscv64 is sign-extended, so it is necessary to sign-extend the 2nd parameter
"old" of the LoweredAtomicCas32, so that the instruction BNE after LL can get
the desired result.
The function prototype of LoweredAtomicCas32 in golang:
func Cas32(ptr *uint32, old, new uint32) bool
When using an intrinsify implementation:
case 1: (*ptr) <= 0x80000000 && old < 0x80000000
E.g: (*ptr) = 0x7FFFFFFF, old = Rarg1= 0x7FFFFFFF
After run the instruction "LL (Rarg0), Rtmp": Rtmp = 0x7FFFFFFF
Rtmp ! = Rarg1(old) is false, the result we expect
case 2: (*ptr) >= 0x80000000 && old >= 0x80000000
E.g: (*ptr) = 0x80000000, old = Rarg1= 0x80000000
After run the instruction "LL (Rarg0), Rtmp": Rtmp = 0xFFFFFFFF_80000000
Rtmp ! = Rarg1(old) is true, which we do not expect
When using an non-intrinsify implementation:
Because Rarg1 is loaded from the stack using sign-extended instructions
ld.w, the situation described in Case 2 above does not occur
Benchmarks on linux/loong64:
name old time/op new time/op delta
Cas 50.0ns ± 0% 50.1ns ± 0% ~ (p=1.000 n=1+1)
Cas64 50.0ns ± 0% 50.1ns ± 0% ~ (p=1.000 n=1+1)
Cas-4 56.0ns ± 0% 56.0ns ± 0% ~ (p=1.000 n=1+1)
Cas64-4 56.0ns ± 0% 56.0ns ± 0% ~ (p=1.000 n=1+1)
Benchmarks on Loongson 3A4000 (GOARCH=mips64le, 1.8GHz)
name old time/op new time/op delta
Cas 70.4ns ± 0% 70.3ns ± 0% ~ (p=1.000 n=1+1)
Cas64 70.7ns ± 0% 70.6ns ± 0% ~ (p=1.000 n=1+1)
Cas-4 81.1ns ± 0% 80.8ns ± 0% ~ (p=1.000 n=1+1)
Cas64-4 80.9ns ± 0% 80.9ns ± 0% ~ (p=1.000 n=1+1)
Fixes#57282
Change-Id: I190a7fc648023b15fa392f7fdda5ac18c1561bac
Reviewed-on: https://go-review.googlesource.com/c/go/+/457135
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
The comment on `slicebytetostringtmp` mention that `==` operator does
not allocate []byte to string conversion, but the test was testing only
`==` and `!=` and the compiler actually optimizes all comparison
operators.
Also added a test for concatenation comparison, which also should not
allocate.
Change-Id: I6f4c5c4f238808138fa901732e1fd5b6ab25f725
Reviewed-on: https://go-review.googlesource.com/c/go/+/456415
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fix a buglet in cmd/cover in how we handle package name/path for the
"go build -o foo.exe *.go" and "go run *.go" cases.
The go command assigns a dummy import path of "command-line-arguments"
to the main package built in these cases; rather than expose this
dummy to the user in coverage reports, the cover tool had a special
case hack intended to rewrite such package paths to "main". The hack
was too general, however, and was rewriting the import path of all
packages with (p.name == "main") to an import path of "main". The hack
also produced unexpected results for cases such as
go test -cover foo.go foo_test.go
This patch removes the hack entirely, leaving the package path for
such cases as "command-line-arguments".
Fixes#57169.
Change-Id: Ib6071db5e3485da3b8c26e16ef57f6fa1712402c
Reviewed-on: https://go-review.googlesource.com/c/go/+/456237
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
These short timeouts can overrun due to system scheduling delay
(or GC latency) on a slow or heavily-loaded host.
Moreover, if the test deadlocks we will probably want to know what the
GC goroutines were doing at the time. With an arbitrary timeout, we
never get that information; however, if we allow the test to time out
completely we will get a goroutine dump (and, if GOTRACEBACK is
configured in the environment, that may even include GC goroutines).
Fixes#57166.
Change-Id: I136501883373c3ce4e250dc8340c60876b375f44
Reviewed-on: https://go-review.googlesource.com/c/go/+/456118
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
It is possible that CL 455166 fixes this. Try unskipping the test
and see. If it fails again we can skip it again.
Fixes#48655.
Change-Id: Ia81b06cb7608f74adb276bc018e8fc840285bc11
Reviewed-on: https://go-review.googlesource.com/c/go/+/455358
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In the profiler, when unwinding the stack, we have special
handling for VDSO calls. Currently, the special handling is only
used when the normal unwinding fails. If the signal lands in the
function that makes the VDSO call (e.g. nanotime1) and after the
stack switch, the normal unwinding doesn't fail but gets a stack
trace with exactly one frame (the nanotime1 frame). The stack
trace stops because of the stack switch. This 1-frame stack trace
is not as helpful. Instead, if vdsoSP is set, we know we are in
VDSO call or right before or after it, so use vdsoPC and vdsoSP
for unwinding. Do the same for libcall.
Also remove _TraceTrap for VDSO unwinding, as vdsoPC and vdsoSP
correspond to a call, not an interrupted instruction.
Fixes#56574.
Change-Id: I799aa7644d0c1e2715ab038a9eef49481dd3a7f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/455166
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Without it, at least on ARM64 with older BFD linker, it will
include the file of the object file (which is of a temporary path)
as a debug symbol into the binary, causing the build to be
nondeterministic. Adding a .file directive makes it to create a
STT_FILE symbol with deterministic input, and prevent the linker
creating one using the temporary object file path.
Fixes#57035.
Change-Id: I3ab716b240f60f7a891af2f7e10b467df67d1f31
Reviewed-on: https://go-review.googlesource.com/c/go/+/454838
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Pointed out in review of CL 453602,
but it looks like I forgot to re-upload before submitting.
Change-Id: I8f4fac52ea0f904f6f9b06e13fc8ed2e778f2360
Reviewed-on: https://go-review.googlesource.com/c/go/+/454835
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Julie Qiu <julieqiu@google.com>
A potential user did not realize Deps included all transitive dependencies,
not just direct dependencies of the main module. Clarify that and add
various other useful information.
Change-Id: I5b8e1314bb26092edbcc090ba8eb9859f0a70662
Reviewed-on: https://go-review.googlesource.com/c/go/+/453602
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Disable the "torture" portion of the maphash tests if -race is in
effect (these tests can cause timeouts on the longtest -race builder).
Fixes#57030.
Change-Id: I23d7561dac3e81d979cad9e0efa6f5b7154aadd2
Reviewed-on: https://go-review.googlesource.com/c/go/+/454455
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
A couple of the windows runtime tests were being gated by "if
testenv.Builder() == ..." guards that referred to builders that have
long since been obsoleted (e.g. "windows-amd64-gce"). Use a more
generic guard instead, checking for windows-<goarch> prefix.
Change-Id: Ibdb9ce2b0cfe10bba986bd210a5b8ce5c1b1d675
Reviewed-on: https://go-review.googlesource.com/c/go/+/453035
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
All mips variant perform syscalls similarly. R2 (v0) holds r1 and R3
(v1) holds r2 of a syscall. The latter is only used by 2-ret syscalls.
A 1-ret syscall would not touch R3 but keeps it as is, making r2 be a
random value. Always reset it to 0 before SYSCALL to fix the issue.
Fixes#56426
Change-Id: Ie49965c0c3c224c4a895703ac659205cd040ff56
Reviewed-on: https://go-review.googlesource.com/c/go/+/452975
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Meng Zhuo <mzh@golangcn.org>
Keith Randall
Bryan C. Mills
Quim Muntal
Than McIntosh
Tobias Klauser
Jakub Ciolek
j2gg0s
David Chase
Daniel Martí
Srinivas Pokala
liu-xuewen
Marcel Meyer
Russ Cox
Archana R
zhengchaopu
Ian Lance Taylor
Austin Clements
Cherry Mui
Guoqi Chen
Oleg Zaytsev
Rongrong