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>
Relocate the last few functions in dwarf2.go to dwarf.go,
and remove dwarf2.go. No change in functionality.
Change-Id: Id1b5f0d1c496d5912b955b8ba942e33bc43aa93b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237425
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>
Users don't necessarily know if their OS is ELF-based.
For #37419.
Change-Id: I4a4256c5f5eb34211729e1311582bb0e210f1f8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/238240
Reviewed-by: Than McIntosh <thanm@google.com>
Adjust these two tests for Plan 9,
which uses $path instead of $PATH,
and $home instead of $HOME.
Fixes#39599
Change-Id: Idba95e07d307e76f0f61acd904905b417c52d43a
Reviewed-on: https://go-review.googlesource.com/c/go/+/237941
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This saves a redirect and makes the document more consistent.
For #37419
Change-Id: Ic3bd62f8caacf67ffe43a359624e11bed8b8cfaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/237540
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
When a signal is received, the runtime probes whether an
alternate signal stack is set, if so, adjust gsignal's stack to
point to the alternate signal stack. This is done in
adjustSignalStack, which calls sigaltstack "syscall", which is a
libc call on darwin through asmcgocall. asmcgocall decides
whether to do stack switch based on whether we're running on g0
stack, gsignal stack, or regular g stack. If g is not set to
gsignal, asmcgocall may make wrong decision. Set g first.
adjustSignalStack is recursively nosplit, so it is okay that
temporarily gsignal.stack doesn't match the stack we're running
on.
May fix#39079.
Change-Id: I59b2c5dc08c3c951f1098fff038bf2e06d7ca055
Reviewed-on: https://go-review.googlesource.com/c/go/+/238020
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the binutils Git repository instead of CVS.
Change-Id: I10100ca44d64ab3621367d1d4ac9e9a50d212d0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/237839
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The GCC code repository is now hosted on Git. Adjust the instructions in
gccgo_install.html accordingly.
Change-Id: I443a8b645b63e63785979bc0554521e3dc3b0bf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/237798
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make sure that if a field comparison might panic, we evaluate
(and short circuit if not equal) all previous fields, and don't
evaluate any subsequent fields.
Add a bunch more tests to the equality+panic checker.
Update #8606
Change-Id: I6a159bbc8da5b2b7ee835c0cd1fc565575b58c46
Reviewed-on: https://go-review.googlesource.com/c/go/+/237919
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts golang.org/cl/190659 and golang.org/cl/226218, minus the
regression tests in the latter.
The original work happened in golang.org/cl/151157, which was reverted
in golang.org/cl/190909 due to a crash found by fuzzing.
We tried a second time in golang.org/cl/190659, which shipped with Go
1.14. A bug was found, where strings would be mangled in certain edge
cases. The fix for that was golang.org/cl/226218, which was backported
into Go 1.14.4.
Unfortunately, a second regression was just reported in #39555, which is
a similar case of strings getting mangled when decoding under certain
conditions. It would be possible to come up with another small patch to
fix that edge case, but instead, let's just revert the entire
optimization, as it has proved to do more harm than good. Moreover, it's
hard to argue or prove that there will be no more such regressions.
However, all the work wasn't for nothing. First, we learned that the way
the decoder unquotes tokenized strings isn't simple; initially, we had
wrongly assumed that each string was unquoted exactly once and in order.
Second, we have gained a number of regression tests which will be useful
to prevent the same mistakes in the future, including the test cases we
add in this CL.
Fixes#39555.
Change-Id: I66a6919c2dd6d9789232482ba6cf3814eaa70f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/237838
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TestNetpollBreak was sometimes timing out on Plan 9, where
netpoll_stub.go implements only enough of the network poller
to support runtime timers, using a notetsleep / notewakeup
pair. The runtime.lock which serialises the use of the note
doesn't guarantee fairness, and in practice the netpoll call
used by the test can be starved by the netpoll call from the
scheduler which supports the overall 'go test' timeout.
Calling osyield after relinquishing the lock gives the two
callers a more even chance to take a turn, which prevents
the test from timing out.
Fixes#39437
Change-Id: Ifbe6aaf95336d162d9d0b6deba19b8debf17b071
Reviewed-on: https://go-review.googlesource.com/c/go/+/237698
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The benchmark throughput numbers don't change, but the set-up time (the
time taken before the b.ResetTimer() call) drops from 460ms to 4ms.
Change-Id: I5a6756643dff6127f6d902455d83459c084834fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/237757
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This alert is triggering occasionally. I've investigated the
collisions that happen, and they all seem to be pairwise, so they are
not a big deal. "pairwise" = when there are 32 collisions, it is two
keys mapping to the same hash, 32 times, not 33 keys all mapping to
the same hash.
Add some t.Logf calls in case this comes back, which will help isolate
the problem.
Fixes#39352
Change-Id: I1749d7c8efd0afcf9024d8964d15bc0f58a86e4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/237718
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Support for linux/386 was added to Delve in version 1.4.1, but the
version of Delve currently installed on the linux-386-longtest
builder is 1.2.0. That isn't new enough, which causes the test
to fail. Skip it on that builder until it can be made to work.
The only reason it used to pass on the linux-386-longtest builder
before is because that builder was misconfigured to run tests for
linux/amd64. This was resolved in CL 234520.
Also improve internal documentation and the text of skip reasons.
Fixes#39309.
Change-Id: I395cb1f076e59dd3a3feb53e1dcdce5101e9a0f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/237603
Reviewed-by: David Chase <drchase@google.com>
Currently, a method of a reachable type is live if it matches a
method of a reachable interface. In fact, we only need to retain
the method if the type is actually converted to an interface. If
the type is never converted to an interface, there is no way to
call the method through an interface method call (but the type
descriptor could still be used, e.g. in calling
runtime.newobject).
A type can be used in an interface in two ways:
- directly converted to interface. (Any interface counts, as it
is possible to convert one interface to another.)
- obtained by reflection from a related type (e.g. obtaining an
interface of T from []T).
For the former, we let the compiler emit a marker on the type
descriptor symbol when it is converted to an interface. In the
linker, we only need to check methods of marked types.
For the latter, when the linker visits a marked type, it needs to
visit all its "child" types as marked (i.e. potentially could be
converted to interface).
This reduces binary size:
cmd/compile 18792016 18706096 (-0.5%)
cmd/go 14120572 13398948 (-5.1%)
Change-Id: I4465c7eeabf575f4dc84017214c610fa05ae31fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/237298
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>
Other command line arguments are written in code tags, so add a code tag for consistency.
For #37419
Change-Id: I1948536c3a1860d93726484be2dc7bcb03dfdc2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/237539
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Replace tab indentation with spaces for consistency, as all other indentation is done with spaces.
For #37419
Change-Id: I728a75ae0d00e637f57eb455b6039ffc1a5feed2
Reviewed-on: https://go-review.googlesource.com/c/go/+/237538
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
We replaced http.DefaultClient with securityPreservingHTTPClient,
but we still need that too many redirects check. This issue introduced
by CL 156838.
We introduce a special path to test rediret requests in the script test
framework. You can specify the number of redirects in the path.
$GOPROXY/redirect/<count>/...
Redirect request sequence details(count=8):
request: $GOPROXY/mod/redirect/8/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/7/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/6/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/5/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/4/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/3/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/2/rsc.io/quote/@v/v1.2.0.mod
redirect: $GOPROXY/mod/redirect/1/rsc.io/quote/@v/v1.2.0.mod
the last: $GOPROXY/mod/rsc.io/quote/@v/v1.2.0.mod
Fixes#39482
Change-Id: I149a3702b2b616069baeef787b2e4b73afc93b0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/237177
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This API and functionality was added late in the Go 1.15 release
cycle, and use within gopls has revealed some shortcomings. It's
possible (but not decided) that we'll want a different API long-term,
so for now this CL renames UsesCgo to a non-exported name to avoid
long-term commitment under the Go 1 compat guarantee.
Updates #16623.
Updates #39072.
Change-Id: I04bc0c161a84adebe43e926df5df406bc794c3db
Reviewed-on: https://go-review.googlesource.com/c/go/+/237417
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The scheduler assumes two special invariants that apply to tuple
selectors (Select0 and Select1 ops):
1. There is only one tuple selector of each type per generator.
2. Tuple selectors and generators reside in the same block.
Prior to this CL the assumption was that these invariants would
only be broken by the CSE pass. The CSE pass therefore contained
code to move and de-duplicate selectors to fix these invariants.
However it is also possible to write relatively basic optimization
rules that cause these invariants to be broken. For example:
(A (Select0 (B))) -> (Select1 (B))
This rule could result in the newly added selector (Select1) being
in a different block to the tuple generator (see issue #38356). It
could also result in duplicate selectors if this rule matches
multiple times for the same tuple generator (see issue #39472).
The CSE pass will 'fix' these invariants. However it will only do
so when optimizations are enabled (since disabling optimizations
disables the CSE pass).
This CL moves the CSE tuple selector fixup code into its own pass
and makes it mandatory even when optimizations are disabled. This
allows tuple selectors to be treated like normal ops for most of
the compilation pipeline until after the new pass has run, at which
point we need to be careful to maintain the invariant again.
Fixes#39472.
Change-Id: Ia3f79e09d9c65ac95f897ce37e967ee1258a080b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237118
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This removes the same logic from run.bat that was removed from
cmd/dist in CL 236819.
The duplicated logic was removed from run.bash and run.rc in CL 6531,
but that part of run.bat was apparently missed (and not noticed
because its effect was redundant).
Also fix a path-separator bug in cmd/addr2line.TestAddr2Line that was
exposed as a result.
Fixes#39478
Updates #39385
Change-Id: I00054966cf92ef92a03681bf23de7f45f46fbb5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/237359
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Restore previously sent SCTs and stapled OCSP response during session
resumption for both TLS 1.2 and 1.3. This behavior is somewhat
complicated for TLS 1.2 as SCTs are sent during the server hello,
so they override what is saved in ClientSessionState. It is likely
that if the server is sending a different set of SCTs there is probably
a reason for doing so, such as a log being retired, or SCT validation
requirements changing, so it makes sense to defer to the server in
that case.
Fixes#39075
Change-Id: I3c0fa2f69c6bf0247a447c48a1b4c733a882a233
Reviewed-on: https://go-review.googlesource.com/c/go/+/234237
Reviewed-by: Filippo Valsorda <filippo@golang.org>
When the arrangement specifier is "B16", the 30-bit should be 1 rather than 0.
This CL fixes this error.
Fixes#39445
Change-Id: Ib44881cdb8b3aab855cb30f2c52a085cd73a6a2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/236638
Run-TryBot: eric fang <eric.fang@arm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
