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.
Fix AIX build.
Change-Id: Ie2515a4cabbd8cf3f6d3868643a28f64ca3365a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/246479
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
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>
The hash maps are used to deduplicate hashed symbols. Once we
loaded all the symbols, we no longer need the hash maps. Drop
them.
Linking cmd/compile,
name old live-B new live-B delta
Loadlib_GC 13.1M ± 0% 11.3M ± 0% -13.62% (p=0.008 n=5+5)
Change-Id: I4bb1f84e1111a56d9e777cd6a68f7d974b60e321
Reviewed-on: https://go-review.googlesource.com/c/go/+/245721
Reviewed-by: Jeremy Faller <jeremy@golang.org>
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>
The type descriptor symbol of a defined (named) type (and pointer
to it) is defined only in the package that defines the type. It
is not dupOK, unlike other type descriptors. So it can be
referenced by index. Currently it is referenced by name for
cross-package references, because the index is not exported and
so not known to the referencing package.
This CL passes the index through the export data, so the symbol
can be referenced by index, and does not need to be looked up by
name. This also makes such symbol references consistent: it is
referenced by index within the defining package and also cross-
package, which makes it easier for content hashing (in later CLs).
One complication is that we need to set flags on referenced
symbols (specifically, the UsedInIface flag). Before, they are
non-package refs, which naturally carry flags in the object file.
For indexed refs, we currently don't put their flags in the
object file. Introduce a new block for this.
Change-Id: I8126f8e318ac4e6609eb2ac136201fd6c264c256
Reviewed-on: https://go-review.googlesource.com/c/go/+/245718
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Not used yet, but add the compilation unit for a function to func.
Change-Id: I7c43fa9f1da044ca63bab030062519771b9f4418
Reviewed-on: https://go-review.googlesource.com/c/go/+/244547
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Leaving creation of the funcID till the linker requires the linker to
load the function and file names into memory. Moving these into the
compiler/assembler prevents this.
This work is a step towards moving all func metadata into the compiler.
Change-Id: Iebffdc5a909adbd03ac263fde3f4c3d492fb1eac
Reviewed-on: https://go-review.googlesource.com/c/go/+/244024
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
The .syso test also fails for ppc64le. Not sure why. For now, just
disable the test for that architecture. The test really only needs to
run on a single builder of any arch.
Change-Id: I346cdc01ada09d43c4c504fbc30be806f59d5422
Reviewed-on: https://go-review.googlesource.com/c/go/+/246358
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We can't depend on getauxval because it only exists in glibc >= 2.16.
Tsan has been updated to avoid that dependency
(https://reviews.llvm.org/D84859). This CL rebuilds the affected
.syso files, and adds a test to make sure we don't regress.
Fixes#37485
Change-Id: I891f54d28ec0d7da50a8df1adadc76dd6e7ab3e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/246258
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
For dupOK symbols, their attributes should be OR'd. Most of the
attributes are expected to be set consistently across multiple
definitions, but UsedInIface must be OR'd, and for alignment we
need to pick the largest one. Currently the attributes are not
always OR'd, depending on addSym returning true or false. This
doesn't cause any real problem, but it would be a problem if we
make type descriptor symbols content-addressable.
This CL removes the second result of addSym, and lets preloadSyms
always set the attributes. Also removes the alignment handling on
addSym, handles it in preloadSyms only.
Change-Id: I06b3f0adb733f6681956ea9ef54736baa86ae7bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/245720
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Consider this test package:
package p
// enum E { E0 };
// union U { long x; };
// void f(enum E e, union U* up) {}
import "C"
func f() {
C.f(C.enum_E(C.E0), (*C.union_U)(nil))
}
In Go 1.14, cgo translated this to (omitting irrelevant details):
type _Ctype_union_U [8]byte
func f() {
_Cfunc_f(uint32(_Ciconst_E0), (*[8]byte)(nil))
}
func _Cfunc_f(p0 uint32, p1 *[8]byte) (r1 _Ctype_void) { ... }
Notably, _Ctype_union_U was declared as a defined type, but uses were
being rewritten into uses of the underlying type, which matched how
_Cfunc_f was declared.
After CL 230037, cgo started consistently rewriting "C.foo" type
expressions as "_Ctype_foo", which caused it to start emitting:
type _Ctype_enum_E uint32
type _Ctype_union_U [8]byte
func f() {
_Cfunc_f(_Ctype_enum_E(_Ciconst_E0), (*_Ctype_union_U)(nil))
}
// _Cfunc_f unchanged
Of course, this fails to type-check because _Ctype_enum_E and
_Ctype_union_U are defined types.
This CL changes cgo to emit:
type _Ctype_enum_E = uint32
type _Ctype_union_U = [8]byte
// f unchanged since CL 230037
// _Cfunc_f still unchanged
It would probably be better to fix this in (*typeConv).loadType so
that cgo generated code uses the _Ctype_foo aliases too. But as it
wouldn't have any effect on actual compilation, it's not worth the
risk of touching it at this point in the release cycle.
Updates #39537.
Fixes#40494.
Change-Id: I88269660b40aeda80a9a9433777601a781b48ac0
Reviewed-on: https://go-review.googlesource.com/c/go/+/246057
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently pageAlloc.find attempts to find a better estimate for the
first free page in the heap, even if the space its looking for isn't
necessarily going to be the first free page in the heap (e.g. if npages
>= 2). However, in doing so it has the potential to return a searchAddr
candidate that doesn't actually correspond to mapped memory, but this
candidate might still be adopted. As a result, pageAlloc.alloc's fast
path may look at unmapped summary memory and segfault. This case is rare
on most operating systems since the heap is kept fairly contiguous, so
the chance that the candidate searchAddr discovered is unmapped is
fairly low. Even so, this is totally possible and outside the user's
control when it happens (in fact, it's likely to happen consistently for
a given user on a given system).
Fix this problem by ensuring that our candidate always points to mapped
memory. We do this by looking at mheap's arenas structure first. If it
turns out our candidate doesn't correspond to mapped memory, then we
look at inUse to round up the searchAddr to the next mapped address.
While we're here, clean up some documentation related to searchAddr.
Fixes#40191.
Change-Id: I759efec78987e4a8fde466ae45aabbaa3d9d4214
Reviewed-on: https://go-review.googlesource.com/c/go/+/242680
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We're reworking pclntab generation in the linker, and with that we're
moving FuncID generation in to the compiler. Determining the FuncID is
done by a lookup on the package.function name; therefore, we need the
package whenever we make the TEXT symbols.
Change-Id: I805445ffbf2f895f06ce3a91fb09126d012bf86e
Reviewed-on: https://go-review.googlesource.com/c/go/+/245318
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Move the function names out of runtime.pclntab_old, creating
runtime.funcnametab. There is an unfortunate artifact in this change in
that calculating the funcID still requires loading the name. Future work
will likely pull this out and put it into the object file Funcs.
ls -l cmd/compile (darwin):
before: 18524016
after: 18519952
The difference in size can be attributed to alignment in pclntab_old.
Change-Id: Ibcbb230d4632178f8fcd0667165f5335786381f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/243223
Reviewed-by: Austin Clements <austin@google.com>
Non functional change.
As runtime.pclntab breaks up, it'll be easier if we can just pass around
the pclntab state. Also, eliminate the globals in pclntab.
Change-Id: I2a5849e8f5f422a336a881e53a261e3997d11c44
Reviewed-on: https://go-review.googlesource.com/c/go/+/242599
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The SUNDEFEXT case was lost during the refactoring. Add it back.
Fix ppc64le build.
Change-Id: I14594ee2c3e0a794c93839247fb3e6206c2e657a
Reviewed-on: https://go-review.googlesource.com/c/go/+/245919
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
As of July 2020, a fair amount of the new linker's live memory, and
runtime is spent generating pclntab. In an effort to streamline that
code, this change starts breaking up the generation of runtime.pclntab
into smaller chunks that can run later in a link. These changes are
described in an (as yet not widely distributed) document that lays out
an improved format. Largely the work consists of breaking up
runtime.pclntab into smaller pieces, stopping much of the data
rewriting, and getting runtime.pclntab into a form where we can reason
about its size and look to shrink it. This change is the first part of
that work -- just pulling out the header, and demonstrating where a
majority of that work will be.
Change-Id: I65618d0d0c780f7e5977c9df4abdbd1696fedfcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/241598
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
The symbol's data in the object file (sym.P) may already not
contain trailing zeros (e,g, for [10]int{1}), but sometimes it
does (e.g. for [10]int{1,0}). The linker can already handle this
case. We just always trim the trailing zeros for content hashing,
so it can deduplicate [10]int{1} and [10]int{1,0}.
Note: in theory we could just trim the zeros in the symbol data
as well. But currently the linker depends on reading symbol data
for certain symbols (e.g. type symbol decoding), and trimming
will complicates things in the linker.
Change-Id: I9e90e41e6ac808b36855b0713a85e61c33bf093a
Reviewed-on: https://go-review.googlesource.com/c/go/+/245717
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently in addLocalInductiveFacts, we only check whether
direct edge from if block to phi block exists. If not, the
following logic will treat the phi block as the first successor,
which is wrong.
This patch makes prove pass more conservative, so we disable
some cases in test/prove.go. We will do some optimization in
the following CL and enable these cases then.
Fixes#40367.
Change-Id: I27cf0248f3a82312a6f7dabe11c79a1a34cf5412
Reviewed-on: https://go-review.googlesource.com/c/go/+/244579
Reviewed-by: Zach Jones <zachj1@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rename Reloc2 to Reloc, At2 to At, Aux2 to Aux.
Change-Id: Ic98d83c080e8cd80fbe1837c8f0aa134033508ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/245578
Reviewed-by: Jeremy Faller <jeremy@golang.org>
We have Reloc and Reloc2. Reloc2 is the better approach and most
code uses Reloc2. There are still uses of Reloc. This CL migrates
them to Reloc2, and removes Reloc.
Change-Id: Id5f6a6019e1e044add682d05e70ebb1548ec58d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/245577
Reviewed-by: Jeremy Faller <jeremy@golang.org>
We used to generate all external relocations in memory, then emit
the relocation records at a later pass. The data structures were
chosen so that it takes as little memory as possible. Now we just
stream out external relocations, and ExtReloc is just a local
variable. Change the data structure to avoid repeated read of
some fields. Also get rid of ExtRelocView, as it is no longer
necessary.
Change-Id: I40209bbe4387af231b29788125c3b4ebb0ff4a33
Reviewed-on: https://go-review.googlesource.com/c/go/+/245479
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This reverts CL 202578 and CL 230677 which added an optimization
to use KDSA when available on s390x.
Inconsistencies have been found between the two implementations
in their handling of certain edge cases. Since the Go 1.15 release
is extremely soon it seems prudent to remove this optimization
for now and revisit it in a future release.
Fixes#40475.
Change-Id: Ifb2ed9b9e573784df57383671f1c29d8abae90d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/245497
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ruixin(Peter) Bao <ruixin.bao@ibm.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
globrunqputbatch should never be called without sched.lock held.
runqputbatch's documentation even says it may acquire sched.lock in
order to call it.
Fixes#40457.
Change-Id: I5421b64f1da3a6087dfebbef7203db0c95d213a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/245377
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We accidentally passed the address of a local to a function
pointer, where we should pass the address of a global.
Linking cmd/compile with external linking:
Asmb2_GC 32.5ms ± 5% 21.6ms ± 3% -33.57% (p=0.016 n=5+4)
Asmb2_GC 29.2MB ± 0% 6.4MB ± 0% -78.20% (p=0.008 n=5+5)
Asmb2_GC 1.43M ± 0% 0.00M ± 4% -99.98% (p=0.008 n=5+5)
Change-Id: I4754189bcc20f824627d95858ba35285d53c614d
Reviewed-on: https://go-review.googlesource.com/c/go/+/245337
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
If no M is available, startm first grabs an idle P, then drops
sched.lock and calls newm to start a new M to run than P.
Unfortunately, that leaves a window in which a G (e.g., returning from a
syscall) may find no idle P, add to the global runq, and then in stopm
discover that there are no running M's, a condition that should be
impossible with runnable G's.
To avoid this condition, we pre-allocate the new M ID in startm before
dropping sched.lock. This ensures that checkdead will see the M as
running, and since that new M must eventually run the scheduler, it will
handle any pending work as necessary.
Outside of startm, most other calls to newm/allocm don't have a P at
all. The only exception is startTheWorldWithSema, which always has an M
if there is 1 P (i.e., the currently running M), and if there is >1 P
the findrunnable spinning dance ensures the problem never occurs.
This has been tested with strategically placed sleeps in the runtime to
help induce the correct race ordering, but the timing on this is too
narrow for a test that can be checked in.
Fixes#40368
Change-Id: If5e0293a430cc85154b7ed55bc6dadf9b340abe2
Reviewed-on: https://go-review.googlesource.com/c/go/+/245018
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
They were missed as part of the refactoring to use a separate
addressing modes pass.
Fixes#40426
Change-Id: Ie0418b2fac4ba1ffe720644ac918f6d728d5e420
Reviewed-on: https://go-review.googlesource.com/c/go/+/244859
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
During the transitioning period, we mark symbols from Go shared
libraries reachable unconditionally. That might be useful when
there was still a large portion of the linker using sym.Symbols,
and only reachable symbols were converted to sym.Symbols. Marking
them reachable brings them to the dynamic symbol table, even if
they are not needed, increased the binary size unexpectedly.
That time has passed. Now we largely operate on loader symbols,
and it is not needed to mark them reachable anymore.
Fixes#40416.
Change-Id: I1e2bdb93a960ba7dc96575fabe15af93d8e95329
Reviewed-on: https://go-review.googlesource.com/c/go/+/244839
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This fixes a mistake in CL 220422. This changes code that is only
executed on Linux kernel versions earlier than 2.6.27.
Change-Id: I01280184f4d7b75e06387c38f1891e8f0a81f793
Reviewed-on: https://go-review.googlesource.com/c/go/+/244630
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
For #38029
Change-Id: I71de2b66c1de617d32c46d4f2c1866f9ff1756ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/244631
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Both ELF and Xcoff.
Now we support streaming on all platforms. Later CLs will clean
up the old code.
Change-Id: Ieeef7844a3e229429983a8bc108d7f3fabf618e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/244358
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Do them in the same CL so ARM's archreloc doesn't need to support
both streaming and non-streaming.
TODO: we haven't switched to using mmap to emit external
relocations on Windows.
Change-Id: Ica2ee89c03fc74839efd6b9e26c80585fcdce45c
Reviewed-on: https://go-review.googlesource.com/c/go/+/244357
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
All the bits are there. Just need to enable it.
Change-Id: I12ee25317c6385838493dadc900fb57c3b49a416
Reviewed-on: https://go-review.googlesource.com/c/go/+/244277
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The previous CL changed it to drop the Data when writing out a
symbol. Don't read the data.
Fix ARM64 build.
Change-Id: I121e9b0ebef123dbbc4ddffc02bf1a42788532f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/244038
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
OutData was used for a symbol to point to its data in the output
buffer, in order to apply relocations. Now we fold relocation
application to Asmb next to symbol data writing. We can just pass
the output data as a local variable.
Linking cmd/compile,
name old time/op new time/op delta
Asmb_GC 19.0ms ±10% 16.6ms ± 9% -12.50% (p=0.032 n=5+5)
name old alloc/op new alloc/op delta
Asmb_GC 3.78MB ± 0% 0.14MB ± 1% -96.41% (p=0.008 n=5+5)
name old live-B new live-B delta
Asmb_GC 27.5M ± 0% 23.9M ± 0% -13.24% (p=0.008 n=5+5)
Change-Id: Id870a10dce2a0a7447a05029c6d0ab39b47d0a12
Reviewed-on: https://go-review.googlesource.com/c/go/+/244017
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Support streaming external relocations on ARM64. Support
architecture-specific relocations.
Also support streaming external relocations on Darwin. Do it in
the same CL so ARM64's archreloc doesn't need to support both
streaming and non-streaming.
Change-Id: Ia7fee9957892f98c065022c69a51f47402f4d6e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/243644
Reviewed-by: Jeremy Faller <jeremy@golang.org>
For content-addressable symbols, we build its content hash based
on the symbol data and relocations. When the compiler builds the
symbol data, it may not always include the trailing zeros, e.g.
the data of [10]int64{1,2,3} is only the first 24 bytes.
Therefore, we may end up with symbols with the same contents
(thus same hash) but different sizes. This is not actually a hash
collision. In this case, we can deduplicate them and keep the one
with the larger size.
Change-Id: If6834542d7914cc00f917d7db151955e5aee6f30
Reviewed-on: https://go-review.googlesource.com/c/go/+/243718
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The ICE reported as #33308 was fixed by a related CL; this change adds
a regression test with the crasher.
Fixes#33308
Change-Id: I3260075dbe3823b56b8825e6269e57a0fad185a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/243458
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
