1. 28 Oct, 2019 6 commits
    • Lynn Boger's avatar
      runtime: fix textOff for multiple text sections · 0ae93896
      Lynn Boger authored
      If a compilation has multiple text sections, code in
      textOff must compare the offset argument against the range
      for each text section to determine which one it is in.
      The comparison looks like this:
      
      if uintptr(off) >= sectaddr && uintptr(off) <= sectaddr+sectlen
      
      If the off value being compared is equal to sectaddr+sectlen then it
      is not within the range of the text section but after it. The
      comparison should be just '<'.
      
      Updates #35207
      
      Change-Id: I114633fd734563d38f4e842dd884c6c239f73c95
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203817
      Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.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>
      0ae93896
    • Lynn Boger's avatar
      crypto/elliptic: add asm implementation for p256 on ppc64le · adef06c7
      Lynn Boger authored
      This adds an asm implementation of the p256 functions used
      in crypto/elliptic, utilizing VMX, VSX to improve performance.
      On a power9 the improvement is:
      
      elliptic benchmarks:
      name            old time/op    new time/op    delta
      BaseMult          1.40ms ± 0%    1.44ms ± 0%   +2.66%  (p=0.029 n=4+4)
      BaseMultP256       317µs ± 0%      50µs ± 0%  -84.14%  (p=0.029 n=4+4)
      ScalarMultP256     854µs ± 2%     214µs ± 0%  -74.91%  (p=0.029 n=4+4)
      
      ecdsa benchmarks:
      name           old time/op    new time/op    delta
      SignP256          377µs ± 0%     111µs ± 0%  -70.57%  (p=0.029 n=4+4)
      SignP384         6.55ms ± 0%    6.48ms ± 0%   -1.03%  (p=0.029 n=4+4)
      VerifyP256       1.19ms ± 0%    0.26ms ± 0%  -78.54%  (p=0.029 n=4+4)
      KeyGeneration     319µs ± 0%      52µs ± 0%  -83.56%  (p=0.029 n=4+4)
      
      This implemenation is based on the s390x implementation, using
      comparable instructions for most with some minor changes where the
      instructions are not quite the same.
      
      Some changes were also needed since s390x is big endian and ppc64le
      is little endian.
      
      This also enables the fuzz_test for ppc64le.
      
      Change-Id: I59a69515703b82ad2929f68ba2f11208fa833181
      Reviewed-on: https://go-review.googlesource.com/c/go/+/168478
      Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMichael Munday <mike.munday@ibm.com>
      adef06c7
    • Phil Pearl's avatar
      encoding/json: remove allocation when using a Marshaler with value receiver · acbed037
      Phil Pearl authored
      If we marshal a non-pointer struct field whose type implements Marshaler with
      a non-pointer receiver, then we avoid an allocation if we take the address of
      the field before casting it to an interface.
      
      name               old time/op    new time/op    delta
      EncodeMarshaler-8     104ns ± 1%      92ns ± 2%  -11.72%  (p=0.001 n=7+7)
      
      name               old alloc/op   new alloc/op   delta
      EncodeMarshaler-8     36.0B ± 0%      4.0B ± 0%  -88.89%  (p=0.000 n=8+8)
      
      name               old allocs/op  new allocs/op  delta
      EncodeMarshaler-8      2.00 ± 0%      1.00 ± 0%  -50.00%  (p=0.000 n=8+8)
      
      Test coverage already looks good enough for this change. TestRefValMarshal
      already covers all possible combinations of value & pointer receivers on
      value and pointer struct fields.
      
      Change-Id: I6fc7f72396396d98f9a90c3c86e813690f41c099
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203608Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      acbed037
    • Tobias Klauser's avatar
      cmd/nm, runtime/cgo: add cgo support for freebsd/arm64 · 33e3983d
      Tobias Klauser authored
      Based on work by Mikaël Urankar (@MikaelUrankar).
      
      Updates #24715
      Updates #35197
      
      Change-Id: I91144101043d67d3f8444bf8389c9606abe2a66c
      Reviewed-on: https://go-review.googlesource.com/c/go/+/199919
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      33e3983d
    • Dmitri Goutnik's avatar
      cmd/link: switch to ld.bfd on freebsd/arm64 · 152dddee
      Dmitri Goutnik authored
      Updates golang/go#35197
      
      Change-Id: I4fd85c84475761d71d2c17e62796e0a411cf91d8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203519
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTobias Klauser <tobias.klauser@gmail.com>
      152dddee
    • Dan Scales's avatar
      runtime: fix dumpgoroutine() to deal with open-coded defers · 1f3339f4
      Dan Scales authored
      _defer.fn can be nil, so we need to add a check when dumping
      _defer.fn.fn.
      
      Fixes #35172
      
      Change-Id: Ic1138be5ec9dce915a87467cfa51ff83acc6e3a9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203697
      Run-TryBot: Dan Scales <danscales@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      1f3339f4
  2. 27 Oct, 2019 3 commits
  3. 26 Oct, 2019 16 commits
  4. 25 Oct, 2019 15 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