1. 15 Jul, 2019 1 commit
  2. 14 Jul, 2019 1 commit
  3. 12 Jul, 2019 3 commits
  4. 11 Jul, 2019 2 commits
  5. 10 Jul, 2019 6 commits
  6. 09 Jul, 2019 2 commits
    • Gernot Vormayr's avatar
      cmd/cgo: fix check for conversion of ptr to struct field · 84fce983
      Gernot Vormayr authored
      According to the documentation "When passing a pointer to a field in a
      struct, the Go memory in question is the memory occupied by the field,
      not the entire struct.". checkAddr states that this should also work
      with type conversions, which is implemented in isType. However,
      ast.StarExpr must be enclosed in ast.ParenExpr according to the go spec
      (see example below), which is not considered in the checks.
      
      Example:
          // struct Si { int i; int *p; }; void f(struct I *x) {}
          import "C"
          type S {
              p *int
              i C.struct_Si
          }
          func main() {
              v := &S{new(int)}
              C.f((*C.struct_I)(&v.i)) // <- panic
          }
      
      This example will cause cgo to emit a cgoCheck that checks the whole
      struct S instead of just S.i causing the panic "cgo argument has Go
      pointer to Go pointer".
      
      This patch fixes this situation by adding support for ast.ParenExpr to
      isType and adds a test, that fails without the fix.
      
      Fixes #32970.
      
      Change-Id: I15ea28c98f839e9fa708859ed107a2e5f1483133
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185098
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      84fce983
    • LE Manh Cuong's avatar
      cmd/compile: fix unsafeValue handles OLSH/ORSH wrong · 06ef108c
      LE Manh Cuong authored
      For OLSH/ORSH, the right node is not a uintptr-typed. However,
      unsafeValue still be called recursively for it, causing the
      compiler crashes.
      
      To fixing, the right node only needs to be evaluated
      for side-effects, so just discard its value.
      
      Fixes #32959
      
      Change-Id: I34d5aa0823a0545f6dad1ec34774235ecf11addc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185039
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      06ef108c
  7. 08 Jul, 2019 5 commits
    • Agniva De Sarker's avatar
      Revert "go/parser: include more comments in a struct or interface" · a19c0ced
      Agniva De Sarker authored
      This reverts commit https://golang.org/cl/161177/.
      
      Reason for revert: this led to non-contiguous comments spaced
      by an empty line to be grouped into a single CommentGroup
      
      Fixes #32944
      Updates #10858
      
      Change-Id: I5e16663b308c3b560496da8e66c33befdf9ed9dd
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185040Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      a19c0ced
    • Alexander Rakoczy's avatar
      doc: document Go 1.12.7 · c893ea8f
      Alexander Rakoczy authored
      Change-Id: Id5d2f4cc6bc310bed2516ce0f50c395802475f66
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185258Reviewed-by: default avatarDmitri Shuralyov <dmitshur@golang.org>
      c893ea8f
    • Alexander Rakoczy's avatar
      doc: document Go 1.11.12 · 0fddd668
      Alexander Rakoczy authored
      Change-Id: I1b2e369befc58b3f88ac201442a2d9f76d87d54e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185257Reviewed-by: default avatarDmitri Shuralyov <dmitshur@golang.org>
      0fddd668
    • Russ Cox's avatar
      net/http: fix Transport.MaxConnsPerHost limits & idle pool races · fbaf881c
      Russ Cox authored
      There were at least three races in the implementation of the pool of
      idle HTTP connections before this CL.
      
      The first race is that HTTP/2 connections can be shared for many
      requests, but each requesting goroutine would take the connection out
      of the pool and then immediately return it before using it; this
      created unnecessary, tiny little race windows during which another
      goroutine might dial a second connection instead of reusing the first.
      This CL changes the idle pool to just leave the HTTP/2 connection in
      the pool permanently (until there is reason to close it), instead of
      doing the take-it-out-put-it-back dance race.
      
      The second race is that “is there an idle connection?” and
      “register to wait for an idle connection” were implemented as two
      separate steps, in different critical sections. So a client could end
      up registered to wait for an idle connection and be waiting or perhaps
      dialing, not having noticed the idle connection sitting in the pool
      that arrived between the two steps.
      
      The third race is that t.getIdleConnCh assumes that the inability to
      send on the channel means the client doesn't need the result, when it
      could mean that the client has not yet entered the select.
      That is, the main dial does:
      
      	idleConnCh := t.getIdleConnCh(cm)
      	select {
      	case v := <-dialc:
      		...
      	case pc := <-idleConnCh
      		...
      	...
      	}
      
      But then tryPutIdleConn does:
      
      	waitingDialer := t.idleConnCh[key] // what getIdleConnCh(cm) returned
      	select {
      	case waitingDialer <- pconn:
      		// We're done ...
      		return nil
      	default:
      		if waitingDialer != nil {
      			// They had populated this, but their dial won
      			// first, so we can clean up this map entry.
      			delete(t.idleConnCh, key)
      		}
      	}
      
      If the client has returned from getIdleConnCh but not yet reached the
      select, tryPutIdleConn will be unable to do the send, incorrectly
      conclude that the client does not care anymore, and put the connection
      in the idle pool instead, again leaving the client dialing unnecessarily
      while a connection sits in the idle pool.
      
      (It's also odd that the success case does not clean up the map entry,
      and also that the map has room for only a single waiting goroutine for
      a given host.)
      
      None of these races mattered too much before Go 1.11: at most they
      meant that connections were not reused quite as promptly as possible,
      or a few more than necessary would be created. But Go 1.11 added
      Transport.MaxConnsPerHost, which limited the number of connections
      created for a given host. The default is 0 (unlimited), but if a user
      did explicitly impose a low limit (2 is common), all these misplaced
      conns could easily add up to the entire limit, causing a deadlock.
      This was causing intermittent timeouts in TestTransportMaxConnsPerHost.
      
      The addition of the MaxConnsPerHost support added its own races.
      
      For example, here t.incHostConnCount could increment the count
      and return a channel ready for receiving, and then the client would
      not receive from it nor ever issue the decrement, because the select
      need not evaluate these two cases in order:
      
      	select {
      	case <-t.incHostConnCount(cmKey):
      		// count below conn per host limit; proceed
      	case pc := <-t.getIdleConnCh(cm):
      		if trace != nil && trace.GotConn != nil {
      			trace.GotConn(httptrace.GotConnInfo{Conn: pc.conn, Reused: pc.isReused()})
      		}
      		return pc, nil
      	...
      	}
      
      Obviously, unmatched increments are another way to get to a deadlock.
      TestTransportMaxConnsPerHost deadlocked approximately 100% of
      the time with a small random sleep added between incHostConnCount
      and the select:
      
      	ch := t.incHostConnCount(cmKey):
      	time.Sleep(time.Duration(rand.Intn(10))*time.Millisecond)
      	select {
      	case <-ch
      		// count below conn per host limit; proceed
      	case pc := <-t.getIdleConnCh(cm):
      		...
      	}
      
      The limit also did not properly apply to HTTP/2, because of the
      decrement being attached to the underlying net.Conn.Close
      and net/http not having access to the underlying HTTP/2 conn.
      The alternate decrements for HTTP/2 may also have introduced
      spurious decrements (discussion in #29889). Perhaps those
      spurious decrements or other races caused the other intermittent
      non-deadlock failures in TestTransportMaxConnsPerHost,
      in which the HTTP/2 phase created too many connections (#31982).
      
      This CL replaces the buggy, racy code with new code that is hopefully
      neither buggy nor racy.
      
      Fixes #29889.
      Fixes #31982.
      Fixes #32336.
      
      Change-Id: I0dfac3a6fe8a6cdf5f0853722781fe2ec071ac97
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184262
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      fbaf881c
    • Than McIntosh's avatar
      test: add new test case for gccgo compiler bug · ddc8439b
      Than McIntosh authored
      Test case that causes incorrect compiler error from gccgo.
      
      Updates #32922
      
      Change-Id: I59432a8e8770cf03eda293f6d110c081c18fa88b
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184918
      Run-TryBot: Than McIntosh <thanm@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      ddc8439b
  8. 06 Jul, 2019 1 commit
  9. 05 Jul, 2019 5 commits
    • Austin Clements's avatar
      cmd/cgo: accept weak dynamic imports · a2fb5cd8
      Austin Clements authored
      cgo produces dynamic imports for Go binaries by scanning the dynamic
      imports table of a binary produced by the system C compiler and
      linker. Currently, since it uses elf.File.ImportedSymbols, it only
      reads global symbols. Unfortunately, recent versions of lld emit weak
      symbol imports for several pthread symbols, which means the cgo tool
      doesn't emit dynamic imports for them, which ultimately causes linking
      of cgo binaries to fail.
      
      Fix this by using elf.File.DynamicSymbols instead and filtering down
      to both global and weak symbols.
      
      Fixes #31912.
      
      Change-Id: If346a7eca6733e3bfa2cccf74a9cda02a3e81d38
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184100
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      a2fb5cd8
    • Austin Clements's avatar
      debug/elf: add version information to all dynamic symbols · 7aac3436
      Austin Clements authored
      Currently, File.ImportedSymbols is the only API that exposes the GNU
      symbol version information for dynamic symbols. Unfortunately, it also
      filters to specific types of symbols, and only returns symbol names.
      
      The cgo tool is going to need symbol version information for more
      symbols. In order to support this and make the API more orthogonal,
      this CL adds version information to the Symbol type and updates
      File.DynamicSymbols to fill this in. This has the downside of
      increasing the size of Symbol, but seems to be the most natural API
      for exposing this. I also explored 1) adding a method to get the
      version information for the i'th dynamic symbol, but we don't use
      symbol indexes anywhere else in the API, and it's not clear if this
      index would be 0-based or 1-based, and 2) adding a
      DynamicSymbolVersions method that returns a slice of version
      information that parallels the DynamicSymbols slice, but that's less
      efficient to implement and harder to use.
      
      For #31912.
      
      Change-Id: I69052ac3894f7af2aa9561f7085275130e0cf717
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184099
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      7aac3436
    • Keith Randall's avatar
      test: add another test for issue 32680 · 9a00e646
      Keith Randall authored
      Update #32680
      
      Change-Id: I0318c22c22c5cd6ab6441d1aa2d1a40d20d71242
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185137
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      9a00e646
    • Mohit Agarwal's avatar
      doc/go1.13: fix links and a closing tag · e94472a3
      Mohit Agarwal authored
      - fix link for `Time.Format`
      - fix closing tag for `go get`
      - add links for `runtime.Caller`, `runtime.Callers`
      - remove link for `TypedArrayOf` since it has been removed (CL 177537)
      
      Change-Id: I1dc38226e6d91c68fbd2f02c1acfad5327f4ebe8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185038Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      e94472a3
    • Patrik Lundin's avatar
      net/http: stop ExampleServer_Shutdown from hanging on error · 04e2e81e
      Patrik Lundin authored
      Running the example code when not having permissions
      to bind to port 80 will cause the program to hang after
      printing the error message.
      
      Change-Id: I2433ba2629b362fc8f1731e40cab5eea72ec354f
      GitHub-Last-Rev: 0bb3dc08b6f646470fc6ff208ea12bca901a2299
      GitHub-Pull-Request: golang/go#32947
      Reviewed-on: https://go-review.googlesource.com/c/go/+/185157Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      04e2e81e
  10. 03 Jul, 2019 6 commits
  11. 02 Jul, 2019 3 commits
  12. 01 Jul, 2019 2 commits
  13. 30 Jun, 2019 1 commit
    • Ian Lance Taylor's avatar
      runtime: use a pipe to wake up signal_recv on Darwin · c485e8b5
      Ian Lance Taylor authored
      The implementation of semaphores, and therefore notes, used on Darwin
      is not async-signal-safe. The runtime has one case where a note needs
      to be woken up from a signal handler: the call to notewakeup in sigsend.
      That notewakeup call is only called on a single note, and it doesn't
      need the full functionality of notes: nothing ever does a timed wait on it.
      So change that one note to use a different implementation on Darwin,
      based on a pipe. This lets the wakeup code use the write call, which is
      async-signal-safe.
      
      Fixes #31264
      
      Change-Id: If705072d7a961dd908ea9d639c8d12b222c64806
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184169
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      c485e8b5
  14. 29 Jun, 2019 1 commit
    • Russ Cox's avatar
      crypto/tls: deflake localPipe in tests · 623d653d
      Russ Cox authored
      The localPipe implementation assumes that every successful net.Dial
      results in exactly one successful listener.Accept. I don't believe this
      is guaranteed by essentially any operating system. For this test, we're
      seeing flakes on dragonfly (#29583).
      
      But see also #19519, flakes due to the same assumption on FreeBSD
      and macOS in package net's own tests.
      
      This CL rewrites localPipe to try a few times to get a matching pair
      of connections on the dial and accept side.
      
      Fixes #29583.
      
      Change-Id: Idb045b18c404eae457f091df20456c5ae879a291
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184157
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      623d653d
  15. 28 Jun, 2019 1 commit
    • Russ Cox's avatar
      net: deflake TestVariousDeadlines · 2e0cd2ae
      Russ Cox authored
      TestVariousDeadlines starts a client and server.
      The client dials the server, sets a timeout on the connection,
      reads from it, gets a timeout error, closes the connection.
      The server writes an infinite stream of a's to each connection
      it accepts.
      
      The test was trying to run these in lockstep:
      run a client dial+read+timeout+close,
      wait for server to accept+write+error out on write to closed connection,
      repeat.
      
      On FreeBSD 11.2 and less frequently on macOS we see
      the test timeout waiting for the server to do its half of
      the lockstep dance.
      
      I believe the problem is that the client can do its step
      of the dance with such a short timeout that the read,
      timeout, and close happens before the server ever returns
      from the accept(2) system call. For the purposes of testing
      the client-side read timeout, this is fine. But I suspect
      that under some circumstances, the "TCP-accepted"
      connection does not translate into a "socket-layer-accepted"
      connection that triggers a return from accept(2).
      That is, the Go server never sees the connection at all.
      And the test sits there waiting for it to acknowledge
      being done with a connection it never started with.
      
      Fix the problem by not trying to lockstep with the server.
      
      This definitely fixes the flake, since the specific line that
      was calling t.Fatal is now deleted.
      
      This exposes a different flake, seen on a trybot run for an
      early version of this CL, in which the client's io.Copy does
      not stop within the time allotted. The problem now is that
      there is no guarantee that a read beyond the deadline with
      available data returns an error instead of the available data,
      yet the test assumes this guarantee, and in fact the opposite
      is usually true - we don't bother checking the deadline unless
      the read needs to block. That is, deadlines don't cut off a
      flood of available data, yet this test thinks they do.
      
      This CL therefore also changes the server not to send an
      infinite flood of data - don't send any data at all - so that
      the read deadline is guaranteed to be exercised.
      
      Fixes #19519.
      
      Change-Id: I58057c3ed94ac2aebab140ea597f317abae6e65e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184137
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      2e0cd2ae