1. 06 Jun, 2014 1 commit
    • Russ Cox's avatar
      runtime: fix panic stack during runtime.Goexit during panic · 4534fdb1
      Russ Cox authored
      A runtime.Goexit during a panic-invoked deferred call
      left the panic stack intact even though all the stack frames
      are gone when the goroutine is torn down.
      The next goroutine to reuse that struct will have a
      bogus panic stack and can cause the traceback routines
      to walk into garbage.
      
      Most likely to happen during tests, because t.Fatal might
      be called during a deferred func and uses runtime.Goexit.
      
      This "not enough cleared in Goexit" failure mode has
      happened to us multiple times now. Clear all the pointers
      that don't make sense to keep, not just gp->panic.
      
      Fixes #8158.
      
      LGTM=iant, dvyukov
      R=iant, dvyukov
      CC=golang-codereviews
      https://golang.org/cl/102220043
      4534fdb1
  2. 05 Jun, 2014 4 commits
  3. 04 Jun, 2014 1 commit
  4. 03 Jun, 2014 7 commits
    • Ian Lance Taylor's avatar
      debug/elf: support DWARF that needs relocs for 386 · 0e197515
      Ian Lance Taylor authored
      It's not clear how widespread this issue is, but we do have a
      test case generated by a development version of clang.
      
      I don't know whether this should go into 1.3 or not; happy to
      hear arguments either way.
      
      LGTM=rsc
      R=golang-codereviews, bradfitz, rsc
      CC=golang-codereviews
      https://golang.org/cl/96680045
      0e197515
    • Ian Lance Taylor's avatar
      compress/gzip: allow Reset on Reader without NewReader · 68bbf9d4
      Ian Lance Taylor authored
      Fixes #8126.
      
      LGTM=bradfitz
      R=golang-codereviews, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/103020044
      68bbf9d4
    • Russ Cox's avatar
      cmd/gc: fix escape analysis of func returning indirect of parameter · fe3c9134
      Russ Cox authored
      I introduced this bug when I changed the escape
      analysis to run in phases based on call graph
      dependency order, in order to be more precise about
      inputs escaping back to outputs (functions returning
      their arguments).
      
      Given
      
              func f(z **int) *int { return *z }
      
      we were tagging the function as 'z does not escape
      and is not returned', which is all true, but not
      enough information.
      
      If used as:
      
              var x int
              p := &x
              q := &p
              leak(f(q))
      
      then the compiler might try to keep x, p, and q all
      on the stack, since (according to the recorded
      information) nothing interesting ends up being
      passed to leak.
      
      In fact since f returns *q = p, &x is passed to leak
      and x needs to be heap allocated.
      
      To trigger the bug, you need a chain that the
      compiler wants to keep on the stack (like x, p, q
      above), and you need a function that returns an
      indirect of its argument, and you need to pass the
      head of the chain to that function. This doesn't
      come up very often: this bug has been present since
      June 2012 (between Go 1 and Go 1.1) and we haven't
      seen it until now. It helps that most functions that
      return indirects are getters that are simple enough
      to be inlined, avoiding the bug.
      
      Earlier versions of Go also had the benefit that if
      &x really wasn't used beyond x's lifetime, nothing
      broke if you put &x in a heap-allocated structure
      accidentally. With the new stack copying, though,
      heap-allocated structures containing &x are not
      updated when the stack is copied and x moves,
      leading to crashes in Go 1.3 that were not crashes
      in Go 1.2 or Go 1.1.
      
      The fix is in two parts.
      
      First, in the analysis of a function, recognize when
      a value obtained via indirect of a parameter ends up
      being returned. Mark those parameters as having
      content escape back to the return results (but we
      don't bother to write down which result).
      
      Second, when using the analysis to analyze, say,
      f(q), mark parameters with content escaping as
      having any indirections escape to the heap. (We
      don't bother trying to match the content to the
      return value.)
      
      The fix could be less precise (simpler).
      In the first part we might mark all content-escaping
      parameters as plain escaping, and then the second
      part could be dropped. Or we might assume that when
      calling f(q) all the things pointed at by q escape
      always (for any f and q).
      
      The fix could also be more precise (more complex).
      We might record the specific mapping from parameter
      to result along with the number of indirects from the
      parameter to the thing being returned as the result,
      and then at the call sites we could set up exactly the
      right graph for the called function. That would make
      notleaks(f(q)) be able to keep x on the stack, because
      the reuslt of f(q) isn't passed to anything that leaks it.
      
      The less precise the fix, the more stack allocations
      become heap allocations.
      
      This fix is exactly as precise as it needs to be so that
      none of the current stack allocations in the standard
      library turn into heap allocations.
      
      Fixes #8120.
      
      LGTM=iant
      R=golang-codereviews, iant
      CC=golang-codereviews, khr, r
      https://golang.org/cl/102040046
      fe3c9134
    • Brad Fitzpatrick's avatar
      crypto/tls: fix typo referencing the required Config field · 19fe9a2c
      Brad Fitzpatrick authored
      Thanks to Frithjof Schulze for noticing.
      
      LGTM=adg
      R=adg
      CC=agl, golang-codereviews, r
      https://golang.org/cl/107740043
      19fe9a2c
    • Brad Fitzpatrick's avatar
      time: support version 3 zone records · 14a75ecf
      Brad Fitzpatrick authored
      Fixes #8134
      
      LGTM=iant
      R=golang-codereviews, iant
      CC=golang-codereviews, r, rsc
      https://golang.org/cl/100930044
      14a75ecf
    • Russ Cox's avatar
      cmd/gc: fix liveness for address-taken variables in inlined functions · eb540792
      Russ Cox authored
      The 'address taken' bit in a function variable was not
      propagating into the inlined copies, causing incorrect
      liveness information.
      
      LGTM=dsymonds, bradfitz
      R=golang-codereviews, bradfitz
      CC=dsymonds, golang-codereviews, iant, khr, r
      https://golang.org/cl/96670046
      eb540792
    • Russ Cox's avatar
      runtime: fix 1-byte return during x.(T) for 0-byte T · d646040f
      Russ Cox authored
      The 1-byte write was silently clearing a byte on the stack.
      If there was another function call with more arguments
      in the same stack frame, no harm done.
      Otherwise, if the variable at that location was already zero,
      no harm done.
      Otherwise, problems.
      
      Fixes #8139.
      
      LGTM=dsymonds
      R=golang-codereviews, dsymonds
      CC=golang-codereviews, iant, r
      https://golang.org/cl/100940043
      d646040f
  5. 02 Jun, 2014 8 commits
  6. 01 Jun, 2014 7 commits
  7. 31 May, 2014 8 commits
  8. 30 May, 2014 2 commits
    • Russ Cox's avatar
      cmd/6g: treat vardef-initialized fat variables as live at calls · 1afbceb5
      Russ Cox authored
      This CL forces the optimizer to preserve some memory stores
      that would be redundant except that a stack scan due to garbage
      collection or stack copying might look at them during a function call.
      As such, it forces additional memory writes and therefore slows
      down the execution of some programs, especially garbage-heavy
      programs that are already limited by memory bandwidth.
      
      The slowdown can be as much as 7% for end-to-end benchmarks.
      
      These numbers are from running go1.test -test.benchtime=5s three times,
      taking the best (lowest) ns/op for each benchmark. I am excluding
      benchmarks with time/op < 10us to focus on macro effects.
      All benchmarks are on amd64.
      
      Comparing tip (a27f34c771cb) against this CL on an Intel Core i5 MacBook Pro:
      
      benchmark                          old ns/op      new ns/op      delta
      BenchmarkBinaryTree17              3876500413     3856337341     -0.52%
      BenchmarkFannkuch11                2965104777     2991182127     +0.88%
      BenchmarkGobDecode                 8563026        8788340        +2.63%
      BenchmarkGobEncode                 5050608        5267394        +4.29%
      BenchmarkGzip                      431191816      434168065      +0.69%
      BenchmarkGunzip                    107873523      110563792      +2.49%
      BenchmarkHTTPClientServer          85036          86131          +1.29%
      BenchmarkJSONEncode                22143764       22501647       +1.62%
      BenchmarkJSONDecode                79646916       85658808       +7.55%
      BenchmarkMandelbrot200             4720421        4700108        -0.43%
      BenchmarkGoParse                   4651575        4712247        +1.30%
      BenchmarkRegexpMatchMedium_1K      71986          73490          +2.09%
      BenchmarkRegexpMatchHard_1K        111018         117495         +5.83%
      BenchmarkRevcomp                   648798723      659352759      +1.63%
      BenchmarkTemplate                  112673009      112819078      +0.13%
      
      Comparing tip (a27f34c771cb) against this CL on an Intel Xeon E5520:
      
      BenchmarkBinaryTree17              5461110720     5393104469     -1.25%
      BenchmarkFannkuch11                4314677151     4327177615     +0.29%
      BenchmarkGobDecode                 11065853       11235272       +1.53%
      BenchmarkGobEncode                 65000655        6959837        +7.07%
      BenchmarkGzip                      647478596      671769097      +3.75%
      BenchmarkGunzip                    139348579      141096376      +1.25%
      BenchmarkHTTPClientServer          69376          73610          +6.10%
      BenchmarkJSONEncode                30172320       31796106       +5.38%
      BenchmarkJSONDecode                113704905      114239137      +0.47%
      BenchmarkMandelbrot200             6032730        6003077        -0.49%
      BenchmarkGoParse                   6775251        6405995        -5.45%
      BenchmarkRegexpMatchMedium_1K      111832         113895         +1.84%
      BenchmarkRegexpMatchHard_1K        161112         168420         +4.54%
      BenchmarkRevcomp                   876363406      892319935      +1.82%
      BenchmarkTemplate                  146273096      148998339      +1.86%
      
      Just to get a sense of where we are compared to the previous release,
      here are the same benchmarks comparing Go 1.2 to this CL.
      
      Comparing Go 1.2 against this CL on an Intel Core i5 MacBook Pro:
      
      BenchmarkBinaryTree17              4370077662     3856337341     -11.76%
      BenchmarkFannkuch11                3347052657     2991182127     -10.63%
      BenchmarkGobDecode                 8791384        8788340        -0.03%
      BenchmarkGobEncode                 4968759        5267394        +6.01%
      BenchmarkGzip                      437815669      434168065      -0.83%
      BenchmarkGunzip                    94604099       110563792      +16.87%
      BenchmarkHTTPClientServer          87798          86131          -1.90%
      BenchmarkJSONEncode                22818243       22501647       -1.39%
      BenchmarkJSONDecode                97182444       85658808       -11.86%
      BenchmarkMandelbrot200             4733516        4700108        -0.71%
      BenchmarkGoParse                   5054384        4712247        -6.77%
      BenchmarkRegexpMatchMedium_1K      67612          73490          +8.69%
      BenchmarkRegexpMatchHard_1K        107321         117495         +9.48%
      BenchmarkRevcomp                   733270055      659352759      -10.08%
      BenchmarkTemplate                  109304977      112819078      +3.21%
      
      Comparing Go 1.2 against this CL on an Intel Xeon E5520:
      
      BenchmarkBinaryTree17              5986953594     5393104469     -9.92%
      BenchmarkFannkuch11                4861139174     4327177615     -10.98%
      BenchmarkGobDecode                 11830997       11235272       -5.04%
      BenchmarkGobEncode                 6608722        6959837        +5.31%
      BenchmarkGzip                      661875826      671769097      +1.49%
      BenchmarkGunzip                    138630019      141096376      +1.78%
      BenchmarkHTTPClientServer          71534          73610          +2.90%
      BenchmarkJSONEncode                30393609       31796106       +4.61%
      BenchmarkJSONDecode                139645860      114239137      -18.19%
      BenchmarkMandelbrot200             5988660        6003077        +0.24%
      BenchmarkGoParse                   6974092        6405995        -8.15%
      BenchmarkRegexpMatchMedium_1K      111331         113895         +2.30%
      BenchmarkRegexpMatchHard_1K        165961         168420         +1.48%
      BenchmarkRevcomp                   995049292      892319935      -10.32%
      BenchmarkTemplate                  145623363      148998339      +2.32%
      
      Fixes #8036.
      
      LGTM=khr
      R=golang-codereviews, josharian, khr
      CC=golang-codereviews, iant, r
      https://golang.org/cl/99660044
      1afbceb5
    • Ian Lance Taylor's avatar
      runtime: add zero field to rtype · aad4609c
      Ian Lance Taylor authored
      The rtype struct is meant to be a copy of reflect.rtype.  The
      zero field was added to reflect.rtype in 18495:6e50725ac753.
      
      LGTM=rsc
      R=khr, rsc
      CC=golang-codereviews
      https://golang.org/cl/93660045
      aad4609c
  9. 29 May, 2014 2 commits
    • Russ Cox's avatar
      cmd/gc: fix x=x crash · 89d46fed
      Russ Cox authored
      [Same as CL 102820043 except applied changes to 6g/gsubr.c
      also to 5g/gsubr.c and 8g/gsubr.c. The problem I had last night
      trying to do that was that 8g's copy of nodarg has different
      (but equivalent) control flow and I was pasting the new code
      into the wrong place.]
      
      Description from CL 102820043:
      
      The 'nodarg' function is used to obtain a Node*
      representing a function argument or result.
      It returned a brand new Node*, but that violates
      the guarantee in most places in the compiler that
      two Node*s refer to the same variable if and only if
      they are the same Node* pointer. Reestablish that
      invariant by making nodarg return a preexisting
      named variable if present.
      
      Having fixed that, avoid any copy during x=x in
      componentgen, because the VARDEF we emit
      before the copy marks the lhs x as dead incorrectly.
      
      The change in walk.c avoids modifying the result
      of nodarg. This was the only place in the compiler
      that did so.
      
      Fixes #8097.
      
      LGTM=khr
      R=golang-codereviews, khr
      CC=golang-codereviews, iant, khr, r
      https://golang.org/cl/103750043
      89d46fed
    • Russ Cox's avatar
      undo CL 102820043 / b0ce6dbafc18 · 9dd062b8
      Russ Cox authored
      Breaks 386 and arm builds.
      The obvious reason is that this CL only edited 6g/gsubr.c
      and failed to edit 5g/gsubr.c and 8g/gsubr.c.
      However, the obvious CL applying the same edit to those
      files (CL 101900043) causes mysterious build failures
      in various of the standard package tests, usually involving
      reflect. Something deep and subtle is broken but only on
      the 32-bit systems.
      
      Undo this CL for now.
      
      ««« original CL description
      cmd/gc: fix x=x crash
      
      The 'nodarg' function is used to obtain a Node*
      representing a function argument or result.
      It returned a brand new Node*, but that violates
      the guarantee in most places in the compiler that
      two Node*s refer to the same variable if and only if
      they are the same Node* pointer. Reestablish that
      invariant by making nodarg return a preexisting
      named variable if present.
      
      Having fixed that, avoid any copy during x=x in
      componentgen, because the VARDEF we emit
      before the copy marks the lhs x as dead incorrectly.
      
      The change in walk.c avoids modifying the result
      of nodarg. This was the only place in the compiler
      that did so.
      
      Fixes #8097.
      
      LGTM=r, khr
      R=golang-codereviews, r, khr
      CC=golang-codereviews, iant
      https://golang.org/cl/102820043
      »»»
      
      TBR=r
      CC=golang-codereviews, khr
      https://golang.org/cl/95660043
      9dd062b8