go_android_exec gets the exit status of the process run inside the
Android emulator by sending a small shell script that runs the desired
command and then prints "exitcode=" followed by the exit code. This is
necessary because adb does not reliably pass through the exit status
of the subprocess.
An old bug about this
(https://code.google.com/p/android/issues/detail?id=3254) was closed
in 2016 as fixed in Android N (7.0), but it seems that the adb on the
Android builder at least still sometimes fails to pass through the
exit code.
Unfortunately, this workaround has the effect of injecting
"exitcode=N" into the output of the subprocess it runs, which messes
up tests that are looking for golden output from a subprocess.
Fix this by inserting a filter Writer that looks for the final
"exitcode=N" and strips it from the exec wrapper's own stdout.
For #15919.
This will help us in cleaning up "host tests" for #37486.
Bryan C. Mills [Tue, 2 May 2023 13:37:00 +0000 (09:37 -0400)]
cmd/api: move support checks into individual tests
This makes 'go test -list cmd/api' work, and fixes an infinite
recursion via testenv.HasExec that would otherwise occur.
As of CL 488076, testenv.HasExec tries to re-exec the test
executable using -list to suppress running the tests, which
produces a fork bomb if TestMain itself calls HasExec.
For this test, it turns out that the HasExec check is redundant
anyway: if we can exec 'go build', we can certainly exec programs in
general too.
Felix Geisendörfer [Wed, 26 Apr 2023 08:02:22 +0000 (10:02 +0200)]
runtime: remove systemstack logic from adjustframe
Remove logic for skipping some adjustframe logic for systemstack (aka
FuncID_systemstack_switch). This was introduced in 2014 by 9198ed4bd6ec7b7dd37aa2797e96f15ddbb1e6cd but doesn't seem to be needed
anymore.
Updates #59692
Change-Id: I2368d64f9bb28ced4e7f15c9b15dac7a29194389
Reviewed-on: https://go-review.googlesource.com/c/go/+/489116 Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Felix Geisendörfer [Wed, 26 Apr 2023 08:07:02 +0000 (10:07 +0200)]
runtime: add test for systemstack frame pointer adjustment
Add TestSystemstackFramePointerAdjust as a regression test for CL
489015.
By turning stackPoisonCopy into a var instead of const and introducing
the ShrinkStackAndVerifyFramePointers() helper function, we are able to
trigger the exact combination of events that can crash traceEvent() if
systemstack restores a frame pointer that is pointing into the old
stack.
Updates #59692
Change-Id: I60fc6940638077e3b60a81d923b5f5b4f6d8a44c
Reviewed-on: https://go-review.googlesource.com/c/go/+/489115
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Roland Shoemaker [Thu, 13 Apr 2023 21:01:50 +0000 (14:01 -0700)]
html/template: emit filterFailsafe for empty unquoted attr value
An unquoted action used as an attribute value can result in unsafe
behavior if it is empty, as HTML normalization will result in unexpected
attributes, and may allow attribute injection. If executing a template
results in a empty unquoted attribute value, emit filterFailsafe
instead.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Fixes #59722
Fixes CVE-2023-29400
Change-Id: Ia38d1b536ae2b4af5323a6c6d861e3c057c2570a
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1826631 Reviewed-by: Julie Qiu <julieqiu@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/491617
Run-TryBot: Carlos Amedee <carlos@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Roland Shoemaker [Thu, 13 Apr 2023 22:40:44 +0000 (15:40 -0700)]
html/template: disallow angle brackets in CSS values
Angle brackets should not appear in CSS contexts, as they may affect
token boundaries (such as closing a <style> tag, resulting in
injection). Instead emit filterFailsafe, matching the behavior for other
dangerous characters.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Fixes #59720
Fixes CVE-2023-24539
Change-Id: Iccc659c9a18415992b0c05c178792228e3a7bae4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1826636 Reviewed-by: Julie Qiu <julieqiu@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/491615 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Thu, 27 Apr 2023 00:44:24 +0000 (17:44 -0700)]
io/fs: add FormatFileInfo and FormatDirEntry functions
For #54451
Change-Id: I3214066f77b1398ac1f2786ea035c83f32f0a826
Reviewed-on: https://go-review.googlesource.com/c/go/+/489555
Run-TryBot: 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> Reviewed-by: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Nick Ripley [Mon, 1 May 2023 13:08:01 +0000 (09:08 -0400)]
runtime/trace: enable frame pointer unwinding by default for amd64 and arm64
Re-enable frame pointer unwinding for execution tracing on amd64 by
default, now that CL 489015 and CL 488755 have fixed recently-discovered
crashes. This reverts CL 486382.
These fixes, together with CL 241158 to fix up frame pointers when
copying stacks on arm64, also make frame pointer unwinding for tracing
safe to enable for arm64. This should significantly reduce the CPU and
latency overhead of execution tracing on arm64, as it has for amd64.
Co-Authored-By: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Change-Id: I64a88bd69dfd8cb13956ec46f8b1203dbeaa26a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/490815 Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Run-TryBot: Nick Ripley <nick.ripley@datadoghq.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Alan Donovan [Mon, 1 May 2023 17:02:04 +0000 (13:02 -0400)]
cmd/cgo: reject attempts to declare methods on C types
This change causes cgo to emit an error (with the same
message as the compiler) when it encounters a declaration
of a method whose receiver type is C.T or *C.T.
Conceptually, C is another package, but unfortunately
the desugaring of C.T is a type within the same package,
causing the previous behavior to accept invalid input.
It is likely that at least some users are intentionally
exploiting this behavior, so this may break their build.
We should mention it in the release notes.
Cherry Mui [Mon, 1 May 2023 16:47:15 +0000 (12:47 -0400)]
cmd/link: work around dsymutils not cleaning temp file
Some versions of dsymutils, notably the one in clang 14.0.3, which
is shipped in some versions of Xcode, have a bug that it creates a
temporary directory but doesn't clean it up at exit. The temporary
directory is created in DSYMUTIL_REPRODUCER_PATH (if set,
otherwise TMPDIR). Work around the issue by setting
DSYMUTIL_REPRODUCER_PATH to the linker's temporary directory, so
the linker will clean it up at exit anyway.
Fixes #59026.
Change-Id: Ie3e90a2d6a01f90040dc2eac91e8e536ccdda5a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/490818 Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Cherry Mui [Sat, 29 Apr 2023 02:15:48 +0000 (22:15 -0400)]
cmd/compile: don't generate DWARF info for static vars
Static data symbols are compiler generated, not user symbols. The
linker already does not include them in the final DWARF section.
Don't generate the DWARF info in the first place.
Change-Id: Id2ae36683bfc1ed60b9924b7305eae5e8aa14d80
Reviewed-on: https://go-review.googlesource.com/c/go/+/490817
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
qmuntal [Thu, 12 Jan 2023 14:47:19 +0000 (15:47 +0100)]
cmd/link: generate .xdata PE section
This CL adds a .xdata section to the PE file generated by the Go linker.
It is also the first CL of the SEH chain that adds effective support
for unwinding the Go stack, as demonstrated by the newly added tests.
The .xdata section is a standard PE section that contains
an array of unwind data info structures. This structures are used to
record the effects a function has on the stack pointer,
and where the nonvolatile registers are saved on the stack [1].
Note that this CL still does not support unwinding the cgo stack.
qmuntal [Tue, 17 Jan 2023 07:15:33 +0000 (08:15 +0100)]
cmd/link: generate .pdata PE section
This CL adds a .pdata section to the PE file generated by the Go linker.
The .pdata section is a standard section [1] that contains an array of
function table entries that are used for stack unwinding.
The table entries layout is taken from [2].
This CL just generates the table entries without any unwinding
information, which is enough to start doing some E2E tests
between the Go linker and the Win32 APIs.
The goal of the .pdata table is to allow Windows retrieve
unwind information for a function at a given PC. It does so by doing
a binary search on the table, looking for an entry that meets
BeginAddress >= PC < EndAddress.
Each table entry takes 12 bytes and only non-leaf functions with
frame pointer needs an entry on the .pdata table.
The result is that PE binaries will be ~0.7% bigger due to the unwind
information, a reasonable amount considering the benefits in
debuggability.
Johan Brandhorst-Satzkorn [Fri, 28 Apr 2023 04:39:57 +0000 (21:39 -0700)]
internal/testenv: probe for symlink on wasip1
Certain WASI runtimes do not support generic symlinks, and
instead return permission errors when they are attempted.
Perform a runtime probe of symlink support in hasSymlink
on wasip1 to determine whether the runtime supports
generic symlinks.
Also perform the same probe on android.
For #59583
Change-Id: Iae5b704e670650d38ee350a5a98f99dcce8b5b28
Reviewed-on: https://go-review.googlesource.com/c/go/+/490115
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Achille Roussel <achille.roussel@gmail.com>
TryBot-Bypass: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Cherry Mui [Sat, 29 Apr 2023 01:35:31 +0000 (21:35 -0400)]
cmd/link, cmd/internal/obj: use aux symbol for global variable DWARF info
Currently, for a global variable, its debug info symbol is a named
symbol with the variable's name with a special prefix. And the
linker looks it up by name. This CL makes the debug info symbol an
aux symbol of the variable symbol.
Change-Id: I55614d0ef2af9c53eb40144ad80e09339bf3cbee
Reviewed-on: https://go-review.googlesource.com/c/go/+/490816
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
cui fliter [Mon, 1 May 2023 04:41:56 +0000 (12:41 +0800)]
runtime: fix comment typo in page allocator
A commit looks to have some minor bug that makes comments look confusing.
The commit in question: https://go-review.googlesource.com/c/go/+/250517
Change-Id: I7859587be15a22f8330d6ad476058f74ca2ca6ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/490795
Run-TryBot: shuang cui <imcusg@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Sat, 29 Apr 2023 00:46:00 +0000 (17:46 -0700)]
go/types, types2: isParameterized must be able to handle tuples
CL 484615 rewrote isParameterized by handling tuple types only where
they occur (function signatures). However, isParameterized is also
called from Checker.callExpr, with a result parameter list which
is a tuple. This CL handles tuples again.
Fixes #59890.
Change-Id: I35159ff65f23322432557e6abcab939933933d40
Reviewed-on: https://go-review.googlesource.com/c/go/+/490695 Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Will Faught [Fri, 28 Apr 2023 23:44:50 +0000 (23:44 +0000)]
text/template: reword uncover to unwrap
Matches the preceding "wrap" terminology.
Change-Id: Ia783de578c2942fe1474281c3d6056b1074d41b0
GitHub-Last-Rev: 4fcff4e9b2836d428ba668186441089a9618c028
GitHub-Pull-Request: golang/go#59891
Reviewed-on: https://go-review.googlesource.com/c/go/+/490675
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
misc/android: rename to misc/go_android_exec, make go build work
This makes it reasonable to "go build" from this directory by changing
the name of the directory to a more reasonable name for the binary and
dropping the unnecessary "ignore" build tag. The resulting binary
doesn't *quite* have the necessary name for a Go exec wrapper because
that needs to have the form, go_android_$GOARCH_exec, but it's close.
Ian Lance Taylor [Tue, 25 Apr 2023 00:27:33 +0000 (17:27 -0700)]
debug/pe: return error on reading from section with uninitialized data
A section with uninitialized data contains no bytes and occupies
no space in the file. This change makes it return an error on reading
from this section so that it will force the caller to check for
a section with uninitialized data.
This is the debug/pe version of CL 429601.
This will break programs that expect a byte slice with the length
described by the SizeOfRawData field. There are two reasons to
introduce this breaking change: 1) uninitialized data is uninitialized
and there is no reason to allocate memory for it; 2) it could result
in an OOM if the file is corrupted and has a large invalid SizeOfRawData.
No test case because the problem can only happen for invalid data. Let
the fuzzer find cases like this.
For #47653
Fixes #59817
Change-Id: I1ae94e9508f803b37926275d9a571f724a09af9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/488475 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: kortschak <dan@kortschak.io> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Ian Lance Taylor <iant@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>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Felix Geisendörfer [Sat, 22 Apr 2023 06:54:46 +0000 (09:54 +0300)]
runtime: fix systemstack frame pointer adjustment
Change adjustframe to adjust the frame pointer of systemstack (aka
FuncID_systemstack_switch) before returning early.
Without this fix it is possible for traceEvent() to crash when using
frame pointer unwinding. The issue occurs when a goroutine calls
systemstack in order to call shrinkstack. While returning, systemstack
will restore the unadjusted frame pointer from its frame as part of its
epilogue. If the callee of systemstack then triggers a traceEvent, it
will try to unwind into the old stack. This can lead to a crash if the
memory of the old stack has been reused or freed in the meantime.
The most common situation in which this will manifest is when when
gcAssistAlloc() invokes gcAssistAlloc1() on systemstack() and performs a
shrinkstack() followed by a traceGCMarkAssistDone() or Gosched()
triggering traceEvent().
See CL 489115 for a deterministic test case that triggers the issue.
Meanwhile the problem can frequently be observed using the command
below:
Go programs targeting GOOS=wasip1 were failing to open directories when
executed with runtimes like wasmtime or wasmedge due to requesting
rights for operations that are not supported on directories such as
fd_read, fd_write, etc...
This change addresses the issue by performing a second path_open when
observing EISDIR, and masking the requested rights to only ask for
permissions to perform operations supported by a directory.
Change-Id: Ibf65acf4a38bc848a649f41dbd026507d8b63c82
Reviewed-on: https://go-review.googlesource.com/c/go/+/490755
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Fri, 28 Apr 2023 22:05:36 +0000 (15:05 -0700)]
cmd/distpack: remove internal/platform/zosarch.go
cmd/dist started generating that file in CL 483695.
Also rearrange the list of files to remove slightly to explain
better where they come from.
Fixes #59889
Change-Id: I062c858596d801157e0d943e4ba2761c0547ac3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/490655
Auto-Submit: 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>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Paul E. Murphy [Tue, 17 Jan 2023 17:33:28 +0000 (11:33 -0600)]
cmd/link: load external ELF PPC64 objects which set st_other=1
This indicates the symbol does not use or preserve the TOC pointer in
R2. Likewise, it does not have a distinct local entry point. This
happens when gcc compiles an object with -mcpu=power10.
Recycle the SymLocalentry field of a text symbol to pass through this
hint as the bogus value 1 (A valid offset must be a multiple of 4
bytes), and update the usage to check and generate errors further into
the linking process. This matches the behavior of st_other as used by
ELFv2.
Change-Id: Ic89ce17b57f400ab44213b21a3730a98c7cdf842
Reviewed-on: https://go-review.googlesource.com/c/go/+/490295
Run-TryBot: Paul Murphy <murp@ibm.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Nick Ripley [Tue, 25 Apr 2023 18:48:00 +0000 (14:48 -0400)]
runtime/trace: avoid frame pointer unwinding for events during cgocallbackg
The current mp.incgocallback() logic allows for trace events to be
recorded using frame pointer unwinding during cgocallbackg when they
shouldn't be. Specifically, mp.incgo will be false during the
reentersyscall call at the end. It's possible to crash with tracing
enabled because of this, if C code which uses the frame pointer register
for other purposes calls into Go. This can be seen, for example, by
forcing testprogcgo/trace_unix.c to write a garbage value to RBP prior
to calling into Go.
We can drop the mp.incgo check, and instead conservatively avoid doing
frame pointer unwinding if there is any C on the stack. This is the case
if mp.ncgo > 0, or if mp.isextra is true (meaning we're coming from a
thread created by C). Rename incgocallback to reflect that we're
checking if there's any C on the stack. We can also move the ncgo
increment in cgocall closer to where the transition to C happens, which
lets us use frame pointer unwinding for the entersyscall event during
the first Go-to-C call on a stack, when there isn't yet any C on the
stack.
Fixes #59830.
Change-Id: If178a705a9d38d0d2fb19589a9e669cd982d32cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/488755 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Nick Ripley <nick.ripley@datadoghq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Joel Sing [Sun, 8 Jan 2023 18:14:13 +0000 (05:14 +1100)]
cmd/internal/obj/arm64: pass obj.As to oaddi rather than an instruction
This simplifies callers, as they do not need to call opirr before calling oaddi.
Additionally, use appropriate types (int16) for registers, which avoids the need
to continually cast.
Change-Id: I8ca3807a97867ac49d63792f6922a18f35824448
Reviewed-on: https://go-review.googlesource.com/c/go/+/471520 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Joel Sing [Fri, 28 Apr 2023 16:00:15 +0000 (02:00 +1000)]
cmd/internal/obj/arm64: use appropriate return type for regoff
All of the callers of regoff cast the return value from uint32 to int32.
Instead, simply return int32 in the first place.
Change-Id: I43a672bb3143a71f4a37779ed8ae9adcda623ba4
Reviewed-on: https://go-review.googlesource.com/c/go/+/490355
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
os: fix TestChdirAndGetwd/ReaddirnamesOneAtATime on wasip1 to run on Windows hosts
TestReaddirnamesOneAtATime and TestChdirAndGetwd assumes the underlying file system
has /usr/bin but it is not the case when running it on WASI runtime hosted on Windows.
This change adds wasip1 in the special cased switch case to make them host OS agonstic.
Change-Id: Idb667021b565f939c814b9cd9e637cd75f9a610d
Reviewed-on: https://go-review.googlesource.com/c/go/+/489575
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Cherry Mui [Wed, 26 Apr 2023 21:29:33 +0000 (17:29 -0400)]
cmd/link: write buildid to plugin
Currently, in plugin build mode we don't write the build ID. This
is disabled in CL 29394 since plugin is supported on Darwin. Maybe
it caused some problem with the Darwin dynamic linker. But it
seems no problem currently. Enabled it.
Fixes #59845.
Change-Id: I60589ffc7937e4d30055960d391cac1e7cd0cd42
Reviewed-on: https://go-review.googlesource.com/c/go/+/489457
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
runtime: resolve checkdead panic by refining `startm` lock handling in caller context
This change addresses a `checkdead` panic caused by a race condition between
`sysmon->startm` and `checkdead` callers, due to prematurely releasing the
scheduler lock. The solution involves allowing a `startm` caller to acquire the
scheduler lock and call `startm` in this context. A new `lockheld` bool
argument is added to `startm`, which manages all lock and unlock calls within
the function. The`startIdle` function variable in `injectglist` is updated to
call `startm` with the lock held, ensuring proper lock handling in this
specific case. This refined lock handling resolves the observed race condition
issue.
Fixes #59600
Change-Id: I11663a15536c10c773fc2fde291d959099aa71be
Reviewed-on: https://go-review.googlesource.com/c/go/+/487316 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Ian Lance Taylor [Thu, 27 Apr 2023 22:56:34 +0000 (15:56 -0700)]
net: re check conf.goos even if it equals runtime.GOOS
This field is only for testing purposes, where we can't assume
that the conf value was initialized as expected for that GOOS.
This fixes the net tests on android.
Change-Id: I8432587f219a05adbb4d234a813467f876a764b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/489975
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@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: Ian Lance Taylor <iant@google.com>
Bryan C. Mills [Fri, 10 Mar 2023 19:25:41 +0000 (14:25 -0500)]
internal/testenv: allow 'go build' on android when supported
As of CL 472096, it should work on android/arm64 always (because
internal linking is supported on that platform), and on other android
platforms when a C toolchain is present in the test environment.
Updates #58775.
Change-Id: Ifa38dc69b258b38dcc341979dcbf8cd61265c787
Reviewed-on: https://go-review.googlesource.com/c/go/+/475456
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Changkun Ou <mail@changkun.de>
Bryan C. Mills [Tue, 28 Feb 2023 17:07:36 +0000 (17:07 +0000)]
misc/android: rework GOROOT installation
- Fall back to 'go env GOROOT' to locate GOROOT if runtime.GOROOT() is
empty (as may be the case if the tool is built with -trimpath).
- Copy all of $GOROOT/android_$GOARCH/bin, not just cmd/go, to
$GOROOT/bin.
- For consistency with CL 404134, place $GOROOT/bin at the beginning
of $PATH, not the end.
- Don't use the install target for the "runtime" package to locate pkg/tool.
As of Go 1.20 "runtime" doesn't have an install directory anyway.
Since the real reason we need pkg/tool is for commands in "cmd",
use an arbitrary command (namely "cmd/compile") to locate it.
- Use 'go list' to determine the package import path for the current
directory, instead of assuming that it is within GOROOT or GOPATH.
(That assumption does not hold in module mode.)
Updates #58775.
Change-Id: If76ff22bce76d05175c40678230f046a4aff0940
Reviewed-on: https://go-review.googlesource.com/c/go/+/472096
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Changkun Ou <mail@changkun.de>
Auto-Submit: Bryan Mills <bcmills@google.com>
Wasmtime used to error when opening a directory without passing the
O_DIRECTORY flag, which required doing a stat to determine whether to
inject the flag prior to opening any file.
The workaround was subject to races since the stat and open calls were
not atomic.
Wasmtime fixed the issue in v8.0.1.
For details see:
- https://github.com/bytecodealliance/wasmtime/pull/4967
- https://github.com/bytecodealliance/wasmtime/pull/6163
- https://github.com/bytecodealliance/wasmtime/releases/tag/v8.0.1
Change-Id: I0f9fe6a696024b70fffe63b83e8f61e48acd0c4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/489955
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Ian Lance Taylor [Thu, 20 Apr 2023 23:55:13 +0000 (16:55 -0700)]
net: rewrite and simplify resolver configuration
The resulting code behaves mostly the same. There are some minor
differences in error cases when the cgo resolver is not available:
instead of just falling back we keep trying to work out the right
nsswitch.conf order.
Change-Id: I17fadc940528fa2397043ac8f8ed7da3bd7a95c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/487196 Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@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> Reviewed-by: Mateusz Poliwczak <mpoliwczak34@gmail.com>
cmd/dist,internal: enable buildmode=c-archive for linux/loong64
Now the shared flag is supported on the linux/loong64 platform and
misc/cgo/testcarchive has been passed, buildmode=c-archive can be used.
Change-Id: Ice450dc11fcb91942fdf2ddd34bb163853267e01
Reviewed-on: https://go-review.googlesource.com/c/go/+/489576 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Meidan Li <limeidan@loongson.cn>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: WANG Xuerui <git@xen0n.name> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name>
cmd/go: skip the 'git' part of get_insecure_redirect in short mode
Invoking 'git' adds about 200ms to this test on a fast machine,
probably more on a slow one. (As a general habit we skip the
'git' tests uniformly in short mode.)
Dmitri Shuralyov [Wed, 26 Apr 2023 20:41:41 +0000 (16:41 -0400)]
cmd/api: remove unused functionality
We no longer use the optional parameter to compareAPI.
We now always set allowAdd to false.
(Except in tests, making them less useful than they could be.)
Flags and parsing their value are no more.
Remove all the unused functionality and update test cases so they're
closer to what the API checker does when it runs for real. Order the
features, required, exception variables and fields more consistently.
For #43956.
Change-Id: Iaa4656a89a3fca3129742165a448d385e55e4a98
Reviewed-on: https://go-review.googlesource.com/c/go/+/489436
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Dmitri Shuralyov [Mon, 24 Apr 2023 15:26:23 +0000 (11:26 -0400)]
cmd/api: make api/next/* entries required, not optional
We want the API check to catch if some API present in api/next/*
files is no longer implemented in the tree, and report it in the
same CL that is making the change (by failing loudly). Arguably
this should've been the case since CL 315350, but I didn't notice
it at the time. Do it now.
For #43956.
Change-Id: I73330dd5fd3f5706a1fdf13b2bf8e0f24c6b48e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/488135 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
cui fliter [Thu, 6 Apr 2023 09:08:49 +0000 (17:08 +0800)]
crypto: use t.Parallel() to increase speed
It is necessary to invoke the t.Parallel() method in both the top-level test function and its subtest function to maximize parallelism. In doing so, all subtest functions calling the t.Parallel() method in the package will work in parallel.
On my machine, the execution time of this test file was cut in half.
Change-Id: If09147a2a9969bb044932d71e6bfea29492866d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/482755
Run-TryBot: shuang cui <imcusg@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Michael Pratt [Mon, 17 Apr 2023 18:51:28 +0000 (14:51 -0400)]
runtime/cgo: store M for C-created thread in pthread key
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.
[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 485500 description]
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.
Fixes #51676.
Fixes #59294.
Fixes #59678.
Change-Id: Ic62be31a06ee83568215e875a891df37084e08ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/485500
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Paul E. Murphy [Tue, 25 Apr 2023 21:41:51 +0000 (16:41 -0500)]
cmd/link/internal/loadelf: set AttrExternal on text section symbols
PPC64 processes external object relocations against the section
symbols. This needs to be set correctly to determine the type of
PLT stub to generate when both Go and External code make PLT calls.
Change-Id: I5abdd5a0473866164083c33e80324dffcc1707f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/488895 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Than McIntosh [Thu, 13 Apr 2023 18:27:33 +0000 (14:27 -0400)]
coverage: fix count vs emit discrepancy in coverage counter data writing
This patch revises the way coverage counter data writing takes place
to avoid problems where useful counter data (for user-written functions)
is skipped in favor of counter data from stdlib functions that are
executed "late in the game", during the counter writing process itself.
Reading counter values from a running "--coverpkg=all" program is an
inherently racy operation; while the the code that scans the coverage
counter segment is reading values, the program is still executing,
potentially updating those values, and updates can include execution
of previously un-executed functions. The existing counter data writing
code was using a two-pass model (initial sweep over the counter
segment to count live functions, second sweep to actually write data),
and wasn't properly accounting for the fact that the second pass could
see more functions than the first.
In the bug in question, the first pass discovered an initial set of
1240 functions, but by the time the second pass kicked in, several
additional new functions were also live. The second pass scanned the
counter segment again to write out exactly 1240 functions, but since
some of the counters for the newly executed functions were earlier in
the segment (due to linker layout quirks) than the user's selected
function, the sweep terminated before writing out counters for the
function of interest.
The fix rewrites the counter data file encoder to make a single sweep
over the counter segment instead of using a two-pass scheme.
Fixes #59563.
Change-Id: I5e908e226bb224adb90a2fb783013e52deb341da
Reviewed-on: https://go-review.googlesource.com/c/go/+/484535 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Than McIntosh [Mon, 24 Apr 2023 14:33:01 +0000 (10:33 -0400)]
internal/coverage/slicewriter: fix off-by-1 error in seek utilities
The slicewriter Seek method was being too restrictive on offsets
accepted, due to an off-by-one problem in the error checking code.
This fixes the problem and touches up the unit tests.
Change-Id: I75d6121551de19ec9275f0e331810db231db6ea9
Reviewed-on: https://go-review.googlesource.com/c/go/+/488116
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The Linux implementation requires that the address addr be
page-aligned, and allows length to be zero.
See Linux notes:
https://man7.org/linux/man-pages/man2/madvise.2.html
Change-Id: Ic49960c32991ef12f23de2de76e9689567c82d03
GitHub-Last-Rev: 35e7f8e5cc0b045043a88d9f304ef5bb1e9c1ab2
GitHub-Pull-Request: golang/go#59793
Reviewed-on: https://go-review.googlesource.com/c/go/+/488015
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Wed, 12 Apr 2023 01:55:23 +0000 (18:55 -0700)]
crypto/sha512: add WriteString and WriteByte method
This can reduce allocations when hashing a string or byte
rather than []byte.
For #38776
Change-Id: I4926ae2749f6b167edbebb73d8f68763ffb2f0c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/483816 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Joel Sing <joel@sing.id.au>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Tue, 11 Apr 2023 22:28:23 +0000 (15:28 -0700)]
crypto/sha1: add WriteString and WriteByte method
This can reduce allocations when hashing a string or byte
rather than []byte.
For #38776
Change-Id: I7c1fbdf15abf79d2faf360f75adf4bc550a607e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/483815
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Joel Sing <joel@sing.id.au>
Ian Lance Taylor [Mon, 3 Apr 2023 04:21:24 +0000 (21:21 -0700)]
crypto/sha256: add WriteString and WriteByte method
This can reduce allocations when hashing a string or byte
rather than []byte.
For #38776
Change-Id: I1c6dd1bc018220784a05939e92b47558c0562110
Reviewed-on: https://go-review.googlesource.com/c/go/+/481478 Reviewed-by: Joel Sing <joel@sing.id.au>
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: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
cmd/dist: actually only compile tests with -compile-only
Currently, "dist test -compile-only" still runs the test binaries,
just with -run=^$ so no tests are run. It does this because, until
recently, "go test -c" would fail if passed multiple test packages.
But this has some unexpected consequences: init code still runs,
TestMain still runs, and we generally can't test cross-compiling of
tests.
Now that #15513 is fixed, we can pass multiple packages to "go test
-c". Hence, this CL make dist just use "go test -c" as one would
expect.
Found in the course of working on #37486, though it doesn't really
affect that.
internal/cpu: add a detection for Neoverse(N2, V2) cores
The memmove implementation relies on the variable
runtime.arm64UseAlignedLoads to select fastest code
path. Considering Neoverse N2 and V2 cores prefer aligned
loads, this patch adds code to detect them for
memmove performance.
And this patch uses a new variable ARM64.IsNeoverse to
represent all Neoverse cores, removing the more specific
versions.
Change-Id: I9e06eae01a0325a0b604ac6af1e55711dd6133f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/487815 Reviewed-by: Joel Sing <joel@sing.id.au> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Fannie Zhang <Fannie.Zhang@arm.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
cui fliter [Sun, 23 Apr 2023 10:44:05 +0000 (18:44 +0800)]
html/template: fix unavailable url
The previous link is no longer accessible. use latest link.
Change-Id: I76411ee00785f3d92014c5012e4efb446924adaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/487835 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Andrew Polukhin <andrewmathematics2003@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: shuang cui <imcusg@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Sun, 23 Apr 2023 19:25:47 +0000 (12:25 -0700)]
runtime: add raceFiniLock to lock ranking
Also preserve the PC/SP in reentersyscall when doing lock ranking.
The test is TestDestructorCallbackRace with the staticlockranking
experiment enabled.
For #59711
Change-Id: I87ac1d121ec0d399de369666834891ab9e7d11b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/487955
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
internal/testenv: actually try to exec on ios and wasm
Due to a stray edit in CL 486275, the assignment to tryExecOk
in tryExec on ios would be immediately overwritten back to false.
This change fixes the stray edit.
We added a check for incorrect baseOffset in CL 408734, but in doing so
we introduced a panic when directoryOffset overflowed a int64. The zip
spec uses uint64, but since io.SectionReader requires int64 we convert,
and possibly introduce an overflow. If offset < 0 && size-offset < 0,
SectionReader will panic when we attempt to read from it.
Since it's extremely unlikely we're ever going to process a zip file
larger than 1<<63-1 byte, just limit directory size and offset to the
max int64.
Change-Id: I1aaa755cf4da927a6e12ef59f97dfc83a3426d86
Reviewed-on: https://go-review.googlesource.com/c/go/+/488195 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
runtime: fix CgoRaceprof and CgoRaceSignal build failures
TestRaceProf and TestRaceSignal were changed to run on all platforms
that support the race detector as of CL 487575, but the testprogcgo
source files needed to run the test rely on POSIX threads and were
still build-constrained to only linux/amd64 and freebsd/amd64.
Since the C test program appears to require only POSIX APIs, update
the constraint to build the source file on all Unix platforms, and
update the tests to skip on Windows.
This may slightly increase testprogcgo build time on Unix platforms
that do not support the race detector.
Cherry Mui [Fri, 21 Apr 2023 20:28:57 +0000 (16:28 -0400)]
cmd/link: don't sort data symbols by name
For data symbols, we currently sort them by size, then by name if
the size is the same. Sorting by name is not really necessary.
Instead, we sort by symbol index. Like name, the symbol index is
deterministic, and pretty stable if only a small portion of the
input is changed, and also naturally partitioned by packages. This
reduces the CPU time for reading the symbol names and comparing
strings.
Cherry Mui [Fri, 21 Apr 2023 16:02:30 +0000 (12:02 -0400)]
cmd/link: use uint32 as symbol index
Currently, a symbol's global index, the Sym type, is defined as an
int, which is 64-bit on 64-bit machines. We're unlikely to have
more than 4 billion symbols in the near future. Even if we will,
we will probably hit some other limit (e.g. section size) before
the symbol number limit. Use a 32-bit type to reduce memory usage.
E,g, linking cmd/compile in external linking mode (on macOS/amd64)
Bryan C. Mills [Fri, 3 Mar 2023 14:48:10 +0000 (14:48 +0000)]
cmd/go: declare net hosts in script tests
Although we aren't precise about enforcing the hosts just yet,
we can eventually use the declared hostnames to selectively skip
tests (for example, if an external service has an outage while
a Go release is being tested).
Also relax the constraint to [short] in tests that require only
vcs-test.golang.org, which has redirected to an in-process server
since around CL 427914.
Also enforce that tests that use the network actually use the [net]
constraint, by setting TESTGONETWORK=panic in the test environment
until the condition is evaluated.
runtime/metrics: set /sched/latencies:seconds as cumulative
The current implementation for this metric populates a histogram
that is never reset, i.e. where each bucket count increases
monotonically.
The comment in the definition of the Cumulative attribute calls
out that cumulative means that if the metric is a distribution,
then each bucket count increases monotonically.
In that sense, the cumulative attribute should be set to true for
this metric.
Change-Id: Ifc34e965a62f2d7881b5c8e8cbb8b7207a4d5757
Reviewed-on: https://go-review.googlesource.com/c/go/+/486755
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Cherry Mui [Fri, 21 Apr 2023 03:33:42 +0000 (23:33 -0400)]
cmd/link: use slice and bitmap for some attributes
Currently, a symbol's outer symbol, the "special" attribute, and
whether a symbol is a generator symbol are represented as maps,
and are accessed in some loops over nearly all reachable symbols.
The map lookups are a bit expensive.
For outer symbol, a non-trivial portion of the symbols have outer
symbol set (e.g. type symbols, which we put into container symbols
like "type:*"). Using a slice to access more efficiently.
For the special and generator symbol attributes, use a bitmap.
There are not many symbols have those attributes, so the bitmap is
quite sparse. The bitmap is not too large anyway, so use it for
now. If we want to further reduce memory usage we could consider
some other data structure like a Bloom filter.
Linking cmd/compile in external linking mode (on macOS/amd64)
When server and client have mismatch in curve preference, the server will
send HelloRetryRequest during TLSv1.3 PSK resumption. There was a bug
introduced by Go1.19.6 or later and Go1.20.1 or later, that makes the client
calculate the PSK binder hash incorrectly. Server will reject the TLS
handshake by sending alert: invalid PSK binder.
Fixes #59424
Change-Id: I2ca8948474275740a36d991c057b62a13392dbb9
GitHub-Last-Rev: 1aad9bcf27f563449c1a7ed6d0dd1d247cc65713
GitHub-Pull-Request: golang/go#59425
Reviewed-on: https://go-review.googlesource.com/c/go/+/481955 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Ian Lance Taylor [Sat, 22 Apr 2023 01:48:38 +0000 (18:48 -0700)]
runtime: call _exit, not exit, on AIX and Solaris
This is the AIX and Solaris equivalent of CL 269378.
On AIX and Solaris, where we use libc for syscalls, when the runtime exits,
it calls the libc exit function, which may call back into user code,
such as invoking functions registered with atexit. In particular, it
may call back into Go. But at this point, the Go runtime is
already exiting, so this wouldn't work.
On non-libc platforms we use exit syscall directly, which doesn't
invoke any callbacks. Use _exit on AIX and Solaris to achieve the same
behavior.
Test is TestDestructorCallback.
For #59711
Change-Id: I666f75538d3e3d8cf3b697b4c32f3ecde8332890
Reviewed-on: https://go-review.googlesource.com/c/go/+/487635
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Fri, 21 Apr 2023 22:28:19 +0000 (15:28 -0700)]
runtime: use platform.RaceDetectorSupported for -race tests
Don't try to duplicate the list of targets that support -race.
Change-Id: I889d5c2f4884de89d88f8efdc89608aa73584a8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/487575
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
internal/testenv: in HasExec, try to actually exec on ios and wasm platforms
Some iOS environments may support exec. wasip1 and js do not, but
trying to exec on those platforms is inexpensive anyway and gives
better test coverage for the ios path.
Xiaodong Liu [Thu, 9 Sep 2021 08:07:38 +0000 (16:07 +0800)]
cmd/compile: support -buildmode=c-shared on linux/mips64{,le}
The modification of these rules is optimization to load/store global
variables. If there are a sequence of loads/stores nearby a global
variable address, the address can only be loaded from GOT once instead
of every time.
For #43264
Change-Id: Idedaf6c81f085955371320f51bca148ffb42a2d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/348732
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Keith Randall [Wed, 29 Mar 2023 16:12:20 +0000 (09:12 -0700)]
flag: panic if a flag is defined after being set
As part of developing #57411, we ran into cases where a flag was
defined in one package init and Set in another package init, and there
was no init ordering implied by the spec between those two
packages. Changes in initialization ordering as part of #57411 caused
a Set to happen before the definition, which makes the Set silently
fail.
This CL makes the Set fail loudly in that situation.
Currently Set *does* fail kinda quietly in that situation, in that it
returns an error. (It seems that no one checks the error from Set,
at least for string flags.) Ian suggsted that instead we panic at
the definition site if there was previously a Set called on that
(at the time undefined) flag.
So Set on an undefined flag is ok and returns an error (as before),
but defining a flag which has already been Set causes a panic. (The
API for flag definition has no way to return an error, and does
already panic in some situations like a duplicate definition.)
Update #57411
Change-Id: I39b5a49006f9469de0b7f3fe092afe3a352e4fcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/480215
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Johan Brandhorst-Satzkorn [Tue, 18 Apr 2023 05:13:52 +0000 (22:13 -0700)]
misc/wasm: support wasmtime in wasip1
Allow switching to wasmtime through the GOWASIRUNTIME variable. This
will allow builders to run the wasip1 standard library tests against
the wasmtime WASI runtime.
For #59583
Change-Id: I4d5200df7bb27b66e041f00e89d4c2e585f5da7c
Reviewed-on: https://go-review.googlesource.com/c/go/+/485615 Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Bypass: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Ian Lance Taylor [Wed, 19 Apr 2023 21:16:37 +0000 (14:16 -0700)]
runtime: in __tsan_fini tell scheduler we are entering non-Go code
__tsan_fini will call exit which will call destructors which
may in principle call back into Go functions. Prepare the scheduler
by calling entersyscall before __tsan_fini.
Fixes #59711
Change-Id: Ic4df8fba3014bafa516739408ccfc30aba4f22ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/486615 Reviewed-by: Michael Pratt <mpratt@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>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Keith Randall [Tue, 21 Mar 2023 16:25:43 +0000 (09:25 -0700)]
cmd/compile: introduce separate memory op combining pass
Memory op combining is currently done using arch-specific rewrite rules.
Instead, do them as a arch-independent rewrite pass. This ensures that
all architectures (with unaligned loads & stores) get equal treatment.
This removes a lot of rewrite rules.
The new pass is a bit more comprehensive. It handles things like out-of-order
writes and is careful not to apply partial optimizations that then block
further optimizations.
Change-Id: I780ff3bb052475cd725a923309616882d25b8d9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/478475 Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
runtime: prevent double lock in checkdead by unlocking before throws
This change resolves an issue where checkdead could result in a double lock when shedtrace is enabled. This fix involves adding unlocks before all throws in the checkdead function to ensure the scheduler lock is properly released.
Fixes #59758
Change-Id: If3ddf9969f4582c3c88dee9b9ecc355a63958103
Reviewed-on: https://go-review.googlesource.com/c/go/+/487375
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Leaked goroutines are the only explanation I can think of for excess
allocs in TestDiscard, and TestOutputRace is the only place I can see
where the log package leaks goroutines. Let's fix that leak and see if
it eliminates the TestDiscard flakes.
runtime, cmd: rationalize StackLimit and StackGuard
The current definitions of StackLimit and StackGuard only indirectly
specify the NOSPLIT stack limit and duplicate a literal constant
(928). Currently, they define the stack guard delta, and from there
compute the NOSPLIT limit.
Rationalize these by defining a new constant, abi.StackNosplitBase,
which consolidates and directly specifies the NOSPLIT stack limit (in
the default case). From this we then compute the stack guard delta,
inverting the relationship between these two constants. While we're
here, we rename StackLimit to StackNosplit to make it clearer what's
being limited.
This change does not affect the values of these constants in the
default configuration. It does slightly change how
StackGuardMultiplier values other than 1 affect the constants, but
this multiplier is a pretty rough heuristic anyway.
before after
stackNosplit 800 800
_StackGuard 928 928
stackNosplit -race 1728 1600
_StackGuard -race 1856 1728