Michael Munday [Sun, 20 Mar 2016 23:25:58 +0000 (19:25 -0400)]
test/fixedbugs: add s390x case to issue11656
An instruction consisting of all 0s causes an illegal instruction
signal on s390x. Since 0s are the default in this test this CL just
makes it explicit.
can never be true: if dup != nil, then s.Name is ".dup" (and this is not new:
the same broken logic is present in 1.4, at least). Delete the whole block.
Dave Cheney [Mon, 21 Mar 2016 03:19:57 +0000 (14:19 +1100)]
cmd/internal/obj: move Nocache helper to arm back end
The obj.Nocache helper was only used by the arm back end, move it there.
Change-Id: I5c9faf995499991ead1f3d8c8ffc3b6af7346876
Reviewed-on: https://go-review.googlesource.com/20868 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Matthew Dempsky [Sun, 20 Mar 2016 21:54:13 +0000 (14:54 -0700)]
math/big: cleanup documentation for Format methods
'b' is a standard verb for floating point values. The runes like '+'
and '#' are called "flags" by package fmt's documentation. The flag
'-' controls left/right justification, not anything related to signs.
Richard Miller [Sun, 20 Mar 2016 19:17:36 +0000 (19:17 +0000)]
net/http: adaptive wait time in PersistConnLeak tests
In tests TransportPersistConnLeak and TransportPersistConnLeakShortBody,
there's a fixed wait time (100ms and 400ms respectively) to allow
goroutines to exit after CloseIdleConnections is called. This
is sometimes too short on a slow host running many simultaneous
tests.
This CL replaces the fixed sleep in each test with a sequence of
shorter sleeps, testing the number of remaining goroutines until
it reaches the threshold or an overall time limit of 500ms expires.
This prevents some failures in the plan9_arm builder, while reducing
the test time on faster machines.
Ian Lance Taylor [Sun, 20 Mar 2016 17:25:46 +0000 (10:25 -0700)]
cmd/compile: fix varexpr handling of ODOT
For a long time varexpr has handled ODOT incorrectly: it has always
returned false. Before https://golang.org/cl/20890 this has been
because an ODOT had a Right field with an ONAME with no Class, for which
varexpr returns false. CL 20890 preserved the behavior of varexpr for
ODOT, so that the change would pass toolstash -cmp.
This CL fixes varexpr so that ODOT can return true in some cases. This
breaks toolstash -cmp. While the changed compiler allocates temporary
variables in a different order, I have not been able to find any
examples where the generated code is different, other than using
different stack offsets and, in some cases, registers. It seems that
other parts of the compiler will force the ODOT into a temporary anyhow.
Still, this change is clearly correct, and is a minor compiler cleanup.
Change-Id: I71506877aa3c13966bb03c281aa16271ee7fe80a
Reviewed-on: https://go-review.googlesource.com/20907
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Sat, 19 Mar 2016 01:12:14 +0000 (18:12 -0700)]
cmd/compile: don't penalize ODOT and friends when inlining
Historically ODOT and friends have been considered to cost an extra
budget point when deciding whether they should be inlined, because they
had an ONAME node that represented the name to the right of the dot.
This doesn't really make sense, as in general that symbol does not add
any extra instructions; it just affects the offset of the load or store
instruction. And the ONAME node is gone now. So, remove the extra
cost.
This does not pass toolstash -cmp, as it changes inlining decisions.
For example, mspan.init in runtime/mheap.go is now considered to be an
inlining candidate.
Change-Id: I5ad27f08c66fd5daa4c8472dd0795df989183f5e
Reviewed-on: https://go-review.googlesource.com/20891 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Matthew Dempsky [Sun, 20 Mar 2016 00:51:17 +0000 (17:51 -0700)]
cmd/compile: ignore receiver name when checking duplicate methods
In golang.org/cl/20602, I changed the semantics of Eqtype to stop
checking the receiver parameters for type equality, and pushed this
responsibility to addmethod (the only Eqtype caller that cared).
However, I accidentally made the check stricter by making it start
requiring that receiver names were identical.
In general, this is a non-problem because the receiver names in export
data will always match the original source. But running
GO_GCFLAGS=-newexport ./all.bash at one point tries to load both old
and new format export data for package sync, which reveals the
problem. (See golang.org/issue/14877 for details.)
Easy fix: just check the receiver type for type equality in addmethod,
instead of the entire receiver parameter list.
Martin Möhrmann [Sat, 19 Mar 2016 12:18:43 +0000 (13:18 +0100)]
fmt: remove depth argument from handleMethods and printArg
handleMethods can format Error() and String() directly as its known
these return strings that can be directly printed using fmtString.
Remove the obsolete depth argument from handleMethods.
Remove the depth argument from printArg since it is only ever
called with depth set to 0. Recursion for formatting complex
arguments is handled only by printValue which keeps track of depth.
Change-Id: I4c4be588751de12ed999e7561a51bc168eb9eb2d
Reviewed-on: https://go-review.googlesource.com/20911
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Ian Lance Taylor [Fri, 18 Mar 2016 23:52:30 +0000 (16:52 -0700)]
cmd/compile: change ODOT and friends to use Sym, not Right
The Node type ODOT and its variants all represent a selector, with a
simple name to the right of the dot. Before this change this was
represented by using an ONAME Node in the Right field. This ONAME node
served no useful purpose. This CL changes these Node types to store the
symbol in the Sym field instead, thus not requiring allocating a Node
for each selector.
When compiling x/tools/go/types this CL eliminates nearly 5000 calls to
newname and reduces the total number of Nodes allocated by about 6.6%.
It seems to cut compilation time by 1 to 2 percent.
Getting this right was somewhat subtle, and I added two dubious changes
to produce the exact same output as before. One is to ishairy in
inl.go: the ONAME node increased the cost of ODOT and friends by 1, and
I retained that, although really ODOT is not more expensive than any
other node. The other is to varexpr in walk.go: because the ONAME in
the Right field of an ODOT has no class, varexpr would always return
false for an ODOT, although in fact for some ODOT's it seemingly ought
to return true; I added an && false for now. I will send separate CLs,
that will break toolstash -cmp, to clean these up.
Todd Neal [Wed, 16 Mar 2016 23:44:17 +0000 (18:44 -0500)]
cmd/compile: allow inlining of functions that declare a const
Consider functions with an ODCLCONST for inlining and modify exprfmt to
ignore those nodes when exporting. Don't add symbols to the export list
if there is no definition. This occurs when OLITERAL symbols are looked
up via Pkglookup for non-exported symbols.
Martin Möhrmann [Fri, 18 Mar 2016 15:56:47 +0000 (16:56 +0100)]
fmt: simplify handling of reporting flags to formatters
Remove rewriting of flags before calling formatters.
Change Flag method to directly take plusV and sharpV flags
into account when reporting if plus or sharp flag is set.
Change-Id: Ic3423881ad89e5a5f9fff5ab59e842062394ef6d
Reviewed-on: https://go-review.googlesource.com/20859
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Marcel van Lohuizen [Fri, 29 Jan 2016 15:55:35 +0000 (16:55 +0100)]
testing: hoisted chunks of code to prepare for Run method
testing.go:
- run method will evolve into the Run method.
- added level field in common
benchmark.go:
- benchContext will be central to distinguish handling of benchmarks
between normal Run methods and ones called from within Benchmark
function.
- expandCPU will evolve into the processing hook for Run methods
called within normal processing.
- runBench will evolve into the Run method.
Marcel van Lohuizen [Tue, 19 Jan 2016 21:43:52 +0000 (22:43 +0100)]
testing: prepare for the introduction of Run methods
The biggest change is that each test is now responsible for managing
the starting and stopping of its parallel subtests.
The "Main" test could be run as a tRunner as well. This shows that
the introduction of subtests is merely a generalization of and
consistent with the current semantics.
Christopher Nelson [Tue, 15 Mar 2016 13:14:22 +0000 (09:14 -0400)]
cmd/go: fix TestShadowingLogic fails when GOROOT path has spaces
Improve the test by also translating " " to "_".
Fixes #14671.
Change-Id: Ie5997934b93c7663d7b8432244fad47bb5d3ffbe
Reviewed-on: https://go-review.googlesource.com/20714 Reviewed-by: Ian Lance Taylor <iant@golang.org>
David Chase [Thu, 17 Mar 2016 18:12:12 +0000 (14:12 -0400)]
cmd/compile: get gcflags to bootstrap; ssa debug opts for "all"
This is intended to help debug compiler problems that pop
up in the bootstrap phase of make.bash. GO_GCFLAGS does not
normally apply there. Options-for-all phases is intended
to allow crude tracing (and full timing) by turning on timing
for all phases, not just one.
Phase names can also be specified using a regular expression,
for example
BOOT_GO_GCFLAGS=-d='ssa/~^.*scc$/off' \
GO_GCFLAGS='-d=ssa/~^.*scc$/off' ./make.bash
I just added this because it was the fastest way to get
me to a place where I could easily debug the compiler.
Change-Id: I0781f3e7c19651ae7452fa25c2d54c9a245ef62d
Reviewed-on: https://go-review.googlesource.com/20775 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mikio Hara [Wed, 16 Mar 2016 20:33:13 +0000 (05:33 +0900)]
net: make unexposed methods start with lowercase letters
This change makes unexposed methods start with lowercase letters for
avoiding unnecessary confusion because the net package uses many
embedding structures and intrefaces for controlling exposure of APIs.
Note that this change leaves DNS-related methods as they are.
Change-Id: I253758d1659175c5d0af6b2efcd30ce83f46543d
Reviewed-on: https://go-review.googlesource.com/20784
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alexandru Moșoi [Sun, 13 Mar 2016 21:12:03 +0000 (22:12 +0100)]
encoding/binary: remove bound checks from conversions.
* This the simplest solution I could came up with
that doesn't required changing the compiler.
* The bound checks become constants now
so they are removed during opt phase.
Matthew Dempsky [Thu, 17 Mar 2016 08:47:16 +0000 (01:47 -0700)]
cmd/compile: get rid of Type's {This,In,Out}tuple fields
Boolean expressions involving t.Thistuple were converted to use
t.Recv(), because it's a bit clearer and will hopefully reveal cases
where we could remove redundant calls to t.Recv() (in followup CLs).
The other cases were all converted to use t.Recvs().NumFields(),
t.Params().NumFields(), or t.Results().NumFields().
Ian Lance Taylor [Thu, 17 Mar 2016 05:22:58 +0000 (22:22 -0700)]
cmd/compile: don't create 2 Sym's and 2 Node's for every string
For every string constant the compiler was creating 2 Sym's and 2
Node's. It would never refer to them again, but would keep them alive
in gostringpkg. This changes the code to just use obj.LSym's instead.
When compiling x/tools/go/types, this yields about a 15% reduction in
the number of calls to newname and a 3% reduction in the total number of
Node objects. Unfortunately I couldn't see any change in compile time,
but reducing memory usage is desirable anyhow.
David Chase [Mon, 29 Feb 2016 15:43:18 +0000 (10:43 -0500)]
cmd/compile: escape analysis explanations added to -m -m output
This should probably be considered "experimental" at this stage, but
what it needs is feedback from adventurous adopters. I think the data
structure used for describing escape reasons might be extendable to
allow a cleanup of the underlying algorithms, which suffers from
insufficiently separated concerns (the graph does not deal well with
escape level adjustments, so it is augmented by a second custom-walk
portion of the "flood" phase. It would be better to put it all,
including level adjustments, in a single graph structure, and then
simply flood the graph.
Tweaked to avoid allocations in the no-logging case.
Modified run.go to ignore lines with leading "#" in the output (since
it can never match a line), and in -update_errors to ignore leading
tabs in output lines and to normalize embedded filenames.
Currently requires -m -m because otherwise the noise/update
burden for the other escape tests is considerable.
There is a partial test. Existing escape analysis tests seem to
cover all except the panic case and what looks like it might be
unreachable code in escape analysis.
Richard Miller [Thu, 17 Mar 2016 10:20:54 +0000 (10:20 +0000)]
syscall: avoid failure in plan9 StartProcess from fd close race
Between the enumeration of fdsToClose in the parent and the
closing of fds in the child, it's possible for a file to be
closed in another thread. If that file descriptor is reused
when opening the child-parent status pipe, it will be closed
prematurely in the child and the forkExec gets out of sync.
This has been observed to cause failures in builder tests
when the link step of a build is started before the compile
step has run, with "file does not exist" messages as the
visible symptom.
The simple workaround is to check against closing the pipe.
A more comprehensive solution would be to rewrite the fd
closing code to avoid races, along the lines of the long
ago proposed https://golang.org/cl/57890043 - but meanwhile
this correction will prevent some builder failures.
Change-Id: I4ef5eaea70c21d00f4df0e0847a1c5b2966de7da
Reviewed-on: https://go-review.googlesource.com/20800
Run-TryBot: David du Colombier <0intro@gmail.com> Reviewed-by: David du Colombier <0intro@gmail.com>
Martin Möhrmann [Fri, 11 Mar 2016 12:29:23 +0000 (13:29 +0100)]
fmt: separate unicode and integer formatting
Separate unicode formatting into its own fmt_unicode function.
Remove the fmtUnicode wrapper and the f.unicode and f.uniQuote
flags that are not needed anymore. Remove mangling and restoring
of the precision and sharp flags.
Removes the buffer copy needed for %#U by moving
the character encoding before the number encoding.
Changes the behavior of plus and space flag to have
no effect instead of printing a plus or space before "U+".
Always print at least four digits after "U+"
even if precision is set to less than 4.
Change-Id: If9a0ee79e9eca2c76f06a4e0fdd75d98393899ac
Reviewed-on: https://go-review.googlesource.com/20574
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Keith Randall [Wed, 16 Mar 2016 03:45:50 +0000 (20:45 -0700)]
cmd/compile: keep value use counts in SSA
Keep track of how many uses each Value has. Each appearance in
Value.Args and in Block.Control counts once.
The number of uses of a value is generically useful to
constrain rewrite rules. For instance, we might want to
prevent merging index operations into loads if the same
index expression is used lots of times.
But I have one use in particular for which the use count is required.
We must make sure we don't combine ops with loads if the load has
more than one use. Otherwise, we may split a single load
into multiple loads and that breaks perceived behavior in
the presence of races. In particular, the load of m.state
in sync/mutex.go:Lock can't be done twice. (I have a separate
CL which triggers the mutex failure. This CL has a test which
demonstrates a similar failure.)
Michael Hudson-Doyle [Wed, 16 Mar 2016 23:38:51 +0000 (12:38 +1300)]
cmd/link: do not add duplicate symbols to Allsym
When building shared libraries, all symbols on Allsym are marked reachable.
What I didn't realize was that this includes the ".dup" symbols created when
"dupok" symbols are read from multiple package files. This breaks now because
deadcode makes some assumptions that fail for these ".dup" symbols, but in any
case was a bad idea -- I suspect this change makes libstd.so a bunch smaller,
but creating it was broken before this CL so I can't be sure.
This change simply stops adding these symbols to Allsym, which might make some
of the many iterations over Allsym the linker does a touch quicker, although
that's not the motivation here.
Add a test that no symbols called ".dup" makes it into the runtime shared
library.
Fixes #14841
Change-Id: I65dd6e88d150a770db2d01b75cfe5db5fd4f8d25
Reviewed-on: https://go-review.googlesource.com/20780
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Matthew Dempsky [Fri, 11 Mar 2016 22:38:16 +0000 (14:38 -0800)]
cmd/compile: ignore receiver parameters in Eqtype
Receiver parameters generally aren't relevant to the function
signature type. In particular:
1. When checking whether a type's method implements an interface's
method, we specifically want to ignore the receiver parameters,
because they'll be different.
2. When checking interface type equality, interface methods always
use the same "fakethis" *struct{} type as their receiver.
3. Finally, method expressions and method values degenerate into
receiver-less function types.
The only case where we care about receiver types matching is in
addmethod, which is easily handled by adding an extra Eqtype check of
the receiver parameters. Also, added a test for this, since
(surprisingly) there weren't any.
As precedence, go/types.Identical ignores receiver parameters when
comparing go/types.Signature values.
Notably, this allows us to slightly simplify the "implements"
function, which is used for checking whether type/interface t
implements interface iface. Currently, cmd/compile actually works
around Eqtype's receiver parameter checking by creating new throwaway
TFUNC Types without the receiver parameter.
(Worse, the compiler currently only provides APIs to build TFUNC Types
from Nod syntax trees, so building those throwaway types also involves
first building throwaway syntax trees.)
Passes toolstash -cmp.
Change-Id: Ib07289c66feacee284e016bc312e8c5ff674714f
Reviewed-on: https://go-review.googlesource.com/20602 Reviewed-by: Robert Griesemer <gri@golang.org>
James Bardin [Wed, 16 Mar 2016 17:53:53 +0000 (13:53 -0400)]
cmd/cgo: add C.CBytes
Add a C.CBytes function to copy a Go byte slice into C memory. This
returns an unsafe.Pointer, since that is what needs to be passed to
C.free, and the data is often opaque bytes anyway.
Fixes #14838
Change-Id: Ic7bc29637eb6f1f5ee409b3898c702a59833a85a
Reviewed-on: https://go-review.googlesource.com/20762 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Wed, 16 Mar 2016 22:22:58 +0000 (18:22 -0400)]
cmd/compile: omit write barrier when assigning global function
Currently we generate write barriers when the right side of an
assignment is a global function. This doesn't fall into the existing
case of storing an address of a global because we haven't lowered the
function to a pointer yet.
This write barrier is unnecessary, so eliminate it.
Robert Griesemer [Wed, 16 Mar 2016 21:02:56 +0000 (14:02 -0700)]
cmd/compile: remove dead code handling '~' operator
The parser code was not reachable ever since some of the lexer cleanups.
We could recognize '~' in the lexer, complain, and return a '^' instead,
but it's been a few years since Go was new and this may have been a use-
ful error. The lexer complains with "illegal character U+007E '~'" which
is good enough.
For #13244.
Change-Id: Ie3283738486eb6f8462d594f2728ac98333c0520
Reviewed-on: https://go-review.googlesource.com/20768 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change-Id: I9623a40ff0d568f11afd1279b6aaa1c33eda644c
Reviewed-on: https://go-review.googlesource.com/20730 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Mon, 15 Feb 2016 23:30:48 +0000 (18:30 -0500)]
runtime: shrink stacks during concurrent mark
Currently we shrink stacks during STW mark termination because it used
to be unsafe to shrink them concurrently. For some programs, this
significantly increases pause time: stack shrinking costs ~5ms/MB
copied plus 2µs/shrink.
Now that we've made it safe to shrink a stack without the world being
stopped, shrink them during the concurrent mark phase.
This reduces the STW time in the program from issue #12967 by an order
of magnitude and brings it from over the 10ms goal to well under:
name old 95%ile-markTerm-time new 95%ile-markTerm-time delta
Stackshrink-4 23.8ms ±60% 1.80ms ±39% -92.44% (p=0.008 n=5+5)
Fixes #12967.
This slows down the go1 and garbage benchmarks overall by < 0.5%.
name old time/op new time/op delta
XBenchGarbage-12 2.48ms ± 1% 2.49ms ± 1% +0.45% (p=0.005 n=25+21)
Austin Clements [Mon, 15 Feb 2016 23:24:06 +0000 (18:24 -0500)]
runtime: generalize work.finalizersDone to work.markrootDone
We're about to add another root marking job that needs to happen only
during the first markroot pass (whether that's concurrent or STW),
just like finalizer scanning. Rather than introducing another flag
that has the same value as finalizersDone, just rename finalizersDone
to markrootDone.
Austin Clements [Mon, 15 Feb 2016 22:38:06 +0000 (17:38 -0500)]
runtime: make shrinkstack concurrent-safe
Currently shinkstack is only safe during STW because it adjusts
channel-related stack pointers and moves send/receive stack slots
without synchronizing with the channel code. Make it safe to use when
the world isn't stopped by:
1) Locking all channels the G is blocked on while adjusting the sudogs
and copying the area of the stack that may contain send/receive
slots.
2) For any stack frames that may contain send/receive slot, using an
atomic CAS to adjust pointers to prevent races between adjusting a
pointer in a receive slot and a concurrent send writing to that
receive slot.
In principle, the synchronization could be finer-grained. For example,
we considered synchronizing around the sudogs, which would allow
channel operations involving other Gs to continue if the G being
shrunk was far enough down the send/receive queue. However, using the
channel lock means no additional locks are necessary in the channel
code. Furthermore, the stack shrinking code holds the channel lock for
a very short time (much less than the time required to shrink the
stack).
This does not yet make stack shrinking concurrent; it merely makes
doing so safe.
This has negligible effect on the go1 and garbage benchmarks.
Austin Clements [Sun, 21 Feb 2016 15:40:39 +0000 (10:40 -0500)]
runtime: define lock order between G status and channel lock
Currently, locking a G's stack by setting its status to _Gcopystack or
_Gscan is unordered with respect to channel locks. However, when we
make stack shrinking concurrent, stack shrinking will need to lock the
G and then acquire channel locks, which imposes an order on these.
Document this lock ordering and fix closechan to respect it.
Everything else already happens to respect it.
Austin Clements [Thu, 18 Feb 2016 14:34:43 +0000 (09:34 -0500)]
runtime: protect sudog.elem with hchan.lock
Currently sudog.elem is never accessed concurrently, so in several
cases we drop the channel lock just before reading/writing the
sent/received value from/to sudog.elem. However, concurrent stack
shrinking is going to have to adjust sudog.elem to point to the new
stack, which means it needs a way to synchronize with accesses to
sudog.elem. Hence, add sudog.elem to the fields protected by
hchan.lock and scoot the unlocks down past the uses of sudog.elem.
While we're here, better document the channel synchronization rules.
Austin Clements [Thu, 25 Feb 2016 20:37:40 +0000 (15:37 -0500)]
runtime: fix transient _Gwaiting states in newstack
With concurrent stack shrinking, the stack can move the instant after
a G enters _Gwaiting. There are only two places that put a G into
_Gwaiting: gopark and newstack. We fixed uses of gopark. This commit
fixes newstack by simplifying its G transitions and, in particular,
eliminating or narrowing the transient _Gwaiting states it passes
through so it's clear nothing in the G is accessed while in _Gwaiting.
Austin Clements [Fri, 26 Feb 2016 15:50:54 +0000 (10:50 -0500)]
runtime: never pass stack pointers to gopark
gopark calls the unlock function after setting the G to _Gwaiting.
This means it's generally unsafe to access the G's stack from the
unlock function because the G may start running on another P. Once we
start shrinking stacks concurrently, a stack shrink could also move
the stack the moment after it enters _Gwaiting and before the unlock
function is called.
Document this restriction and fix the two places where we currently
violate it.
This is unlikely to be a problem in practice for these two places
right now, but they're already skating on thin ice. For example, the
following sequence could in principle cause corruption, deadlock, or a
panic in the select code:
On M1/P1:
1. G1 selects on channels A and B.
2. selectgoImpl calls gopark.
3. gopark puts G1 in _Gwaiting.
4. gopark calls selparkcommit.
5. selparkcommit releases the lock on channel A.
On M2/P2:
6. G2 sends to channel A.
7. The send puts G1 in _Grunnable and puts it on P2's run queue.
8. The scheduler runs, selects G1, puts it in _Grunning, and resumes G1.
9. On G1, the sellock immediately following the gopark gets called.
10. sellock grows and moves the stack.
On M1/P1:
11. selparkcommit continues to scan the lock order for the next
channel to unlock, but it's now reading from a freed (and possibly
reused) stack.
This shouldn't happen in practice because step 10 isn't the first call
to sellock, so the stack should already be big enough. However, once
we start shrinking stacks concurrently, this reasoning won't work any
more.
Austin Clements [Tue, 16 Feb 2016 16:06:00 +0000 (11:06 -0500)]
runtime: put g.waiting list in lock order
Currently the g.waiting list created by a select is in poll order.
However, nothing depends on this, and we're going to need access to
the channel lock order in other places shortly, so modify select to
put the waiting list in channel lock order.
For #12967.
Change-Id: If0d38816216ecbb37a36624d9b25dd96e0a775ec
Reviewed-on: https://go-review.googlesource.com/20037 Reviewed-by: Rick Hudson <rlh@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Austin Clements [Tue, 16 Feb 2016 04:50:58 +0000 (23:50 -0500)]
runtime: use indexes for select lock order
Currently the select lock order is a []*hchan. We're going to need to
refer to things other than the channel itself in lock order shortly,
so switch this to a []uint16 of indexes into the select cases. This
parallels the existing representation for the poll order.
Change-Id: I89262223fe20b4ddf5321592655ba9eac489cda1
Reviewed-on: https://go-review.googlesource.com/20036 Reviewed-by: Rick Hudson <rlh@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>