Skip to content

G
golang
Switch branch/tag
  1. 04 Apr, 2014 2 commits
    • Russ Cox's avatar
      cmd/gc, runtime: make GODEBUG=gcdead=1 mode work with liveness · 28f1868f
      Russ Cox authored 
      Trying to make GODEBUG=gcdead=1 work with liveness
and in particular ambiguously live variables.

1. In the liveness computation, mark all ambiguously live
variables as live for the entire function, except the entry.
They are zeroed directly after entry, and we need them not
to be poisoned thereafter.

2. In the liveness computation, compute liveness (and deadness)
for all parameters, not just pointer-containing parameters.
Otherwise gcdead poisons untracked scalar parameters and results.

3. Fix liveness debugging print for -live=2 to use correct bitmaps.
(Was not updated for compaction during compaction CL.)

4. Correct varkill during map literal initialization.
Was killing the map itself instead of the inserted value temp.

5. Disable aggressive varkill cleanup for call arguments if
the call appears in a defer or go statement.

6. In the garbage collector, avoid bug scanning empty
strings. An empty string is two zeros. The multiword
code only looked at the first zero and then interpreted
the next two bits in the bitmap as an ordinary word bitmap.
For a string the bits are 11 00, so if a live string was zero
length with a 0 base pointer, the poisoning code treated
the length as an ordinary word with code 00, meaning it
needed poisoning, turning the string into a poison-length
string with base pointer 0. By the same logic I believe that
a live nil slice (bits 11 01 00) will have its cap poisoned.
Always scan full multiword struct.

7. In the runtime, treat both poison words (PoisonGC and
PoisonStack) as invalid pointers that warrant crashes.

Manual testing as follows:

- Create a script called gcdead on your PATH containing:

        #!/bin/bash
        GODEBUG=gcdead=1 GOGC=10 GOTRACEBACK=2 exec "$@"
- Now you can build a test and then run 'gcdead ./foo.test'.
- More importantly, you can run 'go test -short -exec gcdead std'
   to run all the tests.

Fixes #7676.

While here, enable the precise scanning of slices, since that was
disabled due to bugs like these. That now works, both with and
without gcdead.

Fixes #7549.

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83410044
      28f1868f
    • Mikio Hara's avatar
      net: don't export netFD closeRead and closeWrite methods · ebe5f203
      Mikio Hara authored 
      LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/83910043
      ebe5f203
  3. 03 Apr, 2014 20 commits
  5. 02 Apr, 2014 13 commits
    • Dave Cheney's avatar
      cmd/pack: fix format string error in log message · 1aaea50c
      Dave Cheney authored 
      Fixes #7693.

pack.go:347: possible formatting directive in Fatal call

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/83310045
      1aaea50c
    • Brad Fitzpatrick's avatar
      crypto/tls: deflake TestConnReadNonzeroAndEOF · 84db9e09
      Brad Fitzpatrick authored 
      Fixes #7683

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/83080048
      84db9e09
    • Russ Cox's avatar
      runtime: revert change to PoisonPtr value · 81bc9b3f
      Russ Cox authored 
      Submitted accidentally in CL 83630044.
Fixes various builds.

TBR=khr
CC=golang-codereviews
https://golang.org/cl/83100047
      81bc9b3f
    • Russ Cox's avatar
      cmd/gc, cmd/ld, runtime: compact liveness bitmaps · 4676fae5
      Russ Cox authored 
      Reduce footprint of liveness bitmaps by about 5x.

1. Mark all liveness bitmap symbols as 4-byte aligned
(they were aligned to a larger size by default).

2. The bitmap data is a bitmap count n followed by n bitmaps.
Each bitmap begins with its own count m giving the number
of bits. All the m's are the same for the n bitmaps.
Emit this bitmap length once instead of n times.

3. Many bitmaps within a function have the same bit values,
but each call site was given a distinct bitmap. Merge duplicate
bitmaps so that no bitmap is written more than once.

4. Many functions end up with the same aggregate bitmap data.
We used to name the bitmap data funcname.gcargs and funcname.gclocals.
Instead, name it gclocals.<md5 of data> and mark it dupok so
that the linker coalesces duplicate sets. This cut the bitmap
data remaining after step 3 by 40%; I was not expecting it to
be quite so dramatic.

