Skip to content

G
go
Switch branch/tag
  1. 28 Oct, 2019 1 commit
  3. 27 Oct, 2019 3 commits
  5. 26 Oct, 2019 16 commits
  7. 25 Oct, 2019 20 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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7