Michael Anthony Knyszek [Tue, 10 Sep 2019 18:53:51 +0000 (18:53 +0000)]
runtime: count scavenged bits for new allocation for new page allocator
This change makes it so that the new page allocator returns the number
of pages that are scavenged in a new allocation so that mheap can update
memstats appropriately.
The accounting could be embedded into pageAlloc, but that would make
the new allocator more difficult to test.
Michael Anthony Knyszek [Wed, 21 Aug 2019 00:24:25 +0000 (00:24 +0000)]
runtime: add scavenging code for new page allocator
This change adds a scavenger for the new page allocator along with
tests. The scavenger walks over the heap backwards once per GC, looking
for memory to scavenge. It walks across the heap without any lock held,
searching optimistically. If it finds what appears to be a scavenging
candidate it acquires the heap lock and attempts to verify it. Upon
verification it then scavenges.
Notably, unlike the old scavenger, it doesn't show any preference for
huge pages and instead follows a more strict last-page-first policy.
Michael Anthony Knyszek [Wed, 14 Aug 2019 16:32:12 +0000 (16:32 +0000)]
runtime: add new page allocator core
This change adds a new bitmap-based allocator to the runtime with tests.
It does not yet integrate the page allocator into the runtime and thus
this change is almost purely additive.
Michael Anthony Knyszek [Mon, 4 Nov 2019 20:01:18 +0000 (20:01 +0000)]
runtime: make sysReserve return page-aligned memory on js-wasm
This change ensures js-wasm returns page-aligned memory. While today
its lack of alignment doesn't cause problems, this is an invariant of
sysAlloc which is documented in HACKING.md but isn't upheld by js-wasm.
Any code that calls sysAlloc directly for small structures expects a
certain alignment (e.g. debuglog, tracebufs) but this is not maintained
by js-wasm's sysAlloc.
Where sysReserve comes into play is that sysAlloc is implemented in
terms of sysReserve on js-wasm. Also, the documentation of sysReserve
says that the returned memory is "OS-aligned" which on most platforms
means page-aligned, but the "OS-alignment" on js-wasm is effectively 1,
which doesn't seem right either.
The expected impact of this change is increased memory use on wasm,
since there's no way to decommit memory, and any small structures
allocated with sysAlloc won't be packed quite as tightly. However, any
memory increase should be minimal. Most calls to sysReserve and sysAlloc
already aligned their request to physPageSize before calling it; there
are only a few circumstances where this is not true, and they involve
allocating an amount of memory returned by unsafe.Sizeof where it's
actually quite important that we get the alignment right.
Updates #35112.
Change-Id: I9ca171e507ff3bd186326ccf611b35b9ebea1bfe
Reviewed-on: https://go-review.googlesource.com/c/go/+/205277
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Richard Musiol <neelance@gmail.com>
Michael Anthony Knyszek [Wed, 25 Sep 2019 15:55:29 +0000 (15:55 +0000)]
runtime: add packed bitmap summaries
This change adds the concept of summaries and of summarizing a set of
pallocBits, a core concept in the new page allocator. These summaries
are really just three integers packed into a uint64. This change also
adds tests and a benchmark for generating these summaries.
Michael Anthony Knyszek [Mon, 12 Aug 2019 19:08:39 +0000 (19:08 +0000)]
runtime: add pallocbits and tests
This change adds a per-chunk bitmap for page allocation called
pallocBits with algorithms for allocating and freeing pages out of the
bitmap. This change also adds tests for pallocBits, but does not yet
integrate it into the runtime.
Updates #35112.
Change-Id: I479006ed9f1609c80eedfff0580d5426b064b0ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/190620
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Brian Kessler [Mon, 9 Sep 2019 03:50:07 +0000 (21:50 -0600)]
cmd/compile: add signed indivisibility by power of 2 rules
Commit 44343c777c (CL 173557) added rules for handling
divisibility checks for powers of 2 for signed integers, x%c ==0.
This change adds the complementary indivisibility rules, x%c != 0.
Fixes #34166
Change-Id: I87379e30af7aff633371acca82db2397da9b2c07
Reviewed-on: https://go-review.googlesource.com/c/go/+/194219
Run-TryBot: Brian Kessler <brian.m.kessler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Michael Anthony Knyszek [Mon, 12 Aug 2019 22:30:39 +0000 (22:30 +0000)]
runtime: add new page allocator constants and description
This change is the first of a series of changes which replace the
current page allocator (which is based on the contents of mgclarge.go
and some of mheap.go) with one based on free/used bitmaps.
It adds in the key constants for the page allocator as well as a comment
describing the implementation.
Updates #35112.
Change-Id: I839d3a07f46842ad379701d27aa691885afdba63
Reviewed-on: https://go-review.googlesource.com/c/go/+/190619
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Michael Anthony Knyszek [Thu, 7 Nov 2019 02:54:50 +0000 (02:54 +0000)]
runtime: map reserved memory as NORESERVE on solaris
This changes makes it so that sysReserve, which creates a PROT_NONE
mapping, maps that memory as NORESERVE. Before this change, relatively
large PROT_NONE mappings could cause fork to fail with ENOMEM, reported
as "not enough space". Presumably this refers to swap space, since
adding this flag causes the failures to go away.
This helps unblock page allocator work, since it allows us to make large
PROT_NONE mappings on solaris safely.
Jay Conrod [Fri, 1 Nov 2019 19:59:46 +0000 (15:59 -0400)]
cmd/go/internal/modfetch/zip_sum_test: long test for zip sum stability
This CL adds a new test package which downloads specific versions of
~1000 modules in direct mode and verifies that modules have the same
sums and the zip files have the same SHA-256 hashes.
This test takes a long time to run and depends heavily on external
data that may disappear. It must be enabled manually with -zipsum.
Fixes #35290
Change-Id: Ic6959e685096e8b09cea291f19d5bd0255432284
Reviewed-on: https://go-review.googlesource.com/c/go/+/204838 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Russ Cox [Tue, 5 Nov 2019 00:43:45 +0000 (19:43 -0500)]
math, cmd/compile: rename Fma to FMA
This API was added for #25819, where it was discussed as math.FMA.
The commit adding it used math.Fma, presumably for consistency
with the rest of the unusual names in package math
(Sincos, Acosh, Erfcinv, Float32bits, etc).
I believe that using an idiomatic Go name is more important here
than consistency with these other names, most of which are historical
baggage from C's standard library.
Early additions like Float32frombits happened before "uppercase for export"
(so they were originally like "float32frombits") and they were not properly
reconsidered when we uppercased the symbols to export them.
That's a mistake we live with.
The names of functions we have added since then, and even a few
that were legacy, are more properly Go-cased, such as IsNaN, IsInf,
and RoundToEven, rather than Isnan, Isinf, and Roundtoeven.
And also constants like MaxFloat32.
For new API, we should keep using proper Go-cased symbols
instead of minimally-upper-cased-C symbols.
So math.FMA, not math.Fma.
This API has not yet been released, so this change does not break
the compatibility promise.
This CL also modifies cmd/compile, since the compiler knows
the name of the function. I could have stopped at changing the
string constants, but it seemed to make more sense to use a
consistent casing everywhere.
Brian Kessler [Wed, 13 Feb 2019 05:21:42 +0000 (22:21 -0700)]
math/big: allow all values for GCD
Allow the inputs a and b to be zero or negative to GCD
with the following definitions.
If x or y are not nil, GCD sets their value such that z = a*x + b*y.
Regardless of the signs of a and b, z is always >= 0.
If a == b == 0, GCD sets z = x = y = 0.
If a == 0 and b != 0, GCD sets z = |b|, x = 0, y = sign(b) * 1.
If a != 0 and b == 0, GCD sets z = |a|, x = sign(a) * 1, y = 0.
Fixes #28878
Change-Id: Ia83fce66912a96545c95cd8df0549bfd852652f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/164972
Run-TryBot: Brian Kessler <brian.m.kessler@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Carlo Alberto Ferraris [Wed, 2 Oct 2019 10:15:53 +0000 (19:15 +0900)]
sync: yield to the waiter when unlocking a starving mutex
When we have already assigned the semaphore ticket to a specific
waiter, we want to get the waiter running as fast as possible since
no other G waiting on the semaphore can acquire it optimistically.
The net effect is that, when a sync.Mutex is contented, the code in
the critical section guarded by the Mutex gets a priority boost.
Fixes #33747
Change-Id: I9967f0f763c25504010651bdd7f944ee0189cd45
Reviewed-on: https://go-review.googlesource.com/c/go/+/200577 Reviewed-by: Rhys Hiltner <rhys@justin.tv> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Michael Anthony Knyszek [Wed, 6 Nov 2019 23:56:03 +0000 (23:56 +0000)]
runtime: define darwin/arm64's address space as 33 bits
On iOS, the address space is not 48 bits as one might believe, since
it's arm64 hardware. In fact, all pointers are truncated to 33 bits, and
the OS only gives applications access to the range [1<<32, 2<<32).
While today this has no effect on the Go runtime, future changes which
care about address space size need this to be correct.
Updates #35112.
Change-Id: Id518a2298080f7e3d31cf7d909506a37748cc49a
Reviewed-on: https://go-review.googlesource.com/c/go/+/205758
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Michael Anthony Knyszek [Wed, 6 Nov 2019 23:18:28 +0000 (23:18 +0000)]
runtime: remove MAP_FIXED in sysReserve for raceenabled on darwin
This change removes a hack which was added to deal with Darwin 10.10's
weird ignorance of mapping hints which would cause race mode to fail
since it requires the heap to live within a certain address range.
We no longer support 10.10, and this is potentially causing problems
related to the page allocator, so drop this code.
Updates #26475.
Updates #35112.
Change-Id: I0e1c6f8c924afe715a2aceb659a969d7c7b6f749
Reviewed-on: https://go-review.googlesource.com/c/go/+/205757
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Kevin Burke [Wed, 6 Nov 2019 17:33:02 +0000 (09:33 -0800)]
cmd/go: fix spelling error
Change-Id: Ib29da1ad77c9a243a623d25113c6f8dd0261f42a
Reviewed-on: https://go-review.googlesource.com/c/go/+/205601 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Thu, 31 Oct 2019 14:03:54 +0000 (10:03 -0400)]
cmd/doc: understand vendor directories in module mode
This change employs the same strategy as in CL 203017
to detect when vendoring is in use, and if so treats
the vendor directory as a (non-module, prefixless) root.
The integration test also verifies that the 'std' and 'cmd'
modules are included and their vendored dependencies are
visible (as they are with 'go list') even when outside of
those modules.
Fixes #35224
Change-Id: I18cd01218e9eb97c1fc6e2401c1907536b0b95f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/205577
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
Joel Sing [Thu, 19 Sep 2019 17:25:16 +0000 (03:25 +1000)]
cmd/link,cmd/internal/objabi: factor out direct call identification
Factor out the direct CALL identification code from objabi.IsDirectJump and
use this in two places that have separately maintained lists of reloc types.
Provide an objabi.IsDirectCallOrJump function that implements the original
behaviour of objabi.IsDirectJump.
Change-Id: I48131bae92b2938fd7822110d53df0b4ffb35766
Reviewed-on: https://go-review.googlesource.com/c/go/+/196577
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Robert Griesemer [Tue, 29 Oct 2019 16:27:57 +0000 (09:27 -0700)]
cmd/compile/internal/syntax: silence test function output
Don't print to stdout in non-verbose (-v) test mode.
Exception: Timing output (2 lines) of TestStdLib. If
we want to disable that as well we should use another
flag to differenciate between -verbose output and
measurement results. Leaving alone for now.
Ian Lance Taylor [Wed, 6 Nov 2019 00:05:09 +0000 (16:05 -0800)]
runtime: don't hold scheduler lock when calling timeSleepUntil
Otherwise, we can get into a deadlock: sysmon takes the scheduler lock
and calls timeSleepUntil which takes each P's timer lock. Simultaneously,
some P calls runtimer (holding the P's own timer lock) which wakes up
the scavenger, calling goready, calling wakep, calling startm, getting
the scheduler lock. Now the sysmon thread is holding the scheduler lock
and trying to get a P's timer lock, while some other thread running on
that P is holding the P's timer lock and trying to get the scheduler lock.
So change sysmon to call timeSleepUntil without holding the scheduler
lock, and change timeSleepUntil to use allpLock, which is only held for
limited periods of time and should never compete with timer locks.
This hopefully
Fixes #35375
At least it should fix the linux-arm64-packet builder problems,
which occurred more reliably as that system has GOMAXPROCS == 96,
giving a lot more scope for this deadlock.
Change-Id: I7a7917daf7a4882e0b27ca416e4f6300cfaaa774
Reviewed-on: https://go-review.googlesource.com/c/go/+/205558
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Elias Naur [Wed, 6 Nov 2019 12:41:56 +0000 (13:41 +0100)]
cmd/link/internal/ld: omit bitcode-incompatible flags on iOS simulator
The -Wl,-headerpad, -Wl,-no_pie, -Wl,-pagezero_size flags are
incompatible with the bitcode-related flags used for iOS.
We already omitted the flags on darwin/arm and darwin/arm64; this change
omits the flags on all platforms != macOS so that building for the iOS
simulator works.
Updates #32963
Change-Id: Ic9af0daf01608f5ae0f70858e3045e399de7e95b
Reviewed-on: https://go-review.googlesource.com/c/go/+/205340
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Sat, 2 Nov 2019 22:39:17 +0000 (18:39 -0400)]
runtime: remove write barrier in WaitForSigusr1
WaitForSigusr1 registers a callback to be called on SIGUSR1 directly
from the runtime signal handler. Currently, this callback has a write
barrier in it, which can crash with a nil P if the GC is active and
the signal arrives on an M that doesn't have a P.
Fix this by recording the ID of the M that receives the signal instead
of the M itself, since that's all we needed anyway. To make sure there
are no other problems, this also lifts the callback into a package
function and marks it "go:nowritebarrierrec".
Fixes #35248.
Updates #35276, since in principle a write barrier at exactly the
wrong time while entering the scheduler could cause issues, though I
suspect that bug is unrelated.
Change-Id: I47b4bc73782efbb613785a93e381d8aaf6850826
Reviewed-on: https://go-review.googlesource.com/c/go/+/204620
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Thu, 31 Oct 2019 03:52:04 +0000 (23:52 -0400)]
cmd/doc: avoid calling token.IsExported on non-tokens
token.IsExported expects to be passed a token, and does not check for
non-token arguments such as "C:\workdir\go\src\text".
While we're at it, clean up a few other parts of the code that
are assuming a package path where a directory may be passed instead.
There are probably others lurking around here, but I believe this
change is sufficient to get past the test failures on the
windows-amd64-longtest builder.
Fixes #35236
Change-Id: Ic79fa035531ca0777f64b1446c2f9237397b1bdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/204442
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Clément Chigot [Tue, 5 Nov 2019 15:31:05 +0000 (16:31 +0100)]
cmd/link: fix the size of typerel.* with c-archive buildmode
With buildmode=c-archive, "runtime.types" type isn't STYPE but
STYPERELRO.
On AIX, this symbol is present in the symbol table and not under
typerel.* outersymbol. Therefore, the size of typerel.* must be adapted.
Fixes #35342
Change-Id: Ib982c6557d9b41bc3d8775e4825650897f9e0ee6
Reviewed-on: https://go-review.googlesource.com/c/go/+/205338
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitry Vyukov [Tue, 22 Oct 2019 12:55:17 +0000 (14:55 +0200)]
test: add tests for runtime.itab.init
We seem to lack any tests for some corner cases of itab.init
(multiple methods with the same name, breaking itab.init doesn't
seem to fail any tests). We also lack tests that fix text of panics.
Add more tests for itab.init.
Dmitry Vyukov [Tue, 22 Oct 2019 12:44:29 +0000 (14:44 +0200)]
runtime: remove stale runtime check in tests
The check is not relevant anymore.
The comment claims that go run does not rebuild packages,
but this is not true. And we use go build anyway.
We may have added the check because without caching
rebuilding everything starting from runtime for each test
takes a while. But now we have caching.
So from every side this check just adds code and pain.
Dmitry Vyukov [Tue, 22 Oct 2019 12:20:51 +0000 (14:20 +0200)]
runtime: clarify that itab.hash of dynamic entries is unused
The hash is used in type switches. However, compiler statically generates itab's
for all interface/type pairs used in switches (which are added to itabTable
in itabsinit). The dynamically-generated itab's never participate in type switches,
and thus the hash is irrelevant.
Bryan C. Mills [Wed, 30 Oct 2019 15:44:43 +0000 (11:44 -0400)]
cmd/go: avoid upgrading to +incompatible versions if the latest compatible one has a go.mod file
Previously we would always “upgrade” to the semantically-highest
version, even if a newer compatible version exists.
That made certain classes of mistakes irreversible: in general we
expect users to address bad releases by releasing a new (higher)
version, but if the bad release was an unintended +incompatible
version, then no release that includes a go.mod file can ever have a
higher version, and the bad release will be treated as “latest”
forever.
Instead, when considering a +incompatible version we now consult the
latest compatible (v0 or v1) release first. If the compatible release
contains a go.mod file, we ignore the +incompatible releases unless
they are expicitly requested (by version, commit ID, or branch name).
Fixes #34165
Updates #34189
Change-Id: I7301eb963bbb91b21d3b96a577644221ed988ab7
Reviewed-on: https://go-review.googlesource.com/c/go/+/204440
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Bryan C. Mills [Mon, 28 Oct 2019 20:32:24 +0000 (16:32 -0400)]
cmd/go/internal/modfetch: prune +incompatible versions more aggressively
codeRepo.Versions previously checked every possible +incompatible
version for a 'go.mod' file. That is wasteful and counterproductive.
It is wasteful because typically, a project will adopt modules at some
major version, after which they will (be required to) use semantic
import paths for future major versions.
It is counterproductive because it causes an accidental
'+incompatible' tag to exist, and no compatible tag can have higher
semantic precedence.
This change prunes out some of the +incompatible versions in
codeRepo.Versions, eliminating the “wasteful” part but not all of the
“counterproductive” part: the extraneous versions can still be fetched
explicitly, and proxies may include them in the @v/list endpoint.
Updates #34165
Updates #34189
Updates #34533
Change-Id: Ifc52c725aa396f7fde2afc727d0d5950acd06946
Reviewed-on: https://go-review.googlesource.com/c/go/+/204439
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
Filippo Valsorda [Tue, 5 Nov 2019 20:45:22 +0000 (15:45 -0500)]
doc: mention the anti-spam bypass in security.html
We had some issues with reports being marked as spam, so I added a
filter to never mark as spam something that mentions the word
"vulnerability". We get too much spam at that address to disable the
filter entirely, so instead meantion the bypass in the docs.
Katie Hockman [Mon, 14 Oct 2019 20:42:21 +0000 (16:42 -0400)]
crypto/dsa: prevent bad public keys from causing panic
dsa.Verify might currently use a nil s inverse in a
multiplication if the public key contains a non-prime Q,
causing a panic. Change this to check that the mod
inverse exists before using it.
Cherry Zhang [Thu, 31 Oct 2019 14:32:31 +0000 (10:32 -0400)]
runtime: don't fetch G from signal stack when using cgo
When using cgo, we save G to TLS, and when a signal happens, we
load G from TLS in sigtramp. This should give us a valid G. Don't
try to fetch from the signal stack. In particular, C code may
change the signal stack or call our signal handler directly (e.g.
TSAN), so we are not necessarily running on the original gsignal
stack where we saved G.
Also skip saving G on the signal stack when using cgo.
Updates #35249.
Change-Id: I40749ce6682709bd4ebfdfd9f23bd0f317fc197d
Reviewed-on: https://go-review.googlesource.com/c/go/+/204519 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Thu, 31 Oct 2019 03:14:34 +0000 (23:14 -0400)]
runtime: setg after sigFetchG
In the normal case, sigFetchG just returns the G register. But in
the case that sigFetchG fetches the G from somewhere else, the G
register still holding an invalid value. Setg here to make sure
they match.
This is particularly useful because setGsignalStack, called by
adjustSignalStack from sigtrampgo before setg to gsignal,
accesses the G register.
Should fix #35249.
Change-Id: I64c85143cb05cdb2ecca7f9936dbd8bfec186c2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/204441 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Tue, 5 Nov 2019 15:24:18 +0000 (07:24 -0800)]
runtime: keep adjusted timers in timerMoving status until moved
Before this CL adjustTimers left timers being moved in an inconsistent
state: status timerWaiting but not on a P. Simplify the code by
leaving the timers in timerMoving status until they are actually moved.
Other functions (deltimer, modtimer) will wait until the move is complete
before changing anything on the timer. This does leave timers in timerMoving
state for longer, but still not all that long.
Fixes #35367
Change-Id: I31851002fb4053bd6914139125b4c82a68bf6fb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/205418
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Bryan C. Mills [Tue, 5 Nov 2019 17:13:48 +0000 (12:13 -0500)]
cmd/compile: do not skip TestFormats on short builders
TestFormats adds ~3s of running time to the test, which may be
slightly annoying in an edit/compile/test cycle but is negligible in a
TryBot run.
The test keeps regressing in the longtest builders, requiring a manual
fix. Instead, run it even in short mode on the builders, so that
TryBot runs will detect regressions ahead of time.
Dan Scales [Fri, 1 Nov 2019 21:04:08 +0000 (14:04 -0700)]
cmd/compile: fix liveness for open-coded defer args for infinite loops
Once defined, a stack slot holding an open-coded defer arg should always be marked
live, since it may be used at any time if there is a panic. These stack slots are
typically kept live naturally by the open-defer code inlined at each return/exit point.
However, we need to do extra work to make sure that they are kept live if a
function has an infinite loop or a panic exit.
For this fix, only in the case of a function that is using open-coded defers, we
compute the set of blocks (most often empty) that cannot reach a return or a
BlockExit (panic) because of an infinite loop. Then, for each block b which
cannot reach a return or BlockExit or is a BlockExit block, we mark each defer arg
slot as live, as long as the definition of the defer arg slot dominates block b.
For this change, had to export (*Func).sdom (-> Sdom) and SparseTree.isAncestorEq
(-> IsAncestorEq)
Updates #35277
Change-Id: I7b53c9bd38ba384a3794386dd0eb94e4cbde4eb1
Reviewed-on: https://go-review.googlesource.com/c/go/+/204802
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Bryan C. Mills [Mon, 4 Nov 2019 03:08:35 +0000 (22:08 -0500)]
cmd/go: derive TestExecutableGOROOT environment from tg.env instead of os.Environ()
TestExecutableGOROOT, unlike most other tests in go_test.go, was
running subcommands in a process with an environment derived directly
from os.Environ(), rather than using tg.env on its testgoData object.
Since tg.env is what sets GO111MODULE=off for GOPATH-mode tests, that
caused TestExecutableGOROOT to unexpectedly run in module mode instead
of GOPATH mode. If the user's environment included 'GOFLAGS=-mod=mod',
that would cause the test to spuriously fail due to the inability to
download modules to $HOME (which in this test binary is hard-coded to
"/test-go-home-does-not-exist").
Updates #33848
Change-Id: I2f343008dd9e38cd76b9919eafd5a3181d0cbd6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/205064
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Tue, 5 Nov 2019 16:01:10 +0000 (11:01 -0500)]
cmd/go/internal/modfetch: remove non-hermetic test
The test for gopkg.in/yaml.v2@v2 assumes that there are
no future upstream releases. That assumption empirically
does not hold. Backporting fixes to this test is annoying,
and other gopkg.in cases are already reasonably covered,
so remove the problematic test.
Ian Lance Taylor [Tue, 5 Nov 2019 04:22:01 +0000 (20:22 -0800)]
cmd/compile: update TestFormats for CL 196959
CL 196959 uses %v to print *EscLocation values. This happens at least at
Fatalf("path inconsistency: %v != %v", edge.src, src)
in (*Escape).explainPath.
Change-Id: I1c761406af6a1025403dfefa5ec40aee75e72944
Reviewed-on: https://go-review.googlesource.com/c/go/+/205377
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Lars Lehtonen [Tue, 5 Nov 2019 02:13:06 +0000 (18:13 -0800)]
net/smtp: fix dropped test error
Pick up a dropped error in TestSendMailWithAuth() and simplify goroutine
to use an error channel instead of a sync.WaitGroup and an empty struct
doneCh.
Cherry Zhang [Mon, 28 Oct 2019 18:41:49 +0000 (14:41 -0400)]
cmd/internal/obj/mips: add a flag field to Optab
The flag field will be used for marking unsafe points. This CL
just adds the field, not doing anything with it. The next CL will
make use of it. This is for making the diff simpler.
Cherry Zhang [Fri, 25 Oct 2019 04:51:10 +0000 (00:51 -0400)]
cmd/compile: mark architecture-specific unsafe points
Introduce a mechanism for marking architecture-specific Ops
unsafe. And mark ones that use REGTMP on ARM64, as for async
preemption we will be using REGTMP as a temporary register in the
injected call.
Cherry Zhang [Fri, 25 Oct 2019 04:50:00 +0000 (00:50 -0400)]
cmd/compile: not use REGTMP in ZeroRange on ARM64
For async preemption, we will be using REGTMP as a temporary
register in injected call on ARM64, which will clobber it. So any
code that uses REGTMP is not safe for async preemption.
For ZeroRange, which is inserted at the function entry where
there is no register live, we could just use a different register
and avoid REGTMP.
Matthew Dempsky [Tue, 1 Oct 2019 18:22:07 +0000 (11:22 -0700)]
cmd/compile: fix //go:uintptrescapes for basic method calls
The logic for keeping arguments alive for calls to //go:uintptrescapes
functions was only applying to direct function calls. This CL changes
it to also apply to direct method calls, which should address most
uses of Proc.Call and LazyProc.Call.
It's still an open question (#34684) whether other call forms (e.g.,
method expressions, or indirect calls via function values, method
values, or interfaces).
Fixes #34474.
Change-Id: I874f97145972b0e237a4c9e8926156298f4d6ce0
Reviewed-on: https://go-review.googlesource.com/c/go/+/198043
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Matthew Dempsky [Mon, 28 Oct 2019 22:30:35 +0000 (15:30 -0700)]
cmd/compile, runtime: add comparison tracing for libFuzzer
This CL extends cmd/compile's experimental libFuzzer support with
calls to __sanitizer_cov_trace_{,const_}cmp{1,2,4,8}. This allows much
more efficient fuzzing of comparisons.
Only supports amd64 and arm64 for now.
Updates #14565.
Change-Id: Ibf82a8d9658f2bc50d955bdb1ae26723a3f0584d
Reviewed-on: https://go-review.googlesource.com/c/go/+/203887
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Matthew Dempsky [Sat, 19 Oct 2019 00:39:39 +0000 (17:39 -0700)]
cmd/compile, cmd/link: add coverage instrumentation for libfuzzer
This CL adds experimental coverage instrumentation similar to what
github.com/dvyukov/go-fuzz produces in its -libfuzzer mode. The
coverage can be enabled by compiling with -d=libfuzzer. It's intended
to be used in conjunction with -buildmode=c-archive to produce an ELF
archive (.a) file that can be linked with libFuzzer. See #14565 for
example usage.
The coverage generates a unique 8-bit counter for each basic block in
the original source code, and emits an increment operation. These
counters are then collected into the __libfuzzer_extra_counters ELF
section for use by libFuzzer.
Updates #14565.
Change-Id: I239758cc0ceb9ca1220f2d9d3d23b9e761db9bf1
Reviewed-on: https://go-review.googlesource.com/c/go/+/202117
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Michael Knyszek [Thu, 10 Oct 2019 20:58:43 +0000 (16:58 -0400)]
runtime: place lower limit on trigger ratio
This change makes it so that the GC pacer's trigger ratio can never fall
below 0.6. Upcoming changes to the allocator make it significantly more
scalable and thus much faster in certain cases, creating a large gap
between the performance of allocation and scanning. The consequence of
this is that the trigger ratio can drop very low (0.07 was observed) in
order to drop GC utilization. A low trigger ratio like this results in a
high amount of black allocations, which causes the live heap to appear
larger, and thus the heap, and RSS, grows to a much higher stable point.
This change alleviates the problem by placing a lower bound on the
trigger ratio. The expected (and confirmed) effect of this is that
utilization in certain scenarios will no longer converge to the expected
25%, and may go higher. As a result of this artificially high trigger
ratio, more time will also be spent doing GC assists compared to
dedicated mark workers, since the GC will be on for an artifically short
fraction of time (artificial with respect to the pacer). The biggest
concern of this change is that allocation latency will suffer as a
result, since there will now be more assists. But, upcoming changes to
the allocator reduce the latency enough to outweigh the expected
increase in latency from this change, without the blowup in RSS observed
from the changes to the allocator.
Richard Musiol [Sat, 26 Oct 2019 19:01:32 +0000 (21:01 +0200)]
syscall/js: garbage collect references to JavaScript values
The js.Value struct now contains a pointer, so a finalizer can
determine if the value is not referenced by Go any more.
Unfortunately this breaks Go's == operator with js.Value. This change
adds a new Equal method to check for the equality of two Values.
This is a breaking change. The == operator is now disallowed to
not silently break code.
Additionally the helper methods IsUndefined, IsNull and IsNaN got added.
Matthew Dempsky [Mon, 23 Sep 2019 17:57:00 +0000 (10:57 -0700)]
cmd/compile: restore -m=2 diagnostics
This is a rough attempt at restoring -m=2 escape analysis diagnostics
on par with those that were available with esc.go. It's meant to be
simple and non-invasive.
For example, given this random example from bytes/reader.go:
bytes/reader.go:138:7: leaking param content: r
bytes/reader.go:138:7: from r.s (dot of pointer) at bytes/reader.go:143:8
bytes/reader.go:138:7: from b (assigned) at bytes/reader.go:143:4
bytes/reader.go:138:7: from w.Write(b) (parameter to indirect call) at bytes/reader.go:144:19
With this CL, escape.go now reports:
bytes/reader.go:138:7: parameter r leaks to {heap} with derefs=1:
bytes/reader.go:138:7: flow: b = *r:
bytes/reader.go:138:7: from r.s (dot of pointer) at bytes/reader.go:143:8
bytes/reader.go:138:7: from r.s[r.i:] (slice) at bytes/reader.go:143:10
bytes/reader.go:138:7: from b := r.s[r.i:] (assign) at bytes/reader.go:143:4
bytes/reader.go:138:7: flow: {heap} = b:
bytes/reader.go:138:7: from w.Write(b) (call parameter) at bytes/reader.go:144:19
Updates #31489.
Change-Id: I0c2b943a0f9ce6345bfff61e1c635172a9290cbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/196959
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: David Chase <drchase@google.com>
Michael Munday [Mon, 7 Oct 2019 20:14:57 +0000 (13:14 -0700)]
internal/bytealg: add SIMD byte count implementation for s390x
Add a 'single lane' SIMD implemementation of the single byte count
function for use on machines that support the vector facility. This
allows up to 16 bytes to be counted per loop iteration.
We can probably improve performance further by adding more 'lanes'
(i.e. counting more bytes in parallel) however this will increase
the complexity of the function so I'm not sure it is worth doing
yet.
TestArchiveBuildInvokeWithExec is failing on darwin due to
duplicated symbols, because the C definition (int fortytwo;) is
copied to two generated cgo sources. In fact, this test is about
building c-archive, but doesn't need to import "C". Removed the
"C" import.
Ian Lance Taylor [Fri, 1 Nov 2019 19:38:10 +0000 (12:38 -0700)]
runtime: wake netpoller when dropping P, don't sleep too long in sysmon
When dropping a P, if it has any timers, and if some thread is
sleeping in the netpoller, wake the netpoller to run the P's timers.
This mitigates races between the netpoller deciding how long to sleep
and a new timer being added.
In sysmon, if all P's are idle, check the timers to decide how long to sleep.
This avoids oversleeping if no thread is using the netpoller.
This can happen in particular if some threads use runtime.LockOSThread,
as those threads do not block in the netpoller.
Also, print the number of timers per P for GODEBUG=scheddetail=1.
Before this CL, TestLockedDeadlock2 would fail about 1% of the time.
With this CL, I ran it 150,000 times with no failures.
Russ Cox [Mon, 4 Nov 2019 18:29:29 +0000 (13:29 -0500)]
hash/maphash: revise API to be more idiomatic
This CL makes these changes to the hash/maphash API to make it fit a bit
more into the standard library:
- Move some of the package doc onto type Hash, so that `go doc maphash.Hash` shows it.
- Instead of having identical AddBytes and Write methods,
standardize on Write, the usual name for this function.
Similarly, AddString -> WriteString, AddByte -> WriteByte.
- Instead of having identical Hash and Sum64 methods,
standardize on Sum64 (for hash.Hash64). Dropping the "Hash" method
also helps because Hash is usually reserved to mean the state of a
hash function (hash.Hash etc), not the hash value itself.
- Make an uninitialized hash.Hash auto-seed with a random seed.
It is critical that users not use the same seed for all hash functions
in their program, at least not accidentally. So the Hash implementation
must either panic if uninitialized or initialize itself.
Initializing itself is less work for users and can be done lazily.
- Now that the zero hash.Hash is useful, drop maphash.New in favor of
new(maphash.Hash) or simply declaring a maphash.Hash.
- Add a [0]func()-typed field to the Hash so that Hashes cannot be compared.
(I considered doing the same for Seed but comparing seeds seems OK.)
- Drop the integer argument from MakeSeed, to match the original design
in golang.org/issue/28322. There is no point to giving users control
over the specific seed bits, since we want the interpretation of those
bits to be different in every different process. The only thing users
need is to be able to create a new random seed at each call.
(Fixes a TODO in MakeSeed's public doc comment.)
This API is new in Go 1.14, so these changes do not violate the compatibility promise.
Fixes #35060.
Fixes #35348.
Change-Id: Ie6fecc441f3f5ef66388c6ead92e875c0871f805
Reviewed-on: https://go-review.googlesource.com/c/go/+/205069
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Cuong Manh Le [Wed, 30 Oct 2019 07:06:54 +0000 (14:06 +0700)]
cmd/compile: add test for skipping empty init functions
CL 200958 adds skipping empty init function feature without any tests
for it. A codegen test sounds ideal, but it's unlikely that we can make
one for now, so use a program to manipulate runtime/proc.go:initTask
directly.
Updates #34869
Change-Id: I2683b9a1ace36af6861af02a3a9fb18b3110b282
Reviewed-on: https://go-review.googlesource.com/c/go/+/204217
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Than McIntosh [Mon, 28 Oct 2019 15:51:13 +0000 (11:51 -0400)]
cmd/link: directly exec archive command if external tmpdir
When linking a Go archive, if the archiver invocation is the very last
thing that needs to happen in the link (no "atexit" cleanups required
remove the locally created tmpdir) then call syscall.Exec to invoke
the archiver command instead of the usual exec.Command. This has the
effect of reducing peak memory use for the linker overall, since we
don't be holding onto all of the linker's live memory while the
archiver is running.
Dan Scales [Wed, 9 Oct 2019 19:18:26 +0000 (12:18 -0700)]
runtime: ensure that Goexit cannot be aborted by a recursive panic/recover
When we do a successful recover of a panic, we resume normal execution by
returning from the frame that had the deferred call that did the recover (after
executing any remaining deferred calls in that frame).
However, suppose we have called runtime.Goexit and there is a panic during one of the
deferred calls run by the Goexit. Further assume that there is a deferred call in
the frame of the Goexit or a parent frame that does a recover. Then the recovery
process will actually resume normal execution above the Goexit frame and hence
abort the Goexit. We will not terminate the thread as expected, but continue
running in the frame above the Goexit.
To fix this, we explicitly create a _panic object for a Goexit call. We then
change the "abort" behavior for Goexits, but not panics. After a recovery, if the
top-level panic is actually a Goexit that is marked to be aborted, then we return
to the Goexit defer-processing loop, so that the Goexit is not actually aborted.
Actual code changes are just panic.go, runtime2.go, and funcid.go. Adjusted the
test related to the new Goexit behavior (TestRecoverBeforePanicAfterGoexit) and
added several new tests of aborted panics (whose behavior has not changed).
Fixes #29226
Change-Id: Ib13cb0074f5acc2567a28db7ca6912cfc47eecb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/200081
Run-TryBot: Dan Scales <danscales@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Cherry Zhang [Sun, 3 Nov 2019 22:04:28 +0000 (17:04 -0500)]
cmd/link: enable PIE on darwin/arm
We used to pass -no_pie to external linker on darwin/arm, which
is incompatible with -fembed-bitcode. CL 201358 attempted to
remove the -no_pie flag, but it resulted the darwin linker to
complain about absolute addressing in TEXT segment.
On darwin/arm, we already get away from absolute addressing in
the TEXT section. The complained absolute addressing is in
RODATA, which was embedded in the TEXT segment. This CL moves
RODATA to the DATA segment, like what we already did on ARM64
and on AMD64 in c-archive/c-shared buildmodes for the same reason.
So there is no absolute addressing in the TEXT segment, which
allows us to remove -no_pie flag.
Fixes #35252.
Updates #32963.
Change-Id: Id6e3a594cb066d257d4f58fadb4a3ee4672529f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/205060 Reviewed-by: Elias Naur <mail@eliasnaur.com>
Carlos Eduardo Seo [Thu, 3 Oct 2019 02:11:24 +0000 (23:11 -0300)]
cmd/internal/obj/ppc64: add support for DQ-form instructions
POWER9 (ISA 3.0) introduced a new format of load/store instructions to
implement indexed load/store quadword, using an immediate value instead
of a register index.
This change adds support for this new instruction encoding and adds the
new load/store quadword instructions (lxv/stxv) to the assembler.
This change also adds the missing XX1-form loads/stores (halfword and byte)
included in ISA 3.0.
Change-Id: Ibcdf53c342d7a352d64a9403c2fe7b25be9c3b24
Reviewed-on: https://go-review.googlesource.com/c/go/+/200399
Run-TryBot: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Alex Brainman [Sat, 2 Nov 2019 23:32:37 +0000 (10:32 +1100)]
crypto/x509: make '-gcflags=all=-d=checkptr' flag work
Replace
buf := [HUGE_CONST]*T)(unsafe.Pointer(p))[:]
with
buf := [HUGE_CONST]*T)(unsafe.Pointer(p))[:n:n]
Pointer p points to n of T elements. New unsafe pointer conversion
logic verifies that both first and last elements point into the
same Go variable. And this change adjusts all code to comply with
this rule.
Verified by running
go test -a -short -gcflags=all=-d=checkptr crypto/x509
The test does not fail even with original version of this code. I
suspect it is because all variables I changed live outside of Go
memory. But I am just guessing, I don't really know how pointer
checker works.
We don't async preempt assembly functions. We do that by checking
whether the function has a local pointer map, and assume it is
an assembly (or, non-Go) function if there isn't one. However,
assembly functions marked with NO_LOCAL_POINTERS still have local
pointer maps, and we wouldn't identify them. For them, check for
the special pointer map runtime.no_pointers_stackmap as well, and
treat them as not async preemptible.