Applied to "go build -ldflags -w code.google.com/p/go.tools/cmd/godoc":

                bitmaps           pclntab           binary on disk
before this CL  1326600           1985854           12738268
4-byte align    1154288 (0.87x)   1985854 (1.00x)   12566236 (0.99x)
one bitmap len   782528 (0.54x)   1985854 (1.00x)   12193500 (0.96x)
dedup bitmap     414748 (0.31x)   1948478 (0.98x)   11787996 (0.93x)
dedup bitmap set 245580 (0.19x)   1948478 (0.98x)   11620060 (0.91x)

While here, remove various dead blocks of code from plive.c.

Fixes #6929.
Fixes #7568.

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83630044
      4676fae5
    • David du Colombier's avatar
      cmd/8g, cmd/gc: fix warnings on Plan 9 · 9f9c9abb
      David du Colombier authored 
      warning: src/cmd/8g/ggen.c:35 non-interruptable temporary
warning: src/cmd/gc/walk.c:656 set and not used: l
warning: src/cmd/gc/walk.c:658 set and not used: l

LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews
https://golang.org/cl/83660043
      9f9c9abb
    • Russ Cox's avatar
      cmd/gc: shorten even more temporary lifetimes · 96d90d09
      Russ Cox authored 
      1. Use n->alloc, not n->left, to hold the allocated temp being
passed from orderstmt/orderexpr to walk.

2. Treat method values the same as closures.

3. Use killed temporary for composite literal passed to
non-escaping function argument.

4. Clean temporaries promptly in if and for statements.

5. Clean temporaries promptly in select statements.
As part of this, move all the temporary-generating logic
out of select.c into order.c, so that the temporaries can
be reclaimed.

With the new temporaries, can re-enable the 1-entry
select optimization. Fixes issue 7672.

While we're here, fix a 1-line bug in select processing
turned up by the new liveness test (but unrelated; select.c:72).
Fixes #7686.

6. Clean temporaries (but not particularly promptly) in switch
and range statements.

7. Clean temporary used during convT2E/convT2I.

8. Clean temporaries promptly during && and || expressions.

---

CL 81940043 reduced the number of ambiguously live temps
in the godoc binary from 860 to 711.

CL 83090046 reduced the number from 711 to 121.

This CL reduces the number from 121 to 23.

15 the 23 that remain are in fact ambiguously live.
The final 8 could be fixed but are not trivial and
not common enough to warrant work at this point
in the release cycle.

These numbers only count ambiguously live temps,
not ambiguously live user-declared variables.
There are 18 such variables in the godoc binary after this CL,
so a total of 41 ambiguously live temps or user-declared
variables.

The net effect is that zeroing anything on entry to a function
should now be a rare event, whereas earlier it was the
common case.

This is good enough for Go 1.3, and probably good
enough for future releases too.

Fixes #7345.

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83000048
      96d90d09
    • Keith Randall's avatar
      cmd/gc: Don't zero more than we need. · 47acf167
      Keith Randall authored 
      Don't merge with the zero range, we may
end up zeroing more than we need.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/83430044
      47acf167
    • Mikio Hara's avatar
      net: enable unixpacket test on available platforms · e88e7ed6
      Mikio Hara authored 
      DragonFlyBSD, FreeBSD 9 and beyond, NetBSD 6 and beyond, and
Solaris (illumos) support AF_UNIX+SOCK_SEQPACKET socket.

LGTM=dave
R=golang-codereviews, dave
CC=golang-codereviews
https://golang.org/cl/83390043
      e88e7ed6
    • Mikio Hara's avatar
      net: make WriteTo, WriteToUnix and WriteMsgUnix fail when connectionless-mode… · 67a51810
      Mikio Hara authored 
      net: make WriteTo, WriteToUnix and WriteMsgUnix fail when connectionless-mode UnixConn is already connected

This CL tries to fill the gap between Linux and other Unix-like systems
in the same way UDPConn already did.

Fixes #7677.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/83330045
      67a51810
    • Dmitriy Vyukov's avatar
      runtime: ignore pointers to global objects in SetFinalizer · f4ef6977
      Dmitriy Vyukov authored 
      Update #7656

