CL 201783 enable -d=checkptr when -race or -msan is specified
everywhere but windows.
But, now that all unsafe pointer conversions in the standard
library are fixed, enable -d=checkptr even on windows.
Updates #34964
Updates #34972
Change-Id: Id912fa83b0d5b46c6f1c134c742fd94d2d185835
Reviewed-on: https://go-review.googlesource.com/c/go/+/227003
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We still disallow inlining for an immediately-recursive function, but allow
inlining if a function is in a recursion chain.
If all functions in the recursion chain are simple, then we could inline
forever down the recursion chain (eventually running out of stack on the
compiler), so we add a map to keep track of the functions we have
already inlined at a call site. We stop inlining when we reach a
function that we have already inlined in the recursive chain. Of course,
normally the inlining will have stopped earlier, because of the cost
function.
We could also limit the depth of inlining by a simple count (say, limit
max inlining of 10 at any given site). Would that limit other
opportunities too much?
Added a test in test/inline.go. runtime.BenchmarkStackCopyNoCache() is
also already a good test that triggers the check to stop inlining
when we reach the start of the recursive chain again.
For the bent benchmark suite, the performance improvement was mostly not
statistically significant, but the geomean averaged out to: -0.68%. The text size
increase was less than .1% for all bent benchmarks. The cmd/go text size increase
was 0.02% and the cmd/compile text size increase was .1%.
Fixes#29737
Change-Id: I892fa84bb07a947b3125ec8f25ed0e508bf2bdf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/226818
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The only merge conflict is the addition of -spectre flag on
master and the addition of -go115newobj flag on dev.link.
Resolved trivially.
Change-Id: I5b46c2b25e140d6c3d8cb129acbd7a248ff03bb9
Add back the newobj flag, renamed to go115newobj, for feature
gating. The flag defaults to true.
This essentially reverts CL 206398 as well as CL 220060.
The old object format isn't working yet. Will fix in followup CLs.
Change-Id: I1ace2a9cbb1a322d2266972670d27bda4e24adbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/224623
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This commit extends the -spectre flag to cmd/asm and adds
a new Spectre mitigation mode "ret", which enables the use
of retpolines.
Retpolines prevent speculation about the target of an indirect
jump or call and are described in more detail here:
https://support.google.com/faqs/answer/7625886
Change-Id: I4f2cb982fa94e44d91e49bd98974fd125619c93a
Reviewed-on: https://go-review.googlesource.com/c/go/+/222661
Reviewed-by: Keith Randall <khr@golang.org>
This commit adds a new cmd/compile flag -spectre,
which accepts a comma-separated list of possible
Spectre mitigations to apply, or the empty string (none),
or "all". The only known mitigation right now is "index",
which uses conditional moves to ensure that x86-64 CPUs
do not speculate past index bounds checks.
Speculating past index bounds checks may be problematic
on systems running privileged servers that accept requests
from untrusted users who can execute their own programs
on the same machine. (And some more constraints that
make it even more unlikely in practice.)
The cases this protects against are analogous to the ones
Microsoft explains in the "Array out of bounds load/store feeding ..."
sections here:
https://docs.microsoft.com/en-us/cpp/security/developer-guidance-speculative-execution?view=vs-2019#array-out-of-bounds-load-feeding-an-indirect-branch
Change-Id: Ib7532d7e12466b17e04c4e2075c2a456dc98f610
Reviewed-on: https://go-review.googlesource.com/c/go/+/222660
Reviewed-by: Keith Randall <khr@golang.org>
It has been a while we have not done this.
Merge conflict resolution:
- deleted/rewritten code modified on master
- CL 214286, ported in CL 217317
(cmd/internal/obj/objfile.go)
- CL 210678, it already includes a fix to new code
(cmd/link/internal/ld/deadcode.go)
- CL 209317, applied in this CL
(cmd/link/internal/loadelf/ldelf.go)
Change-Id: Ie927ea6a1d69ce49e8d03e56148cb2725e377876
Change the type printer to take a map of types that we're currently
printing. When we happen upon a type that we're already in the middle
of printing, print a reference to it instead.
A reference to another type is built using the offset of the first
byte of that type's string representation in the result. To facilitate
that computation (and it's probably more efficient, regardless), we
print the type to a buffer as we go, and build the string at the end.
It would be nice to use string.Builder instead of bytes.Buffer, but
string.Builder wasn't around in Go 1.4, and we'd like to bootstrap
from that version.
Fixes#29312
Change-Id: I49d788c1fa20f770df7b2bae3b9979d990d54803
Reviewed-on: https://go-review.googlesource.com/c/go/+/214239
Reviewed-by: Robert Griesemer <gri@golang.org>
Bring in Than's fix of #35779.
The only merge conflict is cmd/link/internal/loadelf/ldelf.go,
with a modification-deletion conflict.
Change-Id: Id2fcfd2094a31120966a6ea9c462b4ec76646b10
There are more cleanups to do, but I want to keep this CL mostly
a pure deletion.
Change-Id: Icd2ff0a4b648eb4adf3d29386542617e49620818
Reviewed-on: https://go-review.googlesource.com/c/go/+/206398
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is intended to allow IDEs to note where the optimizer
was not able to improve users' code. There may be other
applications for this, for example in studying effectiveness
of optimizer changes more quickly than running benchmarks,
or in verifying that code changes did not accidentally disable
optimizations in performance-critical code.
Logging of nilcheck (bad) for amd64 is implemented as
proof-of-concept. In general, the intent is that optimizations
that didn't happen are what will be logged, because that is
believed to be what IDE users want.
Added flag -json=version,dest
Check that version=0. (Future compilers will support a
few recent versions, I hope that version is always <=3.)
Dest is expected to be one of:
/path (or \path in Windows)
will create directory /path and fill it w/ json files
file://path
will create directory path, intended either for
I:\dont\know\enough\about\windows\paths
trustme_I_know_what_I_am_doing_probably_testing
Not passing an absolute path name usually leads to
json splattered all over source directories,
or failure when those directories are not writeable.
If you want a foot-gun, you have to ask for it.
The JSON output is directed to subdirectories of dest,
where each subdirectory is net/url.PathEscape of the
package name, and each for each foo.go in the package,
net/url.PathEscape(foo).json is created. The first line
of foo.json contains version and context information,
and subsequent lines contains LSP-conforming JSON
describing the missing optimizations.
Change-Id: Ib83176a53a8c177ee9081aefc5ae05604ccad8a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/204338
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Previously langSupported applied -lang as though it's a global
restriction, but it's actually a per-package restriction. This CL
fixes langSupported to take a *types.Pkg parameter to reflect this and
updates its callers accordingly.
This is relevant for signed shifts (added in Go 1.12), because they
can be inlined into a Go 1.11 package; and for overlapping interfaces
(added in Go 1.13), because they can be exported as part of the
package's API.
Today we require all Go packages to be compiled with the same
toolchain, and all uses of langSupported are for controlling
backwards-compatible features. So we can simply assume that since the
imported packages type-checked successfully, they must have been
compiled with an appropriate -lang setting.
In the future if we ever want to use langSupported to control
backwards-incompatible language changes, we might need to record the
-lang flag used for compiling a package in its export data.
Fixes#35437.
Fixes#35442.
Change-Id: Ifdf6a62ee80cd5fb4366cbf12933152506d1b36e
Reviewed-on: https://go-review.googlesource.com/c/go/+/205977
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
In the dev.link branch we implemented the new object file format
and (part of) the linker improvements described in
https://golang.org/s/better-linker
The new object file is index-based and provides random access.
The linker maps the object files into read-only memory, and
access symbols on-demand using indices, as opposed to reading
all object files sequentially into the heap with the old format.
The linker carries symbol informations using indices (as opposed
to Symbol data structure). Symbols are created after the
reachability analysis, and only created for reachable symbols.
This reduces the linker's memory usage.
Linking cmd/compile, it creates ~25% fewer Symbols, and reduces
memory usage (inuse_space) by ~15%. (More results from Than.)
Currently, both the old and new object file formats are supported.
The old format is used by default. The new format can be turned
on by using the compiler/assembler/linker's -newobj flag. Note
that the flag needs to be specified consistently to all
compilations, i.e.
go build -gcflags=all=-newobj -asmflags=all=-newobj -ldflags=-newobj
Change-Id: Ia0e35306b5b9b5b19fdc7fa7c602d4ce36fa6abd
Flip back to the old object files for Go 1.14.
Change-Id: I4ad499460fb7156b63fc63e9c6ea4f7099e20af2
Reviewed-on: https://go-review.googlesource.com/c/go/+/204098
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL adds experimental coverage instrumentation similar to what
github.com/dvyukov/go-fuzz produces in its -libfuzzer mode. The
coverage can be enabled by compiling with -d=libfuzzer. It's intended
to be used in conjunction with -buildmode=c-archive to produce an ELF
archive (.a) file that can be linked with libFuzzer. See #14565 for
example usage.
The coverage generates a unique 8-bit counter for each basic block in
the original source code, and emits an increment operation. These
counters are then collected into the __libfuzzer_extra_counters ELF
section for use by libFuzzer.
Updates #14565.
Change-Id: I239758cc0ceb9ca1220f2d9d3d23b9e761db9bf1
Reviewed-on: https://go-review.googlesource.com/c/go/+/202117
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Generate inline code at defer time to save the args of defer calls to unique
(autotmp) stack slots, and generate inline code at exit time to check which defer
calls were made and make the associated function/method/interface calls. We
remember that a particular defer statement was reached by storing in the deferBits
variable (always stored on the stack). At exit time, we check the bits of the
deferBits variable to determine which defer function calls to make (in reverse
order). These low-cost defers are only used for functions where no defers
appear in loops. In addition, we don't do these low-cost defers if there are too
many defer statements or too many exits in a function (to limit code increase).
When a function uses open-coded defers, we produce extra
FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and
for each defer, the stack slots where the closure and associated args have been
stored. The funcdata also includes the location of the deferBits variable.
Therefore, for panics, we can use this funcdata to determine exactly which defers
are active, and call the appropriate functions/methods/closures with the correct
arguments for each active defer.
In order to unwind the stack correctly after a recover(), we need to add an extra
code segment to functions with open-coded defers that simply calls deferreturn()
and returns. This segment is not reachable by the normal function, but is returned
to by the runtime during recovery. We set the liveness information of this
deferreturn() to be the same as the liveness at the first function call during the
last defer exit code (so all return values and all stack slots needed by the defer
calls will be live).
I needed to increase the stackguard constant from 880 to 896, because of a small
amount of new code in deferreturn().
The -N flag disables open-coded defers. '-d defer' prints out the kind of defer
being used at each defer statement (heap-allocated, stack-allocated, or
open-coded).
Cost of defer statement [ go test -run NONE -bench BenchmarkDefer$ runtime ]
With normal (stack-allocated) defers only: 35.4 ns/op
With open-coded defers: 5.6 ns/op
Cost of function call alone (remove defer keyword): 4.4 ns/op
Text size increase (including funcdata) for go binary without/with open-coded defers: 0.09%
The average size increase (including funcdata) for only the functions that use
open-coded defers is 1.1%.
The cost of a panic followed by a recover got noticeably slower, since panic
processing now requires a scan of the stack for open-coded defer frames. This scan
is required, even if no frames are using open-coded defers:
Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ]
Without open-coded defers: 62.0 ns/op
With open-coded defers: 255 ns/op
A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers:
CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ]
Without open-coded defers: 443 ns/op
With open-coded defers: 347 ns/op
Updates #14939 (defer performance)
Updates #34481 (design doc)
Change-Id: I63b1a60d1ebf28126f55ee9fd7ecffe9cb23d1ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/202340
Reviewed-by: Austin Clements <austin@google.com>
It can still be manually disabled again using -d=checkptr=0.
It's also still disabled by default for GOOS=windows, because the
Windows standard library code has a lot of unsafe pointer conversions
that need updating.
Updates #34964.
Change-Id: Ie0b8b4fdf9761565e0dcb00d69997ad896ac233d
Reviewed-on: https://go-review.googlesource.com/c/go/+/201783
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When building a program that links against Go shared libraries,
it needs to reference symbols defined in the shared library. At
compile time, we don't know where the shared library boundary is.
If we reference a symbol in package p by index, and package p is
actually part of a shared library, we cannot resolve the index at
link time, as the linker doesn't see the object file of p.
So when linking against Go shared libraries, always use named
reference for now.
To do this, the compiler needs to know whether we will be linking
against Go shared libraries. The -dynlink flag kind of indicates
that (as the document says), but currently it is actually
overloaded: it is also used when building a plugin or a shared
library, which is self-contained (if -linkshared is not otherwise
specified) and could use index for symbol reference. So we
introduce another compiler flag, -linkshared, specifically for
linking against Go shared libraries. The go command will pass
this flag if its -linkshared flag is specified
("go build -linkshared").
There may be better way to handle this. For example, we can
put the symbol indices in a special section in the shared library
that the linker can read. Or we can generate some per-package
description file to include the indices. (Currently we generate
a .shlibname file for each package that is included in a shared
library, which contains the path of the library. We could
consider extending this.) That said, this CL is a stop-gap
solution. And it is no worse than the old object files.
If we were to redesign the build system so that the shared
library boundary is known at compile time, we could use indices
for symbol references that do not cross shared library boundary,
as well as doing other things better.
Change-Id: I9c02aad36518051cc4785dbe25c4b4cef8f3faeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/201818
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Escaping all unsafe.Pointer conversions for -d=checkptr seems like it
might be a little too aggressive to enable for -race/-msan mode, since
at least some tests are written to expect unsafe.Pointer conversions
to not affect escape analysis.
So instead only enable that functionality behind -d=checkptr=2.
Updates #22218.
Updates #34959.
Change-Id: I2f0a774ea5961dabec29bc5b8ebe387a1b90d27b
Reviewed-on: https://go-review.googlesource.com/c/go/+/201840
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL adds -d=checkptr as a compile-time option for adding
instrumentation to check that Go code is following unsafe.Pointer
safety rules dynamically. In particular, it currently checks two
things:
1. When converting unsafe.Pointer to *T, make sure the resulting
pointer is aligned appropriately for T.
2. When performing pointer arithmetic, if the result points to a Go
heap object, make sure we can find an unsafe.Pointer-typed operand
that pointed into the same object.
These checks are currently disabled for the runtime, and can also be
disabled through a new //go:nocheckptr annotation. The latter is
necessary for functions like strings.noescape, which intentionally
violate safety rules to workaround escape analysis limitations.
Fixes#22218.
Change-Id: If5a51273881d93048f74bcff10a3275c9c91da6a
Reviewed-on: https://go-review.googlesource.com/c/go/+/162237
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Generate inline code at defer time to save the args of defer calls to unique
(autotmp) stack slots, and generate inline code at exit time to check which defer
calls were made and make the associated function/method/interface calls. We
remember that a particular defer statement was reached by storing in the deferBits
variable (always stored on the stack). At exit time, we check the bits of the
deferBits variable to determine which defer function calls to make (in reverse
order). These low-cost defers are only used for functions where no defers
appear in loops. In addition, we don't do these low-cost defers if there are too
many defer statements or too many exits in a function (to limit code increase).
When a function uses open-coded defers, we produce extra
FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and
for each defer, the stack slots where the closure and associated args have been
stored. The funcdata also includes the location of the deferBits variable.
Therefore, for panics, we can use this funcdata to determine exactly which defers
are active, and call the appropriate functions/methods/closures with the correct
arguments for each active defer.
In order to unwind the stack correctly after a recover(), we need to add an extra
code segment to functions with open-coded defers that simply calls deferreturn()
and returns. This segment is not reachable by the normal function, but is returned
to by the runtime during recovery. We set the liveness information of this
deferreturn() to be the same as the liveness at the first function call during the
last defer exit code (so all return values and all stack slots needed by the defer
calls will be live).
I needed to increase the stackguard constant from 880 to 896, because of a small
amount of new code in deferreturn().
The -N flag disables open-coded defers. '-d defer' prints out the kind of defer
being used at each defer statement (heap-allocated, stack-allocated, or
open-coded).
Cost of defer statement [ go test -run NONE -bench BenchmarkDefer$ runtime ]
With normal (stack-allocated) defers only: 35.4 ns/op
With open-coded defers: 5.6 ns/op
Cost of function call alone (remove defer keyword): 4.4 ns/op
Text size increase (including funcdata) for go cmd without/with open-coded defers: 0.09%
The average size increase (including funcdata) for only the functions that use
open-coded defers is 1.1%.
The cost of a panic followed by a recover got noticeably slower, since panic
processing now requires a scan of the stack for open-coded defer frames. This scan
is required, even if no frames are using open-coded defers:
Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ]
Without open-coded defers: 62.0 ns/op
With open-coded defers: 255 ns/op
A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers:
CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ]
Without open-coded defers: 443 ns/op
With open-coded defers: 347 ns/op
Updates #14939 (defer performance)
Updates #34481 (design doc)
Change-Id: I51a389860b9676cfa1b84722f5fb84d3c4ee9e28
Reviewed-on: https://go-review.googlesource.com/c/go/+/190098
Reviewed-by: Austin Clements <austin@google.com>
Switch the default to new object files.
Internal linking cgo is disabled for now, as it does not work yet
in newobj mode.
Shared libraries are also broken.
Disable some tests that are known broken for now.
Change-Id: I8ca74793423861d607a2aa7b0d89a4f4d4ca7671
Reviewed-on: https://go-review.googlesource.com/c/go/+/200161
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This is part two if the nacl removal. Part 1 was CL 199499.
This CL removes amd64p32 support, which might be useful in the future
if we implement the x32 ABI. It also removes the nacl bits in the
toolchain, and some remaining nacl bits.
Updates #30439
Change-Id: I2475d5bb066d1b474e00e40d95b520e7c2e286e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/200077
Reviewed-by: Ian Lance Taylor <iant@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>
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>
This is better handled by tools like cmd/gofmt, which can
automatically rewrite the source code and already supports a syntactic
version of this simplification. (go/types can be used if
type-sensitive simplification is actually necessary.)
Change-Id: I51332a8f3ff4ab3087bc6b43a491c6d92b717228
Reviewed-on: https://go-review.googlesource.com/c/go/+/197118
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use the following (suboptimal) script to obtain a list of possible
typos:
#!/usr/bin/env sh
set -x
git ls-files |\
grep -e '\.\(c\|cc\|go\)$' |\
xargs -n 1\
awk\
'/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
hunspell -d en_US -l |\
grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
sort -f |\
uniq -c |\
awk '$1 == 1 { print $2; }'
Then, go through the results manually and fix the most obvious typos in
the non-vendored code.
Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
This CL extends {defer,resume}checkwidth to support nesting, which
simplifies usage.
Updates #33658.
Change-Id: Ib3ffb8a7cabfae2cbeba74e21748c228436f4726
Reviewed-on: https://go-review.googlesource.com/c/go/+/192721
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Drops support for old escape analysis pass. Subsequent, separate CL
will remove dead code.
While here, fix a minor error in fmt.go: it was still looking for
esc.go's NodeEscState in n.Opt() rather than escape.go's EscLocation.
But this only affected debug diagnostics printed during escape
analysis itself.
Change-Id: I62512e1b31c75ba0577550a5fd7824abc3159ed5
Reviewed-on: https://go-review.googlesource.com/c/go/+/187597
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
dist passes the -allabis flag to the compiler to avoid having to
recreate the cross-package ABI logic from cmd/go. However, we removed
that logic from cmd/go in CL 179863 and replaced it with a different
mechanism that doesn't depend on the build system. Hence, passing
-allabis in dist is no longer necessary.
This CL removes -allabis from dist and, since that was the only use of
it, removes support for it from the compiler as well.
Updates #31230.
Change-Id: Ib005db95755a7028f49c885785e72c3970aea4f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/181079
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Tool refactoring smallStacks into smallFrames helpfully
"corrected" the capitalization in a string, this undoes
the help.
This is necessary to ensure correct (re)building when the
flag is used to research stack-marking GC latency bugs.
Updates #27732.
Change-Id: Ib7c8d4a36c9e4f9612559be68bd481f9d9cc69f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/180958
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Shrinks the size of things that can be stack allocated from
10M to 128k for declared variables and from 64k to 16k for
implicit allocations (new(T), &T{}, etc).
Usage: "go build -gcflags -smallframes hello.go"
An earlier GOEXPERIMENT version of this caused only one
problem, when a gc-should-detect-oversize-stack test no
longer had an oversized stack to detect. The change was
converted to a flag to make it easier to access (for
diagnosing "long" GC-related single-thread pauses) and to
remove interference with the test.
Includes test to verify behavior.
Updates #27732.
Change-Id: I1255d484331e77185e07c78389a8b594041204c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/180817
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We compile package sort as part of the compiler bootstrap,
to make sure the compiler uses a consistent sort algorithm
no matter what version of Go it is compiled against.
(This matters for elements that compare "equal" but are distinguishable.)
Package sort was compiled in such a way as to disallow
sort.Slice entirely during bootstrap (at least with some compilers),
while cmd/internal/obj was compiled in such a way as to
make obj.SortSlice available to all compilers, precisely because
sort.Slice was not. This is all highly confusing.
Simplify by making sort.Slice available all the time.
Followup to CL 169137 and #30440
(and also CL 40114 and CL 73951).
Change-Id: I127f4e02d6c71392805d256c3a90ef7c51f9ba0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/174525
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was originally
Revert "cmd/link: fix up debug_range for dsymutil (revert CL 72371)"
which has the effect of no longer using Base Address Selection
Entries in DWARF. However, the build-time costs of that are
about 2%, so instead the hacky fixup that generated technically
incorrect DWARF was removed from the linker, and the choice
is instead made in the compiler, dependent on platform, but
also under control of a flag so that we can report this bug
against LLDB/dsymutil/dwarfdump (really, the LLVM dwarf
libraries).
This however does not solve #31188; debugging still fails,
but dwarfdump no longer complains. There are at least two
LLDB bugs involved, and this change will at allow us
to report them without them being rejected because our
now-obsolete workaround for the first bug creates
not-quite-DWARF.
Updates #31188.
Change-Id: I5300c51ad202147bab7333329ebe961623d2b47d
Reviewed-on: https://go-review.googlesource.com/c/go/+/170638
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Found by Josh, who says in the bug that it shrinks cmd/compile by 1.6 MB (6.5%).
Fixes#31563
Change-Id: I35127af539630e628a0a4f2273af519093536c38
Reviewed-on: https://go-review.googlesource.com/c/go/+/172997
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
RELNOTE=yes
The new escape analysis pass is more precise, which for most Go code
should be an improvement. However, it may also break code that
happened to work before (e.g., code that violated the unsafe.Pointer
safety rules).
The old escape analysis pass can be re-enabled with "go build
-gcflags=all=-newescape=false". N.B., it's NOT recommended to mix the
old and new escape analysis passes such as by omitting "all=". While
the old and new escape analysis passes use similar and mostly
compatible metadata, there are cases (e.g., closure handling) where
they semantically differ and could lead to memory corruption errors in
compiled programs.
Fixes#23109.
Change-Id: I0b1b6a6de5e240cb30c87a165f47bb8795491158
Reviewed-on: https://go-review.googlesource.com/c/go/+/170448
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This CL adds a new escape analysis implementation, which can be
enabled through the -newescape compiler flag.
This implementation focuses on simplicity, but in the process ends up
using less memory, speeding up some compile-times, fixing memory
corruption issues, and overall significantly improving escape analysis
results.
Updates #23109.
Change-Id: I6176d9a7ae9d80adb0208d4112b8a1e1f4c9143a
Reviewed-on: https://go-review.googlesource.com/c/go/+/170322
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Add a new custom attribute to compile units containing the package name
of the package (i.e. the name after the 'package' keyword), so that
debuggers can know it when it's different from the last segment
of the package path.
Change-Id: Ieadaab6f47091aabf2f4dc42c8524452eaa6715b
Reviewed-on: https://go-review.googlesource.com/c/go/+/163677
Run-TryBot: Alessandro Arzilli <alessandro.arzilli@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Currently, obj.Ctxt's symbol table does not distinguish between ABI0
and ABIInternal symbols. This is *almost* okay, since a given symbol
name in the final object file is only going to belong to one ABI or
the other, but it requires that the compiler mark a Sym as being a
function symbol before it retrieves its LSym. If it retrieves the LSym
first, that LSym will be created as ABI0, and later marking the Sym as
a function symbol won't change the LSym's ABI.
Marking a Sym as a function symbol before looking up its LSym sounds
easy, except Syms have a dual purpose: they are used just as interned
strings (every function, variable, parameter, etc with the same
textual name shares a Sym), and *also* to store state for whatever
package global has that name. As a result, it's easy to slip up and
look up an LSym when a Sym is serving as the name of a local variable,
and then later mark it as a function when it's serving as the global
with the name.
In general, we were careful to avoid this, but #29610 demonstrates one
case where we messed up. Because of on-demand importing from indexed
export data, it's possible to compile a method wrapper for a type
imported from another package before importing an init function from
that package. If the argument of the method is named "init", the
"init" LSym will be created as a data symbol when compiling the
wrapper, before it gets marked as a function symbol.
To fix this, we separate obj.Ctxt's symbol tables for ABI0 and
ABIInternal symbols. This way, the compiler will simply get a
different LSym once the Sym takes on its package-global meaning as a
function.
This fixes the above ordering issue, and means we no longer need to go
out of our way to create the "init" function early and mark it as a
function symbol.
Fixes#29610.
Updates #27539.
Change-Id: Id9458b40017893d46ef9e4a3f9b47fc49e1ce8df
Reviewed-on: https://go-review.googlesource.com/c/157017
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When functions are inlined, for instructions in the inlined body, does
-S print the location of the call, or the location of the body? Right
now, we do the former. I'd like to do the latter by default, it makes
much more sense when reading disassembly. With mid-stack inlining
enabled in more cases, this quandry will come up more often.
The original behavior is still available with -S=2. Some tests
use this mode (so they can find assembly generated by a particular
source line).
This helped me with understanding what the compiler was doing
while fixing #29007.
Change-Id: Id14a3a41e1b18901e7c5e460aa4caf6d940ed064
Reviewed-on: https://go-review.googlesource.com/c/153241
Reviewed-by: David Chase <drchase@google.com>
This experiment is less effective and less needed since the
introduction of stack objects.
We can't clobber stack objects because we don't know statically
whether they are live or not.
We don't really need this experiment that much any more, as it was
primarily used to test the complicated ambiguously-live logic in the
liveness analysis, which has been removed in favor of stack objects.
It is also ~infeasible to maintain once we have safepoints everywhere.
Fixes#27326
Change-Id: I3bdde480b93dd508d048703055d4586b496176af
Reviewed-on: https://go-review.googlesource.com/c/151317
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL adds CFGs to ssa.html.
It execs dot to generate SVG,
which then gets inlined into the html.
Some standard naming and javascript hacks
enable integration with the rest of ssa.html.
Clicking on blocks highlights the relevant
part of the CFG, and vice versa.
Sample output and screenshots can be seen in #28177.
CFGs can be turned on with the suffix mask:
:* - dump CFG for every phase
:lower - just the lower phase
:lower-layout - lower through layout
:w,x-y - phases w and x through y
Calling dot after every pass is noticeably slow,
instead use the range of phases.
Dead blocks are not displayed on CFG.
User can zoom and pan individual CFG
when the automatic adjustment has failed.
Dot-related errors are reported
without bringing down the process.
Fixes#28177
Change-Id: Id52c42d86c4559ca737288aa10561b67a119c63d
Reviewed-on: https://go-review.googlesource.com/c/142517
Run-TryBot: Yury Smolsky <yury@smolsky.by>
Reviewed-by: David Chase <drchase@google.com>
