Ian Lance Taylor [Tue, 4 Apr 2023 20:59:06 +0000 (13:59 -0700)]
misc/cgo: gofmt
Change-Id: I5d02279d0593a8368b2f249a6b53650b89aed7b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/482275 Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Tue, 28 Mar 2023 16:45:17 +0000 (12:45 -0400)]
cmd/compile: don't inline hot calls into big functions
Standard inlining has a reduced maximum cost of callees (20 instead of
80) when inlining into a "big" function, to limit how much bigger we
will make an already big function.
When adding PGO hot call budget increases, we inadvertently bypassed
this "big" function restriction, allowing hot calls of up to
inlineHotMaxBudget, even into big functions.
Add the restriction back, even for hot calls. If a function is already
very large, we probably shouldn't inline even more.
A very important note here is that function "big"-ness is computed prior
to any inlining. One potential problem with PGO is that many hot calls
inline into an initially-small function and ultimately make it very
large. This CL does nothing to address that case, which would require
recomputing size after inlining.
This CL has no impact on sweet PGO benchmarks. I specifically dug into
tile38, which contained 0 hot big functions. Other benchmarks are
probably similar.
Change-Id: I3b6304eaf7738a219359d4b8bb121d68babfea8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/482157
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Michael Matloob [Mon, 3 Apr 2023 20:07:41 +0000 (16:07 -0400)]
cmd/go: return and handle errors from loadImport for bad imports
And assign the error to the importing package. Before this change, for
some errors for bad imports, such as importing a vendored package with
the wrong path, we would make a dummy package for the imported package
with the error on it. Instead report the error on the importing
package where it belongs. Do so by returning an error from loadImport
and handling it on the importing package.
For #59157
Change-Id: I952e1a82af3857efc5da4fd3f8bc6be473a60cf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/482877 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Daniel Martí [Fri, 7 Apr 2023 17:41:10 +0000 (18:41 +0100)]
reflect: remove typedmemmovepartial as it is unused
It appears to have been unused since https://go.dev/cl/298670
in April 2021, as that change removed its only use.
It is always in the git history if it is needed again.
Change-Id: Ie55d059c102dfaa75bd253e09d48a4b30f45e941
Reviewed-on: https://go-review.googlesource.com/c/go/+/483136 Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
qmuntal [Thu, 9 Mar 2023 13:55:31 +0000 (14:55 +0100)]
runtime: support GOTRACEBACK=wer on Windows
GOTRACEBACK=wer is a new traceback level that acts as "crash" and
also enables WER. The same effect can be achieved using
debug.SetTraceback("wer").
The Go runtime currently crashes using exit(2), which bypasses WER
even if it is enabled. To best way to trigger WER is calling
RaiseFailFastException [1] instead, which internally launches the
WER machinery.
This CL also changes how GOTRACEBACK=crash crashes, so both "wer" and
"crash" crash using RaiseFailFastException, which simplifies the
implementation and resolves a longstanding TODO.
cmd/compile: teach prove about bitwise OR operation
For now, only apply the rule if either of arguments are constants. That
would catch a lot of real user code, without slowing down the compiler
with code generated for string comparison (experience in CL 410336).
Updates #57959
Fixes #45928
Change-Id: Ie2e830d6d0d71cda3947818b22c2775bd94f7971
Reviewed-on: https://go-review.googlesource.com/c/go/+/483359
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
limeidan [Thu, 25 Aug 2022 03:07:25 +0000 (11:07 +0800)]
runtime: add support for --buildmode=c-shared on loong64
These c-shared related CLs are follow up of CLs 455016, 455017, 455018. Here we
follow the LoongArch ELF psABI v2 standard, which requires the support of the
PCALAU12I instruction.
Updates #53301
Updates #58784
Change-Id: I7f1ddbf3b2470d610f12069d147aa9b3a6a96f32
Reviewed-on: https://go-review.googlesource.com/c/go/+/425474 Reviewed-by: abner chenc <chenguoqi@loongson.cn> Reviewed-by: WANG Xuerui <git@xen0n.name> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@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>
WANG Xuerui [Sat, 3 Dec 2022 13:16:49 +0000 (21:16 +0800)]
cmd/internal/obj/loong64, cmd/link/internal: switch to LoongArch ELF psABI v2 relocs
The LoongArch ELF psABI v2 [1] relocs are vastly simplified from the v1
which involved a stack machine for computing the reloc values, but the
details of PC-relative addressing are changed as well. Specifically, the
`pcaddu12i` instruction is substituted with the `pcalau12i`, which is
like arm64's `adrp` -- meaning the lower bits of a symbol's address now
have to be absolute and not PC-relative.
However, apart from the little bit of added complexity, the obvious
advantage is that only 1 reloc needs to be emitted for every kind of
external reloc we care about. This can mean substantial space savings
(each RELA reloc occupies 24 bytes), and no open-coded stack ops has to
remain any more.
While at it, update the preset value for the output ELF's flags to
indicate the psABI update.
Change-Id: I5c13bc710eaf58293a32e930dd33feff2ef14c28
Reviewed-on: https://go-review.googlesource.com/c/go/+/455017
Run-TryBot: Ben Shi <powerman1st@163.com> Reviewed-by: xiaodong liu <teaofmoli@gmail.com> Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Meidan Li <limeidan@loongson.cn>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
WANG Xuerui [Sun, 4 Dec 2022 07:06:45 +0000 (15:06 +0800)]
cmd/compile, cmd/dist, internal: enable buildmode=pie for linux/loong64
This is actually not tied to the ELF psABI v2 upgrade, and can be
enabled "for free".
Change-Id: I6906d9eb4bd8655c685b059283e200cb7e210369
Reviewed-on: https://go-review.googlesource.com/c/go/+/455075 Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Run-TryBot: Ben Shi <powerman1st@163.com> Reviewed-by: Meidan Li <limeidan@loongson.cn>
Run-TryBot: WANG Xuerui <git@xen0n.name>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
Joel Sing [Wed, 5 Apr 2023 20:11:14 +0000 (06:11 +1000)]
cmd/compile/internal/amd64: improve fix up code for signed division
In order to avoid a CPU exception resulting from signed overflow, the signed
division code tests if the divisor is -1 and if it is, runs fix up code to
manually compute the quotient and remainder (thus avoiding IDIV and potential
signed overflow).
However, the way that this is currently structured means that the normal code
path for the case where the divisor is not -1 results in five instructions
and two branches (CMP, JEQ, followed by sign extension, IDIV and another JMP
to skip over the fix up code).
Rework the fix up code such that the final JMP is incurred by the less likely
divisor is -1 code path, rather than more likely code path (which is already
more expensive due to IDIV). This result in a four instruction sequence
(CMP, JNE, sign extension, IDIV), with only a single branch.
Updates #59089
Change-Id: Ie8d065750a178518d7397e194920b201afeb0530
Reviewed-on: https://go-review.googlesource.com/c/go/+/482658
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Joel Sing [Wed, 5 Apr 2023 20:11:10 +0000 (06:11 +1000)]
cmd/compile/internal/amd64: simplify code generation for signed division
The same switch statement handles code generation for signed division of
words, double words and quad words. Rather than using multiple switch
statements to select the appropriate instructions, determine all of the
correctly sized operands up front, then use them as needed.
Updates #59089
Change-Id: I2b7567c8e0ecb9904c37607332538c95b0521dca
Reviewed-on: https://go-review.googlesource.com/c/go/+/482657
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Fri, 7 Apr 2023 21:03:55 +0000 (14:03 -0700)]
slices: add DeleteFunc
Fixes #54768
Change-Id: I588ae33c13e0bbd9d324c11771667b22a864047d
Reviewed-on: https://go-review.googlesource.com/c/go/+/483175 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: Eli Bendersky <eliben@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Keith Randall [Sat, 1 Apr 2023 17:33:26 +0000 (10:33 -0700)]
cmd/compile: use correct type for byteswaps on multi-byte stores
Use the type of the store for the byteswap, not the type of the
store's value argument.
Normally when we're storing a 16-bit value, the value being stored is
also typed as 16 bits. But sometimes it is typed as something smaller,
usually because it is the result of an upcast from a smaller value,
and that upcast needs no instructions.
If the type of the store's arg is thinner than the type being stored,
and the byteswap'd value uses that thinner type, and the byteswap'd
value needs to be spilled & restored, that spill/restore happens using
the thinner type, which causes us to lose some of the top bits of the
value.
Fixes #59367
Change-Id: If6ce1e8a76f18bf8e9d79871b6caa438bc3cce4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/481395 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 7 Apr 2023 00:26:55 +0000 (20:26 -0400)]
cmd/internal/objfile: align the load address of ELF binary
The ELF ABI just requires that the address and the offset of a
segment are congruent modulo the alignment, but does not require
the start address to be aligned. While usually the segment's
start address is aligned, apparently the zig linker generates
binary with unaligned address.
At the run time, the memory mapping that contains the segment
starts at an aligned address (rounding down). Use the aligned
address for the load address, which matches the mapping.
Apparently this is what the pprof library expects.
Fixes #59466.
Change-Id: Ife78909b20b7bc975ac4c76f2c5f5db325ddec9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/483035
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
WANG Xuerui [Tue, 4 Apr 2023 08:35:06 +0000 (16:35 +0800)]
cmd/link: bump loong64 function alignment to 16 bytes
The loong64 PCALIGN directive works with PCs relative to beginning of
functions. So if the function alignment is less than that requested by
PCALIGN, the following code may in fact not be aligned as such, leading
to unexpected performance.
The current function alignment on loong64 is 8 bytes, which seems to
stem from mips64 or riscv64. In order to make performance more
predictable on loong64, it is raised to 16 bytes to ensure that at
least `PCALIGN $16` works.
As alignment of loops written in Go is yet to be tackled, and the
codegen is not otherwise touched, benchmark numbers for this change are
not going to be meaningful, and not included.
Change-Id: I2120ef3746ce067e274920c82091810073bfa3be
Reviewed-on: https://go-review.googlesource.com/c/go/+/481936
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org>
WANG Xuerui [Tue, 28 Mar 2023 11:58:17 +0000 (19:58 +0800)]
internal/bytealg, runtime: align some loong64 asm loops to 16-byte boundaries
The LA464 micro-architecture is very sensitive to alignment of loops,
so the final performance of linked binaries can vary wildly due to
uncontrolled alignment of certain performance-critical loops. Now that
PCALIGN is available on loong64, let's make use of it and manually align
some assembly loops. The functions are identified based on perf records
of some easily regressed go1 benchmark cases (e.g. FmtFprintfPrefixedInt,
RegexpMatchEasy0_1K and Revcomp are particularly sensitive; even those
optimizations purely reducing dynamic instruction counts can regress
those cases by 6~12%, making the numbers almost useless).
Benchmark results on Loongson 3A5000 (which is an LA464 implementation):
Change-Id: I9ac28d936e03d21c46bb19fa100018f61ace6b42
Reviewed-on: https://go-review.googlesource.com/c/go/+/479816
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: WANG Xuerui <git@xen0n.name> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@golang.org>
WANG Xuerui [Tue, 28 Mar 2023 11:30:04 +0000 (19:30 +0800)]
cmd/link, cmd/internal/obj/loong64: support the PCALIGN directive
Allow writing `PCALIGN $imm` where imm is a power-of-2 between 8 and
2048 (inclusive), for ensuring that the following instruction is
placed at an imm-byte boundary relative to the beginning of the
function. If the PC is not sufficiently aligned, NOOPs will be
inserted to make it so, otherwise the directive will do nothing.
This could be useful for both asm performance hand-tuning, and future
scenarios where a certain bigger alignment might be required.
Change-Id: Iad6244669a3d5adea88eceb0dc7be1af4f0d4fc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/479815 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: abner chenc <chenguoqi@loongson.cn>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Daniel Martí [Fri, 7 Apr 2023 17:21:00 +0000 (18:21 +0100)]
encoding/xml: use reflect.Value.Grow
Like https://go.dev/cl/481376 did for encoding/gob,
but now for encoding/xml, which had very similar code.
One minor difference is that encoding/xml now needs a SetLen before the
call to Index, as otherwise we index just past the length.
Still, calling Grow and SetLen is easier to understand,
and avoids needing to make a new zero value.
goos: linux
goarch: amd64
pkg: encoding/xml
cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
│ old │ new │
│ sec/op │ sec/op vs base │
Unmarshal-8 6.904µ ± 1% 6.980µ ± 1% +1.10% (p=0.009 n=6)
│ old │ new │
│ B/op │ B/op vs base │
Unmarshal-8 7.656Ki ± 0% 7.586Ki ± 0% -0.92% (p=0.002 n=6)
│ old │ new │
│ allocs/op │ allocs/op vs base │
Unmarshal-8 188.0 ± 0% 185.0 ± 0% -1.60% (p=0.002 n=6)
Change-Id: Id83feb467a9c59c80c7936aa892780aae7e8b809
Reviewed-on: https://go-review.googlesource.com/c/go/+/483135
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Ian Lance Taylor <iant@google.com>
David Chase [Thu, 6 Apr 2023 18:42:53 +0000 (14:42 -0400)]
cmd/compile: remove broken LEA "optimization"
CL 440035 added rewrite rules to simplify "costly" LEA
instructions, but the types in the rewrites were wrong and
the code would go bad if the wrong-typed register was spilled.
CL 482536 attempted to fix this by correcting the type in the
rewrite, but that "fix" broke something on windows-amd64-race.
Instead / for-now, remove the offending rewrite rules.
Updates #21735.
Fixes #59432.
Change-Id: I0497c42db414f2055e1378e0a53e2bceee9cd5d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/482820
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Damien Neil [Fri, 7 Apr 2023 17:04:42 +0000 (10:04 -0700)]
net/http: wait forever for write results in tests
After performing a round trip on a connection, the connection is
usually returned to the idle connection pool. If the write of the
request did not complete successfully, the connection is not
returned.
It is possible for the response to be read before the write
goroutine has finished signalling that its write has completed.
To allow for this, the check to see if the write completed successfully
waits for 50ms for the write goroutine to report the result of the
write.
See comments in persistConn.wroteRequest for more details.
On a slow builder, it is possible for the write goroutine to take
longer than 50ms to report the status of its write, leading to test
flakiness when successive requests unexpectedly use different connections.
Set the timeout for waiting for the writer to an effectively
infinite duration in tests.
net/http: fix a race in TestResponseControllerSetPastReadDeadline
If the Write goroutine is delayed for long enough after its first
Write, the handler may have closed both the readc and donec channels
by the time it selects over them, and the donec case may be randomly
chosen. Handle that case by explicitly checking readc as well.
Than McIntosh [Tue, 4 Apr 2023 22:31:46 +0000 (18:31 -0400)]
cmd/compile: allow more inlining of functions that construct closures
[This is a roll-forward of CL 479095, which was reverted due to a bad
interaction between inlining and escape analysis since fixed in CL 482355.]
Currently, when the inliner is determining if a function is
inlineable, it descends into the bodies of closures constructed by
that function. This has several unfortunate consequences:
- If the closure contains a disallowed operation (e.g., a defer), then
the outer function can't be inlined. It makes sense that the
*closure* can't be inlined in this case, but it doesn't make sense
to punish the function that constructs the closure.
- The hairiness of the closure counts against the inlining budget of
the outer function. Since we currently copy the closure body when
inlining the outer function, this makes sense from the perspective
of export data size and binary size, but ultimately doesn't make
much sense from the perspective of what should be inlineable.
- Since the inliner walks into every closure created by an outer
function in addition to starting a walk at every closure, this adds
an n^2 factor to inlinability analysis.
This CL simply drops this behavior.
In std, this makes 57 more functions inlinable, and disallows inlining
for 10 (due to the basic instability of our bottom-up inlining
approach), for an net increase of 47 inlinable functions (+0.6%).
This will help significantly with the performance of the functions to
be added for #56102, which have a somewhat complicated nesting of
closures with a performance-critical fast path.
The downside of this seems to be a potential increase in export data
and text size, but the practical impact of this seems to be
negligible:
Than McIntosh [Tue, 4 Apr 2023 22:56:21 +0000 (18:56 -0400)]
cmd/compile: deadcode unreferenced hidden closures during inlining
When a closure is inlined, it may contain other hidden closures, which
the inliner will duplicate, rendering the original nested closures as
unreachable. Because they are unreachable, they don't get processed in
escape analysis, meaning that go/defer statements don't get rewritten,
which can then in turn trigger errors in walk. This patch looks for
nested hidden closures and marks them as dead, so that they can be
skipped later on in the compilation flow. NB: if during escape
analysis we rediscover a hidden closure (due to an explicit reference)
that was previously marked dead, revive it at that point.
Fixes #59404.
Change-Id: I76db1e9cf1ee38bd1147aeae823f916dbbbf081b
Reviewed-on: https://go-review.googlesource.com/c/go/+/482355
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Ian Lance Taylor [Thu, 6 Apr 2023 23:38:47 +0000 (16:38 -0700)]
runtime: permit core dumps in darwin-amd64
Previously we did not permit them as Go programs generated enormous
core dumps on macOS. However, according to an investigation in #59446,
they are OK now.
For #59446
Change-Id: I1d7a3f500a6bc525aa6de8dfa8a1d8dbb15feadc
Reviewed-on: https://go-review.googlesource.com/c/go/+/483015
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
cmd/link/internal/ld: don't set IMAGE_FILE_DEBUG_STRIPPED on PE binaries
The IMAGE_FILE_DEBUG_STRIPPED characteristic is used to inform that
the debugging information have been removed from the PE files and moved
into a DBG file, but the Go linker doesn't generate DBG files.
Having this characteristic can confuse debugging tools, so better
don't set it.
While here, remove also IMAGE_FILE_LINE_NUMS_STRIPPED, which is
deprecated and should be zero [1].
cmd/compile: get more bounds info from logic operators in prove pass
Currently, the prove pass can get knowledge from some specific logic
operators only before the CFG is explored, which means that the bounds
information of the branch will be ignored.
This CL updates the facts table by the logic operators in every
branch. Combined with the branch information, this will be helpful for
BCE in some circumstances.
Fixes #57243
Change-Id: I0bd164f1b47804ccfc37879abe9788740b016fd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/419555 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
Damien Neil [Thu, 6 Apr 2023 19:09:04 +0000 (12:09 -0700)]
net/http: improve failure mode for TestResponseControllerSetPastReadDeadline
A test flake in #59447 seems to indicate that this test got stuck
waiting for the test handler to close the readc channel.
If the handler returns early due to an unexpected error, it might
fail to close this channel. Add a second channel to act as a
signal that the handler has given up and the test should stop.
This won't fix whatever happened in the flake, but might help
us debug it if it happens again.
For #59447
Change-Id: I05d84c6176aa938887d93126a6f3bb4dc941c90d
Reviewed-on: https://go-review.googlesource.com/c/go/+/482935 Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Johan Brandhorst-Satzkorn [Sat, 25 Mar 2023 15:48:03 +0000 (08:48 -0700)]
misc/wasm: add wasip1/wasm exec script
This script uses Wazero, the open source, zero dependencies
pure Go Wasm and WASI runtime. This is the runtime that allows
the greatest number of standard library tests to pass.
For #58141
Co-authored-by: Richard Musiol <neelance@gmail.com> Co-authored-by: Achille Roussel <achille.roussel@gmail.com> Co-authored-by: Julien Fabre <ju.pryz@gmail.com> Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: I789465ae4daf2b380f3c05a9365b8d449c6af56c
Reviewed-on: https://go-review.googlesource.com/c/go/+/479620 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
cmd/go/internal/modfetch/codehost: set core.longpaths in Git repos on Windows
This setting appears to be needed to avoid “Filename too long” errors
when downloading modules from repos with long branch names,
particularly if the path to the module cache is already fairly long
(as may be the case in CI systems and in tests of cmd/go itself).
Damien Neil [Thu, 6 Apr 2023 18:01:05 +0000 (11:01 -0700)]
net/http: add tests covering non-GET methods for file serving
ServeFile and FileServer will respond to methods such as DELETE by
serving the file contents. This is surprising, but we don't want to
change it without some consideration.
Heschi Kreinick [Wed, 29 Mar 2023 17:13:19 +0000 (13:13 -0400)]
cmd/go: fix tests for new builder environment
Fix two long tests that fail in the builders we're trying out:
- TestQueryImport was failing with:
open /nonexist-gopath/pkg/sumdb/sum.golang.org/latest: no such file or directory
which eventually turns out to be because it couldn't create
/nonexist-gopath because it wasn't running as root. The test already
uses a temporary GOPATH, but missed overriding a configuration
variable set at init time.
- test_flags fails if the working directory has /x/ in it, which it now
happens to.
David Chase [Fri, 31 Mar 2023 21:26:04 +0000 (17:26 -0400)]
cmd/compile: modify debug-hash to support match exclusion
The goal here is to enable a search that will locate all the instances
of a failure, not just the first one. This helps with searches for
loopvar-change breakage, FP differences from fused-multiply-add, and
allows certain semantics queries that can be implemented as compiler
changes (for example, where does integer overflow routinely occur?)
Change-Id: Ic28f1695d47e421c2089d1f3f7c4b40c56db970f
Reviewed-on: https://go-review.googlesource.com/c/go/+/481195
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
David Chase [Wed, 5 Apr 2023 19:49:07 +0000 (15:49 -0400)]
cmd/compile: use correct type in amd64 late-lower rules
The wrong type causes the wrong width spill, which corrupts
the value. I tried to write a test for this and did not
succeed, but was able (using gossahash and ssa.html) to
isolate to exact change and spill.
Fixes #59432.
Change-Id: I85ad82c9f8fed7674c69d6a2b0a62e111f690454
Reviewed-on: https://go-review.googlesource.com/c/go/+/482536 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Mon, 28 Nov 2022 19:53:20 +0000 (19:53 +0000)]
runtime: flush each idle P's page cache at the end of each GC cycle
Currently pages may linger in an idle P's page cache, hiding the memory
from the scavenger precisely when it's useful to return memory to the OS
and reduce the application's footprint.
Change-Id: I49fbcd806b6c66991d1ca87949f76a9f06708e70
Reviewed-on: https://go-review.googlesource.com/c/go/+/453622
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Fri, 11 Nov 2022 22:16:20 +0000 (22:16 +0000)]
runtime: initialize the memory limit in mallocinit
Currently the memory limit is left uninitialized before gcinit, and
allocations may happen. The result is that the span allocation path
might try to scavenge memory unnecessarily. Prevent this by setting the
memory limit up early to its default value.
Change-Id: I886d9a8fa645861e4f88e0d54af793418426f520
Reviewed-on: https://go-review.googlesource.com/c/go/+/450736
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Joel Sing [Mon, 27 Feb 2023 07:39:09 +0000 (18:39 +1100)]
cmd/internal/obj/arm64: use more appropriate types for olsr9s/olsr12u
This allows for a large number of casts to be removed at call sites.
While here, use consistent register naming.
Change-Id: I78a2a928b78c9f09f91fb6ed6ad440aa4e63923d
Reviewed-on: https://go-review.googlesource.com/c/go/+/471517
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Joel Sing [Sun, 26 Feb 2023 19:03:31 +0000 (06:03 +1100)]
cmd/internal/obj/arm64: use more appropriate types for opldpstp
This allows for a large number of casts to be removed at call sites.
While here, use consistent register naming.
Change-Id: I68cee985f6500ed9523f7fb3efbc11bb849681dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/471516 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Joel Sing [Sun, 26 Feb 2023 18:52:04 +0000 (05:52 +1100)]
cmd/internal/obj/arm64: use more appropriate types for opbfm and opextr
This allows for a large number of casts to be removed at call sites.
Change-Id: I44a162040cc5b4de02e106d3a6de10f8e0870cb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/471515
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Johan Brandhorst-Satzkorn [Sat, 25 Mar 2023 06:07:58 +0000 (23:07 -0700)]
all: add wasip1 asm and link logic
Add wasip1 asm and symbols to cmd/internal/obj, cmd/link and
runtime.
For #58141
Co-authored-by: Richard Musiol <neelance@gmail.com> Co-authored-by: Achille Roussel <achille.roussel@gmail.com> Co-authored-by: Julien Fabre <ju.pryz@gmail.com> Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: Ie088d9b65ea13e231694af6341465f95be33093f
Reviewed-on: https://go-review.googlesource.com/c/go/+/479617 Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Nick Ripley [Mon, 3 Apr 2023 13:54:53 +0000 (09:54 -0400)]
runtime: save frame pointer to the stack in signal handlers for arm64
When taking over the goroutine stack in the panic or preemption signal
handlers on arm64, the frame pointer should be saved on the stack (like
the link register) so that frame-pointer unwinding from a panic stack
works properly. Otherwise, tests like TestStackWrapperStackPanic will
fail with the frame pointer check in adjustframe (enabled with
debugCheckBP) when checking the sigpanic frame.
Updates #39524, #58432
Change-Id: I8b89e6fc4877af29b1b81e55e591e6398159855c
Reviewed-on: https://go-review.googlesource.com/c/go/+/481635 Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Nick Ripley <nick.ripley@datadoghq.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Sameer Ajmani [Wed, 29 Mar 2023 21:47:50 +0000 (17:47 -0400)]
context: clean up test files
Tests in package context cannot depend directly on package testing due to an import cycle.
We resolved this by having test functions in package context_test (x_test.go) forward to
test functions in package context (context_test.go). This is fragile, since it's easy
to add a test to context_test.go and forget to add the forwarding function, and tests
written in this way cannot easily use testing package features like t.Run for subtests.
It turns out that only four test functions actually use unexported members of package
context. This CL moves all except those four to x_test.go and makes them regular tests.
It also updates TestCause to use t.Run and t.Parallel to parallelize its test cases.
It also adds documentation indicating when tests should be added to each file.
Change-Id: Ic60bae32a7a44e07831b5388c9af219d53ba9af3
Reviewed-on: https://go-review.googlesource.com/c/go/+/480375
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Sameer Ajmani <sameer@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Michael Anthony Knyszek [Tue, 4 Apr 2023 22:07:09 +0000 (22:07 +0000)]
html/template,mime/multipart: document new GODEBUG settings
This change documents the new GODEBUG settings introduced for
html/template and mime/multipart, released with Go 1.19.8 and Go 1.20.3
as part of a security fix.
Updates #59153.
Updates #59234.
Change-Id: I25f4d8245da3301dccccfb44da8ff1a5985392a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/482238
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Imre Rad [Tue, 7 Mar 2023 16:31:13 +0000 (16:31 +0000)]
archive/zip: return ErrInsecurePath for unsafe paths by OpenReader
zip.NewReader was recently improved to return ErrInsecurePath when
insecure entries are encountered.
This change adopts the same logic for the OpenReader interface as well.
Fixes #58641
Change-Id: I0d8be94d073cc14cf93a914dc250f85b19cec4ab
GitHub-Last-Rev: 68391dc51562aebc893ec70fdfbdfb181955983a
GitHub-Pull-Request: golang/go#58658
Reviewed-on: https://go-review.googlesource.com/c/go/+/470735
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Adjust the PosixFallocate wrappers on freebsd to account for that.
Also, CL 479715 used the same syscall wrapper for 386 and arm. However,
on arm the syscall argument order is different. The wrapper was
generated using mksyscall.go from the golang.org/x/sys/unix package,
adjusting the r1 check correspondingly.
Ian Lance Taylor [Tue, 4 Apr 2023 21:43:46 +0000 (14:43 -0700)]
cmd/dist: skip static linking tests for linux boringcrypto
Otherwise we get warnings from the C linker.
Fixes #59422
Change-Id: I61843dbe5245da0185b0f23dc4b774767fffed40
Reviewed-on: https://go-review.googlesource.com/c/go/+/482315
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
net/http: drop client address comparison in TestTransportRemovesDeadIdleConnections
Since the first client connection is explicitly closed before making
the second request, we cannot in general assume that the second
request uses a different port (it is equally valid to open the new
connection on the same port as the old one that was closed).
Lasse Folger [Mon, 3 Apr 2023 11:52:56 +0000 (13:52 +0200)]
go/internal/gcimporter: use saferio.ReadData instead of io.ReadAll when possible
saferio.ReadData avoids unnecessary allocations because the buffer can be
preallocated with the right size (up to a limit) instead of having to resize
and copy it step by step.
Change-Id: Id70f6908971d4f126c601a9571ac3c67ea0accdc
Reviewed-on: https://go-review.googlesource.com/c/go/+/481616 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Constantin Konstantinidis [Mon, 14 May 2018 11:50:56 +0000 (13:50 +0200)]
encoding/binary: add word size to the error message of the failed constraint
Test added.
Fixes #22860
Change-Id: I08304834a2b7b10b4ac729bf36761692eb4731da
Reviewed-on: https://go-review.googlesource.com/c/go/+/113075 Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
os/exec: skip remount in TestFindExecutableVsNoexec on EROFS
To allow using testenv.SyscallIsNotSupported, rewrite the test to use
the exported API only. Given that path is an absolute path,
exec.LookPath is equivalent to exec.findExecutable on linux.
Felix Geisendörfer [Sun, 2 Apr 2023 00:19:37 +0000 (20:19 -0400)]
runtime/trace: Fix TestTraceSymbolize on solaris and illumos
Fix a regression caused by CL 463835. Unlike most platforms, solaris and
illumos don't use a libc_read_trampoline, so we need to skip one frame
less when using frame pointer unwinding in traceGoSysCall.
The solution is a bit hacky, so it might make sense to implement
gp.syscallbp if this causes more test failures in the future.
Fixes #59350
Change-Id: I0f0b08f36efe8a492eb4a535e752c03636857057
Reviewed-on: https://go-review.googlesource.com/c/go/+/481336
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Ian Lance Taylor [Tue, 4 Apr 2023 01:41:54 +0000 (18:41 -0700)]
math/rand: clarify Seed deprecation note
Fixes #59331
Change-Id: I62156be2f2758c59349c3b02db6cf9140429c9e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/481915
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com> Reviewed-by: Russ Cox <rsc@golang.org>
Roland Shoemaker [Mon, 20 Mar 2023 18:01:13 +0000 (11:01 -0700)]
html/template: disallow actions in JS template literals
ECMAScript 6 introduced template literals[0][1] which are delimited with
backticks. These need to be escaped in a similar fashion to the
delimiters for other string literals. Additionally template literals can
contain special syntax for string interpolation.
There is no clear way to allow safe insertion of actions within JS
template literals, as handling (JS) string interpolation inside of these
literals is rather complex. As such we've chosen to simply disallow
template actions within these template literals.
A new error code is added for this parsing failure case, errJsTmplLit,
but it is unexported as it is not backwards compatible with other minor
release versions to introduce an API change in a minor release. We will
export this code in the next major release.
The previous behavior (with the cavet that backticks are now escaped
properly) can be re-enabled with GODEBUG=jstmpllitinterp=1.
This change subsumes CL471455.
Thanks to Sohom Datta, Manipal Institute of Technology, for reporting
this issue.
Damien Neil [Mon, 20 Mar 2023 17:43:19 +0000 (10:43 -0700)]
mime/multipart: limit parsed mime message sizes
The parsed forms of MIME headers and multipart forms can consume
substantially more memory than the size of the input data.
A malicious input containing a very large number of headers or
form parts can cause excessively large memory allocations.
Set limits on the size of MIME data:
Reader.NextPart and Reader.NextRawPart limit the the number
of headers in a part to 10000.
Reader.ReadForm limits the total number of headers in all
FileHeaders to 10000.
Both of these limits may be set with with
GODEBUG=multipartmaxheaders=<values>.
Reader.ReadForm limits the number of parts in a form to 1000.
This limit may be set with GODEBUG=multipartmaxparts=<value>.
Thanks for Jakob Ackermann (@das7pad) for reporting this issue.
For CVE-2023-24536
For #59153
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802455
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Julie Qiu <julieqiu@google.com>
Change-Id: I08dd297bd75724aade4b0bd6a7d19aeca5bbf99f
Reviewed-on: https://go-review.googlesource.com/c/go/+/482077
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Damien Neil [Thu, 16 Mar 2023 21:18:04 +0000 (14:18 -0700)]
mime/multipart: avoid excessive copy buffer allocations in ReadForm
When copying form data to disk with io.Copy,
allocate only one copy buffer and reuse it rather than
creating two buffers per file (one from io.multiReader.WriteTo,
and a second one from os.File.ReadFrom).
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
For CVE-2023-24536
For #59153
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802453
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Julie Qiu <julieqiu@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
Change-Id: I732bd2e1e7467918cac8ab9d65d089272ba4656f
Reviewed-on: https://go-review.googlesource.com/c/go/+/482075
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Damien Neil [Fri, 10 Mar 2023 22:21:05 +0000 (14:21 -0800)]
net/textproto: avoid overpredicting the number of MIME header keys
A parsed MIME header is a map[string][]string. In the common case,
a header contains many one-element []string slices. To avoid
allocating a separate slice for each key, ReadMIMEHeader looks
ahead in the input to predict the number of keys that will be
parsed, and allocates a single []string of that length.
The individual slices are then allocated out of the larger one.
The prediction of the number of header keys was done by counting
newlines in the input buffer, which does not take into account
header continuation lines (where a header key/value spans multiple
lines) or the end of the header block and the start of the body.
This could lead to a substantial amount of overallocation, for
example when the body consists of nothing but a large block of
newlines.
Fix header key count prediction to take into account the end of
the headers (indicated by a blank line) and continuation lines
(starting with whitespace).
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
For #58975
Fixes CVE-2023-24534
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802452
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Julie Qiu <julieqiu@google.com>
Change-Id: Iacc1c2b5ea6509529845a972414199f988ede1e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/481994 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Mateusz Poliwczak [Thu, 30 Mar 2023 07:22:35 +0000 (07:22 +0000)]
net: treat EAI_NODATA as errNoSuchHost
man getaddrinfo:
EAI_NODATA
The specified network host exists, but does not have any
network addresses defined.
In the go resolver we treat this kind of error as nosuchhost.
Change-Id: I69fab6f8da8e3a86907e65104bca9f055968633a
GitHub-Last-Rev: b4891e2addaeccecb242d0485daa168bb55cf54e
GitHub-Pull-Request: golang/go#57507
Reviewed-on: https://go-review.googlesource.com/c/go/+/459955 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Mateusz Poliwczak <mpoliwczak34@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
cui fliter [Thu, 30 Mar 2023 16:00:19 +0000 (00:00 +0800)]
all: fix misuses of "a" vs "an"
Fixes the misuse of "a" vs "an", according to English grammatical
expectations and using https://www.a-or-an.com/
Change-Id: I53ac724070e3ff3d33c304483fe72c023c7cda47
Reviewed-on: https://go-review.googlesource.com/c/go/+/480536
Run-TryBot: shuang cui <imcusg@gmail.com> Reviewed-by: Ian Lance Taylor <iant@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: Michael Knyszek <mknyszek@google.com>
Daniel Martí [Sat, 1 Apr 2023 17:55:06 +0000 (18:55 +0100)]
encoding/gob: use reflect.Value.Grow
Growing by one is a simpler, and often cheaper,
operation compared to appending one (newly created) zero value.
The method was introduced in Go 1.20.
growSlice in dec_helpers.go is left alone,
as it grows using the builtin append instead of reflect.Append.
No noticeable performance difference on any of our benchmarks,
as this code only runs for slices large enough to not fit in
saferio.SliceCap, and none of our benchmarks use data that large.
goos: linux
goarch: amd64
pkg: encoding/gob
cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
│ old │ new │
│ sec/op │ sec/op vs base │
DecodeBytesSlice-8 11.37µ ± 1% 11.46µ ± 4% ~ (p=0.315 n=10)
DecodeInterfaceSlice-8 96.49µ ± 1% 95.75µ ± 1% ~ (p=0.436 n=10)
geomean 33.12µ 33.12µ +0.01%
│ old │ new │
│ B/op │ B/op vs base │
DecodeBytesSlice-8 22.39Ki ± 0% 22.39Ki ± 0% ~ (p=1.000 n=10)
DecodeInterfaceSlice-8 80.25Ki ± 0% 80.25Ki ± 0% ~ (p=0.650 n=10)
geomean 42.39Ki 42.39Ki +0.00%
│ old │ new │
│ allocs/op │ allocs/op vs base │
DecodeBytesSlice-8 169.0 ± 0% 169.0 ± 0% ~ (p=1.000 n=10) ¹
DecodeInterfaceSlice-8 3.178k ± 0% 3.178k ± 0% ~ (p=1.000 n=10) ¹
geomean 732.9 732.9 +0.00%
Change-Id: I468aebf4ae6f197a1fd35f6fee809ca591c1788f
Reviewed-on: https://go-review.googlesource.com/c/go/+/481376 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Johan Brandhorst-Satzkorn [Thu, 30 Mar 2023 19:14:55 +0000 (12:14 -0700)]
cmd/go: add wasip1 to modindex syslist
This was intended to be merged together with changes in CL 479616.
For #58141
Change-Id: I76c38d3d4dfee93a1a170e28af28f0c9d6382830
Reviewed-on: https://go-review.googlesource.com/c/go/+/480656
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Paul E. Murphy [Mon, 13 Mar 2023 20:58:15 +0000 (15:58 -0500)]
runtime: cleanup PPC64/linux runtime.sigtramp
Add new helper macros to further simplify the transition from
the host's ABI to Go. Fortunately the same one should work for
all PPC64 targets.
Update the other site which uses these wrappers to further
consolidate. Also, update the call to runtime.sigtrampgo to
call the ABIInternal version directly.
Also, update the SAVE/RESTORE_VR macros to accept R0.
Change-Id: I0046176029e1e1b25838688e4b7bf57805b01bd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/476297 Reviewed-by: Archana Ravindar <aravind5@in.ibm.com>
Run-TryBot: Paul Murphy <murp@ibm.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Than McIntosh [Mon, 3 Apr 2023 18:09:03 +0000 (14:09 -0400)]
cmd/compile/internal/test: skip testpoint due to revert of CL 479095
Skip one of the testpoints that verifies inlining, since it
no longer passes as a result of reverting CL 479095. Once we
roll forward with a new version of CL 479095 we can re-enable
this testpoint.
doujiang24 [Fri, 31 Mar 2023 19:45:39 +0000 (15:45 -0400)]
runtime/cgo: store M for C-created thread in pthread key
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.
Russ Cox [Tue, 22 Nov 2022 15:24:11 +0000 (10:24 -0500)]
reflect: deprecate SliceHeader and StringHeader
These are replaced by unsafe.String etc, which were added in Go 1.20.
Per https://go.dev/wiki/Deprecated, we must wait until Go 1.21
to mark them deprecated.
Keith Randall [Thu, 30 Mar 2023 16:52:39 +0000 (09:52 -0700)]
crypto/subtle: don't cast to *uintptr when word size is 0
Casting to a *uintptr is not ok if there isn't at least 8 bytes of
data backing that pointer (on 64-bit archs).
So although we end up making a slice of 0 length with that pointer,
the cast itself doesn't know that.
Instead, bail early if the result is going to be 0 length.
Fixes #59334
Change-Id: Id3c0e09d341d838835c0382cccfb0f71dc3dc7e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/480575
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
qmuntal [Thu, 30 Mar 2023 08:08:15 +0000 (10:08 +0200)]
go/internal/gcimport: unskip Windows tests
There are many tests in internal/gcimporter that are skipped on
Windows because they build a test program that needs the -D flag
when invoking the Go compiler.
This flag is already passed since CL 442303, so there is no need to
skip those tests.