LGTM=rsc
R=rsc, iant
CC=golang-codereviews
https://golang.org/cl/82560043
      f4ef6977
    • Keith Randall's avatar
      runtime: zero at start of frame more efficiently. · 383963b5
      Keith Randall authored 
      Use Duff's device for zeroing.  Combine adjacent regions.

Update #7680
Update #7624

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/83200045
      383963b5
    • Russ Cox's avatar
      cmd/5g, cmd/8g: fix build · 9c8f11ff
      Russ Cox authored 
      Botched during CL 83090046.

TBR=khr
CC=golang-codereviews
https://golang.org/cl/83070046
      9c8f11ff
    • Russ Cox's avatar
      cmd/gc: shorten more temporary lifetimes · daca06f2
      Russ Cox authored 
      1. In functions with heap-allocated result variables or with
defer statements, the return sequence requires more than
just a single RET instruction. There is an optimization that
arranges for all returns to jump to a single copy of the return
epilogue in this case. Unfortunately, that optimization is
fundamentally incompatible with PC-based liveness information:
it takes PCs at many different points in the function and makes
them all land at one PC, making the combined liveness information
at that target PC a mess. Disable this optimization, so that each
return site gets its own copy of the 'call deferreturn' and the
copying of result variables back from the heap.
This removes quite a few spurious 'ambiguously live' variables.

2. Let orderexpr allocate temporaries that are passed by address
to a function call and then die on return, so that we can arrange
an appropriate VARKILL.

2a. Do this for ... slices.

2b. Do this for closure structs.

2c. Do this for runtime.concatstring, which is the implementation
of large string additions. Change representation of OADDSTR to
an explicit list in typecheck to avoid reconstructing list in both
walk and order.

3. Let orderexpr allocate the temporary variable copies used for
range loops, so that they can be killed when the loop is over.
Similarly, let it allocate the temporary holding the map iterator.

CL 81940043 reduced the number of ambiguously live temps
in the godoc binary from 860 to 711.

This CL reduces the number to 121. Still more to do, but another
good checkpoint.

Update #7345

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83090046
      daca06f2
  7. 01 Apr, 2014 5 commits
    • Andrew Gerrand's avatar
      time: increase timeout in negative sleep duration test · 2f3776ac
      Andrew Gerrand authored 
      There's enough jitter in the scheduler on overloaded machines
that 25ms is not enough.

LGTM=dave
R=golang-codereviews, gobot, rsc, dave
CC=golang-codereviews
https://golang.org/cl/83300044
      2f3776ac
    • Keith Randall's avatar
      runtime: get rid of most uses of REP for copying/zeroing. · 6c7cbf08
      Keith Randall authored 
      REP MOVSQ and REP STOSQ have a really high startup overhead.
Use a Duff's device to do the repetition instead.

benchmark                 old ns/op     new ns/op     delta
BenchmarkClearFat32       7.20          1.60          -77.78%
BenchmarkCopyFat32        6.88          2.38          -65.41%
BenchmarkClearFat64       7.15          3.20          -55.24%
BenchmarkCopyFat64        6.88          3.44          -50.00%
BenchmarkClearFat128      9.53          5.34          -43.97%
BenchmarkCopyFat128       9.27          5.56          -40.02%
BenchmarkClearFat256      13.8          9.53          -30.94%
BenchmarkCopyFat256       13.5          10.3          -23.70%
BenchmarkClearFat512      22.3          18.0          -19.28%
BenchmarkCopyFat512       22.0          19.7          -10.45%
BenchmarkCopyFat1024      36.5          38.4          +5.21%
BenchmarkClearFat1024     35.1          35.0          -0.28%

TODO: use for stack frame zeroing
TODO: REP prefixes are still used for "reverse" copying when src/dst
regions overlap.  Might be worth fixing.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews, r
https://golang.org/cl/81370046
      6c7cbf08
    • Russ Cox's avatar
      runtime: use correct pc to obtain liveness info during stack copy · cfb347fc
      Russ Cox authored 
      The old code was using the PC of the instruction after the CALL.
Variables live during the call but not live when it returns would
not be seen as live during the stack copy, which might lead to
corruption. The correct PC to use is the one just before the
return address. After this CL the lookup matches what mgc0.c does.

