1. 02 Apr, 2014 2 commits
    • 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
  2. 01 Apr, 2014 8 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
    • Shenghou Ma's avatar
      api: update next.txt · cc562e40
      Shenghou Ma authored
      LGTM=bradfitz
      R=golang-codereviews, gobot, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/81890044
      cc562e40
    • Shenghou Ma's avatar
      html/template: fix broken links · 16b95507
      Shenghou Ma authored
      Fixes #7562.
      
      LGTM=nigeltao
      R=nigeltao
      CC=golang-codereviews
      https://golang.org/cl/81190044
      16b95507
    • Shenghou Ma's avatar
      cmd/gc: fix spurious "bad negated constant" for complex constants. · 15bc7ab9
      Shenghou Ma authored
      Fixes #7648.
      
      LGTM=r, remyoudompheng
      R=golang-codereviews, r, remyoudompheng, jscrockett01
      CC=golang-codereviews
      https://golang.org/cl/80560045
      15bc7ab9
  3. 31 Mar, 2014 1 commit
  4. 29 Mar, 2014 5 commits
  5. 28 Mar, 2014 8 commits
  6. 27 Mar, 2014 12 commits
    • Adam Langley's avatar
      crypto/x509: update tests because Windows removed the Verisign root. · 0f272d13
      Adam Langley authored
      The root update on 3/11/2014 removed the Verisign root cert that the Go
      tests use. This only affects the 'TestSystemVerify' test in
      crypto/x509.
      
      Fixes #7523.
      
      LGTM=bradfitz
      R=golang-codereviews, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/80000044
      0f272d13
    • Rui Ueyama's avatar
      misc/emacs: do not highlight built-in function if not followed by '(' · 5f5e280e
      Rui Ueyama authored
      Name of built-in function is not reserved word in Go, and you can
      use it as variable name. "new" is often used as local variable, for
      instance.
      
      This patch is to apply font-lock-builtin-face only when built-in
      function name is followed by '(', so that it doesn't highlight
      non-function variable that happen to have the same name as built-in
      function.
      
      LGTM=dominik.honnef
      R=golang-codereviews, dominik.honnef, adonovan
      CC=golang-codereviews
      https://golang.org/cl/79260043
      5f5e280e
    • Daniel Morsing's avatar
      cmd/cgo: enforce typing of 0-sized types · 0f82cfd3
      Daniel Morsing authored
      cgo represents all 0-sized and unsized types internally as [0]byte. This means that pointers to incomplete types would be interchangable, even if given a name by typedef.
      
      Fixes #7409.
      
      LGTM=iant
      R=golang-codereviews, bradfitz, iant
      CC=golang-codereviews
      https://golang.org/cl/76450043
      0f82cfd3
    • Rui Ueyama's avatar
      misc/emacs: handle backslash in raw string in Emacs 23 · 444dd26b
      Rui Ueyama authored
      Go-mode in Emacs 23 does not recognize a backslash followed
      by a backquote as end of raw string literal, as it does not
      support syntax-propertize-function which Go-mode uses to
      remove special meaning from backslashes in ``.
      
      This patch provides a fallback mechanism to do the same thing
      using font-lock-syntactic-keywords, which is supported by
      Emacs 23.
      
      LGTM=dominik.honnef
      R=golang-codereviews, dominik.honnef
      CC=adonovan, golang-codereviews
      https://golang.org/cl/78730048
      444dd26b
    • Russ Cox's avatar
      runtime: enable 'bad pointer' check during garbage collection of Go stack frames · 5a23a7e5
      Russ Cox authored
      This is the same check we use during stack copying.
      The check cannot be applied to C stack frames, even
      though we do emit pointer bitmaps for the arguments,
      because (1) the pointer bitmaps assume all arguments
      are always live, not true of outputs during the prologue,
      and (2) the pointer bitmaps encode interface values as
      pointer pairs, not true of interfaces holding integers.
      
      For the rest of the frames, however, we should hold ourselves
      to the rule that a pointer marked live really is initialized.
      The interface scanning already implicitly checks this
      because it interprets the type word  as a valid type pointer.
      
      This may slow things down a little because of the extra loads.
      Or it may speed things up because we don't bother enqueuing
      nil pointers anymore. Enough of the rest of the system is slow
      right now that we can't measure it meaningfully.
      Enable for now, even if it is slow, to shake out bugs in the
      liveness bitmaps, and then decide whether to turn it off
      for the Go 1.3 release (issue 7650 reminds us to do this).
      
      The new m->traceback field lets us force printing of fp=
      values on all goroutine stack traces when we detect a
      bad pointer. This makes it easier to understand exactly
      where in the frame the bad pointer is, so that we can trace
      it back to a specific variable and determine what is wrong.
      
      Update #7650
      
      LGTM=khr
      R=khr
      CC=golang-codereviews
      https://golang.org/cl/80860044
      5a23a7e5
    • Russ Cox's avatar
      cmd/gc: liveness-related bug fixes · 6722d456
      Russ Cox authored
      1. On entry to a function, only zero the ambiguously live stack variables.
      Before, we were zeroing all stack variables containing pointers.
      The zeroing is pretty inefficient right now (issue 7624), but there are also
      too many stack variables detected as ambiguously live (issue 7345),
      and that must be addressed before deciding how to improve the zeroing code.
      (Changes in 5g/ggen.c, 6g/ggen.c, 8g/ggen.c, gc/pgen.c)
      
      Fixes #7647.
      
      2. Make the regopt word-based liveness analysis preserve the
      whole-variable liveness property expected by the garbage collection
      bitmap liveness analysis. That is, if the regopt liveness decides that
      one word in a struct needs to be preserved, make sure it preserves
      the entire struct. This is particularly important for multiword values
      such as strings, slices, and interfaces, in which all the words need
      to be present in order to understand the meaning.
      (Changes in 5g/reg.c, 6g/reg.c, 8g/reg.c.)
      
      Fixes #7591.
      
      3. Make the regopt word-based liveness analysis treat a variable
      as having its address taken - which makes it preserved across
      all future calls - whenever n->addrtaken is set, for consistency
      with the gc bitmap liveness analysis, even if there is no machine
      instruction actually taking the address. In this case n->addrtaken
      is incorrect (a nicer way to put it is overconservative), and ideally
      there would be no such cases, but they can happen and the two
      analyses need to agree.
      (Changes in 5g/reg.c, 6g/reg.c, 8g/reg.c; test in bug484.go.)
      
      Fixes crashes found by turning off "zero everything" in step 1.
      
      4. Remove spurious VARDEF annotations. As the comment in
      gc/pgen.c explains, the VARDEF must immediately precede
      the initialization. It cannot be too early, and it cannot be too late.
      In particular, if a function call sits between the VARDEF and the
      actual machine instructions doing the initialization, the variable
      will be treated as live during that function call even though it is
      uninitialized, leading to problems.
      (Changes in gc/gen.c; test in live.go.)
      
      Fixes crashes found by turning off "zero everything" in step 1.
      
      5. Do not treat loading the address of a wide value as a signal
      that the value must be initialized. Instead depend on the existence
      of a VARDEF or the first actual read/write of a word in the value.
      If the load is in order to pass the address to a function that does
      the actual initialization, treating the load as an implicit VARDEF
      causes the same problems as described in step 4.
      The alternative is to arrange to zero every such value before
      passing it to the real initialization function, but this is a much
      easier and more efficient change.
      (Changes in gc/plive.c.)
      
      Fixes crashes found by turning off "zero everything" in step 1.
      
      6. Treat wide input parameters with their address taken as
      initialized on entry to the function. Otherwise they look
      "ambiguously live" and we will try to emit code to zero them.
      (Changes in gc/plive.c.)
      
      Fixes crashes found by turning off "zero everything" in step 1.
      
      7. An array of length 0 has no pointers, even if the element type does.
      Without this change, the zeroing code complains when asked to
      clear a 0-length array.
      (Changes in gc/reflect.c.)
      
      LGTM=khr
      R=khr
      CC=golang-codereviews
      https://golang.org/cl/80160044
      6722d456
    • Russ Cox's avatar
      cmd/dist: zero output variables on entry to goc2c functions · f94bff79
      Russ Cox authored
      Zeroing the outputs makes sure that during function calls
      in those functions we do not let the garbage collector
      treat uninitialized values as pointers.
      
      The garbage collector may still see uninitialized values
      if a preemption occurs during the function prologue,
      before the zeroing has had a chance to run.
      
      This reduces the number of 'bad pointer' messages when
      that runtime check is enabled, but it doesn't fix all of them,
      so the check is still disabled.
      
      It will also avoid leaks, although I doubt any of these were
      particularly serious.
      
      LGTM=iant, khr
      R=iant, khr
      CC=golang-codereviews
      https://golang.org/cl/80850044
      f94bff79
    • Russ Cox's avatar
      regexp/syntax: remove InstLast · ef3c0e7e
      Russ Cox authored
      This was added by the one-pass CL (post Go 1.2)
      so it can still be removed.
      
      Removing because surely there will be new operations
      added later, and we can't change the constant value
      once we define it, so "last" is a bad concept to expose.
      
      Nothing uses it.
      
      LGTM=bradfitz
      R=golang-codereviews, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/81160043
      ef3c0e7e
    • Jan Ziak's avatar
      cmd/gc: fix spurious 'use of untyped nil' error · 21b2e168
      Jan Ziak authored
      Fixes #6402
      
      LGTM=rsc
      R=rsc
      CC=golang-codereviews
      https://golang.org/cl/81340044
      21b2e168
    • Rui Ueyama's avatar
      misc/bash, misc/zsh: fix completion rules · 6119dc1b
      Rui Ueyama authored
      This patch includes fixes pointed out in CL 52140043, which was
      originally written by john.gosset.
      
      LGTM=minux.ma
      R=golang-codereviews, minux.ma
      CC=golang-codereviews
      https://golang.org/cl/80320043
      6119dc1b
    • Russ Cox's avatar
      runtime: initialize complete Hiter during mapiterinit · fc6befba
      Russ Cox authored
      The garbage collector will scan these pointers,
      so make sure they are initialized.
      
      LGTM=bradfitz, khr
      R=khr, bradfitz
      CC=golang-codereviews
      https://golang.org/cl/80960047
      fc6befba
    • Rob Pike's avatar
      doc/go1.3.html: explain the change to the memory model · a4380927
      Rob Pike authored
      LGTM=iant, rsc
      R=rsc, iant, mtj
      CC=golang-codereviews
      https://golang.org/cl/80260044
      a4380927
  7. 26 Mar, 2014 4 commits