1. 25 Oct, 2019 21 commits
    • 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
  2. 24 Oct, 2019 15 commits
    • Bryan C. Mills's avatar
      cmd/go/internal/modfetch/codehost: remove invariantly-empty return value from Repo.ReadZip · 680ed10c
      Bryan C. Mills authored
      Previously, codehost.Repo.ReadZip returned an 'actualSubdir' value
      that was the empty string in all current implementations.
      
      Updates #26092
      
      Change-Id: I6708dd0f13ba88bcf1a1fb405e9d818fd6f9197e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203277
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      680ed10c
    • Jay Conrod's avatar
      cmd/go: add -modfile flag that sets go.mod file to read/write · c357b363
      Jay Conrod authored
      This change adds the -modfile flag to module aware build commands and
      to 'go mod' subcommands. -modfile may be set to a path to an alternate
      go.mod file to be read and written. A real go.mod file must still
      exist and is used to set the module root directory. However, it is not
      opened.
      
      When -modfile is set, the effective location of the go.sum file is
      also changed to the -modfile with the ".mod" suffix trimmed (if
      present) and ".sum" added.
      
      Updates #34506
      
      Change-Id: I2d1e044e18af55505a4f24bbff09b73bb9c908b4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202564
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      c357b363
    • Jay Conrod's avatar
      cmd/go/internal/modload: remove cwd global · c4c37547
      Jay Conrod authored
      base.Cwd should be used instead.
      
      Change-Id: I3dbdecf745b0823160984cc942c883dc04c91d7b
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203037
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      c4c37547
    • Bryan C. Mills's avatar
      cmd/go: correct GoMod field in 'go list' for replacements that lack an explicit go.mod file · f922cc64
      Bryan C. Mills authored
      Change-Id: I241a3bbaf9c4b779b74146232d2f144bb46a0dc7
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203178
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      f922cc64
    • Jay Conrod's avatar
      doc: add skeleton module documentation with headings · bd622872
      Jay Conrod authored
      Sections will be filled in with individual CLs before Go 1.14.
      
      NOTE: This document is currently in Markdown for ease of writing /
      reviewing. Before Go 1.14, we will either ensure that x/website
      can render Markdown (flavor TBD) or check in a rendered HTML file that
      can be displayed directly.
      
      Updates #33637
      
      Change-Id: Icd43fa2bdb7d256b28a56b93214b70343f43492e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202081Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      bd622872
    • Bryan C. Mills's avatar
      cmd/go: re-enable 'go list -m' with -mod=vendor for limited patterns · fc335068
      Bryan C. Mills authored
      I had prohibited 'go list -m' with -mod=vendor because the module
      graph is incomplete, but I've realized that many queries do not
      actually require the full graph — and may, in fact, be driven using
      modules previously reported by 'go list' for specific, vendored
      packages. Queries for those modules should succeed.
      
      Updates #33848
      
      Change-Id: I1000b4cf586a830bb78faf620ebf62d73a3cb300
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203138
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      fc335068
    • Bryan C. Mills's avatar
      cmd/go: populate available module information for packages in vendor mode · b36f22bf
      Bryan C. Mills authored
      Updates #33848
      
      Change-Id: I10b4c79faef8bc3dee2ceba14d496fa049e84fb2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202977
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      b36f22bf
    • Cuong Manh Le's avatar
      syscall: correct comment in testGetdirentries · 05ee5065
      Cuong Manh Le authored
      Correct comment about allocating big enough slice to copy result of
      Getdirentries.
      
      While at it, also convert from Dirent directly to slice of byte.
      
      Updates #35092
      
      Change-Id: I892de7953120622882e1561728e1e56b009a2351
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202880
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      05ee5065
    • Dan Scales's avatar
      cmd/compile, cmd/link, runtime: make defers low-cost through inline code and extra funcdata · be64a19d
      Dan Scales authored
      Generate inline code at defer time to save the args of defer calls to unique
      (autotmp) stack slots, and generate inline code at exit time to check which defer
      calls were made and make the associated function/method/interface calls. We
      remember that a particular defer statement was reached by storing in the deferBits
      variable (always stored on the stack). At exit time, we check the bits of the
      deferBits variable to determine which defer function calls to make (in reverse
      order). These low-cost defers are only used for functions where no defers
      appear in loops. In addition, we don't do these low-cost defers if there are too
      many defer statements or too many exits in a function (to limit code increase).
      
      When a function uses open-coded defers, we produce extra
      FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and
      for each defer, the stack slots where the closure and associated args have been
      stored. The funcdata also includes the location of the deferBits variable.
      Therefore, for panics, we can use this funcdata to determine exactly which defers
      are active, and call the appropriate functions/methods/closures with the correct
      arguments for each active defer.
      
      In order to unwind the stack correctly after a recover(), we need to add an extra
      code segment to functions with open-coded defers that simply calls deferreturn()
      and returns. This segment is not reachable by the normal function, but is returned
      to by the runtime during recovery. We set the liveness information of this
      deferreturn() to be the same as the liveness at the first function call during the
      last defer exit code (so all return values and all stack slots needed by the defer
      calls will be live).
      
      I needed to increase the stackguard constant from 880 to 896, because of a small
      amount of new code in deferreturn().
      
      The -N flag disables open-coded defers. '-d defer' prints out the kind of defer
      being used at each defer statement (heap-allocated, stack-allocated, or
      open-coded).
      
      Cost of defer statement  [ go test -run NONE -bench BenchmarkDefer$ runtime ]
        With normal (stack-allocated) defers only:         35.4  ns/op
        With open-coded defers:                             5.6  ns/op
        Cost of function call alone (remove defer keyword): 4.4  ns/op
      
      Text size increase (including funcdata) for go binary without/with open-coded defers:  0.09%
      
      The average size increase (including funcdata) for only the functions that use
      open-coded defers is 1.1%.
      
      The cost of a panic followed by a recover got noticeably slower, since panic
      processing now requires a scan of the stack for open-coded defer frames. This scan
      is required, even if no frames are using open-coded defers:
      
      Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ]
        Without open-coded defers:        62.0 ns/op
        With open-coded defers:           255  ns/op
      
      A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers:
      
      CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ]
        Without open-coded defers:        443 ns/op
        With open-coded defers:           347 ns/op
      
      Updates #14939 (defer performance)
      Updates #34481 (design doc)
      
      Change-Id: I63b1a60d1ebf28126f55ee9fd7ecffe9cb23d1ff
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202340Reviewed-by: default avatarAustin Clements <austin@google.com>
      be64a19d
    • Bryan C. Mills's avatar
      cmd/go/internal/list: ensure that cfg.BuildMod is initialized before reading it in 'go list -m' · dc77dc2b
      Bryan C. Mills authored
      The default value of cfg.BuildMod depends on the 'go' version in the
      go.mod file. The go.mod file is read and parsed, and its settings are
      applied, in modload.InitMod.
      
      As it turns out, modload.Enabled does not invoke InitMod, so
      cfg.BuildMod is not necessarily set even if modload.Enabled returns
      true.
      
      Updates #33848
      
      Change-Id: I13a4dd80730528e6f1a5acc492fcfe07cb59d94e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202917
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      dc77dc2b
    • Cuong Manh Le's avatar
      syscall: remove un-used const ptrSize · a42a3968
      Cuong Manh Le authored
      Change-Id: Ic809a533f9c4042373bdad3ba1cd237d203bacff
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202881Reviewed-by: default avatarTobias Klauser <tobias.klauser@gmail.com>
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      a42a3968
    • Cuong Manh Le's avatar
      syscall: make convertFromDirents11 checkptr safe · 722b0e32
      Cuong Manh Le authored
      Fixes #35092
      
      Change-Id: I8f1ee2b79d42b2291548fd5645940a61f6d67582
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202878
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      722b0e32
    • Brad Fitzpatrick's avatar
      syscall: make TestGetdirentries checkptr safe · 68981bf3
      Brad Fitzpatrick authored
      Fixes Darwin.
      
      Updates #35092
      
      Change-Id: I045f070c8549d00610b459e3a82cac870d9ddb54
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203077
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarCuong Manh Le <cuong.manhle.vn@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      68981bf3
    • Robert Griesemer's avatar
      math/big: make Rat.Denom side-effect free · a52c0a19
      Robert Griesemer authored
      A Rat is represented via a quotient a/b where a and b are Int values.
      To make it possible to use an uninitialized Rat value (with a and b
      uninitialized and thus == 0), the implementation treats a 0 denominator
      as 1.
      
      Rat.Num and Rat.Denom return pointers to these values a and b. Because
      b may be 0, Rat.Denom used to first initialize it to 1 and thus produce
      an undesirable side-effect (by changing the Rat's denominator).
      
      This CL changes Denom to return a new (not shared) *Int with value 1
      in the rare case where the Rat was not initialized. This eliminates
      the side effect and returns the correct denominator value.
      
      While this is changing behavior of the API, the impact should now be
      minor because together with (prior) CL https://golang.org/cl/202997,
      which initializes Rats ASAP, Denom is unlikely used to access the
      denominator of an uninitialized (and thus 0) Rat. Any operation that
      will somehow set a Rat value will ensure that the denominator is not 0.
      
      Fixes #33792.
      Updates #3521.
      
      Change-Id: I0bf15ac60513cf52162bfb62440817ba36f0c3fc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/203059
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      a52c0a19
    • Robert Griesemer's avatar
      math/big: normalize unitialized denominators ASAP · 4412181e
      Robert Griesemer authored
      A Rat is represented via a quotient a/b where a and b are Int values.
      To make it possible to use an uninitialized Rat value (with a and b
      uninitialized and thus == 0), the implementation treats a 0 denominator
      as 1.
      
      For each operation we check if the denominator is 0, and then treat
      it as 1 (if necessary). Operations that create a new Rat result,
      normalize that value such that a result denominator 1 is represened
      as 0 again.
      
      This CL changes this behavior slightly: 0 denominators are still
      interpreted as 1, but whenever we (safely) can, we set an uninitialized
      0 denominator to 1. This simplifies the code overall.
      
      Also: Improved some doc strings.
      
      Preparation for addressing issue #33792.
      
      Updates #33792.
      
      Change-Id: I3040587c8d0dad2e840022f96ca027d8470878a0
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202997
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      4412181e
  3. 23 Oct, 2019 4 commits
    • Cherry Zhang's avatar
      runtime: save/fetch g register during VDSO on ARM and ARM64 · 758eb020
      Cherry Zhang authored
      On ARM and ARM64, during a VDSO call, the g register may be
      temporarily clobbered by the VDSO code. If a signal is received
      during the execution of VDSO code, we may not find a valid g
      reading the g register. In CL 192937, we conservatively assume
      g is nil. But this approach has a problem: we cannot handle
      the signal in this case. Further, if the signal is not a
      profiling signal, we'll call badsignal, which calls needm, which
      wants to get an extra m, but we don't have one in a non-cgo
      binary, which cuases the program to hang.
      
      This is even more of a problem with async preemption, where we
      will receive more signals than before. I ran into this problem
      while working on async preemption support on ARM64.
      
      In this CL, before making a VDSO call, we save the g on the
      gsignal stack. When we receive a signal, we will be running on
      the gsignal stack, so we can fetch the g from there and move on.
      
      We probably want to do the same for PPC64. Currently we rely on
      that the VDSO code doesn't actually clobber the g register, but
      this is not guaranteed and we don't have control with.
      
      Idea from discussion with Dan Cross and Austin.
      
      Should fix #34391.
      
      Change-Id: Idbefc5e4c2f4373192c2be797be0140ae08b26e3
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202759
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      758eb020
    • Rohan Verma's avatar
      doc/play: match the hello world snippet in x/website · 20bba866
      Rohan Verma authored
      Fixes #35099
      
      Change-Id: Ieaf3174540087bd20443b38703ef88d9f9638052
      GitHub-Last-Rev: 12973bc66f39caebb4156396de3b9df670bf12b5
      GitHub-Pull-Request: golang/go#35123
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202998Reviewed-by: default avatarDmitri Shuralyov <dmitshur@golang.org>
      Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      20bba866
    • Dmitri Shuralyov's avatar
      misc: delete benchcmp forwarding script · e9e4f6c4
      Dmitri Shuralyov authored
      benchcmp was moved out of misc into x/tools in CL 60100043 in 2014,
      and then replaced by a forwarding script in CL 82710043.
      Five years have since passed, and the forwarding script has outlived
      its usefulness. It's now more confusing than helpful. Delete it.
      
      Change-Id: I8c7d65b97e0b3fe367df69a86ae10c7960c05be3
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202762Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      e9e4f6c4
    • Jason A. Donenfeld's avatar
      internal/syscall/windows/registry: blacklist certain registry keys in TestWalkFullRegistry · 033e066c
      Jason A. Donenfeld authored
      It turns out that Windows has "legitimate" keys that have bogus type
      values or bogus lengths that don't correspond with their type. On up to
      date Windows 10 systems, this test always fails for this reason. These
      keys exist because of bugs in Microsoft's code. This commit works around
      the problem by simply blacklisting known instances. It also expands the
      error message a bit so that we can make adjustments should the problem
      ever happen again, and reformats the messages so that it makes copy and
      pasting into the blacklist easier.
      
      Updates #35084
      
      Change-Id: I50322828c0eb0ccecbb62d6bf4f9c726fa0b3c27
      Reviewed-on: https://go-review.googlesource.com/c/go/+/202897
      Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      033e066c