Jay Conrod [Mon, 18 Mar 2019 22:35:44 +0000 (18:35 -0400)]
cmd/go: avoid link error when -coverpkg covers main packages (more)
This fixes two problems missed in CL 164877.
First, p.Internal.BuildInfo is now part of the cache key. This is
important since p.Internal.BuildInfo causes the build action to
synthesize a new source file, which affects the output.
Second, recompileForTest is always called for test
packages. Previously, it was only called when there were internal test
sources, so the fix in CL 164877 did not apply to packages that only
had external tests.
Fixes #30374
Change-Id: Iac2d7e8914f0313f9ab4222299a866f67889eb2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/168200
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Thu, 21 Feb 2019 09:46:47 +0000 (10:46 +0100)]
runtime: fix sigfwd for aix/ppc64
This commit fixes runtime.sigfwd for aix/ppc64. fn is a function
descriptor and not a function. R2 must be saved and restored.
Change-Id: Ie506b0bdde562ca37202d19973ba1d537c3d64e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/164015 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Mikio Hara [Thu, 14 Mar 2019 20:10:08 +0000 (05:10 +0900)]
runtime, internal/poll: report only critical event scanning error
This change makes the runtime-integrated network poller report only
critical event scanning errors.
In the previous attempt, CL 166497, we treated any combination of error
events as event scanning errors and it caused false positives in event
waiters because platform-dependent event notification mechanisms allow
event originators to use various combination of events.
To avoid false positives, this change makes the poller treat an
individual error event as a critical event scanning error by the
convention of event notification mechanism implementations.
Updates #30624.
Fixes #30817.
Fixes #30840.
Change-Id: I906c9e83864527ff73f636fd02bab854d54684ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/167777
Run-TryBot: Mikio Hara <mikioh.public.networking@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Thu, 21 Feb 2019 09:45:40 +0000 (10:45 +0100)]
runtime: enable external linker tests for aix/ppc64
Change-Id: Icc42843adb15c2aba1cfea854fad049c6704344b
Reviewed-on: https://go-review.googlesource.com/c/go/+/164014
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Wèi Cōngruì [Mon, 4 Mar 2019 10:07:07 +0000 (10:07 +0000)]
internal/poll, os: cancel pending I/O when closing pipes on Windows
When closing a pipe, use CancelIoEx to cancel pending I/O.
This makes concurrent Read and Write calls return os.ErrClosed.
This change also enables some pipe tests on Windows.
Fixes #28477
Fixes #25835
Change-Id: If52bb7d80895763488a61632e4682a78336e8420
Reviewed-on: https://go-review.googlesource.com/c/go/+/164721
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Thu, 21 Feb 2019 09:14:46 +0000 (10:14 +0100)]
runtime: disable pprof test with cgo on aix/ppc64
This commit disables new cgo pprof tests and adds an handler in sigtramp
to refuse SIGPROF signal.
Updates #28555
Change-Id: I152a871f8636e93328d411329104c6f023bd1691
Reviewed-on: https://go-review.googlesource.com/c/go/+/164013
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Wed, 20 Feb 2019 16:01:15 +0000 (17:01 +0100)]
runtime: fix TestSigStackSwapping on aix/ppc64
This commit fixes TestSigStackSwapping by increasing the signal stack
size. This is needed because SIGSTKSZ is too small when VMX is used on
AIX.
Change-Id: Ic2b5faa65745228d0768383b3d6ebd4b6f9f532c
Reviewed-on: https://go-review.googlesource.com/c/go/+/164012
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Wed, 20 Feb 2019 15:52:35 +0000 (16:52 +0100)]
cmd, runtime: fix C trampolines on aix/ppc64
C trampolines are made by fixup CSECTS which are added between two
symbols. If such CSECTS is added inside Go functions, all method
offsets stored in moduledatas will be wrong.
In order to prevent this, every C code is moved at the end of the
executable and long calls are created for GO functions called by C
code.
The main function can't longer be made in Go as AIX __start isn't using
a long call to branch on it. Therefore, a main is defined on
runtime/cgo.
Change-Id: I214b18decdb83107cf7325b298609eef9f9d1330
Reviewed-on: https://go-review.googlesource.com/c/go/+/164010
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Sat, 5 Jan 2019 01:34:33 +0000 (17:34 -0800)]
cmd/compile: better write barrier removal when initializing new objects
When initializing a new object, we're often writing
1) to a location that doesn't have a pointer to a heap object
2) a pointer that doesn't point to a heap object
When both those conditions are true, we can avoid the write barrier.
This CL detects case 1 by looking for writes to known-zeroed
locations. The results of runtime.newobject are zeroed, and we
perform a simple tracking of which parts of that object are written so
we can determine what part remains zero at each write.
This CL detects case 2 by looking for addresses of globals (including
the types and itabs which are used in interfaces) and for nil pointers.
Makes cmd/go 0.3% smaller. Some particular cases, like the slice
literal in #29573, can get much smaller.
TODO: we can remove actual zero writes also with this mechanism.
Philipp Stephani [Mon, 18 Mar 2019 20:13:17 +0000 (20:13 +0000)]
cmd/cgo: use C exact-width integer types to represent Go types
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer. This ensures that
they have the same domain and representation as the corresponding Go types.
Fixes #29878
Change-Id: Ie8a51e91666dfd89731c7859abe47356c94ca1be
GitHub-Last-Rev: 546a2cc3f1e22dc282757f73c01c91b00899d911
GitHub-Pull-Request: golang/go#29907
Reviewed-on: https://go-review.googlesource.com/c/go/+/159258
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Mon, 18 Mar 2019 20:17:35 +0000 (13:17 -0700)]
cmd/compile: rename init function from init.ializers back to init
The name change init -> init.ializers was initially required for
initialization code.
With CL 161337 there's no wrapper code any more, there's a data
structure instead (named .inittask). So we can go back to just
plain init appearing in tracebacks.
Keith Randall [Wed, 6 Feb 2019 00:22:38 +0000 (16:22 -0800)]
cmd/compile: reorganize init functions
Instead of writing an init function per package that does the same
thing for every package, just write that implementation once in the
runtime. Change the compiler to generate a data structure that encodes
the required initialization operations.
Reduces cmd/go binary size by 0.3%+. Most of the init code is gone,
including all the corresponding stack map info. The .inittask
structures that replace them are quite a bit smaller.
Most usefully to me, there is no longer an init function in every -S output.
(There is an .inittask global there, but it's much less distracting.)
After this CL we could change the name of the "init.ializers" function
back to just "init".
Matthew Dempsky [Mon, 18 Mar 2019 19:19:26 +0000 (12:19 -0700)]
cmd/compile: fix importing rewritten f(g()) calls
golang.org/cl/166983 started serializing the Ninit field of OCALL
nodes within function inline bodies (necessary to fix a regression in
building crypto/ecdsa with -gcflags=-l=4), but this means the Ninit
field needs to be typechecked when the imported function body is used.
It's unclear why this wasn't necessary for the crypto/ecdsa
regression.
Fixes #30907.
Change-Id: Id5f0bf3c4d17bbd6d5318913b859093c93a0a20c
Reviewed-on: https://go-review.googlesource.com/c/go/+/168199
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Keith Randall [Wed, 6 Feb 2019 22:12:36 +0000 (14:12 -0800)]
cmd/compile,runtime: provide index information on bounds check failure
A few examples (for accessing a slice of length 3):
s[-1] runtime error: index out of range [-1]
s[3] runtime error: index out of range [3] with length 3
s[-1:0] runtime error: slice bounds out of range [-1:]
s[3:0] runtime error: slice bounds out of range [3:0]
s[3:-1] runtime error: slice bounds out of range [:-1]
s[3:4] runtime error: slice bounds out of range [:4] with capacity 3
s[0:3:4] runtime error: slice bounds out of range [::4] with capacity 3
Note that in cases where there are multiple things wrong with the
indexes (e.g. s[3:-1]), we report one of those errors kind of
arbitrarily, currently the rightmost one.
An exhaustive set of examples is in issue30116[u].out in the CL.
The message text has the same prefix as the old message text. That
leads to slightly awkward phrasing but hopefully minimizes the chance
that code depending on the error text will break.
Increases the size of the go binary by 0.5% (amd64). The panic functions
take arguments in registers in order to keep the size of the compiled code
as small as possible.
Fixes #30116
Change-Id: Idb99a827b7888822ca34c240eca87b7e44a04fdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/161477
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Martin Möhrmann [Wed, 7 Nov 2018 08:33:12 +0000 (09:33 +0100)]
runtime: replace division by span element size by multiply and shifts
Divisions are generally slow. The compiler can optimize a division
to use a sequence of faster multiplies and shifts (magic constants)
if the divisor is not know at compile time.
The value of the span element size in mcentral.grow is not known at
compile time but magic constants to compute n / span.elementsize
are already stored in class_to_divmagic and mspan.
They however need to be adjusted to work for
(0 <= n <= span.npages * pagesize) instead of
(0 <= n < span.npages * pagesize).
Change-Id: Ieea59f1c94525a88d012f2557d43691967900deb
Reviewed-on: https://go-review.googlesource.com/c/go/+/148057
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Daniel Martí [Sun, 17 Mar 2019 22:45:30 +0000 (22:45 +0000)]
encoding/json: fix performance regression in the decoder
In golang.org/cl/145218, a feature was added where the JSON decoder
would keep track of the entire path to a field when reporting an
UnmarshalTypeError.
However, we all failed to check if this affected the benchmarks - myself
included, as a reviewer. Below are the numbers comparing the CL's parent
with itself, once it was merged:
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 28.2ms ± 2% +119.33% (p=0.002 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 69MB/s ± 3% -54.40% (p=0.002 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 109.39MB ± 0% +3891.83% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 168.5k ± 0% +117.30% (p=0.004 n=6+5)
The reason why the decoder got twice as slow is because it now allocated
~40x as many objects, which puts a lot of pressure on the garbage
collector.
The reason is that the CL concatenated strings every time a nested field
was decoded. In other words, practically every field generated garbage
when decoded. This is hugely wasteful, especially considering that the
vast majority of JSON decoding inputs won't return UnmarshalTypeError.
Instead, use a stack of fields, and make sure to always use the same
backing array, to ensure we only need to grow the slice to the maximum
depth once.
The original CL also introduced a bug. The field stack string wasn't
reset to its original state when reaching "d.opcode == scanEndObject",
so the last field in a decoded struct could leak. For example, an added
test decodes a list of structs, and encoding/json before this CL would
fail:
got: cannot unmarshal string into Go struct field T.Ts.Y.Y.Y of type int
want: cannot unmarshal string into Go struct field T.Ts.Y of type int
To fix that, simply reset the stack after decoding every field, even if
it's the last.
Below is the original performance versus this CL. There's a tiny
performance hit, probably due to the append for every decoded field, but
at least we're back to the usual ~150MB/s.
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 13.0ms ± 1% +1.25% (p=0.009 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 149MB/s ± 1% -1.24% (p=0.009 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 2.74MB ± 0% +0.00% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 77.5k ± 0% +0.00% (p=0.002 n=6+6)
Finally, make all of these benchmarks report allocs by default. The
decoder ones are pretty sensitive to generated garbage, so ReportAllocs
would have made the performance regression more obvious.
Tobias Klauser [Sun, 17 Mar 2019 21:39:42 +0000 (22:39 +0100)]
internal/bytealg: share code for IndexByte functions on arm
Move the shared code of IndexByte and IndexByteString into
indexbytebody. This will allow to implement optimizations (e.g.
for #29001) in a single function.
Elias Naur [Sun, 17 Mar 2019 15:01:27 +0000 (16:01 +0100)]
os: only fallback to root directory if $HOME fails for UserHomeDir
UserHomeDir always returns "/" for platforms where the home directory
is not always well defined. However, the user might set HOME before
running a Go program on those platforms and on at least iOS, HOME
is actually set to something useful (the root of the app specific
writable directory).
This CL changes UserHomeDir to use the root directory "/" only if
$HOME is empty.
The 5-12% decrease in performance on Equal/{6,9,15} are due to the
benchmarks splitting the bytes buffer in half. The b argument to Equal
then ends up being unaligned and thus the fast word-wise compare doesn't
kick in.
Elias Naur [Sun, 17 Mar 2019 15:07:25 +0000 (16:07 +0100)]
cmd/go,misc/ios: fix tests on iOS
Now that modules are always on, cmd/go tests require a valid
GOCACHE. However, on iOS where the go tool is not available, the
cmd/go test driver ends up setting GOCACHE to the empty string.
Fix it by falling back to the builtin default cache directory.
The iOS exec wrapper passes the environment variables to the app
on the device, including $HOME used for the default cache directory.
Skip $HOME to let the device specific and writable $HOME be used
instead.
Should fix cmd/go on the iOS builders that broke when GO111MODULE
defaulted to on.
Jay Conrod [Tue, 5 Mar 2019 19:53:03 +0000 (14:53 -0500)]
cmd/go: list directories in module cache replacements
"go list" has allowed listing directory paths to packages in the
module cache since CL 126715. This is sometimes necessary for tools
gathering package information about source files in imported packages.
With this change, we only allow directories in the module cache for
modules in the build list after replacements are applied. Previously,
we ignored replacements when expanding file system path patterns while
constructing the build list.
Fixes #29548
Change-Id: Ic7f89122c4656c8967c14545cb7117f98e89e721
Reviewed-on: https://go-review.googlesource.com/c/go/+/165381
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Austin Clements [Fri, 15 Mar 2019 17:29:40 +0000 (13:29 -0400)]
runtime: fix write barrier on wasm
The current wasm write barrier implementation incorrectly implements
the "deletion" part of the barrier. It correctly greys the new value
of the pointer, but rather than also greying the old value of the
pointer, it greys the object containing the slot (which, since the old
value was just overwritten, is not going to contain the old value).
This can lead to unmarked, reachable objects.
Often, this is masked by other marking activity, but one specific
sequence that can lead to an unmarked object because of this bug is:
1. Initially, GC is off, object A is reachable from just one pointer
in the heap.
2. GC starts and scans the stack of goroutine G.
3. G copies the pointer to A on to its stack and overwrites the
pointer to A in the heap. (Now A is reachable only from G's stack.)
4. GC finishes while A is still reachable from G's stack.
With a functioning deletion barrier, step 3 causes A to be greyed.
Without a functioning deletion barrier, nothing causes A to be greyed,
so A will be freed even though it's still reachable from G's stack.
Than McIntosh [Fri, 18 Jan 2019 20:16:11 +0000 (15:16 -0500)]
debug/dwarf: more graceful handling of unsupported types
Enhance the type decoder to do a better job handling unknown type
tags. DWARF has a number of type DIEs that this package doesn't handle
(things like "pointer to member" types in C++); avoid crashing for
such types, but instead return a placeholder "UnsupportedType" object
(this idea suggested by Austin). This provides a compromise between
implementing the entire kitchen sink and simply returning an error
outright on any unknown type DIE.
Clément Chigot [Wed, 20 Feb 2019 15:48:43 +0000 (16:48 +0100)]
cmd/link: fix trampoline generation for aix/ppc64
This commit fixes trampoline generation on aix/ppc64 which must use TOC
symbols.
It also adds a size to runtime.text.X symbols to prevent ld from moving
them, like runtime.text.
Change-Id: Ida033ec20ad8d7b7fb3faeb0ec4fa7bc4ce86b7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/164009
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Thu, 14 Mar 2019 01:56:37 +0000 (18:56 -0700)]
runtime: introduce and consistently use setNsec for timespec
The general code for setting a timespec value sometimes used set_nsec
and sometimes used a combination of set_sec and set_nsec. Standardize
on a setNsec function that takes a number of nanoseconds and splits
them up to set the tv_sec and tv_nsec fields. Consistently mark
setNsec as go:nosplit, since it has to be that way on some systems
including Darwin and GNU/Linux. Consistently use timediv on 32-bit
systems to help stay within split-stack limits on processors that
don't have a 64-bit division instruction.
Change-Id: I6396bb7ddbef171a96876bdeaf7a1c585a6d725b
Reviewed-on: https://go-review.googlesource.com/c/go/+/167389
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Matthew Dempsky [Thu, 14 Mar 2019 20:54:05 +0000 (13:54 -0700)]
cmd/compile: move Strongly Connected Components code into new file
This logic is used by the current escape analysis pass, but otherwise
logically independent. Move (unchanged) into a separate file to make
that clearer, and to make it easier to replace esc.go later.
Updates #23109.
Change-Id: Iec8c0c47ea04c0008165791731c11d9104d5a474
Reviewed-on: https://go-review.googlesource.com/c/go/+/167715 Reviewed-by: Robert Griesemer <gri@golang.org>
Matthew Dempsky [Wed, 12 Dec 2018 19:15:37 +0000 (11:15 -0800)]
cmd/compile: rewrite f(g()) for multi-value g() during typecheck
This is a re-attempt at CL 153841, which caused two regressions:
1. crypto/ecdsa failed to build with -gcflags=-l=4. This was because
when "t1, t2, ... := g(); f(t1, t2, ...)" was exported, we were losing
the first assignment from the call's Ninit field.
2. net/http/pprof failed to run with -gcflags=-N. This is due to a
conflict with CL 159717: as of that CL, package-scope initialization
statements are executed within the "init.ializer" function, rather
than the "init" function, and the generated temp variables need to be
moved accordingly too.
[Rest of description is as before.]
This CL moves order.go's copyRet logic for rewriting f(g()) into t1,
t2, ... := g(); f(t1, t2, ...) earlier into typecheck. This allows the
rest of the compiler to stop worrying about multi-value functions
appearing outside of OAS2FUNC nodes.
This changes compiler behavior in a few observable ways:
1. Typechecking error messages for builtin functions now use general
case error messages rather than unnecessarily differing ones.
2. Because f(g()) is rewritten before inlining, saved inline bodies
now see the rewritten form too. This could be addressed, but doesn't
seem worthwhile.
3. Most notably, this simplifies escape analysis and fixes a memory
corruption issue in esc.go. See #29197 for details.
Fixes #15992.
Fixes #29197.
Change-Id: I930b10f7e27af68a0944d6c9bfc8707c3fab27a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/166983
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Richard Musiol [Tue, 12 Mar 2019 22:40:03 +0000 (23:40 +0100)]
misc/wasm: add workaround for missed timeout events
TryBot is sometimes running into deadlocks on js/wasm. We haven't been
able to reproduce them yet. This workaround is an experiment to resolve
these deadlocks by retrying a missed timeout event.
A timeout event is scheduled by Go to be woken by JavaScript after a
certain amount of time. The checkTimeouts function then checks which
notes to wake by comparing their deadline to nanotime. If this
check fails erroneously then the note may stay asleep forever, causing
a deadlock. This may or may not be the reason of the observed
deadlocks.
Carlos Eduardo Seo [Thu, 31 Jan 2019 17:22:21 +0000 (11:22 -0600)]
cmd/internal/obj/ppc64: fix wrong register encoding in XX1-Form instructions
A bug in the encoding of XX1-Form is flipping bit 31 of such instructions.
This may result in register clobering when using VSX instructions.
This was not exposed before because we currently don't generate these
instructions in SSA, and the asm files in which they are present aren't
affected by register clobbering.
This change fixes the bug and adds a testcase for the problem.
Russ Cox [Thu, 14 Mar 2019 00:06:37 +0000 (20:06 -0400)]
crypto/x509: move debug prints to standard error
Standard output is reserved for actual program output.
Debug print should be limited in general (here they are
enabled by an environment variable) and always go to
standard error.
Russ Cox [Thu, 7 Mar 2019 03:24:54 +0000 (22:24 -0500)]
fmt: put back named results in ss.scanBasePrefix
CL 165619 removed these names when it removed
the use of the plain 'return'. But the names help for
documentation purposes even without being mentioned
directly in the function, so removing them makes the
code less readable. Put them back. I renamed found
to zeroFound to make the meaning clearer.
Change-Id: I1010931f08290af0b0ede7d21b1404c2eea196a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/165899
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Wed, 13 Mar 2019 17:57:03 +0000 (17:57 +0000)]
builtin: make len's godoc less ambiguous
The len godoc uses a blockquote to list the rules for its semantics.
The item that describes channels is a bit long, so it's split in two
lines. However, the first line ends with a semicolon, and the second
line can be read as a sentence of its own, so it's easy to misinterpret
that the two lines are separate.
Making that easy mistake would lead to an incorrect understanding of
len:
if v is nil, len(v) is zero.
This could lead us to think that len(nil) is valid and should return
zero. When in fact, that statement only applies to nil channels.
To make this less ambiguous, add a bit of indentation to the follow-up
line, to align with the channel body. If lists are added to godoc in the
future via #7873, perhaps this text can be simplified.
Hana Kim [Mon, 14 Jan 2019 16:13:34 +0000 (11:13 -0500)]
cmd/go: fix the default build output name for versioned binaries
`go build` has chosen the last element of the package import path
as the default output name when -o option is given. That caused
the output of a package build when the module root is the major
version component such as 'v2'.
A similar issue involving `go install` was fixed in
https://golang.org/cl/128900. This CL refactors the logic added
with the change and makes it available as
internal/load.DefaultExecName.
This CL makes 'go test' to choose the right default test binary
name when the tested package is in the module root. (E.g.,
instead of v2.test, choose pkg.test for the test of 'path/pkg/v2')
Fixes #27283.
Change-Id: I6905754f0906db46e3ce069552715f45356913ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/140863 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Matthew Dempsky [Thu, 28 Feb 2019 02:18:47 +0000 (18:18 -0800)]
cmd/compile: simplify isGoConst
The only ways to construct an OLITERAL node are (1) a basic literal
from the source package, (2) constant folding within evconst (which
only folds Go language constants), (3) the universal "nil" constant,
and (4) implicit conversions of nil to some concrete type.
Passes toolstash-check.
Change-Id: I30fc6b07ebede7adbdfa4ed562436cbb7078a2ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/166981
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Carlos Eduardo Seo [Fri, 11 Jan 2019 19:16:28 +0000 (17:16 -0200)]
cmd/compile: add processor level selection support to ppc64{,le}
ppc64{,le} processor level selection allows the compiler to generate instructions
targeting newer processors and processor-specific optimizations without breaking
compatibility with our current baseline. This feature introduces a new environment
variable, GOPPC64.
GOPPC64 is a GOARCH=ppc64{,le} specific option, for a choice between different
processor levels (i.e. Instruction Set Architecture versions) for which the
compiler will target. The default is 'power8'.
Change-Id: Ic152e283ae1c47084ece4346fa002a3eabb3bb9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/163758
Run-TryBot: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
David Chase [Mon, 28 Jan 2019 23:00:01 +0000 (18:00 -0500)]
cmd/compile: move statement marks from jumps to targets
When a jump at the end of a block is about to be marked as
a statement, if the first real instruction in the target
block is also a statement for the same line, remove the
mark from the jump.
This is a first effort at a minimal-harm heuristic.
A better heuristic might skip over any "not-statement"
values preceding a definitely marked value.
Daniel Martí [Sun, 3 Mar 2019 17:24:57 +0000 (17:24 +0000)]
cmd/go/internal/base: remove MergeEnvLists
This internally exported function allowed merging environment variable
lists, and was mostly a convenience for the rest of cmd/go/internal.
It seems to date all the way back to 2013.
However, since CL 37586 in early 2017, os/exec has already taken care of
deduplicating environment variable lists. Thus, it's unnecessary for
cmd/go to take care of that before calling exec.Cmd.Start.
Moreover, because os/exec will deduplicate the list in any case, we're
adding extra work in all these scenarios.
Finally, remove an unnecessary addition of GOROOT= in internal/tool.
cfg.OrigEnv may not have the correct GOROOT set up, but os.Environ does;
cmd/go's main function makes sure of that.
Daniel Martí [Sun, 25 Nov 2018 17:30:36 +0000 (17:30 +0000)]
encoding/base64: speed up the decoder
Most of the decoding time is spent in the first Decode loop, since the
rest of the function only deals with the few remaining bytes. Any
unnecessary work done in that loop body matters tremendously.
One such unnecessary bottleneck was the use of the enc.decodeMap table.
Since enc is a pointer receiver, and the field is used within the
non-inlineable function decode64, the decoder must perform a nil check
at every iteration.
To fix that, move the enc.decodeMap uses to the parent function, where
we can lift the nil check outside the loop. That gives roughly a 15%
speed-up. The function no longer performs decoding per se, so rename it.
While at it, remove the now unnecessary receivers.
An unfortunate side effect of this change is that the loop now contains
eight bounds checks on src instead of just one. However, not having to
slice src plus the nil check removal well outweigh the added cost.
The other piece that made decode64 slow was that it wasn't inlined, and
had multiple branches. Use a simple bitwise-or trick suggested by Roger
Peppe, and collapse the rest of the bitwise logic into a single
expression. Inlinability and the reduced branching give a further 10%
speed-up.
Finally, add these two functions to TestIntendedInlining, since we want
them to stay inlinable.
Apply the same refactor to decode32 for consistency, and to let 32-bit
architectures see a similar performance gain for large inputs.
Mikio Hara [Sun, 10 Mar 2019 05:30:06 +0000 (14:30 +0900)]
runtime, internal/poll, net: report event scanning error on read event
This change makes it possible the runtime-integrated network poller and
APIs in the package internal/poll to report an event scanning error on a
read event.
The latest Go releases open up the way of the manipulation of the poller
for users. On the other hand, it starts misleading users into believing
that the poller accepts any user-configured file or socket perfectly
because of not reporting any error on event scanning, as mentioned in
issue 30426. The initial implementation of the poller was designed for
just well-configured, validated sockets produced by the package net.
However, the assumption is now obsolete.
Clément Chigot [Wed, 20 Feb 2019 15:42:11 +0000 (16:42 +0100)]
cmd: always allow bigtoc generation with gcc on aix/ppc64
-mcmodel=large and -Wl,-bbigtoc must always be passed to gcc in order to
prevent TOC overflow error. However, a warning is still issued by ld. It
is removed as it doesn't give any useful information.
Change-Id: I95a78e8993cc7b5c0f329654d507409785f7eea6
Reviewed-on: https://go-review.googlesource.com/c/go/+/164008
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Wed, 20 Feb 2019 15:39:09 +0000 (16:39 +0100)]
cmd: disable DWARF with old ld on aix/ppc64
DWARF relocations isn't working with some older ld, because of
-Wl,-bnoobjreorder which is needed on Go.
This commit checks ld's version and disable DWARF generation in cmd/link
if it's too old. Some tests must therefore be skipped.
Change-Id: I2e794c263eb0dfe0b42e7062fb80c26f086b44d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/164007
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Fri, 8 Mar 2019 01:44:50 +0000 (17:44 -0800)]
math/big: add support for underscores '_' in numbers
The primary change is in nat.scan which now accepts underscores for base 0.
While at it, streamlined error handling in that function as well.
Also, improved the corresponding test significantly by checking the
expected result values also in case of scan errors.
The second major change is in scanExponent which now accepts underscores when
the new sepOk argument is set. While at it, essentially rewrote that
function to match error and underscore handling of nat.scan more closely.
Added a new test for scanExponent which until now was only tested
indirectly.
Finally, updated the documentation for several functions and added many
new test cases to clients of nat.scan.
A major portion of this CL is due to much better test coverage.
Daniel Martí [Mon, 11 Mar 2019 17:04:48 +0000 (17:04 +0000)]
text/tabwriter: use a single defer per Write call
Lines with single cells prompt a flush. Unfortunately, a call to
Writer.Flush also means two defers, which is an expensive operation to
do if many lines consist of single cells.
This is common when formatting code with aligned comments. Most lines
aren't going to have any comments at all, so the performance hit is
going to be noticeable.
The Write method already has a "defer handlePanic" of its own, so we
don't need to worry about panics leaking out. The error will now mention
"Write" instead of "Flush" if a panic is encountered during that nested
flush, but arguably that's a good thing; the user called Write, not
Flush.
For the reset call, add a non-deferred call as part of flushNoDefers, as
that's still necessary. Otherwise, the exported Flush method still does
a "defer b.reset".
The current tabwriter benchmarks are unaffected, since they don't
contain many single-cell lines, and because lines are written one at a
time. For that reason, we add a benchmark which has both of these
characteristics.
name old time/op new time/op delta
Code-8 2.72µs ± 0% 1.77µs ± 0% -34.88% (p=0.000 n=6+5)
name old alloc/op new alloc/op delta
Code-8 648B ± 0% 648B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
Code-8 13.0 ± 0% 13.0 ± 0% ~ (all equal)
Perhaps unsurprisingly, go/printer also gets a bit faster, as it too
buffers its output before writing it to tabwriter.
name old time/op new time/op delta
Print-8 6.53ms ± 0% 6.39ms ± 0% -2.22% (p=0.008 n=5+5)
Change-Id: Ie01fea5ced43886a9eb796cb1e6c810f7a810853
Reviewed-on: https://go-review.googlesource.com/c/go/+/166797
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Bryan C. Mills [Tue, 12 Mar 2019 20:43:45 +0000 (16:43 -0400)]
cmd/go: fix typo in GoGetInsecure to actually set GOPROXY
I typo'd this variable in CL 165745, and neither I, the reviewer, nor the TryBots noticed.
But the longtest builder noticed, and it's not happy about it.