Currently it's possible for user code to exploit the high scheduler
priority of the GC worker in conjunction with the runnext optimization
to elevate a user goroutine to high priority so it will always run
even if there are other runnable goroutines.
For example, if a goroutine is in a tight allocation loop, the
following can happen:
1. Goroutine 1 allocates, triggering a GC.
2. G 1 attempts an assist, but fails and blocks.
3. The scheduler runs the GC worker, since it is high priority.
Note that this also starts a new scheduler quantum.
4. The GC worker does enough work to satisfy the assist.
5. The GC worker readies G 1, putting it in runnext.
6. GC finishes and the scheduler runs G 1 from runnext, giving it
the rest of the GC worker's quantum.
7. Go to 1.
Even if there are other goroutines on the run queue, they never get a
chance to run in the above sequence. This requires a confluence of
circumstances that make it unlikely, though not impossible, that it
would happen in "real" code. In the test added by this commit, we
force this confluence by setting GOMAXPROCS to 1 and GOGC to 1 so it's
easy for the test to repeated trigger GC and wake from a blocked
assist.
We fix this by making GC always put user goroutines at the end of the
run queue, instead of in runnext. This makes it so user code can't
piggy-back on the GC's high priority to make a user goroutine act like
it has high priority. The only other situation where GC wakes user
goroutines is waking all blocked assists at the end, but this uses the
global run queue and hence doesn't have this problem.
Fixes#15706.
Change-Id: I1589dee4b7b7d0c9c8575ed3472226084dfce8bc
Reviewed-on: https://go-review.googlesource.com/23172
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently ready always puts the readied goroutine in runnext. We're
going to have to change this for some uses, so add a flag for whether
or not to use runnext.
For now we always pass true so this is a no-op change.
For #15706.
Change-Id: Iaa66d8355ccfe4bbe347570cc1b1878c70fa25df
Reviewed-on: https://go-review.googlesource.com/23171
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
When the generated stub functions write back the results to the stack,
they can in some cases be writing to the same memory on the g0 stack.
There is no race here (assuming there is no race in the Go code), but
the thread sanitizer does not know that. Turn off the thread sanitizer
for the stub functions to prevent false positive warnings.
Current clang suggests the no_sanitize("thread") attribute, but that
does not work with clang 3.6 or GCC. clang 3.6, GCC, and current clang
all support the no_sanitize_thread attribute, so use that
unconditionally.
The test case and first version of the patch are from Dmitriy Vyukov.
Change-Id: I80ce92824c6c8cf88ea0fe44f21cf50cf62474c9
Reviewed-on: https://go-review.googlesource.com/23252
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
OpenBSD 6.0 (due out November 2016) will support PT_TLS, which will
allow for the OpenBSD cgo pthread_create() workaround to be removed.
However, in order for Go to continue working on supported OpenBSD
releases (the current release and the previous release - 5.9 and 6.0,
once 6.0 is released), we cannot enable PT_TLS immediately. Instead,
adjust the existing code so that it works with the previous TCB
allocation and the new TIB allocation. This allows the same Go
runtime to work on 5.8, 5.9 and later 6.0.
Once OpenBSD 5.9 is no longer supported (May 2017, when 6.1 is
released), PT_TLS can be enabled and the additional cgo runtime
code removed.
Change-Id: I3eed5ec593d80eea78c6656cb12557004b2c0c9a
Reviewed-on: https://go-review.googlesource.com/23197
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gobot Gobot <gobot@golang.org>
They get rewritten to NEWs, and they must be marked as escaping
so walk doesn't try to allocate them back onto the stack.
Fixes#15733
Change-Id: I433033e737c3de51a9e83a5a273168dbc9110b74
Reviewed-on: https://go-review.googlesource.com/23223
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The test case in #15639 somehow causes an invalid syscall frame. The
failure is obscured because the throw occurs when throwsplit == true,
which causes a "stack split at bad time" error when trying to print the
throw message.
This CL fixes the "stack split at bad time" by using systemstack. No
test because there shouldn't be any way to trigger this error anyhow.
Update #15639.
Change-Id: I4240f3fd01bdc3c112f3ffd1316b68504222d9e1
Reviewed-on: https://go-review.googlesource.com/23153
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Generate load/stores for small zeroing/move, DUFFZERO/DUFFCOPY for
medium zeroing/move, and loops for large zeroing/move.
cmd/compile/internal/gc/testdata/{copy_ssa.go,zero_ssa.go} tests
passed.
Progress on SSA backend for ARM. Still not complete. A few packages
in the standard library compile and tests passed, including
container/list, hash/crc32, unicode/utf8, etc.
Updates #15365.
Change-Id: Ieb4b68b44ee7de66bf7b68f5f33a605349fcc6fa
Reviewed-on: https://go-review.googlesource.com/23097
Reviewed-by: Keith Randall <khr@golang.org>
Implement shifts and multiplications for up to 32-bit values.
Also handle Exit block.
Progress on SSA backend for ARM. Still not complete.
container/heap, crypto/subtle, hash/adler32 packages compile and
tests passed.
Updates #15365.
Change-Id: I6bee4d5b0051e51d5de97e8a1938c4b87a36cbf8
Reviewed-on: https://go-review.googlesource.com/23096
Reviewed-by: Keith Randall <khr@golang.org>
Fix hardcoded flag register mask in ssa/flagalloc.go by auto-generating
the mask.
Also fix a mistake (in previous CL) about conditional branches.
Progress on SSA backend for ARM. Still not complete. Now "container/ring"
package compiles and tests passed.
Updates #15365.
Change-Id: Id7c8805c30dbb8107baedb485ed0f71f59ed6ea8
Reviewed-on: https://go-review.googlesource.com/23093
Reviewed-by: Keith Randall <khr@golang.org>
Instead, decline the session and do a full handshake. The semantics of
cross-version resume are unclear, and all major client implementations
treat this as a fatal error. (This doesn't come up very much, mostly if
the client does the browser version fallback without sharding the
session cache.)
See BoringSSL's bdf5e72f50e25f0e45e825c156168766d8442dde and OpenSSL's
9e189b9dc10786c755919e6792e923c584c918a1.
Change-Id: I51ca95ac1691870dd0c148fd967739e2d4f58824
Reviewed-on: https://go-review.googlesource.com/21152
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates x/net/http2 to golang.org/cl/23220
(http2: with Go 1.7 set Request.Context in ServeHTTP handlers)
Fixes#15134
Change-Id: I73bac2601118614528f051e85dab51dc48e74f41
Reviewed-on: https://go-review.googlesource.com/23221
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Make the temporary, conservative restrictions from rev 79d9f48c in Go
1.6 permanent, and also don't do automatic TLS if the user configured
a Dial or DialTLS hook. (Go 1.7 has Transport.Dialer instead, for
tweaking dialing parameters)
Fixes#14275
Change-Id: I5550d5c1e3a293e103eb4251a3685dc204a23941
Reviewed-on: https://go-review.googlesource.com/23222
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run live vars test only on ssa builds.
We can't just drop KeepAlive ops during regalloc. We need
to replace them with copies.
Change-Id: Ib4b3b1381415db88fdc2165fc0a9541b73ad9759
Reviewed-on: https://go-review.googlesource.com/23225
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Introduce a KeepAlive op which makes sure that its argument is kept
live until the KeepAlive. Use KeepAlive to mark pointer input
arguments as live after each function call and at each return.
We do this change only for pointer arguments. Those are the
critical ones to handle because they might have finalizers.
Doing compound arguments (slices, structs, ...) is more complicated
because we would need to track field liveness individually (we do
that for auto variables now, but inputs requires extra trickery).
Turn off the automatic marking of args as live. That way, when args
are explicitly nulled, plive will know that the original argument is
dead.
The KeepAlive op will be the eventual implementation of
runtime.KeepAlive.
Fixes#15277
Change-Id: I5f223e65d99c9f8342c03fbb1512c4d363e903e5
Reviewed-on: https://go-review.googlesource.com/22365
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
In https://golang.org/3210, Transport errors occurring before
receiving response headers were wrapped in another error type to
indicate to the retry logic elsewhere that the request might be
re-tryable. But a check for err == io.EOF was missed, which then became
false once io.EOF was wrapped in the beforeRespHeaderError type.
The beforeRespHeaderError was too fragile. Remove it. I tried to fix
it in an earlier version of this CL and just broke different things
instead.
Also remove the "markBroken" method. It's redundant and confusing.
Also, rename the checkTransportResend method to shouldRetryRequest and
make it return a bool instead of an error. This also helps readability.
Now the code recognizes the two main reasons we'd want to retry a
request: because we never wrote the request in the first place (so:
count the number of bytes we've written), or because the server hung
up on us before we received response headers for an idempotent request.
As an added bonus, this could make POST requests safely re-tryable
since we know we haven't written anything yet. But it's too late in Go
1.7 to enable that, so we'll do that later (filed #15723).
This also adds a new internal (package http) test, since testing this
blackbox at higher levels in transport_test wasn't possible.
Fixes#15446
Change-Id: I2c1dc03b1f1ebdf3f04eba81792bd5c4fb6b6b66
Reviewed-on: https://go-review.googlesource.com/23160
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
in root_cgo_darwin.go only certificates from the System Domain
were being used in FetchPEMRoots. This patch adds support for
getting certificates from all three domains (System, Admin,
User). Also it will only read trusted certificates from those
Keychains. Because it is possible to trust a non Root certificate,
this patch also adds a checks to see if the Subject and Issuer
name are the same.
Fixes#14514
Change-Id: Ia03936d7a61d1e24e99f31c92f9927ae48b2b494
Reviewed-on: https://go-review.googlesource.com/20351
Reviewed-by: Russ Cox <rsc@golang.org>
The fact that crypto/ecdsa.Verify didn't reject negative inputs was a
mistake on my part: I had unsigned numbers on the brain. However, it
doesn't generally cause problems. (ModInverse results in zero, which
results in x being zero, which is rejected.)
The amd64 P-256 code will crash when given a large, negative input.
This fixes both crypto/ecdsa to reject these values and also the P-256
code to ignore the sign of inputs.
Change-Id: I6370ed7ca8125e53225866f55b616a4022b818f8
Reviewed-on: https://go-review.googlesource.com/22093
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Go is being proposed as an officially supported language for elements of
OpenStack:
https://review.openstack.org/#/c/312267/
As such, repos that exist in OpenStack's git infrastructure
are likely to become places from which people might want to go get
things. Allow optional .git suffixes to allow writing code that depends
on git.openstack.org repos that will work with older go versions while
we wait for this support to roll out.
Change-Id: Ia64bdb1dafea33b1c3770803230d30ec1059df22
Reviewed-on: https://go-review.googlesource.com/23135
Reviewed-by: Russ Cox <rsc@golang.org>
sparseSet and sparseMap only need 32 bit integers in their
arrays, since a sparseEntry key is also limited to 32 bits.
This appears to reduce the space allocated for at least
one pathological compilation by 1%, perhaps more.
Not necessarily for 1.7, but it saves a little and is very
low-risk.
Change-Id: Icf1185859e9f5fe1261a206b441e02c34f7d02fd
Reviewed-on: https://go-review.googlesource.com/22972
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
On some systems, gdb is set to: "startup-with-shell on". This
breaks runtime_test. This just make sure gdb does not start by
spawning a shell.
Fixes#15354
Change-Id: Ia040931c61dea22f4fdd79665ab9f84835ecaa70
Reviewed-on: https://go-review.googlesource.com/23142
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously statements like
f(unsafe.Pointer(g()), int(h()))
would be reordered into a sequence of statements like
autotmp_g := g()
autotmp_h := h()
f(unsafe.Pointer(autotmp_g), int(autotmp_h))
which can leave g's temporary value on the stack as a uintptr, rather
than an unsafe.Pointer. Instead, recognize uintptr-to-unsafe.Pointer
conversions when reordering function calls to instead produce:
autotmp_g := unsafe.Pointer(g())
autotmp_h := h()
f(autotmp_g, int(autotmp_h))
Fixes#15329.
Change-Id: I2cdbd89d233d0d5c94791513a9fd5fd958d11ed5
Reviewed-on: https://go-review.googlesource.com/22273
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Looks like some version of Android still fails with "servname not
supported for ai_socktype". It probably doesn't support
ai_socktype=SOCK_STREAM.
Updates #14576.
Change-Id: I77ecff147d5b759e3281b3798c60f150a4aab811
Reviewed-on: https://go-review.googlesource.com/23194
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#14793
Change-Id: I408056d096cd6a999fa5e349704b5ea8e26d2e4e
Reviewed-on: https://go-review.googlesource.com/23201
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The signal might get delivered to a different thread, and that thread
might not run again before the currently running thread returns and
exits. Sleep to give the other thread time to pick up the signal and
crash.
Not tested for all cases, but, optimistically:
Fixes#14063.
Change-Id: Iff58669ac6185ad91cce85e0e86f17497a3659fd
Reviewed-on: https://go-review.googlesource.com/23203
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Update the doc for CreateCertificateRequest
to state that it creates a
`new certificate request`
instead of just a
`new certificate`
Fixes#14649.
Change-Id: Ibbbcf91d74168998990990e78e5272a6cf294d51
Reviewed-on: https://go-review.googlesource.com/23204
Reviewed-by: Russ Cox <rsc@golang.org>
Verify that for a server doing chunked encoding, with the final data
and EOF arriving together, the client will reuse the connection even
if it closes the body without seeing an EOF. The server sends at least
one non-zero chunk and one zero chunk. This verifies that the client's
bufio reading reads ahead and notes the EOF, so even if the JSON
decoder doesn't read the EOF itself, as long as somebody sees it, a
close won't forcible tear down the connection. This was true at least
of https://golang.org/cl/21291
No code change. Test already passed (even with lots of runs, including
in race mode with randomized goroutine scheduling).
Updates #15703
Change-Id: I2140b3eec6b099b6b6e54f153fe271becac5d949
Reviewed-on: https://go-review.googlesource.com/23200
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
racefini calls __tsan_fini which is C code and at the end of it
invoked the standard C library exit(3) call. This has undefined
behavior if invoked more than once. Specifically in C++ programs
it caused static destructors to run twice. At least on glibc
impls it also means the at_exit handlers list (where those are
stored) also free's a list entry when it completes these. So invoking
twice results in a double free at exit which trips debug memory
allocation tracking.
Fix all of this by using an atomic as a boolean barrier around
calls to racefini being invoked > 1 time.
Fixes#15578
Change-Id: I49222aa9b8ded77160931f46434c61a8379570fc
Reviewed-on: https://go-review.googlesource.com/22882
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#15675
Change-Id: I8bad220988e5d690f20804db970b2db037c81187
Reviewed-on: https://go-review.googlesource.com/23086
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The initial implementation of reflect.StructOf in
https://golang.org/cl/9251 had a limitation that field names had to be
ASCII, which was later lifted by https://golang.org/cl/21777. Remove
the out-of-date documentation disallowing UTF-8 field names.
Updates: #5748
Updates: #15064
Change-Id: I2c5bfea46bfd682449c6e847fc972a1a131f51b7
Reviewed-on: https://go-review.googlesource.com/23170
Reviewed-by: David Crawshaw <crawshaw@golang.org>
os.SIGINT is not defined, os.Interrupt or syscall.SIGINT should be used.
Change-Id: I39867726d28e179d1160a4fd353b7bea676c9dbb
Reviewed-on: https://go-review.googlesource.com/23127
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change drops parseInterfaceTable which becomes unnecessary by the
golang.org/x/net/route plumbing.
Change-Id: I05f96e347de950bb1e9292bb3eeff01bb40e292f
Reviewed-on: https://go-review.googlesource.com/23125
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
before this change, when io.MultiReader was called many times but contain few
underlying readers, calls to Read were unnecessarily expensive.
Fixes#13558
Change-Id: I3ec4e88c7b50c075b148331fb1b7348a5840adbe
Reviewed-on: https://go-review.googlesource.com/17873
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Adds a transparent sort to the mime/multipart package, which is
only used in the CreatePart func. This will ensure the ordering
of the MIMEHeader.
The point of this change was to ensure the output would be consistent
and something that could be depended on.
Fixes#13522
Change-Id: I9584ef9dbe98ce97d536d897326914653f8d9ddf
Reviewed-on: https://go-review.googlesource.com/17497
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Issue #15613 points out that the darwin builders have been getting
regular failures in which a process that should exit with a SIGPIPE
signal is instead exiting with exit status 2. The code calls
runtime.raise. On most systems runtime.raise is the equivalent of
pthread_kill(gettid(), sig); that is, it kills the thread with the
signal, which should ensure that the program does not keep going. On
darwin, however, runtime.raise is actually kill(getpid(), sig); that is,
it sends a signal to the entire process. If the process decides to
deliver the signal to a different thread, then it is possible that in
some cases the thread that calls raise is able to execute the next
system call before the signal is actually delivered. That would cause
the observed error.
I have not been able to recreate the problem myself, so I don't know
whether this actually fixes it. But, optimistically:
Fixed#15613.
Change-Id: I60c0a9912aae2f46143ca1388fd85e9c3fa9df1f
Reviewed-on: https://go-review.googlesource.com/23152
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This adds a sparse method for locating nearest ancestors
in a dominator tree, and checks blocks with more than one
predecessor for differences and inserts phi functions where
there are.
Uses reversed post order to cut number of passes, running
it from first def to last use ("last use" for paramout and
mem is end-of-program; last use for a phi input from a
backedge is the source of the back edge)
Includes a cutover from old algorithm to new to avoid paying
large constant factor for small programs. This keeps normal
builds running at about the same time, while not running
over-long on large machine-generated inputs.
Add "phase" flags for ssa/build -- ssa/build/stats prints
number of blocks, values (before and after linking references
and inserting phis, so expansion can be measured), and their
product; the product governs the cutover, where a good value
seems to be somewhere between 1 and 5 million.
Among the files compiled by make.bash, this is the shape of
the tail of the distribution for #blocks, #vars, and their
product:
#blocks #vars product
max 6171 28180 173,898,780
99.9% 1641 6548 10,401,878
99% 463 1909 873,721
95% 152 639 95,235
90% 84 359 30,021
The old algorithm is indeed usually fastest, for 99%ile
values of usually.
The fix to LookupVarOutgoing
( https://go-review.googlesource.com/#/c/22790/ )
deals with some of the same problems addressed by this CL,
but on at least one bug ( #15537 ) this change is still
a significant help.
With this CL:
/tmp/gopath$ rm -rf pkg bin
/tmp/gopath$ time go get -v -gcflags -memprofile=y.mprof \
github.com/gogo/protobuf/test/theproto3/combos/...
...
real 4m35.200s
user 13m16.644s
sys 0m36.712s
and pprof reports 3.4GB allocated in one of the larger profiles
With tip:
/tmp/gopath$ rm -rf pkg bin
/tmp/gopath$ time go get -v -gcflags -memprofile=y.mprof \
github.com/gogo/protobuf/test/theproto3/combos/...
...
real 10m36.569s
user 25m52.286s
sys 4m3.696s
and pprof reports 8.3GB allocated in the same larger profile
With this CL, most of the compilation time on the benchmarked
input is spent in register/stack allocation (cumulative 53%)
and in the sparse lookup algorithm itself (cumulative 20%).
Fixes#15537.
Change-Id: Ia0299dda6a291534d8b08e5f9883216ded677a00
Reviewed-on: https://go-review.googlesource.com/22342
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This should help with debugging failures.
For #15138 and #15477.
Change-Id: I77db2b6375d8b4403d3edf5527899d076291e02c
Reviewed-on: https://go-review.googlesource.com/23134
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The convention for writing something like "64 kB" is 64<<10, since
this is easier to read than 1<<16. Update gcBitsChunkBytes to follow
this convention.
Change-Id: I5b5a3f726dcf482051ba5b1814db247ff3b8bb2f
Reviewed-on: https://go-review.googlesource.com/23132
Reviewed-by: Rick Hudson <rlh@golang.org>
Make clear negotiation can happen via NPN or ALPN, similar to
http.Transport.TLSNextProto and x/net/http2.NextProtoTLS.
Change-Id: Ied00b842bc04e11159d6d2107beda921cefbc6ca
Reviewed-on: https://go-review.googlesource.com/23108
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If client does not provided User-Agent header, do not set default one
used by net/http package when doing request to backend.
Fixes#15524
Change-Id: I9a46bb3b7ec106bc7c3071e235b872d279994d67
Reviewed-on: https://go-review.googlesource.com/23089
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
There was a typo introduced in the initial
implementation of the Plan 9 support of
the mime package.
On Plan 9, the mime type file name should be
/sys/lib/mimetype instead of /sys/lib/mimetypes.
Change-Id: If0f0a9b6f3fbfa8dde551f790e83bdd05e8f0acb
Reviewed-on: https://go-review.googlesource.com/23087
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change makes use of new routing message APIs for BSD variants to
support FreeBSD 11 and newer versions of other BSDs.
Fixes#7849.
Fixes#14724.
Change-Id: I56c7886d6622cdeddd7cc29c8a8062dcc06216d5
Reviewed-on: https://go-review.googlesource.com/22451
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The 17-31 byte code is broken. Disabled it.
Added a bunch of tests to at least cover the cases
in indexShortStr. I'll channel Brad and wonder why
this CL ever got in without any tests.
Fixes#15679
Change-Id: I84a7b283a74107db865b9586c955dcf5f2d60161
Reviewed-on: https://go-review.googlesource.com/23106
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Local.String() returns "Local" on every OS, but windows.
Change windows code to do like others.
Updates #15568
Change-Id: I7a4d2713d940e2a01cff9d7f5cefc89def07546a
Reviewed-on: https://go-review.googlesource.com/23078
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The resource is available over (and redirects to) HTTPS, it seems like a good
idea to save a redirect and ensure an encrypted connection.
Change-Id: I262c7616ae289cdd756b6f67573ba6bd7e3e0ca6
Reviewed-on: https://go-review.googlesource.com/23104
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently the heapBitsSetType documentation says that there are no
races on the heap bitmap, but that isn't exactly true. There are no
*write-write* races, but there are read-write races. Expand the
documentation to explain this and why it's okay.
Change-Id: Ibd92b69bcd6524a40a9dd4ec82422b50831071ed
Reviewed-on: https://go-review.googlesource.com/23092
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently we only execute a publication barrier for scan objects (and
skip it for noscan objects). This used to be okay because GC would
never consult the object itself (so it wouldn't observe uninitialized
memory even if it found a pointer to a noscan object), and the heap
bitmap was pre-initialized to noscan.
However, now we explicitly initialize the heap bitmap for noscan
objects when we allocate them. While the GC will still never consult
the contents of a noscan object, it does need to see the initialized
heap bitmap. Hence, we need to execute a publication barrier to make
the bitmap visible before user code can expose a pointer to the newly
allocated object even for noscan objects.
Change-Id: Ie4133c638db0d9055b4f7a8061a634d970627153
Reviewed-on: https://go-review.googlesource.com/23043
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
By picking up a spurious tFlagExtraStar, the method type was printing
as unc instead of func.
Updates #15673
Change-Id: I0c2c189b99bdd4caeb393693be7520b8e3f342bf
Reviewed-on: https://go-review.googlesource.com/23103
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit 7af2ce3f15.
The commit had a wrong prefix in the description line, probably
misreconginized something. As a result it broke golang.org/x/tools/godoc
and golang.org/x/tools/cmd/godoc like the following:
--- FAIL: TestCLI (10.90s)
--- FAIL: TestWeb (13.74s)
FAIL
FAIL golang.org/x/tools/cmd/godoc 36.428s
--- FAIL: TestCommandLine (0.00s)
FAIL
FAIL golang.org/x/tools/godoc 0.068s
Change-Id: I362a862a4ded8592dec7488a28e7a256adee148f
Reviewed-on: https://go-review.googlesource.com/23076
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The httptrace.ConnectStart and ConnectDone hooks are just about the
post-DNS connection to the host. We were accidentally also firing on
the UDP dials to DNS. Exclude those for now. We can add them back
later as separate hooks if desired. (but they'd only work for pure Go
DNS)
This wasn't noticed earlier because I was developing on a Mac at the
time, which always uses cgo for DNS. When running other tests on
Linux, I started seeing UDP dials.
Updates #12580
Change-Id: I2b2403f2483e227308fe008019f1100f6300250b
Reviewed-on: https://go-review.googlesource.com/23069
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Return an error message instead of eating memory and eventually
triggering a stack overflow.
Fixes#15618
Change-Id: I3dcf1d669104690a17847a20fbfeb6d7e39e8751
Reviewed-on: https://go-review.googlesource.com/23091
Reviewed-by: Rob Pike <r@golang.org>
Re-apply @adg's CL https://golang.org/cl/7129048 that was
previously disabled in https://golang.org/cl/7235052 because
it broke `godoc net/http` for go1.1.
Currently `godoc net/http` seems to work fine with this CL.
Fixes#3428.
Change-Id: I7df06df02fd62dededac6ec60bea62561be59cf1
Reviewed-on: https://go-review.googlesource.com/23013
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Trace viewer cannot handle traces larger than 256MB (limit on js string size):
https://github.com/catapult-project/catapult/issues/627
And even that is problematic (chrome hangs and crashes).
Split large traces into 100MB parts. Somewhat clumsy, but I don't see any other
solution (other than rewriting trace viewer). At least it works reliably now.
Fixes#15482
Change-Id: I993b5f43d22072c6f5bd041ab5888ce176f272b2
Reviewed-on: https://go-review.googlesource.com/22731
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Step 5 was deleted in f3575a9 however the numbering of the other
steps wasn't adjusted accordingly.
While we're here: clean up the whitespace, add curly braces where
appropriate and delete semicolons.
Change-Id: I4e77b2d3ee8460abe4bfb993674f83e35be8ff17
Reviewed-on: https://go-review.googlesource.com/23066
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In future releases of OpenBSD, the sigreturn syscall will no longer
exist. As such, stop using sigreturn on openbsd/386 and just return
from the signal trampoline (as we already do for openbsd/amd64 and
openbsd/arm).
Change-Id: Ic4de1795bbfbfb062a685832aea0d597988c6985
Reviewed-on: https://go-review.googlesource.com/23024
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Minor cleanup. Each of these cases appears both during export and
import when running all.bash and thus is tested by all.bash.
Change-Id: Iaa4a5a5b163cefe33e43d08d396e02a02e5c22a5
Reviewed-on: https://go-review.googlesource.com/23060
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is addressing feedback given on golang.org/cl/23052;
we do it in a separate CL to separate the functional from
the rename change.
ONAME was not used in the export data, but it's the natural node op
where we used OPACK instead. Renamed.
Furthermore, OPACK and ONONAME nodes are replaced by the type checker
with ONAME nodes, so OPACK nodes cannot occur when exporting type-checked
code. Removed a special-case for OPACK nodes since they don't appear.
Change-Id: I78b01a1badbf60e9283eaadeca2578a65d28cbd2
Reviewed-on: https://go-review.googlesource.com/23053
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The pprof tools properly cleans up all files it creates, but forgets
to clean up the temporary directory itself. This CL fixes that.
Fixes#13863
Change-Id: I1151c36cdad5ace7cc97e7e04001cf0149ef0f63
Reviewed-on: https://go-review.googlesource.com/23019
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Include integration test. Confirmed that without the fix, the test case
TestDeleteReadOnly fails.
This permits to revert "cmd/go: reset read-only flag during TestIssue10952"
This reverts commit 3b7841b3af.
Fixes#9606
Change-Id: Ib55c151a8cf1a1da02ab18c34a9b58f615c34254
Reviewed-on: https://go-review.googlesource.com/18235
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The //extern comments are incorrect and cause undefined symbol
errorswhen building cgo code with -compiler=gccgo. The code is already
designed to use weak references, and that support relies on the cgo
check functions being treated as local functions.
Change-Id: Ib38a640cc4ce6eba74cfbf41ba7147ec88769ec0
Reviewed-on: https://go-review.googlesource.com/23014
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Summary: Go's HTTP/1.x server closes the request body once writes are
flushed. Go's HTTP/2 server supports concurrent read & write.
Added a TODO to make the HTTP/1.x server also support concurrent
read+write. But for now, document it.
Updates #15527
Change-Id: I81f7354923d37bfc1632629679c75c06a62bb584
Reviewed-on: https://go-review.googlesource.com/23011
Reviewed-by: Andrew Gerrand <adg@golang.org>
This change reorganizes test cases for surveying network interfaces and
address prefixes to make sure which part of the functionality is broken.
Updates #7849.
Change-Id: If6918075802eef69a7f1ee040010b3c46f4f4b97
Reviewed-on: https://go-review.googlesource.com/22990
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The importer uses a global (shared) package map across multiple imports
to determine if a package was imported before. That package map is usually
indexed by package (import) path ('id' in this code). However, the binary
importer was using the incoming (possibly unclean) path.
Fixes#15517.
Change-Id: I0c32a708dfccf345e0353fbda20ad882121e437c
Reviewed-on: https://go-review.googlesource.com/23012
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The -systemdll and -xsys flags generate broken code in some situations
(see issue for details). Fix all that.
This CL only fixes bugs in existing code, but I have more changes comming:
golang.org/x/sys/windows is not the only package that uses mksyscall_windows.go.
golang.org/x/exp/shiny and github.com/derekparker/delve do too. I also have
few personal packages that use mksyscall_windows.go. None of those packages
are aware of new -xsys flag. I would like to change mksyscall_windows.go, so
external packages do not need to use -xsys flag. I would love to get rid of
-xsys flag altogether, but I don't see how it is possible. So I will, probably,
replace -xsys with a flag that means opposite to -xsys, and use new flag
everywhere in standard libraries. Flag name suggestions are welcome.
-systemdll flag makes users code more "secure". I would like to make -systemdll
behaviour a default for all mksyscall_windows.go users. We use that already in
standard library. If we think "secure" is important, we should encourage it in
all users code. If mksyscall_windows.go user insist on using old code, provide
-use_old_loaddll (need good name here) flag for that. So -systemdll flag will
be replaced with -use_old_loaddll.
Fixes#15167
Change-Id: I516369507867358ba1b66aabe00a17a7b477016e
Reviewed-on: https://go-review.googlesource.com/21645
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Noticed and fix by Alex Brainman.
Tested in https://golang.org/cl/23005 (which makes all compiler
warnings fatal during development)
Fixes#15623
Change-Id: Ic19999fce8bb8640d963965cc328574efadd7855
Reviewed-on: https://go-review.googlesource.com/23010
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Instead of exporting the C function mygetgrouplist as a global symbol to
conflict with other symbols of the same name, use trivial Go code and a
static C function.
Change-Id: I98dd667814d0a0ed8f7b1d4cfc6483d5a6965b26
Reviewed-on: https://go-review.googlesource.com/23008
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Although calls to getaddrinfo can't be portably interrupted,
we still benefit from more granular resource management by
pushing the context downwards.
Fixes#15321
Change-Id: I5506195fc6493080410e3d46aaa3fe02018a24fe
Reviewed-on: https://go-review.googlesource.com/22961
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Progress on SSA backend for ARM. Still not complete. Now "helloworld"
function compiles and runs.
Updates #15365.
Change-Id: I02f66983cefdf07a6aed262fb4af8add464d8e9a
Reviewed-on: https://go-review.googlesource.com/22854
Reviewed-by: Keith Randall <khr@golang.org>
We fixed the implementation of the pread syscall in
the Plan 9 kernel, so calling pread doesn't update the
channel offset when reading a file.
Fixes#11194.
Change-Id: Ie4019e445542a73479728af861a50bb54caea3f6
Reviewed-on: https://go-review.googlesource.com/22245
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the Plan 9 kernel, there used to be a bug in the implementation of
the pread syscall, where the channel offset was erroneously updated after
calling pread on a file.
This test verifies that ReadAt is behaving as expected.
Fixes#14534.
Change-Id: Ifc9fd40a1f94879ee7eb09b2ffc369aa2bec2926
Reviewed-on: https://go-review.googlesource.com/22244
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change makes encoding and decoding support integer types in map
keys, converting to/from JSON string keys.
JSON object keys are still sorted lexically, even though the keys may be
integer strings.
For backwards-compatibility, the existing Text(Un)Marshaler support for
map keys (added in CL 20356) does not take precedence over the default
encoding for string types. There is no such concern for integer types,
so integer map key encoding is only used as a fallback if the map key
type is not a Text(Un)Marshaler.
Fixes#12529.
Change-Id: I7e68c34f9cd19704b1d233a9862da15fabf0908a
Reviewed-on: https://go-review.googlesource.com/22060
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The decryption example for AES-GCM was not executed, hiding the fact
that the provided ciphertext could not be authenticated.
This commit adds the required output comment, replaces the ciphertext
with a working example, and removes an unnecessary string conversion
along the way.
Change-Id: Ie6729ca76cf4a56c48b33fb3b39872105faa604b
Reviewed-on: https://go-review.googlesource.com/22953
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The time package has never depended on the io package until
a recent change during Go 1.7 to use the io.Seek* constants.
The go/build dependency check didn't catch this because "time" was
allowed to depend on meta package group "L0", which included "io".
Adding the "io" package broke one of Dmitry's tools. The tool is
fixable, but it's also not necessary for us to depend on "io" at all
for some constants. Mirror the constants instead, and change
deps_test.go to prevent an io dependency in the future.
Change-Id: I74325228565279a74fa4a2f419643f5710e3e09f
Reviewed-on: https://go-review.googlesource.com/22960
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
After upgrading builder device (android/arm) to android 5.0.2,
the test started failing. Running 'ln -s' from shell fails with
permission error.
Change-Id: I5b9e312806d58532b41ea3560ff079dabbc6424e
Reviewed-on: https://go-review.googlesource.com/22962
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Address two documentation issues:
1) Document that the GZIP and ZLIB footer is only verified when the
reader has been fully consumed.
2) The zlib reader is guaranteed to not read past the EOF if the
input io.Reader is also a io.ByteReader. This functionality was
documented in the flate and gzip packages but not on zlib.
Fixes#14867
Change-Id: I43d46b93e38f98a04901dc7d4f18ed2f9e09f6fb
Reviewed-on: https://go-review.googlesource.com/21218
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In regalloc, a sparse map is preallocated for later use by
spill-in-loop sinking. However, variables (spills) are added
during register allocation before spill sinking, and a map
query involving any of these new variables will index out of
bounds in the map.
To fix:
1) fix the queries to use s.orig[v.ID].ID instead, to ensure
proper indexing. Note that s.orig will be nil for values
that are not eligible for spilling (like memory and flags).
2) add a test.
Fixes#15585.
Change-Id: I8f2caa93b132a0f2a9161d2178320d5550583075
Reviewed-on: https://go-review.googlesource.com/22911
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This flag is experimental and the semantics may change
even after Go 1.7 is released. There are no changes to code
not using the flag.
The first part is for reading by future compiles.
The second part is for reading by the final link step.
Splitting the file this way allows distributed build systems
to ship the compile-input part only to compile steps and
the linker-input part only to linker steps.
The first part is basically just the export data,
and the second part is basically everything else.
The overall files still have the same broad structure,
so that existing tools will work with both halves.
It's just that various pieces are empty in the two halves.
This also copies the two bits of data the linker needed from
export data into the object header proper, so that the linker
doesn't need any export data at all. That eliminates a TODO
that was left for switching to the binary export data.
(Now the linker doesn't need to know about the switch.)
The default is still to write out a combined output file.
Nothing changes unless you pass -linkobj to the compiler.
There is no support in the go command for -linkobj,
since the go command doesn't copy objects around.
The expectation is that other build systems (like bazel, say)
might take advantage of this.
The header adjustment and the option for the split output
was intended as part of the zip archives, but the zip archives
have been cut from Go 1.7. Doing this to the current archives
both unblocks one step in the switch to binary export data
and enables alternate build systems to experiment with the
new flag using the Go 1.7 release.
Change-Id: I8b6eab25b8a22b0a266ba0ac6d31e594f3d117f3
Reviewed-on: https://go-review.googlesource.com/22500
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The test sometimes fails on builders.
The test uses sleeps to establish the necessary goroutine
execution order. If sleeps undersleep/oversleep
the race is still reported, but it can be reported when the
main test goroutine returns. In such case test driver
can't match the race with the test and reports failure.
Wait for both test goroutines to ensure that the race
is reported in the test scope.
Fixes#15579
Change-Id: I0b9bec0ebfb0c127d83eb5325a7fe19ef9545050
Reviewed-on: https://go-review.googlesource.com/22951
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently 386 ELF binaries are generated with dynamic symbols that have
a size of zero bytes, even though the symbol in the symbol table has
the correct size. Fix this by specifying the correct size when creating
dynamic symbols.
Issue found on OpenBSD -current, where ld.so is now producing link
warnings due to mismatched symbol sizes.
Fixes#15593.
Change-Id: Ib1a12b23ff9159c61ac980bf48a983b86f3df256
Reviewed-on: https://go-review.googlesource.com/22912
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new export format keeps track of all types that are exported.
If a type is seen that was exported before, only a reference to
that type is emitted. The importer maintains a list of all the
seen types and uses that list to resolve type references.
The existing compiler infrastructure's invariants assumes that
only named types are referred to before they are fully set up.
Referring to unnamed incomplete types causes problems. One of
the issues was #15548.
Added a new internal flag 'trackAllTypes' to enable/disable
this type tracking. With this change only named types are
tracked.
Verified that this fix also addresses #15548, even w/o the
prior fix for that issue (in fact that prior fix is turned
off if trackAllTypes is disabled because it's not needed).
The test for #15548 covers also this change.
For #15548.
Change-Id: Id0b3ff983629703d025a442823f99649fd728a56
Reviewed-on: https://go-review.googlesource.com/22839
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Buildmode c-archive now supports position independent code for
darwin/arm (in addition to darwin/arm64). Make PIC (-shared) the
default for both platforms in the default buildmode.
Without this change, gomobile will go install the standard library
into its separate package directory without PIC support.
Also add -shared to darwin/arm64 in buildmode c-archive, for
symmetry (darwin/arm64 always generates position independent code).
Fixes#15519
Change-Id: If27d2cbea8f40982e14df25da2703cbba572b5c6
Reviewed-on: https://go-review.googlesource.com/22920
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The boolean destination in an OAS2DOTTYPE expression craps out during
compilation when trying to assign to a map entry because, unlike slice entries,
map entries are not directly addressable in memory. The solution is to
properly order the boolean destination node so that map entries are set
via autotmp variables.
Fixes#14678
Change-Id: If344e8f232b5bdac1b53c0f0d21eeb43ab17d3de
Reviewed-on: https://go-review.googlesource.com/22833
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In issue #13992, Russ mentioned that the heap bitmap footprint was
halved but that the bitmap size calculation hadn't been updated. This
presents the opportunity to either halve the bitmap size or double
the addressable virtual space. This CL doubles the addressable virtual
space. On 32 bit this can be tweaked further to allow the bitmap to
cover the entire 4GB virtual address space, removing a failure mode
if the kernel hands out memory with a too low address.
First, fix the calculation and double _MaxArena32 to cover 4GB virtual
memory space with the same bitmap size (256 MB).
Then, allow the fallback mode for the initial memory reservation
on 32 bit (or 64 bit with too little available virtual memory) to not
include space for the arena. mheap.sysAlloc will automatically reserve
additional space when the existing arena is full.
Finally, set arena_start to 0 in 32 bit mode, so that any address is
acceptable for subsequent (additional) reservations.
Before, the bitmap was always located just before arena_start, so
fix the two places relying on that assumption: Point the otherwise unused
mheap.bitmap to one byte after the end of the bitmap, and use it for
bitmap addressing instead of arena_start.
With arena_start set to 0 on 32 bit, the cgoInRange check is no longer a
sufficient check for Go pointers. Introduce and call inHeapOrStack to
check whether a pointer is to the Go heap or stack.
While we're here, remove sysReserveHigh which seems to be unused.
Fixes#13992
Change-Id: I592b513148a50b9d3967b5c5d94b86b3ec39acc2
Reviewed-on: https://go-review.googlesource.com/20471
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Might deflake the occasional linux-amd64-race failures.
Change-Id: I273b0e32bb92236168eb99887b166e079799c1f1
Reviewed-on: https://go-review.googlesource.com/22858
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: Ice9c234960adc7857c8370b777a0b18e29d59281
Reviewed-on: https://go-review.googlesource.com/22853
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We run the external network tests on builders, but some of our
builders have less-than-ideal DNS connectivity. This change continues
to run the tests on all builders, but marks certain builders as flaky
(network-wise), and only validates their DNS results if they got DNS
results.
Change-Id: I826dc2a6f6da55add89ae9c6db892b3b2f7b526b
Reviewed-on: https://go-review.googlesource.com/22852
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I meant to delete these in CL 22850, actually.
Change-Id: I0c286efd2b9f1caf0221aa88e3bcc03649c89517
Reviewed-on: https://go-review.googlesource.com/22851
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is the follow-on to CL 22610: now that it's the child instead of
the parent which lists unwanted fds to close in syscall.StartProcess,
plan9 no longer needs the ForkLock to protect the list from changing.
The readdupdevice function is also now unused and can be removed.
Change-Id: I904c8bbf5dbaa7022b0f1a1de0862cd3064ca8c7
Reviewed-on: https://go-review.googlesource.com/22842
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#12272
Change-Id: I2115ec62ed4061084c482eb385a583a1c1909888
Reviewed-on: https://go-review.googlesource.com/22838
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
The Reader and Writer have hard-coded constants regarding the
offsets and lengths of certain fields in the tar format sprinkled
all over. This makes it harder to verify that the offsets are
correct since a reviewer would need to search for them throughout
the code. Instead, all information about the layout of header
fields should be centralized in one single file. This has the
advantage of being both centralized, and also acting as a form
of documentation about the header struct format.
This method was chosen over using "encoding/binary" since that
method would cause an allocation of a header struct every time
binary.Read was called. This method causes zero allocations and
its logic is no longer than if structs were declared.
Updates #12594
Change-Id: Ic7a0565d2a2cd95d955547ace3b6dea2b57fab34
Reviewed-on: https://go-review.googlesource.com/14669
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This can only happen when profiling and there is foreign code
at the top of the g0 stack but we're not in cgo.
That in turn only happens with the race detector.
Fixes#13568.
Change-Id: I23775132c9c1a3a3aaae191b318539f368adf25e
Reviewed-on: https://go-review.googlesource.com/18322
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL/19862 (f79b50b8d5) recently introduced the constants
SeekStart, SeekCurrent, and SeekEnd to the io package. We should use these constants
consistently throughout the code base.
Updates #15269
Change-Id: If7fcaca7676e4a51f588528f5ced28220d9639a2
Reviewed-on: https://go-review.googlesource.com/22097
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Negative-case conversion code was wrong for minimum int32,
used negate-then-widen instead of widen-then-negate.
Test already exists; this fixes the failure.
Fixes#15563.
Change-Id: I4b0b3ae8f2c9714bdcc405d4d0b1502ccfba2b40
Reviewed-on: https://go-review.googlesource.com/22830
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This test should now succeed after CL 22610 which fixes issue #7118
Change-Id: Ie785a84d77b27c832a1ddd81699bf25dab24b97d
Reviewed-on: https://go-review.googlesource.com/22640
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviving earlier work by @ality in https://golang.org/cl/57890043
to make the closing of extra file descriptors in syscall.StartProcess
less race-prone. Instead of making a list of open fds in the parent
before forking, the child can read through the list of open fds and
close the ones not explicitly requested. Also eliminate the
complication of keeping open any extra fds which were inherited by
the parent when it started.
This CL will be followed by one to eliminate the ForkLock in plan9,
which is now redundant.
Fixes#5605
Change-Id: I6b4b942001baa54248b656c52dced3b62021c486
Reviewed-on: https://go-review.googlesource.com/22610
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
So we can start working on other architectures here.
Change is a dummy to keep git happy.
Change-Id: I1caa62a242790601810a1ff72af7ea9773d4da76
Reviewed-on: https://go-review.googlesource.com/22822
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Adds a small function signame that infers a signal name
from the signal table, otherwise will fallback to using
hex(sig) as previously. No signal table is present for
Windows hence it will always print the hex value.
Sample code and new result:
```go
package main
import (
"fmt"
"time"
)
func main() {
defer func() {
if err := recover(); err != nil {
fmt.Printf("err=%v\n", err)
}
}()
ticker := time.Tick(1e9)
for {
<-ticker
}
}
```
```shell
$ go run main.go &
$ kill -11 <pid>
fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0xb01dfacedebac1e
pc=0xc71db]
...
```
Fixes#13969
Change-Id: Ie6be312eb766661f1cea9afec352b73270f27f9d
Reviewed-on: https://go-review.googlesource.com/22753
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The following performance improvements have been made to the
low-level atomic functions for ppc64le & ppc64:
- For those cases containing a lwarx and stwcx (or other sizes):
sync, lwarx, maybe something, stwcx, loop to sync, sync, isync
The sync is moved before (outside) the lwarx/stwcx loop, and the
sync after is removed, so it becomes:
sync, lwarx, maybe something, stwcx, loop to lwarx, isync
- For the Or8 and And8, the shifting and manipulation of the
address to the word aligned version were removed and the
instructions were changed to use lbarx, stbcx instead of
register shifting, xor, then lwarx, stwcx.
- New instructions LWSYNC, LBAR, STBCC were tested and added.
runtime/atomic_ppc64x.s was changed to use the LWSYNC opcode
instead of the WORD encoding.
Fixes#15469
Ran some of the benchmarks in the runtime and sync directories.
Some results varied from run to run but the trend was improvement
based on best times for base and new:
runtime.test:
BenchmarkChanNonblocking-128 0.88 0.89 +1.14%
BenchmarkChanUncontended-128 569 511 -10.19%
BenchmarkChanContended-128 63110 53231 -15.65%
BenchmarkChanSync-128 691 598 -13.46%
BenchmarkChanSyncWork-128 11355 11649 +2.59%
BenchmarkChanProdCons0-128 2402 2090 -12.99%
BenchmarkChanProdCons10-128 1348 1363 +1.11%
BenchmarkChanProdCons100-128 1002 746 -25.55%
BenchmarkChanProdConsWork0-128 2554 2720 +6.50%
BenchmarkChanProdConsWork10-128 1909 1804 -5.50%
BenchmarkChanProdConsWork100-128 1624 1580 -2.71%
BenchmarkChanCreation-128 237 212 -10.55%
BenchmarkChanSem-128 705 667 -5.39%
BenchmarkChanPopular-128 5081190 4497566 -11.49%
BenchmarkCreateGoroutines-128 532 473 -11.09%
BenchmarkCreateGoroutinesParallel-128 35.0 34.7 -0.86%
BenchmarkCreateGoroutinesCapture-128 4923 4200 -14.69%
sync.test:
BenchmarkUncontendedSemaphore-128 112 94.2 -15.89%
BenchmarkContendedSemaphore-128 133 128 -3.76%
BenchmarkMutexUncontended-128 1.90 1.67 -12.11%
BenchmarkMutex-128 353 310 -12.18%
BenchmarkMutexSlack-128 304 283 -6.91%
BenchmarkMutexWork-128 554 541 -2.35%
BenchmarkMutexWorkSlack-128 567 556 -1.94%
BenchmarkMutexNoSpin-128 275 242 -12.00%
BenchmarkMutexSpin-128 1129 1030 -8.77%
BenchmarkOnce-128 1.08 0.96 -11.11%
BenchmarkPool-128 29.8 27.4 -8.05%
BenchmarkPoolOverflow-128 40564 36583 -9.81%
BenchmarkSemaUncontended-128 3.14 2.63 -16.24%
BenchmarkSemaSyntNonblock-128 1087 1069 -1.66%
BenchmarkSemaSyntBlock-128 897 893 -0.45%
BenchmarkSemaWorkNonblock-128 1034 1028 -0.58%
BenchmarkSemaWorkBlock-128 949 886 -6.64%
Change-Id: I4403fb29d3cd5254b7b1ce87a216bd11b391079e
Reviewed-on: https://go-review.googlesource.com/22549
Reviewed-by: Michael Munday <munday@ca.ibm.com>
Reviewed-by: Minux Ma <minux@golang.org>
If b has exactly one predecessor, as happens
frequently with static calls, we can make
lookupVarOutgoing generate less garbage.
Instead of generating a value that is just
going to be an OpCopy and then get eliminated,
loop. This can lead to lots of looping.
However, this loop is way cheaper than generating
lots of ssa.Values and then eliminating them.
For a subset of the code in #15537:
Before:
28.31 real 36.17 user 1.68 sys
2282450944 maximum resident set size
After:
9.63 real 11.66 user 0.51 sys
638144512 maximum resident set size
Updates #15537.
Excitingly, it appears that this also helps
regular code:
name old time/op new time/op delta
Template 288ms ± 6% 276ms ± 7% -4.13% (p=0.000 n=21+24)
Unicode 143ms ± 8% 141ms ±10% ~ (p=0.287 n=24+25)
GoTypes 932ms ± 4% 874ms ± 4% -6.20% (p=0.000 n=23+22)
Compiler 4.89s ± 4% 4.58s ± 4% -6.46% (p=0.000 n=22+23)
MakeBash 40.2s ±13% 39.8s ± 9% ~ (p=0.648 n=23+23)
name old user-ns/op new user-ns/op delta
Template 388user-ms ±10% 373user-ms ± 5% -3.80% (p=0.000 n=24+25)
Unicode 203user-ms ± 6% 202user-ms ± 7% ~ (p=0.492 n=22+24)
GoTypes 1.29user-s ± 4% 1.17user-s ± 4% -9.67% (p=0.000 n=25+23)
Compiler 6.86user-s ± 5% 6.28user-s ± 4% -8.49% (p=0.000 n=25+25)
name old alloc/op new alloc/op delta
Template 51.5MB ± 0% 47.6MB ± 0% -7.47% (p=0.000 n=22+25)
Unicode 37.2MB ± 0% 37.1MB ± 0% -0.21% (p=0.000 n=25+25)
GoTypes 166MB ± 0% 138MB ± 0% -16.83% (p=0.000 n=25+25)
Compiler 756MB ± 0% 628MB ± 0% -16.96% (p=0.000 n=25+23)
name old allocs/op new allocs/op delta
Template 450k ± 0% 445k ± 0% -1.02% (p=0.000 n=25+25)
Unicode 356k ± 0% 356k ± 0% ~ (p=0.374 n=24+25)
GoTypes 1.31M ± 0% 1.25M ± 0% -4.18% (p=0.000 n=25+25)
Compiler 5.29M ± 0% 5.02M ± 0% -5.15% (p=0.000 n=25+23)
It also seems to help in other cases in which
phi insertion is a pain point (#14774, #14934).
Change-Id: Ibd05ed7b99d262117ece7bb250dfa8c3d1cc5dd2
Reviewed-on: https://go-review.googlesource.com/22790
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
The legacy mips64 backend doesn't handle large uint->float conversion
correctly. See #15552.
Change-Id: I84ceeaa95cc4e85f09cc46dfb30ab5d151f6b205
Reviewed-on: https://go-review.googlesource.com/22800
Reviewed-by: Minux Ma <minux@golang.org>
Builder is too slow. This test passed on builder machines but took
15+ min.
Change-Id: Ief9d67ea47671a57e954e402751043bc1ce09451
Reviewed-on: https://go-review.googlesource.com/22798
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I overlooked it when rebasing CL 19803.
Change-Id: Ife9d6bcc6a772715d137af903c64bafac0cdb216
Reviewed-on: https://go-review.googlesource.com/22797
Reviewed-by: Minux Ma <minux@golang.org>
Since tracebackctxt.go uses //export functions, the C functions can't be
externally visible in the C comment. The code was using attributes to
work around that, but that failed on Windows.
Change-Id: If4449fd8209a8998b4f6855ea89e5db1471b2981
Reviewed-on: https://go-review.googlesource.com/22786
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CLs 22181, 22332 and 22336 intorduced new functionality to be used
in cmd/link (see issue #15345 for details). But we didn't have chance
to use new functionality yet. Unexport newly introduced identifiers,
so we don't have to commit to the API until we actually tried it.
Rename File.COFFSymbols into File._COFFSymbols,
COFFSymbol.FullName into COFFSymbol._FullName,
Section.Relocs into Section._Relocs,
Reloc into _Relocs,
File.StringTable into File._StringTable and
StringTable into _StringTable.
Updates #15345
Change-Id: I770eeb61f855de85e0c175225d5d1c006869b9ec
Reviewed-on: https://go-review.googlesource.com/22720
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Adds the FCFIDU instruction and uses it instead of the FCFID
instruction for unsigned integer to float casts. This change means
that unsigned integers do not have to be cast to signed integers
before being cast to a floating point value. Therefore it is no
longer necessary to insert instructions to detect and fix
values that overflow int64.
The previous code generating the uint64 to int64 cast handled
overflow by truncating the uint64 value. This truncation can
change the result of the rounding performed by the integer to
float cast.
The FCFIDU instruction was added in Power ISA 2.06B.
Fixes#15539.
Change-Id: Ia37a9631293eff91032d4cd9a9bec759d2142437
Reviewed-on: https://go-review.googlesource.com/22772
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
debug/elf is only needed to determine the endianness of the host
machine, which is easy to do without debug/elf.
Fixes#15180.
Change-Id: I21035ed3884871270765a1ca3b812a5d4890a7ee
Reviewed-on: https://go-review.googlesource.com/21662
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
To speed up the ssacheck check builder and make it on by default as a
trybot.
Change-Id: I91a3347491507c84f4878dff744ca426ba3e2e9f
Reviewed-on: https://go-review.googlesource.com/22755
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
external linking is now supported.
Change-Id: I3f552f5f09391205fced509fe8a5a38297ea8153
Reviewed-on: https://go-review.googlesource.com/19810
Reviewed-by: Minux Ma <minux@golang.org>
Fixes#14544
Change-Id: I58b0b164ebbfeafe4ab32039a063df53e3018a6d
Reviewed-on: https://go-review.googlesource.com/22730
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Sean Lake <odysseus9672@gmail.com>
Similar to the flate Writer pools already used,
this adds pooling for flate Readers.
compress/flate allows re-using of Readers, see
https://codereview.appspot.com/97140043/
In a real-world scenario when reading ~ 500 small files from a ZIP
archive this gives a speedup of 1.5x-2x.
Fixes#14289
Change-Id: I2d98ad983e95ab7d97e06fd0145f619b4f47caa4
Reviewed-on: https://go-review.googlesource.com/19416
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Consider three shared libraries:
libBase.so -- defines a type T
lib2.so -- references type T
lib3.so -- also references type T, and something from lib2
lib2.so will contain a type symbol for T in its symbol table, but no
definition. If, when linking lib3.so the linker reads the symbols from lib2.so
before libBase.so, the linker didn't read the type data and later crashed.
The fix is trivial but the test change is a bit messy because the order the
linker reads the shared libraries in ends up depending on the order of the
import statements in the file so I had to rename one of the test packages so
that gofmt doesn't fix the test by accident...
Fixes#15516
Change-Id: I124b058f782c900a3a54c15ed66a0d91d0cde5ce
Reviewed-on: https://go-review.googlesource.com/22744
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Why not? Because the 386 backend can't handle one of them.
But other than that, it should work.
Change-Id: Iaeb9735f8c3c281136a0734376dec5ddba21be3b
Reviewed-on: https://go-review.googlesource.com/22748
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Change https://golang.org/cl/8945 allowed Go to use its own DNS resolver
instead of libc in a number of cases. The code parses nsswitch.conf and
attempts to resolve things in the same order. Unfortunately, builds with
netgo completely ignore this parsing and always search via
hostLookupFilesDNS.
This commit modifies the logic to allow binaries built with netgo to
parse nsswitch.conf and attempt to resolve using the order specified
there. If the parsing results in hostLookupCGo, it falls back to the
original hostLookupFilesDNS. Tests are also added to ensure that both
the parsing and the fallback work properly.
Fixes#14354
Change-Id: Ib079ad03d7036a4ec57f18352a15ba55d933f261
Reviewed-on: https://go-review.googlesource.com/19523
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If we collected a cgo traceback when entering the SIGPROF signal
handler, record it as part of the profiling stack trace.
This serves as the promised test for https://golang.org/cl/21055 .
Change-Id: I5f60cd6cea1d9b7c3932211483a6bfab60ed21d2
Reviewed-on: https://go-review.googlesource.com/22650
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
It never makes sense to CSE two ops that generate memory.
We might as well start those ops off in their own partition.
Fixes#15520
Change-Id: I0091ed51640f2c10cd0117f290b034dde7a86721
Reviewed-on: https://go-review.googlesource.com/22741
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
1) Blank parameters cannot be accessed so the package doesn't matter.
Do not export it, and consistently use localpkg when importing a
blank parameter.
2) More accurately replicate fmt.go and parser.go logic when importing
a blank struct field. Blank struct fields get exported without
package qualification.
(This is actually incorrect, even with the old textual export format,
but we will fix that in a separate change. See also issue 15514.)
Fixes#15491.
Change-Id: I7978e8de163eb9965964942aee27f13bf94a7c3c
Reviewed-on: https://go-review.googlesource.com/22714
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
1.7 traces embed symbol info and we now generate symbolized pprof profiles,
so we don't need the binary. Make binary argument optional as 1.5 traces
still need it.
Change-Id: I65eb13e3d20ec765acf85c42d42a8d7aae09854c
Reviewed-on: https://go-review.googlesource.com/22410
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Race runtime also needs local malloc caches and currently uses
a mix of per-OS-thread and per-goroutine caches. This leads to
increased memory consumption. But more importantly cache of
synchronization objects is per-goroutine and we don't always
have goroutine context when feeing memory in GC. As the result
synchronization object descriptors leak (more precisely, they
can be reused if another synchronization object is recreated
at the same address, but it does not always help). For example,
the added BenchmarkSyncLeak has effectively runaway memory
consumption (based on a real long running server).
This change updates race runtime with support for per-P contexts.
BenchmarkSyncLeak now stabilizes at ~1GB memory consumption.
Long term, this will allow us to remove race runtime dependency
on glibc (as malloc is the main cornerstone).
I've also implemented a different scheme to pass P context to
race runtime: scheduler notified race runtime about association
between G and P by calling procwire(g, p)/procunwire(g, p).
But it turned out to be very messy as we have lots of places
where the association changes (e.g. syscalls). So I dropped it
in favor of the current scheme: race runtime asks scheduler
about the current P.
Fixes#14533
Change-Id: Iad10d2f816a44affae1b9fed446b3580eafd8c69
Reviewed-on: https://go-review.googlesource.com/19970
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Runqempty is a critical predicate for scheduler. If runqempty spuriously
returns true, then scheduler can fail to schedule arbitrary number of
runnable goroutines on idle Ps for arbitrary long time. With the addition
of runnext runqempty predicate become broken (can spuriously return true).
Consider that runnext is not nil and the main array is empty. Runqempty
observes that the array is empty, then it is descheduled for some time.
Then queue owner pushes another element to the queue evicting runnext
into the array. Then queue owner pops runnext. Then runqempty resumes
and observes runnext is nil and returns true. But there were no point
in time when the queue was empty.
Fix runqempty predicate to not return true spuriously.
Change-Id: Ifb7d75a699101f3ff753c4ce7c983cf08befd31e
Reviewed-on: https://go-review.googlesource.com/20858
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
GCC, unlike clang, does not provide any way for code being compiled to tell if
-fsanitize-thread was passed. But cgo can look to see if that flag is being
passed and generate different code in that case.
Fixes#14602
Change-Id: I86cb5318c2e35501ae399618c05af461d1252d2d
Reviewed-on: https://go-review.googlesource.com/22688
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The format has been tweaked several times in the latest cycle, so
replace go13ld with go17ld.
Change-Id: I343c49b02b7516fd781bc96ad46640579da68c59
Reviewed-on: https://go-review.googlesource.com/22708
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I got a complaint that cgo output triggers warnings with
-Wdeclaration-after-statement. I don't think it's worth testing for
this--C has permitted declarations after statements since C99--but it is
easy enough to fix. It may break again; so it goes.
This CL also fixes errno handling to avoid getting confused if the tsan
functions happen to change the global errno variable.
Change-Id: I0ec7c63a6be5653ef44799d134c8d27cb5efa441
Reviewed-on: https://go-review.googlesource.com/22686
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Turn SSAing of variables off when compiling with optimizations off.
This helps keep variable names around that would otherwise be
optimized away.
Fixes#14744
Change-Id: I31db8cf269c068c7c5851808f13e5955a09810ca
Reviewed-on: https://go-review.googlesource.com/22681
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
:= is the wrong thing here. The new variable masks the old
variable so we allocate the slice afresh each time around the loop.
Change-Id: I759c30e1bfa88f40decca6dd7d1e051e14ca0844
Reviewed-on: https://go-review.googlesource.com/22679
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
The Transport's automatic gzip uncompression lost information in the
process (the compressed Content-Length, if known). Normally that's
okay, but it's not okay for reverse proxies which have to be able to
generate a valid HTTP response from the Transport's provided
*Response.
Reverse proxies should normally be disabling compression anyway and
just piping the compressed pipes though and not wasting CPU cycles
decompressing them. So also document that on the new Uncompressed
field.
Then, using the new field, fix Response.Write to not inject a bogus
"Connection: close" header when it doesn't see a transfer encoding or
content-length.
Updates #15366 (the http2 side remains, once this is submitted)
Change-Id: I476f40aa14cfa7aa7b3bf99021bebba4639f9640
Reviewed-on: https://go-review.googlesource.com/22671
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds a context key named LocalAddrContextKey (for now, see #15229) to
let users access the net.Addr of the net.Listener that accepted the connection
that sent an HTTP request. This is similar to ServerContextKey which provides
access to the *Server. (A Server may have multiple Listeners)
Fixes#6732
Change-Id: I74296307b68aaaab8df7ad4a143e11b5227b5e62
Reviewed-on: https://go-review.googlesource.com/22672
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Don't keep idle HTTP client connections open forever. Add a new knob,
Transport.IdleConnTimeout, and make the default be 90 seconds. I
figure 90 seconds is more than a minute, and less than infinite, and I
figure enough code has things waking up once a minute polling APIs.
This also removes the Transport's idleCount field which was unused and
redundant with the size of the idleLRU map (which was actually used).
Change-Id: Ibb698a9a9a26f28e00a20fe7ed23f4afb20c2322
Reviewed-on: https://go-review.googlesource.com/22670
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Before this CL:
$ go test -bench=CompressedZipGarbage -count=5 -run=NONE archive/zip
BenchmarkCompressedZipGarbage-8 50 20677087 ns/op 42973 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 100 20584764 ns/op 24294 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 50 20859221 ns/op 42973 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 100 20901176 ns/op 24294 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 50 21282409 ns/op 42973 B/op 47 allocs/op
The B/op number is effectively meaningless. There
is a surprisingly large one-time cost that gets
divided by the number of iterations that your
machine can get through in a second.
This CL discards the first run, which helps.
It is not a panacea. Running with -benchtime=10s
will allow the sync.Pool to be emptied,
which brings the problem back.
However, since there are more iterations to divide
the cost through, it’s not quite as bad,
and running with a high benchtime is rare.
This CL changes the meaning of the B/op number,
which is unfortunate, since it won’t have the
same order of magnitude as previous Go versions.
But it wasn’t really comparable before anyway,
since it didn’t have any reliable meaning at all.
After this CL:
$ go test -bench=CompressedZipGarbage -count=5 -run=NONE archive/zip
BenchmarkCompressedZipGarbage-8 100 20881890 ns/op 5616 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 50 20622757 ns/op 5616 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 50 20628193 ns/op 5616 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 100 20756612 ns/op 5616 B/op 47 allocs/op
BenchmarkCompressedZipGarbage-8 100 20639774 ns/op 5616 B/op 47 allocs/op
Change-Id: Iedee04f39328974c7fa272a6113d423e7ffce50f
Reviewed-on: https://go-review.googlesource.com/22585
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
a new relocation R_ADDRMIPSTLS is added, which resolves to 16-bit offset
of a TLS address on mips64x.
Change-Id: Ic60d0e1ba49ff1c433cead242f5884677ab227a5
Reviewed-on: https://go-review.googlesource.com/19804
Reviewed-by: Minux Ma <minux@golang.org>
This updates some comments that became out of date when we moved the
mark bit out of the heap bitmap and started using the high bit for the
first word as a scan/dead bit.
Change-Id: I4a572d16db6114cadff006825466c1f18359f2db
Reviewed-on: https://go-review.googlesource.com/22662
Reviewed-by: Rick Hudson <rlh@golang.org>
MIPS N64 ABI passes arguments in registers R4-R11, return value in R2.
R16-R23, R28, R30 and F24-F31 are callee-save. gcc PIC code expects
to be called with indirect call through R25.
Change-Id: I24f582b4b58e1891ba9fd606509990f95cca8051
Reviewed-on: https://go-review.googlesource.com/19805
Reviewed-by: Minux Ma <minux@golang.org>
Factor out the Aux/AuxInt handling in (*Value).LongString() and
use it in (*Value).LongHTML() as well.
This especially improves readability of auxFloat32, auxFloat64,
and auxSymValAndOff values which would otherwise be printed as
opaque integers.
This change also makes LongString() slightly less verbose by
eliding offsets that are zero (as is very often the case).
Additionally, ensure the HTML is interpreted as UTF-8 so that
non-ASCII characters (especially the "middle dots" in some symbols)
show up correctly.
Change-Id: Ie26221df876faa056d322b3e423af63f33cd109d
Reviewed-on: https://go-review.googlesource.com/22641
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Frits van Bommel <fvbommel@gmail.com>
SB register (R28) is introduced for access external addresses with shorter
instruction sequences. It is loaded at entry points. External data within
2G of SB can be accessed this way.
cmd/internal/obj: relocaltion R_ADDRMIPS is split into two relocations
R_ADDRMIPS and R_ADDRMIPSU, handling the low 16 bits and the "upper" 16
bits of external addresses, respectively, since the instructios may not
be adjacent. It might be better if relocation Variant could be used.
cmd/link/internal/mips64: support new relocations.
cmd/compile/internal/mips64: reserve SB register.
runtime: initialize SB register at entry points.
Change-Id: I5f34868f88c5a9698c042a8a1f12f76806c187b9
Reviewed-on: https://go-review.googlesource.com/19802
Reviewed-by: Minux Ma <minux@golang.org>
Leave R28 to SB register, which will be introduced in CL 19802.
Change-Id: I1cf7a789695c5de664267ec8086bfb0b043ebc14
Reviewed-on: https://go-review.googlesource.com/19863
Reviewed-by: Minux Ma <minux@golang.org>
on mips64, address is 64 bit, not a WORD. also it is never used anywhere.
Change-Id: Ic6bf6d6a21c8d2f1eb7bfe9efc5a29186ec2a8ef
Reviewed-on: https://go-review.googlesource.com/19801
Reviewed-by: Minux Ma <minux@golang.org>
The HTTP client had a limit for the maximum number of idle connections
per-host, but not a global limit.
This CLs adds a global idle connection limit too,
Transport.MaxIdleConns.
All idle conns are now also stored in a doubly-linked list. When there
are too many, the oldest one is closed.
Fixes#15461
Change-Id: I72abbc28d140c73cf50f278fa70088b45ae0deef
Reviewed-on: https://go-review.googlesource.com/22655
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Clarify that it includes the RFC 7230 "request-line".
Fixes#15494
Change-Id: I9cc5dd5f2d85ebf903229539208cec4da5c38d04
Reviewed-on: https://go-review.googlesource.com/22656
Reviewed-by: Andrew Gerrand <adg@golang.org>
Previously named byte types like json.RawMessage could get dirty
database memory from a call to Scan. These types would activate a
code path that didn't clone the byte data coming from the database
before assigning it. Another thread could then overwrite the byte
array in src, which has unexpected consequences.
Originally reported by Jason Moiron; the patch and test are his
suggestions. Fixes#13905.
Change-Id: Iacfef61cbc9dd51c8fccef9b2b9d9544c77dd0e0
Reviewed-on: https://go-review.googlesource.com/22393
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
With the switch to separate mark bitmaps, the scan/dead bit for the
first word of each object is now unused. Reclaim this bit and use it
as a scan/dead bit, just like words three and on. The second word is
still used for checkmark.
This dramatically simplifies heapBitsSetTypeNoScan and hasPointers,
since they no longer need different cases for 1, 2, and 3+ word
objects. They can instead just manipulate the heap bitmap for the
first word and be done with it.
In order to enable this, we change heapBitsSetType and runGCProg to
always set the scan/dead bit to scan for the first word on every code
path. Since these functions only apply to types that have pointers,
there's no need to do this conditionally: it's *always* necessary to
set the scan bit in the first word.
We also change every place that scans an object and checks if there
are more pointers. Rather than only checking morePointers if the word
is >= 2, we now check morePointers if word != 1 (since that's the
checkmark word).
Looking forward, we should probably reclaim the checkmark bit, too,
but that's going to be quite a bit more work.
Tested by setting doubleCheck in heapBitsSetType and running all.bash
on both linux/amd64 and linux/386, and by running GOGC=10 all.bash.
This particularly improves the FmtFprintf* go1 benchmarks, since they
do a large amount of noscan allocation.
name old time/op new time/op delta
BinaryTree17-12 2.34s ± 1% 2.38s ± 1% +1.70% (p=0.000 n=17+19)
Fannkuch11-12 2.09s ± 0% 2.09s ± 1% ~ (p=0.276 n=17+16)
FmtFprintfEmpty-12 44.9ns ± 2% 44.8ns ± 2% ~ (p=0.340 n=19+18)
FmtFprintfString-12 127ns ± 0% 125ns ± 0% -1.57% (p=0.000 n=16+15)
FmtFprintfInt-12 128ns ± 0% 122ns ± 1% -4.45% (p=0.000 n=15+20)
FmtFprintfIntInt-12 207ns ± 1% 193ns ± 0% -6.55% (p=0.000 n=19+14)
FmtFprintfPrefixedInt-12 197ns ± 1% 191ns ± 0% -2.93% (p=0.000 n=17+18)
FmtFprintfFloat-12 263ns ± 0% 248ns ± 1% -5.88% (p=0.000 n=15+19)
FmtManyArgs-12 794ns ± 0% 779ns ± 1% -1.90% (p=0.000 n=18+18)
GobDecode-12 7.14ms ± 2% 7.11ms ± 1% ~ (p=0.072 n=20+20)
GobEncode-12 5.85ms ± 1% 5.82ms ± 1% -0.49% (p=0.000 n=20+20)
Gzip-12 218ms ± 1% 215ms ± 1% -1.22% (p=0.000 n=19+19)
Gunzip-12 36.8ms ± 0% 36.7ms ± 0% -0.18% (p=0.006 n=18+20)
HTTPClientServer-12 77.1µs ± 4% 77.1µs ± 3% ~ (p=0.945 n=19+20)
JSONEncode-12 15.6ms ± 1% 15.9ms ± 1% +1.68% (p=0.000 n=18+20)
JSONDecode-12 55.2ms ± 1% 53.6ms ± 1% -2.93% (p=0.000 n=17+19)
Mandelbrot200-12 4.05ms ± 1% 4.05ms ± 0% ~ (p=0.306 n=17+17)
GoParse-12 3.14ms ± 1% 3.10ms ± 1% -1.31% (p=0.000 n=19+18)
RegexpMatchEasy0_32-12 69.3ns ± 1% 70.0ns ± 0% +0.89% (p=0.000 n=19+17)
RegexpMatchEasy0_1K-12 237ns ± 1% 236ns ± 0% -0.62% (p=0.000 n=19+16)
RegexpMatchEasy1_32-12 69.5ns ± 1% 70.3ns ± 1% +1.14% (p=0.000 n=18+17)
RegexpMatchEasy1_1K-12 377ns ± 1% 366ns ± 1% -3.03% (p=0.000 n=15+19)
RegexpMatchMedium_32-12 107ns ± 1% 107ns ± 2% ~ (p=0.318 n=20+19)
RegexpMatchMedium_1K-12 33.8µs ± 3% 33.5µs ± 1% -1.04% (p=0.001 n=20+19)
RegexpMatchHard_32-12 1.68µs ± 1% 1.73µs ± 0% +2.50% (p=0.000 n=20+18)
RegexpMatchHard_1K-12 50.8µs ± 1% 52.0µs ± 1% +2.50% (p=0.000 n=19+18)
Revcomp-12 381ms ± 1% 385ms ± 1% +1.00% (p=0.000 n=17+18)
Template-12 64.9ms ± 3% 62.6ms ± 1% -3.55% (p=0.000 n=19+18)
TimeParse-12 324ns ± 0% 328ns ± 1% +1.25% (p=0.000 n=18+18)
TimeFormat-12 345ns ± 0% 334ns ± 0% -3.31% (p=0.000 n=15+17)
[Geo mean] 52.1µs 51.5µs -1.00%
Change-Id: I13e74da3193a7f80794c654f944d1f0d60817049
Reviewed-on: https://go-review.googlesource.com/22632
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes this code better self-documenting and makes it easier to
find these places in the future.
Change-Id: I31dc5598ae67f937fb9ef26df92fd41d01e983c3
Reviewed-on: https://go-review.googlesource.com/22631
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
heapBits.bits is carefully written to produce good machine code. Use
it in heapBits.morePointers and heapBits.isPointer to get good machine
code there, too.
Change-Id: I208c7d0d38697e7a22cad67f692162589b75f1e2
Reviewed-on: https://go-review.googlesource.com/22630
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix issues introduced in 5f9a870.
Change-Id: Ia75945ef563956613bf88bbe57800a96455c265d
Reviewed-on: https://go-review.googlesource.com/22661
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add support for the context function set by runtime.SetCgoTraceback.
The context function was added in CL 17761, without support.
This CL is the support.
This CL has not been tested for real C code, as a working context
function for C code requires unwind support that does not seem to exist.
I wanted to get the CL out before the freeze.
I apologize for the length of this CL. It's mostly plumbing, but
unfortunately the plumbing is processor-specific.
Change-Id: I8ce11a0de9b3dafcc29efd2649d776e93bff0e90
Reviewed-on: https://go-review.googlesource.com/22508
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit adds the new 'ctrAble' interface to the crypto/cipher
package. The role of ctrAble is the same as gcmAble but for CTR
instead of GCM. It allows block ciphers to provide optimized CTR
implementations.
The primary benefit of adding CTR support to the s390x AES
implementation is that it allows us to encrypt the counter values
in bulk, giving the cipher message instruction a larger chunk of
data to work on per invocation.
The xorBytes assembly is necessary because xorBytes becomes a
bottleneck when CTR is done in this way. Hopefully it will be
possible to remove this once s390x has migrated to the ssa
backend.
name old speed new speed delta
AESCTR1K 160MB/s ± 6% 867MB/s ± 0% +442.42% (p=0.000 n=9+10)
Change-Id: I1ae16b0ce0e2641d2bdc7d7eabc94dd35f6e9318
Reviewed-on: https://go-review.googlesource.com/22195
Reviewed-by: Adam Langley <agl@golang.org>
This commit adds the cbcEncAble and cbcDecAble interfaces that
can be implemented by block ciphers that support an optimized
implementation of CBC. This is similar to what is done for GCM
with the gcmAble interface.
The cbcEncAble, cbcDecAble and gcmAble interfaces all now have
tests to ensure they are detected correctly in the cipher
package.
name old speed new speed delta
AESCBCEncrypt1K 152MB/s ± 1% 1362MB/s ± 0% +795.59% (p=0.000 n=10+9)
AESCBCDecrypt1K 143MB/s ± 1% 1362MB/s ± 0% +853.00% (p=0.000 n=10+9)
Change-Id: I715f686ab3686b189a3dac02f86001178fa60580
Reviewed-on: https://go-review.googlesource.com/22523
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This commit moves the GC from free list allocation to
bit mark allocation. Instead of using the bitmaps
generated during the mark phases to generate free
list and then using the free lists for allocation we
allocate directly from the bitmaps.
The change in the garbage benchmark
name old time/op new time/op delta
XBenchGarbage-12 2.22ms ± 1% 2.13ms ± 1% -3.90% (p=0.000 n=18+18)
Change-Id: I17f57233336f0ca5ef5404c3be4ecb443ab622aa
nextFreeFast is currently not inlined by the compiler due
to its size and complexity. This CL simplifies
nextFreeFast by letting the slow path handle (nextFree)
handle a corner cases.
Change-Id: Ia9c5d1a7912bcb4bec072f5fd240f0e0bafb20e4
Reviewed-on: https://go-review.googlesource.com/22598
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
This is necessary to avoid disrupting the go1 suite and gives
us a place to put other tests of basic compiler function and
correctness.
Change-Id: I36933819ff2bfe6a2121fff2be9a98efd2123d9a
Reviewed-on: https://go-review.googlesource.com/22597
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Break really long lines.
Add spacing to line up columns.
In AMD64, put all the optimization rules after all the
lowering rules.
Change-Id: I45cc7368bf278416e67f89e74358db1bd4326a93
Reviewed-on: https://go-review.googlesource.com/22470
Reviewed-by: David Chase <drchase@google.com>
sweep used to skip mcental.freeSpan (and its locking) if it didn't
find any new free objects. We lost that optimization when the
freed-object counting changed in dad83f7 to count total free objects
instead of newly freed objects.
The previous commit brings back counting of newly freed objects, so we
can easily revive this optimization by checking that count (like we
used to) instead of the total free objects count.
Change-Id: I43658707a1c61674d0366124d5976b00d98741a9
Reviewed-on: https://go-review.googlesource.com/22596
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>