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.
Austin Clements [Sun, 13 Oct 2019 01:23:29 +0000 (21:23 -0400)]
runtime: implement async scheduler preemption
This adds signal-based preemption to preemptone.
Since STW and forEachP ultimately use preemptone, this also makes
these work with async preemption.
This also makes freezetheworld more robust so tracebacks from fatal
panics should be far less likely to report "goroutine running on other
thread; stack unavailable".
For #10958, #24543. (This doesn't fix it yet because asynchronous
preemption only works on POSIX platforms on 386 and amd64 right now.)
Austin Clements [Tue, 8 Oct 2019 17:23:51 +0000 (13:23 -0400)]
runtime: use signals to preempt Gs for suspendG
This adds support for pausing a running G by sending a signal to its
M.
The main complication is that we want to target a G, but can only send
a signal to an M. Hence, the protocol we use is to simply mark the G
for preemption (which we already do) and send the M a "wake up and
look around" signal. The signal checks if it's running a G with a
preemption request and stops it if so in the same way that stack check
preemptions stop Gs. Since the preemption may fail (the G could be
moved or the signal could arrive at an unsafe point), we keep a count
of the number of received preemption signals. This lets stopG detect
if its request failed and should be retried without an explicit
channel back to suspendG.
Austin Clements [Thu, 10 Oct 2019 18:38:15 +0000 (14:38 -0400)]
runtime: scan stacks conservatively at async safe points
This adds support for scanning the stack when a goroutine is stopped
at an async safe point. This is not yet lit up because asyncPreempt is
not yet injected, but prepares us for that.
This works by conservatively scanning the registers dumped in the
frame of asyncPreempt and its parent frame, which was stopped at an
asynchronous safe point.
Conservative scanning works by only marking words that are pointers to
valid, allocated heap objects. One complication is pointers to stack
objects. In this case, we can't determine if the stack object is still
"allocated" or if it was freed by an earlier GC. Hence, we need to
propagate the conservative-ness of scanning stack objects: if all
pointers found to a stack object were found via conservative scanning,
then the stack object itself needs to be scanned conservatively, since
its pointers may point to dead objects.
Austin Clements [Mon, 14 Oct 2019 23:28:58 +0000 (19:28 -0400)]
runtime: support for injecting calls at signals on x86
This adds a sigctxt.pushCall method that pushes a call at the signaled
site. We'll use this to inject asynchronous preemptions and in some
places we use it to clean up preparePanic.
For the moment this only works on 386 and amd64. We stub it out on
other platforms and will avoid calling the stubbed version.
Austin Clements [Fri, 18 Oct 2019 19:45:52 +0000 (15:45 -0400)]
cmd/compile: fix missing unsafe-points
Currently, the compiler fails to mark any unsafe-points in the initial
instructions of a function as unsafe points. This happens because
unsafe points are encoded as a stack map index of -2 and the compiler
emits PCDATA instructions when there's a change in the stack map
index, but I had set the initial stack map index to -2. The actual
initial PCDATA value assumed by the PCDATA encoder and the runtime is
-1. Hence, if the first instructions had a stack map index of -2, no
PCDATA was emitted, which cause the runtime to assume the index was -1
instead.
This was particularly problematic in the runtime, where the compiler
was supposed to mark only calls as safe-points and everything else as
unsafe-points. Runtime leaf functions, for example, should have been
marked as entirely unsafe-points, but were instead marked entirely as
safe-points.
Fix this by making the PCDATA instruction generator assume the initial
PCDATA value is -1 instead of -2, so it will emit a PCDATA instruction
right away if the first real instruction is an unsafe-point.
This increases the size of the cmd/go binary by 0.02% since we now
emit slightly more PCDATA than before.
Current implementation of httputil.DumpRequestOut
incorrectly resets the Request.Body prematurely
before Content-Length/Transfer-Encoding detection
in newTransferWriter()
This fix avoids resetting the Request.Body when
Request.ContentLength is set to '0' by the caller
and Request.Body is set to a custom reader. To allow
newTransferWriter() to treat this situation as
'Transfer-Encoding: chunked'.
Alberto Donizetti [Sat, 19 Oct 2019 10:31:59 +0000 (12:31 +0200)]
doc: update ports list description to reflect current status
This change updates the GOARCH/GOOS discussion at the top of the
"Installing Go from source" document to better reflect the current
status. In particular:
- The GOARCH list now focuses on simply listing the supported
architectures, with no notes about their supposed "maturity", since
the same GOARCH can be mature on a GOOS and not so mature on another.
- Outdated notes about some archs being new and "not well-exercised"
have been removed in favour of a following list of which ports are
first class.
- The list of supported OS has been updated (added: AIX, Illumos),
and sorted in alphabetical order.
- A note about the runtime support being the same for all ARCHS,
"including garbage collection and efficient array slicing and" etc etc
has been removed, since it doesn't seem particularly relevant in a
"install from source" instruction page, and it's likely a leftover
from the time this doc page was the landing place for new people and
it felt the need to "sell" Go.
Joshua M. Clulow [Sat, 2 Nov 2019 05:26:59 +0000 (22:26 -0700)]
runtime: check for events when port_getn fails with ETIME
On illumos systems, and at least historically on Solaris systems, it is
possible for port_getn(3C) calls to return some number of events and
then fail with error ETIME.
Generally we expect this to happen if the caller passes an nget value
larger than 1 and calls with a timeout; if less than the requested
number of events accumulate the system will still return them after
timeout failure so the caller must check the updated nget value in the
ETIME case. Note that although less likely this can still happen even
when requesting just 1 event, especially with a short timeout value or
on a busy system.
Fixes #35261
Change-Id: I0d83251b69a2fadc64c4e8e280aa596e2e1548ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/204801 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ian Lance Taylor [Fri, 1 Nov 2019 05:46:27 +0000 (22:46 -0700)]
runtime: use atomic.Cas to change timerRemoved to timerWaiting
If multiple goroutines call time.(*Timer).Reset then the timer will go
from timerWaiting to timerDeleted to timerModifying to timerModifiedLater.
The timer can be on a different P, meaning that simultaneously cleantimers
could change it from timerDeleted to timerRemoving to timerRemoved.
If Reset sees timerRemoved, it was doing an atomic.Store of timerWaiting,
meaning that it did not necessarily see the other values set in the timer,
so the timer could appear to be in an inconsistent state. Use atomic.Cas
to avoid that possibility.
Updates #6239
Updates #27707
Fixes #35272
Change-Id: I1d59a13dc4f2ff4af110fc6e032c8c9d59cfc270
Reviewed-on: https://go-review.googlesource.com/c/go/+/204717
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Cuong Manh Le [Fri, 1 Nov 2019 02:13:40 +0000 (09:13 +0700)]
cmd/compile: revert change to "bound" in typecheckarraylit
In CL 204617, I intend to make "bound" parameter to have special meaning
in typecheckarraylit, so we can distinguish between type-checks array
literal and slice literal. But we end up with other solution. The CL was
submitted without reverting the "bound" parameter in case of slice
literal.
Technically, it's not harmful, but causes the code harder to read and maintain.
Change-Id: Ia522ccc9a6b8e25d7eaad4aa4957cb4fa18edc60
Reviewed-on: https://go-review.googlesource.com/c/go/+/204618
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Domas Tamašauskas [Thu, 10 Oct 2019 18:54:18 +0000 (21:54 +0300)]
time: document an alternative way of setting the system's time zone
User's program was mutating time.Local variable and crashing
itself as a consequence. Instead of documenting that time.Local
variable should not be mutated, recommended way of setting the
system's time zone has been documented.
Fixes #34814
Change-Id: I7781189855c3bf2ea979dfa07f86c283eed27091
Reviewed-on: https://go-review.googlesource.com/c/go/+/200457 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Fri, 1 Nov 2019 18:06:21 +0000 (11:06 -0700)]
runtime: unlock sched lock when checkdead throws due to a deadlock
I was doing some testing with GODEBUG=schedtrace=1,scheddetail=1 and I
noticed that the program hung after a throw with "all goroutines are
asleep". This is because when doing a throw or fatal panic with schedtrace
the panic code does a final schedtrace, which needs to acquire the
scheduler lock. The checkdead function is always called with the scheduler
lock held. So checkdead would throw with the scheduler lock held, then
the panic code would call schedtrace, which would block trying to acquire
the scheduler lock.
This problem will only happen for people debugging the runtime, but
it's easy to avoid by having checkdead unlock the scheduler lock before
it throws. I only did this for the throws that can happen for a normal
program, not for throws that indicate some corruption in the scheduler data.
Change-Id: Ic62277b3ca6bee6f0fca8d5eb516c59cb67855cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/204778
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Michael Anthony Knyszek [Fri, 25 Oct 2019 18:23:35 +0000 (18:23 +0000)]
runtime: turn off scavenger when there's <1 physical page of work
This change turns off the scavenger if there's less than one physical
page of work to do. If there's less than one phyiscal page of work
today, then the computed time for the work to be done will be zero,
resulting in a floating point division by zero.
This is bad on two accounts. On the one hand it could cause a fault on
some systems. On the other hand, it could cause the pacing computations
done by the scavenger to be nonsense. While this is generally harmless
in the case where there's a very small amount of work to do anyway (the
scavenger might just back off expontentially forever, or do some work
and immediately sleep, because there's not much of it to do), it causes
problems for the deadlock checker. On platforms with a larger physical
page size, such as 64 KiB, we might hit this path in a deadlock
scenario, in which case the deadlock checker will never fire and we'll
just hang.
Specifically, this happens on ppc64 trybot tests, which is where the
issue was discovered.
Bryan C. Mills [Fri, 1 Nov 2019 16:06:24 +0000 (12:06 -0400)]
cmd/go: adjust module-related logging
Suppress “finding” messages unless they are unusually slow, and
“extracting” messages always (they almost always occur conjunction
with “downloading”, which is already logged).
Log “found” messages for module dependencies added to satisfy missing
import paths.
Log top-level version changes in 'go get' when the selected version
is not identical to the version requested on the command line.
Updates #26152
Updates #33284
Change-Id: I4d0de60fab58d7cc7df8a2aff05c8b5b2220e626
Reviewed-on: https://go-review.googlesource.com/c/go/+/204777
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Lorenz Bauer [Fri, 1 Nov 2019 10:39:35 +0000 (10:39 +0000)]
encoding/binary: cache struct sizes to speed up Read and Write
A majority of work is spent in dataSize when en/decoding the same
struct over and over again. This wastes a lot of work, since
the result doesn't change for a given reflect.Value.
Cache the result of the function for structs, so that subsequent
calls to dataSize can avoid doing work.
name old time/op new time/op delta
ReadStruct 1.00µs ± 1% 0.37µs ± 1% -62.99% (p=0.029 n=4+4)
WriteStruct 1.00µs ± 3% 0.37µs ± 1% -62.69% (p=0.008 n=5+5)
name old speed new speed delta
ReadStruct 75.1MB/s ± 1% 202.9MB/s ± 1% +170.16% (p=0.029 n=4+4)
WriteStruct 74.8MB/s ± 3% 200.4MB/s ± 1% +167.96% (p=0.008 n=5+5)
Ian Lance Taylor [Wed, 30 Oct 2019 22:12:52 +0000 (15:12 -0700)]
runtime: use correct state machine in addAdjustedTimers
The addAdjustedTimers function was a late addition, and it got some of
the state machine wrong, leading to failures like
https://storage.googleapis.com/go-build-log/930576b6/windows-amd64-2016_53d0319e.log
Updates #6239
Updates #27707
Change-Id: I9e94e563b4698ff3035ce609055ca292b9cab3df
Reviewed-on: https://go-review.googlesource.com/c/go/+/204280
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>