Make the stack traces more readable for new
Go programmers while preserving their utility for old hands.
- Change status number [4] to string.
- Elide frames in runtime package (internal details).
- Swap file:line and arguments.
- Drop 'created by' for main goroutine.
- Show goroutines in order of allocation:
implies main goroutine first if nothing else.
There is no option to get the extra frames back.
Uncomment 'return 1' at the bottom of symtab.c.
$ 6.out
throw: all goroutines are asleep - deadlock!
goroutine 1 [chan send]:
main.main()
/Users/rsc/g/go/src/pkg/runtime/x.go:22 +0x8a
goroutine 2 [select (no cases)]:
main.sel()
/Users/rsc/g/go/src/pkg/runtime/x.go:11 +0x18
created by main.main
/Users/rsc/g/go/src/pkg/runtime/x.go:19 +0x23
goroutine 3 [chan receive]:
main.recv(0xf8400010a0, 0x0)
/Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
/Users/rsc/g/go/src/pkg/runtime/x.go:20 +0x50
goroutine 4 [chan receive (nil chan)]:
main.recv(0x0, 0x0)
/Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
/Users/rsc/g/go/src/pkg/runtime/x.go:21 +0x66
$
$ 6.out index
panic: runtime error: index out of range
goroutine 1 [running]:
main.main()
/Users/rsc/g/go/src/pkg/runtime/x.go:25 +0xb9
$
$ 6.out nil
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x22ca]
goroutine 1 [running]:
main.main()
/Users/rsc/g/go/src/pkg/runtime/x.go:28 +0x211
$
$ 6.out panic
panic: panic
goroutine 1 [running]:
main.main()
/Users/rsc/g/go/src/pkg/runtime/x.go:30 +0x101
$
R=golang-dev, qyzhai, n13m3y3r, r
CC=golang-dev
https://golang.org/cl/4907048
When rnd is called with a second argument of 1, it simply
returns the first argument anyway.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/4820045
Remove complicated PRNG algorithm
(argument is limited by uint16 and can't be <= 1).
Do not require chansend/chanrecv selgen to be bumped with CAS.
R=rsc, ken
CC=golang-dev
https://golang.org/cl/4816041
* Reduces malloc counts during gob encoder/decoder test from 6/6 to 3/5.
The current reflect uses Set to mean two subtly different things.
(1) If you have a reflect.Value v, it might just represent
itself (as in v = reflect.NewValue(42)), in which case calling
v.Set only changed v, not any other data in the program.
(2) If you have a reflect Value v derived from a pointer
or a slice (as in x := []int{42}; v = reflect.NewValue(x).Index(0)),
v represents the value held there. Changing x[0] affects the
value returned by v.Int(), and calling v.Set affects x[0].
This was not really by design; it just happened that way.
The motivation for the new reflect implementation was
to remove mallocs. The use case (1) has an implicit malloc
inside it. If you can do:
v := reflect.NewValue(0)
v.Set(42)
i := v.Int() // i = 42
then that implies that v is referring to some underlying
chunk of memory in order to remember the 42; that is,
NewValue must have allocated some memory.
Almost all the time you are using reflect the goal is to
inspect or to change other data, not to manipulate data
stored solely inside a reflect.Value.
This CL removes use case (1), so that an assignable
reflect.Value must always refer to some other piece of data
in the program. Put another way, removing this case would
make
v := reflect.NewValue(0)
v.Set(42)
as illegal as
0 = 42.
It would also make this illegal:
x := 0
v := reflect.NewValue(x)
v.Set(42)
for the same reason. (Note that right now, v.Set(42) "succeeds"
but does not change the value of x.)
If you really wanted to make v refer to x, you'd start with &x
and dereference it:
x := 0
v := reflect.NewValue(&x).Elem() // v = *&x
v.Set(42)
It's pretty rare, except in tests, to want to use NewValue and then
call Set to change the Value itself instead of some other piece of
data in the program. I haven't seen it happen once yet while
making the tree build with this change.
For the same reasons, reflect.Zero (formerly reflect.MakeZero)
would also return an unassignable, unaddressable value.
This invalidates the (awkward) idiom:
pv := ... some Ptr Value we have ...
v := reflect.Zero(pv.Type().Elem())
pv.PointTo(v)
which, when the API changed, turned into:
pv := ... some Ptr Value we have ...
v := reflect.Zero(pv.Type().Elem())
pv.Set(v.Addr())
In both, it is far from clear what the code is trying to do. Now that
it is possible, this CL adds reflect.New(Type) Value that does the
obvious thing (same as Go's new), so this code would be replaced by:
pv := ... some Ptr Value we have ...
pv.Set(reflect.New(pv.Type().Elem()))
The changes just described can be confusing to think about,
but I believe it is because the old API was confusing - it was
conflating two different kinds of Values - and that the new API
by itself is pretty simple: you can only Set (or call Addr on)
a Value if it actually addresses some real piece of data; that is,
only if it is the result of dereferencing a Ptr or indexing a Slice.
If you really want the old behavior, you'd get it by translating:
v := reflect.NewValue(x)
into
v := reflect.New(reflect.Typeof(x)).Elem()
v.Set(reflect.NewValue(x))
Gofix will not be able to help with this, because whether
and how to change the code depends on whether the original
code meant use (1) or use (2), so the developer has to read
and think about the code.
You can see the effect on packages in the tree in
https://golang.org/cl/4423043/.
R=r
CC=golang-dev
https://golang.org/cl/4435042
The list elements are already being allocated out of a
single memory buffer. We can drop the Link* pointer
following and the memory it requires, replacing it with
index operations.
The change also keeps a channel from containing a pointer
back into its own allocation block, which would create a
cycle. Blocks involved in cycles are not guaranteed to be
finalized properly, and channels depend on finalizers to
free OS-level locks on some systems. The self-reference
was keeping channels from being garbage collected.
runtime-gdb.py will need to be updated in order to dump
the content of buffered channels with the new data structure.
Fixes#1676.
R=ken2, r
CC=golang-dev
https://golang.org/cl/4411045
Also fix comment.
The only caller of chanrecv initializes the value to false, so
this patch makes no difference at present. But it seems like
the right thing to do.
R=rsc
CC=golang-dev
https://golang.org/cl/4312053
Fix problems found.
On amd64, various library routines had bigger
stack frames than expected, because large function
calls had been added.
runtime.assertI2T: nosplit stack overflow
120 assumed on entry to runtime.assertI2T
8 after runtime.assertI2T uses 112
0 on entry to runtime.newTypeAssertionError
-8 on entry to runtime.morestack01
runtime.assertE2E: nosplit stack overflow
120 assumed on entry to runtime.assertE2E
16 after runtime.assertE2E uses 104
8 on entry to runtime.panic
0 on entry to runtime.morestack16
-8 after runtime.morestack16 uses 8
runtime.assertE2T: nosplit stack overflow
120 assumed on entry to runtime.assertE2T
16 after runtime.assertE2T uses 104
8 on entry to runtime.panic
0 on entry to runtime.morestack16
-8 after runtime.morestack16 uses 8
runtime.newselect: nosplit stack overflow
120 assumed on entry to runtime.newselect
56 after runtime.newselect uses 64
48 on entry to runtime.printf
8 after runtime.printf uses 40
0 on entry to vprintf
-8 on entry to runtime.morestack16
runtime.selectdefault: nosplit stack overflow
120 assumed on entry to runtime.selectdefault
56 after runtime.selectdefault uses 64
48 on entry to runtime.printf
8 after runtime.printf uses 40
0 on entry to vprintf
-8 on entry to runtime.morestack16
runtime.selectgo: nosplit stack overflow
120 assumed on entry to runtime.selectgo
0 after runtime.selectgo uses 120
-8 on entry to runtime.gosched
On arm, 5c was tagging functions NOSPLIT that should
not have been, like the recursive function printpanics:
printpanics: nosplit stack overflow
124 assumed on entry to printpanics
112 after printpanics uses 12
108 on entry to printpanics
96 after printpanics uses 12
92 on entry to printpanics
80 after printpanics uses 12
76 on entry to printpanics
64 after printpanics uses 12
60 on entry to printpanics
48 after printpanics uses 12
44 on entry to printpanics
32 after printpanics uses 12
28 on entry to printpanics
16 after printpanics uses 12
12 on entry to printpanics
0 after printpanics uses 12
-4 on entry to printpanics
R=r, r2
CC=golang-dev
https://golang.org/cl/4188061
The sanity checking in pass 2 is wrong
when a select is offering to communicate in
either direction on a channel and neither case
is immediately ready.
R=ken2
CC=golang-dev
https://golang.org/cl/3991047
Close of closed channel panics.
Receive from closed channel never panics,
even if done repeatedly.
Fixes#1349.
Fixes#1419.
R=gri, iant, ken2, r, gri1, r2, iant2, rog, albert.strasheim, niemeyer, ejsherry
CC=golang-dev
https://golang.org/cl/3989042
The o+i*p approach to visiting select cases in random
order stops being fair when there is some case that
is never ready. If that happens, then the case that follows
it in the order gets more chances than the others.
In general the only way to ensure fairness is to make
all permutations equally likely. I've done that by computing
one explicitly.
Makes the permutations correct for n >= 4 where
previously they were broken. For n > 12, there's not
enough randomness to do a perfect job but this should
still be much better than before.
Fixes#1425.
R=r, ken2, ejsherry
CC=golang-dev
https://golang.org/cl/4037043
Prefix all external symbols in runtime by runtime·,
to avoid conflicts with possible symbols of the same
name in linked-in C libraries. The obvious conflicts
are printf, malloc, and free, but hide everything to
avoid future pain.
The symbols left alone are:
** known to cgo **
_cgo_free
_cgo_malloc
libcgo_thread_start
initcgo
ncgocall
** known to linker **
_rt0_$GOARCH
_rt0_$GOARCH_$GOOS
text
etext
data
end
pclntab
epclntab
symtab
esymtab
** known to C compiler **
_divv
_modv
_div64by32
etc (arch specific)
Tested on darwin/386, darwin/amd64, linux/386, linux/amd64.
Built (but not tested) for freebsd/386, freebsd/amd64, linux/arm, windows/386.
R=r, PeterGo
CC=golang-dev
https://golang.org/cl/2899041
data just read from the channel.
this will make it easier to
recognize when to garbage
collect and finalize.
R=rsc
CC=golang-dev
https://golang.org/cl/882043
* adds pass 3 to dequeue from channels eagerly
various other cleanup/churn:
* use switch on cas->send in each pass to
factor out common code.
* longer goto labels, commented at target
* be more agressive about can't happen:
throw instead of print + cope.
* use "select" instead of "selectgo" in errors
* use printf for debug prints when possible
R=ken2, ken3
CC=golang-dev, r
https://golang.org/cl/875041
(Thanks to ken and rsc for pointing this out)
rsc:
ken pointed out that there's a race in the new
one-lock-per-channel code. the issue is that
if one goroutine has gone to sleep doing
select {
case <-c1:
case <-c2:
}
and then two more goroutines try to send
on c1 and c2 simultaneously, the way that
the code makes sure only one wins is the
selgen field manipulation in dequeue:
// if sgp is stale, ignore it
if(sgp->selgen != sgp->g->selgen) {
//prints("INVALID PSEUDOG POINTER\n");
freesg(c, sgp);
goto loop;
}
// invalidate any others
sgp->g->selgen++;
but because the global lock is gone both
goroutines will be fiddling with sgp->g->selgen
at the same time.
This results in a 7% slowdown in the single threaded case for a
ping-pong microbenchmark.
Since the cas predominantly succeeds, adding a simple check first
didn't make any difference.
R=rsc
CC=golang-dev
https://golang.org/cl/180068
On a microbenchmark that ping-pongs on lots of channels, this makes
the multithreaded case about 20% faster and the uniprocessor case
about 1% slower. (Due to cache effects, I expect.)
R=rsc, agl
CC=golang-dev
https://golang.org/cl/166043
selfree maintains a cache of Select structures for several sizes. In
newselect, we'll use an entry from the cache if one is found. However,
the Scase structures corresponding to a send may have been allocated
for the wrong size. In this case we'll write off the end of the Scase
into random memory and, generally, read some amount of junk in the
receive.
This patch fixes the issue by removing the cache, on the advice of
rsc.
R=rsc
CC=go-dev
http://go/go-review/1016002
because they are in package runtime.
another step to enforcing package boundaries.
R=r
DELTA=732 (114 added, 93 deleted, 525 changed)
OCL=35811
CL=35824
