Now that we removed the "weird thing" about runtime.etext symbol,
we can remove this special case.
Change-Id: I2e4558367758d37e898a802bcd30671c7dd6fe89
Reviewed-on: https://go-review.googlesource.com/c/go/+/240066
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently, on most platforms, the start/end symbols runtime.text
and runtime.etext are defined in symtab pass and assigned values
in address pass. In some cases (darwin+dynlink or AIX+external),
however, they are defined and assigned values in textaddress pass
(because they need non-zero sizes). Then their values get
overwritten in address pass. This is bad. The linker expects
their values to be consistent. In particular, in CL 239281,
findfunctab is split to two parts. The two parts need to have a
consistent view of the start/end symbols. If its value changes in
between, bad things can happen.
This CL fixes it by always defining runtime.text/etext symbols in
the textaddress pass.
Fix darwin and AIX builds.
Change-Id: Ifdc1bcb69d99be1b7e5b4fd31d473650c03e3b9d
Reviewed-on: https://go-review.googlesource.com/c/go/+/240065
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Create a new class of symbols internal to the linker. These symbols live
in the Loader, and are real smybols, but have no data, only size. After
symbols are allocated in the binary in asmb() a function is called that
is responsible for filling in the data.
This allows the linker to create large symbols, but not pay the price on
the heap memory.
Change-Id: Ib4291fc6e578478057ed2ec163d7b27426f1d5ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/239280
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
It doesn't have to. The type in the aux field is authoritative.
There are cases involving casting from interface{} where pointers
have a placeholder pointer type (because the type is not known when
the IData op is generated).
The check was introduced in CL 13447.
Fixes#39459
Change-Id: Id77a57577806a271aeebd20bea5d92d08ee7aa6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/239817
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
File.Sync was returning *SyscallError instead of *PathError on Plan 9.
Adjust the error type to match other systems.
Fixes#39800
Change-Id: I844e716eb61c193ef78d29cb0b4a3ef790bb3320
Reviewed-on: https://go-review.googlesource.com/c/go/+/239857
Reviewed-by: David du Colombier <0intro@gmail.com>
The Go compiler includes special treatment for a small set of very
commonly used type symbols (26 to be exact); for these types it
doesn't bother to emit type descriptors for "normal" compilations, and
instead only generates them for the runtime package, so as to reduce
object file bloat.
This patch moves the set of type symbols in question from the
PkgIdxNone index space (in the object file) to the PkgIdxBuiltin
space, which saves some work in the compiler and loader (reduces each
package's index space slightly).
Change-Id: I039c805e05c1aef26f035e52760fd0a0af40f7a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/239658
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>
Change the object file writer to avoid adding entries to the object
file string table for builtin functions. This helps save some very
small amount of space in the object file.
Change-Id: Ic3b94a154e00eb4c7378b57613580c7073b841bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/239657
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Now that we've added a os.File.ReadFrom method, io.CopyBuffer to a
os.File will no longer use the provided buffer.
For #16474
For #36817
For #37419
Change-Id: I79a3bf778ff93eab88e88dd9ecbb8c7ea101e868
Reviewed-on: https://go-review.googlesource.com/c/go/+/238864
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
After CL 228645 some mentions of the Deadline methods referred
to the Timeout method, and some to os.ErrDeadlineExceeded.
Stop referring to the Timeout method, to encourage ErrDeadlineExceeded.
For #31449
Change-Id: I27b8ff34f31798f38b06437546886af8cce98ca4
Reviewed-on: https://go-review.googlesource.com/c/go/+/239705
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Switched the generator to using the open source releases of the root
store rather than HTML parsing, while trying to emulate the sorting
algorithm of the table to reduce churn.
Updates #38843
Change-Id: I78608d245eabc2a35c2f98635ed5f1a531ad2ba8
Reviewed-on: https://go-review.googlesource.com/c/go/+/239557
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
On AIX, in relocsym we call Xcoffadddynrel, which adds a
relocation record to a global array. relocsym already runs in
parallel. In the past we only parallelize over segments, and
we call Xcoffadddynrel only for symbols in data segment, so it is
effectively called sequentially. In CL 239197 we started to do
more fine-grained parallelism, so we need to make sure it is safe
to call Xcoffadddynrel in parallel.
Fix AIX build.
Change-Id: I3128193995a5a99d9fa04c8e728e590f17298da3
Reviewed-on: https://go-review.googlesource.com/c/go/+/239561
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 code in the compiler's DWARF line table generation emits line
table ops at the end of each function fragment to reset the state
machine registers back to an initial state, so that when the line
table fragments for each function are stitched together into a
compilation unit, each fragment will have a clean starting point. The
set-file/set-line ops emitted in this code were being applied to the
last row of the line table, however, meaning that they were
overwriting the existing values.
To avoid this problem, add code to advance the PC past the end of the
last instruction in the function, and switch to just using an
end-of-sequence operator at the end of each function instead of
explicit set-file/set-line ops.
Updates #39757.
Change-Id: Ieb30f83444fa86fb1f2cd53862d8cc8972bb8763
Reviewed-on: https://go-review.googlesource.com/c/go/+/239286
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Normally, packages are loaded in dependency order, and if a
Library object is not nil, it is already loaded with the actual
fingerprint. In shared build mode, however, packages may be added
not in dependency order (e.g. go install -buildmode=shared std
adds all std packages before loading them), and it is possible
that a Library's fingerprint is not yet loaded. Skip the check
in this case (when the fingerprint is the zero value).
Fixes#39777.
Change-Id: I66208e92bf687c8778963ba8e33e9bd948f82f3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/239517
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
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>
Follow glibc's implementation and check secondary group memberships
using Getgroups.
No test since we cannot easily change file permissions when not running
as root and the test is meaningless if running as root.
Same as CL 238722 did for x/sys/unix
Updates #39660
Change-Id: I6af50e27b255e33405558947a0ab3dfbc33b2d50
Reviewed-on: https://go-review.googlesource.com/c/go/+/238937
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
