Currently, the prove pass derives implicit relations between len and
cap in the code that adds branch conditions. This is fine right now
because that's the only place we can encounter len and cap, but we're
about to add a second way to add assertions to the facts table that
can also produce facts involving len and cap.
Prepare for this by moving the fact derivation from updateRestrictions
(where it only applies on branches) to factsTable.update, which can
derive these facts no matter where the root facts come from.
Passes toolstash -cmp.
Change-Id: If09692d9eb98ffaa93f4cfa58ed2d8ba0887c111
Reviewed-on: https://go-review.googlesource.com/102602
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Currently, we never add a relation between two constants to prove's
fact table because these are eliminated before prove runs, so it
currently doesn't handle facts like this very well even though they're
easy to prove.
We're about to start asserting some conditions that don't appear in
the SSA, but are constructed from existing SSA values that may both be
constants.
Hence, improve the fact table to understand relations between
constants by initializing the constant bounds of constant values to
the value itself, rather than noLimit.
Passes toolstash -cmp.
Change-Id: I71f8dc294e59f19433feab1c10b6d3c99b7f1e26
Reviewed-on: https://go-review.googlesource.com/102601
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Inlining was refactored to perform tuning experiments,
with the "knobs" now set to also inline functions/methods
that include panic(), and -l=4 (inline calls) now expressed
as a change to costs, rather than scattered if-thens.
The -l=4 inline-calls penalty is chosen to be the best
found during experiments; it makes some programs much
larger and slower (notably, the compiler itself) and is
believed to be risky for machine-generated code in general,
which is why it is not the default. It is also not
well-tested with the debugger and DWARF output.
This change includes an explicit go:noinline applied to the
method that is the largest cause of compiler binary growth
and slowdown for midstack inlining; there are others,
ideally whatever heuristic eventually appears will make
this unnecessary.
Change-Id: Idf7056ed2f961472cf49d2fd154ee98bef9421e2
Reviewed-on: https://go-review.googlesource.com/109918
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change brings back the EKU checking from 1.9. In 1.10, we checked
EKU nesting independent of the requested EKUs so that, after verifying a
certifciate, one could inspect the EKUs in the leaf and trust them.
That, however, was too optimistic. I had misunderstood that the PKI was
/currently/ clean enough to require that, rather than it being
desirable. Go generally does not push the envelope on these sorts of
things and lets the browsers clear the path first.
Fixes#24590
Change-Id: I18c070478e3bbb6468800ae461c207af9e954949
Reviewed-on: https://go-review.googlesource.com/113475
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Now that raise on darwin targets the current thread, we can remove
the workaround in dieFromSignal.
Change-Id: I1e468dc05e49403ee0bbe0a3a85e764c81fec4f2
Reviewed-on: https://go-review.googlesource.com/110476
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This CL is the darwin/arm and darwin/arm64 equivalent to CL 108679,
110215, 110437, 110438, 111258, 110655.
Updates #17490
Change-Id: Ia95b27b38f9c3535012c566f17a44b4ed26b9db6
Reviewed-on: https://go-review.googlesource.com/111015
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
pthread_self and pthread_kill are not safe to call from a signal
handler. In particular, pthread_self fails in iOS when called from
a signal handler context.
Use raise instead; it is signal handler safe and simpler.
Change-Id: I0cbfe25151aed245f55d7b76719ce06dc78c6a75
Reviewed-on: https://go-review.googlesource.com/113877
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When an object spans heap arenas, its bitmap is discontiguous, so
heapBitsSetType unrolls the bitmap into the object itself and then
copies it out to the real heap bitmap. Unfortunately, since this code
path is rare, it had two unnoticed bugs related to the head and tail
of the bitmap:
1. At the head of the object, we were using hbitp as the destination
bitmap pointer rather than h.bitp, but hbitp points into the
*temporary* bitmap space (that is, the object itself), so we were
failing to copy the partial bitmap byte at the head of an object.
2. The core copying loop copied all of the full bitmap bytes, but
always drove the remaining word count down to 0, even if there was a
partial bitmap byte for the tail of the object. As a result, we never
wrote partial bitmap bytes at the tail of an object.
I found these by enabling out-of-place unrolling all the time. To
improve our chances of detecting these sorts of bugs in the future,
this CL mimics this by enabling out-of-place mode 50% of the time when
doubleCheck is enabled so that we test both in-place and out-of-place
mode.
Change-Id: I69e5d829fb3444be4cf11f4c6d8462c26dc467e8
Reviewed-on: https://go-review.googlesource.com/110995
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
We have a workaround in place in the runtime (see CL 16853 and
CL 111176) to keep arm and arm64 Go binaries working under QEMU
in user-emulation mode (Issue #13024).
This change adds a regression test about arm/arm64 QEMU emulation
to cmd/go.
Change-Id: Ic67f476e7c30a7d7852d9b01834f1dcabfac2ff7
Reviewed-on: https://go-review.googlesource.com/111477
Reviewed-by: Ian Lance Taylor <iant@golang.org>
First, the regions sort was buggy, as its last comparison was
ineffective.
Second, the insyscall and insyscallRuntime fields were unsigned, so the
check for them being negative was pointless. Make them signed instead,
to also prevent the possibility of underflows when decreasing numbers
that might realistically be 0.
Third, the color constants were all untyped strings except the first
one. Be consistent with their typing.
Change-Id: I4eb8d08028ed92589493c2a4b9cc5a88d83f769b
Reviewed-on: https://go-review.googlesource.com/113895
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The function signatures in the comments used a C-like style. Using
Go function signatures is cleaner.
Change-Id: I1a093ed8fe5df59f3697c613cf3fce58bba4f5c1
Reviewed-on: https://go-review.googlesource.com/113876
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use the math/bits functions to calculate the number of leading/
trailing zeros, bit length and the population count.
The math/bits package is built as part of the bootstrap process
so we do not need to provide an alternative implementation for
Go versions prior to 1.9.
Passes toolstash-check -all.
Change-Id: I393b4cc1c8accd0ca7cb3599d3926fa6319b574f
Reviewed-on: https://go-review.googlesource.com/113336
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use mach_absolute_time and mach_timebase_info to get nanosecond-level
timing information from libc on Darwin.
The conversion code from Apple's arbitrary time unit to nanoseconds is
really annoying. It would be nice if we could replace the internal
runtime "time" with arbitrary units and put the conversion to nanoseconds
only in the places that really need it (so it isn't in every nanotime call).
It's especially annoying because numer==denom==1 for all the machines
I tried. Makes it hard to test the conversion code :(
Update #17490
Change-Id: I6c5d602a802f5c24e35184e33d5e8194aa7afa86
Reviewed-on: https://go-review.googlesource.com/110655
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A few libc_ calls were missing stack switches.
Unfortunately, adding the stack switches revealed a deeper problem.
systemstack() is fundamentally flawed because when you do
systemstack(func() { ... })
There's no way to mark the anonymous function as nosplit. At first I
thought it didn't matter, as that function runs on the g0 stack. But
nosplit is still required, because some syscalls are done when stack
bounds are not set up correctly (e.g. in a signal handler, which runs
on the g0 stack, but g is still pointing at the g stack). Instead use
asmcgocall and funcPC, so we can be nosplit all the way down.
Mid-stack inlining now pushes darwin over the nosplit limit also.
Leaving that as a TODO.
Update #23168
This might fix the cause of occasional darwin hangs.
Update #25181
Update #17490
Change-Id: If9c3ef052822c7679f5a1dd192443f714483327e
Reviewed-on: https://go-review.googlesource.com/111258
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This parses the import table properly which allows for debug/pe
to extract import symbols from pecoffs linked with an import
table in a section named something other than ".idata"
The section names in a pecoff object aren't guaranteed to actually
mean anything, so hardcoding a search for the ".idata" section
is not guaranteed to find the import table in all shared libraries.
This resulted in debug/pe being unable to read import symbols
from some libraries.
The proper way to locate the import table is to validate the
number of data directory entries, locate the import entry, and
then use the va to identify the section containing the import
table. This patch does exactly this.
Fixes#16103.
Change-Id: I3ab6de7f896a0c56bb86c3863e504e8dd4c8faf3
GitHub-Last-Rev: ce8077cb15
GitHub-Pull-Request: golang/go#25193
Reviewed-on: https://go-review.googlesource.com/110555
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
There's a glitch in how attributes from procs that do not
generate code are combined, and the workaround for this
glitch appeared in two places.
"One big pile is better than two little ones."
Updates #25426.
Change-Id: I252f9adc5b77591720a61fa22e6f9dda33d95350
Reviewed-on: https://go-review.googlesource.com/113717
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This commit improves wasm_exec.js to give more control to the
code that uses this helper:
- Allow to load and run more than one Go program at the same time.
- Move WebAssembly.instantiate out of wasm_exec.js so the caller
can optimize for load-time performance, e.g. by using
instantiateStreaming.
- Allow caller to provide argv, env and exit callback.
Updates #18892
Change-Id: Ib582e6f43848c0118ea5c89f2e24b371c45c2050
Reviewed-on: https://go-review.googlesource.com/113515
Reviewed-by: Agniva De Sarker <agniva.quicksilver@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This prevents a crash in go/types due to a nil condition in an 'if'
statement. There's more we can do to make go/types more robust but
this will address the immediate cause and also makes sure that the
parser returns a valid AST in this case.
Fixes#25438.
Change-Id: Ie55dc2c722352a5ecb17af6a16983741e8a8b515
Reviewed-on: https://go-review.googlesource.com/113735
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
Reviewed-by: Alan Donovan <adonovan@google.com>
The check has worked this way for a long time, but it has never been
well documented.
For #25453.
Change-Id: If603e53348ba51f73b2f449b943c6f97f64aa3eb
Reviewed-on: https://go-review.googlesource.com/113755
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Without this running go vet -compiler=gccgo causes vet to fail.
The vet tool does need to know the compiler, but it is passed in
vetConfig.Compiler.
Change-Id: If857be4f336f6d7c425972fabcf82fae2cdc8a90
Reviewed-on: https://go-review.googlesource.com/113715
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
ASN.1 has an private class, but current implementation does not support it.
Change-Id: I3ebf07a048831869572f75223cb17d4c115caef7
GitHub-Last-Rev: b3c69ad091
GitHub-Pull-Request: golang/go#25195
Reviewed-on: https://go-review.googlesource.com/110561
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
If go toolchain is not built with GOEXPERIMENT=fieldtrack,
skip fieldtrack pass in the linker as it does full symtab traversal.
For linking "hello world" example from net/http:
name old time/op new time/op delta
Linker-4 530ms ± 2% 525ms ± 2% -1.03% (p=0.028 n=17+19)
Fixes#20318
Updates #14624
Change-Id: I99336513db77d13f95f47d27339d76f01c42a5da
Reviewed-on: https://go-review.googlesource.com/113635
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 113095 tried to deflake net tests on iOS by skipping the test
that uses the most sockets. That didn't work well enough and will
be reverted in CL 113555.
The flakes appeared after the iOS exec harness started to forward
environment variables, causing testenv.Builder to be non-empty on
the iOS builder. This CL attempts to fix the flakes with the more
conservative strategy of skipping tests that only run on builders.
The skipped tests happen to be those requiring external network
access; it's plausible that the iOS builder network isn't reliable
enough to run the many parallel DNS lookups and dial outs, while
keeping the number of open file descriptors below the 250 limit.
Change-Id: I9cafdaf2845dd6f3844c4819dcaaaa5970f5da15
Reviewed-on: https://go-review.googlesource.com/113575
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 3027932ac3.
Reason for revert: It didn't work well enough; a replacement CL that skips external net tests altogether on iOS is coming.
Change-Id: Ib2c5656cee92bcae744f9c99fbcb9f9f2baa0694
Reviewed-on: https://go-review.googlesource.com/113555
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The file is not used ('go tool' uses cmd/pprof/pprof.go instead)
and the external package import in this file causes test failure.
Fixes#25367
Change-Id: I71fbf8a3631efb1bd9e459b8247e5f7a6683894c
Reviewed-on: https://go-review.googlesource.com/113295
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds functionality to properly handle NoPadding in NewDecoder.
Removes the following expectations when using NoPadding:
* the input message length is a multiple of 8
* the input message length is 0, or longer than 7 characters
Fixes#25332
Change-Id: I7c38160df23f7e8da4f85a5629530016e7bf71f3
GitHub-Last-Rev: 68ab8d2291
GitHub-Pull-Request: golang/go#25394
Reviewed-on: https://go-review.googlesource.com/113215
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
So we can have builders running go test -short=false.
Updates golang/go#12508
Change-Id: If90f0f6d9f89268c33b1d1876139ad551fecd3d8
Reviewed-on: https://go-review.googlesource.com/113435
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This block of code once was commented by the original author, but commenting
code looks a little annoying. However, the debugSelect flag is just for the
situation that debug code will be compiled when debuging, when release this
code will be eliminated by the compiler.
Change-Id: I7b94297e368b515116ef44a36058214ddddf9adb
Reviewed-on: https://go-review.googlesource.com/113395
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
After CL 113016 the "a" article is no longer necessary.
Change-Id: I5a80a210bd2b9eedd73d5f9f3f338d7f22c29ea6
Reviewed-on: https://go-review.googlesource.com/113355
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When a variable of type int is compared with sizeof's return
value, gcc warns:
comparison between signed and unsigned integer expressions
Change the type of a couple loop indices that looped over sizeof from
int to size_t to silence the warnings.
Fixes#25411
Change-Id: I2c7858f84237e77945651c7b1b6a75b97edcef65
Reviewed-on: https://go-review.googlesource.com/113335
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Before:
unexpected at 2721:load with unexpected source op v3278unexpected at 2775:load with
unexpected source op v3281unexpected at 2249:load with unexpected source op
v3289unexpected at 2875:load with unexpected source op v3278unexpected at 2232:load
with unexpected source op v286unexpected at 2231:load with unexpected source op
v3291unexpected at 2784:load with unexpected source op v3289unexpected at 2785:load
with unexpected source op v3291
After:
debug info generation: v2721: load with unexpected source op: Phi (v3278)
debug info generation: v2775: load with unexpected source op: Phi (v3281)
debug info generation: v2249: load with unexpected source op: Phi (v3289)
debug info generation: v2875: load with unexpected source op: Phi (v3278)
debug info generation: v2232: load with unexpected source op: Phi (v286)
debug info generation: v2231: load with unexpected source op: Phi (v3291)
debug info generation: v2784: load with unexpected source op: Phi (v3289)
debug info generation: v2785: load with unexpected source op: Phi (v3291)
Updates #25404.
Change-Id: Ib97722848d27ca18bdcd482a610626bc3c6def7d
Reviewed-on: https://go-review.googlesource.com/113275
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change updates the vendored copy of golang.org/x/crypto to
commit 1a580b3eff7814fc9b40602fd35256c63b50f491.
An import of golang.org/x/sys/cpu was replaced with an import of
internal/cpu as required by
https://github.com/golang/go/issues/24843#issuecomment-383194779.
The following bash command can be used to replicate this import
update:
find `pwd` -name '*.go' -exec sed -i 's/golang\.org\/x\/sys\/cpu/internal\/cpu/g' '{}' \;
Change-Id: Ic80d361f940a96c70e4196f594d791c63421d73c
Reviewed-on: https://go-review.googlesource.com/113175
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The preallocated memory size is comparable to bytes.Buffer bootstraping
array length (bytes.Buffer was used before rewrite to strings.Builder).
Without preallocation, encodeWord does more than one allocation for
almost any possible input.
The regression happens because bytes.Buffer did a 80-bytes allocation
at the beginning of encodeWord while strings.Builder did several
smaller allocations (started with cap=0).
Comparison with reported regression:
name old time/op new time/op delta
QEncodeWord-4 781ns ± 1% 593ns ± 1% -24.08% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
QEncodeWord-4 152B ± 0% 80B ± 0% -47.37% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
QEncodeWord-4 5.00 ± 0% 2.00 ± 0% -60.00% (p=0.008 n=5+5)
Comparison with buffer solution (like before strings.Builder, but
without sync pool for buffer re-using):
name old time/op new time/op delta
QEncodeWord-4 595ns ± 1% 593ns ± 1% ~ (p=0.460 n=5+5)
name old alloc/op new alloc/op delta
QEncodeWord-4 160B ± 0% 80B ± 0% -50.00% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
QEncodeWord-4 2.00 ± 0% 2.00 ± 0% ~ (all equal)
We avoid allocation in buf.String(), as expected.
Fixes#25379
Change-Id: I19763f0e593a27390c1a549b86ce6507b489046b
Reviewed-on: https://go-review.googlesource.com/113235
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit addresses a FIXME left in the code of wasm_exec.js to
properly get the upper 32 bit of a JS number to be stored as an
64-bit integer. A bitshift operation is not possible, because in
JavaScript bitshift operations only operate on the lower 32 bits.
Change-Id: I8f627fd604e592682d9d322942a4852db64a7f66
Reviewed-on: https://go-review.googlesource.com/113076
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit changes wasm_exec.js so it only puts the single
name "go" into the global namespace. Other names became private
or were turned into a property/method of "go".
Change-Id: I633829dfd3c06936f092c0a14b9978bf855e41fe
Reviewed-on: https://go-review.googlesource.com/112980
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Agniva De Sarker <agniva.quicksilver@gmail.com>
The iOS builder recently gained access to the GO_BUILDER_NAME
environment variable, which in turn enabled some net tests that
were previously guarded by testenv.Builder() == "". Some such tests
have been disabled because they don't work; others have increased
the pressure on open file descriptors, pushing the low iOS limit of
250.
Since many net tests run with t.Parallel(), the "too many open files"
error hit many different tests, so instead of playing whack-a-mole,
lower the file descriptor demand by skipping the most file
descriptor hungry test, TestTCPSpuriousConnSetupCompletionWithCancel.
Before:
$ GO_BUILDER_NAME=darwin-arm64 GOARCH=arm64 go test -short -v net
...
Socket statistical information:
...
(inet4, stream, default): opened=5245 connected=193 listened=75 accepted=177 closed=5399 openfailed=0 connectfailed=5161 listenfailed=0 acceptfailed=143 closefailed=0
...
After:
$ GO_BUILDER_NAME=darwin-arm64 GOARCH=arm64 go test -short -v net
...
Socket statistical information:
...
(inet4, stream, default): opened=381 connected=194 listened=75 accepted=169 closed=547 openfailed=0 connectfailed=297 listenfailed=0 acceptfailed=134 closefailed=0
...
Fixes#25365 (Hopefully).
Change-Id: I8343de1b687ffb79001a846b1211df7aadd0535b
Reviewed-on: https://go-review.googlesource.com/113095
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TestStmtLines has been added in CL 102435.
This test is failing on Plan 9 because executables
don't have a DWARF symbol table.
Fixes#25387.
Change-Id: I6ae7cba0e8ad4ab569a29ea8920b7849acfb9846
Reviewed-on: https://go-review.googlesource.com/113115
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This test measures "line churn" which was minimized to help
improve the debugger experience. With proper is_stmt markers,
this is no longer necessary, and it is more accurate (for
profiling) to allow line numbers to vary willy-nilly.
"Debugger experience" is now better measured by
cmd/compile/internal/ssa/debug_test.go
This CL made the obsoleting change:
https://go-review.googlesource.com/c/go/+/102435
Change-Id: I874ab89f3b243b905aaeba7836118f632225a667
Reviewed-on: https://go-review.googlesource.com/113155
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since subtests and subbenchmarks run in a separate goroutine, and thus
a separate stack, this entails capturing the stack trace at the point
tb.Run is called. The work of getting the file and line information from
this stack is only done when needed, however.
Continuing the search into the parent test also requires temporarily
holding its mutex. Since Run does not hold it while waiting for the
subtest to complete, there should be no risk of a deadlock due to this.
Fixes#24128
Change-Id: If0bb169f3ac96bd48794624e619ade7edb599f83
Reviewed-on: https://go-review.googlesource.com/108658
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
When running a benchmark multiple times, instead of re-computing the
value of b.N each time, use the value found by the first run.
For
go test -bench=. -benchtime 3s -count 2 p_test.go
on the benchmark in the linked issue; before:
BenchmarkBenchmark-4 500 10180593 ns/op
--- BENCH: BenchmarkBenchmark-4
p_test.go:13: single call took 10.111079ms
p_test.go:13: single call took 1.017298685s
p_test.go:13: single call took 5.090096124s
BenchmarkBenchmark-4 500 10182164 ns/op
--- BENCH: BenchmarkBenchmark-4
p_test.go:13: single call took 10.098169ms
p_test.go:13: single call took 1.017712905s
p_test.go:13: single call took 5.090898517s
PASS
ok command-line-arguments 12.244s
and after:
BenchmarkBenchmark-4 500 10177076 ns/op
--- BENCH: BenchmarkBenchmark-4
p_test.go:13: single call took 10.091301ms
p_test.go:13: single call took 1.016943125s
p_test.go:13: single call took 5.088376028s
BenchmarkBenchmark-4 500 10171497 ns/op
--- BENCH: BenchmarkBenchmark-4
p_test.go:13: single call took 10.140245ms
p_test.go:13: single call took 5.085605921s
PASS
ok command-line-arguments 11.218s
Fixes#23423
Change-Id: Ie66a8c5ac43881eb8741e14105db28745b4d56d3
Reviewed-on: https://go-review.googlesource.com/110775
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In prove, reuse posets between different functions by storing them
in the per-worker cache.
Allocation count regression caused by prove improvements is down
from 5% to 3% after this CL.
Updates #25179
Change-Id: I6d14003109833d9b3ef5165fdea00aa9c9e952e8
Reviewed-on: https://go-review.googlesource.com/110455
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
prove uses the poset datastructure in a DFS walk, and always undoes
it back to its pristine status. Before this CL, poset's undo of
a new node creation didn't fully deallocate the node, which means
that at the end of prove there was still some allocated memory pending.
This was not a problem until now because the posets used by prove
were discarded after each function, but it would prevent recycling
them between functions (as a followup CL does).
Change-Id: I1c1c99c03fe19ad765395a43958cb256f686765a
Reviewed-on: https://go-review.googlesource.com/112976
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
This marks the first instruction after the prologue for
consumption by debuggers, specifically Delve, who asked
for it. gdb appears to ignore it, lldb appears to use it.
The bits for end-of-prologue and beginning-of-epilogue
are added to Pos (reducing maximum line number by 4x, to
1048575). They're added in cmd/internal/obj/<ARCH>.go
(currently x86 only), so the compiler-proper need not
deal with them.
The linker currently does nothing with beginning-of-epilogue,
but the plumbing exists to make it easier in the future.
This also upgrades the line number table to DWARF version 3.
This CL includes a regression in the coverage for
testdata/i22558.gdb-dbg.nexts, this appears to be a gdb
artifact but the fix would be in the preceding CL in the
stack.
Change-Id: I3bda5f46a0ed232d137ad48f65a14835c742c506
Reviewed-on: https://go-review.googlesource.com/110416
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
A new pass run after ssa building (before any other
optimization) identifies the "first" ssa node for each
statement. Other "noise" nodes are tagged as being never
appropriate for a statement boundary (e.g., VarKill, VarDef,
Phi).
Rewrite, deadcode, cse, and nilcheck are modified to move
the statement boundaries forward whenever possible if a
boundary-tagged ssa value is removed; never-boundary nodes
are ignored in this search (some operations involving
constants are also tagged as never-boundary and also ignored
because they are likely to be moved or removed during
optimization).
Code generation treats all nodes except those explicitly
marked as statement boundaries as "not statement" nodes,
and floats statement boundaries to the beginning of each
same-line run of instructions found within a basic block.
Line number html conversion was modified to make statement
boundary nodes a bit more obvious by prepending a "+".
The code in fuse.go that glued together the value slices
of two blocks produced a result that depended on the
former capacities (not lengths) of the two slices. This
causes differences in the 386 bootstrap, and also can
sometimes put values into an order that does a worse job
of preserving statement boundaries when values are removed.
Portions of two delve tests that had caught problems were
incorporated into ssa/debug_test.go. There are some
opportunities to do better with optimized code, but the
next-ing is not lying or overly jumpy.
Over 4 CLs, compilebench geomean measured binary size
increase of 3.5% and compile user time increase of 3.8%
(this is after optimization to reuse a sparse map instead
of creating multiple maps.)
This CL worsens the optimized-debugging experience with
Delve; we need to work with the delve team so that
they can use the is_stmt marks that we're emitting now.
The reference output changes from time to time depending
on other changes in the compiler, sometimes better,
sometimes worse.
This CL now includes a test ensuring that 99+% of the lines
in the Go command itself (a handy optimized binary) include
is_stmt markers.
Change-Id: I359c94e06843f1eb41f9da437bd614885aa9644a
Reviewed-on: https://go-review.googlesource.com/102435
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
