1. 27 Nov, 2019 5 commits
  2. 26 Nov, 2019 4 commits
  3. 25 Nov, 2019 8 commits
  4. 24 Nov, 2019 1 commit
  5. 23 Nov, 2019 4 commits
  6. 22 Nov, 2019 15 commits
    • Dmitri Shuralyov's avatar
      net/http: rename tests for Redirect and StripPrefix · 6ba1bf39
      Dmitri Shuralyov authored
      Before May 2018, I mistakenly thought the _suffix naming convention¹
      used by examples also applied to tests. Thanks to a code review comment²
      from Ian Lance Taylor, I have since learned that is not true.
      This trivial change fixes some collateral damage from my earlier
      misunderstanding, resulting in improved test naming consistency.
      
      ¹ https://golang.org/pkg/testing/#hdr-Examples
      ² https://go-review.googlesource.com/c/go/+/112935/1/src/path/filepath/path_test.go#1075
      
      Change-Id: I555f60719629eb64bf2f096aa3dd5e00851827cd
      Reviewed-on: https://go-review.googlesource.com/c/go/+/207446
      Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      6ba1bf39
    • Brad Fitzpatrick's avatar
      net/http: make Transport.IdleConnTimeout consider wall (not monotonic) time · 980f0c00
      Brad Fitzpatrick authored
      Both laptops closing their lids and cloud container runtimes
      suspending VMs both faced the problem where an idle HTTP connection
      used by the Transport could be cached for later reuse before the
      machine is frozen, only to wake up many minutes later to think that
      their HTTP connection was still good (because only a second or two of
      monotonic time passed), only to find out that the peer hung up on them
      when they went to write.
      
      HTTP/1 connection reuse is inherently racy like this, but no need for
      us to step into a trap if we can avoid it. Also, not everybody sets
      Request.GetBody to enable re-tryable POSTs. And we can only safely
      retry requests in some cases.
      
      So with this CL, before reusing an old connection, double check the walltime.
      
      Testing was done both with a laptop (closing the lid for a bit) and
      with QEMU, running "stop" and "cont" commands in the monitor and
      sending QMP guest agent commands to update its wall clock after the
      "cont":
      
      echo '{"execute":"guest-set-time"}' | socat STDIN UNIX-CONNECT:/var/run/qemu-server/108.qga
      
      In both cases, I was running
      https://gist.github.com/bradfitz/260851776f08e4bc4dacedd82afa7aea and
      watching that the RemoteAddr changed after resume.
      
      It's kinda difficult to write an automated test for. I gave a lightning talk on
      using pure emulation user mode qemu for such tests:
      
         https://www.youtube.com/watch?v=69Zy77O-BUM
         https://docs.google.com/presentation/d/1rAAyOTCsB8GLbMgI0CAbn69r6EVWL8j3DPl4qc0sSlc/edit?usp=sharing
         https://github.com/google/embiggen-disk/blob/master/integration_test.go
      
      ... that would probably be a good direction if we want an automated
      test here. But I don't have time to do that now.
      
      Updates #29308 (HTTP/2 remains)
      
      Change-Id: I03997e00491f861629d67a0292da000bd94ed5ca
      Reviewed-on: https://go-review.googlesource.com/c/go/+/204797Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      980f0c00
    • Bryan C. Mills's avatar
      cmd/go/internal/work: reduce code duplication in buildModeInit by using sys.BuildModeSupported · 9f3c2b6d
      Bryan C. Mills authored
      Updates #34347
      
      Change-Id: I6ea02d4737999bf24f5335508b5ed2352b498122
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208458
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      9f3c2b6d
    • Jay Conrod's avatar
      cmd/go: fix and re-enable build_trimpath test · 8324acad
      Jay Conrod authored
      The test was comparing a binary built from a list of files to a test
      build from a named package. That should not (and did not) work. The
      test now compares two binaries built the same way in different
      directories.
      
      Also add a portion of the test for GOPATH and fix the gccgo portion of
      the test (verified manually).
      
      Fixes #35435
      
      Change-Id: I2535a0011c9d97d2274e5550ae277302dbb91e6f
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208234
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      8324acad
    • Bryan C. Mills's avatar
      cmd/go: do not panic when computing Shlib for a package with no Target · 941ac9ce
      Bryan C. Mills authored
      In module mode, a non-main package lacks an install target.
      
      The location of the .shlib corresponding to a given target is stored
      in a .shlibname file alongside its install target, so in module mode
      a non-main package also lacks a .shlibname file.
      
      This also implies that such a package cannot be installed with
      'go install -buildmode=linkshared', but that is a problem
      for another day.
      
      Fixes #35759
      Updates #34347
      
      Change-Id: Id3e0e068266d5fb9b061a59e70f9a65985d4973b
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208233
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      941ac9ce
    • Bryan C. Mills's avatar
      cmd/go: add a 'buildmode' condition for script tests · 28314cf1
      Bryan C. Mills authored
      In CL 208233 I am fixing a panic that occurs only with a specific
      build mode. I want that test to run on all platforms that support that
      build mode, but the logic for determining support is somewhat
      involved.
      
      For now, I am duplicating that logic into the cmd/internal/sys
      package, which already reports platform support for other build flags.
      
      We can refactor cmd/go/internal/work to use the extracted function in
      a followup CL.
      
      Updates #35759
      
      Change-Id: Ibbaedde4d1e8f683c650beedd10849bc27e7a6e7
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208457
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      28314cf1
    • Bryan C. Mills's avatar
      misc/cgo/testshared: make -v output less verbose · c931f1b6
      Bryan C. Mills authored
      Previously, 'go test -v' in this directory would result in a massive
      dump of go command output, because the test plumbed -v to 'build -x'.
      This change separates them into distinct flags, so that '-v' only
      implies the display of default 'go' command output.
      
      Updates #30316
      
      Change-Id: Ifb125f35ec6a0bebe7e8286e7c546d132fb213df
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208232
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      c931f1b6
    • Michael Anthony Knyszek's avatar
      runtime: release worldsema before Gosched in STW GC mode · 05511a5c
      Michael Anthony Knyszek authored
      After CL 182657 we no longer hold worldsema across the GC, we hold
      gcsema instead.
      
      However in STW GC mode we don't release worldsema before calling Gosched
      on the user goroutine (note that user goroutines are disabled during STW
      GC) so that user goroutine holds onto it. When the GC is done and the
      runtime inevitably wants to "stop the world" again (though there isn't
      much to stop) it'll sit there waiting for worldsema which won't be
      released until the aforementioned goroutine is scheduled, which it won't
      be until the GC is done!
      
      So, we have a deadlock.
      
      The fix is easy: just release worldsema before calling Gosched.
      
      Fixes #34736.
      
      Change-Id: Ia50db22ebed3176114e7e60a7edaf82f8535c1b4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208379
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      05511a5c
    • Michael Anthony Knyszek's avatar
      runtime: increase TestPhysicalMemoryUtilization threshold · 1c5bd345
      Michael Anthony Knyszek authored
      TestPhysicalMemoryUtilization occasionally fails on some platforms by
      only a small margin. The reason for this is that it assumes the
      scavenger will always be able to scavenge all the memory that's released
      by sweeping, but because of the page cache, there could be free and
      unscavenged memory held onto by a P which the scavenger simply cannot
      get to.
      
      As a result, if the page cache gets filled completely (512 KiB of free
      and unscavenged memory) this could skew a test which expects to
      scavenge roughly 8 MiB of memory. More specifically, this is 512 KiB of
      memory per P, and if a system is more inclined to bounce around
      between Ps (even if there's only one goroutine), this memory can get
      "stuck".
      
      Through some experimentation, I found that failures correlated highly
      with relatively large amounts of memory ending up in some page cache
      (like 60 or 64 pages) on at least one P.
      
      This change changes the test's threshold such that it accounts for the
      page cache, and scales up with GOMAXPROCS. Because the test constants
      themselves don't change, however, the test must now also bound
      GOMAXPROCS such that the threshold doesn't get too high (at which point
      the test becomes meaningless).
      
      Fixes #35580.
      
      Change-Id: I6bdb70706de991966a9d28347da830be4a19d3a1
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208377
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      1c5bd345
    • Jay Conrod's avatar
      cmd/go: add 'go generate' commands to modfile_flag test · 300f5d5b
      Jay Conrod authored
      Verify that 'go generate' works with -modfile. Also check that
      go commands starts with 'go generate' do not inherit -modfile, but
      they should still work if -modfile is set in GOFLAGS.
      
      Updates #34506
      
      Change-Id: I5e1f897b4e38e4fdaccc0fbb7a71b8d0e9fc0660
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208236
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      300f5d5b
    • Bryan C. Mills's avatar
      misc/cgo/testcshared: avoid writing to GOROOT in tests · 3922c006
      Bryan C. Mills authored
      The tests in this package invoked 'go install -i -buildmode=c-shared'
      in order to generate an archive as well as multiple C header files.
      
      Unfortunately, the behavior of the '-i' flag is inappropriately broad
      for this use-case: it not only generates the library and header files
      (as desired), but also attempts to install a number of (unnecessary)
      archive files for transitive dependencies to
      GOROOT/pkg/$GOOS_$GOARCH_testcshared_shared, which may not be writable
      — for example, if GOROOT is owned by the root user but the test is
      being run by a non-root user.
      
      Instead, for now we generate the header files for transitive dependencies
      separately by running 'go tool cgo -exportheader'.
      
      In the future, we should consider how to improve the ergonomics for
      generating transitive header files without coupling that to
      unnecessary library installation.
      
      Updates #28387
      Updates #30316
      Updates #35715
      
      Change-Id: I622426a860828020d98f7040636f374e5c766d28
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208119
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      3922c006
    • Bryan C. Mills's avatar
      misc/cgo/testcarchive: avoid writing to GOROOT in tests · c02f3b86
      Bryan C. Mills authored
      Also add a -testwork flag to facilitate debugging the test itself.
      
      Three of the tests of this package invoked 'go install -i
      -buildmode=c-archive' in order to generate an archive as well as
      multiple C header files.
      
      Unfortunately, the behavior of the '-i' flag is inappropriately broad
      for this use-case: it not only generates the library and header files
      (as desired), but also attempts to install a number of (unnecessary)
      archive files for transitive dependencies to
      GOROOT/pkg/$GOOS_$GOARCH_shared, which may not be writable — for
      example, if GOROOT is owned by the root user but the test is being run
      by a non-root user.
      
      Instead, for now we generate the header files for transitive dependencies
      separately by running 'go tool cgo -exportheader'.
      
      In the future, we should consider how to improve the ergonomics for
      generating transitive header files without coupling that to
      unnecessary library installation.
      
      Updates #28387
      Updates #30316
      Updates #35715
      
      Change-Id: I3d483f84e22058561efe740aa4885fc3f26137b5
      Reviewed-on: https://go-review.googlesource.com/c/go/+/208117
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      c02f3b86
    • Hana Kim's avatar
      runtime/pprof: avoid crash due to truncated stack traces · ea18a1c2
      Hana Kim authored
      The profile proto message builder maintains a location entry cache
      that maps a location (possibly involving multiple user frames
      that represent inlined function calls) to the location id. We have
      been using the first pc of the inlined call sequence as the key of
      the cached location entry assuming that, for a given pc, the sequence
      of frames representing the inlined call stack is deterministic and
      stable. Then, when analyzing the new stack trace, we expected the
      exact number of pcs to be present in the captured stack trace upon
      the cache hit.
      
      This assumption does not hold, however, in the presence of the stack
      trace truncation in the runtime during profiling, and also with the
      potential bugs in runtime.
      
      A better fix is to use all the pcs of the inlined call sequece as
      the key instead of the first pc. But that is a bigger code change.
      This CL avoids the crash assuming the trace was truncated.
      
      Fixes #35538
      
      Change-Id: I8c6bae98bc8b178ee51523c7316f56b1cce6df16
      Reviewed-on: https://go-review.googlesource.com/c/go/+/207609
      Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      ea18a1c2
    • two's avatar
      reflect: remove obsolete comment about gobitVector · 95be9b75
      two authored
      Change-Id: Ie3495a51ac2021a55e7c1ee43a66d07a5bf2757a
      GitHub-Last-Rev: b6a6bab3ab840b361021b25cac37eb6891c0fe4b
      GitHub-Pull-Request: golang/go#35709
      Reviewed-on: https://go-review.googlesource.com/c/go/+/207853Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      95be9b75
    • David Chase's avatar
      cmd/compile: try harder to not use an empty src.XPos for a bogus line · 0e02cfb3
      David Chase authored
      The fix for #35652 did not guarantee that it was using a non-empty
      src position to replace an empty one.  The new code checks again
      and falls back to a more certain position.  (The input in question
      compiles to a single empty infinite loop, and none of the actual instructions
      had any source position at all.  That is a bug, but given the pathology
      of this input, not one worth dealing with this late in the release cycle,
      if ever.)
      
      Literally:
      
      00000 (5) TEXT "".f(SB), ABIInternal
      00001 (5) PCDATA $0, $-2
      00002 (5) PCDATA $1, $-2
      00003 (5) FUNCDATA $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
      00004 (5) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
      00005 (5) FUNCDATA $2, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
      b2
      00006 (?) XCHGL AX, AX
      b6
      00007 (+1048575) JMP 6
      00008 (?) END
      
      TODO: Add runtime.InfiniteLoop(), replace infinite loops with a call to
      that, and use an eco-friendly runtime.gopark instead.  (This was Cherry's
      excellent idea.)
      
      Updates #35652
      Fixes #35695
      
      Change-Id: I4b9a841142ee4df0f6b10863cfa0721a7e13b437
      Reviewed-on: https://go-review.googlesource.com/c/go/+/207964
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      0e02cfb3
  7. 21 Nov, 2019 3 commits