For defer/go calls, the function/method value are evaluated immediately.
So after devirtualizing, it may trigger a panic when implicitly deref
a nil pointer receiver, causing the program behaves unexpectedly.
It's safer to not devirtualizing defer/go calls at all.
Fixes #52072
Change-Id: I562c2860e3e577b36387dc0a12ae5077bc0766bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/428495 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Change-Id: I5d4c89a8d87a8697d038bf91b17d7ffc847a49c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/428137 Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: xie cui <523516579@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I9658c37a548a386c57d83dc13d5e9925a9c13211
Reviewed-on: https://go-review.googlesource.com/c/go/+/428140
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: xie cui <523516579@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I05ebaf4e11b5b6ca7d9bbb0b2241def7773b11cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/428138
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: xie cui <523516579@qq.com>
Change-Id: Ie5a8ae104970237e6e33e75cae807c436bcd08e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/428268
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I407d11b43349cd78b709deb7b0e774bd93922caa
Reviewed-on: https://go-review.googlesource.com/c/go/+/428269 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: Ifb51413894791154489bd1d4f529088958212c62
Reviewed-on: https://go-review.googlesource.com/c/go/+/428270
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: Iaf75e19c9ae8d77b89b3eabb2e9142f59c124536
Reviewed-on: https://go-review.googlesource.com/c/go/+/425878 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Andy Pan [Wed, 31 Aug 2022 13:59:29 +0000 (21:59 +0800)]
os: delete unused errClosed
errClosed was introduced by CL 163058 and was supposed to be removed by CL 243906,
but somehow it was left out, now we should get it deleted.
Change-Id: I74c4b36b8bbc4844e1860acb022a16b0aa3272b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/427035
Run-TryBot: Andy Pan <panjf2000@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I49f8c764d49cabaad4d6859c219ba7220a389c1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/427140
Run-TryBot: xie cui <523516579@qq.com> Reviewed-by: Ian Lance Taylor <iant@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> Reviewed-by: Michael Knyszek <mknyszek@google.com>
runtime: in traceback, only jump stack if M doesn't change
CL 424257 modified gentraceback to switch gp when jumping from a
system stack to a user stack to simplify reasoning through the rest of
the function. This has the unintended side-effect of also switching
all references to gp.m. The vast majority of the time, g0.m and curg.m
are the same across a stack switch, making this a no-op, but there's
at least one case where this isn't true: if a profiling signal happens
in execute between setting mp.curg and setting gp.m. In this case,
mp.curg.m is briefly nil, which can cause gentraceback to crash with a
nil pointer dereference. We see this failure (surprisingly
frequently!) in profiling tests in the morestack=mayMoreStackPreempt
testing mode (#48297).
Fix this by making only jumping stacks if doing so will not switch Ms.
This restores the original property that gp.m doesn't change across
the stack jump, and makes gentraceback a little more conservative
about jumping stacks.
Change-Id: I02c4664f1ba72623a5470e92bbebabb2f4862428
Reviewed-on: https://go-review.googlesource.com/c/go/+/428264 Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: Ibab20627ccd1f79e77e3972e2bc6ca42c2c7de13
Reviewed-on: https://go-review.googlesource.com/c/go/+/428263
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: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I0a51aa9e7800689c123ce3eaf74742a4641b7681
Reviewed-on: https://go-review.googlesource.com/c/go/+/428261
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: Iab6ef8c6c74c09e8358f7d7088a82089725479ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/428260
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Change-Id: Iee846c4ac0f111ff97aa618dd42f6b2d14aa4342
Reviewed-on: https://go-review.googlesource.com/c/go/+/428259
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Change-Id: Ib07699df8ea41fd8d1bca8ad050859fac24623de
Reviewed-on: https://go-review.googlesource.com/c/go/+/428258 Reviewed-by: Robert Griesemer <gri@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>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Change-Id: I59db11212758b478248f34c031501a167335348c
Reviewed-on: https://go-review.googlesource.com/c/go/+/428257
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: Ief82f12a5c48c46165c28140997d5fc83700c758
Reviewed-on: https://go-review.googlesource.com/c/go/+/428256
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Change-Id: I07e022ffc022bca6aa12165871e723db3ed83141
Reviewed-on: https://go-review.googlesource.com/c/go/+/428255 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Change-Id: Icb53d32f2de13287b1b4f4f67dab90fe5ee7a3df
Reviewed-on: https://go-review.googlesource.com/c/go/+/428254 Reviewed-by: Robert Griesemer <gri@google.com>
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>
Change-Id: Ic7f827b8ee758c991ed76be65026b0330207deea
Reviewed-on: https://go-review.googlesource.com/c/go/+/428262 Reviewed-by: Ian Lance Taylor <iant@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> Reviewed-by: Michael Pratt <mpratt@google.com>
Daniel Martí [Mon, 5 Sep 2022 08:37:48 +0000 (08:37 +0000)]
Revert "internal/poll: drop redundant ENOSYS and EXDEV error checks in CopyFileRange()"
This reverts https://go.dev/cl/425881.
Reason for revert: broke make.bash on linux/amd64 with Linux 5.19.6.
[...]
Building Go toolchain2 using go_bootstrap and Go toolchain1.
go install internal/unsafeheader: copying /tmp/go-build4206185186/b007/_pkg_.a to /home/mvdan/tip/pkg/linux_amd64/internal/unsafeheader.a: write /home/mvdan/tip/pkg/linux_amd64/internal/unsafeheader.a: copy_file_range: invalid cross-device link
go install internal/goarch: copying /tmp/go-build4206185186/b006/_pkg_.a to /home/mvdan/tip/pkg/linux_amd64/internal/goarch.a: write /home/mvdan/tip/pkg/linux_amd64/internal/goarch.a: copy_file_range: invalid cross-device link
[...]
Change-Id: I793856935d4315a870c2d31da46be00cc342b5f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/428396 Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: hopehook <hopehook@golangcn.org> Reviewed-by: Andy Pan <panjf2000@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Leonard Wang [Wed, 13 Apr 2022 13:14:22 +0000 (21:14 +0800)]
runtime: refactor finalizer goroutine status
Use an atomic.Uint32 to represent the state of finalizer goroutine.
fingStatus will only be changed to fingWake in non fingWait state,
so it is safe to set fingRunningFinalizer status in runfinq.
name old time/op new time/op delta
Finalizer-8 592µs ± 4% 561µs ± 1% -5.22% (p=0.000 n=10+10)
FinalizerRun-8 694ns ± 6% 675ns ± 7% ~ (p=0.059 n=9+8)
Change-Id: I7e4da30cec98ce99f7d8cf4c97f933a8a2d1cae1
Reviewed-on: https://go-review.googlesource.com/c/go/+/400134 Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Michael Pratt <mpratt@google.com>
Andy Pan [Sat, 27 Aug 2022 02:44:25 +0000 (10:44 +0800)]
internal/poll: drop redundant ENOSYS and EXDEV error checks in CopyFileRange()
The initial CL 229101 didn't limit the kernel version, but relies on error checking to
ensure the kernel version >= 4.5 or >= 5.3 when it's calling copy_file_range(2) to copy data across file systems.
Since we have now put the kernel version checking at the beginning of the function, introduced by CL 268338,
which returns early instead of going forward to the code behind when the kernel verion is older than 5.3,
therefore, those subsequent related error checks are no longer needed.
Change-Id: Ifc4a530723e21f0bde91d6420cde9cb676081922
Reviewed-on: https://go-review.googlesource.com/c/go/+/425881
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: hopehook <hopehook@golangcn.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Heschi Kreinick <heschi@google.com>
Eliza Weisman [Wed, 31 Aug 2022 19:16:18 +0000 (19:16 +0000)]
net/http: don't time out idle server connections after ReadHeaderTimeout
Consistently wait for idle connections to become readable before
starting the ReadHeaderTimeout timer. Previously, connections with no
idle timeout skipped directly to reading headers, so the
ReadHeaderTimeout also included time spent idle.
Tomasz Jezierski [Sun, 24 Jul 2022 21:02:53 +0000 (23:02 +0200)]
net: precompute rfc6724policyTable in addrselect
As net package has one of the biggest init time in standard library, I have tried to improve performance by doing two things in net/addrselect.go:
1. Precompute slice with RFC rules. Currently the rules are computed and sorted in init() function. We could save the time and allocations by using prepopulated values in sorted manner. The rules haven't changed since 2015. To be extra safe we could move order validation as test case. It should slightly speed up startup of each binary with "net" package and go dns resolver. It also saves 38 allocations, ~50% of allocations in init phase of `net` module.
2. Replace internal net.IP usage with netip.Addr in `sortByRFC6724` function. It results in ~40% performance improvement on samples from tests.
The only risk is the difference between net.IP and netip.Addr behaviour.
Robert Griesemer [Fri, 2 Sep 2022 19:10:24 +0000 (12:10 -0700)]
go/types, types2: use strings.Builder instead of bytes.Buffer where possible
Also, consistently use declaration: var buf strings.Builder.
We don't change exported signatures to match go/types (where we
can't change the exported signatures for backward-compatibility).
Change-Id: I75350886aa231889ae2fd5c4008dd4be9ed6e09f
Reviewed-on: https://go-review.googlesource.com/c/go/+/428094
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Kir Kolyshkin [Fri, 15 Jul 2022 02:40:23 +0000 (19:40 -0700)]
syscall: fix skipping some tests on Linux
The kernel knob /proc/sys/kernel/unprivileged_userns_clone is
only available in Debian (and Ubuntu) kernels, so if the tests
are run on e.g. Fedora, skipUnprivilegedUserClone() skips a lot
of tests.
Modify it to treat ENOENT as "it should work".
Change-Id: I959201ede139ede989cc8ab646c9bf51e0539ada
Reviewed-on: https://go-review.googlesource.com/c/go/+/417694
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Kirill Kolyshkin <kolyshkin@gmail.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Carlo Alberto Ferraris [Thu, 1 Sep 2022 05:06:55 +0000 (14:06 +0900)]
fmt: recycle printers with large buffers
Previously when a printer had a large buffer we dropped both
the buffer and the printer. There is no need to drop the printer
in this case, as a printer with a nil buffer is valid. So we
just drop the buffer and recycle the printer anyway.
This saves one allocation in case the buffer is over the limit.
Also tighten some of the tests for other unrelated cases.
Change-Id: Iba1b6a71ca4691464b8c68ab0b6ab0d4d5d6168c
Reviewed-on: https://go-review.googlesource.com/c/go/+/427395 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Rob Pike <r@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Rob Pike <r@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Matthew Dempsky [Thu, 1 Sep 2022 23:58:15 +0000 (16:58 -0700)]
cmd/compile/internal/noder: optimize itabs section of runtime dicts
Currently, the itabs section for runtime dictionaries includes its own
redundant *runtime._type pointers for typ and iface, which were
sometimes necessary. This simplified the initial implementation, but
is a little wasteful of space when the same type or interface appeared
across multiple (typ, iface) pairs.
This CL instead reuses the pointers from the rtypes section.
Change-Id: I48448515c319c0403c1a8e7706794d443176f0a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/427754 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Joel Sing [Sat, 27 Aug 2022 19:23:28 +0000 (05:23 +1000)]
cmd/compile: optimise subtraction with const on riscv64
Convert subtraction from const to a negated ADDI with negative const
value, where possible. At worst this avoids a register load and uses
the same number of instructions. At best, this allows for further
optimisation to occur, particularly where equality is involved.
For example, this sequence:
li t0,-1
sub t1,t0,a0
snez t1,t1
Becomes:
addi t0,a0,1
snez t0,t0
Removes more than 2000 instructions from the Go binary on linux/riscv64.
Change-Id: I68f3be897bc645d4a8fa3ab3cef165a00a74df19
Reviewed-on: https://go-review.googlesource.com/c/go/+/426263 Reviewed-by: Meng Zhuo <mzh@golangcn.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Joel Sing [Sat, 27 Aug 2022 16:32:06 +0000 (02:32 +1000)]
cmd/compile: negate comparision with FNES/FNED on riscv64
The FNES and FNED instructions are pseudo-instructions, which the
assembler expands to FEQS/NEG or FEQD/NEG - if we're comparing the
result via a branch instruction, we can avoid an instruction by
negating both the branch comparision and the floating point
comparision.
This only removes a handful of instructions from the Go binary,
however, it will provide benefit to floating point intensive code.
Change-Id: I4e3124440b7659acc4d9bc9948b755a4900a422f
Reviewed-on: https://go-review.googlesource.com/c/go/+/426261 Reviewed-by: Meng Zhuo <mzh@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Run-TryBot: Meng Zhuo <mzh@golangcn.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Robert Griesemer [Fri, 2 Sep 2022 17:24:53 +0000 (10:24 -0700)]
go/types, types2: consistently write "x | y" rather than "x|y" for unions
Use the same spacing convention ("x | y") for union terms everythere,
matching the gofmt precedent.
Fixes #53279.
Change-Id: Ic3ccd7433b5f62402ba41cf05a75f9a1d99a8086
Reviewed-on: https://go-review.googlesource.com/c/go/+/410955 Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Austin Clements [Wed, 17 Aug 2022 13:06:14 +0000 (09:06 -0400)]
runtime: consolidate stkframe and its methods into stkframe.go
The stkframe struct and its methods are strewn across different source
files. Since they actually have a pretty coherent theme at this point,
migrate it all into a new file, stkframe.go. There are no code changes
in this CL.
Austin Clements [Sun, 14 Aug 2022 01:39:56 +0000 (21:39 -0400)]
runtime: replace stkframe.arglen/argmap with methods
Currently, stkframe.arglen and stkframe.argmap are populated by
gentraceback under a particular set of circumstances. But because they
can be constructed from other fields in stkframe, they don't need to
be computed eagerly at all. They're also rather misleading, as they're
only part of computing the actual argument map and most callers should
be using getStackMap, which does the rest of the work.
This CL drops these fields from stkframe. It shifts the functions that
used to compute them, getArgInfoFast and getArgInfo, into
corresponding methods stkframe.argBytes and stkframe.argMapInternal.
argBytes is expected to be used by callers that need to know only the
argument frame size, while argMapInternal is used only by argBytes and
getStackMap.
We also move some of the logic from getStackMap into argMapInternal
because the previous split of responsibilities didn't make much sense.
This lets us return just a bitvector from argMapInternal, rather than
both a bitvector, which carries a size, and an "actually use this
size".
The getArgInfoFast function was inlined before (and inl_test checked
this). We drop that requirement from stkframe.argBytes because the
uses of this have shifted and now it's only called from heap dumping
(which never happens) and conservative stack frame scanning (which
very, very rarely happens).
There will be a few follow-up clean-up CLs.
For #54466. This is a nice clean-up on its own, but it also serves to
remove pointers from the traceback state that would eventually become
troublesome write barriers once we stack-rip gentraceback.
runtime: switch gp when jumping stacks during traceback
Currently, when traceback jumps from the system stack to a user stack
(e.g., during profiling tracebacks), it leaves gp pointing at the g0.
This is currently harmless since it's only used during profiling, so
the code paths in gentraceback that care about gp aren't used, but
it's really confusing and would certainly break if _TraceJumpStack
were ever used in a context other than profiling.
Fix this by updating gp to point to the user g when we switch stacks.
Austin Clements [Tue, 16 Aug 2022 17:04:16 +0000 (13:04 -0400)]
runtime: drop redundant argument to getArgInfo
The f funcInfo argument is always the same as frame.fn, so we don't
need to pass it. I suspect that was there to make the signatures of
getArgInfoFast and getArgInfo more similar, but it's not necessary.
Austin Clements [Mon, 15 Aug 2022 14:41:03 +0000 (10:41 -0400)]
runtime: drop function context from traceback
Currently, gentraceback tracks the closure context of the outermost
frame. This used to be important for "unstarted" calls to reflect
function stubs, where "unstarted" calls are either deferred functions
or the entry-point of a goroutine that hasn't run. Because reflect
function stubs have a dynamic argument map, we have to reach into
their closure context to fetch to map, and how to do this differs
depending on whether the function has started. This was discovered in
issue #25897.
However, as part of the register ABI, "go" and "defer" were made much
simpler, and any "go" or "defer" of a function that takes arguments or
returns results gets wrapped in a closure that provides those
arguments (and/or discards the results). Hence, we'll see that closure
instead of a direct call to a reflect stub, and can get its static
argument map without any trouble.
The one case where we may still see an unstarted reflect stub is if
the function takes no arguments and has no results, in which case the
compiler can optimize away the wrapper closure. But in this case we
know the argument map is empty: the compiler can apply this
optimization precisely because the target function has no argument
frame.
As a result, we no longer need to track the closure context during
traceback, so this CL drops all of that mechanism.
We still have to be careful about the unstarted case because we can't
reach into the function's locals frame to pull out its context
(because it has no locals frame). We double-check that in this case
we're at the function entry.
I would prefer to do this with some in-code PCDATA annotations of
where to find the dynamic argument map, but that's a lot of mechanism
to introduce for just this. It might make sense to consider this along
with #53609.
Finally, we beef up the test for this so it more reliably forces the
runtime down this path. It's fundamentally probabilistic, but this
tweak makes it better. Scheduler testing hooks (#54475) would make it
possible to write a reliable test for this.
For #54466, but it's a nice clean-up all on its own.
Matthew Dempsky [Thu, 1 Sep 2022 23:06:11 +0000 (16:06 -0700)]
cmd/compile/internal/noder: allow OCONVNOP for identical iface conversions
In go.dev/cl/421821, I included a hack to force OCONVNOP back to
OCONVIFACE for conversions involving shape types and non-empty
interfaces. The comment correctly noted that this was only needed for
conversions between non-identical types, but the code was conservative
and applied to even conversions between identical types.
This CL adds an extra bool to record whether the conversion is between
identical types, so we can keep OCONVNOP instead of forcing back to
OCONVIFACE. This has a small improvement to generated code, because we
no longer need a convI2I call (as demonstrated by codegen/ifaces.go).
But more usefully, this is relevant to pruning unnecessary itab slots
in runtime dictionaries (next CL).
Change-Id: I94f89e961cd26629b925037fea58d283140766ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/427678 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Derek Parker [Tue, 30 Aug 2022 21:48:17 +0000 (21:48 +0000)]
cmd/compile: new inline heuristic for struct compares
This CL changes the heuristic used to determine whether we can inline a
struct equality check or if we must generate a function and call that
function for equality.
The old method was to count struct fields, but this can lead to poor
in lining decisions. We should really be determining the cost of the
equality check and use that to determine if we should inline or generate
a function.
The new benchmark provided in this CL returns the following when compared
against tip:
```
name old time/op new time/op delta
EqStruct-32 2.46ns ± 4% 0.25ns ±10% -89.72% (p=0.000 n=39+39)
```
Fixes #38494
Change-Id: Ie06b80a2b2a03a3fd0978bcaf7715f9afb66e0ab
GitHub-Last-Rev: e9a18d93893cc6493794683bf75b9848478a4de6
GitHub-Pull-Request: golang/go#53326
Reviewed-on: https://go-review.googlesource.com/c/go/+/411674 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
Tobias Klauser [Wed, 31 Aug 2022 17:05:55 +0000 (19:05 +0200)]
cmd/compile/internal/base: use runtime.KeepAlive in MapFile
Go 1.17 will be used instead of Go 1.4 as minimum required version for
bootstrap, so runtime.KeepAlive introduced in Go 1.7 can be used in
cmd/compile.
Robert Griesemer [Fri, 2 Sep 2022 03:47:41 +0000 (20:47 -0700)]
go/types: use function name position for init errors
This seems more sensible than the func keyword. With this change,
go/types uses the same error position as types2 and we can narrow
the error tolerance a bit.
(The types2 change doesn't change its position, but it makes the
code clearer and symmetric to go/types.)
Change-Id: Iedea7c80caa7239a4343c8748cb779ec545e84d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/427775
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Cherry Mui [Thu, 1 Sep 2022 23:31:04 +0000 (19:31 -0400)]
cmd/link: only add dummy XCOFF reference if the symbol exists
On AIX when external linking, for some symbols we need to add
dummy references to prevent the external linker from discarding
them. Currently we add the reference unconditionally. But if the
symbol doesn't exist, the linking fails in a later stage for
generating external relocation of a nonexistent symbol. The
symbols are special symbols that almost always exist, except that
go:buildid may not exist if the linker is invoked without the
-buildid flag. The go command invokes the linker with the flag, so
this can only happen with manual linker invocation. Specifically,
test/run.go does this in some cases.
Fix this by checking the symbol existence before adding the
reference. Re-enable tests on AIX.
Perhaps the linker should always emit a dummy buildid even if the
flag is not set...
Fixes #54814.
Change-Id: I43d81587151595309e189e38960cbda9a1c5ca32
Reviewed-on: https://go-review.googlesource.com/c/go/+/427620
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Cuong Manh Le [Sun, 7 Aug 2022 18:10:18 +0000 (01:10 +0700)]
cmd/compile: restrict //go:notinheap to runtime/internal/sys
So it won't be visible outside of runtime package. There are changes to
make tests happy:
- For test/directive*.go files, using "go:noinline" for testing misplaced
directives instead.
- Restrict test/fixedbugs/bug515.go for gccgo only.
- For test/notinheap{2,3}.go, using runtime/cgo.Incomplete for marking
the type as not-in-heap. Though it's somewhat clumsy, it's the easiest
way to keep the test errors for not-in-heap types until we can cleanup
further.
- test/typeparam/mdempsky/11.go is about defined type in user code marked
as go:notinheap, which can't happen after this CL, though.
Fixes #46731
Change-Id: I869f5b2230c8a2a363feeec042e7723bbc416e8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/421882
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Joedian Reid <joedian@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Robert Griesemer [Fri, 2 Sep 2022 01:38:08 +0000 (18:38 -0700)]
go/types, types2: move shared tests into internal/types/testdata
This CL moves the directories check, examples, fixedbugs, and spec
from inside go/types/testdata to internal/types/testdata. Except
for the directory adjustments to check_test.go files, this is a
pure file move.
With this CL, both type checkers now share identical tests in an
independent location.
Fixes #54511.
Change-Id: Ib335692d927e93867a158b338f105c2b87e74dbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/427674 Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Robert Griesemer [Thu, 1 Sep 2022 00:40:21 +0000 (17:40 -0700)]
go/types: test shifts that are disabled in shared test file
CL 425735 consolidated the testdata/check/shifts.go files between
go/types and types2. Because some shifts don't work correctly with
types2, the corresponding tests were disabled in the shared file.
Make sure we keep testing those shifts for go/types by adding a
local test file.
For #52080.
For #54511.
Change-Id: I53507e535bf83b204eaf18fc6c2efefcebf5ebf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/426661 Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Robert Griesemer [Thu, 1 Sep 2022 00:06:29 +0000 (17:06 -0700)]
cmd/compile/internal/types2: use go/types/testdata/fixedbugs tests
Since the fixedbugs tests are now identical between the two type checkers,
remove the local copy of the fixedbugs tests and (for now) use the tests
in go/types/testdata/fixedbugs instead. Eventually we may decide to move
all tests out of the type checker directories and place them in a
shared space (e.g. internal/types/testdata).
For #54511.
Change-Id: I451c20c784710c36fa50b1d24ac97af554c572af
Reviewed-on: https://go-review.googlesource.com/c/go/+/426658
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Wed, 31 Aug 2022 23:56:17 +0000 (16:56 -0700)]
go/types, types2: consolidate testdata/fixedbugs test files
Use the go/types version of testdata/fixedbugs tests where diffs
are only in the error positions (the types2 test harness allows
for some position tolerance). Consolidate files where there are
other minor differences.
Add files to respective directories if they only existed for
one of the type checkers.
Move types2-only test issue47996.go out of testdata/fixedbugs
into testdata. Making it work for both type checkers requires
some more work.
With this CL, the testdata/fixedbugs files are identical between
the two type checkers.
For #54511.
Change-Id: I0d67f0db75ad1743c62da9181a6d0032c8bdb728
Reviewed-on: https://go-review.googlesource.com/c/go/+/427236 Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>