Remove logic for skipping some adjustframe logic for systemstack (aka
FuncID_systemstack_switch). This was introduced in 2014 by
9198ed4bd6 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>
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>
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>
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>
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>
The type name symbol is always from a Go object file and we never
change it. Convert the data to string using unsafe conversion
without allocation.
Linking cmd/go (on macOS/amd64),
name old alloc/op new alloc/op delta
Deadcode_GC 1.25MB ± 0% 1.17MB ± 0% -6.29% (p=0.000 n=20+20)
name old allocs/op new allocs/op delta
Deadcode_GC 8.98k ± 0% 0.10k ± 3% -98.91% (p=0.000 n=20+20)
Change-Id: I33117ad1f991e4f14ce0b38cceec50b041e3c0a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/490915
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
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>
The test will wait for all goroutines.
A race can occur if the writing goroutine uses the Log after the test exits.
For #58264
For #59883
For #59884
Change-Id: I9b8ec7c9d024ff74b922b69efa438be5a4fa3483
Reviewed-on: https://go-review.googlesource.com/c/go/+/490255
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
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.
Fixes#57926
Change-Id: I513cffb7e13bc93d08a07b7e61301ac1762fd42d
Reviewed-on: https://go-review.googlesource.com/c/go/+/490819
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
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>
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>
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.
Updates #57302
[1] https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-unwind_info
Change-Id: I6f305a51ed130b758ff9ca7b90c091e50a109a6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/457455
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Davis Goodin <dagood@microsoft.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
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.
Updates #57302
[1] https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-pdata-section
[2] https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-runtime_function
Change-Id: If675d10c64452946dbab76709da20569651e3e9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/461738
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
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>
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>
We only did it for testing. Remove the single test that required it.
Change-Id: Ib6c3a2debfd3f48e95af37f23fdfde847ff87a41
Reviewed-on: https://go-review.googlesource.com/c/go/+/490395
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
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>
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>
Matches the preceding "wrap" terminology.
Change-Id: Ia783de578c2942fe1474281c3d6056b1074d41b0
GitHub-Last-Rev: 4fcff4e9b2
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>
cgo.NewHandle atomically increments a global uintptr index using
atomic.AddUintptr. Use atomic.Uintptr instead, which is
cleaner and clearer.
Change-Id: I845b3e4cb8c461e787a9b9bb2a9ceaaef1d21d8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/490775
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
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.
Change-Id: I036cb1af9c034462a952b176a794526fa3ffd1ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/490495
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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 #47653Fixes#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>
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:
$ GODEBUG=tracefpunwindoff=0 ../bin/go test -trace /dev/null -run TestDeferHeapAndStack ./runtime
SIGSEGV: segmentation violation
PC=0x45f977 m=14 sigcode=128
goroutine 0 [idle]:
runtime.fpTracebackPCs(...)
.../go/src/runtime/trace.go:945
runtime.traceStackID(0xcdab904677a?, {0x7f1584346018, 0x0?, 0x80}, 0x0?)
.../go/src/runtime/trace.go:917 +0x217 fp=0x7f1565ffab00 sp=0x7f1565ffaab8 pc=0x45f977
runtime.traceEventLocked(0x0?, 0x0?, 0x0?, 0xc00003dbd0, 0x12, 0x0, 0x1, {0x0, 0x0, 0x0})
.../go/src/runtime/trace.go:760 +0x285 fp=0x7f1565ffab78 sp=0x7f1565ffab00 pc=0x45ef45
runtime.traceEvent(0xf5?, 0x1, {0x0, 0x0, 0x0})
.../go/src/runtime/trace.go:692 +0xa9 fp=0x7f1565ffabe0 sp=0x7f1565ffab78 pc=0x45ec49
runtime.traceGoPreempt(...)
.../go/src/runtime/trace.go:1535
runtime.gopreempt_m(0xc000328340?)
.../go/src/runtime/proc.go:3551 +0x45 fp=0x7f1565ffac20 sp=0x7f1565ffabe0 pc=0x4449a5
runtime.newstack()
.../go/src/runtime/stack.go:1077 +0x3cb fp=0x7f1565ffadd0 sp=0x7f1565ffac20 pc=0x455feb
runtime.morestack()
.../go/src/runtime/asm_amd64.s:593 +0x8f fp=0x7f1565ffadd8 sp=0x7f1565ffadd0 pc=0x47644f
goroutine 19 [running]:
runtime.traceEvent(0x2c?, 0xffffffffffffffff, {0x0, 0x0, 0x0})
.../go/src/runtime/trace.go:669 +0xe8 fp=0xc0006e6c28 sp=0xc0006e6c20 pc=0x45ec88
runtime.traceGCMarkAssistDone(...)
.../go/src/runtime/trace.go:1497
runtime.gcAssistAlloc(0xc0003281a0)
.../go/src/runtime/mgcmark.go:517 +0x27d fp=0xc0006e6c88 sp=0xc0006e6c28 pc=0x421a1d
runtime.deductAssistCredit(0x0?)
.../go/src/runtime/malloc.go:1287 +0x54 fp=0xc0006e6cb0 sp=0xc0006e6c88 pc=0x40fed4
runtime.mallocgc(0x400, 0x7a9420, 0x1)
.../go/src/runtime/malloc.go:1002 +0xc9 fp=0xc0006e6d18 sp=0xc0006e6cb0 pc=0x40f709
runtime.newobject(0xb3?)
.../go/src/runtime/malloc.go:1324 +0x25 fp=0xc0006e6d40 sp=0xc0006e6d18 pc=0x40ffc5
runtime_test.deferHeapAndStack(0xb4)
.../go/src/runtime/stack_test.go:924 +0x165 fp=0xc0006e6e20 sp=0xc0006e6d40 pc=0x75c2a5
Fixes#59692
Co-Authored-By: Cherry Mui <cherryyz@google.com>
Co-Authored-By: Michael Knyszek <mknyszek@google.com>
Co-Authored-By: Nick Ripley <nick.ripley@datadoghq.com>
Change-Id: I1c0c28327fc2fac0b8cfdbaa72e25584331be31e
Reviewed-on: https://go-review.googlesource.com/c/go/+/489015
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>
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>
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>
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>
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>
Symbol names in the final executable apply escaping to the final
component of a package path (main in example.com becomes
example%2ecom.main).
ir.PkgFuncName does not perform this escaping, meaning we'd fail to
match functions that are escaped in the profile.
Add ir.LinkFuncName which does perform escaping and use it for PGO.
Fixes#59887.
Change-Id: I10634d63d99d0a6fd2f72b929ab35ea227e1336f
Reviewed-on: https://go-review.googlesource.com/c/go/+/490555
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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>
Put zero-sized data symbols at same address as runtime.zerobase,
so zero-sized global variables have the same address as zero-sized
allocations.
Change-Id: Ib3145dc1b663a9794dfabc0e6abd2384960f2c49
Reviewed-on: https://go-review.googlesource.com/c/go/+/490435
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
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>
While at it, also simplify mustTypecheck again as it can just use
typecheck.
Change-Id: I6cb07b1078d9a39e0f22851028fdd4442127f2f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/490015
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This simplifies explicit tests and ensures that the error messages
contain the package name instead of a generic file name like "p.go".
Fixes#59736.
Change-Id: I1b42e30f53ba88456e92f990d80ca68ffc987e20
Reviewed-on: https://go-review.googlesource.com/c/go/+/486617
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
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>
Syntactically incorrect source files may produce valid (but
unexpected) syntax trees, leading to difficult to understand
test failures.
Make sure to call mustParse when we call mustTypecheck.
Change-Id: I9f5ba3fe57ad3bbc16caabf285d2e7aeb5b9de0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/489995
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
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>
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>
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>
This reverts CL 486895.
Reason for revert: This breaks internal tests at Google, see b/280035614.
Change-Id: I48772a44f5f6070a7f06b5704e9f9aa272b5f978
Reviewed-on: https://go-review.googlesource.com/c/go/+/490156
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Michael Stapelberg <stapelberg@google.com>
Reviewed-by: David Chase <drchase@google.com>
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>
- 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>
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>
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>
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.)
For #52545.
Change-Id: Iea6d86a8c9c8b0f1fe51888faf7f5fe7dd8f1eb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/488236
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
