Than McIntosh [Sat, 2 Nov 2019 13:36:28 +0000 (09:36 -0400)]
[dev.link] cmd/link/internal/loader: reduce ABI alias postprocessing overhead
In loadObjFull when populating the sym.Reloc vector for live symbols,
avoid using the loader.SymType() method to determine if a relocation
targets an ABI alias; since invoking loader.SymType requires a
global-to-local index translation and a read from the object file.
Instead just look at the target symbol itself, which has already been
created at this point.
Hyperkube performance numbers for this change:
name old time/op new time/op delta
RelinkHyperkube 29.9s ± 2% 29.2s ± 3% -2.42% (p=0.000 n=20+20)
RelinkWithoutDebugHyperkube 22.0s ± 3% 21.4s ± 3% -2.58% (p=0.000 n=20+20)
Change-Id: Ib7696d8760dd0485240246d6d640668fbf451d71
Reviewed-on: https://go-review.googlesource.com/c/go/+/205257
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org>
Cherry Zhang [Sat, 2 Nov 2019 21:25:39 +0000 (17:25 -0400)]
[dev.link] cmd/link: use the start of compilation unit for R_ADDRCUOFF
R_ADDRCUOFF refers to the offset from the start of the
compilation unit. Now that we could have multiple compilation
units per package, we should use the start of the compilation
unit, instead of the start of the package.
Cherry Zhang [Sat, 2 Nov 2019 14:42:50 +0000 (10:42 -0400)]
[dev.link] cmd/internal/dwarf: expand import path in function DIE
Currently, at compile time we emit a function DIE with '"".' in
the function name, and we expand it at link time, with a really
ugly function. We can just expand it at compile time instead.
This way, we don't need to modify the symbol content at link time,
and also no need to allocate memory for that.
Keep the linker expansion, in case the compiler is invoked
without the import path.
Change-Id: Id53cd2e2d3eb61efceb8d44479c4b6ef890baa43
Reviewed-on: https://go-review.googlesource.com/c/go/+/204826 Reviewed-by: Than McIntosh <thanm@google.com>
Cherry Zhang [Thu, 31 Oct 2019 02:24:27 +0000 (22:24 -0400)]
[dev.link] cmd/link: use min-heap for work queue for better locality
In the deadcode pass, we use a work queue for the flood algorithm.
Currently this is a simple LIFO queue. In this order, there is
poor locality in accessing object files.
Since the global indices are assigned in package DAG order, edges
are mostly either within a package or from a smaller index to a
larger one. (With named symbols, there can be backward edges, but
shouldn't be too many.) Using a min-heap for the work queue, we
access all symbols in one object, then move to next one. It
rarely needs to revisit an object that is already visted. This
should result in better locality.
Benchmark result from Than (thanks!):
name old time/op new time/op delta
LinkCompiler 1.74s ±11% 1.61s ± 9% -7.80% (p=0.000 n=20+19)
LinkWithoutDebugCompiler 1.27s ±11% 1.15s ± 9% -9.02% (p=0.000 n=20+20)
Currently this uses the container/heap package, which uses
interface elements. If this allocates too much, we may consider
to hand-code the min heap.
Change-Id: I216d5291c432fe1f40b0b8f4f1b9d388807bf6c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/204438 Reviewed-by: Jeremy Faller <jeremy@golang.org>
Cherry Zhang [Thu, 31 Oct 2019 01:24:22 +0000 (21:24 -0400)]
[dev.link] cmd/link: add internal packages at the end
Currently in the linker we load internal packges first, then the
main package, and then load imported packages following the
dependency graph. As a result, packages are loaded mostly in the
dependency order, except the internal packages. The global symbol
indices are assigned the same way.
By loading the internal packages at the end, the packages are
loaded in the dependency order, so are the global indices. This
way, a relocation edge is mostly either within a packge or a
forward edge from a smaller index to a larger one. This allows
us to use a min-heap work queue in the deadcode pass, to achieve
better spatial locality (in the next CL).
Change-Id: I01fa9b3cf0c9e9e66006040f6378a51fd78f0f39
Reviewed-on: https://go-review.googlesource.com/c/go/+/204437 Reviewed-by: Jeremy Faller <jeremy@golang.org>
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>
Cherry Zhang [Fri, 1 Nov 2019 14:58:27 +0000 (10:58 -0400)]
[dev.link] cmd/link: set cgo attributes early when internal cgo linking
In newobj mode, cgo attributes are typically set later, as we
create sym.Symbols later. But when internal cgo linking, the
host object loaders still work with sym.Symbols, and the cgo
attributes need to be set for them to work properly. Therefore,
set them early. This will cause creating some Symbols eagerly,
but they are mostly host object symbols and will need to be
created anyway.
Now all cgo internal linking tests pass on ELF systems.
Change-Id: I023a4df4429acc8ebf5e185f62e6809198497a78
Reviewed-on: https://go-review.googlesource.com/c/go/+/204857 Reviewed-by: Than McIntosh <thanm@google.com>
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)
Than McIntosh [Thu, 24 Oct 2019 15:27:47 +0000 (11:27 -0400)]
[dev.link] cmd/link/internal/loader: do more bulk allocation
Change the loader to do more bulk allocation when making slices of
small objects (sym.Reloc, etc) as part of creating and populating
sym.Symbols in loader.LoadFull(). This replaces a large number of
small allocations with a smaller number of large allocations,
improving performace. Compilebench numbers (linker portion) for this
change:
name old time/op new time/op delta
LinkCompiler 1.71s ±11% 1.57s ± 9% -8.35% (p=0.000 n=19+20)
LinkWithoutDebugCompiler 1.19s ±14% 1.10s ±13% -7.93% (p=0.000 n=20+19)
name old user-time/op new user-time/op delta
LinkCompiler 1.86s ±15% 1.34s ±10% -28.02% (p=0.000 n=20+20)
LinkWithoutDebugCompiler 1.05s ±14% 0.95s ± 9% -9.17% (p=0.000 n=19+20)
Hyperkube from kubernetes doesn't show any significant benefit (which
seems a little surprising).
Change-Id: Ide97f78532fb60b08bb6e4cfa097e9058f7ea8ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/203457
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Than McIntosh [Wed, 30 Oct 2019 16:31:55 +0000 (12:31 -0400)]
[dev.link] cmd/link: add support to new deadcode for field tracking
Fix up the new dead code pass to include support for populating the
ctxt "Reachparent" map, which is needed to support field tracking.
Since we don't have sym.Symbols created at the point where new dead
code runs, keep track of reachability using global symbol indices, and
then once loader.LoadFull is complete we can translate the index
mappings into symbol mappings.
The fieldtracking output is unfortunately different relative to
master, due to differences in the order in which symbols are
encountered in deadcode, but I have eyeballed the results to make sure
they look reasonable.
Cherry Zhang [Fri, 1 Nov 2019 01:53:49 +0000 (21:53 -0400)]
[dev.link] cmd/link: resolve ABI aliases for external symbols
ABI alias references in Go symbols are resolved during
loadObjFull. But for external symbols they are not resolved. If
there is a reference from an external symbol to a Go ABIInternal
symbol, this reference will be invalid as it is not resolved.
The old code resolve ABI aliases in the deadcode pass. But the
new deadcode pass doesn't do it, as it works with indices instead
of Symbols. We do this in LoadFull.
This makes all internal cgo linking tests pass on Mach-O.
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>
Cherry Zhang [Thu, 31 Oct 2019 23:35:55 +0000 (19:35 -0400)]
[dev.link] cmd/link: process cgo_import_static before host object loading
In internal linking mode, we need to process cgo_import_static
directives before loading host objects, because the directive is
to tell the host object loader how to deal with imported symbols.
This should fix linking with old object files. I think there
needs some similar logic for new object files, but I'll leave
that for later.
Austin Clements [Wed, 23 Oct 2019 15:25:38 +0000 (11:25 -0400)]
runtime: atomically set span state and use as publication barrier
When everything is working correctly, any pointer the garbage
collector encounters can only point into a fully initialized heap
span, since the span must have been initialized before that pointer
could escape the heap allocator and become visible to the GC.
However, in various cases, we try to be defensive against bad
pointers. In findObject, this is just a sanity check: we never expect
to find a bad pointer, but programming errors can lead to them. In
spanOfHeap, we don't necessarily trust the pointer and we're trying to
check if it really does point to the heap, though it should always
point to something. Conservative scanning takes this to a new level,
since it can only guess that a word may be a pointer and verify this.
In all of these cases, we have a problem that the span lookup and
check can race with span initialization, since the span becomes
visible to lookups before it's fully initialized.
Furthermore, we're about to start initializing the span without the
heap lock held, which is going to introduce races where accesses were
previously protected by the heap lock.
To address this, this CL makes accesses to mspan.state atomic, and
ensures that the span is fully initialized before setting the state to
mSpanInUse. All loads are now atomic, and in any case where we don't
trust the pointer, it first atomically loads the span state and checks
that it's mSpanInUse, after which it will have synchronized with span
initialization and can safely check the other span fields.
Austin Clements [Thu, 24 Oct 2019 13:00:27 +0000 (09:00 -0400)]
runtime: fully initialize span in alloc_m
Currently, several important fields of a heap span are set by
heapBits.initSpan, which happens after the span has already been
published and returned from the locked region of alloc_m. In
particular, allocBits is set very late, which makes mspan.isFree
unsafe even if you were to lock the heap because it tries to access
allocBits.
This CL fixes this by populating these fields in alloc_m. The next CL
builds on this to only publish the span once it is fully initialized.
Together, they'll make it safe to check allocBits even if there is a
race with alloc_m.
Olivier Poitrey [Fri, 10 May 2019 00:31:34 +0000 (17:31 -0700)]
crypto/tls: send ec_points_format extension in ServerHello
Follow the recommandation from RFC 8422, section 5.1.2 of sending back the
ec_points_format extension when requested by the client. This is to fix
some clients declining the handshake if omitted.
Daniel Martí [Wed, 21 Aug 2019 16:22:24 +0000 (18:22 +0200)]
encoding/json: avoid work when unquoting strings, take 2
This is a re-submission of CL 151157, since it was reverted in CL 190909
due to an introduced crash found by a fuzzer. The revert CL included
regression tests, while this CL includes a fixed version of the original
change.
In particular, what we forgot in the original optimization was that we
still need the length and trailing quote checks at the beginning of
unquoteBytes. Without those, we could end up in a crash later on.
We can work out how many bytes can be unquoted trivially in
rescanLiteral, which already iterates over a string's bytes.
Removing the extra loop in unquoteBytes simplifies the function and
speeds it up, especially when decoding simple strings, which are common.
While at it, we can remove the check that s[0]=='"', since all call
sites already meet that condition.
name old time/op new time/op delta
CodeDecoder-8 10.6ms ± 2% 10.5ms ± 1% -1.01% (p=0.004 n=20+10)
name old speed new speed delta
CodeDecoder-8 183MB/s ± 2% 185MB/s ± 1% +1.02% (p=0.003 n=20+10)
Updates #28923.
Change-Id: I8c6b13302bcd86a364bc998d72451332c0809cde
Reviewed-on: https://go-review.googlesource.com/c/go/+/190659
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Peter Weinberger <pjw@google.com>
Michael Munday [Wed, 30 Oct 2019 12:45:33 +0000 (12:45 +0000)]
runtime/internal/atomic: add tests for And8 and Or8
Add some simple unit tests for these atomic operations. These can't
catch all the bugs that are possible with these operations but at
least they provide some coverage.
Than McIntosh [Fri, 25 Oct 2019 16:39:20 +0000 (12:39 -0400)]
[dev.link] cmd/link: add new slice interface for querying aux symbols
Add a new loader.Loader.ReadAuxSyms method that returns a slice
containing the ids of the aux symbols for a specified global symbol.
This is similar to the new interface recently added that allows you to
get back a slice of relocations (as opposed to making calls into the
loader for each one). This was idea suggested by Cherry. Compilebench
numbers:
name old time/op new time/op delta
LinkCompiler 1.63s ± 9% 1.57s ± 7% -3.84% (p=0.006 n=20+20)
LinkWithoutDebugCompiler 1.15s ±11% 1.11s ±11% ~ (p=0.108 n=20+20)
name old user-time/op new user-time/op delta
LinkCompiler 1.99s ± 8% 2.00s ±12% ~ (p=0.751 n=19+19)
LinkWithoutDebugCompiler 1.14s ±11% 1.19s ±21% ~ (p=0.183 n=20+20)
Than McIntosh [Wed, 30 Oct 2019 14:14:33 +0000 (10:14 -0400)]
[dev.link] cmd/link/internal/loader: add 1-element object cache for use in toLocal
To speed up the loader.Loader.toLocal() method, cache the index of the
most recently accessed object file and check that object's sym range
in toLocal() before doing a full binary search over all object symbol
ranges. This speeds up relink of kubernetes/hyperkube by about 2%, and
improves compilebench (relative to the dev.link branch) by about 5%:
name old time/op new time/op delta
LinkCompiler 1.62s ± 8% 1.50s ± 9% -7.21% (p=0.000 n=20+19)
LinkWithoutDebugCompiler 1.13s ± 8% 1.09s ±12% ~ (p=0.052 n=20+20)
name old user-time/op new user-time/op delta
LinkCompiler 1.94s ±18% 1.97s ±16% ~ (p=0.813 n=19+20)
LinkWithoutDebugCompiler 1.15s ±16% 1.13s ±12% ~ (p=0.547 n=20+20)
Damien Neil [Thu, 30 May 2019 16:46:56 +0000 (09:46 -0700)]
testing: provide additional information when test funcs panic
Flush the output log up to the root when a test panics. Prior to
this change, only the current test's output log was flushed to its
parent, resulting in no output when a subtest panics.
For the following test function:
func Test(t *testing.T) {
for i, test := range []int{1, 0, 2} {
t.Run(fmt.Sprintf("%v/%v", i, test), func(t *testing.T) {
_ = 1 / test
})
}
}
Output before this change:
panic: runtime error: integer divide by zero [recovered]
panic: runtime error: integer divide by zero
(stack trace follows)
Output after this change:
--- FAIL: Test (0.00s)
--- FAIL: Test/1/0 (0.00s)
panic: runtime error: integer divide by zero [recovered]
(stack trace follows)
Fixes #32121
Change-Id: Ifee07ccc005f0493a902190a8be734943123b6b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/179599
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sam Whited [Tue, 21 Aug 2018 22:11:30 +0000 (17:11 -0500)]
encoding/xml: fix token decoder on early EOF
The documentation for TokenReader suggests that implementations of the
interface may return a token and io.EOF together, indicating that it is
the last token in the stream. This is similar to io.Reader. However, if
you wrap such a TokenReader in a Decoder it complained about the EOF.
A test was added to ensure this behavior on Decoder's.
Clément Chigot [Tue, 29 Oct 2019 14:39:42 +0000 (15:39 +0100)]
runtime: fix nbpipe_test for AIX
Fcntl can't be called using syscall.Syscall as it doesn't work on AIX.
Moreover, fcntl isn't exported by syscall package.
However, it can be accessed by exporting it from runtime package
using export_aix_test.go.
Change-Id: Ib6af66d9d7eacb9ca0525ebc4cd4c92951735f1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/204059
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Wed, 30 Oct 2019 12:56:12 +0000 (13:56 +0100)]
runtime: fix netpollBreak for AIX
Change-Id: I2629711ce02d935130fb2aab29f9028b62ba9fe6
Reviewed-on: https://go-review.googlesource.com/c/go/+/204318
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Lorenz Bauer [Wed, 30 Oct 2019 11:30:57 +0000 (11:30 +0000)]
syscall: treat ENFILE as a temporary error
ENFILE is returned from accept when the whole system has run out of
file descriptors. Mark the error as temporary, so that accept loops
continue working.
Fixes #35131
Updates #1891
Change-Id: Idf44c084731898ff4c720d06c250d3b8a42de312
Reviewed-on: https://go-review.googlesource.com/c/go/+/203117
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Elias Naur [Wed, 16 Oct 2019 08:20:12 +0000 (10:20 +0200)]
cmd/link/internal/ld: remove flags incompatible with -fembed-bitcode
The flags -headerpad, -Wl,-no_pie and -pagezero_size are incompatible with
the -fembed-bitcode flag used by `gomobile build`. Than McIntosh
suggested we might not need the offending flags; this change removes
the flags on darwin/arm64 and -headerpad, -pagezero_size on darwin/arm.
The -Wl,-no_pie flag is left for darwin/arm because linking fails
without it:
ld: warning: PIE disabled. Absolute addressing (perhaps -mdynamic-no-pic) not allowed in code signed PIE, but used in _runtime.rodata from /var/folders/qq/qxn86k813bn9fjxydm095rxw0000gp/T/workdir-host-darwin-amd64-zenly-ios/tmp/go-link-225285265/go.o. To fix this warning, don't compile with -mdynamic-no-pic or link with -Wl,-no_pie
Cholerae Hu [Tue, 29 Oct 2019 05:50:31 +0000 (13:50 +0800)]
cmd/compile: resolve TODO of Mpflt.SetString
Number literal strings returned by the lexer (internal/syntax package) and other
arguments to SetString never contain leading whitespace. There's no need (anymore)
to trim the argument.
Change-Id: Ib060d109f46f79a364a5c8aa33c4f625fe849264
Reviewed-on: https://go-review.googlesource.com/c/go/+/203997 Reviewed-by: Robert Griesemer <gri@golang.org>
Cherry Zhang [Tue, 29 Oct 2019 21:07:21 +0000 (17:07 -0400)]
runtime: clear m.gsignal when the M exits
On some platforms (currently ARM and ARM64), when calling into
VDSO we store the G to the gsignal stack, if there is one, so if
we receive a signal during VDSO we can find the G.
When an M exits, it frees the gsignal stack. But m.gsignal.stack
still points to that stack. When we call nanotime on this M, we
will write to the already freed gsignal stack, which is bad.
Prevent this by unlinking the freed stack from the M.
Should fix #35235.
Change-Id: I338b1fc8ec62aae036f38afaca3484687e11a40d
Reviewed-on: https://go-review.googlesource.com/c/go/+/204158
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 29 Oct 2019 20:14:58 +0000 (13:14 -0700)]
cmd/compile: fix typing of IData opcodes
The rules for extracting the interface data word don't leave
the result typed correctly. If I do i.([1]*int)[0], the result
should have type *int, not [1]*int. Using (IData x) for the result
keeps the typing of the original top-level Value.
I don't think this would ever cause a real codegen bug, bug fixing it
at least makes the typing shown in ssa.html more consistent.
Change-Id: I239d821c394e58347639387981b0510d13b2f7b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/204042
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jay Conrod [Wed, 16 Oct 2019 19:26:15 +0000 (15:26 -0400)]
cmd/go: delete internal packages moved to x/mod
This change deletes several internal packages, replaces imports to
them with the equivalent golang.org/x/mod packages, updates x/mod, and
re-runs 'go mod vendor'.
Dan Scales [Mon, 28 Oct 2019 22:55:36 +0000 (15:55 -0700)]
cmd/compile: handle some missing cases of non-SSAable values for args of open-coded defers
In my experimentation, I had found that most non-SSAable expressions were
converted to autotmp variables during AST evaluation. However, this was not true
generally, as witnessed by issue #35213, which has a non-SSAable field reference
of a struct that is not converted to an autotmp. So, I fixed openDeferSave() to
handle non-SSAable nodes more generally, and make sure that these non-SSAable
expressions are not evaluated more than once (which could incorrectly repeat side
effects).
Fixes #35213
Change-Id: I8043d5576b455e94163599e930ca0275e550d594
Reviewed-on: https://go-review.googlesource.com/c/go/+/203888
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Jay Conrod [Tue, 29 Oct 2019 19:25:22 +0000 (15:25 -0400)]
cmd/go/internal/modfile: don't use cmd/internal/diff
This is a partial revert of CL 203218.
cmd/go/internal/modfile is about to be deleted and replaced with
golang.org/x/mod/modfile in CL 202698. cmd/internal/diff is not
visible from golang.org/x/mod/modfile, and it doesn't make sense to
extract it into a new package there.
Updates #31761
Change-Id: I3bbbc4cae81120020e1092c1138524729530b415
Reviewed-on: https://go-review.googlesource.com/c/go/+/204103
Run-TryBot: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Mon, 28 Oct 2019 15:37:24 +0000 (11:37 -0400)]
go/build: use the main module's root when locating module sources
Previously, we were using srcDir, which would apply the wrong module
dependencies (including the wrong 'replace' and 'exclude' directives)
when locating an import path within a module.
Fixes #34860
Change-Id: Ie59dcc2075a7b51ba40f7cd2f62dae27bf58c9b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/203820 Reviewed-by: Jay Conrod <jayconrod@google.com>
Clément Chigot [Tue, 29 Oct 2019 14:35:42 +0000 (15:35 +0100)]
runtime: initialize netpoll in TestNetpollBreak
Netpoll must be always be initialized when TestNetpollBreak is launched.
However, when it is run in standalone, it won't be the case, so it must
be forced.
Updates: #27707
Change-Id: I28147f3834f3d6aca982c6a298feadc09b55f66e
Reviewed-on: https://go-review.googlesource.com/c/go/+/204058
Run-TryBot: Clément Chigot <clement.chigot%atos.net@gtempaccount.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Than McIntosh [Thu, 10 Oct 2019 16:11:06 +0000 (12:11 -0400)]
cmd/compile: fix spurious R_TLE_LE reloc on android/386
When compiling for GOARCH=386 GOOS=android, the compiler was attaching
R_TLS_LE relocations inappropriately -- as of Go 1.13 the TLS access
recipe for Android refers to a runtime symbol and no longer needs this
type of relocation (which was causing a crash when the linker tried to
process it).
Updates #29674.
Fixes #34788.
Change-Id: Ida01875011b524586597b1f7e273aa14e11815d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/200337
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Elias Naur <mail@eliasnaur.com> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Austin Clements [Tue, 29 Oct 2019 02:14:04 +0000 (22:14 -0400)]
runtime: unblock SIGUSR1 for TestPreemptM
TestPreemptM tests signal delivery using SIGUSR1, but (for unknown
reasons) SIGUSR1 is blocked by default on android/arm and
android/arm64, causing the test to fail.
This fixes the test by ensuring that SIGUSR1 is unblocked for this
test.
Lynn Boger [Mon, 28 Oct 2019 18:40:27 +0000 (14:40 -0400)]
crypto/elliptic: clean up ppc64le implementation slightly
As suggested by comments from the review of CL 168478, this adds
Go code to do reverse bytes and removes the asm code, as well
as making a few cosmetic changes.
Alex Brainman [Sun, 27 Oct 2019 06:13:45 +0000 (17:13 +1100)]
internal/syscall/windows/registry: make '-gcflags=all=-d=checkptr' flag work
Mostly replaced [:x] slice operation with [:x:x].
According to @mdempsky, compiler specially recognizes when you combine
a pointer conversion with a full slice operation in a single expression
and makes an exception.
Updates golang/go#34972
Change-Id: I07d9de3b31da254d55f50d14c18155f8fc8f3ece
Reviewed-on: https://go-review.googlesource.com/c/go/+/203442 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cuong Manh Le [Mon, 21 Oct 2019 16:25:32 +0000 (23:25 +0700)]
cmd/compile: hard fail if n.Opt() is not nil in walkCheckPtrArithmetic
n.Opt() is used in walkCheckPtrArithmetic to prevent infinite loops. The
fact that it's used today because n.Opt() is not used for OCONVNOP
during walk.go. If that changes, then it's not safe to repalce it
anymore. So doing hard fail if that case happens, the author of new
changes will be noticed and must change the usage of n.Opt() inside
walkCheckPtrArithmetic, too.
Change-Id: Ic7094baa1759c647fc10e82457c19026099a0d47
Reviewed-on: https://go-review.googlesource.com/c/go/+/202497
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Cherry Zhang [Sun, 27 Oct 2019 02:48:15 +0000 (22:48 -0400)]
cmd/internal/obj/mips: fix encoding of FCR registers
The asm encoder generally assumes that the lowest 5 bits of the
REG_XX constants match the machine instruction encoding, i.e.
the lowest 5 bits is the register number. This was not true for
FCR registers and M registers. Make it so.
MOV Rx, FCRy was encoded as two machine instructions. The first
is unnecessary. Remove.
Mikhail Fesenko [Mon, 28 Oct 2019 21:51:00 +0000 (21:51 +0000)]
cmd/fix, cmd/go, cmd/gofmt: refactor common code into new internal diff package
Change-Id: Idac8473d1752059bf2f617fd7a781000ee2c3af4
GitHub-Last-Rev: 02a3aa1a3241d3ed4422518f1c954cd54bbe858e
GitHub-Pull-Request: golang/go#35141
Reviewed-on: https://go-review.googlesource.com/c/go/+/203218
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Shenghou Ma [Fri, 25 Oct 2019 01:29:30 +0000 (21:29 -0400)]
cmd/compile/internal/gc: reword "declared and not used" error message
"declared and not used" is technically correct, but might confuse
the user. Switching "and" to "but" will hopefully create the
contrast for the users: they did one thing (declaration), but
not the other --- actually using the variable.
This new message is still not ideal (specifically, declared is not
entirely precise here), but at least it matches the other parsers
and is one step in the right direction.
Change-Id: I725c7c663535f9ab9725c4b0bf35b4fa74b0eb20
Reviewed-on: https://go-review.googlesource.com/c/go/+/203282
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Agniva De Sarker [Wed, 13 Feb 2019 03:19:52 +0000 (08:49 +0530)]
go/ast: fix SortImports to handle block comments (take 2)
This is a 2nd attempt at fixing CL 162337 which had an off-by-one error.
We were unconditionally getting the position of the start of the next line
from the last import without checking whether it is the end of the file or not.
Fix the code to check for that and move the testcase added in CL 190523
to the end of the file for it to trigger the issue properly.
Fixes #18929
Change-Id: I59e77256e256570b160fea6a17bce9ef49e810df
Reviewed-on: https://go-review.googlesource.com/c/go/+/190480
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
John Papandriopoulos [Sun, 29 Sep 2019 23:59:56 +0000 (16:59 -0700)]
cmd/link: pass-through undefined call targets in external link mode
Allows Go asm calls referencing a function in a .syso file to be
passed through to the external linker, that would have otherwise
raised a "relocation target X not defined" error in cmd/link.
Fixes #33139
Change-Id: I2a8eb6063ebcd05fac96f141acf7652cf9189766
Reviewed-on: https://go-review.googlesource.com/c/go/+/198798
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>