Change should be non-functional.
Change-Id: I8ac835762a1aa6458d896b8815cd2d87333b55ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/239279
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
We can apply relocations of a symbol right after the symbol data
is copied to output buffer. This should help locality and
parallelism (parallelizing over blocks, instead of over segments).
Linking cmd/compile,
Asmb+Reloc 23.9ms ±18% 16.5ms ±11% -30.73% (p=0.008 n=5+5)
Linking cmd/compile with external linking,
Asmb+Reloc 74.0ms ± 3% 33.8ms ± 8% -54.32% (p=0.008 n=5+5)
In external linking mode, allocation goes up slightly, as we do
smaller batching now. It doesn't seem too bad.
Asmb+Reloc 15.0MB ± 0% 16.7MB ± 0% +11.22% (p=0.008 n=5+5)
Change-Id: Ide33d9ff86c39124c8f5cfc050d7badc753a1ced
Reviewed-on: https://go-review.googlesource.com/c/go/+/239197
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
addpersrc is called very late, after we have converted to
sym.Symbols and various fields in loader representation have been
dropped. Use the Symbol representation there.
Fixes#39658.
Change-Id: I616e838655b6f01554644171317e2cc5cefabf39
Reviewed-on: https://go-review.googlesource.com/c/go/+/238779
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The preceding paragraph suggests the test run will produce a file called trace.out.
The same name, trace.out, is used in the output from go help testflag, thus we change the go test line instead of changing the preceding paragraph.
Change-Id: Ib1fa7e49e540853e263a2399b16040ea6f41b703
GitHub-Last-Rev: 3535e62bf8
GitHub-Pull-Request: golang/go#39709
Reviewed-on: https://go-review.googlesource.com/c/go/+/238997
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
In os.Getenv and os.Setenv, instead of directly reading and writing the
Plan 9 environment device (which may be shared with other processes),
use a local copy of environment variables cached at the start of
execution. This gives the same semantics for Getenv and Setenv as on
other operating systems which don't share the environment, making it
more likely that Go programs (for example the build tests) will be
portable to Plan 9.
This doesn't preclude writing non-portable Plan 9 Go programs which make
use of the shared environment semantics (for example to have a command
which exports variable definitions to the parent shell). To do this, use
ioutil.ReadFile("/env/"+key) and
ioutil.WriteFile("/env/"+key, value, 0666)
in place of os.Getenv(key) and os.Setenv(key, value) respectively.
Note that CL 5599054 previously added env cacheing, citing efficiency
as the reason. However it made the cache write-through, with Setenv
changing the shared environment as well as the cache (so not consistent
with Posix semantics), and Clearenv breaking the sharing of the
environment between the calling thread and other threads (leading to
unpredictable behaviour). Because of these inconsistencies (#8849),
CL 158970045 removed the cacheing again.
This CL restores cacheing but without write-through. The local cache is
initialised at start of execution, manipulated by the standard functions
in syscall/env_unix.go to ensure the same semantics, and exported only
when exec'ing a new program.
Fixes#34971Fixes#25234Fixes#19388
Updates #38772
Change-Id: I2dd15516d27414afaf99ea382f0e00be37a570c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/236520
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Fazlul Shahriar <fshahriar@gmail.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
reflect.assignTo writes to the target using write barriers. Make sure
that the memory it is writing to is zeroed, so the write barrier does
not read pointers from uninitialized memory.
Fixes#39541
Change-Id: Ia64b2cacc193bffd0c1396bbce1dfb8182d4905b
Reviewed-on: https://go-review.googlesource.com/c/go/+/238760
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes the *noov opcodes so they handle a constant argument properly.
Most of the infrastructure for this CL is in CL 238077 (the arm32 one).
Fixes#39505
Change-Id: Id424a4e18964b848f05aa42f4d78e5f2e2cdf43b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237999
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Encode the flag results in an auxint field instead of having
one opcode per flag state. This helps us handle the new *noov
branches in a unified manner.
This is only for arm, arm64 is in a subsequent CL.
We could extend to other architectures as well, athough it would
only be cleanup, no behavioral change.
Update #39505
Change-Id: Ia46cea596faad540d1496c5915ab1274571543f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/238077
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
These conversion instructions set the condition code and so should
be marked as clobbering flags.
Fixes#39651.
Change-Id: I91cc9687ea70ef0551bb3139c1875071c349d43e
Reviewed-on: https://go-review.googlesource.com/c/go/+/238628
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This updates the ppc64 asm doc file, including information on
updates to the objdump, correcting information on operand order,
and adding some information on shifts.
Change-Id: Ib8ed53eac86c2121ea5b657c361ad92aae31cb32
Reviewed-on: https://go-review.googlesource.com/c/go/+/238237
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
By keeping a tail pointer, we can append to a patchList in constant
time, rather than in time proportional to the length of the list. This
gets rid of the quadratic compile times we were seeing for long series
of alternations.
This is basically the same change as
e9d517989f.
Fixes#39542.
Change-Id: Ib4ca0ca9c55abd1594df1984653c7d311ccf7572
Reviewed-on: https://go-review.googlesource.com/c/go/+/238079
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ensure that the exact Request passed to Transport.RoundTrip
is returned in the Response. Do not replace the Request with
a copy when resetting the request body.
Fixes#39533
Change-Id: Ie6fb080c24b0f6625b0761b7aa542af3d2411817
Reviewed-on: https://go-review.googlesource.com/c/go/+/237560
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently, for runtime functions and nosplit functions, it is
considered "all unsafe", meaning that the entire function body is
unsafe points. In the past, we didn't mark CALLs in such
functions unsafe, which is fixed in CL 230541. We also didn't
mark block control instructions (for mostly-empty blocks) unsafe.
This CL fixes it.
May fix#36110.
Change-Id: I3be8fdcef2b294e5367b31eb1c1b5e79966565fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/236597
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 225357 added tests for Scanner not panicking on bad readers.
CL 225557 created a named error value that is returned instead.
CL 237739 documents that the bufio.ErrBadReadCount is returned
when bufio.Scanner is used with an invalid io.Reader.
This suggests we wouldn't want that behavior to be able to change
without a test noticing it, so modify the tests to check for the
exact error value instead of just any non-nil one.
For #38053.
Change-Id: I4b0b8eb6804ebfe2c768505ddb94f0b1017fcf8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/238217
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This patch introduces parallelization of DWARF generation on a per
compilation unit basis. Each compilation unit now operates on a
separate set of symbols, so it's safe to send each compilation unit to
a goroutine to be processed in parallel.
Doing this requires some restructing to ensure that any new symbols
needed are created up front, since we can't create any new syms during
the parallel portion. Similarly, the parallel portion can't set any
symbol attributes, so the check that verifies we haven't doubly listed
any DIE syms had to be reworked, and setting of reachability has to be
delayed until after the parallel phase is complete.
Change-Id: I3042b76e9b597bb1a6a44dce19efba2d02bed76b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237679
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The loader's SymbolBuilder Add*/Set* methods include a call to mark
the underlying symbol as reachable (as a convenience, so that callers
would not have to set it explicitly). This code was carried over from
the corresponding sym.Symbol methods; back in the sym.Symbol world
unreachable symbols were never removed from the AllSyms slice, hence
setting and checking reachability was a good deal more important.
With the advent of the loader and the new deadcode implementation,
there is less of a need for this sort of fallback, and in addition the
implicit attr setting introduces data races in the the loader if there
are SymbolBuilder Add*/Set* method calls in parallel threads, as well
as adding overhead to the methods.
This patch gets rid of the implicit reachability setting, and instead
marks reachability in CreateSymForUpdate, as well as adding a few
explicit SetAttrReachable calls where needed.
Change-Id: I029a0c5a4a24237826a7831f9cbe5180d44cbc40
Reviewed-on: https://go-review.googlesource.com/c/go/+/237678
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Refactor some of the linker's DWARF generation methods so as to have
helper routines that do all the work for a given comp unit for a given
section (range, loc, etc). No change in functionality, this is just a
reorg in preparation for a later patch in this sequence.
Change-Id: I86fc789220326a4e522904a5924c8971d6757189
Reviewed-on: https://go-review.googlesource.com/c/go/+/237677
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Rework the way symbols are handled in DWARF line table generation to
eliminate copying the data payload for all SWDWARFLINES syms (emitted
by the compiler) into the payload of the ".debug_line" section symbol
(generated by the linker). Instead, chain together the SWDWARFLINES
symbols into a list, then append that list to the section sym list in
dwarfp (this moves us from a single monolithic .debug_line to a
.debug_line section sym followed by a list symbols (one per function
and an epilog symbol per compilation unit). To enable this work, move
the emission of the DW_LNE_set_address op (at the start of each
function) from the linker to the compiler.
Change-Id: Iec61b44a451f7a386c82a89bf944de482b018789
Reviewed-on: https://go-review.googlesource.com/c/go/+/237427
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Don't emit the .debug_pubnames/.debug_pubtypes sections. These sections
are not used by either GDB or Delve, and C++ compilers [notably GCC
and Clang] no longer emit the sections by default.
Change-Id: Ic3309755e88c8e1aa28a29366bc7f0df1748fe64
Reviewed-on: https://go-review.googlesource.com/c/go/+/237426
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The linker's asmb phase for ELF has a chunk of code that decides where
to place the .symtab and .strtab sections, which appear after after
the DWARF data; this code currently tries to align the start of the
.symtab section using the value of -R (stored in *FlagRound). This
patch gets rid of this additional alignment and instead just aligns
.symtab by pointer size. The -R value is needed for loadable
segments/sections (such as text or data), not for non-loadable
sections (e.g. symtab). On most architectures the *FlagRound value is
4k, however on ARM64 it is 64k, meaning that aligning symtab on this
boundary can waste a good chunk of space.
Change-Id: Ib51f3ad5611f5614768355eb8533084ba117a8e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/238019
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Relocate the last few functions in dwarf2.go to dwarf.go,
and remove dwarf2.go. No change in functionality.
Change-Id: Id1b5f0d1c496d5912b955b8ba942e33bc43aa93b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237425
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>
Users don't necessarily know if their OS is ELF-based.
For #37419.
Change-Id: I4a4256c5f5eb34211729e1311582bb0e210f1f8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/238240
Reviewed-by: Than McIntosh <thanm@google.com>
Adjust these two tests for Plan 9,
which uses $path instead of $PATH,
and $home instead of $HOME.
Fixes#39599
Change-Id: Idba95e07d307e76f0f61acd904905b417c52d43a
Reviewed-on: https://go-review.googlesource.com/c/go/+/237941
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This saves a redirect and makes the document more consistent.
For #37419
Change-Id: Ic3bd62f8caacf67ffe43a359624e11bed8b8cfaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/237540
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
When a signal is received, the runtime probes whether an
alternate signal stack is set, if so, adjust gsignal's stack to
point to the alternate signal stack. This is done in
adjustSignalStack, which calls sigaltstack "syscall", which is a
libc call on darwin through asmcgocall. asmcgocall decides
whether to do stack switch based on whether we're running on g0
stack, gsignal stack, or regular g stack. If g is not set to
gsignal, asmcgocall may make wrong decision. Set g first.
adjustSignalStack is recursively nosplit, so it is okay that
temporarily gsignal.stack doesn't match the stack we're running
on.
May fix#39079.
Change-Id: I59b2c5dc08c3c951f1098fff038bf2e06d7ca055
Reviewed-on: https://go-review.googlesource.com/c/go/+/238020
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the binutils Git repository instead of CVS.
Change-Id: I10100ca44d64ab3621367d1d4ac9e9a50d212d0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/237839
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The GCC code repository is now hosted on Git. Adjust the instructions in
gccgo_install.html accordingly.
Change-Id: I443a8b645b63e63785979bc0554521e3dc3b0bf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/237798
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make sure that if a field comparison might panic, we evaluate
(and short circuit if not equal) all previous fields, and don't
evaluate any subsequent fields.
Add a bunch more tests to the equality+panic checker.
Update #8606
Change-Id: I6a159bbc8da5b2b7ee835c0cd1fc565575b58c46
Reviewed-on: https://go-review.googlesource.com/c/go/+/237919
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts golang.org/cl/190659 and golang.org/cl/226218, minus the
regression tests in the latter.
The original work happened in golang.org/cl/151157, which was reverted
in golang.org/cl/190909 due to a crash found by fuzzing.
We tried a second time in golang.org/cl/190659, which shipped with Go
1.14. A bug was found, where strings would be mangled in certain edge
cases. The fix for that was golang.org/cl/226218, which was backported
into Go 1.14.4.
Unfortunately, a second regression was just reported in #39555, which is
a similar case of strings getting mangled when decoding under certain
conditions. It would be possible to come up with another small patch to
fix that edge case, but instead, let's just revert the entire
optimization, as it has proved to do more harm than good. Moreover, it's
hard to argue or prove that there will be no more such regressions.
However, all the work wasn't for nothing. First, we learned that the way
the decoder unquotes tokenized strings isn't simple; initially, we had
wrongly assumed that each string was unquoted exactly once and in order.
Second, we have gained a number of regression tests which will be useful
to prevent the same mistakes in the future, including the test cases we
add in this CL.
Fixes#39555.
Change-Id: I66a6919c2dd6d9789232482ba6cf3814eaa70f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/237838
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TestNetpollBreak was sometimes timing out on Plan 9, where
netpoll_stub.go implements only enough of the network poller
to support runtime timers, using a notetsleep / notewakeup
pair. The runtime.lock which serialises the use of the note
doesn't guarantee fairness, and in practice the netpoll call
used by the test can be starved by the netpoll call from the
scheduler which supports the overall 'go test' timeout.
Calling osyield after relinquishing the lock gives the two
callers a more even chance to take a turn, which prevents
the test from timing out.
Fixes#39437
Change-Id: Ifbe6aaf95336d162d9d0b6deba19b8debf17b071
Reviewed-on: https://go-review.googlesource.com/c/go/+/237698
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The benchmark throughput numbers don't change, but the set-up time (the
time taken before the b.ResetTimer() call) drops from 460ms to 4ms.
Change-Id: I5a6756643dff6127f6d902455d83459c084834fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/237757
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This alert is triggering occasionally. I've investigated the
collisions that happen, and they all seem to be pairwise, so they are
not a big deal. "pairwise" = when there are 32 collisions, it is two
keys mapping to the same hash, 32 times, not 33 keys all mapping to
the same hash.
Add some t.Logf calls in case this comes back, which will help isolate
the problem.
Fixes#39352
Change-Id: I1749d7c8efd0afcf9024d8964d15bc0f58a86e4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/237718
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Support for linux/386 was added to Delve in version 1.4.1, but the
version of Delve currently installed on the linux-386-longtest
builder is 1.2.0. That isn't new enough, which causes the test
to fail. Skip it on that builder until it can be made to work.
The only reason it used to pass on the linux-386-longtest builder
before is because that builder was misconfigured to run tests for
linux/amd64. This was resolved in CL 234520.
Also improve internal documentation and the text of skip reasons.
Fixes#39309.
Change-Id: I395cb1f076e59dd3a3feb53e1dcdce5101e9a0f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/237603
Reviewed-by: David Chase <drchase@google.com>
Currently, a method of a reachable type is live if it matches a
method of a reachable interface. In fact, we only need to retain
the method if the type is actually converted to an interface. If
the type is never converted to an interface, there is no way to
call the method through an interface method call (but the type
descriptor could still be used, e.g. in calling
runtime.newobject).
A type can be used in an interface in two ways:
- directly converted to interface. (Any interface counts, as it
is possible to convert one interface to another.)
- obtained by reflection from a related type (e.g. obtaining an
interface of T from []T).
For the former, we let the compiler emit a marker on the type
descriptor symbol when it is converted to an interface. In the
linker, we only need to check methods of marked types.
For the latter, when the linker visits a marked type, it needs to
visit all its "child" types as marked (i.e. potentially could be
converted to interface).
This reduces binary size:
cmd/compile 18792016 18706096 (-0.5%)
cmd/go 14120572 13398948 (-5.1%)
Change-Id: I4465c7eeabf575f4dc84017214c610fa05ae31fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/237298
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Other command line arguments are written in code tags, so add a code tag for consistency.
For #37419
Change-Id: I1948536c3a1860d93726484be2dc7bcb03dfdc2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/237539
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Replace tab indentation with spaces for consistency, as all other indentation is done with spaces.
For #37419
Change-Id: I728a75ae0d00e637f57eb455b6039ffc1a5feed2
Reviewed-on: https://go-review.googlesource.com/c/go/+/237538
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
