Michael Anthony Knyszek [Thu, 31 Jan 2019 16:57:29 +0000 (16:57 +0000)]
runtime: merge codepaths in scavengeLargest
This change just makes the code in scavengeLargest easier to reason
about by reducing the number of exit points to the method. It should
still be correct either way because the condition checked at the end
(released > nbytes) will always be false if we return, but this just
makes the code a little easier to maintain.
The Modified field allows representation of extended timestamps, which provide more accuracy than the legacy MS-DOS timestamps.
The FileInfo method provides an implementation of the os.FileInfo interface for files inside archives.
With this change, we make FileInfo use the Modified field, if present, to return more detailed timestamps from its ModTime method.
Fixes #28350
Change-Id: Ia31b5b871a3e61df38a3a1325787ae23ea0b8088
GitHub-Last-Rev: 13e94be3f8ba58717911354146670fc2bc594692
GitHub-Pull-Request: golang/go#28352
Reviewed-on: https://go-review.googlesource.com/c/go/+/144382
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Nigel Tao [Sat, 6 Apr 2019 23:33:36 +0000 (09:33 +1000)]
image: deprecate ZP and ZR
They were added a very long time ago, as a convenience before Go had
struct literals. Today, it is better to use the zero-valued literal. For
example, the compiler cannot prove that ZP or ZR have not been modified.
Change-Id: I7469f1c751e91bf76fe1eab07b5772eccb5d6405
Reviewed-on: https://go-review.googlesource.com/c/go/+/171097 Reviewed-by: Nigel Tao <nigeltao@golang.org>
Keith Randall [Sat, 6 Apr 2019 19:42:51 +0000 (12:42 -0700)]
cmd/compile: remove AUNDEF opcode
This opcode was only used to mark unreachable code for plive to use.
plive now uses the SSA representation, so it knows locations are
unreachable because they are ends of Exit blocks. It doesn't need
these opcodes any more.
These opcodes actually used space in the binary, 2 bytes per undef
on x86 and more for other archs.
runtime: separate stack freeing from stack shrinking
Currently, shrinkstack will free the stack if the goroutine is dead.
There are only two places that call shrinkstack: scanstack, which will
never call it if the goroutine is dead; and markrootFreeGStacks, which
only calls it on dead goroutines.
Clean this up by separating stack freeing out of shrinkstack.
Austin Clements [Tue, 26 Mar 2019 01:16:33 +0000 (21:16 -0400)]
runtime: use acquirem/releasem more widely
We've copy-pasted the pattern of releasem in many places. This CL
replaces almost everywhere that manipulates g.m.locks and g.preempt
with calls to acquirem/releasem. There are a few where we do something
more complicated, like where exitsyscall has to restore the stack
bound differently depending on the preempt flag, which this CL leaves
alone.
Austin Clements [Tue, 26 Mar 2019 02:24:03 +0000 (22:24 -0400)]
runtime: fix sanity check in notetsleep
CL 3660 replaced m.gcing with m.preemptoff, but unintentionally
reversed the sense of part of a sanity check in notetsleep.
Originally, notetsleep required that it be called from g0 or with
preemption disabled (specifically from within the garbage collector).
CL 3660 made it require that it be called from g0 or that preemption
be *enabled*.
I'm not sure why it had the original exception for being called from a
user g within the garbage collector, but the current garbage collector
certainly doesn't need that, and the new condition is completely wrong.
Make the sanity check just require that it's called on g0.
Austin Clements [Sat, 2 Mar 2019 20:16:29 +0000 (15:16 -0500)]
sync: smooth out Pool behavior over GC with a victim cache
Currently, every Pool is cleared completely at the start of each GC.
This is a problem for heavy users of Pool because it causes an
allocation spike immediately after Pools are clear, which impacts both
throughput and latency.
This CL fixes this by introducing a victim cache mechanism. Instead of
clearing Pools, the victim cache is dropped and the primary cache is
moved to the victim cache. As a result, in steady-state, there are
(roughly) no new allocations, but if Pool usage drops, objects will
still be collected within two GCs (as opposed to one).
This victim cache approach also improves Pool's impact on GC dynamics.
The current approach causes all objects in Pools to be short lived.
However, if an application is in steady state and is just going to
repopulate its Pools, then these objects impact the live heap size *as
if* they were long lived. Since Pooled objects count as short lived
when computing the GC trigger and goal, but act as long lived objects
in the live heap, this causes GC to trigger too frequently. If Pooled
objects are a non-trivial portion of an application's heap, this
increases the CPU overhead of GC. The victim cache lets Pooled objects
affect the GC trigger and goal as long-lived objects.
This has no impact on Get/Put performance, but substantially reduces
the impact to the Pool user when a GC happens. PoolExpensiveNew
demonstrates this in the substantially reduction in the rate at which
the "New" function is called.
Austin Clements [Fri, 1 Mar 2019 20:33:33 +0000 (15:33 -0500)]
sync: use lock-free structure for Pool stealing
Currently, Pool stores each per-P shard's overflow in a slice
protected by a Mutex. In order to store to the overflow or steal from
another shard, a P must lock that shard's Mutex. This allows for
simple synchronization between Put and Get, but has unfortunate
consequences for clearing pools.
Pools are cleared during STW sweep termination, and hence rely on
pinning a goroutine to its P to synchronize between Get/Put and
clearing. This makes the Get/Put fast path extremely fast because it
can rely on quiescence-style coordination, which doesn't even require
atomic writes, much less locking.
The catch is that a goroutine cannot acquire a Mutex while pinned to
its P (as this could deadlock). Hence, it must drop the pin on the
slow path. But this means the slow path is not synchronized with
clearing. As a result,
1) It's difficult to reason about races between clearing and the slow
path. Furthermore, this reasoning often depends on unspecified nuances
of where preemption points can occur.
2) Clearing must zero out the pointer to every object in every Pool to
prevent a concurrent slow path from causing all objects to be
retained. Since this happens during STW, this has an O(# objects in
Pools) effect on STW time.
3) We can't implement a victim cache without making clearing even
slower.
This CL solves these problems by replacing the locked overflow slice
with a lock-free structure. This allows Gets and Puts to be pinned the
whole time they're manipulating the shards slice (Pool.local), which
eliminates the races between Get/Put and clearing. This, in turn,
eliminates the need to zero all object pointers, reducing clearing to
O(# of Pools) during STW.
In addition to significantly reducing STW impact, this also happens to
speed up the Get/Put fast-path and the slow path. It somewhat
increases the cost of PoolExpensiveNew, but we'll fix that in the next
CL.
Austin Clements [Fri, 8 Mar 2019 20:01:34 +0000 (15:01 -0500)]
sync: add Pool benchmarks to stress STW and reuse
This adds two benchmarks that will highlight two problems in Pool that
we're about to address.
The first benchmark measures the impact of large Pools on GC STW time.
Currently, STW time is O(# of items in Pools), and this benchmark
demonstrates 70µs STW times.
The second benchmark measures the impact of fully clearing all Pools
on each GC. Typically this is a problem in heavily-loaded systems
because it causes a spike in allocation. This benchmark stresses this
by simulating an expensive "New" function, so the cost of creating new
objects is reflected in the ns/op of the benchmark.
Austin Clements [Fri, 1 Mar 2019 19:54:00 +0000 (14:54 -0500)]
sync: internal dynamically sized lock-free queue for sync.Pool
This adds a dynamically sized, lock-free, single-producer,
multi-consumer queue that will be used in the new Pool stealing
implementation. It's built on top of the fixed-size queue added in the
previous CL.
Austin Clements [Fri, 1 Mar 2019 18:16:37 +0000 (13:16 -0500)]
sync: internal fixed size lock-free queue for sync.Pool
This is the first step toward fixing multiple issues with sync.Pool.
This adds a fixed size, lock-free, single-producer, multi-consumer
queue that will be used in the new Pool stealing implementation.
Jay Conrod [Wed, 9 Jan 2019 21:40:01 +0000 (16:40 -0500)]
cmd/go: parallelize package loading
load.PackageAndErrors now preloads data used to build load.Package
structures. Multiple packages may be preloaded in parallel, so this
parallelizes most of the package loading work.
The actual package construction and error-checking process is still
sequential, since this process needs to detect and report cycles.
Fixes #29758
Change-Id: Icf37e6669836ce8aad076e34fd895f97f4f3f9e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/161397
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Austin Clements [Mon, 31 Dec 2018 00:47:27 +0000 (19:47 -0500)]
runtime: ring buffer for binary debug logging
This adds an internal runtime debug log. It uses per-M time-stamped
ring buffers of binary log records. On panic, these buffers are
collected, interleaved, and printed.
The entry-point to the debug log is a new "dlog" function. dlog is
designed so it can be used even from very constrained corners of the
runtime such as signal handlers or inside the write barrier.
The facility is only enabled if the debuglog build tag is set.
Otherwise, it compiles away to a no-op implementation.
The debug log format is also designed so it would be reasonable to
decode from a core dump, though this hasn't been implemented.
grant [Wed, 3 Apr 2019 14:21:21 +0000 (14:21 +0000)]
net: use libSystem bindings for DNS resolution on macos if cgo is unavailable
This change adds directives to link the res_search function in libSystem.
The corresponding Go function is then used in `lookup_darwin.go` for
resolution when cgo is disabled. This makes DNS resolution logic more
reliable as macOS has some unique quirks such as the `/etc/resolver/`
directory for specifying nameservers.
Than McIntosh [Thu, 4 Apr 2019 15:01:22 +0000 (11:01 -0400)]
math/bits: add gccgo-friendly code for compiler bootstrap
When building as part of the bootstrap process, avoid
use of "go:linkname" applied to variables, since this
feature is ill-defined/unsupported for gccgo.
Updates #30771.
Change-Id: Id44d01b5c98d292702e5075674117518cb59e2d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170737 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Richard Musiol [Sun, 24 Mar 2019 11:14:27 +0000 (12:14 +0100)]
cmd/compile: add saturating conversions on wasm
This change adds the GOWASM option "satconv" to enable the generation
of experimental saturating (non-trapping) float-to-int conversions.
It improves the performance of the conversion by 42%.
Previously the conversions had already been augmented with helper
functions to have saturating behavior. Now Wasm.rules is always using
the new operation names and wasm/ssa.go is falling back to the helpers
if the feature is not enabled.
The feature is in phase 4 of the WebAssembly proposal process:
https://github.com/WebAssembly/meetings/blob/master/process/phases.md
More information on the feature can be found at:
https://github.com/WebAssembly/nontrapping-float-to-int-conversions/blob/master/proposals/nontrapping-float-to-int-conversion/Overview.md
Robert Griesemer [Thu, 28 Mar 2019 20:32:31 +0000 (13:32 -0700)]
cmd/compile: better recovery after := (rather than =) in declarations
Before this fix, a mistaken := in a (const/type/var) declaration
ended that declaration with an error from which the parser didn't
recover well. This low-cost change will provide a better error
message and lets the parser recover perfectly.
Fixes #31092.
Change-Id: Ic4f94dc5e29dd00b7ef6d53a80dded638e3cea80
Reviewed-on: https://go-review.googlesource.com/c/go/+/169958 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Keith Randall [Wed, 20 Mar 2019 17:47:17 +0000 (10:47 -0700)]
syscall: avoid _getdirentries64 on darwin
Getdirentries is implemented with the __getdirentries64 function
in libSystem.dylib. That function works, but it's on Apple's
can't-be-used-in-an-app-store-application list.
Implement Getdirentries using the underlying fdopendir/readdir_r/closedir.
The simulation isn't faithful, and could be slow, but it should handle
common cases.
Don't use Getdirentries in the stdlib, use fdopendir/readdir_r/closedir
instead (via (*os.File).readdirnames).
smasher164 [Thu, 7 Feb 2019 08:38:21 +0000 (03:38 -0500)]
cmd/compile: return assignment mismatch error in var declarations
Some var declarations return "extra expression" or "missing expression"
errors when they should return “assignment mismatch” instead. Change
the returned error messages to exhibit the desired behavior.
Fixes #30085.
Change-Id: I7189355fbb0f976d70100779db4f81a9ae64fb11
Reviewed-on: https://go-review.googlesource.com/c/go/+/161558 Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Bryan C. Mills [Tue, 8 Jan 2019 15:34:16 +0000 (10:34 -0500)]
cmd/go/internal/web: reject insecure redirects from secure origins
We rely on SSL certificates to verify the identity of origin servers.
If an HTTPS server redirects through a plain-HTTP URL, that hop can be
compromised. We should allow it only if the user set the -insecure
flag explicitly.
Jay Conrod [Mon, 11 Mar 2019 22:53:08 +0000 (18:53 -0400)]
cmd/go: print require chains in build list errors
mvs.BuildList and functions that invoke it directly (UpgradeAll) now
return an *mvs.BuildListError when there is an error retrieving the
requirements for a module. This new error prints the chain of
requirements from the main module to the module where the error
occurred.
These errors come up most commonly when a go.mod file has an
unexpected module path or can't be parsed for some other reason. It's
currently difficult to debug these errors because it's not clear where
the "bad" module is required from. Tools like "go list -m" and
"go mod why" don't work without the build graph.
Fixes #30661
Change-Id: I3c9d4683dcd9a5d7c259e5e4cc7e1ee209700b10
Reviewed-on: https://go-review.googlesource.com/c/go/+/166984
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
cmd/go/internal/lockedfile: add a sync.Mutex to lockedfile.Mutex
The compiler (and race detector) don't interpret locking a file as a
synchronization operation, so we add an explicit (and redundant)
sync.Mutex to make that property clear.
The additional synchronization makes it safe to parallelize the tests
in cmd/go/internal/modfetch/coderepo_test.go, which cuts the wall time
of that test by around 50%.
Updates #30550
Change-Id: Ief3479020ebf9e0fee524a4aae5568697727c683
Reviewed-on: https://go-review.googlesource.com/c/go/+/170597
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Jay Conrod <jayconrod@google.com>
cmd/go: add a note in help buildmode for c-archive on AIX
As ld on AIX doesn't keep the same layout in .text section,
-Wl,-bnoobjreoder must be passed to gcc when building a C program with a
Go archive.
Change-Id: I89b584cce43ab5792f315192b073923c10d5690e
Reviewed-on: https://go-review.googlesource.com/c/go/+/170538
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Fri, 23 Nov 2018 16:56:23 +0000 (16:56 +0000)]
encoding/json: speed up tokenization of literals
Decoder.Decode and Unmarshal actually scan the input bytes twice - the
first time to check for syntax errors and the length of the value, and
the second to perform the decoding.
It's in the second scan that we actually tokenize the bytes. Since
syntax errors aren't a possibility, we can take shortcuts.
In particular, literals such as quoted strings are very common in JSON,
so we can avoid a lot of work by special casing them.
cmd/go/internal/modload: fix aliasing bug in (*mvsReqs).Required with -mod=vendor
(*mvsReqs).Required assumes that it is safe to mutate the slice
returned by (*mvsReqs).required. In most cases, that was true, but in
the case of -mod=vendor it resulted in unsynchronized (and
potentially interfering) writes to the global vendorList.
splittests already contains most of the tests that cover explode. Add
the missing ones and skip the append test for empty results which would
otherwise lead to an "index out of range" panic.
syscall: on AIX use nsendmsg and nrecvmsg, define SockaddrDatalink
This commit changes sendmsg, recvmsg to use nsendmsg, nrecvmsg on AIX.
These syscalls support the new msghdr structure (with Control
and Controllen) which is needed for golang.org/x/net.
Also define SockaddrDataLink.
Change-Id: I233fbd24f9eb86648e0d4d50c2b56da3626292d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Agniva De Sarker [Mon, 7 Jan 2019 16:45:47 +0000 (22:15 +0530)]
image/jpeg: reduce bound checks from idct and fdct
Before -
$gotip build -gcflags="-d=ssa/check_bce/debug=1" fdct.go idct.go
./fdct.go:89:10: Found IsInBounds
./fdct.go:90:10: Found IsInBounds
./fdct.go:91:10: Found IsInBounds
./fdct.go:92:10: Found IsInBounds
./fdct.go:93:10: Found IsInBounds
./fdct.go:94:10: Found IsInBounds
./fdct.go:95:10: Found IsInBounds
./fdct.go:96:10: Found IsInBounds
./idct.go:77:9: Found IsInBounds
./idct.go:77:27: Found IsInBounds
./idct.go:77:45: Found IsInBounds
./idct.go:78:7: Found IsInBounds
./idct.go:78:25: Found IsInBounds
./idct.go:78:43: Found IsInBounds
./idct.go:78:61: Found IsInBounds
./idct.go:79:13: Found IsInBounds
./idct.go:92:13: Found IsInBounds
./idct.go:93:12: Found IsInBounds
./idct.go:94:12: Found IsInBounds
./idct.go:95:12: Found IsInBounds
./idct.go:97:12: Found IsInBounds
./idct.go:98:12: Found IsInBounds
./idct.go:99:12: Found IsInBounds
After -
$gotip build -gcflags="-d=ssa/check_bce/debug=1" fdct.go idct.go
./fdct.go:90:9: Found IsSliceInBounds
./idct.go:76:11: Found IsSliceInBounds
./idct.go:145:11: Found IsSliceInBounds
Ian Lance Taylor [Tue, 2 Apr 2019 22:05:33 +0000 (15:05 -0700)]
misc/cgo/testcarchive: skip TestSignalForwardingExternal on darwin/amd64
On darwin/amd64 the runtime method sigctxt.fixsigcode changes SIGSEGV
signals so that they are never marked SI_USER. CL 169120 changed the
signal handler to call fixsigcode even when the signal is delivered to
a non-Go thread. This breaks TestSignalForwardingExternal, so skip it.
Change-Id: I6740fb5a8f4f854ca69793537a983a696da3b495
Reviewed-on: https://go-review.googlesource.com/c/go/+/170446
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Elias Naur [Tue, 2 Apr 2019 21:58:43 +0000 (23:58 +0200)]
runtime/cgo: look for android_get_device_api_level in libc.so
The presence of the android_get_device_api_level symbol is used to
detect Android Q or later. Use the suggestion by Ryan Prichard and
look for it in libc.so and not in the entire program where someone
else might have defined it.
Manually tested on an Android Q amd64 emulator and arm64 Pixel.
Package unsafe's safety rules require that pointers converted to
uintptr must be converted back to pointer-type before being stored
into memory. In particular, storing a pointer into a non-pointer-typed
expression does not guarantee the pointer stays valid, even if the
expression refers to a pointer-typed variable.
wasm's StorepNoWB implementation violates these rules by storing a
pointer through a uintptr-typed expression.
This happens to work today because esc.go is lenient in its
implementation of package unsafe's rules, but my escape analysis
rewrite follows them more rigorously, which causes val to be treated
as a non-leaking parameter.
This CL fixes the issue by using a *T-typed expression, where T is
marked //go:notinheap so that the compiler still omits the write
barrier as appropriate.
The certificates argument to verifyServerCertificate must contain
at least one certificate. Simplify the intermediate certificate
handling code accordingly.
Matthew Dempsky [Mon, 1 Apr 2019 21:01:58 +0000 (14:01 -0700)]
cmd/compile: trim more unnecessary escape analysis messages
"leaking closure reference" is redundant for similar reasons as "&x
escapes to heap" for OADDR nodes: the reference itself does not
allocate, and we already report when the referenced variable is moved
to heap.
"mark escaped content" is redundant with "leaking param content".
Alessandro Arzilli [Mon, 25 Feb 2019 12:56:18 +0000 (13:56 +0100)]
compile,link: export package name in debug_info
Add a new custom attribute to compile units containing the package name
of the package (i.e. the name after the 'package' keyword), so that
debuggers can know it when it's different from the last segment
of the package path.
Matthew Dempsky [Mon, 1 Apr 2019 18:58:33 +0000 (11:58 -0700)]
cmd/compile: skip escape analysis diagnostics for OADDR
For most nodes (e.g., OPTRLIT, OMAKESLICE, OCONVIFACE), escape
analysis prints "escapes to heap" or "does not escape" to indicate
whether that node's allocation can be heap or stack allocated.
These messages are also emitted for OADDR, even though OADDR does not
actually allocate anything itself. Moreover, it's redundant because
escape analysis already prints "moved to heap" diagnostics when an
OADDR node like "&x" causes x to require heap allocation.
Because OADDR nodes don't allocate memory, my escape analysis rewrite
doesn't naturally emit the "escapes to heap" / "does not escape"
diagnostics for them. It's also non-trivial to replicate the exact
semantics esc.go uses for OADDR.
Since there are so many of these messages, I'm disabling them in this
CL by themselves. I modified esc.go to suppress the Warnl calls
without any other behavior changes, and then used a shell script to
automatically remove any ERROR messages mentioned by run.go in
"missing error" or "no match for" lines.
Fixes #16300.
Updates #23109.
Change-Id: I3993e2743c3ff83ccd0893f4e73b366ff8871a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/170319
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
Mark Ryan [Mon, 18 Mar 2019 14:48:34 +0000 (15:48 +0100)]
cmd/asm: Fix EVEX RIP-relative addressing
AVX-512 instructions that use RIP-relative addressing and require the
R bit of the EVEX prefix to be zero, i.e., instructions that use Z8-Z15 or
Z24-Z31, are incorrectly encoded by the assembler. The reason is that
the location of the offset at which the relative address is to be written
is incorrectly computed when the R bit is clear.
For example,
VMOVUPS bInitX<>+0(SB), Z0
encodes correctly to
62 f1 7c 48 10 05 66 e9 02 00
whereas
VMOVUPS bInitX<>+0(SB), Z8
encodes incorrectly to
62 71 7c 48 10 05 00 56 e9 02 00
Note the extra zero byte between the ModR/M byte (05) and the relative
address starting with 56. This error results in the first byte of the
following instruction being overwritten and typically, a program crash.
This commit fixes the issue in the same way that is fixed for VEX encoded
instructions, by simply not incrementing the offset for EVEX instructions.
Existing test code created for a similar VEX encoding issue (19518) has
been modified to also test for the issue addressed by this commit.
LE Manh Cuong [Fri, 29 Mar 2019 20:16:44 +0000 (03:16 +0700)]
cmd/compile: use FmtLeft to generate symbol name for unexported interface methods
The bug in 29612 is that there are two similar-looking anonymous interface
types in two different packages, ./p1/ssa and ./p2/ssa:
v.(interface{ foo() }).foo()
These types should be treated differently because the unexported method
makes the types different (according to the spec).
But when generating the type descriptors for those two types, they
both have the name "interface { ssa.foo() }". They thus get the same
symbol, and the linker happily unifies them. It picks an arbitrary one
for the runtime to use, but that breaks conversions from concrete types
that have a foo method from the package which had its interface type
overwritten.
We need to encode the metadata symbol for unexported methods as package
path qualified (The same as we did in CL 27791 for struct fields).
So switching from FmtUnsigned to Fmtleft by default fixes the issue.
In case of generating namedata, FmtUnsigned is used.
The benchmark result ends up in no significant change of compiled binary
compare to the immediate parent.
Fixes #29612
Change-Id: I775aff91ae4a1bb16eb18a48d55e3b606f3f3352
Reviewed-on: https://go-review.googlesource.com/c/go/+/170157 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Jay Conrod [Fri, 15 Mar 2019 17:41:48 +0000 (13:41 -0400)]
cmd/go: refactor load.LoadPackage into other functions
LoadPackage was used to load a *load.Package for a command line
argument, after pattern expansion. It provided two special cases on
top of LoadImport. First, it ensured that "cmd/" packages in GOROOT
were installed in "$GOROOT/bin" or "$GOROOT/pkg/tool". Second, it
translated absolute paths to packages in GOROOT and GOPATH into
regular import paths.
With this change, LoadImport now ensures "cmd/" packages have the
right Target (without the need for a special case) and
search.ImportPaths translates absolute paths.
LoadPackage no longer handles these special cases and has been renamed
to LoadImportWithFlags, since it's still useful for loading implicit
dependencies.
Updates #29758
Change-Id: I9d54036f90c3ccd9b3a0fe0eaddaa7749593cc91
Reviewed-on: https://go-review.googlesource.com/c/go/+/167748
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Segev Finer [Fri, 29 Mar 2019 09:35:06 +0000 (09:35 +0000)]
cmd/doc: correctly indent pre-formatted blocks
They were previously indented at the same level as the normal text when
printing a single symbol or the description of a field.
Running "go doc text/template Must":
Before:
func Must(t *Template, err error) *Template
Must is a helper that wraps a call to a function returning (*Template,
error) and panics if the error is non-nil. It is intended for use in
variable initializations such as
var t = template.Must(template.New("name").Parse("text"))
After:
func Must(t *Template, err error) *Template
Must is a helper that wraps a call to a function returning (*Template,
error) and panics if the error is non-nil. It is intended for use in
variable initializations such as
var t = template.Must(template.New("name").Parse("text"))
Running "go doc http Request.Header":
Before:
type Request struct {
// Header contains the request header fields either received
// by the server or to be sent by the client.
//
// If a server received a request with header lines,
//
// Host: example.com
// accept-encoding: gzip, deflate
// Accept-Language: en-us
// fOO: Bar
// foo: two
//
// then
//
// Header = map[string][]string{
// "Accept-Encoding": {"gzip, deflate"},
// "Accept-Language": {"en-us"},
// "Foo": {"Bar", "two"},
// }
...
After:
type Request struct {
// Header contains the request header fields either received by the server or
// to be sent by the client.
//
// If a server received a request with header lines,
//
// Host: example.com
// accept-encoding: gzip, deflate
// Accept-Language: en-us
// fOO: Bar
// foo: two
//
// then
//
// Header = map[string][]string{
// "Accept-Encoding": {"gzip, deflate"},
// "Accept-Language": {"en-us"},
// "Foo": {"Bar", "two"},
// }
...
Fixes #29708
Change-Id: Ibe1a6a7a76d6b19c5737ba6e8210e3ad0b88ce16
GitHub-Last-Rev: 439c0fe70a01490cbd9c3613eba3fe45a3ffd9be
GitHub-Pull-Request: golang/go#31120
Reviewed-on: https://go-review.googlesource.com/c/go/+/169957
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Michael Munday [Sun, 31 Mar 2019 14:23:07 +0000 (15:23 +0100)]
runtime: always mask shift amount regardless of architecture
Currently the shift amount is only masked on x86. Change it so it
is masked on all architectures. In the worst case we generate a
couple of extra instructions to perform the masking and in the best
case we can elide overflow checks.
This particular shift could also be replaced with a rotate
instruction during optimization which would remove both the masking
instructions and overflow checks on all architectures.
Fixes #31165.
Change-Id: I16b7a8800b4ba8813dc83735dfc59564e661d3b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/170122
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Daniel Martí [Fri, 8 Mar 2019 18:12:07 +0000 (18:12 +0000)]
cmd/go: further reduce init work
The first biggest offender was crypto/des.init at ~1%. It's
cryptographically broken and the init function is relatively expensive,
which is unfortunate as both crypto/tls and crypto/x509 (and by
extension, cmd/go) import it. Hide the work behind sync.Once.
The second biggest offender was flag.sortFlags at just under 1%, used by
the Visit flagset methods. It allocated two slices, which made a
difference as cmd/go iterates over multiple flagsets during init.
Use a single slice with a direct sort.Interface implementation.
Another big offender is initializing global maps. Reducing this work in
cmd/go/internal/imports and net/textproto gives us close to another
whole 1% in saved work. The former can use map literals, and the latter
can hide the work behind sync.Once.
Finally, compress/flate used newHuffmanBitWriter as part of init, which
allocates many objects and slices. Yet it only used one of the slice
fields. Allocating just that slice saves a surprising ~0.3%, since we
generated a lot of unnecessary garbage.
All in all, these little pieces amount to just over 3% saved CPU time.
name old time/op new time/op delta
ExecGoEnv-8 3.61ms ± 1% 3.50ms ± 0% -3.02% (p=0.000 n=10+10)
zdjones [Sat, 30 Mar 2019 17:28:05 +0000 (17:28 +0000)]
cmd/compile: make prove learn index >= 0 from successful bounds checks
When branching at a bounds check for indexing or slicing ops, prove currently
only learns from the upper bound. On the positive branch, we currently learn
i < len(a) (or i <= len(a)) in both the signed and unsigned domains.
This CL makes prove also learn from the lower bound. Specifically, on the
positive branch from index or slicing ops, prove will now ALSO learn i >= 0 in
the signed domain (this fact is of no value in the unsigned domain).
The substantive change itself is only an additional call to addRestrictions,
though I've also inverted the nested switch statements around that call for the
sake of clarity.
This CL removes 92 bounds checks from std and cmd. It passes all tests and
shows no deltas on compilecmp.
Fixes #28885
Change-Id: I13eccc36e640eb599fa6dc5aa3be3c7d7abd2d9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/170121
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Giovanni Bajo <rasky@develer.com>
zdjones [Fri, 29 Mar 2019 19:17:35 +0000 (19:17 +0000)]
cmd/compile: preempt repeated checks for the zero constant in prove
Prove requires access to a zero-valued constant in multiple heavily-used
code paths. Currently, prove is checking for the existence of the constant on
every iteration of these paths, and creating it if not found.
This CL preempts all of these checks by finding or creating the zero constant
Value, just once, when the factsTable is initialised on entry to prove(). The
Method used to initialise the zero constant, func.ConstInt64(), finds an
existing constant if present, or creates one in the entry block otherwise.
Fixes #31141
Change-Id: Ic9a2fd9d79b67025e24d4483f6e87cf8213ead24
Reviewed-on: https://go-review.googlesource.com/c/go/+/170118 Reviewed-by: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jay Conrod [Wed, 27 Mar 2019 00:21:21 +0000 (20:21 -0400)]
cmd/go: clarify error when package is removed in a module
If no module in the build list provides an imported package, we
try to upgrade to the "@latest" version. If there is a requirement on
a version of the module which is newer than the "@latest" version
(e.g., a prerelease or pseudoversion), we cannot upgrade further.
We previously reported "looping trying to add package" when we saw the
package in "@latest" but it was removed later. The meaning of this is
unclear for users, so with this change, we explain the package was
removed.
Fixes #30394
Change-Id: I1b7fec2c37e762fb600e66ee8a4df4aeaf13e67a
Reviewed-on: https://go-review.googlesource.com/c/go/+/169720
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
David Chase [Wed, 19 Sep 2018 20:20:35 +0000 (16:20 -0400)]
cmd/compile: enhance induction variable detection for unrolled loops
Would suggest extending capabilities (32-bit, unsigned, etc)
in separate CLs because prove bugs are so mystifying.
This implements the suggestion in this comment
https://go-review.googlesource.com/c/go/+/104041/10/src/cmd/compile/internal/ssa/loopbce.go#164
for inferring properly bounded iteration for loops of the form
for i := K0; i < KNN-(K-1); i += K
for i := K0; i <= KNN-K; i += K
Where KNN is "known non negative" (i.e., len or cap) and K
is also not negative. Because i <= KNN-K, i+K <= KNN and
no overflow occurs.
Also handles decreasing case (K1 > 0)
for i := KNN; i >= K0; i -= K1
which works when MININT+K1 < K0
(i.e. MININT < K0-K1, no overflow)
Signed only, also only 64 bit for now.
Change-Id: I5da6015aba2f781ec76c4ad59c9c48d952325fdc
Reviewed-on: https://go-review.googlesource.com/c/go/+/136375
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
Joel Sing [Wed, 27 Mar 2019 13:53:19 +0000 (00:53 +1100)]
cmd/link: permit duplicate weak symbols
Permit weak symbols to be duplicates - most external linkers allow
this and there are various situations where they can occur (including
retpoline and retguard).
Fixes #29563
Change-Id: I355493c847fbc8f670a85a643db65a4cf8f9883d
Reviewed-on: https://go-review.googlesource.com/c/go/+/169658
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Munday [Thu, 7 Mar 2019 13:18:38 +0000 (13:18 +0000)]
cmd/asm: add 'insert program mask' instruction for s390x
This CL adds the 'insert program mask' (IPM) instruction to s390x.
IPM stores the current program mask (which contains the condition
code) into a general purpose register.
This instruction will be useful when implementing intrinsics for
the arithmetic functions in the math/bits package. We can also
potentially use it to convert some condition codes into bool
values.
The condition code can be saved and restored using an instruction
sequence such as:
IPM R4 // save condition code to R4
...
TMLH R4, $0x3000 // restore condition code from R4
We can also use IPM to save the carry bit to a register using an
instruction sequence such as:
IPM R4 // save condition code to R4
RISBLGZ $31, $31, $3, R4, R4 // isolate carry bit in R4
Elias Naur [Fri, 29 Mar 2019 11:13:02 +0000 (12:13 +0100)]
runtime/cgo: use free TLS slot on Android Q
Android assumes pthread tls keys correspond to some offset from the
TLS base. This is about to change in a future version of Android.
Fortunately, Android Q leaves a slot open for use to use, TLS_SLOT_APP.
Fixes #29674
Change-Id: Id6ba19afacdfed9b262453714715435e2544185f
Reviewed-on: https://go-review.googlesource.com/c/go/+/170117
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jordan Liggitt [Fri, 29 Mar 2019 03:37:54 +0000 (23:37 -0400)]
net/http/httputil: make ReverseProxy flush headers on FlushInterval
A regression was introduced in CL 137335 (5440bfc) that caused FlushInterval
to not be honored until the first Write() call was encountered. This change
starts the flush timer as part of setting up the maxLatencyWriter.
Ian Lance Taylor [Wed, 27 Mar 2019 21:59:10 +0000 (14:59 -0700)]
runtime: rename p racectx field to raceprocctx
Both g and p had a racectx field, but they held different kinds of values.
The g field held ThreadState values while the p field held Processor values
(to use the names used in the C++ code in the compiler_rt support library).
Rename the p field to raceprocctx to reduce potential confusion.
Change-Id: Iefba0e259d240171e973054c452c3c15bf3f8f8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/169960 Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ben Hoyt [Thu, 28 Mar 2019 21:49:43 +0000 (17:49 -0400)]
bytes, strings: add tests for TrimLeftFunc and TrimRightFunc
When I was working on the fix for #31038 (make TrimSpace return nil on
all-space input) I noticed that there were no tests for TrimLeftFunc
and TrimRightFunc, including the funky nil behavior. So add some!
I've just reused the existing TrimFunc test cases for TrimLeftFunc and
TrimRightFunc, as well as adding new tests for the empty string and
all-trimmed cases (which test the nil-returning behavior of TrimFunc and
TrimLeftFunc).
Change-Id: Ib580d4364e9b3c91350305f9d9873080d7862904
Reviewed-on: https://go-review.googlesource.com/c/go/+/170061
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
zdjones [Wed, 6 Feb 2019 19:49:15 +0000 (19:49 +0000)]
cmd/compile: make prove use poset to check non-negatives
Prove currently fails to remove bounds checks of the form:
if i >= 0 { // hint that i is non-negative
for i < len(data) { // i becomes Phi in the loop SSA
_ = data[i] // data[Phi]; bounds check!!
i++
}
}
addIndVarRestrictions fails to identify that the loop induction
variable, (Phi), is non-negative. As a result, the restrictions,
i <= Phi < len(data), are only added for the signed domain. When
testing the bounds check, addBranchRestrictions is similarly unable
to infer that Phi is non-negative. As a result, the restriction,
Phi >= len(data), is only added/tested for the unsigned domain.
This CL changes the isNonNegative method to utilise the factTable's
partially ordered set (poset). It also adds field factTable.zero to
allow isNonNegative to query the poset using the zero(0) constant
found or created early in prove.
Fixes #28956
Change-Id: I792f886c652eeaa339b0d57d5faefbf5922fe44f
Reviewed-on: https://go-review.googlesource.com/c/go/+/161437
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Giovanni Bajo <rasky@develer.com>
Agniva De Sarker [Wed, 13 Feb 2019 03:19:52 +0000 (08:49 +0530)]
go/ast: fix SortImports to handle block comments
The current algorithm only assumed line comments which always
appear at the end of an import spec. This caused block comments
which can appear before a spec to be attached to the previous spec.
So while mapping a comment to an import spec, we maintain additional
information on whether the comment is supposed to appear on the left
or right of the spec.
And we also take into account the possibility of "//line" comments
in the source. So we use unadjusted line numbers.
While at it, added some more testcases from tools/go/ast/astutil/imports_test.go
Fixes #18929
Change-Id: If920426641702a8a93904b2ec1d3455749169f69
Reviewed-on: https://go-review.googlesource.com/c/go/+/162337
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
runtime: fix TestLldbPython in module mode (again)
When run with GOPATH=/dev/null, go build fails:
$ GOPATH=/dev/null go test -run=TestLldbPython -v -count=1 runtime
=== RUN TestLldbPython
--- FAIL: TestLldbPython (0.21s)
runtime-lldb_test.go:169: building source exit status 1
go: failed to create cache directory /dev/null/pkg/mod/cache: mkdir /dev/null: not a directory
FAIL
FAIL runtime 0.220s
But run.bash sets GOPATH=/dev/null.
Fix this by setting GOPATH to the empty string before passing to 'go build'.
Jay Conrod [Tue, 19 Mar 2019 20:36:21 +0000 (16:36 -0400)]
cmd/go: recognize android suffix when constructing build list
cmd/go/internal/imports.ScanDir extracts a list of imports from a
directory. It's used instead of go/build.ImportDir when constructing
the build list. GOOS and GOARCH may be used to filter files.
With this change, imports.MatchFile understands that when the
"android" tag is set, the "linux" tag is implied.
Fixes #30888
Change-Id: Ia29bd1590b69c9183ab14a879d5fc1b639f8eaef
Reviewed-on: https://go-review.googlesource.com/c/go/+/168378
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Daniel Cormier [Fri, 10 Nov 2017 16:50:13 +0000 (11:50 -0500)]
net/textproto: properly write terminating sequence if DotWriter is closed with no writes
Fixed textproto.Writer.DotWriter() to properly write \r\n.\r\n to the buffer
when Close() is called without any bytes written. This properly writes the
terminating sequence outlined in RFC 5321 section 4.1.1.4 and RFC 3977
section 3.1.1, even when no other bytes are written.
Richard Musiol [Sat, 23 Mar 2019 14:25:42 +0000 (15:25 +0100)]
cmd/compile: add sign-extension operators on wasm
This change adds the GOWASM option "signext" to enable
the generation of experimental sign-extension operators.
The feature is in phase 4 of the WebAssembly proposal process:
https://github.com/WebAssembly/meetings/blob/master/process/phases.md
More information on the feature can be found at:
https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md
Clément Chigot [Tue, 26 Mar 2019 13:55:48 +0000 (14:55 +0100)]
cmd/vet/all: enable AIX checks
Fixes #27985
Change-Id: I2f3d06ced9da9fc56f30f1285a8d393e689c29ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/169019
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>