The only time this matters is if you have back to back CALL instructions:

        CALL f1 // x live here
        CALL f2 // x no longer live

If a stack copy occurs during the execution of f1, the old code will
use the liveness bitmap intended for the execution of f2 and will not
treat x as live.

The only way this situation can arise and cause a problem in a stack copy
is if x lives on the stack has had its address taken but the compiler knows
enough about the context to know that x is no longer needed once f1
returns. The compiler has never known that much, so using the f2 context
cannot currently cause incorrect execution. For the same reason, it is not
possible to write a test for this today.

CL 83090046 will make the compiler precise enough in some cases
that this distinction will start mattering. The existing stack growth tests
in package runtime will fail if that CL is submitted without this one.

While we're here, print the frame PC in debug mode and update the
bitmap interpretation strings.

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83250043
      cfb347fc
    • Russ Cox's avatar
      cmd/gc: shorten temporary lifetimes when possible · b700cb49
      Russ Cox authored 
      The new channel and map runtime routines take pointers
to values, typically temporaries. Without help, the compiler
cannot tell when those temporaries stop being needed,
because it isn't sure what happened to the pointer.
Arrange to insert explicit VARKILL instructions for these
temporaries so that the liveness analysis can avoid seeing
them as "ambiguously live".

The change is made in order.c, which was already in charge of
introducing temporaries to preserve the order-of-evaluation
guarantees. Now its job has expanded to include introducing
temporaries as needed by runtime routines, and then also
inserting the VARKILL annotations for all these temporaries,
so that their lifetimes can be shortened.

In order to do its job for the map runtime routines, order.c arranges
that all map lookups or map assignments have the form:

        x = m[k]
        x, y = m[k]
        m[k] = x

where x, y, and k are simple variables (often temporaries).
Likewise, receiving from a channel is now always:

        x = <-c

In order to provide the map guarantee, order.c is responsible for
rewriting x op= y into x = x op y, so that m[k] += z becomes

        t = m[k]
        t2 = t + z
        m[k] = t2

While here, fix a few bugs in order.c's traversal: it was failing to
walk into select and switch case bodies, so order of evaluation
guarantees were not preserved in those situations.
Added tests to test/reorder2.go.

Fixes #7671.

In gc/popt's temporary-merging optimization, allow merging
of temporaries with their address taken as long as the liveness
ranges do not intersect. (There is a good chance of that now
that we have VARKILL annotations to limit the liveness range.)

Explicitly killing temporaries cuts the number of ambiguously
live temporaries that must be zeroed in the godoc binary from
860 to 711, or -17%. There is more work to be done, but this
is a good checkpoint.

Update #7345

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/81940043
      b700cb49
    • Russ Cox's avatar
      runtime: adjust GODEBUG=allocfreetrace=1 and GODEBUG=gcdead=1 · 1ec4d5e9
      Russ Cox authored 
      GODEBUG=allocfreetrace=1:

The allocfreetrace=1 mode prints a stack trace for each block
allocated and freed, and also a stack trace for each garbage collection.

It was implemented by reusing the heap profiling support: if allocfreetrace=1
then the heap profile was effectively running at 1 sample per 1 byte allocated
(always sample). The stack being shown at allocation was the stack gathered
for profiling, meaning it was derived only from the program counters and
did not include information about function arguments or frame pointers.
The stack being shown at free was the allocation stack, not the free stack.
If you are generating this log, you can find the allocation stack yourself, but
it can be useful to see exactly the sequence that led to freeing the block:
was it the garbage collector or an explicit free? Now that the garbage collector
runs on an m0 stack, the stack trace for the garbage collector was never interesting.

Fix all these problems:

1. Decouple allocfreetrace=1 from heap profiling.
2. Print the standard goroutine stack traces instead of a custom format.
3. Print the stack trace at time of allocation for an allocation,
   and print the stack trace at time of free (not the allocation trace again)
   for a free.
4. Print all goroutine stacks at garbage collection. Having all the stacks
   means that you can see the exact point at which each goroutine was
   preempted, which is often useful for identifying liveness-related errors.

GODEBUG=gcdead=1:

This mode overwrites dead pointers with a poison value.
Detect the poison value as an invalid pointer during collection,
the same way that small integers are invalid pointers.

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/81670043
      1ec4d5e9