The code that runs as a part of loadlibfull converts the linker's
outer/sub state and sets the sym.Symbol AttrSubSymbol if a symbol has
both A) an outer sym, and B) is listed as a sub-symbol by some other
symbol.
Make sure that we have the same logic in the original loader method,
since we need to use it as part of dodata() prior to loadlibfull.
Change-Id: I200adab741d778a6ba821419e8ea131ad19375bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/229440
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Change the timing for preprocessing of integer/floating point constant
symbols so that we populate them with content at an earlier stage.
This is needed to allow them can be picked up by the loader-API
version of dodata().
Change-Id: Icf09f4f4b318b4f77e11d4a0f0a9cbecd76a1d6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229438
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>
Create a new version of the GCProg type + methods that use loader APIs
instead of sym.Symbol.
This code isn't actually used just yet, but will be needed once the
wavefront reaches dodata() and we need to convert that phase.
Change-Id: I087521832015818204fe5c2ac99c7bd3f61b2bf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/229037
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
With old object files, when objdump an object file which, for
example, contains a call of fmt.Fprintf, it shows a symbol
reference like
R_CALL:fmt.Fprintf
With new object files, as the symbol reference is indexed, the
reference becomes
R_CALL:fmt.#33
The object file does not contain information of what symbol #33
in the fmt package is.
To make this more useful, print the index when dumping the symbol
definitions. This way, when dumping the fmt package, e.g.
"go tool nm fmt.a", it will print
6c705 T fmt.Fprintf#33
So we can find out what symbol #33 actually is.
Change-Id: I320776597d28615ce18dd0617c352d2b8180db49
Reviewed-on: https://go-review.googlesource.com/c/go/+/229246
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
It is no longer needed as we have converted the fieldtrack pass
to using the loader.
Also free loader.Reachparent after we are done with it.
Change-Id: Ibc4b29f282e1e4aea363a1b549755e31f84b0295
Reviewed-on: https://go-review.googlesource.com/c/go/+/229322
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently, we use a dense array to store symbol's sections. The
array element is a *sym.Section, which takes 8 bytes per symbol
on a 64-bit machine. And the array is created upfront.
To reduce memory usage, use a 16-bit index for sections, so we
store 2 bytes per symbol. The array is pointerless, reducing GC
work. Also create the array lazily.
This reduces some memory usage: linking cmd/compile,
name old alloc/op new alloc/op delta
Loadlib_GC 42.1MB ± 0% 36.2MB ± 0% -14.01% (p=0.008 n=5+5)
name old live-B new live-B delta
Loadlib_GC 16.8M ± 0% 15.4M ± 0% -8.36% (p=0.008 n=5+5)
Archive_GC 98.2M ± 0% 97.2M ± 0% -1.02% (p=0.008 n=5+5) # at the end
Change-Id: If8c41eded8859660bca648c5e6fdf5830810fbf6
Reviewed-on: https://go-review.googlesource.com/c/go/+/229306
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Split out DWARF symbol-to-section assignment into its own separate
helper routine, to improve readability. No change in functionality.
Change-Id: Ic2e4f4d99afbff65161cbb8bd63e866ea555f322
Reviewed-on: https://go-review.googlesource.com/c/go/+/228957
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Additional cleanups and refactorings in the allocateSections portion
of dodata. Introduce some new helper routines to be used for common
cases in creating sections and assigning symbols, with a goal of
reducing duplicated code blocks and having more readable code.
No change in functionality.
Change-Id: I1b020b3ee993674329b2bebfd7c35995e3a2c043
Reviewed-on: https://go-review.googlesource.com/c/go/+/228883
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This is more or less a direct translation, to get things going.
There are more things we can do to make it better, especially on
the handling of container symbols.
Change-Id: I11a0087e402be8d42b9d06869385ead531755272
Reviewed-on: https://go-review.googlesource.com/c/go/+/229125
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>
Create a couple of helper routines to aid in assigning symbols to
sections in dodata's allocateSections, then replace loops over symbol
lists with calls to the helpers, to reduce the amount of duplicate
code.
This patch also decouples gcprog/gcdata generation from
symbol-to-section assignment (previously intertwined), as an aid to
making the code less complicated.
No change in functionality.
Change-Id: If126579486bce458f697e32bad556df453df53e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/228781
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Change linker DWARF generation to move away from emitting a single
giant list of DWARF symbols, and instead emit a list of descriptors,
with each descriptor holding the symbols for a specific DWARF section.
While placing all DWARF symbols in a single lists does come in handy
in certain instances, it also creates a lot of confusion and weird
code in other cases, specifically where we want to perform operations
on a section-by-section basis (resulting in code that tries to
re-discover section boundaries by walking/inspecting the list).
Change-Id: I4dac81bd38cba903c9fd7004d613597e76dfb77a
Reviewed-on: https://go-review.googlesource.com/c/go/+/228780
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Move more items into the dodata state object (including the "datsize"
variable used in allocateSections) and the Link ctxt pointer), so as
to prepare for follow-on refactorings. No change in functionality.
Change-Id: Ie2b1651c1ac9b89deb3f7692227dcd931240afa9
Reviewed-on: https://go-review.googlesource.com/c/go/+/228779
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>
The fallocate calls will lower the chances of SIGBUS in the linker, but
it might still happen on other unsupported platforms and filesystems.
Darwin cmd/compile stats:
Munmap 16.0ms ± 8% 0.8ms ± 3% -95.19% (p=0.000 n=8+10)
TotalTime 484ms ± 2% 462ms ± 2% -4.52% (p=0.000 n=10+9)
Updates #37310
Change-Id: I41c6e490adec26fa1ebee49a5b268828f5ba05e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/228385
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
deadcode's been completely replaced. Make its death official.
Change-Id: I85f1e3968463f216b8bce2fb7217c3b51641939f
Reviewed-on: https://go-review.googlesource.com/c/go/+/229002
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When using plugins, on darwin we do weird things with
runtime.etext symbol, assigning a value for it, then clear it,
reassign a different value. This breaks the logic of writing text
address directly.
I think we should remove the weird thing with runtime.etext, if
possible. But for now, disable the optimization (this is not a
common case anyway).
Fix darwin-nocgo build.
Change-Id: Iab6a9f8519115226a5bbaaafe4a93f17042a928a
Reviewed-on: https://go-review.googlesource.com/c/go/+/229057
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
The core CPU profiling loop contains a 100ms sleep.
This is important to reduce overhead.
However, it means that it takes 200ms to shutting down a program
with CPU profiling enabled. When trying to collect many samples
by running a short-lived program many times, this adds up.
This change cuts the shutdown penalty in half by skipping
the sleep whenever possible.
Change-Id: Ic3177f8e1a2d331fe1a1ecd7c8c06f50beb42535
Reviewed-on: https://go-review.googlesource.com/c/go/+/228886
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
All callers to gdata knew the kind of node they were working with,
so all calls to gdata have been replaced with more specific calls.
Some OADDR nodes were constructed solely for the purpose of
passing them to gdata for unwrapping. In those cases, we can now
cut to the chase.
Passes toolstash-check.
Change-Id: Iacc1abefd7f748cb269661a03768d3367319b0b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/228888
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If we are internal linking a static executable, in pclntab
generation, the function addresses are known, so we can just use
them directly instead of emitting relocations.
For external linking or other build modes, we are generating a
relocatable binary so we still need to emit relocations.
Reduce some allocations: for linking cmd/compile,
name old alloc/op new alloc/op delta
Pclntab_GC 38.8MB ± 0% 36.4MB ± 0% -6.19% (p=0.008 n=5+5)
TODO: can we also do this in DWARF generation?
Change-Id: I43920d930ab1da97c205871027e01844a07a5e60
Reviewed-on: https://go-review.googlesource.com/c/go/+/228478
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Recreation of CL 228317.
The problem with that original CL was a late requested change,
reordering reloc and asmb, resulting in symbols having stale pointers to
their data. I've fixed this by preallocating the heap variable in OutBuf
for platforms w/o mmap.
Change-Id: Icdb392ac2c8d6518830f4c84cf422e78b8ab68c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/228782
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This is followup of CL 228861, which remove another un-necessary nil
check for s.Pkg.
Passes toolstash-check.
Change-Id: Ide750beddd2594199af21b56ec6af734dfa55b9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/228862
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 228859 refactored detecting reflect package logic in to isReflectPkg
function. The function has un-necessary nil check for p, so remove that
check.
Passes toolstash-check.
Change-Id: I2f3f1ac967fe8d176dda3f3b4698ded08602e2fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/228861
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
A DWARF testpoint was calling t.Fatal() but should have been calling
t.Fatalf(); switch it to the correct method.
Change-Id: I996a1041adea4299cda85c147a35b513a219b970
Reviewed-on: https://go-review.googlesource.com/c/go/+/228790
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>
Convert all the 386 lowering rules to the typed aux form.
Passes
GOARCH=386 gotip build -toolexec 'toolstash -cmp' -a std
Change-Id: I15256f20bc4442391755e6fffb8206dcaab94830
Reviewed-on: https://go-review.googlesource.com/c/go/+/228818
Reviewed-by: Keith Randall <khr@golang.org>
Currently we only check for reflect.Value.Method. And
reflect.Value.MethodByName is covered since it calls
reflect.Value.Method internally. But it is brittle to rely on
implementation detail of the reflect package. Check for
MethodByName explicitly.
Change-Id: Ifa8920e997524003dade03abc4fb3c4e64723643
Reviewed-on: https://go-review.googlesource.com/c/go/+/228881
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
reflect.Type.Method (and MethodByName) can be used to obtain a
reference of a method by reflection. The linker needs to know
if reflect.Type.Method is called, and retain all exported methods
accordingly. This is handled by the compiler, which marks the
caller of reflect.Type.Method with REFLECTMETHOD attribute. The
current code failed to handle the reflect package itself, so the
method wrapper reflect.Type.Method is not marked. This CL fixes
it.
Fixes#38515.
Change-Id: I12904d23eda664cf1794bc3676152f3218fb762b
Reviewed-on: https://go-review.googlesource.com/c/go/+/228880
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
reflect.Value.Call, if reachable, used to bring all exported
methods live. CL 228792 fixes this, removing the check of
reflect.Value.Call. This CL adds a test.
Updates #38505.
Change-Id: Ib4cab3c3c86c9c9702d041266e59b159d0ff0a97
Reviewed-on: https://go-review.googlesource.com/c/go/+/228878
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The types used while generating code, such as Rule and File, have been
exported for a while. This is harmless for a main package, and lets us
easily differentiate types from variables and functions, as well as use
names like "If" since "if" is a keyword.
However, the fields remained unexported. This was a bit inconsistent,
and also meant that we couldn't use some intuitive names like If.else.
Export them.
Besides the capitalization, the only change is that the If type now has
the fields Then and Else, instead of stmt and alt.
Change-Id: I426ff140c6ca186fec394f17b29165861da5fd98
Reviewed-on: https://go-review.googlesource.com/c/go/+/228821
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Update the comment to be in sync with the code.
Change-Id: I19586767a37347c4da1b4d3f7c6dc6cc2292a90f
Reviewed-on: https://go-review.googlesource.com/c/go/+/228877
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the linker's deadcode pass, we need to keep a method live if
it can be reached through reflection. We do this by marking all
exported method live if reflect.Value.Method or
reflect.Type.Method is used. Currently we also check for
reflect.Value.Call, which is unnecessary because in order to call
a method through reflection, the method must be obtained through
reflect.Value.Method or reflect.Type.Method, which we already
check.
Per discussion in https://groups.google.com/d/msg/golang-dev/eG9It63-Bxg/_bnoVy-eAwAJ
Thanks Brad, Russ, and Ian for bringing this up.
Change-Id: I8e9529a224bb898dbf5752674cc9d155db386c14
Reviewed-on: https://go-review.googlesource.com/c/go/+/228792
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
p.literal's doc comment said it returned a value but it doesn't.
While we're here, p.newLiteral is only called from p.literal,
so simplify the code by merging the two.
Change-Id: Ia357937a99f4e7473f0f1ec837113a39eaeb83d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/222659
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Everybody was deferring a flush when main already
did that, so drop all that nonsense. (Flush was doing
the package clause stuff.) But then make sure we do
get a package clause when there is correctly no output,
as for an empty package. Do that by triggering a
package clause in allDoc and packageDoc.
Slightly tricky but way less intricate than before.
Fixes#37969.
Change-Id: Ia86828436e6c4ab46e6fdaf2c550047f37f353f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/226998
Reviewed-by: Russ Cox <rsc@golang.org>
I'm planning to modify this test in a follow-up CL, so we might
as well convert it to a script test. I don't think there's an easy
way to detect whether we have a case-insensitive file system, without
adding a new condition to the script framework, so the test is just
guessing that darwin and windows could have case-insensitive file systems.
Change-Id: I48bb36f86f19898618681515ac448c3bb4735857
Reviewed-on: https://go-review.googlesource.com/c/go/+/228783
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
At least as far as I can tell, this file never explicitly states whether
locks with higher or lower rank should be taken first. It is implied in
some comments, and clear from the code, of course.
Add an explicit comment to make things more clear and hopefully reduce
new locks being adding in the wrong spot.
Change-Id: I17c6fd5fc216954e5f3550cf91f17e25139f1587
Reviewed-on: https://go-review.googlesource.com/c/go/+/228785
Reviewed-by: Dan Scales <danscales@google.com>
When the seconds param is given, the block and mutex profile endpoints
report the difference between two measurements collected the given
seconds apart. Historically, the block and mutex profiles have reported
the cumulative counts since the process start, and it turned out they
are more useful when interpreted along with the time duration.
Note: cpu profile and trace endpoints already accept the "seconds"
parameter. With this CL, the block and mutex profile endpoints will
accept the "seconds" parameter. Providing the "seconds" parameter
to other types of profiles is an error.
This change moves runtime/pprof/internal/profile to internal/profile and
adds part of merge logic from github.com/google/pprof/profile/merge.go to
internal/profile, in order to allow both net/http/pprof and runtime/pprof
to access it.
Fixes#23401
Change-Id: Ie2486f1a63eb8ff210d7d3bc2de683e9335fd5cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/147598
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
The call does nothing when applied to an OLSH node.
It would be unnecessary anyway, since we're shifting by a small constant.
Passes toolstash-check.
Change-Id: If858711f1704f44637fa0f6a4c66cbaad6db24b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/228699
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This first pass makes the rules using the condition code mask
(CCMask) and rotate parameters (RotateParams) aux values strongly
typed. This required adding strongly typed aux handling to the
block rulegen.
More CLs like this to follow, but this is probably the most
complex.
Passes toolstash-check -all.
Change-Id: Ie513b07d527f0c1b398d7748331442dcb5f7b17d
Reviewed-on: https://go-review.googlesource.com/c/go/+/228518
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This test started failing at CL 228106 and was fixed by CL 228677.
Fixes#38496
Change-Id: I2dadcd99227347e8d28179039f5f345e728c4595
Reviewed-on: https://go-review.googlesource.com/c/go/+/228698
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
