Inline the only use of checkMarks which also allows to drop the
always-true report argument. This also ensures the correct line gets
reported in case of an error.
Also remove the unused markTree function and drop the unused testing.T
argument from makeTree.
Change-Id: I4033d3e5ecd929d08ce03c563aa99444e102d931
Reviewed-on: https://go-review.googlesource.com/c/go/+/451615
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If a compatible trampoline has been inserted by a previously laid
function in the same section, and is known to be sufficiently close,
it can be reused.
When testing if the trampoline can be reused, the addend of the direct
call should be ignored. It is already encoded in the trampoline. If the
addend is non-zero, and the target sufficiently far away, and just
beyond direct call reach, this may cause the trampoline to be
incorrectly reused.
This was observed on go1.17.13 and openshift-installer commit f3c53b382
building in release mode with the following error:
github.com/aliyun/alibaba-cloud-sdk-go/services/cms.(*Client).DescribeMonitoringAgentAccessKeyWithChan.func1: direct call too far: runtime.duffzero+1f0-tramp0-1 -2000078
Fixes#56775
Change-Id: I54af957302506d4e3cd5d3121542c83fe980e912
Reviewed-on: https://go-review.googlesource.com/c/go/+/451415
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Than McIntosh <thanm@google.com>
The previous rule may move the phi value into a wrong block.
This CL make it only rewrite the phi value not the If block,
so that the phi value will stay in old block.
Fixes#56777
Change-Id: I9479a5c7f28529786968413d35b82a16181bb1f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/451496
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
This CL cleans up the literal pool implementation and inserts an UNDEF
instruction before the literal pool if the last instruction of the
function is not an unconditional jump instruction, RET or ERET
instruction.
Change-Id: Ifecb9e3372478362dde246c1bc9bc8d527a469d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/424134
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds a new GODEBUG flag called pagetrace that writes a
low-overhead trace of how pages of memory are managed by the Go runtime.
The page tracer is kept behind a GOEXPERIMENT flag due to a potential
security risk for setuid binaries.
Change-Id: I6f4a2447d02693c25214400846a5d2832ad6e5c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/444157
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently, we judge whether we need to fix up the branch instruction
based on Optab.type_ field, but the type_ field in optab may change.
This CL marks the branch instruction in optab, and checks whether to
do fixing up according to the mark. Depending on the constant parameter
range of the branch instruction, there are two labels, BRANCH14BITS,
BRANCH19BITS. For the 26-bit branch, linker will handle it.
Besides this CL removes the unnecessary alignment of the DWORD
instruction. Because the ISA doesn't require it and no 64-bit load
assume it. The only effect is that there is some performance penalty
for loading from DWORDs if the 8-byte DWORD instruction crosses the
cache line, but this is very rare.
Change-Id: I993902b3fb5ad8e081dd6c441e86bcf581031835
Reviewed-on: https://go-review.googlesource.com/c/go/+/424135
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
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.
Fixesgolang/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>
This makes it easier to use the package string prefix in some cases
(cleanup).
Change-Id: I0ae74bf8770999110e7d6e49eac4e42962e78596
Reviewed-on: https://go-review.googlesource.com/c/go/+/451795
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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>
When /etc/nsswitch.conf lists: "hosts: files" then LookupHost returns two nils when no entry inside /etc/hosts is found.
Change-Id: I96d68a079dfe009655c84cf0e697ce19a5bb6698
GitHub-Last-Rev: 894f066bbc
GitHub-Pull-Request: golang/go#56747
Reviewed-on: https://go-review.googlesource.com/c/go/+/450875
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
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.
Fixes#33565.
Fixes#56784.
[1] https://opensource.apple.com/source/Libnotify/Libnotify-241/notify_client.c.auto.html
Change-Id: I16a14a800893c40244678203532a3e8d6214b6bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/451735
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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.
Change-Id: I0aee75cc608a152adad41c51120b661a6c542283
Reviewed-on: https://go-review.googlesource.com/c/go/+/450915
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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.)
Change-Id: I535ae9cd6b679609e247c3d0d9ee572c1a1187cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/450737
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
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>
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>
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>
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>
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.
Fixes#56798
Change-Id: I416e0d8c3aa11ff44e9870755efa95c74d1013f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/451656
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
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>
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>
testenv.Command sets a default timeout based on the test's deadline
and sends SIGQUIT (where supported) in case of a hang.
Change-Id: I32ea9ca11c30d8af3d5490f2db1674314962cc80
Reviewed-on: https://go-review.googlesource.com/c/go/+/451195
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Cl 426334 removed its only usage, and now we have gcflags_noopt.
Change-Id: I3b33a8c868669deea00bf6dfcf8d81981504e293
Reviewed-on: https://go-review.googlesource.com/c/go/+/451255
Reviewed-by: Joedian Reid <joedian@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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.
Fixes#56730.
Change-Id: I445b1d8a1b910208d0608b2186881746adb550e0
GitHub-Last-Rev: 67b1102b5e
GitHub-Pull-Request: golang/go#56754
Reviewed-on: https://go-review.googlesource.com/c/go/+/450936
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Given code like
func itou(i int) uint { return uint(i) }
var x = itou(-1)
the static inliner from CL 450136 was rewriting the code to
var x = uint(-1)
which is not valid Go code. Fix this by converting the
constants appropriately during inlining.
Fixes golang.org/x/image/vector test.
Change-Id: I13448df8504c6a70525b1cdc36e2c947e22cdd33
Reviewed-on: https://go-review.googlesource.com/c/go/+/451376
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
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.
This should fix the darwin-amd64-race builder.
Updates #37486.
Change-Id: I554bb60bc729dbb6f1bc926f1ea329768b0d6d81
Reviewed-on: https://go-review.googlesource.com/c/go/+/451437
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Austin Clements <austin@google.com>
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.
Change-Id: I2eed4098736e1cb285ca20bc248b0ab3515f0dea
Reviewed-on: https://go-review.googlesource.com/c/go/+/451221
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Don't test IsLocal("nul.txt"), since whether this name is reserved
or not is version-dependent.
Change-Id: Ifff3edc77454e052080e325871c08bbba49e692c
Reviewed-on: https://go-review.googlesource.com/c/go/+/451222
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
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>
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#25849Fixes#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>
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>
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".
For #55022.
Change-Id: Iab7974ab8932daf0e83506de505e044a8e412466
Reviewed-on: https://go-review.googlesource.com/c/go/+/438737
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
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.
For #51473.
Updates #51461.
Change-Id: Ib9d1ea93f26d30daa824d75ed634eaf530af086d
Reviewed-on: https://go-review.googlesource.com/c/go/+/450714
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
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.
Change-Id: I491e8a022fe3a64022e9dc593850d483a0d353fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/450739
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
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.
Change-Id: Iee24cc67900eb542141b32beaadafb2c94f5fe26
Reviewed-on: https://go-review.googlesource.com/c/go/+/451055
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
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>
This adds support for host tests to goTest and registerTest and
modifies all uses of registerHostTest to use goTest and registerTest.
This eliminates the last case where go test command lines are
constructed by hand. Next we'll clean up all of the infrastructure
support for that.
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).
Preparation for #37486.
Change-Id: Icb7ec8efdac72bdb819ae24b2f585375d9d9d5b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/450019
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>
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>
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>
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>
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>
