For compiler developers interested in seeing DWARF generation details,
this patch provides symbol "debug asm" dumps for DWARF aux symbols
when -S=2 is in effect.
Change-Id: I5a0b6b65ce7b708948cbbf23c6b0d279bd4f8d9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/223017
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When the compiler emits DWARF for a function F, in addition to the
text symbol for F, it emits a set of sibling or child symbols that
carry the various DWARF bits for F (for example, go.info.F,
go.ranges.F, go.loc.F, and so on).
Prior to the linker modernization work, name lookup was the way you
made your way from a function symbol to one of its child DWARF
symbols. We now have a new mechanism (aux symbols), so there is really
no need for the DWARF sub-symbols to be named or to be dupok.
This patch converts DWARF "range" and "loc" sub-symbols to be pure aux
syms: unnamed, and connected to their parent text symbol only via aux
data. This should presumably have performance benefits in that we add
fewer symbols to the linker lookup tables.
Other related DWARF sub-symbols (ex: go.line.*) will be handled in a
subsequent patch.
Change-Id: Iae3ec2d42452962d4afc1df4a1bd89ccdeadc6e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/222673
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The linker DWARF-gen's line table writing routine contains a loop that
walks all abstract function DIEs looking for files that aren't
referenced in concrete function DIEs. Turns out this loop is no longer
necessary, most likely because the compiler emits an explicit DWARF
file table into the object file.
This patch removes the offending loop. This is a prelude to some
additional work that will hopefully get rid of file renumbering in
writelines altogether (still WIP).
Change-Id: I3b3a9acce1bae7dda878ab6de2d3436de302712e
Reviewed-on: https://go-review.googlesource.com/c/go/+/223145
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>
S390X uses .got instead of .got.plt. It is changed accidentally
in CL 222977. This CL fixes it.
Also, on S390X, we need to set the relocation "variant" of
R_PCREL relocation. In the old code AddPCRelPlus has the magic.
Here we use the equivalent R_PCRELDBL, as the loader doesn't
have variant.
Fix S390X build.
Change-Id: I388e16f02a0568d70287aa9a132fd42b442e3905
Reviewed-on: https://go-review.googlesource.com/c/go/+/223143
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Regenerate this file, since has become a bit out of date.
Change-Id: I4bfa3820f23fb9df36f9a48e63898f4c5de8b31a
Reviewed-on: https://go-review.googlesource.com/c/go/+/223058
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>
Remove the various bits of code in the linker that patch DWARF name
attributes (rewrite "". to packagepath).
Change-Id: I29a38d680d411ccf1c1eeb760f2d15fc8ec22d90
Reviewed-on: https://go-review.googlesource.com/c/go/+/222719
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Adds a new "-p" option to the assembler, for specifying the import
path of the package being compiled. DWARF generation is now conditional
on having a valid package path -- if we don't know the package path,
then don't emit DWARF.
This is intended to lay the groundwork for removing the various
"patchDWARFname" hacks in the linker.
Change-Id: I5f8315c0881791eb8fe1f2ba32f5bb0ae76f6b98
Reviewed-on: https://go-review.googlesource.com/c/go/+/222718
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>
Remove the temporary "-newdw2" linker command line option, along with
the remainder of the legacy sym.Symbol based DWARF generation code.
Change-Id: I86c0581dd021cd4e2209ca9bc45f34037d42323c
Reviewed-on: https://go-review.googlesource.com/c/go/+/222766
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The methods of compilationUnitByStartPC (used in DWARF generation)
were looking at comp unit sym.Symbols instead of loader.Sym's, which
will not be viable once the wavefront reaches DWARF gen phase two.
Rewrite the methods to use only loader.Sym.
Change-Id: I0f520399d5458079c48cff1d882ef879934f8e92
Reviewed-on: https://go-review.googlesource.com/c/go/+/222759
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Move to a shared/reused slice of loader.Reloc's in a couple of places
in the linker's DWARF writelines method, as opposed to allocating a
new slice each time. Small performance improvement.
Change-Id: I06c64cf7ca00162d24d9421f1d6405c8808ebb83
Reviewed-on: https://go-review.googlesource.com/c/go/+/222758
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In a couple of places in the DWARF type generation code there were
calls to the older sym.Symbol "newdie" funtion as opposed to the
loader.Sym based method. This patch converts these to method calls.
Change-Id: I202957464b7c37063ff74e834ed7b76d1dc5bcdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/222757
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Remove another set of unused functions, no longer needed now that
loader.Sym based DWARF type generation is the default.
Change-Id: Ie3e63551d75acf559c550768ba6c95fe4d6e67ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/222021
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>
dodata is a pile of dependencies on *Link. Pull some of these
dependencies out, and tighten up the visibility on the functions.
Change-Id: Id213f19ae7b63dd246ddb47b77ffde99f615aa80
Reviewed-on: https://go-review.googlesource.com/c/go/+/222061
Reviewed-by: Than McIntosh <thanm@google.com>
I tried a couple of different architectures (goroutine per symbol, 8
goroutines handling symbols from channels, and this architecture), and
this was the best. Another possible approach could be to divide up the
space of relocations, forgo the channels, and just pass slices to the
relocation routines, which would possibly be faster.
Reloc 13.9ms ± 5% 9.0ms ±10% -35.09% (p=0.000 n=8+9)
Change-Id: I5111220e855313fae4b89d64277759c4dc33b697
Reviewed-on: https://go-review.googlesource.com/c/go/+/220842
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
'go test -json' should report that a test failed if the test binary
did not exit normally with status 0. This covers panics, non-zero
exits, and abnormal terminations.
These tests don't print a final result when run with -test.v (which is
used by 'go test -json'). The final result should be "PASS" or "FAIL"
on a line by itself. 'go test' prints "FAIL" in this case, but
includes error information.
test2json was changed in CL 192104 to report that a test passed if it
does not report a final status. This caused 'go test -json' to report
that a test passed after a panic or non-zero exit.
With this change, test2json treats "FAIL" with error information the
same as "FAIL" on a line by itself. This is intended to be a minimal
fix for backporting, but it will likely be replaced by a complete
solution for #29062.
Fixes#37555
Updates #29062
Updates #31969
Change-Id: Icb67bcd36bed97e6a8d51f4d14bf71f73c83ac3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/222243
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Implement a TODO in CL 220059, now that CL 220985 is in.
Change-Id: I7273a7b9e1e7c0b916780571c9c954da21366429
Reviewed-on: https://go-review.googlesource.com/c/go/+/222239
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Remove a few lines of debugging trace output code left in
by accident when the final version of CL 220987 was submitted.
Change-Id: Ib84a4658194266597140a0e88a2f90f57f78cf53
Reviewed-on: https://go-review.googlesource.com/c/go/+/222299
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
When generating stacks, the runtime automatically expands inline
functions to inline all inline frames in the stack. However, due to the
stack size limit, the final frame may be truncated in the middle of
several inline frames at the same location.
As-is, we assume that the final frame is a normal function, and emit and
cache a Location for it. If we later receive a complete stack frame, we
will first use the cached Location for the inlined function and then
generate a new Location for the "caller" frame, in violation of the
pprof requirement to merge inlined functions into the same Location.
As a result, we:
1. Nondeterministically may generate a profile with the different stacks
combined or split, depending on which is encountered first. This is
particularly problematic when performing a diff of profiles.
2. When split stacks are generated, we lose the inlining information.
We avoid both of these problems by performing a second expansion of the
last stack frame to recover additional inline frames that may have been
lost. This expansion is a bit simpler than the one done by the runtime
because we don't have to handle skipping, and we know that the last
emitted frame is not an elided wrapper, since it by definition is
already included in the stack.
Fixes#37446
Change-Id: If3ca2af25b21d252cf457cc867dd932f107d4c61
Reviewed-on: https://go-review.googlesource.com/c/go/+/221577
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
The test will remain flaky on the -nocgo builder until #37695 is addressed.
Updates #37695Fixes#33041
Change-Id: I5d661ef39e82ab1dce3a76e0e4059cf556135e89
Reviewed-on: https://go-review.googlesource.com/c/go/+/222158
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This was the last remaining use of staticbytes, so we can now
delete it.
The new code appears slightly faster on amd64:
name old time/op new time/op delta
SliceByteToString/1-4 6.29ns ± 2% 5.89ns ± 1% -6.46% (p=0.000 n=14+14)
This may not be the case on the big-endian architectures, since they have
to do an extra addition.
Updates #37612
Change-Id: Icb84c5911ba025f798de152849992a55be99e4f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/221979
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is one of several changes that were part of a larger rewrite
which I made in early 2019 after switching to the new number literal
syntax implementation. The purpose of the rewrite was to simplify
reading of source code (Unicode character by character) and speed up
the scanner but was never submitted for review due to other priorities.
Part 3 of 3:
This change contains a complete rewrite of source.go, the file that
implements reading individual Unicode characters from the source.
The new implementation is easier to use and has simpler literal
buffer management, resulting in faster scanner and thus parser
performance.
Thew new source.go (internal) API is centered around nextch() which
advances the scanner by one character. The scanner has been adjusted
around nextch() and now consistently does one character look-ahead
(there's no need for complicated ungetr-ing anymore). Only in one
case backtrack is needed (when finding '..' rather than '...') and
that case is now more cleanly solved with the new reset() function.
Measuring line/s parsing peformance by running
go test -run StdLib -fast -skip "syntax/(scanner|source)\.go"
(best of 5 runs on "quiet" MacBook Pro, 3.3GHz Dual-Core i7, 16GB RAM,
OS X 10.15.3) before and after shows consistently 3-5% improvement of
line parsing speed:
old: parsed 1788155 lines (3969 files) in 1.255520307s (1424234 lines/s)
new: parsed 1788155 lines (3969 files) in 1.213197037s (1473919 lines/s)
(scanner.go and parser.go are skipped because this CL changed those files.)
Change-Id: Ida947f4b538d42eb2d2349062c69edb6c9e5ca66
Reviewed-on: https://go-review.googlesource.com/c/go/+/221603
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This converts the second portion of DWARF-gen (dwarfGenerateDebugSyms
function and friends) to use the loader, as opposed to sym.Symbols.
Second phase is invoked as part of dodata(), which is fairly far along
in the linker, meaning that the "converted to loader.Symbol" wavefront
has not yet reached that point.
To account for this, the patch contains some shim code that runs
initially to copy back things from sym.Symbols into the loader
(notable symbol values), and then a second hook to be code after
dwarf-gen is complete to that copies the results back into the
appropriate sym.Symbols. This code is obviously temporary, but needed
for the time being.
Change-Id: I9b830b08b16480a1a5230cb52b592db1c550af18
Reviewed-on: https://go-review.googlesource.com/c/go/+/220987
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>
Remove references to the loader.Loader's "ocache" field, which is no
longer being used the way it was. Add in a new field for object index
to the oReader type.
Change-Id: Icae6c54dbbdbbfa63ab9de3e4dc09e10abd57ee3
Reviewed-on: https://go-review.googlesource.com/c/go/+/222063
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Add a new loader method PatchDWARFName to patch up live DWARF function
DIEs during DWARF generation. This is needed to handle subprogram DIE
symbols emitted by the assembler, which still embeds "". package
tokens into the data sections of these dies.
Note: this is expected to be a temporary hack, as we are going to
transition the assembler to do the patching itself when passed the
"-p" option (once this happens the plan is to toss all of the various
PatchDWARFName helpers).
Change-Id: Id689a751f08d7f4c096c4ac2f99991f9641959e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/220986
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 loader method AssignTextSymbolOrder to return a slice of
all reachable textp symbols, since it will be needed in second-phase
DWARF gen.
Change-Id: Iaf16ee9cf0d5266aeb0d3df596e8117263b35d8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/220985
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>
This allows for zero stores to be performed using the zero register, rather
than loading a separate register with zero.
Change-Id: Ic81d8dbcdacbb2ca2c3f77682ff5ad7cdc33d18d
Reviewed-on: https://go-review.googlesource.com/c/go/+/221684
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
These changes fix go doc -src mode that vomits comments from random files if
filesystem does not sort files by name. The issue was with parse.ParseDir
using the Readdir order of files, which varies between platforms and filesystem
implementations. Another option is to merge comments using token.FileSet.Iterate
order in cmd/doc, but since ParseDir is mostly used in go doc, I’ve opted for
smaller change because it’s unlikely to break other uses or cause any perfomance
issues.
Example (macOS APFS): `go doc -src net.ListenPacket`
Change-Id: I7f9f368c7d9ccd9a2cbc48665f2cb9798c7b3a3f
GitHub-Last-Rev: 654fb45042
GitHub-Pull-Request: golang/go#36104
Reviewed-on: https://go-review.googlesource.com/c/go/+/210999
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This is one of several changes that were part of a larger rewrite
which I made in early 2019 after switching to the new number literal
syntax implementation. The purpose of the rewrite was to simplify
reading of source code (Unicode character by character) and speed up
the scanner but was never submitted for review due to other priorities.
Part 2 of 3:
This change contains improvements to the scanner error messages:
- Use "rune literal" rather than "character literal" to match the
spec nomenclature.
- Shorter, more to the point error messages.
(For instance, "more than one character in rune literal" rather
than "invalid character literal (more than one character)", etc.)
Change-Id: I1aaf79003374a68dbb05926437ed305cf2a8ec96
Reviewed-on: https://go-review.googlesource.com/c/go/+/221602
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is one of several changes that were part of a larger rewrite
which I made in early 2019 after switching to the new number literal
syntax implementation. The purpose of the rewrite was to simplify
reading of source code (Unicode character by character) and speed up
the scanner but was never submitted for review due to other priorities.
Part 1 of 3:
This change contains improvements to the scanner tests.
Change-Id: Iecfcaef00fdeb690b0db786edbd52e828417141b
Reviewed-on: https://go-review.googlesource.com/c/go/+/221601
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There are still two places in src/runtime/string.go that use
staticbytes, so we cannot delete it just yet.
There is a new codegen test to verify that the index calculation
is constant-folded, at least on amd64. ppc64, mips[64] and s390x
cannot currently do that.
There is also a new runtime benchmark to ensure that this does not
slow down performance (tested against parent commit):
name old time/op new time/op delta
ConvT2EByteSized/bool-4 1.07ns ± 1% 1.07ns ± 1% ~ (p=0.060 n=14+15)
ConvT2EByteSized/uint8-4 1.06ns ± 1% 1.07ns ± 1% ~ (p=0.095 n=14+15)
Updates #37612
Change-Id: I5ec30738edaa48cda78dfab4a78e24a32fa7fd6a
Reviewed-on: https://go-review.googlesource.com/c/go/+/221957
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This is the last step requried before relocsym can go parallel.
Change-Id: Id1c1c530c2b9277917208c3767eeb29e02c17a9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/220841
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>
Also correct a few places I hadn't switched over to Target from *Link
context.
Change-Id: I6de6b39723762de3fa5f8d12800085421aca76fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/220840
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Increase the size of the signal stack as the value given by SIGSTKSZ
is too small for the Go signal handler.
Fixes#37609
Change-Id: I56f1006bc69a2a9fb43f9e0da00061964290a690
Reviewed-on: https://go-review.googlesource.com/c/go/+/221804
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The type symbols were always loaded in LoadFull, as the old
sym.Symbol-based DWARF generation needs them to present. Now we
have converted the DWARF type generation pass, which uses the
loader symbols directly. No need to load the type symbols if they
are not live.
This reduces loaded symbols by ~18% for cmd/compile.
Change-Id: Ic1f6fcdff845f3c62784d05ad716b1742f249481
Reviewed-on: https://go-review.googlesource.com/c/go/+/222018
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL converts the stack bound check pass to use the loader and
new symbol representation.
Change-Id: I0e4c102909db20dc2d5a367379146728862e53cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/220059
Reviewed-by: Than McIntosh <thanm@google.com>
Trying this CL again, with a fixed test that allows platforms
to disagree on the exact behavior of converting NaNs.
We store 32-bit floating point constants in a 64-bit field, by
converting that 32-bit float to 64-bit float to store it, and convert
it back to use it.
That works for *almost* all floating-point constants. The exception is
signaling NaNs. The round trip described above means we can't represent
a 32-bit signaling NaN, because conversions strip the signaling bit.
To fix this issue, just forbid NaNs as floating-point constants in SSA
form. This shouldn't affect any real-world code, as people seldom
constant-propagate NaNs (except in test code).
Additionally, NaNs are somewhat underspecified (which of the many NaNs
do you get when dividing 0/0?), so when cross-compiling there's a
danger of using the compiler machine's NaN regime for some math, and
the target machine's NaN regime for other math. Better to use the
target machine's NaN regime always.
Update #36400
Change-Id: Idf203b688a15abceabbd66ba290d4e9f63619ecb
Reviewed-on: https://go-review.googlesource.com/c/go/+/221790
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
