| Age | Commit message (Collapse) | Author |
|
The test was skipped when darwin/arm64 meant iOS, and our LLDB
runner could not handle signals. Now darwin/arm64 is macOS, it
works just fine.
Change-Id: I04b2b60046c6a0df01a5d5c23d7203d469af20de
Reviewed-on: https://go-review.googlesource.com/c/go/+/749941
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Loosely based on CL 678795.
The PE/COFF spec requires RUNTIME_FUNCTION entries in the .pdata
section to be ordered by their function start address. Previously
the linker emitted them in symbol order.
An unsorted table triggers the MSVC linker error:
fatal error LNK1223: invalid or corrupt file: file contains invalid
.pdata contributions
Fixes #65116.
Change-Id: I589cb4e6787a9edb34400b56e60fe23065b59162
Reviewed-on: https://go-review.googlesource.com/c/go/+/743820
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
When internally linking darwin binaries, the linker rejected Mach-O
UNSIGNED (pointer) relocations targeting dynamic import symbols,
producing errors like:
unexpected reloc for dynamic symbol _swift_FORCE_LOAD_$_swiftIOKit
These relocations are legitimate and appear in data sections (e.g.
__DATA/__const) of object files that reference external symbols such as
Swift force-load symbols. The dynamic linker (dyld) needs to bind these
pointers at load time.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64_15,gotip-darwin-amd64_14
Change-Id: I1cc759dec28b8aa076602a45062f403d0d9f45fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/745220
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
This CL follows from the abandoned CL 722961.
Alan Donovan asked if modernize would catch these changes; it does in part.
Here is how modernize was used:
1. Build the Go compiler from latest master
2. Clone x/tools and build modernize with the newest compiler
3. Then, do:
PATH=PATH_TO_BIN:$PATH go list ./... | xargs env PATH=PATH_TO_BIN:$PATH GOMOD=off MODERNIZE -forvar -fix -test -debug fpstv
4. For assurance, move into a package directory (i.e, cmd), and redo Step 3.
From the obtained result, it seems modernize did not remove the loop variable when:
- the range notation was not used
- the loop variable was not directly under the for directive.
Which does happens here:
# git ls-files | xargs -I {} perl -nE 'print "$ARGV:$.:$_" if /\s+(\w+) := \1$/' {} | grep _test
[...]
cmd/compile/internal/types2/api_test.go:2423: i := i
md/go/internal/modload/query_test.go:182: tt := tt
md/go/internal/web/url_test.go:17: tc := tc
cmd/go/internal/web/url_test.go:49: tc := tc
cmd/internal/par/queue_test.go:54: i := i
runtime/syscall_windows_test.go:781: arglen := arglen
Link: https://go-review.googlesource.com/c/go/+/722961/comments/e8d47866_fc399fa1
Co-authored-by: Plamerdi Makela <plamerdi447@gmail.com>
Fixes #76411
Change-Id: I0c191cdca70dbea6efaf6796dca9c60e2afcd9ea
GitHub-Last-Rev: 77c3e11fc21a4ede70b733b567a1690edb944dc1
GitHub-Pull-Request: golang/go#77694
Reviewed-on: https://go-review.googlesource.com/c/go/+/746502
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
|
|
Test that C code can call back into Go (on a separate thread)
before init is done. The callback should wait for the init done.
And this should not cause a false race.
This corresponds to the runtime fix in CL 746581.
Change-Id: I10e6a9d5fe056be16143f5f6a322f51e56fc9fdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/746780
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
We used to do this but it broke in Go 1.10. This restores the rewrite,
but only applied to compiler output for packages that use cgo.
That is all that the original rewrite applied to anyhow.
Fixes #76339
Change-Id: Ife8ee858ddd0ff7bcc7423455b2eabf8381b7bde
Reviewed-on: https://go-review.googlesource.com/c/go/+/721821
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
|
|
Updates #76697
Change-Id: If24eec2bc2f8bfd903a4cc8f5499e77ea2f255c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/736780
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Fixes #76861
Change-Id: Icc8452e48ed736e8240f8afea18637c33b8e3ef8
Reviewed-on: https://go-review.googlesource.com/c/go/+/730600
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Joe Richey <joerichey@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
|
|
When we first implemented DIT (crypto/subtle.WithDataIndependentTiming),
we made it so that enabling DIT on a goroutine would lock that goroutine
to its current OS thread. This was done to ensure that the DIT state
(which is per-thread) would not leak to other goroutines. We also did
not make goroutines inherit the DIT state.
This change makes goroutines inherit the DIT state from their parent
at creation time. It also removes the OS thread locking when enabling
DIT on a goroutine. Instead, we now set the DIT state on the OS thread
in the scheduler whenever we switch to a goroutine that has DIT enabled,
and we unset it when switching to a goroutine that has DIT disabled.
We add a new field to G and M, ditEnabled, to track whether the G wants
DIT enabled, and whether the M currently has DIT enabled, respectively.
When the scheduler executes a goroutine, it checks these fields and
enables/disables DIT on the thread as needed.
Additionally, cgocallbackg is updated to check if DIT is enabled when
being called from C, and sets the G and M fields accordingly. This
ensures that if DIT was enabled/disabled in C, the correct state will be
reflected in the Go runtime.
The behavior as it currently stands is as follows:
- The function passed to crypto/subtle.WithDataIndependentTiming
will have DIT enabled.
- Any goroutine created within that function will inherit DIT enabled
for its lifetime. Any goroutine created from subquent goroutines will
also inherit DIT enabled for their lifetimes.
- Calling into a C function within from a goroutine with DIT enabled
will have DIT enabled.
- If the C code disables DIT, the goroutine will have DIT re-enabled
when returning to Go.
- If the C code enables DIT, the goroutine will have DIT disabled
when returning to Go if it was not previously enabled.
- Calling back into Go code from C will have DIT enabled if it was
enabled when calling into C, or if the C code enabled it.
Change-Id: I8e91e6df13bb88e56e1036e0e0e5f04efd8eebd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/726382
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
When using `any` as param or return type of an exported
function, we currently have the error `unrecognized Go
type any`. `any` is an alias of `interface{}` which is
already supported.
This would avoid such change: https://github.com/php/frankenphp/pull/1976
Fixes #76340
Change-Id: I301838ff72e99ae78b035a8eff2405f6a145ed1a
GitHub-Last-Rev: 7dfbccfa582bbc6e79ed29677391b9ae81a9b5bd
GitHub-Pull-Request: golang/go#76325
Reviewed-on: https://go-review.googlesource.com/c/go/+/720960
Reviewed-by: Mark Freeman <markfreeman@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Otherwise we can't assign to it.
Fixes #75751
Change-Id: Iba680db672297bca1a1d1a33912b80863da66a08
Reviewed-on: https://go-review.googlesource.com/c/go/+/717342
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
This change mechanically replaces all occurrences of interface{}
by 'any' (where deemed safe by the 'any' modernizer) throughout
std and cmd, minus their vendor trees.
Since this fix is relatively numerous, it gets its own CL.
Also, 'go generate go/types'.
Change-Id: I14a6b52856c3291c1d27935409bca8d5fd4242a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/719702
Commit-Queue: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
|
|
This CL improves the error messages after panics due to the
sharing of an unpinned Go pointer (or a pointer to an unpinned Go
pointer) between Go and C.
This occurs when it is:
1. returned from Go to C (through cgoCheckResult)
2. passed as argument to a C function (through cgoCheckPointer).
An unpinned Go pointer refers to a memory location that might be moved or
freed by the garbage collector.
Therefore:
- change the signature of cgoCheckArg (it does the real work behind
cgoCheckResult and cgoCheckPointer)
- change the signature of cgoCheckUnknownPointer (called by cgoCheckArg
for checking unexpected pointers)
- introduce cgoFormatErr (it is called by cgoCheckArg and
cgoCheckUnknownPointer to format panic error messages)
- update the cgo pointer tests (add new tests, and a field errTextRegexp
to the struct ptrTest)
- remove a loop variable in TestPointerChecks (similar to CL 711640).
1. cgoCheckResult
When an unpinned Go pointer (or a pointer to an unpinned Go pointer) is
returned from Go to C,
1 package main
2
3 /*
4 #include <stdio.h>
5
6 extern void* GoFoo();
7
8 static void CFoo() { GoFoo();}
9 */
10 import (
11 "C"
12 )
13
14 //export GoFoo
15 func GoFoo() map[int]int {
16 return map[int]int{0: 1,}
17 }
18
19 func main() {
20 C.CFoo();
21 }
This error shows up at runtime:
panic: runtime error: cgo result is unpinned Go pointer or points to unpinned Go pointer
goroutine 1 [running]:
main._Cfunc_CFoo()
_cgo_gotypes.go:46 +0x3a
main.main()
/mnt/tmp/parse.go:20 +0xf
exit status 2
GoFoo is the faulty Go function; it is not mentioned in the error message.
Moreover the error does not say which kind of pointer caused the panic; for
instance, a Go map.
Retrieve name and file/line of the Go function, as well as the kind of
pointer; use them in the error message:
panic: runtime error: /mnt/tmp/parse.go:15: result of Go function GoFoo called from cgo is unpinned Go map or points to unpinned Go map
goroutine 1 [running]:
main._Cfunc_CFoo()
_cgo_gotypes.go:46 +0x3a
main.main()
/mnt/tmp/parse.go:20 +0xf
exit status 2
2. cgoCheckPointer
When a pointer to an unpinned Go pointer is passed to a C function,
1 package main
2
3 /*
4 #include <stdio.h>
5 void foo(void *bar) {}
6 */
7 import "C"
8 import "unsafe"
9
10 func main() {
11 m := map[int]int{0: 1,}
12 C.foo(unsafe.Pointer(&m))
13 }
This error shows up at runtime:
panic: runtime error: cgo argument has Go pointer to unpinned Go pointer
goroutine 1 [running]:
main.main.func1(...)
/mnt/tmp/cgomap.go:12
main.main()
/mnt/tmp/cgomap.go:12 +0x91
exit status 2
Retrieve kind of pointer; use it in the error message.
panic: runtime error: argument of cgo function has Go pointer to unpinned Go map
goroutine 1 [running]:
main.main.func1(...)
/mnt/tmp/cgomap.go:12
main.main()
/mnt/tmp/cgomap.go:12 +0x9b
exit status 2
Link: https://pkg.go.dev/cmd/cgo#hdr-Passing_pointers
Suggested-by: Ian Lance Taylor <iant@golang.org>
Fixes #75856
Change-Id: Ia72f01df016feeae0cddb2558ced51a1b07e4486
GitHub-Last-Rev: 76257c7dd7c52a3d88234d43ec7f22bda81edcdd
GitHub-Pull-Request: golang/go#75894
Reviewed-on: https://go-review.googlesource.com/c/go/+/711801
Reviewed-by: Funda Secgin <fundasecgin73@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
|
|
Symbols loaded from host files can have the N_WEAK_REF bit set,
which is used to instruct the loader to not fail if that symbol
can't be resolved.
The Go internal linker should honor this information by setting the
BIND_SYMBOL_FLAGS_WEAK_IMPORT flag in the corresponding bind table
entry.
Fixes #76023
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64-longtest,gotip-darwin-amd64_12,gotip-darwin-arm64_12,gotip-darwin-arm64_15,gotip-darwin-arm64-longtest,gotip-darwin-amd64_14
Change-Id: Id2cef247ec7a9cb08455844f3c30ff874772bb7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/713760
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Change-Id: I26302d801732f40b1fe6b30ff69d222047bca490
Reviewed-on: https://go-review.googlesource.com/c/go/+/716740
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Change-Id: Iae74ac910d9db035bb2b726b2128aac09f5b7aae
GitHub-Last-Rev: 7aab5fa8e76df87766e903d7a689ed257d986558
GitHub-Pull-Request: golang/go#75993
Reviewed-on: https://go-review.googlesource.com/c/go/+/713540
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
Found by github.com/mdempsky/unconvert
Change-Id: I88ce10390a49ba768a4deaa0df9057c93c1164de
GitHub-Last-Rev: 3b0f7e8f74f58340637f33287c238765856b2483
GitHub-Pull-Request: golang/go#75974
Reviewed-on: https://go-review.googlesource.com/c/go/+/712940
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
Irregularly typedmemmove and bulkBarrierPreWrite crashes on unaligned
arguments. By aligning the arguments this is fixed.
Fixes #46893
Change-Id: I7beb9fdc31053fcb71bee6c6cb906dea31718c56
GitHub-Last-Rev: 46ae8b96889644aab60ea4284cf447a740354c6a
GitHub-Pull-Request: golang/go#74868
Reviewed-on: https://go-review.googlesource.com/c/go/+/692935
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Binutils defaults to exporting all symbols when building a Windows DLL.
To avoid that we were marking symbols with __declspec(dllexport) in
the cgo-generated headers, which instructs ld to export only those
symbols. However, that approach makes the headers hard to reuse when
importing the resulting DLL into other projects, as imported symbols
should be marked with __declspec(dllimport).
A better approach is to generate a .def file listing the symbols to
export, which gets the same effect without having to modify the headers.
Updates #30674
Fixes #56994
Change-Id: I22bd0aa079e2be4ae43b13d893f6b804eaeddabf
Reviewed-on: https://go-review.googlesource.com/c/go/+/705776
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Than McIntosh <thanm@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
When calling into C via cmd/cgo, the generated code calls
_cgo_tsan_acquire / _cgo_tsan_release around the C call to report a
dummy lock to the C/C++ TSAN runtime. This is necessary because the
C/C++ TSAN runtime does not understand synchronization within Go and
would otherwise report false positive race reports. See the comment in
cmd/cgo/out.go for more details.
Various C functions in runtime/cgo also contain manual calls to
_cgo_tsan_acquire/release where necessary to suppress race reports.
However, the cgo symbolizer and cgo traceback functions called from
callCgoSymbolizer and cgoContextPCs, respectively, do not have any
instrumentation [1]. They call directly into user C functions with no
TSAN instrumentation.
This means they have an opportunity to report false race conditions. The
most direct way is via their argument. Both are passed a pointer to a
struct stored on the Go stack, and both write to fields of the struct.
If two calls are passed the same pointer from different threads, the C
TSAN runtime will think this is a race.
This is simple to achieve for the cgo symbolizer function, which the
new regression test does. callCgoSymbolizer is called on the standard
goroutine stack, so the argument is a pointer into the goroutine stack.
If the goroutine moves Ms between two calls, it will look like a race.
On the other hand, cgoContextPCs is called on the system stack. Each M
has a unique system stack, so for it to pass the same argument pointer
on different threads would require the first M to exit, free its stack,
and the same region of address space to be used as the stack for a new
M. Theoretically possible, but quite unlikely.
Both of these are addressed by providing a C wrapper in runtime/cgo that
calls _cgo_tsan_acquire/_cgo_tsan_release around calls to the symbolizer
and traceback functions.
There is a lot of room for future cleanup here. Most runtime/cgo
functions have manual instrumentation in their C implementation. That
could be removed in favor of instrumentation in the runtime. We could
even theoretically remove the instrumentation from cmd/cgo and move it
to cgocall. None of these are necessary, but may make things more
consistent and easier to follow.
[1] Note that the cgo traceback function called from the signal handler
via x_cgo_callers _does_ have manual instrumentation.
Fixes #73949.
Cq-Include-Trybots: luci.golang.try:gotip-freebsd-amd64,gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Change-Id: I6a6a636c9daa38f7fd00694af76b75cb93ba1886
Reviewed-on: https://go-review.googlesource.com/c/go/+/677955
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
TestNumberOfExportedFunctions checks the number of exported functions
announced in the PE export table, getting it from the .edata section.
If the section is not found, the test is skipped. However, the PE spec
doesn't mandate that the export table be in a section named .edata,
making this test prone to being skipped unnecessarily.
While here, remove a check in cmd/go.testBuildmodePIE that was testing
the same thing in order to verify that the binary had a relocation table
. Not only the test is duplicated, but also it in unnecessary because
it already testing that the PE characteristics doesn't contain the
IMAGE_FILE_RELOCS_STRIPPED flag.
Closes #46719
Cq-Include-Trybots: luci.golang.try:gotip-windows-arm64
Change-Id: I28d1e261b38388868dd3c19ef6ddddad7bf105ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/705755
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
The Universal C Runtime (UCRT) default behavior is to crash the program
when strtol is called with an invalid base (that is, not 0 or 2..36).
This an invalid base (that is, not 0 or 2..36). This changes the test to
skip when running on Windows and linking with UCRT.
When using external linking mode this test passes if using the Mingw-w64
toolchain, even when linking with UCRT. That's because the Mingw-w64
linker adds a _set_invalid_parameter_handler call at startup that
overrides the default UCRT behavior. However, other toolchains, like
MSVC and LLVM, doesn't override the default behavior.
Overriding the default behavior is out of the scope for this test, so
the test is skipped instead.
Fixes #62887
Change-Id: I60f140faf0eda80a2de4e10876be25e0dbe442d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/705455
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
In Go 1.25+, strings.SplitSeq offers better
performance. Here are the benchmark results comparing
strings.Split and strings.SplitSeq in a for-loop, with the
benchmark code located in src/strings/iter_test.go:
goos: darwin
goarch: amd64
pkg: cmd/go/internal/auth
cpu: Intel(R) Core(TM) i7-8569U CPU @ 2.80GHz
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
ParseGitAuth/standard-8 281.4n ± 1% 218.0n ± 11% -22.54% (p=0.000 n=10)
ParseGitAuth/with_url-8 549.1n ± 1% 480.5n ± 13% -12.48% (p=0.002 n=10)
ParseGitAuth/minimal-8 235.4n ± 1% 197.3n ± 7% -16.20% (p=0.000 n=10)
ParseGitAuth/complex-8 797.6n ± 2% 805.2n ± 4% ~ (p=0.481 n=10)
ParseGitAuth/empty-8 87.48n ± 3% 63.25n ± 6% -27.71% (p=0.000 n=10)
ParseGitAuth/malformed-8 228.8n ± 1% 171.2n ± 3% -25.17% (p=0.000 n=10)
geomean 288.9n 237.7n -17.72%
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
ParseGitAuth/standard-8 192.00 ± 0% 96.00 ± 0% -50.00% (p=0.000 n=10)
ParseGitAuth/with_url-8 400.0 ± 0% 288.0 ± 0% -28.00% (p=0.000 n=10)
ParseGitAuth/minimal-8 144.00 ± 0% 80.00 ± 0% -44.44% (p=0.000 n=10)
ParseGitAuth/complex-8 528.0 ± 0% 400.0 ± 0% -24.24% (p=0.000 n=10)
ParseGitAuth/empty-8 32.00 ± 0% 16.00 ± 0% -50.00% (p=0.000 n=10)
ParseGitAuth/malformed-8 176.00 ± 0% 80.00 ± 0% -54.55% (p=0.000 n=10)
geomean 179.0 102.1 -42.96%
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
ParseGitAuth/standard-8 3.000 ± 0% 2.000 ± 0% -33.33% (p=0.000 n=10)
ParseGitAuth/with_url-8 4.000 ± 0% 3.000 ± 0% -25.00% (p=0.000 n=10)
ParseGitAuth/minimal-8 3.000 ± 0% 2.000 ± 0% -33.33% (p=0.000 n=10)
ParseGitAuth/complex-8 4.000 ± 0% 3.000 ± 0% -25.00% (p=0.000 n=10)
ParseGitAuth/empty-8 2.000 ± 0% 1.000 ± 0% -50.00% (p=0.000 n=10)
ParseGitAuth/malformed-8 3.000 ± 0% 2.000 ± 0% -33.33% (p=0.000 n=10)
geomean 3.086 2.040 -33.91%
Updates #69315.
Change-Id: Id0219edea45d9658d527b863162ebe917e7821d9
GitHub-Last-Rev: 392b315e122f2c9ef8703ca2dbce8f82ec198556
GitHub-Pull-Request: golang/go#75259
Reviewed-on: https://go-review.googlesource.com/c/go/+/701015
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>
|
|
Some tests still reach for GO_BUILDER_NAME directly. This change makes
it so that they go through testenv.Builder.
There are a couple more, but changing them may also cause tests to start
failing. Done in a follow-up.
Change-Id: Id2453b7b62f5ebf3594e92fa53724a577a97440f
Reviewed-on: https://go-review.googlesource.com/c/go/+/703135
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Plugins may be loaded in the user's init code.
If loading fails, md.bad is true, and doInit should not be executed.
If loading succeeds, the plugin must run modulesinit and typelinksinit
before doInit. Here is not protected by pluginsMu, and in concurrent
scenarios it is possible to obtain the moduledata of the plugin that
is still in the loading process.
Any added modules after loop starts will do their own doInit calls.
This fixes the issue introduced by CL 520375.
Fixes #75102
Change-Id: I48e91ae21615a0c54176875a6a2dea8e1dade906
Reviewed-on: https://go-review.googlesource.com/c/go/+/697675
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
A test in C has an array bound defined as a "const int", which is
technically a variable. The new version of C compiler in Xcode 26
beta emits a warning "variable length array folded to constant
array as an extension" for this (as an error since we build the
test with -Werror). Work around this by using an enum, which is
syntactically a constant.
Change-Id: Icfa943f293f6eac8f41d0615da40c126330d7d11
Reviewed-on: https://go-review.googlesource.com/c/go/+/692877
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Executing "GODEBUG=fips140=on go test -run TestASAN
./cmd/cgo/internal/testsanitizers" fails because FIPS 140 mode is
incompatible with ASAN.
Change-Id: I1a489f3398bbabf597fe7ffc0982c86c3b86e07e
Reviewed-on: https://go-review.googlesource.com/c/go/+/685495
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Quim Muntal <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Currently, if there is a BSS reference and a DATA symbol
definition with the same name, we pick the DATA symbol, as it
contains the right content. In this case, if the BSS reference
has a larger size, we error out, because it is not safe to access
a smaller symbol as if it has a larger size.
Sometimes code declares a global variable in Go and defines it
in assembly with content. They are expected to be of the same
size. However, in ASAN mode, we insert a red zone for the variable
on the Go side, making it have a larger size, whereas the assembly
symbol is unchanged. This causes the Go reference (BSS) has a
larger size than the assembly definition (DATA). It results in an
error currently. This code is valid and safe, so we should permit
that.
We support this case by increasing the symbol size to match the
larger size (of the BSS side). The symbol content (from the DATA
side) is kept. In some sense, we merge the two symbols. When
loading symbols, it is not easy to change its size (as the object
file may be mapped read-only), so we add it to a fixup list, and
fix it up later after all Go symbols are loaded. This is a very
rare case, so the list should not be long.
We could limit this to just ASAN mode. But it seems okay to allow
this in general. As long as the symbol has the larger size, it is
safe to access it with the larger size.
Fixes #74314.
Change-Id: I3ee6e46161d8f59500e2b81befea11e563355a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/684236
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
Fixes: #72766
Change-Id: I45b521e53c2a11e259dc99e2dfc8e40cac39139a
Reviewed-on: https://go-review.googlesource.com/c/go/+/673575
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Change-Id: I212330962453139fa353db29928786b64c9ff063
Reviewed-on: https://go-review.googlesource.com/c/go/+/667455
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
|
|
```
find . \
-not -path './.git/*' \
-not -path './test/*' \
-not -path './src/cmd/vendor/*' \
-not -wholename './src/strings/example_test.go' \
-type f \
-exec \
sed -i -E 's/strings\.Replace\((.+), -1\)/strings\.ReplaceAll\(\1\)/g' {} \;
```
Change-Id: I59e2e91b3654c41a32f17dd91ec56f250198f0d6
GitHub-Last-Rev: 0868b1eccc945ca62a5ed0e56a4054994d4bd659
GitHub-Pull-Request: golang/go#73370
Reviewed-on: https://go-review.googlesource.com/c/go/+/665395
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
|
|
There is no need to manually construct a human-friendly string for
a exec.Command. The String method does that for us.
Change-Id: Iff1033478000bade9cbdc079f6143a7690374258
Reviewed-on: https://go-review.googlesource.com/c/go/+/659475
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Understanding a failure in the goCmd call is difficult because the
important information might be in the stdout instead of stderr.
Change-Id: Icf42974679103c69016129fe2ebb15d5a0a3b51a
Reviewed-on: https://go-review.googlesource.com/c/go/+/659456
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Require GCC 11 or greater to turn on the location checking portion of
the asan tests in this directory; the copy of libasan.so.6 shipped
with GCC 10 doesn't seem to properly digest the new DWARF 5 being
generated by the Go compiler+linker.
Updates #72752.
Change-Id: I92718c112df844d9333c4c798cddaae95665feb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/656175
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
We currently test the leak detector by running "go build -asan",
which will pass -fsanitize=address to the C compiler.
So use that when testing whether the option works.
Fixes #72128
Change-Id: I4efc0b689bfda04c80dbac30a5c757215f297d2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/655535
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
this does result in a little bit more inlining,
cmd/compile text is 0.5% larger,
bent-benchmark text geomeans grow by only 0.02%.
some of our tests make assumptions about inlining.
Change-Id: I999d1798aca5dc64a1928bd434258a61e702951a
Reviewed-on: https://go-review.googlesource.com/c/go/+/655157
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
This enables the ASAN default behavior of reporting C memory leaks.
It can be disabled with ASAN_OPTIONS=detect_leaks=0.
Fixes #67833
Change-Id: I420da1b5d79cf70d8cf134eaf97bf0a22f61ffd0
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15,gotip-linux-arm64-asan-clang15
Reviewed-on: https://go-review.googlesource.com/c/go/+/651755
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Change-Id: I84ec73d3ddef913a87cb9b48147c44ac3e7c8a8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/651957
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
In test files, using testenv.Executable is more reliable than
os.Executable or os.Args[0].
Change-Id: I88e577efeabc20d02ada27bf706ae4523129128e
Reviewed-on: https://go-review.googlesource.com/c/go/+/651955
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Currently, exported Go functions with no parameters generate C functions
with an empty parameter list. In C, a function with an empty parameter
list can accept any number of arguments, whereas a function with a single
void parameter explicitly declares that it takes no arguments.
To align the generated C functions with their Go prototypes, update the
code generation to explicitly include a void parameter for functions
with no parameters.
Fixes #68411
Change-Id: Iab9456aa0236200bf21d1181a2e18e82869df63f
GitHub-Last-Rev: 6ff21a98dfef81df649c76221d9c0858dc0e35e1
GitHub-Pull-Request: golang/go#70981
Reviewed-on: https://go-review.googlesource.com/c/go/+/638635
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Change-Id: I6b2c25ab045aaec8bb6c25511b12d178d155fc25
Reviewed-on: https://go-review.googlesource.com/c/go/+/630338
Reviewed-by: Cherry Mui <cherryyz@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
The TestASANFuzz test would sometimes create a fuzz corpus
in testdata/testdata/FuzzReverse. Avoid modifying the source
directory by building the test with "go test -c" and running
it in a temporary directory.
Change-Id: I12e2da4c85165fa35e0dc1aa6b46c6d0fbccaab8
Reviewed-on: https://go-review.googlesource.com/c/go/+/630377
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
Some systems don't have permissions to run setarch, for example
when running in a docker container without the --privileged flag.
This change makes the tests skip the setarch command if it fails.
Fixes #70463
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15
Change-Id: I02fbd423ba809f5229b8639c9abe6fd275f32558
Reviewed-on: https://go-review.googlesource.com/c/go/+/630096
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
This tweaks the inlining cost knob for closures
specifically, they receive a doubled budget. The
rationale for this is that closures have a lot of
"crud" in their IR that will disappear after inlining,
so the standard budget penalizes them unnecessarily.
This is also the cause of these bugs -- looking at the
code involved, these closures "should" be inlineable,
therefore tweak the parameters until behavior matches
expectations. It's not costly in binary size, because
the only-called-from-one-site case is common (especially
for rangefunc iterators).
I can imagine better fixes and I am going to try to
get that done, but this one is small and makes things
better.
Fixes #69411, #69539.
Change-Id: I8a892c40323173a723799e0ddad69dcc2724a8f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/629195
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Ever since we had to upgrade from our COS image, we've been experiencing
TSAN test failures. My best guess is that the ASLR randomization entropy
increased, causing TSAN to fail. TSAN already re-execs itself in Clang
18+ with ASLR disabled, so just execute the tests with ASLR disabled on
Linux.
Fixes #59418.
Change-Id: Icb4536ddf0f2f5e7850734564d40f5a208ab8d01
Cq-Include-Trybots: luci.golang.try:gotip-linux-386,gotip-linux-386-clang15,gotip-linux-amd64-clang15,gotip-linux-amd64-boringcrypto,gotip-linux-amd64-aliastypeparams,gotip-linux-amd64-asan-clang15,gotip-linux-amd64-msan-clang15,gotip-linux-amd64-goamd64v3
Reviewed-on: https://go-review.googlesource.com/c/go/+/623956
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Currently, at a cgo callback where there is already a Go frame on
the stack (i.e. C->Go->C->Go), we require that at the inner Go
callback the SP is within the g0's stack bounds set by a previous
callback. This is to prevent that the C code switches stack while
having a Go frame on the stack, which we don't really support. But
this could also happen when we cannot get accurate stack bounds,
e.g. when pthread_getattr_np is not available. Since the stack
bounds are just estimates based on the current SP, if there are
multiple C->Go callbacks with various stack depth, it is possible
that the SP of a later callback falls out of a previous call's
estimate. This leads to runtime throw in a seemingly reasonable
program.
This CL changes it to save the old g0 stack bounds at cgocallback,
update the bounds, and restore the old bounds at return. So each
callback will get its own stack bounds based on the current SP,
and when it returns, the outer callback has the its old stack
bounds restored.
Also, at a cgo callback when there is no Go frame on the stack,
we currently always get new stack bounds. We do this because if
we can only get estimated bounds based on the SP, and the stack
depth varies a lot between two C->Go calls, the previous
estimates may be off and we fall out or nearly fall out of the
previous bounds. But this causes a performance problem: the
pthread API to get accurate stack bounds (pthread_getattr_np) is
very slow when called on the main thread. Getting the stack bounds
every time significantly slows down repeated C->Go calls on the
main thread.
This CL fixes it by "caching" the stack bounds if they are
accurate. I.e. at the second time Go calls into C, if the previous
stack bounds are accurate, and the current SP is in bounds, we can
be sure it is the same stack and we don't need to update the bounds.
This avoids the repeated calls to pthread_getattr_np. If we cannot
get the accurate bounds, we continue to update the stack bounds
based on the SP, and that operation is very cheap.
On a Linux/AMD64 machine with glibc:
name old time/op new time/op delta
CgoCallbackMainThread-8 96.4µs ± 3% 0.1µs ± 2% -99.92% (p=0.000 n=10+9)
Fixes #68285.
Fixes #68587.
Change-Id: I3422badd5ad8ff63e1a733152d05fb7a44d5d435
Reviewed-on: https://go-review.googlesource.com/c/go/+/600296
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
First, skip all the allocation count tests.
In some cases this aligns with existing skips for -race, but in others
we've got new issues. These are debug modes, so some performance loss is
expected, and this is clearly no worse than today where the tests fail.
Next, skip internal linking and static linking tests for msan and asan.
With asan we get an explicit failure that neither are supported by the C
and/or Go compilers. With msan, we only get the Go compiler telling us
internal linking is unavailable. With static linking, we segfault
instead. Filed #70080 to track that.
Next, skip some malloc tests with asan that don't quite work because of
the redzone.
This is because of some sizeclass assumptions that get broken with the
redzone and the fact that the tiny allocator is effectively disabled
(again, due to the redzone).
Next, skip some runtime/pprof tests with asan, because of extra
allocations.
Next, skip some malloc tests with asan that also fail because of extra
allocations.
Next, fix up memstats accounting for arenas when asan is enabled. There
is a bug where more is added to the stats than subtracted. This also
simplifies the accounting a little.
Next, skip race tests with msan or asan enabled; they're mutually
incompatible.
Fixes #70054.
Fixes #64256.
Fixes #64257.
For #70079.
For #70080.
Change-Id: I99c02a0b9d621e44f1f918b307aa4a4944c3ec60
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15,gotip-linux-amd64-msan-clang15
Reviewed-on: https://go-review.googlesource.com/c/go/+/622855
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
|
|
Fixes #66966
Change-Id: I92777a7d7d8afaa82ffcd605aa3e607289b645f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/622477
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
We were missing a case for calling a C function with an index
into a pointer-to-array.
Fixes #70016
Change-Id: I9c74d629e58722813c1aaa0f0dc225a5a64d111b
Reviewed-on: https://go-review.googlesource.com/c/go/+/621576
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Change-Id: Id22ad7c92c54bc61f1f1d5544f17208f2f8648aa
GitHub-Last-Rev: 61eee331dc2da16358425440e5630117f45d5182
GitHub-Pull-Request: golang/go#69866
Reviewed-on: https://go-review.googlesource.com/c/go/+/619836
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|