Antonio Pitasi [Wed, 21 May 2025 09:58:13 +0000 (11:58 +0200)]
log/slog: make TextHandler discard empty Source
Fixes #73808
Change-Id: Ica4b7a63eebbf0fff41d68f4de928f9da90c8ada
Reviewed-on: https://go-review.googlesource.com/c/go/+/674875 Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
David Chase [Fri, 23 May 2025 18:49:13 +0000 (14:49 -0400)]
doc/next: add crudely processed todos
This is the output of relnote -goroot=... todo,
with each todo in a comment, followed by summary
text from the issue and perhaps the CL, lightly
processed into markdown.
For #71661.
Change-Id: I855c4c4ee02491b5b6113822baf69dbafb4e54ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/675877 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Carlos Amedee [Thu, 22 May 2025 17:33:28 +0000 (13:33 -0400)]
runtime/trace: fix flaky test for SetMinAge
This change fixes the flaky test which expects setting SetMinAge to a
small ammount. It expects two sync events but should realistically
expect up to 3.
Change-Id: Ibd02fe55ebca99eb880025eb968fcebae9cb09c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/675597 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Keith Randall [Thu, 22 May 2025 22:17:37 +0000 (15:17 -0700)]
doc: fix TBD mark
Change-Id: I2133e3c62b4de0cec08eeb120d593c644643a62c
Reviewed-on: https://go-review.googlesource.com/c/go/+/675755 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Keith Randall [Wed, 16 Apr 2025 20:50:44 +0000 (13:50 -0700)]
doc: mention stack allocation of variable-sized make calls
Also mention the bisect tool and flag used to track down
incorrect uses.
Change-Id: Id36a236e1bb2733b8611b22a5b16916e7d9f5522
Reviewed-on: https://go-review.googlesource.com/c/go/+/666075 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Thu, 22 May 2025 00:29:14 +0000 (00:29 +0000)]
runtime: don't spin looking for a tiny alloc address with asan or race
CL 674655 modified the checkfinalizers test to spin looking for an
appropriate address to trip the detector, but this doesn't work with
ASAN or in race mode, which both disable the tiny allocator.
Fixes #73834.
Change-Id: I27416da1f29cd953271698551e9ce9724484c683
Reviewed-on: https://go-review.googlesource.com/c/go/+/675395
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Cuong Manh Le [Fri, 10 Jan 2025 05:49:59 +0000 (12:49 +0700)]
cmd/compile: do not shapify when reading reshaping expr
Fixes #71184
Change-Id: I22e7ae5203311e86a90502bfe155b0597007887d
Reviewed-on: https://go-review.googlesource.com/c/go/+/641955
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
cmd/compile: fix ICE with recursive alias type parameter
CL 585399 fixed an initialization loop during IR contruction that
involving alias type, by avoiding publishing alias declarations until
the RHS type expression has been constructed.
There's an assertion to ensure that the alias's type must be the same
during the initialization. However, that assertion is too strict, since
we may construct different instances of the same type, if the type is an
instantination of generic type.
To fix this, we could use types.IdenticalStrict to ensure that these
types matching exactly.
Updates #66873.
Updates #73309.
Change-Id: I2559bed37e21615854333fb1057d7349406e6a1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/668175 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
Michael Matloob [Thu, 22 May 2025 16:40:51 +0000 (12:40 -0400)]
cmd/doc: properly set GOPROXY to avoid deprecation checks
This change fixes a bug that was introduced in CL 675155. Instead of
doing the two step download and run with GOPROXY=off, do the run with
GOPROXY=<download cache>:$GOPROXY, so that we use the previously
downloaded version of pkgsite as the latest.
Fixes #73833
Change-Id: I8803426498ab026602805d6448a130eb11458c99
Reviewed-on: https://go-review.googlesource.com/c/go/+/675576 Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cuong Manh Le [Thu, 22 May 2025 11:06:27 +0000 (18:06 +0700)]
cmd/compile: fix ICE when transforming loopvar
When transforming for loop variables, the compiler does roughly
following steps:
(1) prebody = {z := z' for z in leaked}
...
(4) init' = (init : s/z/z' for z in leaked)
However, the definition of z is not updated to `z := z'` statement,
causing ReassignOracle incorrectly use the new init statement with z'
instead of z, trigger the ICE.
Fixing this by updating the correct/new definition statement for z
during the prebody initialization.
Fixes #73823
Change-Id: Ice2a6741be7478506c58f4000f591d5582029136
Reviewed-on: https://go-review.googlesource.com/c/go/+/675475
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
Jake Bailey [Tue, 13 May 2025 03:39:54 +0000 (20:39 -0700)]
cmd/compile/internal/ssa: eliminate string copies for calls to unique.Make
unique.Make always copies strings passed into it, so it's safe to not
copy byte slices converted to strings either. Handle this just like map
accesses with string(b) as keys.
This CL only handles unique.Make(string(b)), not nested cases like
unique.Make([2]string{string(b1), string(b2)}); this could be done in a
followup CL but the map lookup code in walk is sufficiently different
than the call handling code that I didn't attempt it. (SSA is much
easier).
Fixes #71926
Change-Id: Ic2f82f2f91963d563b4ddb1282bd49fc40da8b85
Reviewed-on: https://go-review.googlesource.com/c/go/+/672135 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cherry Mui [Wed, 21 May 2025 18:33:13 +0000 (14:33 -0400)]
cmd/compile, unique: model data flow of non-string pointers
Currently, hash/maphash.Comparable escapes its parameter if it
contains non-string pointers, but does not escape strings or types
that contain strings but no other pointers. This is achieved by a
compiler intrinsic.
unique.Make does something similar: it stores its parameter to a
central map, with strings cloned. So from the escape analysis's
perspective, the non-string pointers are passed through, whereas
string pointers are not. We currently cannot model this type of
type-dependent data flow directly in Go. So we do this with a
compiler intrinsic. In fact, we can unify this and the intrinsic
above.
Tests are from Jake Bailey's CL 671955 (thanks!).
Fixes #73680.
Change-Id: Ia6a78e09dee39f8d9198a16758e4b5322ee2c56a
Reviewed-on: https://go-review.googlesource.com/c/go/+/675156
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Jake Bailey <jacob.b.bailey@gmail.com>
thepudds [Wed, 21 May 2025 20:50:17 +0000 (16:50 -0400)]
cmd/compile/internal/escape: make escape analysis -m=2 logs more accessible
This was the first CL in a series of CLs aimed at reducing
how often interface arguments escape for the print functions in fmt.
This CL makes some small improvements to the escape analysis logging.
Here is a sample snippet of the current -m=2 logs:
./print.go:587:7: parameter p leaks to {heap} with derefs=0:
./print.go:587:7: flow: p = p:
./print.go:587:7: from (*pp).printArg(p, err, 'v') (call parameter) at ./print.go:613:13
./print.go:587:7: flow: p = p:
./print.go:587:7: from (*pp).handleMethods(p, verb) (call parameter) at ./print.go:749:22
[..]
If we attempt to tease apart some reasons why the -m=2 logs can be
challenging to understand for the uninitiated:
- The "flow" lines are very useful, but contain more-or-less abstracted
pseudocode. The "from" lines most often use actual code. When first
looking at the logs, that distinction might not be apparent, which can
result in looking back to the original code to hunt for pseudocode
that doesn't exist there. (The log example shows 'p = p', but there is
no 'p = p' in the original source).
- Escape analysis can be most interesting with inlining, but that can
result in seeing overlapping short variable names (e.g., p, b, v...).
- The directionality of the "flow" lines might not be obvious,
including whether they build top-to-bottom or bottom-to-top.
- The use of '{' and '}' in the -m=2 logs somewhat intersects with Go
literals (e.g., if the log says "{temp}", an initial thought might
be that represents some temp inside of some Go literal).
- And of course, escape analysis itself is subtle.
This CL:
- Adds the function name to the first -m=2 line to provide more context
and reduce how often the reader needs to lookup line numbers.
- Uses the Unicode left arrow '←' rather than '=' on the flow lines
to make it clearer that these lines are abstracted away from the
original Go code and to help the directionality jump out.
In the future, we can consider changing "{heap}", "{temp}",
"{storage for foo}" to something else, but we leave them as is for now.
Two examples with the modifications:
./f1.go:3:9: parameter inptr leaks to outptr for func1 with derefs=0:
./f1.go:3:9: flow: localptr ← inptr:
./f1.go:3:9: from localptr := inptr (assign) at ./f1.go:4:11
./f1.go:3:9: flow: outptr ← localptr:
./f1.go:3:9: from return localptr (return) at ./f1.go:5:2
./b.go:14:20: []byte{...} escapes to heap in byteOrderExample:
./b.go:14:20: flow: b ← &{storage for []byte{...}}:
./b.go:14:20: from []byte{...} (spill) at ./byteorder.go:14:20
./b.go:14:20: from b := []byte{...} (assign) at ./byteorder.go:14:11
./b.go:14:20: flow: <heap> ← b:
./b.go:14:20: from byteOrder.Uint32(b) (call parameter) at ./byteorder.go:15:32
These changes only affect the -m=2 output and leave the -m=1 output
as is.
Updates #8618
Updates #62653
Change-Id: Ic082a371c3d3fa0d8fd8bfbe4d64ec3e1e53c173
Reviewed-on: https://go-review.googlesource.com/c/go/+/524937 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Carlos Amedee [Wed, 14 May 2025 20:13:24 +0000 (16:13 -0400)]
runtime/trace: add the flight recorder
This change adds the flight recorder to the trace package.
Flight recording is a technique in which trace data is kept
in a circular buffer and can be flushed upon request. The
implementation will be added in follow-up CLs.
The flight recorder has already been implemented inside of the
golang.org/x/exp/trace package. This copies the current implementation
and modifies it to work within the runtime/trace package.
The changes include:
This adds the ability for multiple consumers (both the execution
tracer and the flight recorder) to subscribe to tracing events. This
change allows us to add multiple consumers without making major
modifications to the runtime. Future optimizations are planned
for this functionality.
This removes the use of byte readers from the process that
parses and processes the trace batches.
This modifies the flight recorder to not parse out the trace
clock frequency, since that requires knowledge of the format that's
unfortunate to encode in yet another place. Right now, the trace clock
frequency is considered stable for the lifetime of the program, so just
grab it directly from the runtime.
This change adds an in-band end-of-generation signal to the internal
implementation of runtime.ReadTrace. The internal implementation is
exported via linkname to runtime/trace, so the flight recorder can
identify exactly when a generation has ended. This signal is also useful
for ensuring that subscribers to runtime trace data always see complete
generations, by starting or stopping data streaming only at generation
boundaries.
For #63185
Change-Id: I5c15345981a6bbe9764a3d623448237e983c64ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/673116
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I1b8cf3939b433e1765682196b8fc1aa07d37f895
Reviewed-on: https://go-review.googlesource.com/c/go/+/673476
Auto-Submit: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Roland Shoemaker [Wed, 21 May 2025 18:43:35 +0000 (11:43 -0700)]
crypto/sha3: un-interleave EOR instructions
Move two EOR instructions out of blocks of RAX and
BCAX instructions. This appears to get a teeny
performance improvement, and matches what the
Linux kernel implementation does.
Daniel McCarney [Sat, 17 May 2025 15:17:21 +0000 (11:17 -0400)]
crypto/tls: use decode alert for handshake msg unmarshal err
Previously if instances of the handshakeMessage interface returned false
from unmarshal(), indicating an umarshalling error, the crypto/tls
package would emit an unexpected_message alert. This commit changes to
use a decode_error alert for this condition instead.
The usage-pattern of the handshakeMessage interface is that we switch on
the message type, invoke a specific concrete handshakeMessage type's
unmarshal function, and then return it to the caller on success. At this
point the caller looks at the message type and can determine if the
message was unexpected or not. If it was unexpected, the call-sites emit
the correct error for that case. Only the caller knows the current
protocol state and allowed message types, not the generic handshake
decoding logic.
With the above in mind, if we find that within the unmarshal logic for
a specific message type that the data we have in hand doesn't match the
protocol syntax we should emit a decode_error. An unexpected_message
error isn't appropriate because we don't yet know if the message is
unexpected or not, only that the message can't be decoded based on the
spec's syntax for the type the message claimed to be.
Notably one unit test, TestQUICPostHandshakeKeyUpdate, had to have its
test data adjusted because it was previously not testing the right
thing: it was double-encoding the type & length prefix data for a key
update message and expecting the QUIC logic to reject it as an
inappropriate post-handshake message. In reality it was being rejected
sooner as an invalid key update message from the double-encoding and
this was masked by the previous alert for this condition matching the
expected alert.
Finally, changing our alert allows enabling a handful of BoGo tests
related to duplicate extensions of the form
"DuplicateExtension[Server|Client]-TLS-[TLS1|TLS11|TLS12|TLS13]". One
test remains skipped (DuplicateExtensionClient-TLS-TLS13), as it
requires additional follow-up.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/673738 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Filippo Valsorda [Sat, 15 Mar 2025 14:12:39 +0000 (15:12 +0100)]
crypto/tls: disable SHA-1 signature algorithms in TLS 1.2
This implements RFC 9155 by removing support for SHA-1 algorithms:
- we don't advertise them in ClientHello and CertificateRequest
(where supportedSignatureAlgorithms is used directly)
- we don't select them in our ServerKeyExchange and CertificateVerify
(where supportedSignatureAlgorithms filters signatureSchemesForCertificate)
- we reject them in the peer's ServerKeyExchange and CertificateVerify
(where we check against the algorithms we advertised in ClientHello
and CertificateRequest)
Fixes #72883
Change-Id: I6a6a4656e2aafd2c38cdd32090d3d8a9a8047818
Reviewed-on: https://go-review.googlesource.com/c/go/+/658216
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Roland Shoemaker [Thu, 8 May 2025 23:27:36 +0000 (16:27 -0700)]
crypto: add a test for disallowed instructions
WORD and BYTE usage in crypto assembly cores is an anti-pattern which
makes extremely sensitive code significantly harder to understand, and
can result in unexpected behavior.
Because of this, we've decided to ban their usage in the crypto/ tree
(as part of the cryptography assembly policy).
This test walks the crypto/ tree looking for assembly files (those with
the filetype .s) and look for lines that match the regular rexpression
"(^|;)\s(BYTE|WORD)\s".
Change-Id: I60b5283e05e8588fa53273904a9611a411741f72
Reviewed-on: https://go-review.googlesource.com/c/go/+/671099 Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Filippo Valsorda [Sat, 15 Mar 2025 11:12:22 +0000 (12:12 +0100)]
crypto/tls: don't advertise TLS 1.2-only sigAlgs in TLS 1.3
If a ClientHello only supports TLS 1.3, or if a CertificateRequest is
sent after selecting TLS 1.3, we should not advertise TLS 1.2-only
signature_algorithms like PKCS#1 v1.5 or SHA-1.
However, since crypto/x509 still supports PKCS#1 v1.5, and a direct
CertPool match might not care about the signature in the certificate at
all, start sending a separate signature_algorithms_cert extension to
indicate support for PKCS#1 v1.5 and SHA-1 in certificates.
We were already correctly rejecting these algorithms if the peer
selected them in a TLS 1.3 connection.
Updates #72883
Change-Id: I6a6a4656ab60e1b7fb20fdedc32604dc156953ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/658215 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Dmitri Shuralyov [Wed, 21 May 2025 17:13:48 +0000 (13:13 -0400)]
cmd/link/internal/ld: use 12.0.0 OS/SDK versions for macOS linking
Go 1.25 will require macOS 12 Monterey or later, so macOS 11 will be
unsupported. The comment here suggests using a supported macOS version,
and that it can be the most recent one.
For now, make a minimal change of going from 11.0.0 to 12.0.0 so that
the chosen version is a supported one (although not the most recent).
However, it looks like even in CL 460476 (where the comment was added)
we were staying with the macOS version that matched Go's oldest, so we
might not have have recent experience with going beyond that. Update
the comment accordingly.
For #69839.
Change-Id: I90908971b0d5a8235ce77dc6bc9649e86008270a
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64-longtest,gotip-darwin-arm64-longtest,gotip-darwin-amd64_12,gotip-darwin-amd64_14,gotip-darwin-arm64_12,gotip-darwin-arm64_15
Reviewed-on: https://go-review.googlesource.com/c/go/+/675095
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Michael Matloob [Wed, 21 May 2025 18:47:49 +0000 (14:47 -0400)]
cmd/doc: better support for no network
Allow skipping the deprecation check when GOPROXY=off. The deprecation
check is an informational message so this doesn't affect the success of
the command. We should probably skip the check in more cases when
GOPROXY=off but that's a bigger change that should be made in a later
release.
There are still some deps.dev log messages that we should try to
suppress.
For #68106
Change-Id: Ifa0efd01ed623bb68c7ad7c5cfb6705547d157a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/675155
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Matloob <matloob@google.com> Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Michael Matloob [Wed, 21 May 2025 16:20:59 +0000 (12:20 -0400)]
cmd/go/internal/doc: ignore SIGINT and SIGQUIT
Just like we do in cmd/doc when we start pkgsite, ignore SIGINT (and
SIGQUIT on unix) when we start cmd/doc so that it's handled by cmd/doc
(if pkgsite is not started, and before it is started) or pkgsite, if it
is started. Also exit with the exit status of the command, rather than
using base.Errorf so that we don't print an extra error message to the
terminal.
For #68106
Change-Id: If968e88b95031761432d13dc47c5febe3391945d
Reviewed-on: https://go-review.googlesource.com/c/go/+/675076
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Michael Matloob <matloob@google.com>
reflect: leak packEface input to result rather than heap
This is part of a series of CLs that aim to help allocations
in reflect and reduce how often interface arguments escape
for the print functions in fmt.
Before this change, the reflect.Value parameter for packEface leaks
immediately to the heap due to the various ODOTPTR operations on the
*emptyInterface. The -m=2 logs report:
parameter v leaks to <heap> for packEface with derefs=0:
flow: <heap> ← v:
from v.ptr (dot) at .\value.go:145:13
from e.word = v.ptr (assign) at .\value.go:145:10
After this change, the input leaks to the result, which is what
we want:
parameter v leaks to ~r0 with derefs=0:
flow: e = v:
from v.ptr (dot) at .\value.go:143:13
from e.Data = v.ptr (assign) at .\value.go:143:10
flow: ~r0 = e:
from &e (address-of) at .\value.go:147:32
from *(*any)(unsafe.Pointer(&e)) (indirection) at .\value.go:147:9
from return *(*any)(unsafe.Pointer(&e)) (return) at .\value.go:147:2
This change here is needed, but reflect.Value.Interface still leaks its
input to the heap for other reasons having to do with method values,
which we attempt to address in CL 530097, CL 530095, and CL 530096.
Updates #8618
Updates #71349
Change-Id: Ie77bc850ff261212eeafe190bd6f9a879676a51d
Reviewed-on: https://go-review.googlesource.com/c/go/+/528535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: qiu laidongfeng2 <2645477756@qq.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Matloob [Wed, 21 May 2025 19:19:16 +0000 (15:19 -0400)]
cmd/doc: add more convenient behavior for go doc -http with no args
If we're in a module, go to the module's page. Outside of a module, but
in a workspace go to the home page, and outside of a module or
workspace, show the stdlib docs.
For #68106
Change-Id: I911a90a0e2b0a2bbb622f56e32827d5bdfa7f2fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/675235 Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
The crypto/tls package produces the expected error for this test case,
and so it can be enabled.
Looking at the history of the relevant code it appears the TLS 1.3
implementation has always had the correct behaviour for HRR changing to
an unsupported group after the initial hello.
I think this test was skipped initially because at the time of
initial BoGo config commit we hadn't implemented the -curves argument
for the test shim yet, and this test relies on it. We later added
support for that flag alongside X25519Kyber768Draft00 KX and I think we
missed the chance to enable the test then.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/673756 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net>
Daniel McCarney [Fri, 16 May 2025 17:44:53 +0000 (13:44 -0400)]
crypto/tls: match compression method alert across versions
When a pre-TLS 1.3 server processes a client hello message that
indicates compression methods that don't include the null compression
method, send an illegal parameter alert.
Previously we did this for TLS 1.3 server handshakes only, and the
legacy TLS versions used alertHandshakeFailure for this circumstance. By
switching this to alertIllegalParameter we use a consistent alert across
all TLS versions, and can also enable the NoNullCompression-TLS12 BoGo
test we were skipping.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/673736
TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com>
Daniel McCarney [Fri, 16 May 2025 17:08:16 +0000 (13:08 -0400)]
crypto/tls: verify server chooses advertised curve
When a crypto/tls client using TLS < 1.3 sends supported elliptic_curves
in a client hello message the server must limit itself to choosing one
of the supported options from our message. If we process a server key
exchange message that chooses an unadvertised curve, abort the
handshake w/ an error.
Previously we would not note that the server chose a curve we didn't
include in the client hello message, and would proceed with the
handshake as long as the chosen curve was one that we've implemented.
However, RFC 8422 5.1 makes it clear this is a server acting
out-of-spec, as it says:
If a server does not understand the Supported Elliptic Curves
Extension, does not understand the Supported Point Formats Extension,
or is unable to complete the ECC handshake while restricting itself
to the enumerated curves and point formats, it MUST NOT negotiate the
use of an ECC cipher suite.
Changing our behaviour to enforce this also allows enabling the
UnsupportedCurve BoGo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/673735
TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
thepudds [Sat, 1 Mar 2025 19:30:03 +0000 (14:30 -0500)]
reflect: optimize IsZero with a pointer comparison to global zeroVal
Our prior CL 649078 teaches the compiler to use a pointer to
runtime.zeroVal as the data pointer for an interface in cases it where
it can see that a zero value struct or array is being used in
an interface conversion.
This applies to some uses with reflect, such as:
s := S{}
v := reflect.ValueOf(s)
This CL builds on that to do a cheap pointer check in reflect.IsZero
to see if the Value points to runtime.zeroVal, which means it is a zero
value.
An alternative might be to do an initial pointer check in the typ.Equal
function for types where it makes sense to do but doesn't already.
This CL gives a performance boost of -51.71% geomean for
BenchmarkZero/IsZero, with most of the impact there on
arrays of structs. (The left column is CL 649078 and the right column
is this CL).
Note these are of course micro benchmarks with easily predicted
branches. The extra branch we introduce in the CL might hurt if there
was for example a tight loop where 50% of the values used the
global zeroVal and 50% didn't in a way that is not well predicted,
although if the typ.Equal for many types already does an initial
pointer check, it might not matter much.
For the older BenchmarkIsZero in reflect, this change does not help.
(The compiler does not use the global zeroVal as the data word for the
interfaces in this benchmark because values are part of a larger value
that is too big to be used in the global zeroVal, and also a piece of
the larger value is mutated and is not zero).
Change-Id: Ie083853a5bff03856277a293d94532a681f4a8d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/654135 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org>
thepudds [Wed, 12 Feb 2025 23:45:42 +0000 (18:45 -0500)]
cmd/compile/internal/walk: use global zeroVal in interface conversions for zero values
This is a small-ish adjustment to the change earlier in our
stack in CL 649555, which started creating read-only global storage
for a composite literal used in an interface conversion and setting
the interface data pointer to point to that global storage.
In some cases, there are execution-time performance benefits to point
to runtime.zeroVal in particular. In reflect, pointer checks against
the runtime.zeroVal memory address are used to side-step some work,
such as in reflect.Value.Set and reflect.Value.IsZero.
In this CL, we therefore dig up the zeroVal symbol, and we use the
machinery from earlier in our stack to use a pointer to zeroVal for
the interface data pointer if we see examples like:
sink = S{}
or:
s := S{}
sink = s
CL 649076 (also earlier in our stack) added most of the tests
along with debug diagnostics in convert.go to make it easier
to test this change.
We add a benchmark in reflect to show examples of performance benefit.
The left column is our immediately prior CL 649555, and the right is
this CL. (The arrays of structs here do not seem to benefit, which
we attempt to address in our next CL).
Finally, here are results from the benchmark example from #71323.
Note however that almost all the benefit shown here is from our earlier
CL 649555, which is a more general purpose change and eliminates
the allocation using a different read-only global than this CL.
Change-Id: I64d8cf1a7900f011d2ec59b948388aeda1150676
Reviewed-on: https://go-review.googlesource.com/c/go/+/649078
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
thepudds [Fri, 14 Feb 2025 04:49:50 +0000 (23:49 -0500)]
cmd/compile/internal/walk: convert composite literals to interfaces without allocating
Today, this interface conversion causes the struct literal
to be heap allocated:
var sink any
func example1() {
sink = S{1, 1}
}
For basic literals like integers that are directly used in
an interface conversion that would otherwise allocate, the compiler
is able to use read-only global storage (see #18704).
This CL extends that to struct and array literals as well by creating
read-only global storage that is able to represent for example S{1, 1},
and then using a pointer to that storage in the interface
when the interface conversion happens.
A more challenging example is:
func example2() {
v := S{1, 1}
sink = v
}
In this case, the struct literal is not directly part of the
interface conversion, but is instead assigned to a local variable.
To still avoid heap allocation in cases like this, in walk we
construct a cache that maps from expressions used in interface
conversions to earlier expressions that can be used to represent the
same value (via ir.ReassignOracle.StaticValue). This is somewhat
analogous to how we avoided heap allocation for basic literals in
CL 649077 earlier in our stack, though here we also need to do a
little more work to create the read-only global.
CL 649076 (also earlier in our stack) added most of the tests
along with debug diagnostics in convert.go to make it easier
to test this change.
Change-Id: I8924f0c69ff738ea33439bd6af7b4066af493b90
Reviewed-on: https://go-review.googlesource.com/c/go/+/649555
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
Filippo Valsorda [Wed, 21 May 2025 17:41:06 +0000 (19:41 +0200)]
crypto/internal/fips140: use hash.Hash
Since package hash is just the interface definition, not an
implementation, we can make a good argument that it doesn't impact the
security of the module and can be imported from outside.
For #69521
Change-Id: I6a6a4656b9c3cac8bb9ab8e8df11fa3238dc5d1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/674917 Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Daniel McCarney [Thu, 15 May 2025 17:41:14 +0000 (13:41 -0400)]
crypto/tls: have servers prefer TLS 1.3 when supported
Previously the common Config.mutualVersion() code prioritized the
selected version based on the provided peerVersions being sent in peer
preference order.
Instead we would prefer to see TLS 1.3 used whenever it is
supported, even if the peer would prefer an older protocol version.
This commit updates mutualVersions() to implement this policy change.
Our new behaviour matches the behaviour of other TLS stacks, notably
BoringSSL, and so also allows enabling the IgnoreClientVersionOrder BoGo
test that we otherwise must skip.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/673236
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Roland Shoemaker [Wed, 7 May 2025 18:37:52 +0000 (11:37 -0700)]
crypto/tls: add GetEncryptedClientHelloKeys
This allows servers to rotate their ECH keys without needing to restart
the server.
Fixes #71920
Change-Id: I55591ab3303d5fde639038541c50edcf1fafc9aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/670655
TryBot-Bypass: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Roland Shoemaker [Fri, 14 Mar 2025 00:04:30 +0000 (17:04 -0700)]
crypto: drop pre-AVX2 SHA assembly implementations
Drop the entire pre-AVX2 assembly implementation of SHA-1, SHA-256, and
SHA-512. This also technically impacts the SHA-1 AVX2 implementation,
since it previously called the pre-AVX2 implementation for the last
block if the number of blocks wasn't a multiple of 2.
Instead of keeping the entire implementation just for that case, we
just call the generic implementation for the last block. This will be a
little slower, but still seems like a win.
Updates #69587
Change-Id: Id5234c42910d8c6ec6b8df700a721c0953dff02b
Reviewed-on: https://go-review.googlesource.com/c/go/+/657716 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Julian Zhu [Wed, 21 May 2025 08:36:53 +0000 (16:36 +0800)]
cmd/compile/internal: intrinsify publicationBarrier on mipsx
This enables publicationBarrier to be used as an intrinsic on mipsx.
Change-Id: Ic199f34b84b3058bcfab79aac8f2399ff21a97ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/674856 Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
Mateusz Poliwczak [Tue, 20 May 2025 16:21:22 +0000 (16:21 +0000)]
crypto/x509: disallow negative path length
pathLenConstraint is restricted to unsigned integers.
Also the -1 value of cert.MaxPathLength has a special
meaning, so we shouldn't allow unmarshaling -1.
Julian Zhu [Tue, 20 May 2025 02:55:33 +0000 (10:55 +0800)]
cmd/compile/internal: intrinsify publicationBarrier on mips64x
This enables publicationBarrier to be used as an intrinsic on mips64x.
Change-Id: I4030ea65086c37ee1dcc1675d0d5d40ef8683851
Reviewed-on: https://go-review.googlesource.com/c/go/+/674855
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Michael Anthony Knyszek [Wed, 21 May 2025 17:50:15 +0000 (17:50 +0000)]
internal/trace: skip clock snapshot checks on Windows in stress mode
Windows' monotonic and wall clock granularity is just too coarse to get
reasonable values out of stress mode, which is creating new trace
generations constantly.
Fixes #73813.
Change-Id: Id9cb2fed9775ce8d78a736d0164daa7bf45075e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/675096 Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
thepudds [Wed, 12 Feb 2025 23:55:04 +0000 (18:55 -0500)]
cmd/compile/internal/escape: propagate constants to interface conversions to avoid allocs
Currently, the integer value in the following interface conversion gets
heap allocated:
v := 1000
fmt.Println(v)
In contrast, this conversion does not currently cause the integer value
to be heap allocated:
fmt.Println(1000)
The second example is able to avoid heap allocation because of an
optimization in walk (by Josh in #18704 and related issues) that
recognizes a literal is being used. In the first example, that
optimization is currently thwarted by the literal getting assigned
to a local variable prior to use in the interface conversion.
This CL propagates constants to interface conversions like
in the first example to avoid heap allocations, instead using
a read-only global. The net effect is roughly turning the first example
into the second.
One place this comes up in practice currently is with logging or
debug prints. For example, if we have something like:
func conditionalDebugf(format string, args ...interface{}) {
if debugEnabled {
fmt.Fprintf(io.Discard, format, args...)
}
}
Prior to this CL, this integer is heap allocated, even when the
debugEnabled flag is false, and even when the compiler
inlines conditionalDebugf:
v := 1000
conditionalDebugf("hello %d", v)
With this CL, the integer here is no longer heap allocated, even when
the debugEnabled flag is enabled, because the compiler can now see that
it can use a read-only global.
See the writeup in #71359 for more details.
CL 649076 (earlier in our stack) added most of the tests
along with debug diagnostics in convert.go to make it easier
to test this change.
Change-Id: I19a51e74b36576ebb0b9cf599267cbd2bd847ce4
Reviewed-on: https://go-review.googlesource.com/c/go/+/649079
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
cmd/compile/internal/escape: use an ir.ReassignOracle
Using the new-ish ir.ReassignOracle is more efficient than calling
ir.StaticValue repeatedly.
This CL now uses an ir.ReassignOracle for the recent
make constant propagation introduced in CL 649035.
We also pull the main change from CL 649035 into a new function,
which we will update later in our stack. We will also use the
ReassignOracles introduced here later in our stack.
(We originally did most of this work in CL 649077, but we abandoned
that in favor of CL 649035).
We could also use an ir.ReassignOracle in the older processing of
ir.OCALLFUNC in (*escape).call, but for now, we just leave that
as a TODO.
Updates #71359
Change-Id: I6e02eeac269bde3a302622b4dfe0c8dc63ec9ffc
Reviewed-on: https://go-review.googlesource.com/c/go/+/673795 Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Michael Pratt [Wed, 21 May 2025 14:58:32 +0000 (10:58 -0400)]
cmd/link: require cgo internal linking in TestIssue33979
This was a typo regression in CL 643897, which accidentally dropped the
requirement for cgo internal linking. As a result, this test is
continuously failing on windows-arm64.
For #71395.
Cq-Include-Trybots: luci.golang.try:gotip-windows-arm64
Change-Id: I6a6a636c25fd399cda6649ef94655aa112f10f63
Reviewed-on: https://go-review.googlesource.com/c/go/+/675015
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Roland Shoemaker [Sat, 25 Jan 2025 18:28:02 +0000 (10:28 -0800)]
crypto/tls: replace custom intern cache with weak cache
Uses the new weak package to replace the existing custom intern cache
with a map of weak.Pointers instead. This simplifies the cache, and
means we don't need to store a slice of handles on the Conn anymore.
Change-Id: I5c2bf6ef35fac4255e140e184f4e48574b34174c
Reviewed-on: https://go-review.googlesource.com/c/go/+/644176
TryBot-Bypass: Roland Shoemaker <roland@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Michael Pratt [Mon, 5 May 2025 17:44:26 +0000 (13:44 -0400)]
runtime: use cgroup CPU limit to set GOMAXPROCS
This CL adds two related features enabled by default via compatibility
GODEBUGs containermaxprocs and updatemaxprocs.
On Linux, containermaxprocs makes the Go runtime consider cgroup CPU
bandwidth limits (quota/period) when setting GOMAXPROCS. If the cgroup
limit is lower than the number of logical CPUs available, then the
cgroup limit takes precedence.
On all OSes, updatemaxprocs makes the Go runtime periodically
recalculate the default GOMAXPROCS value and update GOMAXPROCS if it has
changed. If GOMAXPROCS is set manually, this update does not occur. This
is intended primarily to detect changes to cgroup limits, but it applies
on all OSes because the CPU affinity mask can change as well.
The runtime only considers the limit in the leaf cgroup (the one that
actually contains the process), caching the CPU limit file
descriptor(s), which are periodically reread for updates. This is a
small departure from the original proposed design. It will not consider
limits of parent cgroups (which may be lower than the leaf), and it will
not detection cgroup migration after process start.
We can consider changing this in the future, but the simpler approach is
less invasive; less risk to packages that have some awareness of runtime
internals. e.g., if the runtime periodically opens new files during
execution, file descriptor leak detection is difficult to implement in a
stable way.
For #73193.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Change-Id: I6a6a636c631c1ae577fb8254960377ba91c5dc98
Reviewed-on: https://go-review.googlesource.com/c/go/+/670497
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Roland Shoemaker [Sun, 19 Jan 2025 17:24:50 +0000 (09:24 -0800)]
crypto/sha1: add sha-ni AMD64 implementation
Based on the Intel docs. Provides a ~44% speed-up compared to the AVX
implementation and a ~57% speed-up compared to the generic AMD64
assembly implementation.
Roland Shoemaker [Sat, 22 Mar 2025 00:58:55 +0000 (00:58 +0000)]
runtime: add valgrind instrumentation
Add build tag gated Valgrind annotations to the runtime which let it
understand how the runtime manages memory. This allows for Go binaries
to be run under Valgrind without emitting spurious errors.
Instead of adding the Valgrind headers to the tree, and using cgo to
call the various Valgrind client request macros, we just add an assembly
function which emits the necessary instructions to trigger client
requests.
In particular we add instrumentation of the memory allocator, using a
two-level mempool structure (as described in the Valgrind manual [0]).
We also add annotations which allow Valgrind to track which memory we
use for stacks, which seems necessary to let it properly function.
We describe the memory model to Valgrind as follows: we treat heap
arenas as a "pool" created with VALGRIND_CREATE_MEMPOOL_EXT (so that we
can use VALGRIND_MEMPOOL_METAPOOL and VALGRIND_MEMPOOL_AUTO_FREE).
Within the pool we treat spans as "superblocks", annotated with
VALGRIND_MEMPOOL_ALLOC. We then allocate individual objects within spans
with VALGRIND_MALLOCLIKE_BLOCK.
It should be noted that running binaries under Valgrind can be _quite
slow_, and certain operations, such as running the GC, can be _very
slow_. It is recommended to run programs with GOGC=off. Additionally,
async preemption should be turned off, since it'll cause strange
behavior (GODEBUG=asyncpreemptoff=1).
Running Valgrind with --leak-check=yes will result in some errors
resulting from some things not being marked fully free'd. These likely
need more annotations to rectify, but for now it is recommended to run
with --leak-check=off.
Michael Matloob [Wed, 21 May 2025 16:18:03 +0000 (12:18 -0400)]
cmd/doc: allow go doc -http without package in current directory
go doc tries to find a package to display documentation for. In the case
that no package is provided, it uses "." just like go list does. So if
go doc -http is run without any arguments, it tries to show the
documentation for the package in the current directory. As a special
case, if no arguments are provided, allow no package to match the
current directory and just open the root pkgsite page.
For #68106
Change-Id: I6d65b160a838591db953fac630eced6b09106877
Reviewed-on: https://go-review.googlesource.com/c/go/+/675075 Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Tue, 20 May 2025 20:26:56 +0000 (20:26 +0000)]
runtime: guarantee checkfinalizers test allocates in a shared tiny block
Currently the checkfinalizers test (TestDetectCleanupOrFinalizerLeak)
only *tries* to ensure the tiny alloc with a cleanup attached shares a
block with other objects. However, what it does is insufficient, because
it could get unlucky and have the last object allocated be the first
object of a new block.
This change changes the test to guarantee that a tiny object is not at
the start of a fresh block by looking at the alignment of the object's
pointer. If the object's pointer is odd, then that's good enough to know
that it shares a block with something else, since the blocks themselves
are aligned to a much higher power of two.
This fixes a failure I've seen on the builders.
Fixes #73810.
Change-Id: Ieafdbb9cccb0d2dc3659a9a5d9d9233718461635
Reviewed-on: https://go-review.googlesource.com/c/go/+/674655
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Mark Freeman [Mon, 19 May 2025 22:00:31 +0000 (18:00 -0400)]
internal/pkgbits: indent productions and hoist some types up
The types being hoisted are those which cannot be referenced; that is,
where Ref[T] is illegal. These are most clearly owned by pkgbits. The
types which follow are those which can be referenced.
Referenceable types are more hazy due to the reference mechanism of UIR
- sections. These are a detail of the UIR file format and are surfaced
directly to importers.
I suspect that pkgbits would benefit from a reference mechanism not
dependent on sections. This would permit us to push down many types
from the noder into pkgbits, reducing the interface surface without
giving up deduplication.
Change-Id: Ifaf5cd9de20c767ad0941413385b308d628aac6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/674635
Auto-Submit: Mark Freeman <mark@golang.org>
TryBot-Bypass: Mark Freeman <mark@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Felix Geisendörfer [Fri, 28 Feb 2025 16:30:54 +0000 (17:30 +0100)]
internal/trace: add generator tests for sync events
Add generator tests that verify the timestamps for the sync events
emitted in the go1.25 trace format and earlier versions.
Add the ability to configure the properties of the per-generation sync
batches in testgen. Also refactor testgen to produce more realistic
timestamps by keeping track of lastTs and using it for structural
batches that don't have their own timestamps. Otherwise they default to
zero which means the minTs of the generation can't be controlled.
For #69869
Change-Id: I92a49b8281bc4169b63e13c030c1de7720cd6f26
Reviewed-on: https://go-review.googlesource.com/c/go/+/653876
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Felix Geisendörfer [Fri, 28 Feb 2025 16:28:43 +0000 (17:28 +0100)]
internal/trace/internal/testgen: make generated trace version configurable
Replace hard coded references to version.Go122 with the trace version
passed to NewTrace. This allows writing testgen tests for newer trace
versions.
For #69869
Change-Id: Id25350cea1c397a09ca23465526ff259e34a4752
Reviewed-on: https://go-review.googlesource.com/c/go/+/653875
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Felix Geisendörfer [Sat, 3 May 2025 12:56:23 +0000 (15:56 +0300)]
internal/trace: add a validator test for the new clock snapshots
Check that the clock snapshots, when expected to be present, are
non-zero and monotonically increasing.
This required some refactoring to make the validator aware of the
version of the trace it is validating.
Change-Id: I04c4dd10fe6975cbac12bb0ddaebcec3a5284e7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/669715
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: David Chase <drchase@google.com>
Felix Geisendörfer [Fri, 28 Feb 2025 15:07:45 +0000 (16:07 +0100)]
internal/trace: expose clock snapshot timestamps on sync event
Add ClockSnapshot field to the Sync event type and populate it with the
information from the new EvClockSnapshot event when available.
For #69869
Change-Id: I3b24b5bfa15cc7a7dba270f5e6bf189adb096840
Reviewed-on: https://go-review.googlesource.com/c/go/+/653576 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Felix Geisendörfer [Fri, 28 Feb 2025 15:07:16 +0000 (16:07 +0100)]
runtime,internal/trace: emit clock snapshots at the start of trace generations
Replace the per-generation EvEventBatch containing a lone EvFrequency
event with a per-generation EvEventBatch containing a EvSync header
followed by an EvFrequency and EvClockSnapshot event.
The new EvClockSnapshot event contains trace, mono and wall clock
snapshots taken in close time proximity. Ignoring minor resolution
differences, the trace and mono clock are the same on linux, but not on
windows (which still uses a TSC based trace clock).
Emit the new sync batch at the very beginning of every new generation
rather than the end to be in harmony with the internal/trace reader
which emits a sync event at the beginning of every generation as well
and guarantees monotonically increasing event timestamps.
Bump the version of the trace file format to 1.25 since this change is
not backwards compatible.
Update the internal/trace reader implementation to decode the new
events, but do not expose them to the public reader API yet. This is
done in the next CL.
For #69869
Change-Id: I5bfedccdd23dc0adaf2401ec0970cbcc32363393
Reviewed-on: https://go-review.googlesource.com/c/go/+/653575 Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
The RISC-V integer vector multiply add instructions are not encoded
correctly; the first and second arguments are swapped. For example,
the instruction
This is inconsistent with the argument ordering we use for 3
argument vector instructions, in which the argument order, as given
in the RISC-V specifications, is reversed, and also with the vector
FMA instructions which have the same argument ordering as the vector
integer multiply add instructions in the "The RISC-V Instruction Set
Manual Volume I". For example, in the ISA manual we have the
following instruction definitions