1. 27 Mar, 2014 7 commits
    • 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
  2. 26 Mar, 2014 16 commits
  3. 24 Mar, 2014 2 commits
    • Mikio Hara's avatar
      net: deflake TestTCPConcurrentAccept · 4f1aecf2
      Mikio Hara authored
      Some platform that implements inp_localgroup-like shared internet
      protocol control block group looks a bit sensitive about transport
      layer protocol's address:port reuse. Sometimes it rejects a TCP SYN
      packet using TCP RST, and sometimes silence.
      
      For now, until test case refactoring, we admit few Dial failures on
      TestTCPConcurrentAccept as a workaround.
      
      Update #7400
      Update #7541
      
      LGTM=jsing
      R=jsing
      CC=golang-codereviews
      https://golang.org/cl/75920043
      4f1aecf2
    • Mikio Hara's avatar
      net: avoid multiple calling of syscall connect on Unix variants · d1e3ad8b
      Mikio Hara authored
      The previous fix CL 69340044 still leaves a possibility of it.
      This CL prevents the kernel, especially DragonFly BSD, from
      performing unpredictable asynchronous connection establishment
      on stream-based transport layer protocol sockets.
      
      Update #7541
      Update #7474
      
      LGTM=jsing
      R=jsing
      CC=golang-codereviews
      https://golang.org/cl/75930043
      d1e3ad8b
  4. 26 Mar, 2014 3 commits
  5. 25 Mar, 2014 12 commits
    • Andrew Gerrand's avatar
      doc/gopher: add README · 8c9e838f
      Andrew Gerrand authored
      LGTM=r
      R=r
      CC=golang-codereviews
      https://golang.org/cl/79890046
      8c9e838f
    • Brad Fitzpatrick's avatar
      net/http: disable recently-introduced flaky test on Windows · aa2d4dee
      Brad Fitzpatrick authored
      Disable it until it's debugged so it doesn't hide other real
      problems on Windows. The test was known to be unreliable
      anyway (which is why it only needed 1 of 20 runs to pass), but
      apparently it never passes on Windows. Figure out why later.
      
      Update #7634
      
      LGTM=alex.brainman
      R=adg, alex.brainman
      CC=golang-codereviews
      https://golang.org/cl/80110043
      aa2d4dee
    • Keith Randall's avatar
      runtime: WriteHeapDump dumps the heap to a file. · fff63c24
      Keith Randall authored
      See http://golang.org/s/go13heapdump for the file format.
      
      LGTM=rsc
      R=rsc, bradfitz, dvyukov, khr
      CC=golang-codereviews
      https://golang.org/cl/37540043
      fff63c24
    • Ian Lance Taylor's avatar
      bee3848f
    • Keith Randall's avatar
      runtime: redo stack map entries to avoid false retention · 1b45cc45
      Keith Randall authored
      Change two-bit stack map entries to encode:
      0 = dead
      1 = scalar
      2 = pointer
      3 = multiword
      
      If multiword, the two-bit entry for the following word encodes:
      0 = string
      1 = slice
      2 = iface
      3 = eface
      
      That way, during stack scanning we can check if a string
      is zero length or a slice has zero capacity.  We can avoid
      following the contained pointer in those cases.  It is safe
      to do so because it can never be dereferenced, and it is
      desirable to do so because it may cause false retention
      of the following block in memory.
      
      Slice feature turned off until issue 7564 is fixed.
      
      Update #7549
      
      LGTM=rsc
      R=golang-codereviews, bradfitz, rsc
      CC=golang-codereviews
      https://golang.org/cl/76380043
      1b45cc45
    • Ian Lance Taylor's avatar
      runtime: accurately record whether heap memory is reserved · 4ebfa831
      Ian Lance Taylor authored
      The existing code did not have a clear notion of whether
      memory has been actually reserved.  It checked based on
      whether in 32-bit mode or 64-bit mode and (on GNU/Linux) the
      requested address, but it confused the requested address and
      the returned address.
      
      LGTM=rsc
      R=rsc, dvyukov
      CC=golang-codereviews, michael.hudson
      https://golang.org/cl/79610043
      4ebfa831
    • Brad Fitzpatrick's avatar
      net/http: don't re-use Transport connections if we've seen an EOF · cc2c5fc3
      Brad Fitzpatrick authored
      This the second part of making persistent HTTPS connections to
      certain servers (notably Amazon) robust.
      
      See the story in part 1: https://golang.org/cl/76400046/
      
      This is the http Transport change that notes whether our
      net.Conn.Read has ever seen an EOF. If it has, then we use
      that as an additional signal to not re-use that connection (in
      addition to the HTTP response headers)
      
      Fixes #3514
      
      LGTM=rsc
      R=agl, rsc
      CC=golang-codereviews
      https://golang.org/cl/79240044
      cc2c5fc3
    • Brad Fitzpatrick's avatar
      crypto/tls: make Conn.Read return (n, io.EOF) when EOF is next in buffer · f61f18d6
      Brad Fitzpatrick authored
      Update #3514
      
      An io.Reader is permitted to return either (n, nil)
      or (n, io.EOF) on EOF or other error.
      
      The tls package previously always returned (n, nil) for a read
      of size n if n bytes were available, not surfacing errors at
      the same time.
      
      Amazon's HTTPS frontends like to hang up on clients without
      sending the appropriate HTTP headers. (In their defense,
      they're allowed to hang up any time, but generally a server
      hangs up after a bit of inactivity, not immediately.) In any
      case, the Go HTTP client tries to re-use connections by
      looking at whether the response headers say to keep the
      connection open, and because the connection looks okay, under
      heavy load it's possible we'll reuse it immediately, writing
      the next request, just as the Transport's always-reading
      goroutine returns from tls.Conn.Read and sees (0, io.EOF).
      
      But because Amazon does send an AlertCloseNotify record before
      it hangs up on us, and the tls package does its own internal
      buffering (up to 1024 bytes) of pending data, we have the
      AlertCloseNotify in an unread buffer when our Conn.Read (to
      the HTTP Transport code) reads its final bit of data in the
      HTTP response body.
      
      This change makes that final Read return (n, io.EOF) when
      an AlertCloseNotify record is buffered right after, if we'd
      otherwise return (n, nil).
      
      A dependent change in the HTTP code then notes whether a
      client connection has seen an io.EOF and uses that as an
      additional signal to not reuse a HTTPS connection. With both
      changes, the majority of Amazon request failures go
      away. Without either one, 10-20 goroutines hitting the S3 API
      leads to such an error rate that empirically up to 5 retries
      are needed to complete an API call.
      
      LGTM=agl, rsc
      R=agl, rsc
      CC=golang-codereviews
      https://golang.org/cl/76400046
      f61f18d6
    • Ian Lance Taylor's avatar
      runtime: change nproc local variable to uint32 · 8de04c78
      Ian Lance Taylor authored
      The nproc and ndone fields are uint32.  This makes the type
      consistent.
      
      LGTM=minux.ma
      R=golang-codereviews, minux.ma
      CC=golang-codereviews
      https://golang.org/cl/79340044
      8de04c78
    • Nigel Tao's avatar
      database/sql: add "defer rows.Close()" to the example code. · 50ca1a52
      Nigel Tao authored
      Strictly speaking, it's not necessary in example_test.go, as the
      Rows.Close docs say that "If Next returns false, the Rows are closed
      automatically". However, if the for loop breaks or returns early, it's
      not obvious that you'll leak unless you explicitly call Rows.Close.
      
      LGTM=bradfitz
      R=bradfitz
      CC=golang-codereviews, rsc
      https://golang.org/cl/79330043
      50ca1a52
    • Russ Cox's avatar
      runtime: use VEH, not SEH, for windows/386 exception handling · 3750904a
      Russ Cox authored
      Structured Exception Handling (SEH) was the first way to handle
      exceptions (memory faults, divides by zero) on Windows.
      The S might as well stand for "stack-based": the implementation
      interprets stack addresses in a few different ways, and it gets
      subtly confused by Go's management of stacks. It's also something
      that requires active maintenance during cgo switches, and we've
      had bugs in that maintenance in the past.
      
      We have recently come to believe that SEH cannot work with
      Go's stack usage. See http://golang.org/issue/7325 for details.
      
      Vectored Exception Handling (VEH) is more like a Unix signal
      handler: you set it once for the whole process and forget about it.
      
      This CL drops all the SEH code and replaces it with VEH code.
      Many special cases and 7 #ifdefs disappear.
      
      VEH was introduced in Windows XP, so Go on windows/386 will
      now require Windows XP or later. The previous requirement was
      Windows 2000 or later. Windows 2000 immediately preceded
      Windows XP, so Windows 2000 is the only affected version.
      Microsoft stopped supporting Windows 2000 in 2010.
      See http://golang.org/s/win2000-golang-nuts for details.
      
      Fixes #7325.
      
      LGTM=alex.brainman, r
      R=golang-codereviews, alex.brainman, stephen.gutekanst, dave
      CC=golang-codereviews, iant, r
      https://golang.org/cl/74790043
      3750904a
    • Russ Cox's avatar
      time: add comment explaining rejection of years outside [0000,9999] · 3b27343c
      Russ Cox authored
      This has come up twice now. Redirect future questions
      to the explanation in the issue tracker.
      
      LGTM=iant, r
      R=r, iant
      CC=golang-codereviews
      https://golang.org/cl/79550043
      3b27343c