1. 26 Oct, 2019 6 commits
  2. 25 Oct, 2019 22 commits
    • Austin Clements's avatar
      runtime: abstract M preemption check into a function · d1969015
      Austin Clements authored
      We check whether an M is preemptible in a surprising number of places.
      Put it in one function.
      
      For #10958, #24543.
      
      Change-Id: I305090fdb1ea7f7a55ffe25851c1e35012d0d06c
      Reviewed-on: https://go-review.googlesource.com/c/go/+/201439
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      d1969015
    • Austin Clements's avatar
      runtime: only shrink stacks at synchronous safe points · 60586034
      Austin Clements authored
      We're about to introduce asynchronous safe points, where we won't have
      precise pointer maps for all stack frames. That's okay for scanning
      the stack (conservatively), but not for shrinking the stack.
      
      Hence, this CL prepares for this by only shrinking the stack as part
      of the stack scan if the goroutine is stopped at a synchronous safe
      point. Otherwise, it queues up the stack shrink for the next
      synchronous safe point.
      
      We already have one condition under which we can't shrink the stack
      for very similar reasons: syscalls. Currently, we just give up on
      shrinking the stack if it's in a syscall. But with this mechanism, we
      defer that stack shrink until the next synchronous safe point.
      
      For #10958, #24543.
      
      Change-Id: Ifa1dec6f33fdf30f9067be2ce3f7ab8a7f62ce38
      Reviewed-on: https://go-review.googlesource.com/c/go/+/201438
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      60586034
    • Austin Clements's avatar
      runtime: make copystack/sudog synchronization more explicit · 36a432f2
      Austin Clements authored
      When we copy a stack of a goroutine blocked in a channel operation, we
      have to be very careful because other goroutines may be writing to
      that goroutine's stack. To handle this, stack copying acquires the
      locks for the channels a goroutine is waiting on.
      
      One complication is that stack growth may happen while a goroutine
      holds these locks, in which case stack copying must *not* acquire
      these locks because that would self-deadlock.
      
      Currently, stack growth never acquires these locks because stack
      growth only happens when a goroutine is running, which means it's
      either not blocking on a channel or it's holding the channel locks
      already. Stack shrinking always acquires these locks because shrinking
      happens asynchronously, so the goroutine is never running, so there
      are either no locks or they've been released by the goroutine.
      
      However, we're about to change when stack shrinking can happen, which
      is going to break the current rules. Rather than find a new way to
      derive whether to acquire these locks or not, this CL simply adds a
      flag to the g struct that indicates that stack copying should acquire
      channel locks. This flag is set while the goroutine is blocked on a
      channel op.
      
      For #10958, #24543.
      
      Change-Id: Ia2ac8831b1bfda98d39bb30285e144c4f7eaf9ab
      Reviewed-on: https://go-review.googlesource.com/c/go/+/172982
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMichael Knyszek <mknyszek@google.com>
      36a432f2
    • Austin Clements's avatar
      runtime: remove g.gcscanvalid · 8c586157
      Austin Clements authored
      Currently, gcscanvalid is used to resolve a race between attempts to
      scan a stack. Now that there's a clear owner of the stack scan
      operation, there's no longer any danger of racing or attempting to
      scan a stack more than once, so this CL eliminates gcscanvalid.
      
      I double-checked my reasoning by first adding a throw if gcscanvalid
      was set in scanstack and verifying that all.bash still passed.
      
      For #10958, #24543.
      Fixes #24363.
      
      Change-Id: I76794a5fcda325ed7cfc2b545e2a839b8b3bc713
      Reviewed-on: https://go-review.googlesource.com/c/go/+/201139
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      8c586157
    • Austin Clements's avatar
      runtime: remove old stack scanning code · 1b79afe4
      Austin Clements authored
      This removes scang and preemptscan, since the stack scanning code now
      uses suspendG.
      
      For #10958, #24543.
      
      Change-Id: Ic868bf5d6dcce40662a82cb27bb996cb74d0720e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/201138
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      1b79afe4
    • Austin Clements's avatar
      runtime: add general suspendG/resumeG · 3f834114
      Austin Clements authored
      Currently, the process of suspending a goroutine is tied to stack
      scanning. In preparation for non-cooperative preemption, this CL
      abstracts this into general purpose suspendG/resumeG functions.
      
      suspendG and resumeG closely follow the existing scang and restartg
      functions with one exception: the addition of a _Gpreempted status.
      Currently, preemption tasks (stack scanning) are carried out by the
      target goroutine if it's in _Grunning. In this new approach, the task
      is always carried out by the goroutine that called suspendG. Thus, we
      need a reliable way to drive the target goroutine out of _Grunning
      until the requesting goroutine is ready to resume it. The new
      _Gpreempted state provides the handshake: when a runnable goroutine
      responds to a preemption request, it now parks itself and enters
      _Gpreempted. The requesting goroutine races to put it in _Gwaiting,
      which gives it ownership, but also the responsibility to start it
      again.
      
      This CL adds several TODOs about improving the synchronization on the
      G status. The existing code already has these problems; we're just
      taking note of them.
      
      The next CL will remove the now-dead scang and preemptscan.
      
      For #10958, #24543.
      
      Change-Id: I16dbf87bea9d50399cc86719c156f48e67198f16
      Reviewed-on: https://go-review.googlesource.com/c/go/+/201137
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      3f834114
    • pokutuna's avatar
      net/http: fix comment TimeoutHandler no longer supports Flusher · 46e0d724
      pokutuna authored
      Fixes #35161
      Updates #34439
      
      Change-Id: I978534cbb8b9fb32c115dba0066cf099c61d8ee9
      GitHub-Last-Rev: d60581635e8cefb7cfc4b571057542395034c575
      GitHub-Pull-Request: golang/go#35162
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203478Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      46e0d724
    • Bryan C. Mills's avatar
      cmd/go: implement svn support in module mode · dcad8306
      Bryan C. Mills authored
      mod_get_svn passes, and I also tested this manually on a real-world svn-hosted package:
      
      	example.com$ go mod init example.com
      	go: creating new go.mod: module example.com
      
      	example.com$ GOPROXY=direct GONOSUMDB=llvm.org go get -d llvm.org/llvm/bindings/go/llvm
      	go: finding llvm.org/llvm latest
      	go: finding llvm.org/llvm/bindings/go/llvm latest
      	go: downloading llvm.org/llvm v0.0.0-20191022153947-000000375505
      	go: extracting llvm.org/llvm v0.0.0-20191022153947-000000375505
      
      	example.com$ go list llvm.org/llvm/bindings/...
      	llvm.org/llvm/bindings/go
      	llvm.org/llvm/bindings/go/llvm
      
      Fixes #26092
      
      Change-Id: Iefe2151b82a0225c73bb6f8dd7cd8a352897d4c0
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203497
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      dcad8306
    • Tobias Klauser's avatar
      runtime: define emptyfunc as static function in assembly for freebsd/arm64 · 316fb95f
      Tobias Klauser authored
      CL 198544 broke the linux/arm64 build because it declares emptyfunc for
      GOARCH=arm64, but only freebsd/arm64 defines it. Make it a static
      assembly function specific for freebsd/arm64 and remove the stub.
      
      Fixes #35160
      
      Change-Id: I5fd94249b60c6fd259c251407b6eccc8fa512934
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203418Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      316fb95f
    • Bryan C. Mills's avatar
      net/http: skip failing test on windows-amd64-longtest builder · 8bb47a5e
      Bryan C. Mills authored
      bradfitz is actively thinking about a proper fix.
      In the meantime, skip the test to suss out any other failures in the builder.
      
      Updates #35122
      
      Change-Id: I9bf0640222e3d385c1a3e2be5ab52b80d3e8c21a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203500
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      8bb47a5e
    • Bryan C. Mills's avatar
      os/signal: derive TestAtomicStop timeout from overall test timeout · 096126de
      Bryan C. Mills authored
      Previously, TestAtomicStop used a hard-coded 2-second timeout.
      
      That empirically is not long enough on certain builders. Rather than
      adjusting it to a different arbitrary value, use a slice of the
      overall timeout for the test binary. If everything is working, we
      won't block nearly that long anyway.
      
      Updates #35085
      
      Change-Id: I7b789388e3152413395088088fc497419976cf5c
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203499
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      096126de
    • Tobias Klauser's avatar
      cmd/dist: add support for freebsd/arm64 · 3d457f1a
      Tobias Klauser authored
      Updates #24715
      
      Change-Id: I110a10a5d5ed4a471f67f35cbcdcbea296c5dcaf
      Reviewed-on: https://go-review.googlesource.com/c/go/+/198542
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      3d457f1a
    • Tobias Klauser's avatar
      runtime: add support for freebsd/arm64 · 6b6e67f9
      Tobias Klauser authored
      Based on work by Mikaël Urankar (@MikaelUrankar),
      Shigeru YAMAMOTO (@bsd-hacker) and @myfreeweb.
      
      Updates #24715
      
      Change-Id: If3189a693ca0aa627029e22b0f91534bcf322bc0
      Reviewed-on: https://go-review.googlesource.com/c/go/+/198544
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      6b6e67f9
    • Austin Clements's avatar
      runtime: ensure _Grunning Gs have a valid g.m and g.m.p · fc8eb264
      Austin Clements authored
      We already claim on the documentation for _Grunning that this is case,
      but execute transitions to _Grunning before assigning g.m. Fix this
      and make the documentation even more explicit.
      
      For #10958, #24543, but also a good cleanup.
      
      Change-Id: I1eb0108e7762f55cfb0282aca624af1c0a15fe56
      Reviewed-on: https://go-review.googlesource.com/c/go/+/201440
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      fc8eb264
    • Austin Clements's avatar
      runtime: make m.libcallsp check in shrinkstack panic · f82956b8
      Austin Clements authored
      Currently, shrinkstack will not shrink a stack on Windows if
      gp.m.libcallsp != 0. In general, we can't shrink stacks in syscalls
      because the syscall may hold pointers into the stack, and in principle
      this is supposed to be preventing that for libcall-based syscalls
      (which are direct syscalls from the runtime). But this test is
      actually broken and has been for a long time. That turns out to be
      okay because it also appears it's not necessary.
      
      This test is racy. g.m points to whatever M the G was last running on,
      even if the G is in a blocked state, and that M could be doing
      anything, including making libcalls. Hence, observing that libcallsp
      == 0 at one moment in shrinkstack is no guarantee that it won't become
      non-zero while we're shrinking the stack, and vice-versa.
      
      It's also weird that this check is only performed on Windows, given
      that we now use libcalls on macOS, Solaris, and AIX.
      
      This check was added when stack shrinking was first implemented in CL
      69580044. The history of that CL (though not the final version)
      suggests this was necessary for libcalls that happened on Go user
      stacks, which we never do now because of the limited stack space.
      
      It could also be defending against user stack pointers passed to
      libcall system calls from blocked Gs. But the runtime isn't allowed to
      keep pointers into the user stack for blocked Gs on any OS, so it's
      not clear this would be of any value.
      
      Hence, this checks seems to be simply unnecessary.
      
      Rather than simply remove it, this CL makes it defensive. We can't do
      anything about blocked Gs, since it doesn't even make sense to look at
      their M, but if a G tries to shrink its own stack while in a libcall,
      that indicates a bug in the libcall code. This CL makes shrinkstack
      panic in this case.
      
      For #10958, #24543, since those are going to rearrange how we decide
      that it's safe to shrink a stack.
      
      Change-Id: Ia865e1f6340cff26637f8d513970f9ebb4735c6d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/173724
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      f82956b8
    • Austin Clements's avatar
      cmd/internal/obj/x86: correct pcsp for ADJSP · 9d032026
      Austin Clements authored
      The x86 assembler supports an "ADJSP" pseudo-op that compiles to an
      ADD/SUB from SP. Unfortunately, while this seems perfect for an
      instruction that would allow obj to continue to track the SP/FP delta,
      obj currently doesn't do that. As a result, FP-relative references
      won't work and, perhaps worse, the pcsp table will have the wrong
      frame size.
      
      We don't currently use this instruction in any assembly or generate it
      in the compiler, but this is a perfect instruction for solving a
      problem in #24543.
      
      This CL makes ADJSP useful by incorporating it into the SP delta
      logic.
      
      One subtlety is that we do generate ADJSP in obj itself to open a
      function's stack frame. Currently, when preprocess enters the loop to
      compute the SP delta, it may or may not start at this ADJSP
      instruction depending on various factors. We clean this up by instead
      always starting the SP delta at 0 and always starting this loop at the
      entry to the function.
      
      Why not just recognize ADD/SUB of SP? The danger is that could change
      the meaning of existing code. For example, walltime1 in
      sys_linux_amd64.s saves SP, SUBs from it, and aligns it. Later, it
      restores the saved copy and then does a few FP-relative references.
      Currently obj doesn't know any of this is happening, but that's fine
      once it gets to the FP-relative references. If we taught obj to
      recognize the SUB, it would start to miscompile this code. An
      alternative would be to recognize unknown instructions that write to
      SP and refuse subsequent FP-relative references, but that's kind of
      annoying.
      
      This passes toolstash -cmp for std on both amd64 and 386.
      
      Change-Id: Ic6c6a7cbf980bca904576676c07b44c0aaa9c82d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/200877
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      9d032026
    • ZYunH's avatar
      internal/singleflight: format someErr · 00d6b28d
      ZYunH authored
      Error string should not be capitalized.
      
      Change-Id: I8e1d148c6b999450bcd702f420c2a240f82aadc7
      GitHub-Last-Rev: 6ca1b3edb4a61723fa6472a0f54cc6329898edbc
      GitHub-Pull-Request: golang/go#35147
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203339Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      00d6b28d
    • Meng Zhuo's avatar
      cmd/dist: fix wrong goarch on mips64le · dad512f6
      Meng Zhuo authored
      Change-Id: I625f0bc533a7d14010c0344f36e8f157a19c13f2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203437
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Reviewed-by: default avatarTobias Klauser <tobias.klauser@gmail.com>
      dad512f6
    • Meng Zhuo's avatar
      runtime: fix typo of MADV_NOHUGEPAGE · 9a701017
      Meng Zhuo authored
      Change-Id: I60a1ca606fe7492c05697c4d58afc7f19fcc63fe
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203340Reviewed-by: default avatarTobias Klauser <tobias.klauser@gmail.com>
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      9a701017
    • Jason A. Donenfeld's avatar
      internal/syscall/windows/registry: allow for non-null terminated strings · 3c25e5ec
      Jason A. Donenfeld authored
      According to MSDN, "If the data has the REG_SZ, REG_MULTI_SZ or
      REG_EXPAND_SZ type, this size includes any terminating null character or
      characters unless the data was stored without them. [...] If the data
      has the REG_SZ, REG_MULTI_SZ or REG_EXPAND_SZ type, the string may not
      have been stored with the proper terminating null characters. Therefore,
      even if the function returns ERROR_SUCCESS, the application should
      ensure that the string is properly terminated before using it;
      otherwise, it may overwrite a buffer."
      
      It's therefore dangerous to pass it off unbounded as we do, and in fact
      this led to crashes on real systems.
      
      Change-Id: I6d786211814656f036b87fd78631466634cd764a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202937
      Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAlex Brainman <alex.brainman@gmail.com>
      3c25e5ec
    • Rob Pike's avatar
      fmt: fix handling of %% verb in Scanf · 4f70c151
      Rob Pike authored
      There were a couple of bugs, including not requiring a percent and
      returning the wrong error for a bad format containing %%.
      
      Both are addressed by fixing the first.
      
      Fixes #34180.
      
      Change-Id: If96c0c0258bcb95eec49871437d719cb9d399d9b
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202879
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      4f70c151
    • Rémy Oudompheng's avatar
      math/big: use nat pool to reduce allocations in mul and sqr · 8f30d251
      Rémy Oudompheng authored
      This notably allows to reuse temporaries across
      the karatsubaSqr recursion.
      
      benchmark                    old ns/op     new ns/op     delta
      BenchmarkNatMul/10-4         227           228           +0.44%
      BenchmarkNatMul/100-4        8339          8589          +3.00%
      BenchmarkNatMul/1000-4       313796        312272        -0.49%
      BenchmarkNatMul/10000-4      11924720      11873589      -0.43%
      BenchmarkNatMul/100000-4     503813354     503839058     +0.01%
      BenchmarkNatSqr/20-4         549           513           -6.56%
      BenchmarkNatSqr/30-4         945           874           -7.51%
      BenchmarkNatSqr/50-4         1993          1832          -8.08%
      BenchmarkNatSqr/80-4         4096          3874          -5.42%
      BenchmarkNatSqr/100-4        6192          5712          -7.75%
      BenchmarkNatSqr/200-4        20388         19543         -4.14%
      BenchmarkNatSqr/300-4        38735         36715         -5.21%
      BenchmarkNatSqr/500-4        99562         93542         -6.05%
      BenchmarkNatSqr/800-4        195554        184907        -5.44%
      BenchmarkNatSqr/1000-4       286302        275053        -3.93%
      BenchmarkNatSqr/10000-4      9817057       9441641       -3.82%
      BenchmarkNatSqr/100000-4     390713416     379696789     -2.82%
      
      benchmark                    old allocs     new allocs     delta
      BenchmarkNatMul/10-4         1              1              +0.00%
      BenchmarkNatMul/100-4        1              1              +0.00%
      BenchmarkNatMul/1000-4       2              1              -50.00%
      BenchmarkNatMul/10000-4      2              1              -50.00%
      BenchmarkNatMul/100000-4     9              11             +22.22%
      BenchmarkNatSqr/20-4         2              1              -50.00%
      BenchmarkNatSqr/30-4         2              1              -50.00%
      BenchmarkNatSqr/50-4         2              1              -50.00%
      BenchmarkNatSqr/80-4         2              1              -50.00%
      BenchmarkNatSqr/100-4        2              1              -50.00%
      BenchmarkNatSqr/200-4        2              1              -50.00%
      BenchmarkNatSqr/300-4        4              1              -75.00%
      BenchmarkNatSqr/500-4        4              1              -75.00%
      BenchmarkNatSqr/800-4        10             1              -90.00%
      BenchmarkNatSqr/1000-4       10             1              -90.00%
      BenchmarkNatSqr/10000-4      731            1              -99.86%
      BenchmarkNatSqr/100000-4     19687          6              -99.97%
      
      benchmark                    old bytes     new bytes     delta
      BenchmarkNatMul/10-4         192           192           +0.00%
      BenchmarkNatMul/100-4        4864          4864          +0.00%
      BenchmarkNatMul/1000-4       57344         49224         -14.16%
      BenchmarkNatMul/10000-4      565248        498772        -11.76%
      BenchmarkNatMul/100000-4     5749504       7263720       +26.34%
      BenchmarkNatSqr/20-4         672           352           -47.62%
      BenchmarkNatSqr/30-4         992           512           -48.39%
      BenchmarkNatSqr/50-4         1792          896           -50.00%
      BenchmarkNatSqr/80-4         2688          1408          -47.62%
      BenchmarkNatSqr/100-4        3584          1792          -50.00%
      BenchmarkNatSqr/200-4        6656          3456          -48.08%
      BenchmarkNatSqr/300-4        24448         16387         -32.97%
      BenchmarkNatSqr/500-4        36864         24591         -33.29%
      BenchmarkNatSqr/800-4        69760         40981         -41.25%
      BenchmarkNatSqr/1000-4       86016         49180         -42.82%
      BenchmarkNatSqr/10000-4      2524800       487368        -80.70%
      BenchmarkNatSqr/100000-4     68599808      5876581       -91.43%
      
      Change-Id: I8e6e409ae1cb48be9d5aa9b5f428d6cbe487673a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/172017
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      8f30d251
  3. 24 Oct, 2019 12 commits