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>
Pass "set print thread-events off" to gdb to suppress thread
event prints, like "[New Thread 0xe7b83b40 (LWP 18609)]". We
don't check them, and the extra output may confuse our other
checks, in particular, checkCleanBacktrace.
Hopefully fixes#31569.
Change-Id: I6549e1280da7afa1d2e38da2b2fa7cc18c2f0373
Reviewed-on: https://go-review.googlesource.com/c/go/+/172980
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The test doesn't really require cgo, but it does require that we know
the right flags to use to run the C compiler, and that is not
necessarily correct if we don't support cgo.
Fixes#31565
Change-Id: I04dc8db26697caa470e91ad712376aa621cf765d
Reviewed-on: https://go-review.googlesource.com/c/go/+/172981
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The pclntab encoding supports writing only some PCDATA and FUNCDATA values.
However, the encoding is dense: The max index in use determines the space used.
We should thus choose a numbering in which frequently used indices are smaller.
This change re-orders the PCDATA and FUNCDATA indices using that principle,
using a quick and dirty instrumentation to measure index frequency.
It shrinks binaries by about 0.5%.
Updates #6853
file before after Δ %
go 14745044 14671316 -73728 -0.500%
addr2line 4305128 4280552 -24576 -0.571%
api 6095800 6058936 -36864 -0.605%
asm 4930928 4906352 -24576 -0.498%
buildid 2881520 2861040 -20480 -0.711%
cgo 4896584 4867912 -28672 -0.586%
compile 25868408 25770104 -98304 -0.380%
cover 5319656 5286888 -32768 -0.616%
dist 3654528 3634048 -20480 -0.560%
doc 4719672 4691000 -28672 -0.607%
fix 3418312 3393736 -24576 -0.719%
link 6137952 6109280 -28672 -0.467%
nm 4250536 4225960 -24576 -0.578%
objdump 4665192 4636520 -28672 -0.615%
pack 2297488 2285200 -12288 -0.535%
pprof 14735332 14657508 -77824 -0.528%
test2json 2834952 2818568 -16384 -0.578%
trace 11679964 11618524 -61440 -0.526%
vet 8452696 8403544 -49152 -0.581%
Change-Id: I30665dce57ec7a52e7d3c6718560b3aa5b83dd0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/171760
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
'go mod vendor' already drops test files and testdata directories, so
users should not expect the vendored module to include unnecessary
files in general.
Files tagged "ignore" are typically only used to refresh or regenerate
source files within the module to be vendored, so users of that module
do not need them.
Fixes#31088
Change-Id: I1ce9545e9b37c8e779a1826a9d494ac29d2cbfb8
Reviewed-on: https://go-review.googlesource.com/c/go/+/172978
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Its only usage was removed in golang.org/cl/103860
Change-Id: I2a230b9475b0aadf3892b89f5e4ee6d4c5b70394
Reviewed-on: https://go-review.googlesource.com/c/go/+/172917
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#31554
Updates #12032 (also originally about plan9, but later openbsd/arm)
Change-Id: Ib9f35d27a2304f38bf271c38c0b9153d210d8f95
Reviewed-on: https://go-review.googlesource.com/c/go/+/172837
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 172698 documented this flag but the description was missing
punctuation and could be clearer.
Change-Id: I310d91ae8c6b947ce7d1ae7559882f49778f770a
Reviewed-on: https://go-review.googlesource.com/c/go/+/172817
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
There is a DNS resolution bug in Kubernetes (UDP response packets get dropped by conntrack, causing timeouts in DNS queries).
The recommended workaround on Linux is to configure the resolver to use TCP for DNS queries, by setting the use-vc option in resolv.conf.
With this PR, the pure Go resolver searches for "use-vc" in resolv.conf and switches to TCP when found.
Fixes#29358
Change-Id: I26b935cae2c80e5bb9955da83299a8dea84591de
GitHub-Last-Rev: 70bc00fe41
GitHub-Pull-Request: golang/go#29594
Reviewed-on: https://go-review.googlesource.com/c/go/+/156366
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Document that the default quickcheck configuration is to run 100 times
and that there is a flag that configures it called "quickchecks".
Change-Id: I46fdab9d572e132ccc23ef907f9cc6b2d06b37c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/172698
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 172579 added field names in nodl. See that CL for an explanation.
Cuong Manh Le pointed out that we should do the same in newnamel.
This cuts 40k off the cmd/compile binary.
Change-Id: I427b117531c59630dee36f1257aad8975626b2c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/172604
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Running `go mod init` outside of GOPATH with `GO111MODULE=off`
silently fails. This behavior was undocumented.
This CL makes go mod fail with the error:
go: modules disabled by GO111MODULE=off; see 'go help modules'
Comparing with already erroring GO111MODULE=<value> conditions:
* With GO111MODULE=auto, inside GOPATH:
go modules disabled inside GOPATH/src by GO111MODULE=auto; see 'go help modules'
* With GO111MODULE=auto outside of GOPATH:
go: cannot determine module path for source directory /path/to/dir (outside GOPATH, no import comments)
Fixes#31342
Change-Id: I749787d2a8640913c4ac263072d051314d76e778
GitHub-Last-Rev: b38447457d
GitHub-Pull-Request: golang/go#31255
Reviewed-on: https://go-review.googlesource.com/c/go/+/170697
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
New versions of clang can generate multiple sections named ".text"
when using vague C++ linkage. This is valid ELF, but would cause the
Go linker to report an error when using internal linking:
symbol PACKAGEPATH(.text) listed multiple times
Avoid the problem by renaming section symbol names if there is a name
collision.
Change-Id: I41127e95003d5b4554aaf849177b3fe000382c02
Reviewed-on: https://go-review.googlesource.com/c/go/+/172697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The byte-at-a-time allocation done quoting strings in appendQuotedWith
grows the output incrementally, which is poor behavior for very large
strings. An easy fix is to make sure the buffer has enough room at
least for an unquoted string.
Add a benchmark with a megabyte of non-ASCII data.
Before: 39 allocations.
After: 7 allocations.
We could do better by doing a lot more work but this seems like a big
result for little effort.
Fixes#31472.
Change-Id: I852139e0a2bd13722c4dd329ded8ae1759abad5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/172677
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As the stackguard was increased on AIX by CL 157117, every syscalls can
now have libcall directly on the stack. This fixes some concurrency bugs
which seems to occur when semasleep is interrupted by a SIGPROF signal.
Change-Id: I905a9618d13ef227dad6f8328b0f958f2f917a5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/172359
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
variable setg_gcc in runtime/cgo/*.c should be static, otherwise it
will be mixed with the function of the same name in runtime/asm_*.s or
tls_*.s, which causes an error when building PIE with internal linking
mode.
Fixes#31485
Change-Id: I79b311ffcaf450984328db65397840ae7d85e65d
Reviewed-on: https://go-review.googlesource.com/c/go/+/172498
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently the offset values of ELF relocations and Macho relocations
are 256 and 512 respectively, which means that the space reserved for
ELF relocations is only 256. But AARCH64 has more than 256 ELF relocation
types, in fact the maximum AARCH64 ELF relocation type recorded in file
src/debug/elf/elf.go is 1032 currently. So this CL increases the offset
of Macho relocations to 2048 to leave enough space for AARCH64 ELF
relocations.
Change-Id: I784ac38aeb3e102ac7825f6d621086849c8d3146
Reviewed-on: https://go-review.googlesource.com/c/go/+/172497
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Casp1 is implemented in Go on js/wasm, so escape analysis correctly
determines that the "old" parameter does not escape (which is good).
Unfortunately, test/run.go doesn't have a way to indicate that ERROR
messages are optional, and cmd/compile only emits diagnostics for "var
x int" when it's moved to the heap; not when it stays on the stack.
To accomodate that this test currently passes on some GOARCHes but not
others, rewrite the Casp1 test to use "x := new(int)" and allow both
"new(int) escapes to heap" or "new(int) does not escape".
Updates #31525.
Change-Id: I40150a7ff9042f184386ccdb2d4d428f63e8ba4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/172602
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We use a struct to allocate two structs simultaneously.
Because we embed structs rather than using named fields,
the compiler generates forwarding method stubs for the
anonymous type.
In theory, the compiler could detect that these stubs are unnecessary:
The value in question has a very limited scope, the methods are not
called, and there are operations where an interface would need
to be satisfied.
This compiler optimization is unlikely to happen, though;
the ROI is likely to be low.
Instead, just give the fields names. Cuts 64k off the cmd/compile binary.
Change-Id: Id10ec69c23cd2dd33306f4c1bc75724e3c571b56
Reviewed-on: https://go-review.googlesource.com/c/go/+/172579
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The //go:noescape directive says that arguments don't leak at all,
which is too aggressive of a claim for functions that return pointers
derived from their parameters.
Remove the directive for now. Long term fix will require a new
directive that allows more fine-grained control over escape analysis
information supplied for functions implemented in assembly.
Also, update the BAD comments in the test cases for Loadp: we really
want that *ptr leaks to the result parameter, not that *ptr leaks to
the heap.
Updates #31525.
Change-Id: Ibfa61f2b70daa7ed3223056b57eeee777eef2e31
Reviewed-on: https://go-review.googlesource.com/c/go/+/172578
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Cherry pointed out this case in review for CL 136496. That CL was
slightly too aggressive, and I likely would have made the same mistake
if I tried it myself.
Updates #27772.
Change-Id: I1fafabb9f8d9aba0494aa71333a4e17cf1bac5c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/172421
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
There weren't any tests to make sure these work correctly, and this
led to escape analysis regressions in both linux/s390x and js/wasm.
The underlying issue that cmd/compile is only getting some of these
correct because escape analysis doesn't understand //go:linkname is
still present, but at least this addresses the fragility aspect.
Updates #15283.
Change-Id: I546aee1899d098b2e3de45e9b33c3ca22de485f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/172420
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This test fails frequently in the longtest builder, and the failures
on the build dashboard have masked two other regressions so far.
Let's skip it until it can be fixed.
Updates #31263
Change-Id: I82bae216ebc3c5fd395c27c72c196334a130af7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/172423
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There is a DNS resolution issue in Kubernetes (UDP response packets get dropped due to a race in conntrack between the parallel A and AAAA queries, causing timeouts in DNS queries).
A workaround is to enable single-request / single-request-reopen in resolv.conf in order to use sequential A and AAAA queries instead of parallel queries.
With this PR, the pure Go resolver searches for "single-request" and "single-request-reopen" in resolv.conf and send A and AAAA queries sequentially when found.
Fixes#29644
Change-Id: I906b3484008c1b9adf2e3e9241ea23767e29df59
GitHub-Last-Rev: d481acfb4c
GitHub-Pull-Request: golang/go#29661
Reviewed-on: https://go-review.googlesource.com/c/go/+/157377
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Writev was allocating a new []syscall.Iovec every call, rather than
reusing the cached copy available at *fd.iovec.
Fixes#26663.
Change-Id: I5967b0d82dc671ce0eaf4ec36cc2a0e46eadde02
Reviewed-on: https://go-review.googlesource.com/c/go/+/172419
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The CBC mode ciphers in TLS are a disaster. By ordering authentication
and encryption wrong, they are very subtly dependent on details and
implementation of the padding check, admitting attacks such as POODLE
and Lucky13.
crypto/tls does not promise full countermeasures for Lucky13 and still
contains some timing variations. This change fixes one of the easy ones:
by checking the MAC, then the padding, rather than all at once, there is
a very small timing variation between bad MAC and (good MAC, bad
padding).
The consequences depend on the effective padding value used in the MAC
when the padding is bad. extractPadding simply uses the last byte's
value, leaving the padding bytes effectively unchecked. This is the
scenario in SSL 3.0 that led to POODLE. Specifically, the attacker can
take an input record which uses 16 bytes of padding (a full block) and
replace the final block with some interesting block. The MAC check will
succeed with 1/256 probability due to the final byte being 16. This
again means that after 256 queries, the attacker can decrypt one byte.
To fix this, bitwise AND the two values so they may be checked with one
branch. Additionally, zero the padding if the padding check failed, to
make things more robust.
Updates #27071
Change-Id: I332b14d215078928ffafe3cfeba1a68189f08db3
Reviewed-on: https://go-review.googlesource.com/c/go/+/170701
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Though there is variation in the spelling of canceled,
cancellation is always spelled with a double l.
Reference: https://www.grammarly.com/blog/canceled-vs-cancelled/
Change-Id: I240f1a297776c8e27e74f3eca566d2bc4c856f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/170060
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Same as CL 170323, but for s390x instead of wasm.
Fixes#31495.
Change-Id: Ie39f649f5e33690375a8bcb1bc3b92d912ca4398
Reviewed-on: https://go-review.googlesource.com/c/go/+/172417
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In typecheckclosure, a xfunc node will be put to xtop. But that node can
be shared between multiple closures, like in a const declaration group:
const (
x = unsafe.Sizeof(func() {})
y
)
It makes a xfunc node appears multiple times in xtop, causing duplicate
initLSym run.
To fix this issue, we only do typecheck for xfunc one time, and setup
closure node earlier in typecheckclosure process.
Fixes#30709
Change-Id: Ic924a157ee9f3e5d776214bef5390849ddc8aab9
Reviewed-on: https://go-review.googlesource.com/c/go/+/172298
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When we construct the build list by loading packages (e.g., in
"go build", "go list", or "go test"), we may load additional modules
not mentioned in the original build list. If we encounter an error
loading one of these modules, mvs.BuildList currently returns a
BuildListError with a chain of requirments. Unfortunately, this is not
helpful, since the graph is structured such that these missing modules
are direct requirements of the main module.
With this change, loader.load keeps track of the package that caused
each "missing" module to be added. If an error occurs in a missing
module, the chain of package imports is printed instead of the module
requirements.
Fixes#31475
Change-Id: Ie484814af42ceea3e85fedc38e705ba3a38cd495
Reviewed-on: https://go-review.googlesource.com/c/go/+/171859
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
mvs.BuildList reports errors with a chain of modules to make it
clear why the module where the error occurred was part of the
build. This is a little confusing with "go get -u" since there are
edges in the module graph for requirements and for updates.
With this change, we now print "requires" or "updates to" between
each module version in the chain.
Updates #30661
Change-Id: Ie689500ea86857e715b250b9e0cae0bc6686dc32
Reviewed-on: https://go-review.googlesource.com/c/go/+/171150
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
For example, "go get golang.org/x/tools/cmd/..." will add a
requirement for "golang.org/x/tools" to go.mod and will install
executables from the "cmd" subdirectory.
Fixes#29363
Change-Id: Id53f051710708d7760ffe831d4274fd54533d2b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/171138
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This fix removes the special case of auto-deriving the module path
only for VCS github.com. Error message now explicitly requests
the module path. Documentation and its FAQ do not need an update
as only the beginning of the message is mentioned and is not modified.
Fixes#27951
Change-Id: Icaf87a38b5c58451edba9beaa12ae9a68e288ca1
Reviewed-on: https://go-review.googlesource.com/c/go/+/172019
Reviewed-by: Daniel Lublin <daniel@lublin.se>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 change modifies cmd/go/list to format the error correctly in case
-e flag is set. It also fixes a bug where the package loader was only
ever checking the first pattern if it had the go extension. This caused
and error when a file without .go extension was not the first argument.
Fixes#29899
Change-Id: I029bf4465ad4ad054434b8337c1d2a59369783da
Reviewed-on: https://go-review.googlesource.com/c/go/+/166398
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
On some configurations of Windows, directories containing executable
files may be locked for a while after the executable exits (perhaps
due to antivirus scans?). It's probably worth a little extra latency
on exit to avoid filling up the user's temporary directory with leaked
files.
Updates #30789
Change-Id: Iae7fcdd07fb9ecfb05967cfe0c8833db646d2f85
Reviewed-on: https://go-review.googlesource.com/c/go/+/172337
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Transport.DialerAndTLSConfigSupportsHTTP2 was added just earlier
in CL 130256 but we thought of a better name moments after submitting.
ForceAttemptHTTP2 is shorter, more direct, and doesn't constrain what
we can use it with in the future.
Updates #14391
Updates #27011
Change-Id: Ie5fc71bafcbcaa1941b5d49f748b6d710503d477
Reviewed-on: https://go-review.googlesource.com/c/go/+/172299
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Dirent doesn't work properly. Diable the test for now.
Update #31416
Change-Id: I34a8045598a9c303dcc754ce04da3c124f122d1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/171818
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new escape analysis implementation tries to emit debugging
diagnostics that are compatible with the existing implementation, but
there's a handful of cases that are easier to handle by updating the
test expectations instead.
For regress tests that need updating, the original file is copied to
oldescapeXXX.go.go with -newescape=false added to the //errorcheck
line, while the file is updated in place with -newescape=true and new
test requirements.
Notable test changes:
1) escape_because.go looks for a lot of detailed internal debugging
messages that are fairly particular to how esc.go works and that I
haven't attempted to port over to escape.go yet.
2) There are a lot of "leaking param: x to result ~r1 level=-1"
messages for code like
func(p *int) *T { return &T{p} }
that were simply wrong. Here &T must be heap allocated unconditionally
(because it's being returned); and since p is stored into it, p
escapes unconditionally too. esc.go incorrectly reports that p escapes
conditionally only if the returned pointer escaped.
3) esc.go used to print each "leaking param" analysis result as it
discovered them, which could lead to redundant messages (e.g., that a
param leaks at level=0 and level=1). escape.go instead prints
everything at the end, once it knows the shortest path to each sink.
4) esc.go didn't precisely model direct-interface types, resulting in
some values unnecessarily escaping to the heap when stored into
non-escaping interface values.
5) For functions written in assembly, esc.go only printed "does not
escape" messages, whereas escape.go prints "does not escape" or
"leaking param" as appropriate, consistent with the behavior for
functions written in Go.
6) 12 tests included "BAD" annotations identifying cases where esc.go
was unnecessarily heap allocating something. These are all fixed by
escape.go.
Updates #23109.
Change-Id: Iabc9eb14c94c9cadde3b183478d1fd54f013502f
Reviewed-on: https://go-review.googlesource.com/c/go/+/170447
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
