Generate a CALLIND relocation only for indirect calls, not for
indirect jumps. In particular, the RET instruction is lowered to
JMP (LR), an indirect jump, and occurs frequently. The large
amount of spurious relocations causes the linker to do a lot of
extra work.
Change-Id: Ie0edc04609788f5a687fd00c22558c3f83867697
Reviewed-on: https://go-review.googlesource.com/c/go/+/227079
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: David Chase <drchase@google.com>
Change-Id: I93784e9a9efdd1531e3c342aa0899bf059da0ae1
Reviewed-on: https://go-review.googlesource.com/c/go/+/226983 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Thu, 2 Apr 2020 19:00:50 +0000 (12:00 -0700)]
runtime/race: update some .syso files
Update race detector syso files for some platforms.
There's still 2 more to do, but they might take a while so I'm
mailing the ones I have now.
Note: some arm64 tests did not complete successfully due to out
of memory errors, but I suspect the .syso is correct.
Update #14481
Update #37485 (I think?)
Update #37355
Change-Id: I7e7e707a1fd7574855a538ba89dc11acc999c760
Reviewed-on: https://go-review.googlesource.com/c/go/+/226981
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitri Shuralyov [Sun, 29 Mar 2020 03:10:20 +0000 (23:10 -0400)]
net/http: release callbacks after fetch promise completes
When the request context was canceled, the Transport.RoundTrip method
could return before the fetch promise resolved. This would cause the
success and failure callback functions to get called after they've
been released, which in turn prints a "call to released function"
error to the console.
Avoid that problem by releasing the callbacks after the fetch promise
completes, by moving the release calls into the callbacks themselves.
This way we can still return from the Transport.RoundTrip method as
soon as the context is canceled, without waiting on the promise to
resolve. If the AbortController is unavailable and it's not possible to
abort the fetch operation, the promise may take a long time to resolve.
For #38003.
Change-Id: Ied1475e31dcba101b3326521b0cd653dbb345e1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/226204 Reviewed-by: Johan Brandhorst <johan.brandhorst@gmail.com> Reviewed-by: Richard Musiol <neelance@gmail.com>
net: update ParseIP doc to say IPv4-mapped-IPv6 is supported
Change-Id: I49a79c07081cd8f12a3ffef21fd02a9a622a7eb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/226979 Reviewed-by: Ian Lance Taylor <iant@golang.org>
In the dev.link branch we continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker .
The new object file is index-based and provides random access.
The linker maps the object files into read-only memory, and
accesses symbols on-demand using indices, as opposed to reading
all object files sequentially into the heap with the old format.
This work is not done yet. Currently we still convert back to the
old in-memory representation half way through the link process,
but only for symbols that are needed.
At this point, we think it is ready to enable the new object
files and new linker for early testing. Using the new object
files and the new linker, it reduces the linker's memory usage by
~10% and wall-clock run time by ~5%, and more to come.
Currently, both the old and new object file formats are supported.
The new format and new linker are used by default. For feature
gating, as a fallback, the old format and old linker can be used
by setting the compiler/assembler/linker's -go115newobj flag to
false. Note that the flag needs to be specified consistently to
all compilations, i.e.
[dev.link] all: merge branch 'master' into dev.link
The only conflict is a modify-deletion conflict in
cmd/link/internal/ld/link.go, where the old error reporter is
deleted in the new linker. Ported to
cmd/link/internal/ld/errors.go.
crypto/rsa: refactor RSA-PSS signing and verification
Cleaned up for readability and consistency.
There is one tiny behavioral change: when PSSSaltLengthEqualsHash is
used and both hash and opts.Hash were set, hash.Size() was used for the
salt length instead of opts.Hash.Size(). That's clearly wrong because
opts.Hash is documented to override hash.
This ports CL 226997 to the dev.link branch.
- The assembler part and old object file writing are unchanged.
- Changes to cmd/link are applied to cmd/oldlink.
- Add alignment field to new object files for the new linker.
Change-Id: Id00f323ae5bdd86b2709a702ee28bcaa9ba962f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/227025 Reviewed-by: Than McIntosh <thanm@google.com>
Joel Sing [Tue, 25 Feb 2020 15:48:24 +0000 (02:48 +1100)]
cmd/link: skip symbol references when looking for missing symbols
ErrorUnresolved attempts to find the missing symbol in another ABI,
in order to provide more friendly error messages. However, in doing so
it checks the same ABI and can find the symbol reference for the symbol
that it is currently reporting the unresolved error for. Avoid this by
ignoring SXREF symbols, which is the same behaviour used when linking
is performed.
fanzha02 [Thu, 19 Dec 2019 08:15:06 +0000 (08:15 +0000)]
cmd/asm: align an instruction or a function's address
Recently, the gVisor project needs an instruction's address
with 128 bytes alignment to fit the architecture requirement
for interrupt table.
This patch allows aligning an instruction's address to be
aligned to a specific value (2^n and in the range [8, 2048])
The main changes include:
1. Adds a new element in the FuncInfo structure defined in
cmd/internal/obj/link.go file to record the alignment
information.
2. Adds a new element in the Func structure defined in
cmd/internal/goobj/read.go file to read the alignment
information.
3. Adds the assembler support to align an intruction's offset
with a specific value (2^n and in the range [8, 2048]).
e.g. "PCALIGN $256" indicates that the next instruction should
be aligned to 256 bytes.
4. An instruction's alignment is relative to the start of the
function where this instruction is located, so the function's
address must be aligned to the same or coarser boundary.
Michael Munday [Wed, 1 Apr 2020 10:21:03 +0000 (03:21 -0700)]
cmd/compile: mark 'store multiple' as clobbering flags on s390x
Store multiple instructions can clobber flags on s390x when the
offset passed into the assembler is outside the range representable
with a signed 20 bit integer. This is because the assembler uses
the agfi instruction to implement the large offset. The assembler
could use a different sequence of instructions, but for now just
mark the instruction as 'clobberFlags' since this is risk free.
Noticed while investigating #38195.
No test yet since I'm not sure how to get this bug to trigger and
I haven't seen it affect real code.
Change-Id: I4a6ab96455a3ef8ffacb76ef0166b97eb40ff925
Reviewed-on: https://go-review.googlesource.com/c/go/+/226759
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Alex Brainman [Sat, 7 Mar 2020 00:08:06 +0000 (11:08 +1100)]
internal/syscall/windows: change WSAMsg.Name type
The problem was discovered while running
go test -a -short -gcflags=all=-d=checkptr -run=TestUDPConnSpecificMethods net
WSAMsg is type defined by Windows. And WSAMsg.Name could point to two
different structures for IPv4 and IPV6 sockets.
Currently WSAMsg.Name is declared as *syscall.RawSockaddrAny. But that
violates
(1) Conversion of a *T1 to Pointer to *T2.
rule of
https://golang.org/pkg/unsafe/#Pointer
When we convert *syscall.RawSockaddrInet4 into *syscall.RawSockaddrAny,
syscall.RawSockaddrInet4 and syscall.RawSockaddrAny do not share an
equivalent memory layout.
Same for *syscall.SockaddrInet6 into *syscall.RawSockaddrAny.
This CL changes WSAMsg.Name type to *syscall.Pointer. syscall.Pointer
length is 0, and that at least makes type checker happy.
After this change I was able to run
go test -a -short -gcflags=all=-d=checkptr std cmd
without type checker complaining.
Updates #34972
Change-Id: Ic5c2321c20abd805c687ee16ef6f643a2f8cd93f
Reviewed-on: https://go-review.googlesource.com/c/go/+/222457
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
We can initialize the runtime sigqueue packages on first use.
We don't require an explicit initialization step. So, remove it.
Change-Id: I484e02dc2c67395fd5584f35ecda2e28b37168df
Reviewed-on: https://go-review.googlesource.com/c/go/+/226540
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
DWARF constant symbols were always marked and converted to
sym.Symbols when DWARF generation uses sym.Symbols. Now that the
DWARF generation uses the loader, no need to force-mark them.
Change-Id: Ia4032430697cfa901fb4b6d106a483973277ea0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/226803 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org>
[dev.link] cmd/oldlink: decouple from goobj2 package
The new object file support in the old linker should not be used.
This is a minimal change that removes stuff from the old linker's
loader package, so that it decouples from the goobj2 package,
allowing the latter to evolve.
Keep the change local in the loader package, so most of the old
linker doesn't need to change. At this point I don't think we
want to make significant changes to the old linker.
Change-Id: I078c4cbb35dc4627c4b82f512a4aceec9b594925
Reviewed-on: https://go-review.googlesource.com/c/go/+/226800 Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
[dev.link] cmd/internal/goobj2: change StringRef encoding for better locality
Previously, StringRef is encoded as an offset pointing to
{ len, [len]byte }. This CL changes it to { len, offset }, where
offset points the bytes.
With the new format, reading a string header is just reading two
adjacent uint32s, without accessing the string table. This should
improve locality of object file reading.
Change-Id: Iec30708f9d9adb2f0242db6c4767c0f8e730f4df
Reviewed-on: https://go-review.googlesource.com/c/go/+/226797
Run-TryBot: Cherry Zhang <cherryyz@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Keith Randall [Tue, 3 Mar 2020 18:07:32 +0000 (18:07 +0000)]
reflect: when Converting between float32s, don't lose signal NaNs
Trying this CL again, with a test that skips 387.
When converting from float32->float64->float32, any signal NaNs
get converted to quiet NaNs. Avoid that so using reflect.Value.Convert
between two float32 types keeps the signal bit of NaNs.
Skip the test on 387. I don't see any sane way of ensuring that a
float load + float store is faithful on that platform.
Bradford Lamson-Scribner [Sun, 29 Mar 2020 19:17:46 +0000 (13:17 -0600)]
cmd/compile: combine ssa.html columns with identical contents
Combine columns in ssa.html output if they are identical. There
can now be multiple titles per column which are all clickable to
expand and collapse their column. Give collapsed columns some
padding for better readability. Some of the work in this CL was
started by Josh Bleecher Snyder and mailed to me in order to
continue to completion.
Updates #37766
Change-Id: I313b0917dc1bafe1eb99d91798ea915e5bcfaae9
Reviewed-on: https://go-review.googlesource.com/c/go/+/226209 Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
cmd/compile, runtime: use more registers for amd64 write barrier calls
The compiler-inserted write barrier calls use a special ABI
for speed and to minimize the binary size impact.
runtime.gcWriteBarrier takes its args in DI and AX.
This change adds gcWriteBarrier wrapper functions,
varying only in the register used for the second argument.
(Allowing variation in the first argument doesn't offer improvements,
which is convenient, as it avoids quadratic API growth.)
This reduces the number of register copies.
The goals are reduced binary size via reduced register pressure/copies.
One downside to this change is that when the write barrier is on,
we may bounce through several different write barrier wrappers,
which is bad for the instruction cache.
Package runtime write barrier benchmarks for this change:
name old time/op new time/op delta
WriteBarrier-8 16.6ns ± 6% 15.6ns ± 6% -5.73% (p=0.000 n=97+99)
BulkWriteBarrier-8 4.37ns ± 7% 4.22ns ± 8% -3.45% (p=0.000 n=96+99)
However, I don't particularly trust these numbers.
I ran runtime.BenchmarkWriteBarrier multiple times as I rebased
this change, and noticed that the results have high variance
depending on the parent change, perhaps due to aligment.
This change was stress tested with GOGC=1 GODEBUG=gccheckmark=1 go test std.
In CL 187657, I refactored constant conversion logic without realizing
that conversions between int/float and complex types are allowed for
constants (assuming the constant values are representable by the
destination type), but are never allowed for non-constant expressions.
This CL expands convertop to take an extra srcConstant parameter to
indicate whether the source expression is a constant; and if so, to
allow any numeric-to-numeric conversion. (Conversions of values that
cannot be represented in the destination type are rejected by
evconst.)
Fixes #38117.
Change-Id: Id7077d749a14c8fd910be38da170fa5254819f2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/226197
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Jay Conrod [Thu, 12 Mar 2020 22:56:40 +0000 (18:56 -0400)]
cmd/go: add support for GOPROXY fallback on unexpected errors
URLs in GOPROXY may now be separated with commas (,) or pipes (|). If
a request to a proxy fails with any error (including connection errors
and timeouts) and the proxy URL is followed by a pipe, the go command
will try the request with the next proxy in the list. If the proxy is
followed by a comma, the go command will only try the next proxy if
the error a 404 or 410 HTTP response.
The go command will determine how to connect to the checksum database
using the same logic. Before accessing the checksum database, the go
command sends a request to <proxyURL>/sumdb/<sumdb-name>/supported.
If a proxy responds with 404 or 410, or if any other error occurs and
the proxy URL in GOPROXY is followed by a pipe, the go command will
try the request with the next proxy. If all proxies respond with 404
or 410 or are configured to fall back on errors, the go command will
connect to the checksum database directly.
This CL does not change the default value or meaning of GOPROXY.
Fixes #37367
Change-Id: I35dd218823fe8cb9383e9ac7bbfec2cc8a358748
Reviewed-on: https://go-review.googlesource.com/c/go/+/226460
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Mon, 30 Mar 2020 19:43:08 +0000 (15:43 -0400)]
os/signal: make TestStop resilient to initially-blocked signals
For reasons unknown, SIGUSR1 appears to be blocked at process start
for tests on the android-arm-corellium and android-arm64-corellium
builders. (This has been observed before, too: see CL 203957.)
Make the test resilient to blocked signals by always calling Notify
and waiting for potential signal delivery after sending any signal
that is not known to be unblocked.
Also remove the initial SIGWINCH signal from testCancel. The behavior
of an unhandled SIGWINCH is already tested in TestStop, so we don't
need to re-test that same case: waiting for an unhandled signal takes
a comparatively long time (because we necessarily don't know when it
has been delivered), so this redundancy makes the overall test binary
needlessly slow, especially since it is called from both TestReset and
TestIgnore.
Since each signal is always unblocked while we have a notification
channel registered for it, we don't need to modify any other tests:
TestStop and testCancel are the only functions that send signals
without a registered channel.
Fixes #38165
Updates #33174
Updates #15661
Change-Id: I215880894e954b62166024085050d34323431b63
Reviewed-on: https://go-review.googlesource.com/c/go/+/226461
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Richard Miller [Tue, 31 Mar 2020 18:44:19 +0000 (19:44 +0100)]
runtime: skip gdb tests on Plan 9
There's no gdb on Plan 9.
Change-Id: Ibeb0fbd3c096a69181c19b1fb2bc6291612b6da3
Reviewed-on: https://go-review.googlesource.com/c/go/+/226657 Reviewed-by: David du Colombier <0intro@gmail.com>
Bryan C. Mills [Mon, 30 Mar 2020 18:04:08 +0000 (14:04 -0400)]
context: fix a flaky timeout in TestLayersTimeout
In CL 223019, I reduced the short timeout in the testLayers helper to
be even shorter than it was. That exposed a racy (time-dependent)
select later in the function, which failed in one of the slower
builders (android-386-emu).
Also streamline the test to make it easier to test with a very high -count flag:
- Run tests that sleep for shortDuration in parallel to reduce latency.
- Use shorter durations in examples to reduce test running time.
- Avoid mutating global state (in package math/rand) in testLayers.
After this change (but not before it),
'go test -run=TestLayersTimeout -count=100000 context' passes on my workstation.
Fixes #38161
Change-Id: Iaf4abe7ac308b2100d8828267cda9f4f8ae4be82
Reviewed-on: https://go-review.googlesource.com/c/go/+/226457 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Per RFC 8017, reject signatures which are not the same length as the RSA
modulus. This matches the behavior of SignPKCS1v15 which properly left pads
the signatures it generates to the size of the modulus.
Cherry Zhang [Fri, 27 Mar 2020 20:32:22 +0000 (16:32 -0400)]
[dev.link] cmd/link: set attributes atomically
Now concurrent relocsym may access symbols attributes
concurrently, causing data race when using the race detector. I
think it is still safe as we read/write on different bits, and
not write the same symbol's attributes from multiple goroutines,
so it will always reads the right value regardless whether the
write happens before or after, as long as the memory model is not
so insane.
Use atomic accesses to appease the race detector. It doesn't seem
to cost much, at least on x86.
Joel Sing [Mon, 30 Mar 2020 14:57:52 +0000 (01:57 +1100)]
cmd/asm,cmd/internal/obj/riscv: provide branch pseudo-instructions
Implement various branch pseudo-instructions for riscv64. These make it easier
to read/write assembly and will also make it easier for the compiler to generate
optimised code.
fanzha02 [Thu, 19 Dec 2019 08:15:06 +0000 (08:15 +0000)]
cmd/asm: align an instruction or a function's address
Recently, the gVisor project needs an instruction's address
with 128 bytes alignment and a function's start address with
2K bytes alignment to fit the architecture requirement for
interrupt table.
This patch allows aligning the address of an instruction to be
aligned to a specific value (2^n and not higher than 2048) and
the address of a function to be 2048 bytes.
The main changes include:
1. Adds ALIGN2048 flag to align a function's address with
2048 bytes.
e.g. "TEXT ·Add(SB),NOSPLIT|ALIGN2048" indicates that the
address of Add function should be aligned to 2048 bytes.
2. Adds a new element in the FuncInfo structure defined in
cmd/internal/obj/link.go file to record the alignment
information.
3. Adds a new element in the Func structure defined in
cmd/internal/goobj/read.go file to read the alignment
information.
4. Because go introduces a new object file format, also add
a new element in the FuncInfo structure defined in
cmd/internal/goobj2/funcinfo.go to record the alignment
information.
5. Adds the assembler support to align an intruction's offset
with a specific value (2^n and not higher than 2048).
e.g. "PCALIGN $256" indicates that the next instruction should
be aligned to 256 bytes.
This CL also adds a test.
Change-Id: I31cfa6fb5bc35dee2c44bf65913e90cddfcb492a
Reviewed-on: https://go-review.googlesource.com/c/go/+/212767 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills [Mon, 30 Mar 2020 19:01:33 +0000 (15:01 -0400)]
os/signal: in TestStop, skip the final "unexpected signal" check for SIGUSR1 on Android
In CL 226138, I updated TestStop to have more uniform behavior for its signals.
However, that test seems to always fail for SIGUSR1 on the Android ARM builders.
I'm not sure what's special about Android for this particular case,
but let's skip the test to unbreak the builders while I investigate.
For #38165
Updates #33174
Change-Id: I35a70346cd9757a92acd505a020bf95e6871405c
Reviewed-on: https://go-review.googlesource.com/c/go/+/226458
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jeremy Faller [Tue, 17 Mar 2020 14:24:40 +0000 (10:24 -0400)]
[dev.link] cmd/link: parallelize asmb on amd64
Introduces a parallel OutBuf implementation, and POC on amd64. Due to
some of the weird behaviors I saw on MacOS (SIGBUS while calling msync),
I will wait for feedback to port to other architectures.
On my mac, sped up Asmb by ~78% for cmd/compile (below). Will likely
have an appreciable speedup on kubelet benchmark.
Giovanni Bajo [Sun, 29 Mar 2020 12:21:12 +0000 (14:21 +0200)]
cmd/compile: avoid zero extensions after 32-bit shifts
zeroUpper32Bits wasn't checking for shift-extension ops. This would not
check shifts that were marking as bounded by prove (normally, shifts
are wrapped in a sequence that ends with an ANDL, and zeroUpper32Bits
would see the ANDL).
This produces no changes on generated output right now, but will be
important once CL196679 lands because many shifts will be marked
as bounded, and lower will stop generating the masking code sequence
around them.
Change-Id: Iaea94acc5b60bb9a5021c9fb7e4a1e2e5244435e
Reviewed-on: https://go-review.googlesource.com/c/go/+/226338 Reviewed-by: Keith Randall <khr@golang.org>
Than McIntosh [Mon, 30 Mar 2020 14:06:54 +0000 (10:06 -0400)]
[dev.link] cmd/link: move the wavefront past addexport()
Reorganize the linker phase ordering so that addexport() runs before
loadlibfull. In previous CLs addexport() was changed to use loader
APIs but then copy back its work into sym.Symbol, so this change
removes the copying/shim code in question.
Tobias Klauser [Sun, 29 Mar 2020 22:38:09 +0000 (00:38 +0200)]
cmd/cgo, misc/cgo: only cache anonymous struct typedefs with parent name
CL 181857 broke the translation of certain C types using cmd/cgo -godefs
because it stores each typedef, array and qualified type with their
parent type name in the translation cache.
Fix this by only considering the parent type for typedefs of anonymous
structs which is the only case where types might become ambiguous.
Updates #31891
Fixes #37479
Fixes #37621
Change-Id: I301a749ec89585789cb0d213593bb8b7341beb88
Reviewed-on: https://go-review.googlesource.com/c/go/+/226341
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Than McIntosh [Mon, 30 Mar 2020 14:05:23 +0000 (10:05 -0400)]
[dev.link] cmd/link: run setArchSyms earlier
Add some new loader.Sym equivalents to the archSyms struct so that we
can run setArchSyms earlier in the pipeline, and add a "mode" to
setArchSyms to control whether it should create loader.Sym symbols or
their *sym.Symbol equivalents.
These change needed for a subsequent patch in which addexport() is run
earlier as well
Bryan C. Mills [Fri, 27 Mar 2020 19:04:09 +0000 (15:04 -0400)]
os/signal: rework test timeouts and concurrency
Use a uniform function (named “quiesce”) to wait for possible signals
in a way that gives the kernel many opportunities to deliver them.
Simplify channel usage and concurrency in stress tests.
Use (*testing.T).Deadline instead of parsing the deadline in TestMain.
In TestStop, sleep forever in a loop if we expect the test to die from
a signal. That should reduce the flakiness of TestNohup, since
TestStop will no longer spuriously pass when run as a subprocess of
TestNohup.
Since independent signals should not interfere, run the different
signals in TestStop in parallel when testing in short mode.
Since TestNohup runs TestStop as a subprocess, and TestStop needs to
wait many times for signals to quiesce, run its test subprocesses
concurrently and in short mode — reducing the latency of that test by
more than a factor of 2.
The above two changes reduce the running time of TestNohup on my
workstation to ~345ms, making it possible to run much larger counts of
the test in the same amount of wall time. If the test remains flaky
after this CL, we can spend all or part of that latency improvement on
a longer settle time.
Updates #33174
Change-Id: I09206f213d8c1888b50bf974f965221a5d482419
Reviewed-on: https://go-review.googlesource.com/c/go/+/226138
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Lynn Boger [Wed, 25 Mar 2020 17:47:43 +0000 (13:47 -0400)]
cmd/objdump: add support for -gnu option on Go objdump
This adds support for the -gnu option on Go objdump. When
this option is used, then output will include gnu
assembly in comments alongside the Go assembly.
The objdump test was updated to test this new option.
This option is supported for the arches found in
golang.org/x that provide the GNUsyntax function.
Michael Anthony Knyszek [Sat, 28 Mar 2020 16:11:15 +0000 (16:11 +0000)]
runtime: check the correct sanity condition in the page allocator
Currently there are a few sanity checks in the page allocator which
should fail immediately but because it's a check for a negative number
on a uint, it's actually dead-code.
If there's a bug in the page allocator which would cause the sanity
check to fail, this could cause memory corruption by returning an
invalid address (more precisely, one might either see a segfault, or
span overlap).
This change fixes these sanity checks to check the correct condition.
Fixes #38130.
Change-Id: Ia19786cece783d39f26df24dec8788833a6a3f21
Reviewed-on: https://go-review.googlesource.com/c/go/+/226297 Reviewed-by: Giovanni Bajo <rasky@develer.com> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Michał Łowicki [Sun, 29 Mar 2020 16:59:08 +0000 (17:59 +0100)]
doc: fix path to make.bash
Change-Id: I78c7197b8b93590470a782b492bba177a14d80ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/226340 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Theophanes [Wed, 18 Mar 2020 17:03:51 +0000 (10:03 -0700)]
database/sql: add test for Conn.Validator interface
This addresses comments made by Russ after
https://golang.org/cl/174122 was merged. It addes a test
for the connection validator and renames the interface to just
"Validator".
Giovanni Bajo [Sat, 21 Mar 2020 12:22:14 +0000 (13:22 +0100)]
doc: decrease prominence of GOROOT_BOOTSTRAP
Go build scripts on UNIX (make.bash, all.bash) have not required
GOROOT_BOOTSTRAP since August 2017 (CL 57753). Windows build scripts
have followed suit since CL 96455. Most people building Go will have
a Go toolchain in their PATH and will not need to specify a different
toolchain.
This CL removes the GOROOT_BOOTSTRAP mention from the contribution guide
(it was there for Windows only, but it's not required anymore). The guide
is meant to be light and clear for beginners and is not supposed to be
a reference, so there's not need to keep mentioning GOROOT_BOOTSTRAP.
Also update install-source.html to reflect the current status quo,
where using the PATH is probably the first and most used default, and
GOROOT_BOOTSTRAP is just an option.
Change-Id: Iab453e61b0c749c256aaaf81ea9b2ae58822cb89
Reviewed-on: https://go-review.googlesource.com/c/go/+/224717
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Michael Matloob [Thu, 27 Feb 2020 22:14:07 +0000 (17:14 -0500)]
cmd/go: avoid needing to manipulate ImportStack when constructing error
Simplify the printing of PackageErrors by pushing and popping packages
from the import stack when creating the error, rather than when printing
the error. In some cases, we don't have the same amount of information
to recreate the exact error, so we'll print the name of the package
the error is for, even when it's redundant. In the case of import cycle
errors, this change results in the addition of the position information
of the error.
This change supercedes CLs 220718 and 217106. It introduces a simpler
way to format errors.
Fixes #36173
Change-Id: Ie27011eb71f82e165ed4f9567bba6890a3849fc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/224660
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Thu, 26 Mar 2020 16:13:11 +0000 (09:13 -0700)]
runtime: lock mtxpoll in AIX netpollBreak
netpollBreak calls netpollwakeup, and netpollwakeup expects the mtxpoll
lock to be held, so that it has exclusive access to pendingUpdates.
Not acquiring the lock was a mistake in CL 171824. Fortunately it
rarely matters in practice.
Change-Id: I32962ec2575c846ef3d6a91a4d821b2ff02d983c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225618 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Keith Randall [Tue, 24 Mar 2020 20:39:44 +0000 (13:39 -0700)]
cmd/compile: convert 386 port to use addressing modes pass (take 2)
Retrying CL 222782, with a fix that will hopefully stop the random crashing.
The issue with the previous CL is that it does pointer arithmetic
in a way that may briefly generate an out-of-bounds pointer. If an
interrupt happens to occur in that state, the referenced object may
be collected incorrectly.
Suppose there was code that did s[x+c]. The previous CL had a rule
to the effect of ptr + (x + c) -> c + (ptr + x). But ptr+x is not
guaranteed to point to the same object as ptr. In contrast,
ptr+(x+c) is guaranteed to point to the same object as ptr, because
we would have already checked that x+c is in bounds.
For example, strconv.trim used to have this code:
MOVZX -0x1(BX)(DX*1), BP
CMPL $0x30, AL
After CL 222782, it had this code:
LEAL 0(BX)(DX*1), BP
CMPB $0x30, -0x1(BP)
An interrupt between those last two instructions could see BP pointing
outside the backing store of the slice involved.
It's really hard to actually demonstrate a bug. First, you need to
have an interrupt occur at exactly the right time. Then, there must
be no other pointers to the object in question. Since the interrupted
frame will be scanned conservatively, there can't even be a dead
pointer in another register or on the stack. (In the example above,
a bug can't happen because BX still holds the original pointer.)
Then, the object in question needs to be collected (or at least
scanned?) before the interrupted code continues.
This CL needs to handle load combining somewhat differently than CL 222782
because of the new restriction on arithmetic. That's the only real
difference (other than removing the bad rules) from that old CL.
This bug is also present in the amd64 rewrite rules, and we haven't
seen any crashing as a result. I will fix up that code similarly to
this one in a separate CL.
Michael Pratt [Thu, 26 Mar 2020 19:10:21 +0000 (15:10 -0400)]
runtime/pprof: increment fake overflow record PC
gentraceback generates PCs which are usually following the CALL
instruction. For those that aren't, it fixes up the PCs so that
functions processing the output can unconditionally decrement the PC.
runtime_expandInlineFrames does this unconditional decrement when
looking up the function. However, the fake stack frame generated for
overflow records fails to meet the contract, and decrementing the PC
results in a PC in the previous function. If that function contains
inlined call, runtime_expandInlineFrames will not short-circuit and will
panic trying to look up a PC that doesn't exist.
Note that the added test does not fail at HEAD. It will only fail (with
a panic) if the function preceeding lostProfileEvent contains inlined
function calls. At the moment (on linux/amd64), that is
runtime/pprof.addMaxRSS, which does not.
Fixes #38096
Change-Id: Iad0819f23c566011c920fd9a5b1254719228da0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/225661 Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills [Fri, 27 Mar 2020 16:11:21 +0000 (12:11 -0400)]
net/http: remove arbitrary timeouts from TestIdentityResponse and TestTLSHandshakeTimeout
These hard-coded timeouts make the tests flaky on slow builders (such
as solaris-amd64-oraclerel), and make test failures harder to diagnose
anyway (by replacing dumps of the stuck goroutine stacks with failure
messages that do not describe the stuck goroutines). Eliminate them
and simplify the tests.
Andy Pan [Fri, 27 Mar 2020 03:21:17 +0000 (03:21 +0000)]
runtime: converge duplicate calls to netpollBreak into one
There might be some concurrent (maybe not concurrent, just sequential but in a short time window) and duplicate calls to `netpollBreak`, trying to wake up a net-poller. If one has called `netpollBreak` and that waking event hasn't been received by epollwait/kevent/..., then the subsequent calls of `netpollBreak` ought to be ignored or in other words, these calls should be converged into one.
Lynn Boger [Thu, 26 Mar 2020 20:01:40 +0000 (16:01 -0400)]
cmd/compile: add rules to eliminate unnecessary signed shifts
This change to the rules removes some unnecessary signed shifts
that appear in the math/rand functions. Existing rules did not
cover some of the signed cases.
A little improvement seen in math/rand due to removing 1 of 2
instructions generated for Int31n, which is inlined quite a bit.
Bryan C. Mills [Fri, 27 Mar 2020 03:10:32 +0000 (23:10 -0400)]
cmd/go/internal/work: disallow testgo binary from installing to GOROOT
Installing to GOROOT makes tests non-parallelizable, since each test
depends on the installed contents of GOROOT already being up-to-date
and may reasonably assume that those contents do not change over the
course of the test.
Fixes #37573
Updates #30316
Change-Id: I2afe95ad11347bee3bb7c2d77a657db6d691cf05
Reviewed-on: https://go-review.googlesource.com/c/go/+/225897 Reviewed-by: Michael Matloob <matloob@golang.org>
Bryan C. Mills [Tue, 17 Mar 2020 14:22:39 +0000 (10:22 -0400)]
run.{bash,bat,rc}: use ../bin/go instead of the go binary in $PATH
https://golang.org/doc/contribute.html#quick_test currently suggests
running 'make.bash' and 'run.bash' separately, but 'run.bash'
potentially uses a 'go' command resolved from the wrong GOROOT,
which in turn sets the wrong GOROOT for further commands.
Bryan C. Mills [Thu, 26 Mar 2020 16:00:07 +0000 (12:00 -0400)]
cmd/dist: skip API check on plan9 builders
The plan9-arm builder has a very slow filesystem and frequently times
out on this test. The api check verifies the API for all supported
GOOS/GOARCH/CGO_ENABLED combination anyway, so if we skip it on one
builder (or even most builders) there should be no loss of coverage.
Updates #37951
Change-Id: I86a93df2ec60a6af6d942e3954eef09ce67bb39e
Reviewed-on: https://go-review.googlesource.com/c/go/+/225662 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>