Michael Matloob [Wed, 15 Jul 2020 21:38:36 +0000 (17:38 -0400)]
cmd/go: add tracing for querying and downloading from the proxy
This CL adds tracing spans for modload.queryPattern, modload.queryProxy,
modload.QueryPattern, modload.QueryPattern.queryModule,
modload.queryPrefixModules and modfetch.Download.
Updates #38714
Change-Id: I537c7fa4f466c691c1b60ec73ef8a2277af49cd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/242786
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
Michael Matloob [Tue, 30 Jun 2020 21:51:52 +0000 (17:51 -0400)]
cmd/go: do context propagation for tracing downloads
This change does context propagation (and only context propagation)
necessary to add context to modfetch.Download and pkg.LoadImport.
This was done by adding context to their callers, and then
adding context to all call-sites, and then repeating adding
context to callers of those enclosing functions and their
callers until none were left. In some cases the call graph expansion
was pruned by using context.TODOs.
The next CL will add a span to Download. I kept it out of this
change to avoid making it any larger (and harder to review)
than it needs to be.
Updates #38714
Change-Id: I5bf2d599aafef67334c384dfccd5e255198c85b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/248327
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Fri, 14 Aug 2020 21:44:22 +0000 (17:44 -0400)]
cmd/test2json: do not emit a final Action if the result is not known
If we are parsing a test output, and the test does not end in the
usual PASS or FAIL line (say, because it panicked), then we need the
exit status of the test binary in order to determine whether the test
passed or failed. If we don't have that status available, we shouldn't
guess arbitrarily — instead, we should omit the final "pass" or "fail"
action entirely.
(In practice, we nearly always DO have the final status, such as when
running 'go test' or 'go tool test2json some.exe'.)
Fixes #40132
Change-Id: Iae482577361a6033395fe4a05d746b980e18c3de
Reviewed-on: https://go-review.googlesource.com/c/go/+/248624
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Michael Matloob [Thu, 25 Jun 2020 23:11:28 +0000 (19:11 -0400)]
cmd/go/internal: remove some users of par.Work
par.Work is used in a number of places as a parallel
work queue. This change replaces it with goroutines
and channels in a number of simpler places where it's
used.
Change-Id: I0620eda46ec7b2c0599a8b9361639af7bb73a05a
Reviewed-on: https://go-review.googlesource.com/c/go/+/248326
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Wed, 24 Jun 2020 19:58:47 +0000 (15:58 -0400)]
cmd/go: add span for modload.LoadBuildList
This change adds context, and a span to modload.LoadBuildList and
propagates context into modload.BuildList. It's the start
of a run of CLs to add trace spans for module operations.
Updates #38714
Change-Id: I0d58dd394051526338092dc9a5ec29a9e087e4e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/248325
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Mon, 22 Jun 2020 23:02:00 +0000 (19:02 -0400)]
cmd/go: process -debug-trace flag for cmd/test and cmd/vet
These commands are build-like commands that do their own flag
processing, so the value of debug-trace isn't available until
the command starts running. Start tracing in the cmd's run
function.
Updates #38714
Change-Id: I4d633e6ee907bf09feac52c2aff3daceb9b20e12
Reviewed-on: https://go-review.googlesource.com/c/go/+/248324
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Thu, 18 Jun 2020 15:58:46 +0000 (11:58 -0400)]
cmd/go: mark trace flows between actions
This could help make it easier to identify blocking
dependencies when examining traces. Flows can be turned
off when viewing traces to remove potential distractions.
Updates #38714
Change-Id: Ibfd3f1a1861e3cac31addb053a2fca7ee796c4d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/248322
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Wed, 17 Jun 2020 22:18:23 +0000 (18:18 -0400)]
cmd/go: propagate context into Action.Func calls
Action.Func is now a func(*Builder, context.Context, *Action), so that
contexts can be propagated into the action funcs. While context
is traditionally the first parameter of a function, it's the second
parameter of Action.Func's type to continue to allow for methods
on Builder to be used as functions taking a *Builder as the first
parameter. context.Context is instead the first parameter on
those functions.
Change-Id: I5f058d6a99a1e96fe2025f2e8ce30a033d12e935
Reviewed-on: https://go-review.googlesource.com/c/go/+/248321
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Wed, 17 Jun 2020 22:05:16 +0000 (18:05 -0400)]
cmd/go: add trace events for each action
This change adds a trace event for each action and also
annotates each of the action execution goroutines with trace.Goroutine
so that the actions eaxecuted by each goroutine appear on different threads in
the chrome trace viewer.
Updates #38714
Change-Id: I2e58dc5606b2e3f7f87076a61e1cc6a2014255c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/248320
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Wed, 17 Jun 2020 21:40:35 +0000 (17:40 -0400)]
cmd/go/internal/trace: add function to distinguish goroutines
trace.StartGoroutine will associate the trace information on the context
with a new chrome profiler thread id. The chrome profiler doesn't
expect multiple trace events to have the same thread id, so this
will allow us to display concurrent events on the trace.
Updates #38714
Change-Id: I888b0cce15a5a01db66366716fdd85bf86c832cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/248319
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Cherry Zhang [Thu, 13 Aug 2020 20:59:52 +0000 (16:59 -0400)]
cmd/link: emit correct jump instruction on ARM for DYNIMPORT
On ARM, for a JMP/CALL relocation, the instruction bytes is
encoded in Reloc.Add (issue #19811). I really hate it, but before
it is fixed we have to follow the rule and emit the right bits
from r.Add.
Fixes #40769.
Change-Id: I862e105408d344c5cc58ca9140d2e552e4364453
Reviewed-on: https://go-review.googlesource.com/c/go/+/248399 Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Joel Sing <joel@sing.id.au> Reviewed-by: Than McIntosh <thanm@google.com>
Cherry Zhang [Thu, 13 Aug 2020 00:27:57 +0000 (20:27 -0400)]
cmd/link: link dynamic library automatically
cgo_import_dynamic pragma indicates a symbol is imported from a
dynamic library. Currently, the linker does not actually link
against the dynamic library, so we have to "force" it by using
//go:cgo_import_dynamic _ _ "dylib"
syntax, which links in the library unconditionally.
This CL changes it to link in the library automatically when a
symbol is imported from the library, without using the "force"
syntax. (The "force" syntax is still supported.)
Remove the unconditional imports in the runtime. Now,
Security.framework and CoreFoundation.framework are only linked
when the x509 package is imported (or otherwise specified).
Austin Clements [Fri, 5 Jun 2020 20:48:03 +0000 (16:48 -0400)]
runtime: move checkmarks to a separate bitmap
Currently, the GC stores the object marks for checkmarks mode in the
heap bitmap using a rather complex encoding: for one word objects, the
checkmark is stored in the pointer/scalar bit since one word objects
must be pointers; for larger objects, the checkmark is stored in what
would be the scan/dead bit for the second word of the object. This
encoding made more sense when the runtime used the first scan/dead bit
as the regular mark bit, but we moved away from that long ago.
This encoding and overloading of the heap bitmap bits causes a great
deal of complexity in many parts of the allocator and garbage
collector and leads to some subtle bugs like #15903.
This CL moves the checkmarks mark bits into their own per-arena bitmap
and reclaims the second scan/dead bit as a regular scan/dead bit.
I tested this by enabling doubleCheck mode in heapBitsSetType and
running in both regular and GODEBUG=gccheckmark=1 mode.
Fixes #15903.
No performance degradation. (Very slight improvement on a few
benchmarks, but it's probably just noise.)
Austin Clements [Fri, 5 Jun 2020 20:44:29 +0000 (16:44 -0400)]
runtime: simplify heapBitsSetType doubleCheck
The heapBitsSetType function has a slow doubleCheck debugging mode
that checks the bitmap written out by the rest of the function using
far more obvious logic. But even this has some surprisingly complex
logic in it. Simplify it a bit. This also happens to fix the logic on
32-bit.
runtime: replace index and contains with bytealg calls
The runtime has its own implementation of string indexing. To reduce
code duplication and cognitive load, replace this with calls to the
internal/bytealg package. We can't do this on Plan 9 because it needs
string indexing in a note handler (which isn't allowed to use the
optimized bytealg version because it uses SSE), so we can't just
eliminate the index function, but this CL does down-scope it so make
it clear it's only for note handlers on Plan 9.
Change-Id: Ie1a142678262048515c481e8c26313b80c5875df
Reviewed-on: https://go-review.googlesource.com/c/go/+/244537
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Tobias Klauser [Mon, 4 May 2020 16:36:31 +0000 (18:36 +0200)]
runtime: use hw.ncpuonline sysctl in getncpu on netbsd
Since NetBSD 7, hw.ncpuonline reports the number of CPUs online, while
hw.cpu reports the number of CPUs configured. Try hw.cpuonline first and
fall back to hw.ncpu in case it fails (which is the case on NetBSD
before 7.0).
This follows the behavior on OpenBSD (see CL 161757). Also, Go
in pkgsrc is patched to use hw.cpuonline, so this CL would allow said
patch to be dropped.
Martin Möhrmann [Mon, 20 Jul 2020 05:57:06 +0000 (07:57 +0200)]
fmt: avoid badverb formatting for %q when used with integers
Instead of returning a bad verb error format for runes above
utf8.Maxrune return a quoted utf8.RuneError rune (\ufffd).
This makes the behaviour consistent with the "c" verb and
aligns behaviour to not return bad verb error format when
a verb is applied to the correct argument type.
Fixes #14569
Change-Id: I679485f6bb90ebe408423ab68af16cce38816cd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/248759
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Nigel Tao [Thu, 13 Aug 2020 14:04:05 +0000 (00:04 +1000)]
image/gif: add more writer benchmarks
The two existing benchmarks encode randomized pixels, which isn't very
representative. The two new benchmarks encode a PNG photo as a GIF.
Also rename the benchmarks for consistency.
Also fix the bytes-per-op measure for paletted images, which are 1 (not
4) bytes per pixel.
Also simplify BenchmarkEncodeRandomPaletted (formerly just called
BenchmarkEncode). It doesn't need to generate a random palette (and the
GIF encoder largely doesn't care about the palette's RGBA values).
Use palette.Plan9 instead, a pre-existing 256-element color palette.
Change-Id: I10a6ea4e9590bb0d9f76e8cc0f4a88d43b1d650d
Reviewed-on: https://go-review.googlesource.com/c/go/+/248218 Reviewed-by: David Symonds <dsymonds@golang.org>
Keith Randall [Tue, 11 Aug 2020 20:19:57 +0000 (13:19 -0700)]
cmd/compile: stop race instrumentation from clobbering frame pointer
There is an optimization rule that removes calls to racefuncenter and
racefuncexit, if there are no other race calls in the function. The
rule removes the call to racefuncenter, but it does *not* remove the
store of its argument to the outargs section of the frame. If the
outargs section is now size 0 (because the calls to racefuncenter/exit
were the only calls), then that argument store clobbers the frame
pointer instead.
The fix is to remove the argument store when removing the call to
racefuncenter. (Racefuncexit doesn't have an argument.)
Change-Id: I183ec4d92bbb4920200e1be27b7b8f66b89a2a0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/248262 Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This dance ensures that inside duffzero, it looks like there is a
proper frame pointer set up, so that stack walkbacks work correctly if
the kernel samples during duffzero.
However, this instruction sequence depends on data not being clobbered
even though it is below the stack pointer.
If there is an async interrupt at any of those last 3 instructions,
and the interrupt decides to insert a call to asyncPreempt, then the
saved frame pointer on the stack gets clobbered. The last instruction
above then restores junk to the frame pointer.
To prevent this, mark these instructions as async unsafe.
(The body of duffzero is already async unsafe, as it is in package runtime.)
Change-Id: I5562e82f9f5bd2fb543dcf2b6b9133d87ff83032
Reviewed-on: https://go-review.googlesource.com/c/go/+/248261
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Martin Möhrmann <moehrmann@google.com> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Jason A. Donenfeld [Tue, 14 Jul 2020 07:41:03 +0000 (01:41 -0600)]
runtime: do not explicitly exit on ctrl handler
The default ctrl+c handler should process exits in situations where it
makes sense, like console apps, but not in situations where it doesn't,
like libraries or services. Therefore, we should remove the exit(2) so
that the default handler is used for this. This also uses the more
proper windows exit code of STATUS_CONTROL_C_EXIT, with the base case
handler installed by KernelBase.dll. In particular, this helps in the
case of services, which previously would terminate when receiving
shutdown signals, instead of passing them onward to the service program.
In this CL, contrary to CL 244959, we do not need to special case
services with expensive detection algorithms, or rely on hard-coded
library/archive flags.
Fixes #40167.
Fixes #40074.
Change-Id: I9bf6ed6f65cefeff754d270aa33fa4df8d0b451f
Reviewed-on: https://go-review.googlesource.com/c/go/+/243597
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
cmd/compile: handle OCLOSURE/OCALLPART in mustHeapAlloc check
Currently, generated struct wrapper for closure is not handled in
mustHeapAlloc. That causes compiler crashes when the wrapper struct
is too large for stack, and must be heap allocated instead.
Fixes #39292
Change-Id: I14c1e591681d9d92317bb2396d6cf5207aa93e08
Reviewed-on: https://go-review.googlesource.com/c/go/+/244917
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Cuong Manh Le [Wed, 6 May 2020 17:35:28 +0000 (00:35 +0700)]
cmd/compile: another fix initializing blank fields in struct literal
CL 230121 fixed the bug that struct literal blank fields type array/struct
can not be initialized. But it still misses some cases when an expression
causes "candiscard(value)" return false. When these happen, we recursively
call fixedlit with "var_" set to "_", and hit the bug again.
To fix it, just making splitnode return "nblank" whenever "var_" is "nblank".
Fixes #38905
Change-Id: I281941b388acbd551a4d8ca1a235477f8d26fb6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/232617
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
kakulisen [Thu, 14 May 2020 08:20:58 +0000 (16:20 +0800)]
math: simplify the code
Simplifying some code without compromising performance.
My CPU is Intel Xeon Gold 6161, 2.20GHz, 64-bit operating system.
The memory is 8GB. This is my test environment, I hope to help you judge.
Benchmark:
name old time/op new time/op delta
Log1p-4 21.8ns ± 5% 21.8ns ± 4% ~ (p=0.973 n=20+20)
Change-Id: Icd8f96f1325b00007602d114300b92d4c57de409
Reviewed-on: https://go-review.googlesource.com/c/go/+/233940 Reviewed-by: Robert Griesemer <gri@golang.org>
sync: fix goroutine leak for when TestMutexFairness times out
If the timeout triggers before writing to the done channel, the
goroutine will be blocked waiting for a corresponding read that’s
no longer existent, thus a goroutine leak. This change fixes that by
using a buffered channel instead.
Keith Randall [Sat, 8 Aug 2020 14:58:04 +0000 (07:58 -0700)]
cmd/compile: fix live variable computation for deferreturn
Taking the live variable set from the last return point is problematic.
See #40629 for details, but there may not be a return point, or it may
be before the final defer.
Additionally, keeping track of the last call as a *Value doesn't quite
work. If it is dead-code eliminated, the storage for the Value is reused
for some other random instruction. Its live variable information,
if it is available at all, is wrong.
Instead, just mark all the open-defer argument slots as live
throughout the function. (They are already zero-initialized.)
Fixes #40629
Change-Id: Ie456c7db3082d0de57eaa5234a0f32525a1cce13
Reviewed-on: https://go-review.googlesource.com/c/go/+/247522
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dan Scales <danscales@google.com>
modload.Init initialized the build cache with the intent of providing
a better error message in Go 1.12, when the build cache became
mandatory (in module mode, packages aren't installed outside the build
cache). Unfortunately, this didn't provide a more descriptive error
(the cache calls base.Fatalf with its own message), and it caused
errors for commands that don't use the cache (like 'go mod edit').
This CL removes the cache initialization from modload.Init. The
builder will initialize it when it's needed.
For #39882
Change-Id: Ibc01ae4e59358dcd08a07ffc97bf556514d0366f
Reviewed-on: https://go-review.googlesource.com/c/go/+/240548
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Jay Conrod [Thu, 25 Jun 2020 21:50:38 +0000 (17:50 -0400)]
cmd/go: migrate to module.MatchPrefixPatterns
In CL 239797, str.GlobsMatchPath was copied to golang.org/x/mod/module
as MatchPrefixPatterns. This CL updates x/mod, switches calls to use
the new function, and deletes the old function.
For #38725
Change-Id: I7241032228b574aa539426a92d2f5aad9ee001e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/240061
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Jay Conrod [Mon, 8 Jun 2020 22:06:11 +0000 (18:06 -0400)]
cmd/go: don't save sums for modules loaded for import resolution
modfetch.WriteGoSum now accepts a map[module.Version]bool parameter.
This is used to prevent some new sums from being saved to go.sum when
they would be removed by the next 'go mod tidy'. Previusly, sums were
saved for modules looked up during import resolution.
A new function, modload.TrimGoSum, is also introduced, which marks
sums for deletion. 'go mod tidy' now uses this. The new logic
distinguishes between go.mod sums and content sums, which lets 'go mod
tidy' delete sums for modules in the build graph but not the build
list.
Fixes #31580
Fixes #36260
Fixes #33008
Change-Id: I06c4125704a8bbc9969de05265967ec1d2e6d3e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/237017
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Jay Conrod [Thu, 4 Jun 2020 15:41:43 +0000 (11:41 -0400)]
cmd/go: don't initialize Builder in envcmd.MkEnv
The Builder isn't needed by MkEnv, and Builder.Init doesn't have side
effects that change the environment. Builder.Init does currently call
CheckGOOSARCHPair, but that's being moved out in CL 234658.
Builder.Init creates the temporary work directory used by the
builder. For the builder created in MkEnv, this directory is never
used. Creating this directory can cause unnecessary errors for
commands that don't use a builder like 'go clean' and 'go list'.
Fixes #38395
Updates #24398
Change-Id: Ib93ae55afdf958000470657f4c4ff5bd92700e46
Reviewed-on: https://go-review.googlesource.com/c/go/+/236563
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Tim Möhlmann [Thu, 13 Aug 2020 08:56:31 +0000 (11:56 +0300)]
database/sql: make Rows.Scan properly wrap underlying errors
The prior implementation used the format verb %v which unfortunately
improperly wrapped any underlying scanner errors, and we couldn't use
errors.Is nor errors.As. This change fixes that by using the %w verb.
Added a unit to ensure that both error sub string matching works, but
also that errors.Is works as expected.
Michael Munday [Mon, 10 Aug 2020 15:01:21 +0000 (08:01 -0700)]
cmd/internal/obj: fix inline marker issue on s390x
The optimization that replaces inline markers with pre-existing
instructions assumes that 'Prog' values produced by the compiler are
still reachable after the assembler has run. This was not true on
s390x where the assembler was removing NOP instructions from the
linked list of 'Prog' values. This led to broken inlining data
which in turn caused an infinite loop in the runtime traceback code.
Fix this by stopping the s390x assembler backend removing NOP
values. It does not make any difference to the output of the
assembler because NOP instructions are 0 bytes long anyway.
Fixes #40473.
Change-Id: Ib4fabadd1de8adb80421f75950ee9aad2111147a
Reviewed-on: https://go-review.googlesource.com/c/go/+/247697
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Lynn Boger [Tue, 11 Aug 2020 16:04:25 +0000 (12:04 -0400)]
cmd/internal/obj/ppc64: don't remove NOP in assembler
Previously, the assembler removed NOPs from the Prog list in
obj9.go. NOPs shouldn't be removed if they were added as
an inline mark, as described in the issue below.
Fixes #40689
Once the NOPs were left in the Prog list, some instructions
were flagged as invalid because they had an operand which was
not represented in optab. In order to preserve the previous
assembler behavior, entries were added to optab for those
operand cases. They were not flagged as errors before because
the NOP instructions were removed before the code to check the
valid opcode/operand combinations.
Change-Id: Iae5145f94459027cf458e914d7c5d6089807ccf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/247842
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Paul Murphy <murp@ibm.com> Reviewed-by: Michael Munday <mike.munday@ibm.com> Reviewed-by: Keith Randall <khr@golang.org>
Minor cleanup: remove the symbol attribute AttrSeenGlobal, since it is
redundant with the existing attribute AttrOnList (no need to have what
amounts to a separate flag for checking the same property).
Change-Id: Ia269b64de37c2bb4a2314bbecf3d2091c6d57424
Reviewed-on: https://go-review.googlesource.com/c/go/+/239477
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Than McIntosh [Wed, 12 Aug 2020 16:59:55 +0000 (12:59 -0400)]
doc: add a release notes blurb on 1.16 linker improvements
Add a draft version of a blurb on improvements to the linker. This
will need to be finalized later in the release since there are still
some additional changes to be made to the linker in 1.16.
Go 1.14 included a (rather awful) workaround for a Linux kernel bug
that corrupted vector registers on x86 CPUs during signal delivery
(https://bugzilla.kernel.org/show_bug.cgi?id=205663). This bug was
introduced in Linux 5.2 and fixed in 5.3.15, 5.4.2 and all 5.5 and
later kernels. The fix was also back-ported by major distros. This
workaround was necessary, but had unfortunate downsides, including
causing Go programs to exceed the mlock ulimit in many configurations
(#37436).
We're reasonably confident that by the Go 1.16 release, the number of
systems running affected kernels will be vanishingly small. Hence,
this CL removes this workaround.
This effectively reverts CLs 209597 (version parser), 209899 (mlock
top of signal stack), 210299 (better failure message), 223121 (soft
mlock failure handling), and 244059 (special-case patched Ubuntu
kernels). The one thing we keep is the osArchInit function. It's empty
everywhere now, but is a reasonable hook to have.
Updates #35326, #35777 (the original register corruption bugs).
Updates #40184 (request to revert in 1.15).
Fixes #35979.
Change-Id: Ie213270837095576f1f3ef46bf3de187dc486c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/246200
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Wed, 12 Aug 2020 17:30:43 +0000 (13:30 -0400)]
doc/go1.15: clarify external linking can still be used for building PIE
In Go 1.15 we switched the default linking mode for PIE on
Linux/AMD64 and Linux/ARM64 to internal linking. Clarify that
the previous behavior (external linking) can still be used with
a flag.
Fixes #40719.
Change-Id: Ib7042622bc91e1b1aa31f520990d03b5eb6c56bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/248199 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Wed, 12 Aug 2020 21:37:18 +0000 (17:37 -0400)]
cmd: merge branch 'dev.link' into master
In the dev.link branch we have continued developing the new object
file format support and the linker improvements described in
https://golang.org/s/better-linker . Since the last merge (May 1st
2020), more progress has been made to improve the new linker, with
improvements on both linker speed and memory usage.
Michael Matloob [Wed, 17 Jun 2020 19:50:14 +0000 (15:50 -0400)]
cmd/go: propagate context into PackagesForBuild and Do for tracing
This change propagates context into PackagesForErrors and Do for
the purpose of tracing, and calls trace.StartSpan on PackagesForErrors
and Do, so that the trace now shows the broad outline of where
the "Loading" and "Execution" phases are in the build.
Updates #38714
Change-Id: Ib9a7cf7030210f68f76663d1c8a7461e0a226611
Reviewed-on: https://go-review.googlesource.com/c/go/+/238541
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Michael Matloob [Fri, 19 Jun 2020 21:59:16 +0000 (17:59 -0400)]
cmd/trace: move viewer data structs into cmd/internal/traceviewer
The ViewerEvent, ViewerData and ViewerFrame structs are moved into
cmd/internal/traceviewer, and renamed Event, Data, and Frame.
The structs are the same, except for the following: A definition
for the JSON "bp" field that's defined in the trace format, but
missing in the structs has been added. Also, the Tid and Pid fields
on Event have been renamed TID and PID to better match Go style.
Finally, the footer field on ViewerData, which hasn't been used
for a while, has been removed.
This CL is in preparation for the usage of these structs by cmd/go's
tracing functionality.
Updates #38714
Change-Id: I345f23617b96d4629b876ae717f89d56a67e05a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/239098
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
Dmitri Shuralyov [Wed, 12 Aug 2020 14:34:15 +0000 (10:34 -0400)]
doc/go1.16: start draft release notes
This template is based on CL 220278 and previous ones like it.
Include Compiler and Linker sections proactively, they can be
removed if they don't end up being needed for Go 1.16.
Use two spaces of indentation for TODOs to set a better precedent
for the final text that will take its place.
'relnote -html' does not report any changes at this time.
For #40700.
Change-Id: I096b0ce0d33aaaa6fae9c91c0d2dfb89b9c5e94c
Reviewed-on: https://go-review.googlesource.com/c/go/+/248198 Reviewed-by: Carlos Amedee <carlos@golang.org>
Dmitri Shuralyov [Tue, 11 Aug 2020 22:37:44 +0000 (18:37 -0400)]
internal/goversion: update Version to 1.16
This is the start of the Go 1.16 development cycle, so update the
Version value accordingly. It represents the Go 1.x version that
will soon open up for development (and eventually become released).
Historically, we used to bump this at an arbitrary time throughout
the development cycle, but it's better to be more predictable about
updating it. The start of a development cycle should be the most
appropriate time: it clearly marks the boundary between 1.15 and
1.16 development, and doing it early can help catch issues in other
tooling. See issue #38704 for more background.
There is no longer a need to update the list of Go versions in
src/go/build/doc.go because it does not exist as of CL 232981.
For #40705.
Updates #38704.
Updates #37018.
Change-Id: Id8ee733b5e79c53b6cd03509c6560614d8743833
Reviewed-on: https://go-review.googlesource.com/c/go/+/248038 Reviewed-by: Carlos Amedee <carlos@golang.org>
Cherry Zhang [Wed, 5 Aug 2020 00:25:10 +0000 (20:25 -0400)]
runtime: make nanotime1 reentrant
Currently, nanotime1 (and walltime1) is not reentrant, in that it
sets m.vdsoSP at entry and clears it at exit. If a signal lands
in between, and nanotime1 is called from the signal handler, it
will clear m.vdsoSP while we are still in nanotime1. If (in the
unlikely event) it is signaled again, m.vdsoSP will be wrong,
which may cause the stack unwinding code to crash.
This CL makes it reentrant, by saving/restoring the previous
vdsoPC and vdsoSP, instead of setting it to 0 at exit.
TODO: have some way to test?
Change-Id: I9ee53b251f1d8a5a489c71d4b4c0df1dee70c3e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/246763
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jeremy Faller [Wed, 29 Jul 2020 15:06:02 +0000 (11:06 -0400)]
[dev.link] use per package filenames to build pclntab
In order to prevent renumbering of filenames in pclntab generation, use
the per-package file list (previously only used for DWARF generation) as
file-indices. This is the largest step to eliminate renumbering of
filenames in pclntab.
Note, this is probably not the final state of the file table within the
object file. In this form, the linker loads all filenames for all
objects. I'll move to storing the filenames as regular string
symbols,and defaulting all string symbols to using the larger hash value
to make generation of pcln simplest, and most memory friendly.
Than McIntosh [Wed, 8 Jul 2020 15:02:51 +0000 (11:02 -0400)]
[dev.link] cmd/link: add PPC64 section splitting test
Add a new PPC64-only linker test that does a build with the
-debugppc64textsize debugging option (selecting a lower the threshold
for text section splitting) to verify that no bugs have been
introduced in the linker code that manages this process.
Change-Id: Iea3f16a04c894d528eab2cb52f1ec1d75a2770cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/241499
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Than McIntosh [Tue, 7 Jul 2020 16:54:46 +0000 (12:54 -0400)]
[dev.link] cmd/link: add PPC64 debugging option to encourage text section splits
Add a new debugging command line option (-debugppc64textsize=N) that
forces the start of a new text section after ".text" hits N bytes as
opposed to the architected limit of 2^26. This is intended to enable
testing of the linker code paths that handle multiple .text sections
on PPC64 without resorting to building giant applications.
Updates #20492.
Change-Id: I74ab7fd1e412e9124de5bd0d8d248c5e73225ae3
Reviewed-on: https://go-review.googlesource.com/c/go/+/241073
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org>
Than McIntosh [Wed, 8 Jul 2020 14:43:42 +0000 (10:43 -0400)]
[dev.link] cmd/link: fix ppc64-specific bug in genelfsym
The code in the the linker's genelfsym() routine was not properly
including runtime.text.%d marker symbols that are emitted on PPC64
when a very large text section is split into chunks. This bug was
introduced in CL 233338 when portions of asmb2() were converted
from sym.Symbol to loader.Sym usage.
Change-Id: Idfed944c41e1805f78f35be67bcdd18bdefd7819
Reviewed-on: https://go-review.googlesource.com/c/go/+/241498
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org>
Katie Hockman [Tue, 4 Aug 2020 15:45:32 +0000 (11:45 -0400)]
encoding/binary: read at most MaxVarintLen64 bytes in ReadUvarint
This CL ensures that ReadUvarint consumes only a limited
amount of input (instead of an unbounded amount).
On some inputs, ReadUvarint could read an arbitrary number
of bytes before deciding to return an overflow error.
After this CL, ReadUvarint returns that same overflow
error sooner, after reading at most MaxVarintLen64 bytes.
Fix authored by Robert Griesemer and Filippo Valsorda.
Thanks to Diederik Loerakker, Jonny Rhea, Raúl Kripalani,
and Preston Van Loon for reporting this.
Michael Matloob [Tue, 4 Aug 2020 17:24:37 +0000 (13:24 -0400)]
cmd/go: fix error stacks when there are scanner errors
After golang.org/cl/228784 setLoadPackageDataError tries to decide whether an
error is caused by an imported package or an importing package by examining the
error itself to decide. Ideally, the errors themselves would belong to a
specific interface or some other property to make it unambiguous that they
were import errors. Since they don't, setLoadPackageDataError just checked
for nogoerrors and classified all other errors as import errors. But
it missed scanner errors which are also "caused" by the imported
package.
Fixes #40544
Change-Id: I39159bfdc286bee73697decd07b8aa9451f2db06
Reviewed-on: https://go-review.googlesource.com/c/go/+/246717
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Cherry Zhang [Mon, 3 Aug 2020 21:59:41 +0000 (17:59 -0400)]
[dev.link] cmd/link: fix pclntab symbol handling on AIX
On AIX, container symbols are handled in a weird way (unlike
other platforms): the outer symbol needs to have size (but still
no data), and the inner symbols must not be in the symbol table
(otherwise it overlaps with the outer symbol, which the system
linker doesn't like).
As of CL 241598, pclntab becomes a container symbol. We need to
follow the rule above for AIX.
[dev.link] cmd/compile, cmd/link: generate itablink at link time
Currently, at compile time, for each itab symbol, we create an
"itablink" symbol which holds solely the address of the itab
symbol. At link time, all the itablink symbols are grouped
together to form the itablinks slice.
This CL removes the itablink symbols, and directly generate the
itablinks slice in the linker. This removes a number of symbols,
which are dupOK and generally have long names. And also removes
a special handling of itablink symbols in the deadcode pass which
iterates through all symbols.
Change-Id: I475c3c8899e9fbeec9abc7647b1e4a69aa5c3c5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/245901 Reviewed-by: Jeremy Faller <jeremy@golang.org>
[dev.link] cmd/compile, cmd/link: make itab symbols content-addressable
Extend the content-addressable symbol mechanism to itab symbols.
Itab symbols require global uniqueness (as at run time we compare
pointers), so it needs to be reliably deduplicated. Currently the
content hash depends on symbol name expansion, so we can only do
this when all Go packages are built with know package paths. Fall
back to checking names if any Go package is built with unknown
package path.
Change-Id: Icf5e8873755050c20e5fc6549f6de1c883254c89
Reviewed-on: https://go-review.googlesource.com/c/go/+/245719 Reviewed-by: Jeremy Faller <jeremy@golang.org>