CL 188317 introduced a compiler crash during dwarf generation which
was reported as Issue #34520. After CL 188217, the issue appears to be
fixed. Add a testcase to avoid future regressions.
Fixes#34520
Change-Id: I73544a9e9baf8dbfb85c19eb6d202beea05affb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/198546
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Passing ambiguous patterns, ending in `.go`, to `go list` results in them
being interpreted as Go files despite potentially being package references.
This can then result in errors on other package references.
The parsing logic is modified to check for a locally present file
corresponding to any pattern ending in `.go`. If no such file is present
the pattern is considered to be a package reference.
We're also adding a variety of non-regression tests that fail with the
original parsing code but passes after applying the fix.
Fixes#32483Fixes#34653
Change-Id: I073871da0dfc5641a359643f95ac14608fdca09b
GitHub-Last-Rev: 5abc200103
GitHub-Pull-Request: golang/go#34663
Reviewed-on: https://go-review.googlesource.com/c/go/+/198459
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The existing text was hard to parse.
Shorten the sentences and simplify the text.
Change-Id: Ic16f486925090ea303c04e70969e5a4b27a60896
Reviewed-on: https://go-review.googlesource.com/c/go/+/198758
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add support of parsing new object file format. We use the new
parser if the magic string matches the new one, otherwise use the
old one.
The parsed data are still filled into the current goobj API. In
the future we may consider to change the goobj API to a close
match of the object file data.
Now objdump and nm commands support new object file format.
For a reference to a symbol defined in another package, with the
new object file format we don't know its name. Write it as
pkg.<#nn> for now, where nn is its symbol index.
Change-Id: I06d05b2ca834ba36980da3c5d76aee16c3b0a483
Reviewed-on: https://go-review.googlesource.com/c/go/+/196031
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Nilcheck would move statements from NilCheck values to others that
turned out were already dead, which leads to lost statements. Better
to eliminate the dead code first.
One "error" is removed from test/prove.go because the code is
actually dead, and the additional deadcode pass removes it before
prove can run.
Change-Id: If75926ca1acbb59c7ab9c8ef14d60a02a0a94f8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/198479
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
The earlier code was picking nodes that were "poor choices" and
thus sometimes losing statements altogether.
Change-Id: Ibe5ed800ffbd3c926c0ab1bc10c77d72d3042e45
Reviewed-on: https://go-review.googlesource.com/c/go/+/198478
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Aggregate-making nodes that are later decomposed
are poor choices for statements, because the decomposition
phase turns them into multiple sub-values, some of which may be
dead. Better to look elsewhere for a statement mark.
Change-Id: Ibd9584138ab3d1384548686896a28580a2e43f54
Reviewed-on: https://go-review.googlesource.com/c/go/+/198477
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The previous version used the position of the switch statement,
which makes for potentially jumpy stepping and introduces a large
number of statements repeating the line (tricky for inserting
breaks). It also shared a single OBREAK node and this was not
really a syntax "tree".
This improves both the nostmt test (by 6 lines) and
reduces the total badness score from dwarf-goodness (by about 200).
Change-Id: I1f71b231a26f152bdb6ce9bc8f95828bb222f665
Reviewed-on: https://go-review.googlesource.com/c/go/+/188218
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
1) An empty block is treated as not-a-statement unless its line differs
from at least one of its predecessors (it might make sense to
rearrange branches in predecessors, but that is a different issue).
2) When iterating forward to choose a "good" place for a statement,
actually check that the chosen place is in fact good.
3) Refactor same line and same file into methods on XPos and Pos.
This reduces the failure rate of ssa/stmtlines_test by 7-ish lines.
(And interacts favorably with later debugging CLs.)
Change-Id: Idb7cca7068f6fc9fbfdbe25bc0da15bcfc7b9d4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/188217
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Makes it easier to run
go build -a -gcflags=-d=export std |& grep ^BenchmarkExportSize
and get useful output for feeding into benchstat.
Change-Id: I2b52e8f5ff33b7ccb6c25b18e464513344bd9ad9
Reviewed-on: https://go-review.googlesource.com/c/go/+/198698
Reviewed-by: Robert Griesemer <gri@golang.org>
If -newobj is set, write object file in new format, which uses
indices for symbol references instead of symbol names. The file
format is described at the beginning of
cmd/internal/goobj2/objfile.go.
A new package, cmd/internal/goobj2, is introduced for reading and
writing new object files. (The package name is temporary.) It is
written in a way that trys to make the encoding as regular as
possible, and the reader and writer as symmetric as possible.
This is incomplete, and currently nothing will consume the new
object file.
Change-Id: Ifefedbf6456d760d15a9f40a28af6486c93100fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/196030
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
During package initialization, the compiler tries to optimize:
var A = "foo"
var B = A
into
var A = "foo"
var B = "foo"
so that we can statically initialize both A and B and skip emitting
dynamic initialization code to assign "B = A".
However, this isn't safe in the presence of cmd/link's -X flag, which
might overwrite an initialized string-typed variable at link time. In
particular, if cmd/link changes A's static initialization, it won't
know it also needs to change B's static initialization.
To address this, this CL disables this optimization for string-typed
variables.
Fixes#34675.
Change-Id: I1c18f3b855f6d7114aeb39f96aaaf1b452b88236
Reviewed-on: https://go-review.googlesource.com/c/go/+/198657
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's pointless to reach all ms via allgs, and doing so introduces a
race, since the m member of a g can change underneath it. Instead
iterate directly through the allm linked list.
Updates: #31528
Updates: #34130
Change-Id: I34b88402b44339b0a5b4cd76eafd0ce6e43e2be1
Reviewed-on: https://go-review.googlesource.com/c/go/+/198417
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL is a follow up for 198080.
Added a private writeTo method to the Node interface,
in order to use the same builder for printing all nodes
in the tree. Benchmark output against master:
benchmark old ns/op new ns/op delta
BenchmarkParseLarge-8 24594994 25292054 +2.83%
BenchmarkVariableString-8 117 118 +0.85%
BenchmarkListString-8 10475 3353 -67.99%
benchmark old allocs new allocs delta
BenchmarkVariableString-8 3 3 +0.00%
BenchmarkListString-8 149 31 -79.19%
benchmark old bytes new bytes delta
BenchmarkVariableString-8 72 72 +0.00%
BenchmarkListString-8 5698 1608 -71.78%
Change-Id: I2b1cf07cda65c1b80083fb99671289423700feba
Reviewed-on: https://go-review.googlesource.com/c/go/+/198278
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If a request for a PTR record returned a response with a non-PTR
answer, goLookupPTR would loop forever. Skipping non-PTR answers
guarantees progress through the DNS response.
Fixes#34660
Change-Id: I56f9d21e5342d07e7d843d253267e93a29707904
Reviewed-on: https://go-review.googlesource.com/c/go/+/198460
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently cgoCheckPointer is only used with one optional argument.
Using a slice for the optional arguments is quite expensive, hence
replace it with a single interface{}. This results in ~30% improvement.
When checking struct fields, they quite often end up being without
pointers. Check this before calling cgoCheckPointer, which results in
additional ~20% improvement.
Inline some p == nil checks from cgoIsGoPointer which gives
additional ~15% improvement.
All of this translates to:
name old time/op new time/op delta
CgoCall/add-int-32 46.9ns ± 1% 46.6ns ± 1% -0.75% (p=0.000 n=18+20)
CgoCall/one-pointer-32 143ns ± 1% 87ns ± 1% -38.96% (p=0.000 n=20+20)
CgoCall/eight-pointers-32 767ns ± 0% 327ns ± 1% -57.30% (p=0.000 n=18+16)
CgoCall/eight-pointers-nil-32 110ns ± 1% 89ns ± 2% -19.10% (p=0.000 n=19+19)
CgoCall/eight-pointers-array-32 5.09µs ± 1% 3.56µs ± 2% -30.09% (p=0.000 n=19+19)
CgoCall/eight-pointers-slice-32 3.92µs ± 0% 2.57µs ± 2% -34.48% (p=0.000 n=20+20)
Change-Id: I2aa9f5ae8962a9a41a7fb1db0c300893109d0d75
Reviewed-on: https://go-review.googlesource.com/c/go/+/198081
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Because errors like:
certificate has expired or is not yet valid
make it difficult to distinguish between "certificate has expired" and
"my local clock is skewed". Including our idea of the local time
makes it easier to identify the clock-skew case, and including the
violated certificate constraint saves folks the trouble of looking it
up in the target certificate.
Change-Id: I52e0e71705ee36f6afde1bb5a47b9b42ed5ead5b
GitHub-Last-Rev: db2ca4029c
GitHub-Pull-Request: golang/go#34646
Reviewed-on: https://go-review.googlesource.com/c/go/+/198046
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We are planning to use indices for symbol references, instead of
symbol names. Here we assign indices to symbols defined in the
package being compiled, and propagate the indices to the
dependent packages in the export data.
A symbol is referenced by a tuple, (package index, symbol index).
Normally, for a given symbol, this index is unique, and the
symbol index is globally consistent (but with exceptions, see
below). The package index is local to a compilation. For example,
when compiling the fmt package, fmt.Println gets assigned index
25, then all packages that reference fmt.Println will refer it
as (X, 25) with some X. X is the index for the fmt package, which
may differ in different compilations.
There are some symbols that do not have clear package affiliation,
such as dupOK symbols and linknamed symbols. We cannot give them
globally consistent indices. We categorize them as non-package
symbols, assign them with package index 1 and a symbol index that
is only meaningful locally.
Currently nothing will consume the indices.
All this is behind a flag, -newobj. The flag needs to be set for
all builds (-gcflags=all=-newobj -asmflags=all=-newobj), or none.
Change-Id: I18e489c531e9a9fbc668519af92c6116b7308cab
Reviewed-on: https://go-review.googlesource.com/c/go/+/196029
Reviewed-by: Than McIntosh <thanm@google.com>
Add a test that causes generation of arch.ZeroRange calls of various sizes 8-136
bytes in the compiler. This is to test that ZeroRanges of various sizes actually
compile on different architectures, but is not testing runtime correctness (which
is hard to do).
Updates #34604
Change-Id: I4131eb86669bdfe8d4e36f4ae5c2a7b069abd6c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/198045
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 196846 implemented memory mapped output files but forgot to provide
an implementation for Msync. This rectifies that with a simple call to
FlushViewOfFile.
Change-Id: I5aebef9baf3a2a6ad54ceda096952a5d7d660bfe
Reviewed-on: https://go-review.googlesource.com/c/go/+/198418
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TestEnvOverride sets PATH to /wibble before executing a CGI.
So customized Perl that is starting with '#!/usr/bin/env bash' will fail
because /usr/bin/env can't lookup bash.
Fixes#27790
Change-Id: I25e433061a7ff9da8c86429e934418fc15f12f90
Reviewed-on: https://go-review.googlesource.com/c/go/+/196845
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
By lazily starting the signal watch loop only on Notify,
we are able to have deadlock detection even when
"os/signal" is imported.
Thanks to Ian Lance Taylor for the solution and discussion.
With this change in, fix a runtime gorountine count test that
assumed that os/signal.init would unconditionally start the
signal watching goroutine, but alas no more.
Fixes#21576.
Change-Id: I6eecf82a887f59f2ec8897f1bcd67ca311ca42ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/101036
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In #33848, we propose to use 'go 1.14' in the go.mod file to enable
new default behavior. That means that 'go mod init' needs to start
generating that directive by default, which requires the presence of
the updated version tag in the build environment.
Updates #33848
Change-Id: I9f3b8845fdfd843fd76de32f4b55d8f765d691de
Reviewed-on: https://go-review.googlesource.com/c/go/+/198318
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
nextNonSpace has an identical code except the call to
backup at the end.
Change-Id: Iefa5b13950007da38323a800fb6b0ce3d436254b
Reviewed-on: https://go-review.googlesource.com/c/go/+/198277
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It is convenient to have a seekable writer. A later CL will make
use of Seek.
Change-Id: Iba0107ce2975d9a451d97f16aa91a318dd4c90e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/196028
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Currently, at the end of compilation, the compiler writes out the
export data, the linker object file header, then does more
code/data generation, then writes the main content of the linker
object file. This CL refactors it to finish all the code/data
generation before writing any output file.
A later CL will inject some code that operates on all defined
symbols before writing the output. This ensures all the symbols
are available at that point.
Change-Id: I97d946553fd0ffd298234c520219540d29783576
Reviewed-on: https://go-review.googlesource.com/c/go/+/196027
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ADUFFCOPY and ADUFFZERO instructions weren't handled by rewriteToUseTOC.
These instructions are considered as a simple branch except with -dynlink
where they become an indirect call.
Fixes#34604
Change-Id: I16ca6a152164966fb9cbf792219a8a39aad2b53b
Reviewed-on: https://go-review.googlesource.com/c/go/+/197842
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As mentioned in godoc, strings.Builder is more efficient for
concatenating and building strings.
Running a simple bench test on VariableNode.String() gives:
benchmark old ns/op new ns/op delta
BenchmarkParseLarge-8 25676831 24453285 -4.77%
BenchmarkVariableString-8 296 115 -61.15%
benchmark old allocs new allocs delta
BenchmarkVariableString-8 8 3 -62.50%
benchmark old bytes new bytes delta
BenchmarkVariableString-8 112 72 -35.71%
Change-Id: I13c9340080738fcad1edeed859d33ba608e4b05a
Reviewed-on: https://go-review.googlesource.com/c/go/+/198078
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While understanding why syscall.Read is 2x slower on darwin/amd64, I found
out that, contrary to popular belief, the slowdown is not due to the migration
to use libSystem.dylib instead of direct SYSCALLs, i.e., CL 141639 (and #17490),
but due to a subtle change introduced in CL 141639.
Previously, syscall.Read used syscall.Syscall(SYS_READ), whose preamble called
runtime.entersyscall, but after CL 141639, syscall.Read changes to call
runtime.syscall_syscall instead, which in turn calls runtime.entersyscallblock
instead of runtime.entersyscall. And the entire 2x slow down can be attributed
to this change.
I think this is unnecessary as even though syscalls like Read might block, it
does not always block, so there is no need to handoff P proactively for each
Read. Additionally, we have been fine with not handing off P for each Read
prior to Go 1.12, so we probably don't need to change it. This changes restores
the pre-Go 1.12 behavior, where syscall preamble uses runtime.entersyscall,
and we rely on sysmon to take P back from g blocked in syscalls.
Change-Id: If76e97b5a7040cf1c10380a567c4f5baec3121ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/197938
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The new yaml.v2 release broke the longtest builder. Update the expected
data.
Updates #28856
Change-Id: I98ec9e32e55bdb6b26b67e46dc16f34f77c2d40f
Reviewed-on: https://go-review.googlesource.com/c/go/+/198117
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Fix linking with a package having a .syso file in external link mode,
that would otherwise cause an error before executing the external
linker because it can't find symbols that are exported in the said
.syso file.
Fixes#33139
Change-Id: Id3ee737fba1c6f1e37910593dfedf9c84486d398
Reviewed-on: https://go-review.googlesource.com/c/go/+/186417
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Followup to https://golang.org/cl/197997
If you know the number of elements, you don't need append at all.
Either use append to grow, or allocate and index. Here we choose
number 2.
Change-Id: Ic58637231789640ff7b293ece04a95a8de7ccf8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/198097
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When comparing two T-typed values t1 and t2 using the T_eq function,
we used to generate:
pl := &t1
pr := &t2
return T_eq(pl, pr, unsafe.Sizeof(T{}))
This CL changes it to simply generate:
return T_eq(&t1, &t2, unsafe.Sizeof(T{}))
Surprisingly, this does not pass toolstash. For some reason, it seems
like SSA wasn't able to SSA-ify the pl and pr variables in all cases.
Change-Id: I111fbb068a1741fa169c9922cb8cdb6e21579aa4
Reviewed-on: https://go-review.googlesource.com/c/go/+/197601
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
walkcompare already called walkexpr on n.Left and n.Right, which in
turn calls anylit when appropriate.
Passes toolstash-check.
Change-Id: I6912ac5a42b977c04db9d85cb2e7295e275e083d
Reviewed-on: https://go-review.googlesource.com/c/go/+/197600
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
