Robert Findley [Thu, 17 Nov 2022 01:58:58 +0000 (20:58 -0500)]
go/types, types2: ensure signatures are instantiated if all type args
are provided
Improve the accuracy of recorded types and instances for function calls,
by instantiating their signature before checking arguments if all type
arguments are provided. This avoids a problem where fully instantiated
function signatures are are not recorded as such following an error
checking their arguments.
Fixes golang/go#51803
Change-Id: Iec4cbd219a2cd19bb1bcf2a5c4019f556e4304b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/451436 Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Keith Randall [Wed, 9 Nov 2022 01:48:48 +0000 (17:48 -0800)]
runtime: fix conflict between lfstack and checkptr
lfstack does very unsafe things. In particular, it will not
work with nodes that live on the heap. In normal use by the runtime,
that is the case (it is only used for gc work bufs). But the lfstack
test does use heap objects. It goes through some hoops to prevent
premature deallocation, but those hoops are not enough to convince
-d=checkptr that everything is ok.
Instead, allocate the test objects outside the heap, like the runtime
does for all of its lfstack usage. Remove the lifetime workaround
from the test.
Reported in https://groups.google.com/g/golang-nuts/c/psjrUV2ZKyI
Change-Id: If611105eab6c823a4d6c105938ce145ed731781d
Reviewed-on: https://go-review.googlesource.com/c/go/+/448899 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Russ Cox [Thu, 17 Nov 2022 19:24:06 +0000 (14:24 -0500)]
runtime: work around Apple libc bugs to make exec stop hanging
For a while now, we've had intermittent reports about problems with
os/exec on macOS, but no clear way to reproduce them. Recent changes
in the os/exec package test seem to have aligned the stars just right,
at least on my two x86 and ARM MacBook Pro laptops, to make the
package test hang with roughly 50% probability. When it does hang, the
stacks I see in the hung process match the ones reported for the
Go-based hangs in #33565. (They do not match the ones reported in the
so-called C reproducer in that issue, but I think that reproducer is
actually reproducing a different race, between fork and exit.)
The stacks obtained from the hung child processes are in
libSystem_atfork_child, which is supposed to reinitialize various
parts of the C library in the new process.
One common stack dies in _notify_fork_child calling _notify_globals
(inlined) calling _os_alloc_once, because _os_alloc_once detects that
the once lock is held by the parent process and then calls
_os_once_gate_corruption_abort. The allocation is setting up the
globals for the notification subsystem. See the source code at [1].
To work around this, we can allocate the globals earlier in the Go
program's lifetime, before any execs are involved, by calling any
notify routine that is exported, calls _notify_globals, and doesn't do
anything too expensive otherwise. notify_is_valid_token(0) fits the bill.
The other common stack dies in xpc_atfork_child calling
_objc_msgSend_uncached which ends up in
WAITING_FOR_ANOTHER_THREAD_TO_FINISH_CALLING_+initialize. Of course,
whatever thread the child is waiting for is in the parent process and
is not going to finish anything in the child process. There is no
public source code for these routines, so it is unclear exactly what
the problem is. However, xpc_atfork_child turns out to be exported
(for use by libSystem_atfork_child, which is in a different library,
so xpc_atfork_child is unlikely to be unexported any time soon).
It also stands to reason that since xpc_atfork_child is called at the
start of any forked child process, it can't be too harmful to call at
the start of an ordinary Go process. And whatever caches it needs for
a non-deadlocking fast path during exec empirically do get initialized
by calling it at startup.
This CL introduces a function osinit_hack, called at osinit time,
which calls notify_is_valid_token(0) and xpc_atfork_child().
Doing so makes the os/exec test pass reliably on both my laptops -
I can run it successfully hundreds of times in a row when my previous
record was twice in a row.
Cherry Mui [Tue, 15 Nov 2022 18:37:42 +0000 (13:37 -0500)]
cmd/compile/internal/pgo: count only the last two frames as a call edge
Currently for every CPU profile sample, we apply its weight to all
call edges of the entire call stack. Frames higher up the stack
are unlikely to be repeated calls (e.g. runtime.main calling
main.main). So adding weights to call edges higher up the stack
may be not reflecting the actual call edge weights in the program.
This CL changes it to add weights to only the edge between the
last two frames.
Without a branch profile (e.g. LBR records) this is not perfect,
but seems more reasonable.
Cherry Mui [Tue, 15 Nov 2022 18:32:32 +0000 (13:32 -0500)]
cmd/compile: simplify PGO hot caller/callee computation
Currently, we use CDF to compute a weight threshold and then use
the weight threshold to determine whether a call site is hot. As
when we compute the CDF we already have a list of hot call sites
that make up the given percentage of the CDF, just use that list.
Also, when computing the CDF threshold, include the very last node
that makes it to go over the threshold. (I.e. if the CDF threshold
is 50% and one hot node takes 60% of weight, we should include that
node instead of excluding it. In practice it rarely matters,
probably only for testing and micro-benchmarks.)
Cuong Manh Le [Wed, 16 Nov 2022 18:02:26 +0000 (01:02 +0700)]
cmd/compile: fix broken IR for iface -> eface
For implementing interface to empty interface conversion, the compiler
generate code like:
var res *uint8
res = itab
if res != nil {
res = res.type
}
However, itab has type *uintptr, so the assignment is broken. The
problem is not shown up, until CL 450215, which call typecheck on this
broken assignment.
To fix this, just cast itab to *uint8 when doing the conversion.
Fixes #56768
Change-Id: Id42792d18e7f382578b40854d46eecd49673792c
Reviewed-on: https://go-review.googlesource.com/c/go/+/451256 Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Keith Randall [Wed, 9 Nov 2022 23:28:44 +0000 (15:28 -0800)]
sync/atomic: hint users of old API to use new type-based API instead
Fixes #56495
Change-Id: Ib2f39273da68e3056688306aa0d5e274b5507bf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/449237
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Ian Lance Taylor <iant@google.com>
Don't set the server's write deadline until after the client has
read the response headers, avoiding test failures if the deadline
expires before or while writing headers.
Fixes #56807.
Change-Id: I5f80c108b360d030132a13661774a30fac453856
Reviewed-on: https://go-review.googlesource.com/c/go/+/451715
Auto-Submit: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Keith Randall [Tue, 5 Apr 2022 22:07:29 +0000 (15:07 -0700)]
cmd/compile: teach regalloc about temporary registers
Temporary registers are sometimes needed for an architecture backend
which needs to use several machine instructions to implement a single
SSA instruction.
Mark such instructions so that regalloc can reserve the temporary register
for it. That way we don't have to reserve a fixed register like we do now.
Convert the temp-register-using instructions on amd64 to use this
new mechanism. Other archs can follow as needed.
Change-Id: I1d0c8588afdad5cd18b4398eb5a0f755be5dead7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398556
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
Cuong Manh Le [Thu, 17 Nov 2022 12:21:45 +0000 (19:21 +0700)]
cmd/compile: fix static init for inlined calls
CL 450136 made the compiler to be able to handle simple inlined calls in
staticinit. However, it's missed a condition when checking substituting
arg for param. If there's any non-trivial closures, it has captured one
of the param, so the substitution could not happen.
Fixes #56778
Change-Id: I427c9134e333e2f9af136c1a124da4d37d326f10
Reviewed-on: https://go-review.googlesource.com/c/go/+/451555
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
Roland Shoemaker [Thu, 17 Nov 2022 16:51:03 +0000 (08:51 -0800)]
crypto/x509: reduce boring test key size
Generating 8192 bit keys times out on builders relatively frequently. We
just need something that isn't a boringAllowCert allowed key size so we
can test that a non-boringAllowCert signed intermediate works, so just
use 512 instead since it'll be significantly faster.
Robert Griesemer [Fri, 4 Nov 2022 22:12:32 +0000 (15:12 -0700)]
go/types, types2: implement type checking of "clear" built-in
Will become available with Go 1.21.
Recognizing the `clear` built-in early is not causing any problems:
if existing code defines a `clear`, that will be used as before. If
code doesn't define `clear` the error message will make it clear
that with 1.21 the function will be available. It's still possible
to define a local `clear` and get rid of the error; but more likely
the name choice should be avoided going forward, so this provides a
useful early "heads-up".
For #56351.
Change-Id: I3d0fb1eb3508fbc78d7514b6238eac89610158c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/448076
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
cia-rana [Mon, 14 Nov 2022 08:01:03 +0000 (17:01 +0900)]
go/parser: allow trailing commas in embedded instantiated types
go/parser can correctly parse interfaces that instantiate and embed
generic interfaces, but not structs. This is because in the case of
structs, it does not expect RBRACK as a token trailing COMMA in the type
argument, even though it is allowed by the spec.
For example, go/parser produces an error for the type declaration below:
type A struct {
B[byte, []byte,]
}
Fixes #56748
Change-Id: Ibb2addd6cf9b381d8470a6d20eedb93f13f93cd6
Reviewed-on: https://go-review.googlesource.com/c/go/+/450175
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Zeke Lu [Thu, 17 Nov 2022 12:19:49 +0000 (12:19 +0000)]
time: avoid creating a parse error from the next chunk of the value
When it reports a parse error, it uses the "value" variable as the
value element of the parse error. Previously, in some of the cases,
the "value" variable is always updated to the next chunk of the value
to be parsed (even if an earlier chunk is invalid). The reported
parse error is confusing in this case.
This CL addresses this issue by holding the original value, and when
it fails to parse the time, use it to create the parse error.
Austin Clements [Thu, 17 Nov 2022 01:58:38 +0000 (20:58 -0500)]
cmd/dist: skip non-race tests
In -race mode, the dist test command only registers the std, race,
osusergo, and amd64ios tests before returning early from
(*tester).registerTests. Prior to CL 450018, the osusergo and amd64ios
tests weren't even affected by -race mode, so it seems their inclusion
was unintentional. CL 450018 lifted the logic to run tests in race
mode, which means these tests went from running without -race to
running with -race. Unfortunately, amd64ios is not compatible with
-race, so it is now failing on the darwin-amd64-race builder.
Fix this by omitting the osusergo and amd64ios tests from -race mode,
since it seems like they were really intended to be included anyway.
Bryan C. Mills [Thu, 17 Nov 2022 01:24:03 +0000 (20:24 -0500)]
cmd/fix: allow cgo commands in tests to fail if 'go build' is not supported
testenv.HasCgo reports whether the test binary may have been built
with cgo enabled, but having been built with cgo does not necessarily
imply that the test can invoke the cgo tool itself.
This should fix a test failure on the android builders introduced in
CL 450714.
Ian Lance Taylor [Wed, 16 Nov 2022 22:19:47 +0000 (14:19 -0800)]
net: change resolverConfig.dnsConfig to an atomic.Pointer
We were using a RWMutex RLock around a single memory load,
which is not a good use of a RWMutex--it introduces extra work
for the RLock but contention around a single memory load is unlikely.
And, the tryUpdate method was not acquiring the mutex anyhow.
The new atomic.Pointer type is type-safe and easy to use correctly
for a simple use-case like this.
Change-Id: Ib3859c03414c44d2e897f6d15c92c8e4b5c81a11
Reviewed-on: https://go-review.googlesource.com/c/go/+/451416
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Damien Neil [Thu, 22 Sep 2022 23:22:04 +0000 (16:22 -0700)]
archive/tar, archive/zip: return ErrInsecurePath for unsafe paths
Return a distinguishable error when reading an archive file
with a path that is:
- absolute
- escapes the current directory (../a)
- on Windows, a reserved name such as NUL
Users may ignore this error and proceed if they do not need name
sanitization or intend to perform it themselves.
Fixes #25849
Fixes #55356
Change-Id: Ieefa163f00384bc285ab329ea21a6561d39d8096
Reviewed-on: https://go-review.googlesource.com/c/go/+/449937 Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Damien Neil [Thu, 10 Nov 2022 01:49:44 +0000 (17:49 -0800)]
path/filepath: add IsLocal
IsLocal reports whether a path lexically refers to a location
contained within the directory in which it is evaluated.
It identifies paths that are absolute, escape a directory
with ".." elements, and (on Windows) paths that reference
reserved device names.
For #56219.
Change-Id: I35edfa3ce77b40b8e66f1fc8e0ff73cfd06f2313
Reviewed-on: https://go-review.googlesource.com/c/go/+/449239
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Joedian Reid <joedian@golang.org>
Cherry Mui [Mon, 3 Oct 2022 23:07:39 +0000 (19:07 -0400)]
cmd/go: add PGO auto mode
Add "auto" mode for the -pgo build flag. When -pgo=auto is
specified, if there is a default.pgo file in the directory of the
main package, it will be selected and used for the build.
Currently it requires exactly one main package when -pgo=auto is
specified. (We'll support multiple main packages in the future.)
Also apply to other build-related subcommands, "go install", "go
run", "go test", and "go list".
Bryan C. Mills [Tue, 15 Nov 2022 17:34:31 +0000 (12:34 -0500)]
cmd/fix: disallow cgo errors in tests
The 'cgo' command invoked by 'go fix' was not valid when built with
-trimpath, but the test was not failing because errors from the
command were being logged and ignored instead of causing tests to
fail. Changing the code and test not to ignore the errors revealed
that a number of existing tests were always, unconditionally
triggering cgo errors which were then ignored.
This change updates those tests to no longer produce cgo errors,
and to check their results when cgo is enabled.
Russ Cox [Wed, 16 Nov 2022 03:30:45 +0000 (22:30 -0500)]
cmd/go: automatically disable cgo on systems with no C compiler
The documentation for cgo has always said:
> The cgo tool is enabled by default for native builds
> on systems where it is expected to work.
Following the spirit of that rule, this CL disables cgo by default
on systems where $CC is unset and the default C compiler
(clang or gcc) is not found in $PATH.
This CL makes builds of Go code on systems with no C compiler
installed automatically fall back to non-cgo mode.
For example, if building a Go program using package net
in a stripped down Linux container, that build will now run
with cgo disabled, instead of attempting the build with cgo enabled
and only succeeding if the right pre-compiled .a files happen to
be loaded into the container.
This CL makes it safe to drop the pre-compiled .a files
from the Go distribution. Systems that don't have a C compiler
will simply disable cgo when building new .a files for that system.
In general keeping the pre-compiled .a files working in cgo mode
on systems without C compilers has had only mixed success due
to the precise build cache. Today we have had to disable various
checks in the precise build cache so that distributed .a files look
up-to-date even if the current machine's C compiler is a different
version than the one used when packaging the distribution.
Each time we improve precision we have a decent chance of
re-invalidating the files. This CL, combined with dropping the .a files
entirely, will let us re-enable those checks and ensure that the
.a files used in a build actually match the C compiler being used.
On macOS, the distributed .a files for cgo-dependent packages
have been stale (not actually used by the go command) since the
release of Go 1.14 in February 2020, due to CL 216304 setting
a CGO_CFLAGS environment variable that won't match the default
setting on users machines. (To keep the distributed .a files working,
that CL should have instead changed the default in the go command.)
The effect is that for the past six Go releases (!!!), the go command
has been unable to build basic programs like src/net/http/triv.go
on macOS without either disabling cgo or installing Xcode's C compiler.
This CL fixes that problem by disabling cgo when there's no C compiler.
Now it will once again be possible to build basic programs with just
a Go toolchain installed.
In the past, disabling cgo on macOS would have resulted in subpar
implementations of crypto/x509, net, and os/user, but as of CL 449316
those packages have all been updated to use libc calls directly,
so they now provide the same implementation whether or not cgo is enabled.
In the past, disabling cgo on macOS would also have made the
race detector unusable, but CL 451055 makes the race detector
work on macOS even when cgo is disabled.
On Windows, none of the standard library uses cgo today, so all
the necessary .a files can be rebuilt without a C toolchain,
and there is no loss of functionality in the standard library when
cgo is disabled. After this CL, the race detector won't work on
Windows without a C toolchain installed, but that turns out to be
true already: when linking race-enabled programs, even if the Go linker
does not invoke the host linker, it still attempts to read some of the
host C toolchain's .a files to resolve undefined references.
On Unix systems, disabling cgo when a C compiler is not present
will mean that builds get the pure Go net resolver, which is used
by default even in cgo builds when /etc/resolv.conf is simple enough.
It will also mean they get the pure os/user code, which reads
/etc/passwd and /etc/group instead of using shared libraries,
and therefore it may miss out on other sources of user information
such as LDAP. The race detector also will not work without a C compiler.
This would be dire except that nearly all Unix systems have a C compiler
installed by default, and on those that don't it is trivial to add one.
In particular, the vast majority of Go developers running on Linux
and other Unix systems will already have a C compiler and will be
unaffected.
Russ Cox [Wed, 16 Nov 2022 01:36:10 +0000 (20:36 -0500)]
runtime/race: do not use cgo on macOS
The use of an empty import "C" to trigger cgo in runtime/race
serves two purposes:
1. Cause the runtime to use the C library to create system threads,
because the race syso implementation expects things like
thread-local storage to work correctly.
2. Derive the right set of //go:cgo_import_dynamic comments
to pass to the Go linker, so that it doesn't diagnose them as
undefined references.
On macOS, (1) is unnecessary because using the C library
(via DLL calls) is the only way the runtime ever creates threads.
We can accomplish (2) by writing those comments ourselves.
Having done that in this CL, cgo is no longer needed to run
the race detector on macOS, which means that having a
pre-compiled set of .a files is no longer necessary,
nor is having Xcode for use with cgo when rebuilding those .a files.
Austin Clements [Sat, 12 Nov 2022 02:57:57 +0000 (21:57 -0500)]
cmd/dist: simplify old code
Now that all uses of "go test" have been converted over to the new
abstraction, we can delete the old helpers for building "go test"
commands and simplify some code that's only used by the new
abstraction now.
For #37486.
Change-Id: I770cd457e018160d694abcc0b6ac80f7dc2e8425
Reviewed-on: https://go-review.googlesource.com/c/go/+/450020 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Austin Clements [Fri, 4 Nov 2022 18:54:04 +0000 (14:54 -0400)]
cmd/dist: convert most remaining tests to use goTest
This converts most of the remaining manual "go test" command line
construction in cmd/dist to use the goTest abstraction and
registerTest.
At this point, the only remaining place that directly constructs go
test command lines is runHostTest.
This fixes a bug in the "nolibgcc:os/user" test. It was clearly
supposed to pass "-run=^Test[^CS]", but the logic to override the
"-run" flag for "nolibgcc:net" caused "nolibgcc:os/user" to pass
*both* "-run=^Test[^CS]" and "-run=". This was then rewritten into
just "-run=" by flattenCmdline, which caused all os/user tests to run,
and not actually skip the expensive tests as intended. (This is a
great example of why the new abstraction is much more robust than
command line construction.)
I traced all exec calls from cmd/dist on linux/amd64 and, other than
the fix to nolibgcc:os/user, this makes only no-op changes (such as
re-arranging the order of flags).
For #37486.
Change-Id: Ie8546bacc56640ea39f2804a87795c14a3fe4c7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/450018
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Austin Clements [Fri, 4 Nov 2022 18:49:37 +0000 (14:49 -0400)]
cmd/dist: restructure cgo_test
Currently, dist test has a single test called "cgo_test" that runs a
large number of different "go test"s.
This commit restructures cgo_test into several individual tests, each
of which runs a single "go test" that can be described by a goTest
object and registered with registerTest. Since this lets us raise the
abstraction level of constructing these tests and these tests are
mostly covering the Cartesian product of a small number of orthogonal
dimensions, we pull the common logic for constructing these tests into
a helper function.
For consistency, we now pass -tags=static to the static testtls and
nocgo tests, but this tag doesn't affect the build of these tests at
all. I traced all exec calls from cmd/dist on linux/amd64 and this is
the only non-trivial change.
For #37486.
Change-Id: I53c1efa1c38d785dc71968f05e8d7d636b553e96
Reviewed-on: https://go-review.googlesource.com/c/go/+/450017
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Austin Clements [Thu, 13 Oct 2022 14:57:13 +0000 (10:57 -0400)]
cmd/dist: make registerTest take a goTest
The overall goal is to make registerTest the primary entry point for
adding dist tests and to convert nearly all dist tests to be
represented by a goTest, registered via registerTest. This will
centralize the logic for creating dist tests corresponding to go tool
tests.
I traced all exec calls from cmd/dist on linux/amd64 and this makes
only no-op changes (such as re-arranging the order of flags).
For #37486.
Change-Id: I4749e6f3666134d3259b54ee6055d76a4235c60c
Reviewed-on: https://go-review.googlesource.com/c/go/+/450016 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Austin Clements [Wed, 2 Nov 2022 18:33:04 +0000 (14:33 -0400)]
cmd/dist: use goTest for manual go test invocations
This CL rewrites everywhere in dist that manually constructs an
exec.Cmd to run "go test" to use the goTest abstraction. All remaining
invocations of "go test" after this CL construct the command line
manually, but ultimately use addCmd to execute it.
I traced all exec calls from cmd/dist on linux/amd64 and this makes
only no-op changes (such as re-arranging the order of flags).
For #37486.
Change-Id: Idc7497e39bac04def7ddaf2010881c9623e76fd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/450015 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Austin Clements [Wed, 2 Nov 2022 18:27:28 +0000 (14:27 -0400)]
cmd/dist: introduce "go test" abstraction
This introduces an abstraction for constructing and running "go test"
commands. Currently, dist test is basically a shell script written in
Go syntax: it mostly just invokes lots of subprocesses, almost all of
which are "go test" invocations, and it constructs those command lines
directly from strings all over the place.
This CL raises the level of abstraction of invoking go test. The
current level of abstraction is not serving us very well: it's
conveniently terse, but the actual logic for constructing a command
line is typically so spread out that it's difficult to predict what
command will actually run. For example, the `gotest` function
constructs the basic command, but many tests want to override at least
some of these flags, so flattenCmdLine has logic specific to `go test`
for eliminating duplicate flags that `go test` itself would reject. At
the same time, the logic for constructing many common flags is
conditional, leading to a bevy of helpers for constructing flags like
`-short` and `-timeout` and `-run` that are scattered throughout
test.go and very easy to forget to call.
This CL centralizes and flattens all of this knowledge into a new
`goTest` type. This type gives dist a single, unified point where we
can change anything about how it invokes "go test".
There's currently some "unnecessary" abstraction in the implementation
of the goTest type to separate "build" and "run" flags. This will
become important later when we convert host tests and to do separate
build and run steps.
The following CLs will convert dist test to use this type rather than
directly constructing "go test" command lines. Finally, we'll strip
out the scattered helper logic for building command lines.
For #37486.
Change-Id: I9f1633fe6c0921696419ce8127ed2ca7b7a4e01b
Reviewed-on: https://go-review.googlesource.com/c/go/+/448802 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Russ Cox [Wed, 16 Nov 2022 17:04:32 +0000 (12:04 -0500)]
math/rand: deprecate Seed
Programs that call Seed and then expect a specific sequence
of results from the global random source (using functions such as Int)
can be broken when a dependency changes how much it consumes
from the global random source. To avoid such breakages, programs
that need a specific result sequence should use NewRand(NewSource(seed))
to obtain a random generator that other packages cannot access.
Fixes #56319.
Change-Id: Idac33991b719d2c71f109f51dacb3467a649e01e
Reviewed-on: https://go-review.googlesource.com/c/go/+/451375 Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Austin Clements [Fri, 11 Nov 2022 16:51:28 +0000 (11:51 -0500)]
cmd/go: make testterminal18153 a normal test
Currently, cmd/go's testterminal18153 is implemented as a special test
that doesn't run as part of cmd/go's regular tests. Because the test
requires stdout and stderr to be a terminal, it is currently run
directly by "dist test" so it can inherit the terminal of all.bash.
This has a few problems. It's yet another special case in dist test.
dist test also has to be careful to not apply its own buffering to
this test, so it can't run in parallel and it limits dist test's own
scheduler. It doesn't run as part of regular "go test", which means it
usually only gets coverage from running all.bash. And since we have to
skip it if all.bash wasn't run at a terminal, I'm sure it often gets
skipped even when running all.bash.
Fix all of this by rewriting this test to create its own PTY and
re-exec "go test" to check that PTY passes through go test. This makes
the test self-contained, so it can be a regular cmd/go test, we can
drop it from dist test, and it's not sensitive to the environment of
all.bash.
The registerTest function has a special case for commands that start
with "time", but we don't use this case anywhere. Delete this special
case and its support code.
Preparation for #37486.
Change-Id: Ica180417e7aa4e4fc260cb97467942bae972fdb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/448801
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The cmd/dist cgo_test enumerates a large number of platforms in
various special cases. Some combinations are suspiciously absent. This
CL completes the combinations.
I've confirmed using trybots that the newly-enabled tests pass on
android/* (this is not surprising because the gohostos is never
"android" anyway), windows/arm64, linux/ppc64 (no cgo), linux/loong64
(except for one test, filed #56623), linux/mips*, netbsd/arm (except
for one test, filed #56629), and netbsd/arm64. The windows/arm builder
is out to lunch, so I'm assuming that works. Since netbsd/arm and
arm64 mostly passed these tests, I've also enabled them on netbsd/386
and netbsd/amd64, where they seem to work fine as well.
Filippo Valsorda [Tue, 15 Nov 2022 17:43:56 +0000 (18:43 +0100)]
crypto/x509: add support for PKCS8/PKIX X25519 key encodings
This specifically doesn't add support for X25519 certificates.
Refactored parsePublicKey not to depend on the public PublicKeyAlgorithm
values, and ParseCertificate/ParseCertificateRequest to ignore keys that
don't have a PublicKeyAlgorithm even if parsePublicKey supports them.
Russ Cox [Wed, 16 Nov 2022 05:02:14 +0000 (00:02 -0500)]
test: fix noinit on noopt builder
Fix noopt build break from CL 450136 by not running test.
I can't reproduce the failure locally, but it's entirely reasonable
for this test to fail when optimizations are disabled, so just don't
run it when optimizations are disabled.
Russ Cox [Sun, 13 Nov 2022 14:22:35 +0000 (09:22 -0500)]
cmd/compile: handle simple inlined calls in staticinit
Global variable initializers like
var myErr error = &myError{"msg"}
have been converted to statically initialized data
from the earliest days of Go: there is no init-time
execution or allocation for that line of code.
But if the expression is moved into an inlinable function,
the static initialization no longer happens.
That is, this code has always executed and allocated
at init time, even after we added inlining to the compiler,
which should in theory make this code equivalent to
the original:
This CL makes the static initialization rewriter understand
inlined functions consisting of a single return statement,
like in this example, so that myErr2 can be implemented as
statically initialized data too, just like myErr, with no init-time
execution or allocation.
A real example of code that benefits from this rewrite is
all globally declared errors created with errors.New, like
package io
var EOF = errors.New("EOF")
Package io no longer has to allocate and initialize EOF each
time a program starts.
Another example of code that benefits is any globally declared
godebug setting (using the API from CL 449504), like
package http
var http2server = godebug.New("http2server")
These are no longer allocated and initialized at program startup either.
The list of functions that are inlined into static initializers when
compiling std and cmd (along with how many times each occurs) is:
For the cmd/go binary, this CL cuts the number of init-time
allocations from about 1920 to about 1620 (a 15% reduction).
The total executable code footprint of init functions is reduced
by 24kB, from 137kB to 113kB (an 18% reduction).
The overall binary size is reduced by 45kB,
from 15.335MB to 15.290MB (a 0.3% reduction).
(The binary size savings is larger than the executable code savings
because every byte of executable code also requires corresponding
runtime tables for unwinding, source-line mapping, and so on.)
Also merge test/sinit_run.go, which had stopped testing anything
at all as of CL 161337 (Feb 2019) and initempty.go into a new test
noinit.go.
Fixes #30820.
Change-Id: I52f7275b1ac2a0a32e22c29f9095071c7b1fac20
Reviewed-on: https://go-review.googlesource.com/c/go/+/450136 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Joedian Reid <joedian@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Russ Cox [Sat, 12 Nov 2022 17:27:33 +0000 (12:27 -0500)]
cmd/compile: do not emit a few more basic types from every compilation
We already emit types for any and func(error) string in runtime.a
but unlike the other pre-emitted types, we don't then exclude them
from being emitted in other packages. Fix that.
Also add slices of non-func types that we already emit.
Saves 0.3% of .a files in std cmd deps, computed by adding sizes from:
ls -l $(go list -export -f '{{.Export}}' -deps std cmd
The effect is small and not worth doing on its own.
The real improvement is making “what to write always in runtime”
and “what not to write in other packages” more obviously aligned.
Change-Id: Ie5cb5fd7e5a3025d2776d9b4cece775fdf92d3b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/450135 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
Keith Randall [Fri, 11 Nov 2022 23:30:10 +0000 (15:30 -0800)]
cmd/compile: be more careful about pointer incrementing in range loops
For range loops, we use a pointer to the backing store that gets
incremented on each iteration of the loop.
The problem with this scheme is that at the end of the last iteration,
we may briefly have a pointer that points past the end of the backing store
of the slice that is being iterated over. We cannot let the garbage collector
see that pointer.
To fix this problem, have the incremented pointer live briefly as
a uintptr instead of a normal pointer, so it doesn't keep anything
alive. Convert back to a normal pointer just after the loop condition
is checked, but before anything that requires a real pointer representation
(in practice, any call, which is what could cause a GC scan or stack copy).
Fixes #56699
Change-Id: Ia928d23f85a211565357603668bea4e5c534f989
Reviewed-on: https://go-review.googlesource.com/c/go/+/449995 Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Ian Lance Taylor [Tue, 8 Nov 2022 23:08:48 +0000 (15:08 -0800)]
net: rewrite nsswitch.conf parsing to work like other parsers
Seems simpler than having two different parsing mechanisms.
Change-Id: I4f8468bc025f8e03f59ec9c79b17721581b64eed
Reviewed-on: https://go-review.googlesource.com/c/go/+/448855 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Joedian Reid <joedian@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
David Chase [Wed, 2 Nov 2022 21:23:47 +0000 (17:23 -0400)]
cmd/compile: add testing-flag guard to package-is-collected assert
On advice of the department of garbage collection, forcing a garbage
collection generally does not improve performance. However,
this-data-is-now-unreachable is a good property to be able to test,
and that requires finalizers and a forced GC. So, to save build time,
this test was removed from the compiler itself, but to verify the
property, it was added to the fma_test (and the end-to-end dependence
on the flag was tested with an inserted failure in testing the
test).
TODO: also turn on the new -d=gccheck=1 debug flag on the ssacheck
builder.
Benchmarking reveals that it is profitable to avoid this GC,
with about 1.5% reduction in both user and wall time.
Russ Cox [Tue, 15 Nov 2022 14:54:39 +0000 (09:54 -0500)]
sync/atomic: allow linked list of atomic pointers again
For #56603, CL 448275 added a _ [0]T field to atomic.Pointer,
so that different kinds of atomic.Pointer are not convertible.
Unfortunately, that breaks code like:
type List struct {
Next atomic.Pointer[List]
}
which should be valid, just as using Next *List is valid.
Instead, we get:
./atomic_test.go:2533:6: invalid recursive type List
./atomic_test.go:2533:6: List refers to
./atomic_test.go:2534:13: "sync/atomic".Pointer refers to
./atomic_test.go:2533:6: List
Cuong Manh Le [Mon, 14 Nov 2022 16:11:30 +0000 (23:11 +0700)]
cmd/compile: fix missing typecheck for static initialization slice
CL 440455 fixed missing walk pass for static initialization slice.
However, slicelit may produce un-typechecked node, thus we need to do
typecheck for sinit before calling walkStmtList.
Fixes #56727
Change-Id: I40730cebcd09f2be4389d71c5a90eb9a060e4ab7
Reviewed-on: https://go-review.googlesource.com/c/go/+/450215 Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Filippo Valsorda [Sat, 15 Oct 2022 10:06:48 +0000 (12:06 +0200)]
crypto/rsa: add a test walking through every key size
We already had some tests for special cases such as PSS with 513 bit
keys. The upcoming backend rewrite also happened to crash at 63 and 504
bits for different reasons. Might as well be systematic about it.
Also, make sure SignPSS returns ErrMessageTooLong like SignPKCS1v15 when
the key is too small, instead of panicking or returning an unnamed error.
Nick Ripley [Wed, 12 Oct 2022 14:05:51 +0000 (10:05 -0400)]
runtime/pprof: scale mutex profile samples when they are recorded
Samples in the mutex profile have their count and duration scaled
according to the probability they were sampled. This is done when the
profile is actually requested. The adjustment is done using to the
current configured sampling rate. However, if the sample rate is changed
after a specific sample is recorded, then the sample will be scaled
incorrectly. In particular, if the sampling rate is changed to 0, all of
the samples in the encoded profile will have 0 count and duration. This
means the profile will be "empty", even if it should have had samples.
This CL scales the samples in the profile when they are recorded, rather
than when the profile is requested. This matches what is currently done
for the block profile.
With this change, neither the block profile nor mutex profile are scaled
when they are encoded, so the logic for scaling the samples can be
removed.
Change-Id: If228cf39284385aa8fb9a2d62492d839e02f027f
Reviewed-on: https://go-review.googlesource.com/c/go/+/443056
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Joedian Reid <joedian@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>