Now we have ctxt.IsAsm, use that, instead of passing in a
parameter.
Change-Id: I81dedbe6459424fa9a4c2bfbd9abd83d83f3a107
Reviewed-on: https://go-review.googlesource.com/c/go/+/234492
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>
If the package path is known, pass it to the object file writer
so the symbol names are pre-expanded. (We already expand the
package path in debug info.)
Change-Id: I2b2b71edbb98924cbf3c4f9142b7e109e5b7501a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234491
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>
Most Go objects are compiled with known package path, so the
symbol name is already fully expanded. Nevertheless, currently
in the linker strings.Replace is called unconditionally, and most
of the time it doesn't do anything.
This CL records a per-object flag in the object file, and do the
name expansion only when the name is not expanded at compile time.
This gives small speedups for the linker. Linking cmd/compile:
name old time/op new time/op delta
Loadlib 35.1ms ± 2% 32.8ms ± 4% -6.43% (p=0.008 n=5+5)
Symtab 15.8ms ± 2% 14.0ms ± 8% -11.45% (p=0.008 n=5+5)
TotalTime 399ms ± 1% 385ms ± 2% -3.63% (p=0.008 n=5+5)
Change-Id: I735084971a051cd9be4284ad294c284cd5b545f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234490
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>
This was causing issues when fuzzing with
TestMarshalUnmarshal since the test would
occassionally set the version to VersionTLS13,
which would fail when unmarshaling. The check
doesn't add much in practice, and there is no
harm in removing it to de-flake the test.
Fixes#38902
Change-Id: I0906c570e9ed69c85fdd2c15f1b52f9e372c62e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/234486
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
v0.3.0 is a tag on 859b3ef565e2, the version that was already being
used. This change is a no-op, except for letting us use a release
version instead of a pseudo-version.
For #36905
Change-Id: I70b8ce2a3f1451f5602c469501362d7a6a673b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/234002
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
The Plan 9 runtime startup was enabling notes (like Unix signals)
before the gsignal stack was allocated. This left a small window
of time where an interrupt (eg by the parent killing a subprocess
quickly after exec) would cause a null pointer dereference in
sigtramp. This would leave the interrupted process suspended in
'broken' state instead of exiting. We've observed this on the
builders, where it can make a test time out waiting for the broken
process to terminate.
Updates #38772
Change-Id: I54584069fd3109595f06c78724c1f6419e028aab
Reviewed-on: https://go-review.googlesource.com/c/go/+/234397
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
The runtime needs to find the PC of the deferreturn call in a few
places. So for functions that have defer, we record the PC of
deferreturn call in its funcdata.
For very large binaries, the deferreturn call could be made
through a trampoline. The current code of finding deferreturn PC
fails in this case. This CL handles the trampoline as well.
Fixes#39049.
Change-Id: I929be54d6ae436f5294013793217dc2a35f080d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234105
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>
This deletes all sym.Symbol and sym.Reloc references. This is
certainly not complete, and there are more cleanups to do. But I
feel this makes a good first round.
Change-Id: I7621d016957f7ef114be5f0606fcb3ad6aee71c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/234097
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Found this while deleting the old code. This should be data2.
Change-Id: I1232fac22ef63bb3a3f25a0558537cc371af3bd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/234098
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>
This part of the test has been flaky despite repeated attempts to fix it,
and it is unclear what exactly it is testing. Remove it.
Fixes#24616.
Change-Id: If7234f99dd3d3e92f15ccb94ee13e75c6da12537
Reviewed-on: https://go-review.googlesource.com/c/go/+/233942
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Currently, for the special field tracking symbol go.track.XXX,
when they are reachable, we set its type to SCONST. There is no
need to do that. Just leave it unset (as Sxxx). The symbol is
done after this point.
Change-Id: I966d80775008f7fb5d30fbc6b9e4a30ae8316b6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/233998
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Don't include SCONST symbols in the symbol table when
NotInSymbolTable is set. This is what the old code (genasmsym)
does.
In fact, SCONST symbol is only emitted by the field tracking
code, and is always NotInSymbolTable. So we should just not
include them at all, or not generate SCONST symbols at all. But
at this late stage I'll just restore the old behavior.
Change-Id: If6843003e16701d45b8c67b2297098a7babdec52
Reviewed-on: https://go-review.googlesource.com/c/go/+/233997
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently maxOffAddr is defined in terms of the whole 64-bit address
space, assuming that it's all supported, by using ^uintptr(0) as the
maximal address in the offset space. In reality, the maximal address in
the offset space is (1<<heapAddrBits)-1 because we don't have more than
that actually available to us on a given platform.
On most platforms this is fine, because arenaBaseOffset is just
connecting two segments of address space, but on AIX we use it as an
actual offset for the starting address of the available address space,
which is limited. This means using ^uintptr(0) as the maximal address in
the offset address space causes wrap-around, especially when we just
want to represent a range approximately like [addr, infinity), which
today we do by using maxOffAddr.
To fix this, we define maxOffAddr more appropriately, in terms of
(1<<heapAddrBits)-1.
This change also redefines arenaBaseOffset to not be the negation of the
virtual address corresponding to address zero in the virtual address
space, but instead directly as the virtual address corresponding to
zero. This matches the existing documentation more closely and makes the
logic around arenaBaseOffset decidedly simpler, especially when trying
to reason about its use on AIX.
Fixes#38966.
Change-Id: I1336e5036a39de846f64cc2d253e8536dee57611
Reviewed-on: https://go-review.googlesource.com/c/go/+/233497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
CL 233857 fixed the underlying issue for #37246,
which had arisen again as #38916.
Add the test case from #37246 to ensure it stays fixed.
Fixes#37246
Change-Id: If7fd75a096d2ce4364dc15509253c3882838161d
Reviewed-on: https://go-review.googlesource.com/c/go/+/233941
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
When tuple generators and selectors are eliminated as part of the
CSE pass we may end up with tuple selectors that are in different
blocks to the tuple generators that they correspond to. This breaks
the invariant that tuple generators and their corresponding
selectors must be in the same block. Therefore after CSE this
situation must be corrected.
Unfortunately the fixup code did not take into account that selectors
could be eliminated by CSE. It assumed that only the tuple generators
could be eliminated. In some situations this meant that it got into
a state where it was replacing references to selectors with references
to dead selectors in the wrong block.
To fix this we move the fixup code after the CSE rewrites have been
applied. This removes any difficult-to-reason-about interactions
with the CSE rewriter.
Fixes#38916.
Change-Id: I2211982dcdba399d03299f0a819945b3eb93b291
Reviewed-on: https://go-review.googlesource.com/c/go/+/233857
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
- Avoid starting subprocesses when the test is already very close to
timing out. The overhead of starting and stopping processes may
cause the test to exceed its deadline even if each individual
process is signaled soon after it is started.
- If a command does not shut down quickly enough after receiving
os.Interrupt, send it os.Kill using the same style of grace period
as in CL 228438.
- Fail the test if a background command whose exit status is not
ignored is left running at the end of the test. We have no reliable
way to distinguish a failure due to the termination signal from an
unexpected failure, and the termination signal varies across
platforms (so may cause failure on one platform but success on
another).
For #38797
Change-Id: I767898cf551dca45579bf01a9d1bb312e12d6193
Reviewed-on: https://go-review.googlesource.com/c/go/+/233526
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Use uint32 consistently for local index (this is what the object
file uses).
Use a index, instead of a pointer, to refer to the object file.
This reduces memory usage and GC work.
This reduces some allocations. Linking cmd/compile,
name old alloc/op new alloc/op delta
Loadlib_GC 19.9MB ± 0% 16.9MB ± 0% -15.33% (p=0.008 n=5+5)
name old live-B new live-B delta
Loadlib_GC 12.6M ± 0% 11.3M ± 0% -9.97% (p=0.008 n=5+5)
Change-Id: I20ce60bbb6d31abd2e9e932bdf959e2ae840ab98
Reviewed-on: https://go-review.googlesource.com/c/go/+/233779
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
While reviewing CL 228784, I noticed that various filepath.WalkFunc
implementations within cmd/go were dropping non-nil errors.
Those errors turn out to be significant, at least in some cases: for
example, they can cause packages to appear to be missing when any
parent of the directory had the wrong permissions set.
(This also turned up a bug in the existing list_dedup_packages test,
which was accidentally passing a nonexistent directory instead of the
intended duplicate path.)
Change-Id: Ia09a0a33aa7a966d9f132d3747d6c674a5370b2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/232579
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
As per discussion on the accepted proposals, enable these vet checks by
default in the go command. Update corresponding documentation as well.
Updates #32479.
Updates #4483.
Change-Id: Ie93471930c24dbb9bcbf7da5deaf63bc1a97a14f
Reviewed-on: https://go-review.googlesource.com/c/go/+/232660
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
- Don't assume that a process interrupted at 100μs intervals will have
enough remaining time to make progress. (Stop sending signals
in between signal storms to allow the process to quiesce.)
- Don't assume that a child process that spins for 1ms will block long
enough for the parent process to receive signals or make meaningful
progress. (Instead, have the child block indefinitely, and unblock
it explicitly after the signal storm.)
For #39043
Updates #22838
Updates #20400
Change-Id: I85cba23498c346a637e6cfe8684ca0c478562a93
Reviewed-on: https://go-review.googlesource.com/c/go/+/233877
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Remove the loader's PropagateSymbolChangesBackToLoader and
PropagateLoaderChangesToSymbols shim functions. These were used at one
point to enable conversion of phases in the linker that were
"downstream" of loadlibfull -- given the current wavefront position
there's not much point keeping them around.
Change-Id: I3f01f25b70b1b80240369c8f3a10dca89931610f
Reviewed-on: https://go-review.googlesource.com/c/go/+/233817
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We have the information from auxs. Remove the field, slightly
reduce memory usage.
Change-Id: I3881777cfb40b03d0e2b0e7a326b0738080548b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/233778
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Now we no longer create loader.Syms array on most platforms. Use
NSym(), instead of len(Syms), for the number of symbols in -v
log.
Change-Id: I8538c00d9c196b701d154eb7d04d911ee2cad73c
Reviewed-on: https://go-review.googlesource.com/c/go/+/233777
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Minor renaming cleanup to get rid of a couple of old sym.Symbol
adddynrel helpers and rename the current crop of adddynrel2
methods/functions back to adddynrel.
Change-Id: I67e76decff84d603ef765f3b6a0cd78c7f3743ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/233523
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This updates vendored x/arch/ppc64 to pick up new instructions
and fixes for objdump on ppc64/ppc64le.
Change-Id: I8262e8a2af09057bbd21b39c9fcf37230029cfe8
Reviewed-on: https://go-review.googlesource.com/c/go/+/233364
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Adds in support for remaining architectures to the linker's ELF asmb2
path, along with deleting most of the older sym.Symbol based code.
Change-Id: I67c96525db72b7d6dd3187cf2b9f6faddc296291
Reviewed-on: https://go-review.googlesource.com/c/go/+/233362
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This patch converts the linker's Asmb2 phase to use loader APIs
for AMD64 (other architectures to be converted in a subsequent
patch).
Change-Id: I5a9aa9b03769cabc1a22b982f48fd113213d7bcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/233338
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Historically we've assumed that we can install all signal handlers
with the SA_RESTART flag set, and let the system restart slow functions
if a signal is received. Therefore, we don't have to worry about EINTR.
This is only partially true, and we've added EINTR checks already for
connect, and open/read on Darwin, and sendfile on Solaris.
Other cases have turned up in #36644, #38033, and #38836.
Also, #20400 points out that when Go code is included in a C program,
the C program may install its own signal handlers without SA_RESTART.
In that case, Go code will see EINTR no matter what it does.
So, go ahead and check for EINTR. We don't check in the syscall package;
people using syscalls directly may want to check for EINTR themselves.
But we do check for EINTR in the higher level APIs in os and net,
and retry the system call if we see it.
This change looks safe, but of course we may be missing some cases
where we need to check for EINTR. As such cases turn up, we can add
tests to runtime/testdata/testprogcgo/eintr.go, and fix the code.
If there are any such cases, their handling after this change will be
no worse than it is today.
For #22838Fixes#20400Fixes#36644Fixes#38033Fixes#38836
Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896
Reviewed-on: https://go-review.googlesource.com/c/go/+/232862
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Taking over Zach's CL 212277. Just cleaned up and added a test.
For a positive, signed integer, an arithmetic right shift of count
(bit-width - 1) equals zero. e.g. int64(22) >> 63 -> 0. This CL makes
prove replace these right shifts with a zero-valued constant.
These shifts may arise in source code explicitly, but can also be
created by the generic rewrite of signed division by a power of 2.
// Signed divide by power of 2.
// n / c = n >> log(c) if n >= 0
// = (n+c-1) >> log(c) if n < 0
// We conditionally add c-1 by adding n>>63>>(64-log(c))
(first shift signed, second shift unsigned).
(Div64 <t> n (Const64 [c])) && isPowerOfTwo(c) ->
(Rsh64x64
(Add64 <t> n (Rsh64Ux64 <t>
(Rsh64x64 <t> n (Const64 <typ.UInt64> [63]))
(Const64 <typ.UInt64> [64-log2(c)])))
(Const64 <typ.UInt64> [log2(c)]))
If n is known to be positive, this rewrite includes an extra Add and 2
extra Rsh. This CL will allow prove to replace one of the extra Rsh with
a 0. That replacement then allows lateopt to remove all the unneccesary
fixups from the generic rewrite.
There is a rewrite rule to handle this case directly:
(Div64 n (Const64 [c])) && isNonNegative(n) && isPowerOfTwo(c) ->
(Rsh64Ux64 n (Const64 <typ.UInt64> [log2(c)]))
But this implementation of isNonNegative really only handles constants
and a few special operations like len/cap. The division could be
handled if the factsTable version of isNonNegative were available.
Unfortunately, the first opt pass happens before prove even has a
chance to deduce the numerator is non-negative, so the generic rewrite
has already fired and created the extra Ops discussed above.
Fixes#36159
By Printf count, this zeroes 137 right shifts when building std and cmd.
Change-Id: Iab486910ac9d7cfb86ace2835456002732b384a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/232857
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
... and 0-31 for 32-bit shifts.
Generally update the docs for ppc64 shift instructions to be
clearer about what they actually do.
This issue is causing problems for the subsequent CL. The shift
amount was <0 and caused the assembler to report an invalid instruction.
Change-Id: I8c708a15e7f71931835e6e543d8db3c716186e52
Reviewed-on: https://go-review.googlesource.com/c/go/+/232858
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
