Use a version of Floyd's cycle finding algorithm,
but advance by 1 and 1/2 steps per cycle rather
than by 1 and 2. It is simpler and should be cheaper
in the normal, acyclic case.
This should fix the 386 and arm builds,
which are currently hung.
Change-Id: If8bd443011b28a5ecb004a549239991d3dfc862b
Reviewed-on: https://go-review.googlesource.com/13473
Reviewed-by: Keith Randall <khr@golang.org>
We must make sure that all loads that use a store are scheduled
before the next store. Add additional dependency edges to the
value graph to enforce this constraint.
Change-Id: Iab83644f68bc4c30637085b82ca7467b9d5513a5
Reviewed-on: https://go-review.googlesource.com/13470
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
When scanning a hex byte at EOF, the code was ungetting the eof,
which backed up the input and caused double-scanning of a byte.
Delete the call to UnreadRune.
This line appeared in 1.5 for some reason; it was not in 1.4 and
should be removed again for 1.5
Fixes#12090.
Change-Id: Iad1ce8e7db8ec26615c5271310f4b0228cca7d78
Reviewed-on: https://go-review.googlesource.com/13461
Reviewed-by: Andrew Gerrand <adg@golang.org>
Changes the torture test in #12068 from failing about 1/10 times
to not failing in almost 2,000 runs.
This was only happening in -race mode because functions are
bigger in -race mode, so a few of the helpers for heapBitsBulkBarrier
were not being inlined, and they were not marked nosplit,
so (only in -race mode) the write barrier was being preempted by GC,
causing missed pointer updates.
Filed issue #12069 for diagnosis of any other similar errors.
Fixes#12068.
Change-Id: Ic174d9b050ba278b18b08ab0d85a73c33bd5b175
Reviewed-on: https://go-review.googlesource.com/13364
Reviewed-by: Austin Clements <austin@google.com>
Also, crash early on non-Linux SMP ARM systems when GOARM < 7;
without the proper synchronization, SMP cannot work.
Linux is okay because we call kernel-provided routines for
synchronization and barriers, and the kernel takes care of
providing the right routines for the current system.
On non-Linux systems we are left to fend for ourselves.
It is possible to use different synchronization on GOARM=6,
but it's too late to do that in the Go 1.5 cycle.
We don't believe there are any non-Linux SMP GOARM=6 systems anyway.
Fixes#12067.
Change-Id: I771a556e47893ed540ec2cd33d23c06720157ea3
Reviewed-on: https://go-review.googlesource.com/13363
Reviewed-by: Austin Clements <austin@google.com>
Don't nilcheck values that were constructed as a result of OpAddr or
OpAddPtr.
Change-Id: I38053e905d1b76a2a64e77f84e444d38a5217108
Reviewed-on: https://go-review.googlesource.com/13256
Reviewed-by: David Chase <drchase@google.com>
Rather than require an explicit Copy on the RHS of rewrite rules,
use rulegen magic to add it.
The advantages to handling this in rulegen are:
* simpler rules
* harder to accidentally miss a Copy
Change-Id: I46853bade83bdf517eee9495bf5a553175277b53
Reviewed-on: https://go-review.googlesource.com/13242
Reviewed-by: Keith Randall <khr@golang.org>
Currently, runtime.Goexit() calls goexit()—the goroutine exit stub—to
terminate the goroutine. This *mostly* works, but can cause a
"leftover stack barriers" panic if the following happens:
1. Goroutine A has a reasonably large stack.
2. The garbage collector scan phase runs and installs stack barriers
in A's stack. The top-most stack barrier happens to fall at address X.
3. Goroutine A unwinds the stack far enough to be a candidate for
stack shrinking, but not past X.
4. Goroutine A calls runtime.Goexit(), which calls goexit(), which
calls goexit1().
5. The garbage collector enters mark termination.
6. Goroutine A is preempted right at the prologue of goexit1() and
performs a stack shrink, which calls gentraceback.
gentraceback stops as soon as it sees goexit on the stack, which is
only two frames up at this point, even though there may really be many
frames above it. More to the point, the stack barrier at X is above
the goexit frame, so gentraceback never sees that stack barrier. At
the end of gentraceback, it checks that it saw all of the stack
barriers and panics because it didn't see the one at X.
The fix is simple: call goexit1, which actually implements the process
of exiting a goroutine, rather than goexit, the exit stub.
To make sure this doesn't happen again in the future, we also add an
argument to the stub prototype of goexit so you really, really have to
want to call it in order to call it. We were able to reliably
reproduce the above sequence with a fair amount of awful code inserted
at the right places in the runtime, but chose to change the goexit
prototype to ensure this wouldn't happen again rather than pollute the
runtime with ugly testing code.
Change-Id: Ifb6fb53087e09a252baddadc36eebf954468f2a8
Reviewed-on: https://go-review.googlesource.com/13323
Reviewed-by: Russ Cox <rsc@golang.org>
This makes TestTraceStressStartStop much less flaky.
Running under stress, it changes the failure rate from
above 1/100 to under 1/50000. That very unlikely
failure happens when an unexpected GoSysExit is
written. Not sure how that happens yet, but it is much
less important.
Fixes#11953.
Change-Id: I034671936334b4f3ab733614ef239aa121d20247
Reviewed-on: https://go-review.googlesource.com/13321
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
The lowering rules were missing the non-64 bit case.
SBBLcarrymask can be folded to a int32 integer whose
type has a smaller bit size. Without the new AND rules
the following would be generated:
v19 = MOVLconst <uint8> [-1] : SI
v20 = ANDB <uint8> v18 v19 : DI
which is obviously a NOP.
Fixes#12022
Change-Id: I5f4209f78edc0f118e5b9b2908739f09cefebca4
Reviewed-on: https://go-review.googlesource.com/13301
Reviewed-by: Keith Randall <khr@golang.org>
Make sure all referenced Blocks and Values are really there.
Fix deadcode to generate SSA graphs that pass this new test.
Change-Id: Ib002ce20e33490eb8c919bd189d209f769d61517
Reviewed-on: https://go-review.googlesource.com/13147
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
No functional changes.
The intent is just to make this
easier to read and maintain.
Change-Id: Iec207546482cd62bcb22eaae8efe5be6c4f15378
Reviewed-on: https://go-review.googlesource.com/13284
Reviewed-by: Keith Randall <khr@golang.org>
regalloc expects to find all OpSP and OpSB values
in the entry block.
There is no value to moving them; don't.
Change-Id: I775198f03ce7420348721ffc5e7d2bab065465b1
Reviewed-on: https://go-review.googlesource.com/13266
Reviewed-by: Keith Randall <khr@golang.org>
Given (say)
b1: <- b2 b3
v1 = Phi <t> v2 v3
b2:
v2 = ...
b3:
...
tighten will move v2 to b1, since it is only used in b1.
This is wrong; v2 needs to be evaluated before entering b1.
Fix it.
Change-Id: I2cc3b30e3ffd221cf594e36cec534dfd9cf3c6a7
Reviewed-on: https://go-review.googlesource.com/13264
Reviewed-by: Keith Randall <khr@golang.org>
The old code was only allowing the chars we choose not to escape.
We sometimes prefer to escape chars that do not strictly need it.
Allowing those to be used in RawPath lets people override that
preference, which is in fact the whole point of RawPath (new in Go 1.5).
While we are here, also allow [ ] in RawPath.
This is not strictly spec-compliant, but it is what modern browers
do and what at least some people expect, and the [ ] do not cause
any ambiguity (the usual reason they would be escaped, as they are
part of the RFC gen-delims class).
The argument for allowing them now instead of waiting until Go 1.6
is that this way RawPath has one fixed meaning at the time it is
introduced, that we should not need to change or expand.
Fixes#5684.
Change-Id: If9c82a18f522d7ee1d10310a22821ada9286ee5c
Reviewed-on: https://go-review.googlesource.com/13258
Reviewed-by: Andrew Gerrand <adg@golang.org>
The code in question was added as part of allowing zone identifiers
in IPv6 literals like http://[ipv6%zone]:port/foo, in golang.org/cl/2431.
The old condition makes no sense. It refers to §3.2.1, which is the wrong section
of the RFC, it excludes all the sub-delims, which §3.2.2 (the right section)
makes clear are valid, and it allows ':', which is not actually valid,
without an explanation as to why (because we keep :port in the Host field
of the URL struct).
The new condition allows all the sub-delims, as specified in RFC 3986,
plus the additional characters [ ] : seen in IP address literals and :port suffixes,
which we also keep in the Host field.
This allows mysql://a,b,c/path to continue to parse, as it did in Go 1.4 and earlier.
This CL does not break any existing tests, suggesting the over-conservative
behavior was not intended and perhaps not realized.
It is especially important not to over-escape the host field, because
Go does not unescape the host field during parsing: it rejects any
host field containing % characters.
Fixes#12036.
Change-Id: Iccbe4985957b3dc58b6dfb5dcb5b63a51a6feefb
Reviewed-on: https://go-review.googlesource.com/13254
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Go 1.4 and earlier accepted mysql://x@y(z:123)/foo
and I don't see any compelling reason to break that.
The CL during Go 1.5 that broke this syntax was
trying to fix#11208 and was probably too aggressive.
I added a test case for #11208 to make sure that stays
fixed.
Relaxing the check did not re-break #11208 nor did
it cause any existing test to fail. I added a test for the
mysql://x@y(z:123)/foo syntax being preserved.
Fixes#12023.
Change-Id: I659d39f18c85111697732ad24b757169d69284fc
Reviewed-on: https://go-review.googlesource.com/13253
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Strengthening VerifyHostname exposed the fact that for resumed
connections, ConnectionState().VerifiedChains was not being saved
and restored during the ClientSessionCache operations.
Do that.
This change just saves the verified chains in the client's session
cache. It does not re-verify the certificates when resuming a
connection.
There are arguments both ways about this: we want fast, light-weight
resumption connections (thus suggesting that we shouldn't verify) but
it could also be a little surprising that, if the verification config
is changed, that would be ignored if the same session cache is used.
On the server side we do re-verify client-auth certificates, but the
situation is a little different there. The client session cache is an
object in memory that's reset each time the process restarts. But the
server's session cache is a conceptual object, held by the clients, so
can persist across server restarts. Thus the chance of a change in
verification config being surprisingly ignored is much higher in the
server case.
Fixes#12024.
Change-Id: I3081029623322ce3d9f4f3819659fdd9a381db16
Reviewed-on: https://go-review.googlesource.com/13164
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Prior to this change, broken trailers would be handled by body.Read, and
an error would be returned to its caller (likely a Handler), but that
error would go completely unnoticed by the rest of the server flow
allowing a broken connection to be reused. This is a possible request
smuggling vector.
Fixes#12027.
Change-Id: I077eb0b8dff35c5d5534ee5f6386127c9954bd58
Reviewed-on: https://go-review.googlesource.com/13148
Reviewed-by: Russ Cox <rsc@golang.org>
This change alters the certificate used in many tests so that it's no
longer self-signed. This allows some tests to exercise the standard
certificate verification paths in the future.
Change-Id: I9c3fcd6847eed8269ff3b86d9b6966406bf0642d
Reviewed-on: https://go-review.googlesource.com/13244
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
88e945f introduced a non-speculative double check of the heap trigger
before actually starting a concurrent GC. This was necessary to fix a
race for heap-triggered GC, but broke sysmon-triggered periodic GC,
since the heap check will of course fail for periodically triggered
GC.
Fix this by telling startGC whether or not this GC was triggered by
heap size or a timer and only doing the heap size double check for GCs
triggered by heap size.
Fixes#12026.
Change-Id: I7c3f6ec364545c36d619f2b4b3bf3b758e3bcbd6
Reviewed-on: https://go-review.googlesource.com/13168
Reviewed-by: Russ Cox <rsc@golang.org>
Now that it works we need to turn it back on.
Fixes#10119.
Change-Id: I9c62d3026f7bb62c49a601ad73f33bf655372915
Reviewed-on: https://go-review.googlesource.com/13162
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is just far too slow.
I have a CL for Go 1.6 that makes many of these into internal tests.
That will improve the coverage.
It does not matter much, because basically none of the go command
tests are architecture dependent, so the other builders will catch
any problems.
Fixes freebsd-arm builder.
Change-Id: I8b2f6ac2cc1e7657019f7731c6662dc43e20bfb5
Reviewed-on: https://go-review.googlesource.com/13166
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This works after golang.org/cl/13120 is running on the
coordinator (maybe it already is).
Change-Id: I4053d8e2f32fafd47b927203a6f66d5858e23376
Reviewed-on: https://go-review.googlesource.com/13165
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is what is causing freebsd/arm to crash mysteriously when using cgo.
The bug was introduced in golang.org/cl/4030, which moved this code out
of rt0_go and into its own function. The ARM ABI says that calls must
be made with the stack pointer at an 8-byte boundary, but only FreeBSD
seems to crash when this is violated.
Fixes#10119.
Change-Id: Ibdbe76b2c7b80943ab66b8abbb38b47acb70b1e5
Reviewed-on: https://go-review.googlesource.com/13161
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
Implement ITAB, selecting the itable field of an interface.
Soften the lowering check to allow lowerings that leave
generic but dead ops behind. (The ITAB lowering does this.)
Change-Id: Icc84961dd4060d143602f001311aa1d8be0d7fc0
Reviewed-on: https://go-review.googlesource.com/13144
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
I find myself always adding this in temporarily.
Make it permanent.
Change-Id: I1646b3930a07d0ea01840736ccd449b7fd24f06e
Reviewed-on: https://go-review.googlesource.com/13141
Reviewed-by: Keith Randall <khr@golang.org>
Failure to treat control ops as live can lead
to them being eliminated when they live in
other blocks.
Change-Id: I604a1977a3d3884b1f4516bea4e15885ce38272d
Reviewed-on: https://go-review.googlesource.com/13138
Reviewed-by: Keith Randall <khr@golang.org>
They were being omitted after scheduling.
Change-Id: Ia20e2dcb61fde9ec854918b958c3897bafd282a6
Reviewed-on: https://go-review.googlesource.com/13140
Reviewed-by: Keith Randall <khr@golang.org>
Don't put them in the control value's block.
That may be many blocks up the dominator tree.
Change-Id: Iab3ea36a890ffe0e355dadec7aeb676901c4f070
Reviewed-on: https://go-review.googlesource.com/13134
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This is helpful when debugging generated code.
Change-Id: I268efa3593a03bb2c4e9f07d9034c004cd40df41
Reviewed-on: https://go-review.googlesource.com/13099
Reviewed-by: Keith Randall <khr@golang.org>
When commit 510fd13 enabled assists during the scan phase, it failed
to also update the code in the GC controller that computed the assist
CPU utilization and adjusted the trigger based on it. Fix that code so
it uses the start of the scan phase as the wall-clock time when
assists were enabled rather than the start of the mark phase.
Change-Id: I05013734b4448c3e2c730dc7b0b5ee28c86ed8cf
Reviewed-on: https://go-review.googlesource.com/13048
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
At the start of a GC cycle, the garbage collector computes the assist
ratio based on the total scannable heap size. This was intended to be
conservative; after all, this assumes the entire heap may be reachable
and hence needs to be scanned. But it only assumes that the *current*
entire heap may be reachable. It fails to account for heap allocated
during the GC cycle. If the trigger ratio is very low (near zero), and
most of the heap is reachable when GC starts (which is likely if the
trigger ratio is near zero), then it's possible for the mutator to
create new, reachable heap fast enough that the assists won't keep up
based on the assist ratio computed at the beginning of the cycle. As a
result, the heap can grow beyond the heap goal (by hundreds of megs in
stress tests like in issue #11911).
We already have some vestigial logic for dealing with situations like
this; it just doesn't run often enough. Currently, every 10 ms during
the GC cycle, the GC revises the assist ratio. This was put in before
we switched to a conservative assist ratio (when we really were using
estimates of scannable heap), and it turns out to be exactly what we
need now. However, every 10 ms is far too infrequent for a rapidly
allocating mutator.
This commit reuses this logic, but replaces the 10 ms timer with
revising the assist ratio every time the heap is locked, which
coincides precisely with when the statistics used to compute the
assist ratio are updated.
Fixes#11911.
Change-Id: I377b231ab064946228378fa10422a46d1b50f4c5
Reviewed-on: https://go-review.googlesource.com/13047
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This was useful in debugging the mutator assist behavior for #11911,
and it fits with the other gcpacertrace output.
Change-Id: I1e25590bb4098223a160de796578bd11086309c7
Reviewed-on: https://go-review.googlesource.com/13046
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Proportional concurrent sweep is currently based on a ratio of spans
to be swept per bytes of object allocation. However, proportional
sweeping is performed during span allocation, not object allocation,
in order to minimize contention and overhead. Since objects are
allocated from spans after those spans are allocated, the system tends
to operate in debt, which means when the next GC cycle starts, there
is often sweep debt remaining, so GC has to finish the sweep, which
delays the start of the cycle and delays enabling mutator assists.
For example, it's quite likely that many Ps will simultaneously refill
their span caches immediately after a GC cycle (because GC flushes the
span caches), but at this point, there has been very little object
allocation since the end of GC, so very little sweeping is done. The
Ps then allocate objects from these cached spans, which drives up the
bytes of object allocation, but since these allocations are coming
from cached spans, nothing considers whether more sweeping has to
happen. If the sweep ratio is high enough (which can happen if the
next GC trigger is very close to the retained heap size), this can
easily represent a sweep debt of thousands of pages.
Fix this by making proportional sweep proportional to the number of
bytes of spans allocated, rather than the number of bytes of objects
allocated. Prior to allocating a span, both the small object path and
the large object path ensure credit for allocating that span, so the
system operates in the black, rather than in the red.
Combined with the previous commit, this should eliminate all sweeping
from GC start up. On the stress test in issue #11911, this reduces the
time spent sweeping during GC (and delaying start up) by several
orders of magnitude:
mean 99%ile max
pre fix 1 ms 11 ms 144 ms
post fix 270 ns 735 ns 916 ns
Updates #11911.
Change-Id: I89223712883954c9d6ec2a7a51ecb97172097df3
Reviewed-on: https://go-review.googlesource.com/13044
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently it's possible for the next_gc heap size trigger computed for
the next GC cycle to be less than the current allocated heap size.
This means the next cycle will start immediately, which means there's
no time to perform the concurrent sweep between GC cycles. This places
responsibility for finishing the sweep on GC itself, which delays GC
start-up and hence delays mutator assist.
Fix this by ensuring that next_gc is always at least a little higher
than the allocated heap size, so we won't trigger the next cycle
instantly.
Updates #11911.
Change-Id: I74f0b887bf187518d5fedffc7989817cbcf30592
Reviewed-on: https://go-review.googlesource.com/13043
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently there are two sensitive periods during which a mutator can
allocate past the heap goal but mutator assists can't be enabled: 1)
at the beginning of GC between when the heap first passes the heap
trigger and sweep termination and 2) at the end of GC between mark
termination and when the background GC goroutine parks. During these
periods there's no back-pressure or safety net, so a rapidly
allocating mutator can allocate past the heap goal. This is
exacerbated if there are many goroutines because the GC coordinator is
scheduled as any other goroutine, so if it gets preempted during one
of these periods, it may stay preempted for a long period (10s or 100s
of milliseconds).
Normally the mutator does scan work to create back-pressure against
allocation, but there is no scan work during these periods. Hence, as
a fall back, if a mutator would assist but can't yet, simply yield the
CPU. This delays the mutator somewhat, but more importantly gives more
CPU time to the GC coordinator for it to complete the transition.
This is obviously a workaround. Issue #11970 suggests a far better but
far more invasive way to fix this.
Updates #11911. (This very nearly fixes the issue, but about once
every 15 minutes I get a GC cycle where the assists are enabled but
don't do enough work.)
Change-Id: I9768b79e3778abd3e06d306596c3bd77f65bf3f1
Reviewed-on: https://go-review.googlesource.com/13026
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently allocation checks the GC trigger speculatively during
allocation and then triggers the GC without rechecking. As a result,
it's possible for G 1 and G 2 to detect the trigger simultaneously,
both enter startGC, G 1 actually starts GC while G 2 gets preempted
until after the whole GC cycle, then G 2 immediately starts another GC
cycle even though the heap is now well under the trigger.
Fix this by re-checking the GC trigger non-speculatively just before
actually kicking off a new GC cycle.
This contributes to #11911 because when this happens, we definitely
don't finish the background sweep before starting the next GC cycle,
which can significantly delay the start of concurrent scan.
Change-Id: I560ab79ba5684ba435084410a9765d28f5745976
Reviewed-on: https://go-review.googlesource.com/13025
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Do not assume that if stat shows multiple links that we should mark the
file as a hardlink in the tar format. If the hardlink link was not
referenced, this caused a link to "/". On an overlay file system, all
files have multiple links.
The caller must keep the inode references and set TypeLink, Size = 0,
and LinkName themselves.
Change-Id: I873b8a235bc8f8fbb271db74ee54232da36ca013
Reviewed-on: https://go-review.googlesource.com/13045
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The buildmode docs mention exported functions, but don't say anything
about how to export them. Mention the cgo tool to make this somewhat
clearer.
Fixes#11955.
Change-Id: Ie5420445daa87f5aceec6ad743465d5d32d0a786
Reviewed-on: https://go-review.googlesource.com/13080
Reviewed-by: Russ Cox <rsc@golang.org>
Rewrite ^{n}x to be ^{n % 2}x. This will eventually resolve a fuzz
issue that breaks v1.5.
Updates #11352
Change-Id: I1b3f93872d06222f9ff5f6fd5580178ebaf4c003
Reviewed-on: https://go-review.googlesource.com/13110
Reviewed-by: Keith Randall <khr@golang.org>
For niceness, when go/exact was moved from x/tools, it
was renamed go/constant.
For simplicity, when go/types was moved from x/tools, its
imports of (now) go/constant were done with a rename:
import exact "go/constant"
This kept the code just as it was before and avoided the issue
of what to call the internal constant called, um, constant.
But not all was hidden, as the text of some fields of structs and
the like leaked the old name, so things like "exact.Value" appeared
in type definitions and function signatures in the documentation.
This is unacceptable.
Fix the documentation issue by fixing the code. Rename the constant
constant constant_, and remove the renaming import.
This should go into 1.5. It's mostly a mechanical change, is
internal to the package, and fixes the documentation. It contains
no semantic changes except to fix a benchmark that was broken
in the original transition.
Fixes#11949.
Change-Id: Ieb94b6558535b504180b1378f19e8f5a96f92d3c
Reviewed-on: https://go-review.googlesource.com/13051
Reviewed-by: Russ Cox <rsc@golang.org>
The DFS scheduler doesn't do the right thing. If a Value x is used by
more than one other Value, then x is put into the DFS queue when
its first user (call it y) is visited. It is not removed and reinserted
when the second user of x (call it z) is visited, so the dependency
between x and z is not respected. There is no easy way to fix this with
the DFS queue because we'd have to rip values out of the middle of the
DFS queue.
The new scheduler works from the end of the block backwards, scheduling
instructions which have had all of their uses already scheduled.
A simple priority scheme breaks ties between multiple instructions that
are ready to schedule simultaneously.
Keep track of whether we've scheduled or not, and make print() use
the scheduled order if we have.
Fix some shift tests that this change tickles. Add unsigned right shift tests.
Change-Id: I44164c10bb92ae8ab8f76d7a5180cbafab826ea1
Reviewed-on: https://go-review.googlesource.com/13069
Reviewed-by: Todd Neal <todd@tneal.org>
Modify tests to use a known value instead of comparing the backends
directly.
Change-Id: I32e804e12515885bd94c4f83644cbca03b018fea
Reviewed-on: https://go-review.googlesource.com/13042
Reviewed-by: Keith Randall <khr@golang.org>
This was confusing when I was trying to fix go build -o.
Perhaps due to that fix, this can now be simplified from
three functions to one.
Change-Id: I878a6d243b14132a631e7c62a3bb6d101bc243ea
Reviewed-on: https://go-review.googlesource.com/13027
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Quoting the new docs:
«
If the arguments to build are a list of .go files, build treats
them as a list of source files specifying a single package.
When compiling a single main package, build writes
the resulting executable to an output file named after
the first source file ('go build ed.go rx.go' writes 'ed' or 'ed.exe')
or the source code directory ('go build unix/sam' writes 'sam' or 'sam.exe').
The '.exe' suffix is added when writing a Windows executable.
When compiling multiple packages or a single non-main package,
build compiles the packages but discards the resulting object,
serving only as a check that the packages can be built.
The -o flag, only allowed when compiling a single package,
forces build to write the resulting executable or object
to the named output file, instead of the default behavior described
in the last two paragraphs.
»
There is a change in behavior here, namely that 'go build -o x.a x.go'
where x.go is not a command (not package main) did not write any
output files (back to at least Go 1.2) but now writes x.a.
This seems more reasonable than trying to explain that -o is
sometimes silently ignored.
Otherwise the behavior is unchanged.
The lines being deleted in goFilesPackage look like they are
setting up 'go build x.o' to write 'x.a', but they were overridden
by the p.target = "" in runBuild. Again back to at least Go 1.2,
'go build x.go' for a non-main package has never produced
output. It seems better to keep it that way than to change it,
both for historical consistency and for consistency with
'go build strings' and 'go build std'.
All of this behavior is now tested.
Fixes#10865.
Change-Id: Iccdf21f366fbc8b5ae600a1e50dfe7fc3bff8b1c
Reviewed-on: https://go-review.googlesource.com/13024
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
It was failing with multiple goroutines a few out of every thousand
runs (with errRequestCanceled) because it was using the same
*http.Request for all 5 RoundTrips, but the RoundTrips' goroutines
(notably the readLoop method) were all still running, sharing that
same pointer. Because the response has no body (which is what
TestZeroLengthPostAndResponse tests), the readLoop was marking the
connection as reusable early (before the caller read until the body's
EOF), but the Transport code was clearing the Request's cancelation
func *AFTER* the caller had already received it from RoundTrip. This
let the test continue looping and do the next request with the same
pointer, fetch a connection, and then between getConn and roundTrip
have an invariant violated: the Request's cancelation func was nil,
tripping this check:
if !pc.t.replaceReqCanceler(req.Request, pc.cancelRequest) {
pc.t.putIdleConn(pc)
return nil, errRequestCanceled
}
The solution is to clear the request cancelation func in the readLoop
goroutine in the no-body case before it's returned to the caller.
This now passes reliably:
$ go test -race -run=TestZeroLengthPostAndResponse -count=3000
I think we've only seen this recently because we now randomize scheduling
of goroutines in race mode (https://golang.org/cl/11795). This race
has existed for a long time but the window was hard to hit.
Change-Id: Idb91c582919f85aef5b9e5ef23706f1ba9126e9a
Reviewed-on: https://go-review.googlesource.com/13070
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Introduced in https://go-review.googlesource.com/12865 (git rev c2db5f4c).
This fix doesn't add any new lock acquistions: it just moves the
existing one taken by the unreadDataSize method and moves it out
wider.
It became flaky at rev c2db5f4c, but now reliably passes again:
$ go test -v -race -run=TestTransportAndServerSharedBodyRace -count=100 net/http
Fixes#11985
Change-Id: I6956d62839fd7c37e2f7441b1d425793f4a0db30
Reviewed-on: https://go-review.googlesource.com/12909
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
HTTP servers attempt to entirely consume a request body before sending a
response. However, when doing so, it previously would ignore any errors
encountered.
Unfortunately, the errors triggered at this stage are indicative of at
least a couple problems: read timeouts and chunked encoding errors.
This means properly crafted and/or timed requests could lead to a
"smuggled" request.
The fix is to inspect the errors created by the response body Reader,
and treat anything other than io.EOF or ErrBodyReadAfterClose as
fatal to the connection.
Fixes#11930
Change-Id: I0bf18006d7d8f6537529823fc450f2e2bdb7c18e
Reviewed-on: https://go-review.googlesource.com/12865
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes the receiver name consistent with the rest of the methods on
type Server.
Change-Id: Ic2a007d3b5eb50bd87030e15405e9856109cf590
Reviewed-on: https://go-review.googlesource.com/13035
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The test uses external linking mode, which is probably not available
if cgo does not work.
Fixes#11969.
Change-Id: Id1c2828cd2540391e16b422bf51674ba6ff084b0
Reviewed-on: https://go-review.googlesource.com/13005
Reviewed-by: Russ Cox <rsc@golang.org>
The percolation of errors upward in the load process could
drop errors, meaning that a build tree could, depending on the
processing order, import the same directory as both "p/vendor/x"
and as "x". That's not supposed to be allowed. But then, worse,
the build would generate two jobs for building that directory,
which would use the same work space and overwrite each other's files,
leading to very strange failures.
Two fixes:
1. Fix the propagation of errors upward (prefer errors over success).
2. Check explicitly for duplicated packages before starting a build.
New test for #1.
Since #2 can't happen, tested #2 by hand after reverting fix for #1.
Fixes#11913.
Change-Id: I6d2fc65f93b8fb5f3b263ace8d5f68d803a2ae5c
Reviewed-on: https://go-review.googlesource.com/13022
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On most systems, a pointer is the worst case alignment, so adding
a pointer field at the end of a struct guarantees there will be no
padding added after that field (to satisfy overall struct alignment
due to some more-aligned field also present).
In the runtime, the map implementation needs a quick way to
get to the overflow pointer, which is last in the bucket struct,
so it uses size - sizeof(pointer) as the offset.
NaCl/amd64p32 is the exception, as always.
The worst case alignment is 64 bits but pointers are 32 bits.
There's a long history that is not worth going into, but when
we moved the overflow pointer to the end of the struct,
we didn't get the padding computation right.
The compiler computed the regular struct size and then
on amd64p32 added another 32-bit field.
And the runtime assumed it could step back two 32-bit fields
(one 64-bit register size) to get to the overflow pointer.
But in fact if the struct needed 64-bit alignment, the computation
of the regular struct size would have added a 32-bit pad already,
and then the code unconditionally added a second 32-bit pad.
This placed the overflow pointer three words from the end, not two.
The last two were padding, and since the runtime was consistent
about using the second-to-last word as the overflow pointer,
no harm done in the sense of overwriting useful memory.
But writing the overflow pointer to a non-pointer word of memory
means that the GC can't see the overflow blocks, so it will
collect them prematurely. Then bad things happen.
Correct all this in a few steps:
1. Add an explicit check at the end of the bucket layout in the
compiler that the overflow field is last in the struct, never
followed by padding.
2. When padding is needed on nacl (not always, just when needed),
insert it before the overflow pointer, to preserve the "last in the struct"
property.
3. Let the compiler have the final word on the width of the struct,
by inserting an explicit padding field instead of overwriting the
results of the width computation it does.
4. For the same reason (tell the truth to the compiler), set the type
of the overflow field when we're trying to pretend its not a pointer
(in this case the runtime maintains a list of the overflow blocks
elsewhere).
5. Make the runtime use "last in the struct" as its location algorithm.
This fixes TestTraceStress on nacl/amd64p32.
The 'bad map state' and 'invalid free list' failures no longer occur.
Fixes#11838.
Change-Id: If918887f8f252d988db0a35159944d2b36512f92
Reviewed-on: https://go-review.googlesource.com/12971
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This fixes the crypto/subtle tests.
Change-Id: Ie6e721eec3481f67f13de1bfbd7988e227793148
Reviewed-on: https://go-review.googlesource.com/13000
Reviewed-by: Keith Randall <khr@golang.org>
Fixes some minor issues regarding quoted-string when parsing
the local-part.
Those strings should return an error:
- quoted-string without any content: `""@test.com`
- quoted-string containing tab: "\"\t\"@test.com"
Fixes#11293
Change-Id: Ied93eb6831915c9b1f8e727cea14168af21f8d3b
Reviewed-on: https://go-review.googlesource.com/12905
Reviewed-by: Russ Cox <rsc@golang.org>
The code already fixed large non-stack offsets
but explicitly excluded stack references.
Perhaps you could get away with that before,
but current versions of nacl reject such stack
references. Rewrite them the same as the others.
For #11956 but probably not the last problem.
Change-Id: I0db4e3a1ed4f88ccddf0d30228982960091d9fb7
Reviewed-on: https://go-review.googlesource.com/13010
Reviewed-by: Dave Cheney <dave@cheney.net>
Dangling pointer error. Unlikely to trigger in practice, but still.
Found by running GODEBUG=efence=1 GOGC=1 trace.test.
Change-Id: Ice474dedcf62dd33ab77526287a023ba3b166db9
Reviewed-on: https://go-review.googlesource.com/12991
Reviewed-by: Austin Clements <austin@google.com>
In https://golang.org/cl/12080 we forbade installing cross-compiled
binaries into a subdirectory of $GOBIN, in order to fix
https://golang.org/issue/9769. However, that fix was too aggressive,
in that it also forbade installing into a subdirectory of $GOPATH/bin.
This patch permits installing cross-compiled binaries into a
subdirectory $GOPATH/bin while continuing to forbid installing into a
subdirectory of $GOBIN.
Fixes#11778.
Change-Id: Ibc9919554e8c275beff54ec8bf919cfaa03b11ba
Reviewed-on: https://go-review.googlesource.com/12938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The only types that remain in the ssa package
are special compiler-only types.
Change-Id: If957abf128ec0778910d67666c297f97f183b7ee
Reviewed-on: https://go-review.googlesource.com/12933
Reviewed-by: Keith Randall <khr@golang.org>
This should fix the solaris/amd64 builder.
Change-Id: Idd6460cc9e842f7b874c9757379986aa723c974c
Reviewed-on: https://go-review.googlesource.com/12922
Reviewed-by: Austin Clements <austin@google.com>
Fixes#11918
Replace calls to lchown(2) with fchownat(2) for linux/arm64 as the former is not suppored.
This change has also landed on the x/sys repo as CL 12837.
Change-Id: I58d4b144e051e36dd650ec9b7f3a02610ea943e5
Reviewed-on: https://go-review.googlesource.com/12833
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Russ Cox <rsc@golang.org>
From compiling go there were 260 functions where XOR was needed.
Much of the required changes for implementing XOR were already
done in 12813.
Change-Id: I5a68aa028f5ed597bc1d62cedbef3620753dfe82
Reviewed-on: https://go-review.googlesource.com/12901
Reviewed-by: Keith Randall <khr@golang.org>
The existing backend simply elides OCONVNOP.
There's no reason for us to do any differently.
Rather than insert ConvNops and then rewrite them
away, stop creating them in the first place.
Change-Id: I4bcbe2229fcebd189ae18df24f2c612feb6e215e
Reviewed-on: https://go-review.googlesource.com/12810
Reviewed-by: Keith Randall <khr@golang.org>
This only triggers on ARMv7+.
If there are important SMP ARMv6 machines we can reconsider.
Makes TestLFStress tests pass and sync/atomic tests not time out
on Apple iPad Mini 3.
Fixes#7977.
Fixes#10189.
Change-Id: Ie424dea3765176a377d39746be9aa8265d11bec4
Reviewed-on: https://go-review.googlesource.com/12950
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Was not allocating space for the frame above sigpanic,
nor was it pushing the LR into the right place.
Because traceback past sigpanic only needs the
LR for faulting leaves, this was not noticed too much.
But it did break the sync/atomic nil deref tests.
Change-Id: Icba53fffa193423aab744c37f21ee893ce2ee3ac
Reviewed-on: https://go-review.googlesource.com/12926
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Convert shift ops to also encode the size of the shift amount.
Change signed right shift from using CMOV to using bit twiddles.
It is a little bit better (5 instructions instead of 4, but fewer
bytes and slightly faster code). It's also a bit faster than
the 4-instruction branch version, even with a very predictable
branch. As tested on my machine, YMMV.
Implement OCOM while we are here.
Change-Id: I8ca12dd62fae5d626dc0e6da5d4bbd34fd9640d2
Reviewed-on: https://go-review.googlesource.com/12867
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
ODOTTYPE should be treated a whole lot like ODOT,
but it was missing completely from the switch in
escwalk and thus escape status did not propagate
to fields.
Since interfaces are required to trigger this bug,
the test was added to escape_iface.go.
Fixes#11931.
Change-Id: Id0383981cc4b1a160f6ad447192a112eed084538
Reviewed-on: https://go-review.googlesource.com/12921
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
There is absolutely no information about how this was failing.
If we reenable the test then at least we can get a build log from
darwin/arm.
There are not even freebsd/arm or netbsd/arm builders,
so not too worried about those. (That is another problem.)
Change-Id: I0e739a4dd2897adbe110aa400d720d8fa02ae65f
Reviewed-on: https://go-review.googlesource.com/12920
Reviewed-by: Russ Cox <rsc@golang.org>
Instead of pushing the denominator argument on the stack,
the denominator is now passed in m.
This fixes a variety of bugs related to trying to take stack traces
backwards from the middle of the software div/mod routines.
Some of those bugs have been kludged around in the past,
but others have not. Instead of trying to patch up after breaking
the stack, this CL stops breaking the stack.
This is an update of https://golang.org/cl/19810043,
which was rolled back in https://golang.org/cl/20350043.
The problem in the original CL was that there were divisions
at bad times, when m was not available. These were divisions
by constant denominators, either in C code or in assembly.
The Go compiler knows how to generate division by multiplication
for constant denominators, but the C compiler did not.
There is no longer any C code, so that's taken care of.
There was one problematic DIV in runtime.usleep (assembly)
but https://golang.org/cl/12898 took care of that one.
So now this approach is safe.
Reject DIV/MOD in NOSPLIT functions to keep them from
coming back.
Fixes#6681.
Fixes#6699.
Fixes#10486.
Change-Id: I09a13c76ad08ba75b3bd5d46a3eb78e66a84ab38
Reviewed-on: https://go-review.googlesource.com/12899
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In order to fix issue #9401 the compiler was changed to add a padding
byte to any non-empty Go struct that ends in a zero-sized field. That
causes the Go version of such a C struct to have a different size than
the C struct, which can considerable confusion. Change cgo so that it
discards any such zero-sized fields, so that the Go and C structs are
the same size.
This is a change from previous releases, in that it used to be
possible to refer to a zero-sized trailing field (by taking its
address), and with this change it no longer is. That is unfortunate,
but something has to change. It seems better to visibly break
programs that do this rather than to silently break programs that rely
on the struct sizes being the same.
Update #9401.
Fixes#11925.
Change-Id: I3fba3f02f11265b3c41d68616f79dedb05b81225
Reviewed-on: https://go-review.googlesource.com/12864
Reviewed-by: Russ Cox <rsc@golang.org>
We want to adjust the DIV calling convention to use m,
and usleep can be called without an m, so switch to a
multiplication by the reciprocal (and test).
Step toward a fix for #6699 and #10486.
Change-Id: Iccf76a18432d835e48ec64a2fa34a0e4d6d4b955
Reviewed-on: https://go-review.googlesource.com/12898
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If a function is large enough to need to flush the constant pool
mid-function, the line number assignment code was forcing the
line numbers not just for the constant pool but for all the instructions
that follow it. This made the line number information completely
wrong for all but the beginning of large functions on arm.
Same problem in code copied into arm64.
This broke runtime/trace's TestTraceSymbolize.
Fixes arm build.
Change-Id: I84d9fb2c798c4085f69b68dc766ab4800c7a6ca4
Reviewed-on: https://go-review.googlesource.com/12894
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This allows running a cross-compile like
GOOS=darwin GOARCH=arm go build std
to check that everything builds.
Otherwise there is a redefinition error because both
root_nocgo_darwin.go and root_darwin_armx.go
supply initSystemRoots.
Change-Id: Ic95976b2b698d28c629bfc93d8dac0048b023578
Reviewed-on: https://go-review.googlesource.com/12897
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The test expects the dial to take 1.0 seconds
on Windows and allows it to go to 1.095 seconds.
That's far too optimistic.
Recent failures are reporting roughly 1.2 seconds.
Let it have 1.5.
Change-Id: Id69811ccb65bf4b4c159301a2b4767deb6ee8d28
Reviewed-on: https://go-review.googlesource.com/12895
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Urge users of math/rand to consider using crypto/rand when doing
security-sensitive work.
Related to issue #11871. While we haven't reached consensus on how
to make the package inherently safer, everyone agrees that the docs
for math/rand can be improved.
Change-Id: I576a312e51b2a3445691da6b277c7b4717173197
Reviewed-on: https://go-review.googlesource.com/12900
Reviewed-by: Rob Pike <r@golang.org>
For the android/arm builder.
Change-Id: Iad4881689223cd6479870da9541524a8cc458cce
Reviewed-on: https://go-review.googlesource.com/12859
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Fixes arm64 builder crash.
The bug is possible on all architectures; you just have to get lucky
and hit a preemption or a stack growth on entry to assertE2I2.
The test stacks the deck.
Change-Id: I8419da909b06249b1ad15830cbb64e386b6aa5f6
Reviewed-on: https://go-review.googlesource.com/12890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It says to disable until #7564 is fixed. It was fixed in April 2014.
Change-Id: I9bebfe96802bafdd2d1a0a47591df346d91b000c
Reviewed-on: https://go-review.googlesource.com/12858
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also make invalidptr control the recently added GC pointer check,
as documented.
Change-Id: Iccfdf49480219d12be8b33b8f03d8312d8ceabed
Reviewed-on: https://go-review.googlesource.com/12857
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The skips added in CL 12579, based on incorrect time stamps,
should be sufficient to identify and exclude all the time-related
flakiness on these systems.
If there is other flakiness, we want to find out.
For #10512.
Change-Id: I5b588ac1585b2e9d1d18143520d2d51686b563e3
Reviewed-on: https://go-review.googlesource.com/12746
Reviewed-by: Austin Clements <austin@google.com>
Nearly all the flaky failures we've seen in trace tests have been
due to the use of time stamps to determine relative event ordering.
This is tricky for many reasons, including:
- different cores might not have exactly synchronized clocks
- VMs are worse than real hardware
- non-x86 chips have different timer resolution than x86 chips
- on fast systems two events can end up with the same time stamp
Stop trying to make time reliable. It's clearly not going to be for Go 1.5.
Instead, record an explicit event sequence number for ordering.
Using our own counter solves all of the above problems.
The trace still contains time stamps, of course. The sequence number
is just used for ordering.
Should alleviate #10554 somewhat. Then tickDiv can be chosen to
be a useful time unit instead of having to be exact for ordering.
Separating ordering and time stamps lets the trace parser diagnose
systems where the time stamp order and actual order do not match
for one reason or another. This CL adds that check to the end of
trace.Parse, after all other sequence order-based checking.
If that error is found, we skip the test instead of failing it.
Putting the check in trace.Parse means that cmd/trace will pick
up the same check, refusing to display a trace where the time stamps
do not match actual ordering.
Using net/http's BenchmarkClientServerParallel4 on various CPU counts,
not tracing vs tracing:
name old time/op new time/op delta
ClientServerParallel4 50.4µs ± 4% 80.2µs ± 4% +59.06% (p=0.000 n=10+10)
ClientServerParallel4-2 33.1µs ± 7% 57.8µs ± 5% +74.53% (p=0.000 n=10+10)
ClientServerParallel4-4 18.5µs ± 4% 32.6µs ± 3% +75.77% (p=0.000 n=10+10)
ClientServerParallel4-6 12.9µs ± 5% 24.4µs ± 2% +89.33% (p=0.000 n=10+10)
ClientServerParallel4-8 11.4µs ± 6% 21.0µs ± 3% +83.40% (p=0.000 n=10+10)
ClientServerParallel4-12 14.4µs ± 4% 23.8µs ± 4% +65.67% (p=0.000 n=10+10)
Fixes#10512.
Change-Id: I173eecf8191e86feefd728a5aad25bf1bc094b12
Reviewed-on: https://go-review.googlesource.com/12579
Reviewed-by: Austin Clements <austin@google.com>
Otherwise the GC may see uninitialized memory there,
which might be old pointers that are retained, or it might
trigger the invalid pointer check.
Fixes#11907.
Change-Id: I67e306384a68468eef45da1a8eb5c9df216a77c0
Reviewed-on: https://go-review.googlesource.com/12852
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Lots and lots of ops!
Also XOR for good measure.
Add a pass to the compiler generator to check that all of the
architecture-specific opcodes are handled by genValue. We will
catch any missing ones if we come across them during compilation,
but probably better to catch them statically.
Change-Id: Ic4adfbec55c8257f88117bc732fa664486262868
Reviewed-on: https://go-review.googlesource.com/12813
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The last time we tried this, linux/arm64 broke.
The series of CLs leading to this one fixes that problem.
Let's try again.
Fixes#9880.
Change-Id: I67bc1d959175ec972d4dcbe4aa6f153790f74251
Reviewed-on: https://go-review.googlesource.com/12849
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The layout code has to date insisted on stack frames that are 16-aligned
including the saved LR, and it ensured this by growing the frame itself.
This breaks code that refers to values near the top of the frame by positive
offset from SP, and in general it's too magical: if you see TEXT xxx, $N,
you expect that the frame size is actually N, not sometimes N and sometimes N+8.
This led to a serious bug in the compiler where ambiguously live values
were not being zeroed correctly, which in turn triggered an assertion
in the GC about finding only valid pointers. The compiler has been
fixed to always emit aligned frames, and the hand-written assembly
has also been fixed.
Now that everything is aligned, make unaligned an error instead of
something to "fix" silently.
For #9880.
Change-Id: I05f01a9df174d64b37fa19b36a6b6c5f18d5ba2d
Reviewed-on: https://go-review.googlesource.com/12848
Reviewed-by: Austin Clements <austin@google.com>
The nosplit stack overflow checks were confused about morestack.
The comment about not having correct SP information at the call
to morestack was true, but that was a real bug, not something to
work around. I fixed that problem in CL 12144. With that fixed,
no need to special-case morestack in the way done here.
This cleanup and simplification of the code was the first step
to fixing a bug that happened when I started working on the
arm64 frame size adjustments, but the cleanup was sufficient
to make the bug go away.
For #9880.
Change-Id: I16b69a5c16b6b8cb4090295d3029c42d606e3b9b
Reviewed-on: https://go-review.googlesource.com/12846
Reviewed-by: Austin Clements <austin@google.com>
arm64 requires either no stack frame or a frame with a size that is 8 mod 16
(adding the saved LR will make it 16-aligned).
The cmd/internal/obj/arm64 has been silently aligning frames, but it led to
a terrible bug when the compiler and obj disagreed on the frame size,
and it's just generally confusing, so we're going to make misaligned frames
an error instead of something that is silently changed.
This CL prepares by updating assembly files.
Note that the changes in this CL are already being done silently by
cmd/internal/obj/arm64, so there is no semantic effect here,
just a clarity effect.
For #9880.
Change-Id: Ibd6928dc5fdcd896c2bacd0291bf26b364591e28
Reviewed-on: https://go-review.googlesource.com/12845
Reviewed-by: Austin Clements <austin@google.com>
If the compiler doesn't do it, cmd/internal/obj/arm64 will,
and that will break the zeroing of ambiguously live values
done in zerorange, which in turn produces uninitialized
pointer cells that the GC trips over.
For #9880.
Change-Id: Ice97c30bc8b36d06b7b88d778d87fab8e1827fdc
Reviewed-on: https://go-review.googlesource.com/12847
Reviewed-by: Austin Clements <austin@google.com>
From compiling go there were 761 functions where OR was needed.
Change-Id: Ied8bf59cec50a3175273387bc7416bd042def6d8
Reviewed-on: https://go-review.googlesource.com/12766
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This adds a GCCPUFraction field to MemStats that reports the
cumulative fraction of the program's execution time spent in the
garbage collector. This is equivalent to the utilization percent shown
in the gctrace output and makes this available programmatically.
This does make one small effect on the gctrace output: we now report
the duration of mark termination up to just before the final
start-the-world, rather than up to just after. However, unlike
stop-the-world, I don't believe there's any way that start-the-world
can block, so it should take negligible time.
While there are many statistics one might want to expose via MemStats,
this is one of the few that will undoubtedly remain meaningful
regardless of future changes to the memory system.
The diff for this change is larger than the actual change. Mostly it
lifts the code for computing the GC CPU utilization out of the
debug.gctrace path.
Updates #10323.
Change-Id: I0f7dc3fdcafe95e8d1233ceb79de606b48acd989
Reviewed-on: https://go-review.googlesource.com/12844
Reviewed-by: Russ Cox <rsc@golang.org>
Currently we only capture GC phase transition times if
debug.gctrace>0, but we're about to compute GC CPU utilization
regardless of whether debug.gctrace is set, so we need these
regardless of debug.gctrace.
Change-Id: If3acf16505a43d416e9a99753206f03287180660
Reviewed-on: https://go-review.googlesource.com/12843
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The following sequence of events can lead to the runtime attempting an
out-of-bounds access on a stack barrier slice:
1. A SIGPROF comes in on a thread while the G on that thread is in
_Gsyscall. The sigprof handler calls gentraceback, which saves a
local copy of the G's stkbar slice. Currently the G has no stack
barriers, so this slice is empty.
2. On another thread, the GC concurrently scans the stack of the
goroutine being profiled (it considers it stopped because it's in
_Gsyscall) and installs stack barriers.
3. Back on the sigprof thread, gentraceback comes across a stack
barrier in the stack and attempts to look it up in its (zero
length) copy of G's old stkbar slice, which causes an out-of-bounds
access.
This commit fixes this by adding a simple cas spin to synchronize the
SIGPROF handler with stack barrier insertion.
In general I would prefer that this synchronization be done through
the G status, since that's how stack scans are otherwise synchronized,
but adding a new lock is a much smaller change and G statuses are full
of subtlety.
Fixes#11863.
Change-Id: Ie89614a6238bb9c6a5b1190499b0b48ec759eaf7
Reviewed-on: https://go-review.googlesource.com/12748
Reviewed-by: Russ Cox <rsc@golang.org>
The scheduler, work buffer's dispose, and write barriers
can conspire to hide the a pointer from the GC's concurent
mark phase. If this pointer is the only path to a large
amount of marking the STW mark termination phase may take
a lot of time.
Consider the following:
1) dispose places a work buffer on the partial queue
2) the GC is busy so it does not immediately remove and
process the work buffer
3) the scheduler runs a mutator whose write barrier dequeues the
work buffer from the partial queue so the GC won't see it
This repeats until the GC reaches the mark termination
phase where the GC finally discovers the pointer along
with a lot of work to do.
This CL fixes the problem by having the mutator
dispose of the buffer to the full queue instead of
the partial queue. Since the write buffer never asks for full
buffers the conspiracy described above is not possible.
Updates #11694.
Change-Id: I2ce832f9657a7570f800e8ce4459cd9e304ef43b
Reviewed-on: https://go-review.googlesource.com/12840
Reviewed-by: Austin Clements <austin@google.com>
These are the old assemblers written in C, and now they are
not needed.
Fixes#10510.
Change-Id: Id9337ffc8eccfd93c84b2e23f427fb1a576b543d
Reviewed-on: https://go-review.googlesource.com/12784
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
At this stage, dist is only building go_bootstrap as cmd/compile and
the rest of the Go toolchain has already been built.
Change-Id: I6f99fa00ff1d3585e215f4ce84d49344c4fcb8a5
Reviewed-on: https://go-review.googlesource.com/12779
Reviewed-by: Russ Cox <rsc@golang.org>
With this, all non-float, non-complex
binary ops found in the standard library
are implemented.
Change-Id: I6087f115229888c0dce10ab35db3fd36a0e0a8b1
Reviewed-on: https://go-review.googlesource.com/12799
Reviewed-by: Keith Randall <khr@golang.org>
Together with teaching SSA to generate static data,
this fixes the encoding/pem and hash/adler32 tests.
Change-Id: I75f81f6c995dcb9c6d99bd3acda94a4feea8b87b
Reviewed-on: https://go-review.googlesource.com/12791
Reviewed-by: Keith Randall <khr@golang.org>
The existing backend recognizes special
assignment statements as being implementable
with static data rather than code.
Unfortunately, it assumes that it is in the middle
of codegen; it emits data and modifies the AST.
This does not play well with SSA's two-phase
bootstrapping approach, in which we attempt to
compile code but fall back to the existing backend
if something goes wrong.
To work around this:
* Add the ability to inquire about static data
without side-effects.
* Save the static data required for a function.
* Emit that static data during SSA codegen.
Change-Id: I2e8a506c866ea3e27dffb597095833c87f62d87e
Reviewed-on: https://go-review.googlesource.com/12790
Reviewed-by: Keith Randall <khr@golang.org>
For integer types less than a machine register, we have to decide
what the invariants are for the high bits of the register. We used
to set the high bits to the correct extension (sign or zero, as
determined by the type) of the low bits.
This CL makes the compiler ignore the high bits of the register
altogether (they are junk).
On this plus side, this means ops that generate subword results don't
have to worry about correctly extending them. On the minus side,
ops that consume subword arguments have to deal with the input
registers not being correctly extended.
For x86, this tradeoff is probably worth it. Almost all opcodes
have versions that use only the correct subword piece of their
inputs. (The one big exception is array indexing.) Not many opcodes
can correctly sign extend on output.
For other architectures, the tradeoff is probably not so clear, as
they don't have many subword-safe opcodes (e.g. 16-bit compare,
ignoring the high 16/48 bits). Fortunately we can decide whether
we do this per-architecture.
For the machine-independent opcodes, we pretend that the "register"
size is equal to the type width, so sign extension is immaterial.
Opcodes that care about the signedness of the input (e.g. compare,
right shift) have two different variants.
Change-Id: I465484c5734545ee697afe83bc8bf4b53bd9df8d
Reviewed-on: https://go-review.googlesource.com/12600
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Russ Cox fixed this issue for other systems
in CL 12026, but the Plan 9 part was forgotten.
Fixes#11656.
Change-Id: I91c033687987ba43d13ad8f42e3fe4c7a78e6075
Reviewed-on: https://go-review.googlesource.com/12762
Reviewed-by: Russ Cox <rsc@golang.org>
The only slice/interface comparisons that reach
the backend are comparisons to nil.
Funcs, maps, and channels are references types,
so pointer equality is enough.
Change-Id: I60a71da46a36202e9bd62ed370ab7d7f2e2800e7
Reviewed-on: https://go-review.googlesource.com/12715
Reviewed-by: Keith Randall <khr@golang.org>
Before this patch there was only partial support for ANDQconst
which was not lowered. This patch added support for AND operations
for all bit sizes and signs.
Change-Id: I3a6b2cddfac5361b27e85fcd97f7f3537ebfbcb6
Reviewed-on: https://go-review.googlesource.com/12761
Reviewed-by: Keith Randall <khr@golang.org>
The function is already defined between syscall_solaris.go and
syscall2_solaris.go.
Change-Id: I034baf7c8531566bebfdbc5a4061352cbcc31449
Reviewed-on: https://go-review.googlesource.com/12773
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reformat some help messages to stay within 80 characters.
Fixes#11840.
Change-Id: Iebafcb616f202ac44405e5897097492a79a51722
Reviewed-on: https://go-review.googlesource.com/12514
Reviewed-by: Rob Pike <r@golang.org>
This change prevents DNS query results using domain search list
overtaking results not using the list unconditionally, which only
happens when using builtin DNS stub resolver.
The previous internal lookup function lookup is split into lookup and
goLookupIPOrder for iteration over a set of names: FQDN or absolute
FQDN, with domain label suffixes in search list, without domain label
suffixes, and for concurrent A and AAAA record queries.
Fixes#11081.
Change-Id: I9ff0640f69276e372d97e709b149ed5b153e8601
Reviewed-on: https://go-review.googlesource.com/10836
Reviewed-by: Russ Cox <rsc@golang.org>
Rename test name from Http to HTTP, and fix some style nits.
Change-Id: I00fe1cecd69ca2f50be86a76ec90031c2f921707
Reviewed-on: https://go-review.googlesource.com/12760
Reviewed-by: Andrew Gerrand <adg@golang.org>
A further attempt to fix raiseproc on Solaris.
Change-Id: I8d8000d6ccd0cd9f029ebe1f211b76ecee230cd0
Reviewed-on: https://go-review.googlesource.com/12771
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I forgot that the libc raise function only sends the signal to the
current thread. We need to actually use kill and getpid here, as we
do on other systems.
Change-Id: Iac34af822c93468bf68cab8879db3ee20891caaf
Reviewed-on: https://go-review.googlesource.com/12704
Reviewed-by: Russ Cox <rsc@golang.org>
I've also changed TestDialSerialAsyncSpuriousConnection for consistency,
although it always computes a finalDeadline of zero.
Note that #11225 is the root cause of the socket leak; this just hides
it from the unit test by restoring the shorter timeout.
Fixes#11878
Change-Id: Ie0037dd3bce6cc81d196765375489f8c61be74c2
Reviewed-on: https://go-review.googlesource.com/12712
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Paul Marks <pmarks@google.com>
Fixes#11436.
Change-Id: I5c4455e9b13b478838f23ac31e6343672dfc60af
Reviewed-on: https://go-review.googlesource.com/12143
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Until cl/12721 and cl/12574, all standard library tests included
runtime/cgo on darwin/arm64 by virtue of package os including it. Now
that is no longer true, runtime/cgo needs to be added by the go tool
just as it is for darwin/arm. (This installs the Mach exception
handler used to properly handle EXC_BAD_ACCESS.)
Fixes#11901
Change-Id: I991525f46eca5b0750b93595579ebc0ff10e47eb
Reviewed-on: https://go-review.googlesource.com/12723
Reviewed-by: Russ Cox <rsc@golang.org>
The new Token API is meant to sit on the side of the Decoder,
so that you only get the new code (and any latent bugs in it)
if you are actively using the Token API.
The unconditional use of dec.peek in dec.tokenPrepareForDecode
violates that intention.
Change tokenPrepareForDecode not to call dec.peek unless needed
(because the Token API has advanced the state).
This restores the old code path behavior, no peeking allowed.
I checked by patching in the new tests from CL 12726 that
this change suffices to "fix" the error handling bug in dec.peek.
Obviously that bug should be fixed too, but the point is that
with this CL, bugs in dec.peek do not affect plain use of Decode
or Unmarshal.
I also checked by putting a panic in dec.peek that the only
tests that now invoke peek are:
TestDecodeInStream
ExampleDecoder_Token
ExampleDecoder_Decode_stream
and those tests all invoke dec.Token directly.
Change-Id: I0b242d0cb54a9c830548644670dc5ab5ccef69f2
Reviewed-on: https://go-review.googlesource.com/12740
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Peter Waldschmidt <peter@waldschmidt.com>
Rules may span multiple lines,
but if we're still unbalanced at the
end of the file, something is wrong.
I write unbalanced rules depressingly often.
Change-Id: Ibd04aa06539e2a0ffef73bb665febf3542fd11f1
Reviewed-on: https://go-review.googlesource.com/12710
Reviewed-by: Keith Randall <khr@golang.org>
This mimics the way the old backend
compiles OCALLMETH.
Change-Id: I635c8e7a48c8b5619bd837f78fa6eeba83a57b2f
Reviewed-on: https://go-review.googlesource.com/12549
Reviewed-by: Keith Randall <khr@golang.org>
From https://github.com/golang/go/issues/11745#issuecomment-123555313
this implements option (b), having the server pause slightly after
sending the final response on a TCP connection when we're about to close
it when we know there's a request body outstanding. This biases the
client (which might not be Go) to prefer our response header over the
request body write error.
Updates #11745
Change-Id: I07cb0b74519d266c8049d9e0eb23a61304eedbf8
Reviewed-on: https://go-review.googlesource.com/12658
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Seems like the simplest solution for 1.5. All the parts of the test
suite I can run on my current device (for which my exception handler
fix no longer works, apparently) pass without this code. I'll move it
into x/mobile/app.
Fixes#11884
Change-Id: I2da40c8c7b48a4c6970c4d709dd7c148a22e8727
Reviewed-on: https://go-review.googlesource.com/12721
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If flushing a value from a register that might be used by the current
old-schedule value, save it to the home location.
This resolves the error that was changed from panic to unimplemented in
CL 12655.
Change-Id: If864be34abcd6e11d6117a061376e048a3e29b3a
Reviewed-on: https://go-review.googlesource.com/12682
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Currently we enter mark 2 by first flushing all existing gcWork caches
and then setting gcBlackenPromptly, which disables further gcWork
caching. However, if a worker or assist pulls a work buffer in to its
gcWork cache after that cache has been flushed but before caching is
disabled, that work may remain in that cache until mark termination.
If that work represents a heap bottleneck (e.g., a single pointer that
is the only way to reach a large amount of the heap), this can force
mark termination to do a large amount of work, resulting in a long
STW.
Fix this by reversing the order of these steps: first disable caching,
then flush all existing caches.
Rick Hudson <rlh> did the hard work of tracking this down. This CL
combined with CL 12672 and CL 12646 distills the critical parts of his
fix from CL 12539.
Fixes#11694.
Change-Id: Ib10d0a21e3f6170a80727d0286f9990df049fed2
Reviewed-on: https://go-review.googlesource.com/12688
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently the GC coordinator enables GC assists at the same time it
enables background mark workers, after the concurrent scan phase is
done. However, this means a rapidly allocating mutator has the entire
scan phase during which to allocate beyond the heap trigger and
potentially beyond the heap goal with no back-pressure from assists.
This prevents the feedback system that's supposed to keep the heap
size under the heap goal from doing its job.
Fix this by enabling mutator assists during the scan phase. This is
safe because the write barrier is already enabled and globally
acknowledged at this point.
There's still a very small window between when the heap size reaches
the heap trigger and when the GC coordinator is able to stop the world
during which the mutator can allocate unabated. This allows *very*
rapidly allocator mutators like TestTraceStress to still occasionally
exceed the heap goal by a small amount (~20 MB at most for
TestTraceStress). However, this seems like a corner case.
Fixes#11677.
Change-Id: I0f80d949ec82341cd31ca1604a626efb7295a819
Reviewed-on: https://go-review.googlesource.com/12674
Reviewed-by: Russ Cox <rsc@golang.org>
Currently we hand-code a set of phases when draining is allowed.
However, this set of phases is conservative. The critical invariant is
simply that the write barrier must be enabled if we're draining.
Shortly we're going to enable mutator assists during the scan phase,
which means we may drain during the scan phase. In preparation, this
commit generalizes these assertions to check the fundamental condition
that the write barrier is enabled, rather than checking that we're in
any particular phase.
Change-Id: I0e1bec1ca823d4a697a0831ec4c50f5dd3f2a893
Reviewed-on: https://go-review.googlesource.com/12673
Reviewed-by: Russ Cox <rsc@golang.org>
Currently we clear both the mark 1 and mark 2 signals at the beginning
of concurrent mark. If either if these is clear, it acts as a signal
to the scheduler that it should start background workers. However,
this means that in the interim *between* mark 1 and mark 2, the
scheduler basically loops starting up new workers only to have them
return with nothing to do. In addition to harming performance and
delaying mutator work, this approach has a race where workers started
for mark 1 can mistakenly signal mark 2, causing it to complete
prematurely. This approach also interferes with starting assists
earlier to fix#11677.
Fix this by initially setting both mark 1 and mark 2 to "signaled".
The scheduler will not start background mark workers, though assists
can still run. When we're ready to enter mark 1, we clear the mark 1
signal and wait for it. Then, when we're ready to enter mark 2, we
clear the mark 2 signal and wait for it.
This structure also lets us deal cleanly with the situation where all
work is drained *prior* to the mark 2 wait, meaning that there may be
no workers to signal completion. Currently we deal with this using a
racy (and possibly incorrect) check for work in the coordinator itself
to skip the mark 2 wait if there's no work. This change makes the
coordinator unconditionally wait for mark completion and makes the
scheduler itself signal completion by slightly extending the logic it
already has to determine that there's no work and hence no use in
starting a new worker.
This is a prerequisite to fixing the remaining component of #11677,
which will require enabling assists during the scan phase. However, we
don't want to enable background workers until the mark phase because
they will compete with the scan. This change lets us use bgMark1 and
bgMark2 to indicate when it's okay to start background workers
independent of assists.
This is also a prerequisite to fixing #11694. It significantly reduces
the occurrence of long mark termination pauses in #11694 (from 64 out
of 1000 to 2 out of 1000 in one experiment).
Coincidentally, this also reduces the final heap size (and hence run
time) of TestTraceStress from ~100 MB and ~1.9 seconds to ~14 MB and
~0.4 seconds because it significantly shortens concurrent mark
duration.
Rick Hudson <rlh> did the hard work of tracking this down.
Change-Id: I12ea9ee2db9a0ae9d3a90dde4944a75fcf408f4c
Reviewed-on: https://go-review.googlesource.com/12672
Reviewed-by: Russ Cox <rsc@golang.org>
Currently, there are three ways to satisfy a GC assist: 1) the mutator
steals credit from background GC, 2) the mutator actually does GC
work, and 3) there is no more work available. 3 was never really
intended as a way to satisfy an assist, and it causes problems: there
are periods when it's expected that the GC won't have any work, such
as when transitioning from mark 1 to mark 2 and from mark 2 to mark
termination. During these periods, there's no back-pressure on rapidly
allocating mutators, which lets them race ahead of the heap goal.
For example, test/init1.go and the runtime/trace test both have small
reachable heaps and contain loops that rapidly allocate large garbage
byte slices. This bug lets these tests exceed the heap goal by several
orders of magnitude.
Fix this by forcing the assist (and hence the allocation) to block
until it can satisfy its debt via either 1 or 2, or the GC cycle
terminates.
This fixes one the causes of #11677. It's still possible to overshoot
the GC heap goal, but with this change the overshoot is almost exactly
by the amount of allocation that happens during the concurrent scan
phase, between when the heap passes the GC trigger and when the GC
enables assists.
Change-Id: I5ef4edcb0d2e13a1e432e66e8245f2bd9f8995be
Reviewed-on: https://go-review.googlesource.com/12671
Reviewed-by: Russ Cox <rsc@golang.org>
Currently it's possible for the GC assist to signal completion of the
mark phase, which puts the GC coordinator goroutine on the current P's
run queue, and then return to mutator code that delays until the next
forced preemption before actually yielding control to the GC
coordinator, dragging out completion of the mark phase. This delay can
be further exacerbated if the mutator makes other goroutines runnable
before yielding control, since this will push the GC coordinator on
the back of the P's run queue.
To fix this, this adds a Gosched to the assist if it completed the
mark phase. This immediately and directly yields control to the GC
coordinator. This already happens implicitly in the background mark
workers because they park immediately after completing the mark.
This is one of the reasons completion of the mark phase is being
dragged out and allowing the mutator to allocate without assisting,
leading to the large heap goal overshoot in issue #11677. This is also
a prerequisite to making the assist block when it can't pay off its
debt.
Change-Id: I586adfbecb3ca042a37966752c1dc757f5c7fc78
Reviewed-on: https://go-review.googlesource.com/12670
Reviewed-by: Russ Cox <rsc@golang.org>
Currently it's possible to perform GC work on a system stack or when
locks are held if there's an allocation that triggers an assist. This
is generally a bad idea because of the fragility of these contexts,
and it's incompatible with two changes we're about to make: one is to
yield after signaling mark completion (which we can't do from a
non-preemptible context) and the other is to make assists block if
there's no other way for them to pay off the assist debt.
This commit simply skips the assist if it's called from a
non-preemptible context. The allocation will still count toward the
assist debt, so it will be paid off by a later assist. There should be
little allocation from non-preemptible contexts, so this shouldn't
harm the overall assist mechanism.
Change-Id: I7bf0e6c73e659fe6b52f27437abf39d76b245c79
Reviewed-on: https://go-review.googlesource.com/12649
Reviewed-by: Russ Cox <rsc@golang.org>
When notetsleep_internal is called from notetsleepg, notetsleepg has
just given up the P, so write barriers are not allowed in
notetsleep_internal.
Change-Id: I1b214fa388b1ea05b8ce2dcfe1c0074c0a3c8870
Reviewed-on: https://go-review.googlesource.com/12647
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently fractional and idle mark workers dispose of their gcWork
cache during mark 2 after incrementing work.nwait and after checking
whether there are any workers or any work available. This creates a
window for two races:
1) If the only remaining work is in this worker's gcWork cache, it
will see that there are no more workers and no more work on the
global lists (since it has not yet flushed its own cache) and
prematurely signal mark 2 completion.
2) After this worker has incremented work.nwait but before it has
flushed its cache, another worker may observe that there are no
more workers and no more work and prematurely signal mark 2
completion.
We can fix both of these by simply moving the cache flush above the
increment of nwait and the test of the completion condition.
This is probably contributing to #11694, though this alone is not
enough to fix it.
Change-Id: Idcf9656e5c460c5ea0d23c19c6c51e951f7716c3
Reviewed-on: https://go-review.googlesource.com/12646
Reviewed-by: Russ Cox <rsc@golang.org>
GC assists are supposed to steal at most the amount of background GC
credit available so that background GC credit doesn't go negative.
However, they are instead stealing the *total* amount of their debt
but only claiming up to the amount of credit that was available. This
results in draining the background GC credit pool too quickly, which
results in unnecessary assist work.
The fix is trivial: steal the amount of work we meant to steal (which
is already computed).
Change-Id: I837fe60ed515ba91c6baf363248069734a7895ef
Reviewed-on: https://go-review.googlesource.com/12643
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
CL https://golang.org/cl/12470 has reportedly fixed the problems that
the misc/cgo/testsovar test encountered on darwin and netbsd. Let's
actually run the test.
Update #10360.
Update #11654.
Change-Id: I4cdd27a8ec8713620e0135780a03f63cfcc538d0
Reviewed-on: https://go-review.googlesource.com/12702
Reviewed-by: Russ Cox <rsc@golang.org>
This race was identified in #9796, but a sequence of fixes
proposed in golang.org/cl/4152 were rolled into
golang.org/cl/5910 which both fixed the race and
modified the name space behavior.
We rolled back the name space changes and lost the race fix.
Fix the race separate from the name space changes,
following the suggestion made by Roger Peppe in
https://go-review.googlesource.com/#/c/4152/7/src/encoding/xml/marshal.go@897Fixes#9796.
Fixes#11885.
Change-Id: Ib2b68982da83dee9e04db8b8465a8295259bba46
Reviewed-on: https://go-review.googlesource.com/12687
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Currently the gctrace output reports the trigger heap size, rather
than the actual heap size at the beginning of GC. Often these are the
same, or at least very close. However, it's possible for the heap to
already have exceeded this trigger when we first check the trigger and
start GC; in this case, this output is very misleading. We've
encountered this confusion a few times when debugging and this
behavior is difficult to document succinctly.
Change the gctrace output to report the actual heap size when GC
starts, rather than the trigger.
Change-Id: I246b3ccae4c4c7ea44c012e70d24a46878d7601f
Reviewed-on: https://go-review.googlesource.com/12452
Reviewed-by: Russ Cox <rsc@golang.org>
Whenever someone pastes gctrace output into GitHub, it helpfully turns
the GC cycle number into a link to some unrelated issue. Prevent this
by removing the pound before the cycle number. The fact that this is a
cycle number is probably more obvious at a glance than most of the
other numbers.
Change-Id: Ifa5fc7fe6c715eac50e639f25bc36c81a132ffea
Reviewed-on: https://go-review.googlesource.com/12413
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
svn dies due to not being able to validate the googlecode.com certificate.
hg does not even attempt to validate it.
Fixes#11806.
Change-Id: I84ced5aa84bb1e4a4cdb2254f2d08a64a1ef23f6
Reviewed-on: https://go-review.googlesource.com/12558
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes TestGoGetWorksWithVanityWildcards,
but that test uses the network and is not run
on the builders.
For #11806.
Change-Id: I35c6677deaf84e2fa9bdb98b62d80d388b5248ae
Reviewed-on: https://go-review.googlesource.com/12557
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Some of these were right; others weren't.
Fixes 'GOGC=off GOSSAPKG=mime go test -a mime'.
The right long term fix is probably to teach the
register allocator about in-place instructions.
In the meantime, all the tests that we can run
now pass.
Change-Id: I8e37b00a5f5e14f241b427d45d5f5cc1064883a2
Reviewed-on: https://go-review.googlesource.com/12664
Reviewed-by: Keith Randall <khr@golang.org>
Prior to this, we were smashing our own stack,
which caused the crypto/sha256 tests to fail.
Change-Id: I7dd94cf466d175b3be0cd65f9c4fe8b1223081fe
Reviewed-on: https://go-review.googlesource.com/12660
Reviewed-by: Daniel Morsing <daniel.morsing@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
This extends https://golang.org/cl/2811, which only applied to Darwin
and GNU/Linux, to all Unix systems.
Fixes#9591.
Change-Id: Iec3fb438564ba2924b15b447c0480f87c0bfd009
Reviewed-on: https://go-review.googlesource.com/12661
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
From https://github.com/golang/go/issues/11745#issuecomment-123555313 :
The http.RoundTripper interface says you get either a *Response, or an
error.
But in the case of a client writing a large request and the server
replying prematurely (e.g. 403 Forbidden) and closing the connection
without reading the request body, what does the client want? The 403
response, or the error that the body couldn't be copied?
This CL implements the aforementioned comment's option c), making the
Transport give an N millisecond advantage to responses over body write
errors.
Updates #11745
Change-Id: I4485a782505d54de6189f6856a7a1f33ce4d5e5e
Reviewed-on: https://go-review.googlesource.com/12590
Reviewed-by: Russ Cox <rsc@golang.org>
We used to use build.Import to get the dependencies, but that meant
we had to repeat the check for every possible GOOS/GOARCH/cgo
combination, which took too long. So we made the test in short mode
only check the current GOOS/GOARCH/cgo combination.
But some combinations can't run the test at all. For example darwin/arm64
does not run tests with a full source file systems, so it cannot test itself,
so nothing was testing darwin/arm64. This led to bugs like #10455
not being caught.
Rewrite the test to read the imports out of the source files ourselves,
so that we can look at all source files in a directory in one pass,
regardless of which GOOS/GOARCH/cgo/etc they require.
This one complete pass runs in the same amount of time as the
old single combination check ran, so we can now test all systems,
even in short mode.
Change-Id: Ie216303c2515bbf1b6fb717d530a0636e271cb6d
Reviewed-on: https://go-review.googlesource.com/12576
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change adds new methods to Decoder.
* Decoder.Token steps through a JSON document, returning a value for each token.
* Decoder.Decode unmarshals the entire value at the token stream's current
position (in addition to its existing function in a stream of JSON values)
Fixes#6050.
Fixes#6499.
Change-Id: Iff283e0e7b537221ae256392aca6529f06ebe211
Reviewed-on: https://go-review.googlesource.com/9073
Reviewed-by: Russ Cox <rsc@golang.org>
A while back we discovered that the dependencies test allowed
arbitrary dependencies for packages we forgot to list.
To stop the damage we added a grandfathered list and fixed
the code to expect unlisted packages to have no dependencies.
This CL replaces the grandfathered list with some more
careful placement of dependency rules.
Thankfully, there were no terrible inversions.
Fixes#10487.
Change-Id: I5a6f92435bd2c66c47ec8ab629edbd88b189f028
Reviewed-on: https://go-review.googlesource.com/12575
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It's not clear this really belongs anywhere at all,
but this is a better place for it than package os.
This way package os can avoid importing "C".
Fixes#10455.
Change-Id: Ibe321a93bf26f478951c3a067d75e22f3d967eb7
Reviewed-on: https://go-review.googlesource.com/12574
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
It was not running because of invalid use of ArchChar.
I didn't catch this when I scrubbed ArchChar from the tree
because this code wasn't in the tree yet.
The test seems to pass, which is nice.
Change-Id: I59761a7a04a73681e147e25c1e7f010068276aa8
Reviewed-on: https://go-review.googlesource.com/12573
Reviewed-by: Robert Griesemer <gri@golang.org>
There is clearly work to do here with respect to xml name spaces,
but I don't believe the changes in this cycle are clearly correct.
The changes in this cycle have visible impact on the generated xml,
possibly breaking existing programs, and yet it's not clear that they
are the end of the story: there is still significant confusion about how
name spaces work or should work (see #9519, #9775, #8167, #7113).
I would like to wait to make breaking changes until we completely
understand what the behavior should be and can evaluate the benefit
of those breaking changes. My main concern here is that we will break
programs in Go 1.5 for the sake of name space adjustments and then
while trying to fix those other bugs we'll break programs in Go 1.6 too.
Let's wait until we know all the changes we want to make before we
decide whether or how to break existing programs.
This CL reverts:
5ae822b encoding/xml: minor changes
bb7e665 encoding/xml: fix xmlns= behavior
9f9d66d encoding/xml: fix default namespace of tags
b69ea01 encoding/xml: fix namespaces in a>b tags
3be158d encoding/xml: encoding name spaces correctly
and adjusts tests from
a9dddb5 encoding/xml: add more EncodeToken tests.
to expect Go 1.4 behavior.
I have confirmed that the name space parts of the test suite
as of this CL passes against the Go 1.4 encoding/xml package,
indicating that this CL successfully restores the Go 1.4 behavior.
(Other tests do not, but that's because there were some real
bug fixes in this cycle that are being kept. Specifically, the
tests that don't pass in Go 1.4 are TestMarshal's NestedAndComment
case, TestEncodeToken's encoding of newlines, and
TestSimpleUseOfEncodeToken returning an error for invalid
token types.)
I also checked that the Go 1.4 tests pass when run against
this copy of the sources.
Fixes#11841.
Change-Id: I97de06761038b40388ef6e3a55547ff43edee7cb
Reviewed-on: https://go-review.googlesource.com/12570
Reviewed-by: Nigel Tao <nigeltao@golang.org>
The "add a Request.Cancel channel" change (https://golang.org/cl/11601)
added support for "race free" cancellation, but introduced a data race. :)
Noticed while running "go test -race net/http". The test is skipped in
short mode, so we never saw it on the dashboard.
Change-Id: Ica14579d8723f8f9d1691e8d56c30b585b332c64
Reviewed-on: https://go-review.googlesource.com/12663
Reviewed-by: Aaron Jacobs <jacobsa@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix an issue where doasm fails if trying to multiply by a larger
than 32 bit const (doasm: notfound ft=9 tt=14 00008 IMULQ
$34359738369, CX 9 14). Fix truncation of 64 to 32 bit integer
when generating LEA causing incorrect values to be computed.
Change-Id: I1e65b63cc32ac673a9bb5a297b578b44c2f1ac8f
Reviewed-on: https://go-review.googlesource.com/12678
Reviewed-by: Keith Randall <khr@golang.org>
