1. 26 Sep, 2019 18 commits
    • Giovanni Bajo's avatar
      cmd/compile: in prove, learn facts from OpSliceMake · 87e2b34f
      Giovanni Bajo authored
      Now that OpSliceMake is called by runtime.makeslice callers,
      prove can see and record the actual length and cap of each
      slice being constructed.
      
      This small patch is enough to remove 260 additional bound checks
      from cmd+std.
      
      Thanks to Martin Möhrmann for pointing me to CL141822 that
      I had missed.
      
      Updates #24660
      
      Change-Id: I14556850f285392051f3f07d13b456b608b64eb9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196784
      Run-TryBot: Giovanni Bajo <rasky@develer.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      87e2b34f
    • Joel Sing's avatar
      cmd/internal/obj/riscv: require memory targets for load and store instructions · 08165932
      Joel Sing authored
      This allows for `LD 4(X5), X6' rather than `LD $4, X5, X6'. Similar for other
      load and store instructions. It is worth noting that none of these are likely
      to be used directly once the MOV pseudo-instructions are implemented.
      
      Updates #27532
      
      Change-Id: Ie043c2dedd2cdaceb258b27976cfb3f74aa1cc1d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196842Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      08165932
    • Brad Fitzpatrick's avatar
      test: make -all_codegen default to true on linux-amd64 builder · cb418dd0
      Brad Fitzpatrick authored
      Fixes #34297
      
      Change-Id: I4584a97d4562d7af0412d683ba1c206e3c1d9edb
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197539
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      cb418dd0
    • Joel Sing's avatar
      cmd/internal/obj/riscv: implement control transfer instructions · a37f2b4f
      Joel Sing authored
      Add support for assembling control transfer instructions.
      
      Based on the riscv-go port.
      
      Updates #27532
      
      Change-Id: I205d3ccd0a48deeaace0f20fca8516f382a83fae
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196841Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      a37f2b4f
    • Joel Sing's avatar
      cmd/internal/obj/riscv: implement AUIPC and LUI instructions · 430d2aa3
      Joel Sing authored
      Add support for assembling AUIPC and LUI instructions.
      
      Based on the riscv-go port.
      
      Updates #27532
      
      Change-Id: I178868b6dcc6fdc6b8527454569a3538ed50723e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196840Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      430d2aa3
    • Filippo Valsorda's avatar
    • Filippo Valsorda's avatar
      net/textproto: don't normalize headers with spaces before the colon · 41b1f88e
      Filippo Valsorda authored
      RFC 7230 is clear about headers with a space before the colon, like
      
      X-Answer : 42
      
      being invalid, but we've been accepting and normalizing them for compatibility
      purposes since CL 5690059 in 2012.
      
      On the client side, this is harmless and indeed most browsers behave the same
      to this day. On the server side, this becomes a security issue when the
      behavior doesn't match that of a reverse proxy sitting in front of the server.
      
      For example, if a WAF accepts them without normalizing them, it might be
      possible to bypass its filters, because the Go server would interpret the
      header differently. Worse, if the reverse proxy coalesces requests onto a
      single HTTP/1.1 connection to a Go server, the understanding of the request
      boundaries can get out of sync between them, allowing an attacker to tack an
      arbitrary method and path onto a request by other clients, including
      authentication headers unknown to the attacker.
      
      This was recently presented at multiple security conferences:
      https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn
      
      net/http servers already reject header keys with invalid characters.
      Simply stop normalizing extra spaces in net/textproto, let it return them
      unchanged like it does for other invalid headers, and let net/http enforce
      RFC 7230, which is HTTP specific. This loses us normalization on the client
      side, but there's no right answer on the client side anyway, and hiding the
      issue sounds worse than letting the application decide.
      
      Fixes CVE-2019-16276
      Fixes #34540
      
      Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1
      Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@google.com>
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197503
      Run-TryBot: Filippo Valsorda <filippo@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDmitri Shuralyov <dmitshur@golang.org>
      41b1f88e
    • Russ Cox's avatar
      context: use fewer goroutines in WithCancel/WithTimeout · 0ad36867
      Russ Cox authored
      If the parent context passed to WithCancel or WithTimeout
      is a known context implementation (one created by this package),
      we attach the child to the parent by editing data structures directly;
      otherwise, for unknown parent implementations, we make a
      goroutine that watches for the parent to finish and propagates
      the cancellation.
      
      A common problem with this scheme, before this CL, is that
      users who write custom context implementations to manage
      their value sets cause WithCancel/WithTimeout to start
      goroutines that would have not been started before.
      
      This CL changes the way we map a parent context back to the
      underlying data structure. Instead of walking up through
      known context implementations to reach the *cancelCtx,
      we look up parent.Value(&cancelCtxKey) to return the
      innermost *cancelCtx, which we use if it matches parent.Done().
      
      This way, a custom context implementation wrapping a
      *cancelCtx but not changing Done-ness (and not refusing
      to return wrapped keys) will not require a goroutine anymore
      in WithCancel/WithTimeout.
      
      For #28728.
      
      Change-Id: Idba2f435c81b19fe38d0dbf308458ca87c7381e9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196521Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      0ad36867
    • Brad Fitzpatrick's avatar
      Revert "internal/bytealg: add assembly implementation of Compare/CompareString on mips64x" · ae024d9d
      Brad Fitzpatrick authored
      This reverts CL 196837 (commit 78baea83).
      
      Reason for revert: broke the mips64le build.
      
      Change-Id: I531da60d098cb227659c9977a3d229474325b0a5
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197538
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      ae024d9d
    • Michael Anthony Knyszek's avatar
      runtime: fix lock acquire cycles related to scavenge.lock · 62e41565
      Michael Anthony Knyszek authored
      There are currently two edges in the lock cycle graph caused by
      scavenge.lock: with sched.lock and mheap_.lock. These edges appear
      because of the call to ready() and stack growths respectively.
      Furthermore, there's already an invariant in the code wherein
      mheap_.lock must be acquired before scavenge.lock, hence the cycle.
      
      The fix to this is to bring scavenge.lock higher in the lock cycle
      graph, such that sched.lock and mheap_.lock are only acquired once
      scavenge.lock is already held.
      
      To faciliate this change, we move scavenger waking outside of
      gcSetTriggerRatio such that it doesn't have to happen with the heap
      locked. Furthermore, we check scavenge generation numbers with the heap
      locked by using gopark instead of goparkunlock, and specify a function
      which aborts the park should there be any skew in generation count.
      
      Fixes #34047.
      
      Change-Id: I3519119214bac66375e2b1262b36ce376c820d12
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191977
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      62e41565
    • Davor Kapsa's avatar
      SECURITY.md: update go versions · f7a00c99
      Davor Kapsa authored
      Change-Id: I59446965b198767af60d617da29b5c2e457829ba
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197477Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      f7a00c99
    • Bryan C. Mills's avatar
      cmd/go/internal/robustio: extend filesystem workarounds to darwin platforms · 81c6bac0
      Bryan C. Mills authored
      The macOS filesystem seems to have gotten significantly flakier as of
      macOS 10.14, so this causes frequently flakes in the 10.14 builders.
      
      We have no reason to believe that it will be fixed any time soon, so
      rather than trying to detect the specific macOS version, we'll apply
      the same workarounds that we use on Windows: classifying (and
      retrying) the errors known to indicate flakiness and relaxing the
      success criteria for renameio.TestConcurrentReadsAndWrites.
      
      Fixes #33041
      
      Change-Id: I74d8c15677951d7a0df0d4ebf6ea03e43eebddf9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197517
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      81c6bac0
    • Jeremy Faller's avatar
      cmd/compile: remove isStmt symbol from FuncInfo · 7defbffc
      Jeremy Faller authored
      As promised in CL 188238, removing the obsolete symbol.
      
      Here are the latest stats. This is baselined at "e53edafb" with only
      these changes applied, run on magna.cam. The linker looks straight
      better (in memory and speed).
      
      There is still a change I'm working on walking the progs to generate the
      debug_lines data in the compiler. That will likely result in a compiler
      speedup.
      
      name                      old time/op       new time/op       delta
      Template                        324ms ± 3%        317ms ± 3%   -2.07%  (p=0.043 n=10+10)
      Unicode                         142ms ± 4%        144ms ± 3%     ~     (p=0.393 n=10+10)
      GoTypes                         1.05s ± 2%        1.07s ± 2%   +1.59%  (p=0.019 n=9+9)
      Compiler                        4.09s ± 2%        4.11s ± 1%     ~     (p=0.218 n=10+10)
      SSA                             12.5s ± 1%        12.7s ± 1%   +1.00%  (p=0.035 n=10+10)
      Flate                           199ms ± 7%        203ms ± 5%     ~     (p=0.481 n=10+10)
      GoParser                        245ms ± 3%        246ms ± 5%     ~     (p=0.780 n=9+10)
      Reflect                         672ms ± 4%        688ms ± 3%   +2.42%  (p=0.015 n=10+10)
      Tar                             280ms ± 4%        284ms ± 4%     ~     (p=0.123 n=10+10)
      XML                             379ms ± 4%        381ms ± 2%     ~     (p=0.529 n=10+10)
      LinkCompiler                    1.16s ± 4%        1.12s ± 2%   -3.03%  (p=0.001 n=10+9)
      ExternalLinkCompiler            2.28s ± 3%        2.23s ± 3%   -2.51%  (p=0.011 n=8+9)
      LinkWithoutDebugCompiler        686ms ± 9%        667ms ± 2%     ~     (p=0.277 n=9+8)
      StdCmd                          14.1s ± 1%        14.0s ± 1%     ~     (p=0.739 n=10+10)
      
      name                      old user-time/op  new user-time/op  delta
      Template                        604ms ±23%        564ms ± 7%     ~     (p=0.661 n=10+9)
      Unicode                         429ms ±40%        418ms ±37%     ~     (p=0.579 n=10+10)
      GoTypes                         2.43s ±12%        2.51s ± 7%     ~     (p=0.393 n=10+10)
      Compiler                        9.22s ± 3%        9.27s ± 3%     ~     (p=0.720 n=9+10)
      SSA                             26.3s ± 3%        26.6s ± 2%     ~     (p=0.579 n=10+10)
      Flate                           328ms ±19%        333ms ±12%     ~     (p=0.842 n=10+9)
      GoParser                        387ms ± 5%        378ms ± 9%     ~     (p=0.356 n=9+10)
      Reflect                         1.36s ±20%        1.43s ±21%     ~     (p=0.631 n=10+10)
      Tar                             469ms ±12%        471ms ±21%     ~     (p=0.497 n=9+10)
      XML                             685ms ±18%        698ms ±19%     ~     (p=0.739 n=10+10)
      LinkCompiler                    1.86s ±10%        1.87s ±11%     ~     (p=0.968 n=10+9)
      ExternalLinkCompiler            3.20s ±13%        3.01s ± 8%   -5.70%  (p=0.046 n=8+9)
      LinkWithoutDebugCompiler        1.08s ±15%        1.09s ±20%     ~     (p=0.579 n=10+10)
      
      name                      old alloc/op      new alloc/op      delta
      Template                       36.3MB ± 0%       36.4MB ± 0%   +0.26%  (p=0.000 n=10+10)
      Unicode                        28.5MB ± 0%       28.5MB ± 0%     ~     (p=0.165 n=10+10)
      GoTypes                         120MB ± 0%        121MB ± 0%   +0.29%  (p=0.000 n=9+10)
      Compiler                        546MB ± 0%        548MB ± 0%   +0.32%  (p=0.000 n=10+10)
      SSA                            1.84GB ± 0%       1.85GB ± 0%   +0.49%  (p=0.000 n=10+10)
      Flate                          22.9MB ± 0%       23.0MB ± 0%   +0.25%  (p=0.000 n=10+10)
      GoParser                       27.8MB ± 0%       27.9MB ± 0%   +0.25%  (p=0.000 n=10+8)
      Reflect                        77.5MB ± 0%       77.7MB ± 0%   +0.27%  (p=0.000 n=9+9)
      Tar                            34.5MB ± 0%       34.6MB ± 0%   +0.23%  (p=0.000 n=10+10)
      XML                            44.2MB ± 0%       44.4MB ± 0%   +0.32%  (p=0.000 n=10+10)
      LinkCompiler                    239MB ± 0%        230MB ± 0%   -3.86%  (p=0.000 n=10+10)
      ExternalLinkCompiler            243MB ± 0%        243MB ± 0%   +0.22%  (p=0.000 n=10+10)
      LinkWithoutDebugCompiler        164MB ± 0%        155MB ± 0%   -5.45%  (p=0.000 n=10+10)
      
      name                      old allocs/op     new allocs/op     delta
      Template                         371k ± 0%         372k ± 0%   +0.44%  (p=0.000 n=10+10)
      Unicode                          340k ± 0%         340k ± 0%   +0.05%  (p=0.000 n=10+10)
      GoTypes                         1.32M ± 0%        1.32M ± 0%   +0.46%  (p=0.000 n=10+10)
      Compiler                        5.34M ± 0%        5.37M ± 0%   +0.59%  (p=0.000 n=10+10)
      SSA                             17.6M ± 0%        17.7M ± 0%   +0.63%  (p=0.000 n=10+10)
      Flate                            233k ± 0%         234k ± 0%   +0.48%  (p=0.000 n=10+10)
      GoParser                         309k ± 0%         310k ± 0%   +0.40%  (p=0.000 n=10+10)
      Reflect                          964k ± 0%         969k ± 0%   +0.54%  (p=0.000 n=10+10)
      Tar                              346k ± 0%         348k ± 0%   +0.48%  (p=0.000 n=10+9)
      XML                              424k ± 0%         426k ± 0%   +0.51%  (p=0.000 n=10+10)
      LinkCompiler                     751k ± 0%         645k ± 0%  -14.13%  (p=0.000 n=10+10)
      ExternalLinkCompiler            1.79M ± 0%        1.69M ± 0%   -5.30%  (p=0.000 n=10+10)
      LinkWithoutDebugCompiler         217k ± 0%         222k ± 0%   +2.02%  (p=0.000 n=10+10)
      
      name                      old object-bytes  new object-bytes  delta
      Template                        547kB ± 0%        559kB ± 0%   +2.17%  (p=0.000 n=10+10)
      Unicode                         215kB ± 0%        216kB ± 0%   +0.60%  (p=0.000 n=10+10)
      GoTypes                        1.99MB ± 0%       2.03MB ± 0%   +2.02%  (p=0.000 n=10+10)
      Compiler                       7.86MB ± 0%       8.07MB ± 0%   +2.73%  (p=0.000 n=10+10)
      SSA                            26.4MB ± 0%       27.2MB ± 0%   +3.27%  (p=0.000 n=10+10)
      Flate                           337kB ± 0%        343kB ± 0%   +2.02%  (p=0.000 n=10+10)
      GoParser                        432kB ± 0%        441kB ± 0%   +2.11%  (p=0.000 n=10+10)
      Reflect                        1.33MB ± 0%       1.36MB ± 0%   +1.87%  (p=0.000 n=10+10)
      Tar                             477kB ± 0%        487kB ± 0%   +2.24%  (p=0.000 n=10+10)
      XML                             617kB ± 0%        632kB ± 0%   +2.33%  (p=0.000 n=10+10)
      
      name                      old export-bytes  new export-bytes  delta
      Template                       18.5kB ± 0%       18.5kB ± 0%     ~     (all equal)
      Unicode                        7.92kB ± 0%       7.92kB ± 0%     ~     (all equal)
      GoTypes                        35.0kB ± 0%       35.0kB ± 0%     ~     (all equal)
      Compiler                        109kB ± 0%        109kB ± 0%   +0.09%  (p=0.000 n=10+10)
      SSA                             137kB ± 0%        137kB ± 0%   +0.03%  (p=0.000 n=10+10)
      Flate                          4.89kB ± 0%       4.89kB ± 0%     ~     (all equal)
      GoParser                       8.49kB ± 0%       8.49kB ± 0%     ~     (all equal)
      Reflect                        11.4kB ± 0%       11.4kB ± 0%     ~     (all equal)
      Tar                            10.5kB ± 0%       10.5kB ± 0%     ~     (all equal)
      XML                            16.7kB ± 0%       16.7kB ± 0%     ~     (all equal)
      
      name                      old text-bytes    new text-bytes    delta
      HelloSize                       760kB ± 0%        760kB ± 0%     ~     (all equal)
      CmdGoSize                      10.8MB ± 0%       10.8MB ± 0%     ~     (all equal)
      
      name                      old data-bytes    new data-bytes    delta
      HelloSize                      10.7kB ± 0%       10.7kB ± 0%     ~     (all equal)
      CmdGoSize                       312kB ± 0%        312kB ± 0%     ~     (all equal)
      
      name                      old bss-bytes     new bss-bytes     delta
      HelloSize                       122kB ± 0%        122kB ± 0%     ~     (all equal)
      CmdGoSize                       146kB ± 0%        146kB ± 0%     ~     (all equal)
      
      name                      old exe-bytes     new exe-bytes     delta
      HelloSize                      1.11MB ± 0%       1.13MB ± 0%   +1.10%  (p=0.000 n=10+10)
      CmdGoSize                      14.9MB ± 0%       15.0MB ± 0%   +0.77%  (p=0.000 n=10+10)
      
      Change-Id: I42e6087cd6231dbdcfff5464e46d373474e455e1
      Reviewed-on: https://go-review.googlesource.com/c/go/+/192417Reviewed-by: default avatarAustin Clements <austin@google.com>
      Run-TryBot: Austin Clements <austin@google.com>
      7defbffc
    • Michael Munday's avatar
      cmd/compile: use numeric condition code masks on s390x · cf032380
      Michael Munday authored
      Prior to this CL conditional branches on s390x always used an
      extended mnemonic such as BNE, BLT and so on to represent branch
      instructions with different condition code masks. This CL adds
      support for numeric condition code masks to the s390x SSA backend
      so that we can encode the condition under which a Block's
      successor is chosen as a field in that Block rather than in its
      type.
      
      This change will be useful as we come to add support for combined
      compare-and-branch instructions. Rather than trying to add extended
      mnemonics for every possible combination of mask and compare-and-
      branch instruction we can instead use a single mnemonic for each
      instruction.
      
      Change-Id: Idb7458f187b50906877d683695c291dff5279553
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197178Reviewed-by: default avatarKeith Randall <khr@golang.org>
      cf032380
    • Jeremy Faller's avatar
      cmd/link: switch linker over to new debug lines from compiler · 8506b7d4
      Jeremy Faller authored
      This switches the linker over to using the new debug_lines data
      generated in the compiler.
      
      Change-Id: If8362d6fcea7db60aaebab670ed6f702ab1c4908
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191968
      Run-TryBot: Jeremy Faller <jeremy@golang.org>
      Run-TryBot: Austin Clements <austin@google.com>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      8506b7d4
    • Tobias Klauser's avatar
      cmd/compile: use yyerrorl in some typechecking functions · 770a1354
      Tobias Klauser authored
      Replace usage of yyerror with yyerrorl in checkdefergo and copytype in
      typecheck.go.
      
      All covered error messages already appear in the tests and the yyerror
      replacement did not lead to any tests failing.
      
      Passes toolstash-check
      
      Updates #19683
      
      Change-Id: I735e83bcda7ddc6a14afb22e50200bcbb9192fc4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/69910
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      770a1354
    • Emmanuel T Odeke's avatar
      internal/poll: make SendFile work with large files on Windows · b2f40019
      Emmanuel T Odeke authored
      CL 192518 was a minimal simplification to get sendfile
      on Windows to work with chunked files, but as I had mentioned,
      I would add even more improvements.
      
      This CL improves it by:
      * If the reader is not an *io.LimitedReader, since the underlying
      reader is anyways an *os.File, we fallback and stat that
      file to determine the file size and then also invoke the chunked
      sendFile on the underlying reader. This issue existed even
      before the prior CL.
      * Extracting the chunked TransmitFile logic and moving it directly
      into internal/poll.SendFile.
      
      Thus if the callers of net.sendFile don't use *io.LimitedReader,
      but have a huge file (>2GiB), we can still invoke the chunked
      internal/poll.SendFile on it directly.
      
      The test case is not included in this patch as it requires
      creating a 3GiB file, but that if anyone wants to view it, they
      can find it at
          https://go-review.googlesource.com/c/go/+/194218/13/src/net/sendfile_windows_test.go
      
      Updates #33193.
      
      Change-Id: I97a67c712d558c84ced716d8df98b040cd7ed7f7
      Reviewed-on: https://go-review.googlesource.com/c/go/+/194218
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAlex Brainman <alex.brainman@gmail.com>
      b2f40019
    • Tardis Xu's avatar
      runtime: detail the method comment · f73d8080
      Tardis Xu authored
      Change the comment to make more conformable to the function implementation.
      
      Change-Id: I8461e2f09824c50e16223a27d0f61070f04bd21b
      GitHub-Last-Rev: c25a8493d3938b38e2c318f7a7b94c9f2eb11bb4
      GitHub-Pull-Request: golang/go#27404
      Reviewed-on: https://go-review.googlesource.com/c/go/+/132477Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      f73d8080
  2. 25 Sep, 2019 22 commits
    • Dan Scales's avatar
      misc, runtime, test: extra tests and benchmarks for defer · 225f484c
      Dan Scales authored
      Add a bunch of extra tests and benchmarks for defer, in preparation for new
      low-cost (open-coded) implementation of defers (see #34481),
      
       - New file defer_test.go that tests a bunch more unusual defer scenarios,
         including things that might have problems for open-coded defers.
       - Additions to callers_test.go actually verifying what the stack trace looks like
         for various panic or panic-recover scenarios.
       - Additions to crash_test.go testing several more crash scenarios involving
         recursive panics.
       - New benchmark in runtime_test.go measuring speed of panic-recover
       - New CGo benchmark in cgo_test.go calling from Go to C back to Go that
         shows defer overhead
      
      Updates #34481
      
      Change-Id: I423523f3e05fc0229d4277dd00073289a5526188
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197017
      Run-TryBot: Dan Scales <danscales@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      225f484c
    • Michael Munday's avatar
      cmd/asm: add masked branch and conditional load instructions to s390x · 8c99e45e
      Michael Munday authored
      The branch-relative-on-condition (BRC) instruction allows us to use
      an immediate to specify under what conditions the branch is taken.
      For example, `BRC $7, L1` is equivalent to `BNE L1`. It is sometimes
      useful to specify branches in this way when either we don't have
      an extended mnemonic for a particular mask value or we want to
      generate the condition code mask programmatically.
      
      The new load-on-condition (LOCR and LOCGR) and compare-and-branch
      (CRJ, CGRJ, CLRJ, CLGRJ, CIJ, CGIJ, CLIJ and CLGIJ) instructions
      provide the same flexibility for conditional loads and combined
      compare and branch instructions.
      
      Change-Id: Ic6f5d399b0157e278b39bd3645f4ee0f4df8e5fc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196558
      Run-TryBot: Michael Munday <mike.munday@ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      8c99e45e
    • Michael Anthony Knyszek's avatar
      runtime: scavenge on growth instead of inline with allocation · eb96f8a5
      Michael Anthony Knyszek authored
      Inline scavenging causes significant performance regressions in tail
      latency for k8s and has relatively little benefit for RSS footprint.
      
      We disabled inline scavenging in Go 1.12.5 (CL 174102) as well, but
      we thought other changes in Go 1.13 had mitigated the issues with
      inline scavenging. Apparently we were wrong.
      
      This CL switches back to only doing foreground scavenging on heap
      growth, rather than doing it when allocation tries to allocate from
      scavenged space.
      
      Fixes #32828.
      
      Change-Id: I1f5df44046091f0b4f89fec73c2cde98bf9448cb
      Reviewed-on: https://go-review.googlesource.com/c/go/+/183857
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Reviewed-by: default avatarMichael Knyszek <mknyszek@google.com>
      eb96f8a5
    • Austin Clements's avatar
      runtime: grow the heap incrementally · f18109d7
      Austin Clements authored
      Currently, we map and grow the heap a whole arena (64MB) at a time.
      Unfortunately, in order to fix #32828, we need to switch from
      scavenging inline with allocation back to scavenging on heap growth,
      but heap-growth scavenging happens in large jumps because we grow the
      heap in large jumps.
      
      In order to prepare for better heap-growth scavenging, this CL
      separates mapping more space for the heap from actually "growing" it
      (tracking the new space with spans). Instead, growing the heap keeps
      track of the "current arena" it's growing into. It track that with new
      spans as needed, and only maps more arena space when the current arena
      is inadequate. The effect to the user is the same, but this will let
      us scavenge on much smaller increments of heap growth.
      
      There are two slightly subtleties to this change:
      
      1. If an allocation requires mapping a new arena and that new arena
         isn't contiguous with the current arena, we don't want to lose the
         unused space in the current arena, so we have to immediately track
         that with a span.
      
      2. The mapped space must be accounted as released and idle, even
         though it isn't actually tracked in a span.
      
      For #32828, since this makes heap-growth scavenging far more
      effective, especially at small heap sizes. For example, this change is
      necessary for TestPhysicalMemoryUtilization to pass once we remove
      inline scavenging.
      
      Change-Id: I300e74a0534062467e4ce91cdc3508e5ef9aa73a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/189957
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Reviewed-by: default avatarMichael Knyszek <mknyszek@google.com>
      f18109d7
    • Michael Anthony Knyszek's avatar
      runtime: redefine scavenge goal in terms of heap_inuse · 9b308112
      Michael Anthony Knyszek authored
      This change makes it so that the scavenge goal is defined primarily in
      terms of heap_inuse at the end of the last GC rather than next_gc. The
      reason behind this change is that next_gc doesn't take into account
      fragmentation, and we can fall into situation where the scavenger thinks
      it should have work to do but there's no free and unscavenged memory
      available.
      
      In order to ensure the scavenge goal still tracks next_gc, we multiply
      heap_inuse by the ratio between the current heap goal and the last heap
      goal, which describes whether the heap is growing or shrinking, and by
      how much.
      
      Finally, this change updates the documentation for scavenging and
      elaborates on why the scavenge goal is defined the way it is.
      
      Fixes #34048.
      Updates #32828.
      
      Change-Id: I8deaf87620b5dc12a40ab8a90bf27932868610da
      Reviewed-on: https://go-review.googlesource.com/c/go/+/193040
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      9b308112
    • Meng Zhuo's avatar
      internal/bytealg: add assembly implementation of Compare/CompareString on mips64x · 78baea83
      Meng Zhuo authored
      name                         old time/op    new time/op    delta
      BytesCompare/1                 28.9ns ± 4%    22.8ns ± 0%    -21.23%  (p=0.000 n=9+10)
      BytesCompare/2                 34.6ns ± 0%    23.5ns ± 0%    -32.01%  (p=0.000 n=8+10)
      BytesCompare/4                 54.6ns ± 0%    41.4ns ± 0%    -24.18%  (p=0.001 n=8+9)
      BytesCompare/8                 73.9ns ± 0%    49.7ns ± 0%    -32.75%  (p=0.002 n=8+10)
      BytesCompare/16                 113ns ± 0%      23ns ± 0%    -79.56%  (p=0.000 n=9+10)
      BytesCompare/32                 190ns ± 0%      26ns ± 0%    -86.53%  (p=0.000 n=10+10)
      BytesCompare/64                 345ns ± 0%      44ns ± 0%    -87.19%  (p=0.000 n=10+10)
      BytesCompare/128                654ns ± 0%      52ns ± 0%    -91.97%  (p=0.000 n=9+8)
      BytesCompare/256               1.27µs ± 0%    0.08µs ± 1%    -94.10%  (p=0.000 n=8+10)
      BytesCompare/512               2.51µs ± 0%    0.12µs ± 0%    -95.26%  (p=0.000 n=9+9)
      BytesCompare/1024              4.99µs ± 0%    0.21µs ± 1%    -95.84%  (p=0.000 n=8+10)
      BytesCompare/2048              9.94µs ± 0%    0.38µs ± 0%    -96.13%  (p=0.000 n=8+10)
      CompareBytesEqual               105ns ± 0%      64ns ± 0%    -39.05%  (p=0.000 n=10+9)
      CompareBytesToNil              34.8ns ± 1%    39.5ns ± 3%    +13.48%  (p=0.000 n=10+10)
      CompareBytesEmpty              33.6ns ± 3%    36.6ns ± 0%     +8.77%  (p=0.000 n=10+10)
      CompareBytesIdentical          29.7ns ± 0%    36.6ns ± 0%    +23.23%  (p=0.000 n=10+10)
      CompareBytesSameLength         69.1ns ± 0%    51.1ns ± 0%    -26.05%  (p=0.000 n=10+10)
      CompareBytesDifferentLength    69.8ns ± 0%    51.1ns ± 0%    -26.79%  (p=0.000 n=10+10)
      CompareBytesBigUnaligned       5.15ms ± 0%    2.18ms ± 0%    -57.62%  (p=0.000 n=9+9)
      CompareBytesBig                5.28ms ± 0%    0.28ms ± 0%    -94.64%  (p=0.000 n=8+10)
      CompareBytesBigIdentical       29.7ns ± 0%    36.8ns ± 0%    +23.91%  (p=0.000 n=8+8)
      
      name                         old speed      new speed      delta
      CompareBytesBigUnaligned      204MB/s ± 0%   480MB/s ± 0%   +135.94%  (p=0.000 n=9+9)
      CompareBytesBig               198MB/s ± 0%  3703MB/s ± 0%  +1765.85%  (p=0.000 n=8+10)
      CompareBytesBigIdentical     35.3TB/s ± 0%  28.5TB/s ± 0%    -19.31%  (p=0.000 n=8+8)
      
      Change-Id: I112d9de2324986fd65ed237a86b11856a1c0f4a7
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196837Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      78baea83
    • Keith Randall's avatar
      runtime: fix ppc64le race code · 44e752c3
      Keith Randall authored
      This code is not currently compiling, the asm vet checks fail. When running race.bash on ppc64le, I get:
      
      runtime/race_ppc64le.s:104:1: [ppc64le] RaceReadRange: wrong argument size 24; expected $...-16
      runtime/race_ppc64le.s:514:1: [ppc64le] racecallbackthunk: unknown variable cmd; offset 0 is arg+0(FP)
      runtime/race_ppc64le.s:515:1: [ppc64le] racecallbackthunk: unknown variable ctx
      
      I'm also not sure why it ever worked; it looks like it is writing
      the arguments to racecallback in the wrong place (the race detector
      itself probably still works, it would just have trouble symbolizing
      any resulting race report).
      
      At a meta-level, we should really add a ppc64le/race builder.
      Otherwise this code will rot, as evidenced by the rot this CL fixes :)
      
      Update #33309
      
      Change-Id: I3b49c2442aa78538fbb631a143a757389a1368fd
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197337
      Run-TryBot: Keith Randall <khr@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarLynn Boger <laboger@linux.vnet.ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      44e752c3
    • Marwan Sulaiman's avatar
      cmd/go: consistent output for -json failures · 6232dadc
      Marwan Sulaiman authored
      When the -json flag is passed to go mod download,
      the sumdb error is embedded in the json Error field.
      Other errors for the same command behave this way as
      well such as module not found. The fix is done by changing
      base.Fatalf into proper error returns.
      
      Fixes #34485
      
      Change-Id: I2727a5c70c7ab03988cad8661894d0f8ec71a768
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197062
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      6232dadc
    • Matthew Dempsky's avatar
      cmd/compile: optimize escape graph construction and walking · 26204671
      Matthew Dempsky authored
      This CL implements several optimizations for the escape analysis flow
      graph:
      
      1. Instead of recognizing heapLoc specially within Escape.outlives,
      set heapLoc.escapes = true and recognize any location with escapes
      set. This allows us to skip adding edges from the heap to escaped
      variables in two cases:
      
      1a. In newLoc, if the location is for a variable or allocation too
      large to fit on the stack.
      
      1b. During walkOne, if we discover that an object's address flows
      somewhere that naturally outlives it.
      
      2. When recording edges in Escape.flow, if x escapes and we're adding
      an edge like "x = &y", we can simply mark that y escapes too.
      
      3. During walkOne, if we reach a location that's marked as escaping,
      we can skip visiting it again: we've either already walked from it, or
      it's in queue to be walked from again.
      
      On average, reduces the number of visited locations by 15%. Reduces
      time spent in escape analysis for particularly hairy packages like
      runtime and gc by about 8%. Reduces escape.go's TODO count by 22%.
      
      Passes toolstash-check.
      
      Change-Id: Iaf86a29d76044e4b4c8ab581b916ef5bb5df4437
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196811Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      26204671
    • Matthew Dempsky's avatar
      cmd/compile: use proper work queue for escape graph walking · 867ea9c1
      Matthew Dempsky authored
      The old escape analysis code used to repeatedly walk the entire flow
      graph until it reached a fixed point. With escape.go, I wanted to
      avoid this if possible, so I structured the walking code with two
      constraints:
      
      1. Always walk from the heap location last.
      
      2. If an object escapes, ensure it has flow edge to the heap location.
      
      This works, but it precludes some graph construction
      optimizations. E.g., if there's an assignment "heap = &x", then we can
      immediately tell that 'x' escapes without needing to visit it during
      the graph walk. Similarly, if there's a later assignment "x = &y", we
      could immediately tell that 'y' escapes too. However, the natural way
      to implement this optimization ends up violating the constraints
      above.
      
      Further, the constraints above don't guarantee that the 'transient'
      flag is handled correctly. Today I think that's handled correctly
      because of the order that locations happen to be constructed and
      visited based on the AST, but I've felt uneasy about it for a little
      while.
      
      This CL changes walkAll to use a proper work queue (technically a work
      stack) to track locations that need to be visited, and allows walkOne
      to request that a location be re-visited.
      
      Passes toolstash-check.
      
      Change-Id: Iaa6f4d3fe4719c04d67009fb9a2a3e4930b3d7c2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196958
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      867ea9c1
    • Than McIntosh's avatar
      test: add testcase for gccgo compiler buglet · fad0a14d
      Than McIntosh authored
      New test containing code that caused a gccgo compiler failure.
      
      Updates #34503.
      
      Change-Id: Id895a1e1249062b7fb147e54bcaa657e774ed0d9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197217
      Run-TryBot: Than McIntosh <thanm@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      fad0a14d
    • Cuong Manh Le's avatar
      cmd/compile: remove n.SetLikely(false) usage · e0dde990
      Cuong Manh Le authored
      n.SetLikely(false) is probably mean to indicate that the branch is
      "unlikely", but it has the real effect of not marking branch as likely.
      
      So invert the test condition, we can use more meaningful n.SetLikely(true).
      
      Before:
      
      	if l2 < 0 {
      		panicmakeslicelen()
      	}
      
      After:
      
      	if l2 >= 0 {
      	} else {
      		panicmakeslicelen
      	}
      
      Fixes #32486
      
      Change-Id: I156fdba1f9a5d554a178c8903f1a391ed304199d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/195197
      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>
      e0dde990
    • Jeremy Faller's avatar
      cmd/compile: update object file format for DWARF file table · 21bf37b5
      Jeremy Faller authored
      In CL 188317, we generate the debug_lines in the compiler, and created a
      new symbol to hold the line table. Here we modify the object file format
      to output the file table.
      
      Change-Id: Ibee192e80b86ff6af36467a0b1c26ee747dfee37
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191167Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarThan McIntosh <thanm@google.com>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      21bf37b5
    • Matthew Dempsky's avatar
      cmd/compile: use underlying OCOMPLIT's position for OPTRLIT · efb97392
      Matthew Dempsky authored
      Currently, when we create an OPTRLIT node, it defaults to the
      OCOMPLIT's final element's position. But it improves error messages to
      use the OCOMPLIT's own position instead.
      
      Change-Id: Ibb031f543c7248d88d99fd0737685e01d86e2500
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197119
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      efb97392
    • Matthew Dempsky's avatar
      cmd/compile: remove -s flag · 3e428363
      Matthew Dempsky authored
      This is better handled by tools like cmd/gofmt, which can
      automatically rewrite the source code and already supports a syntactic
      version of this simplification. (go/types can be used if
      type-sensitive simplification is actually necessary.)
      
      Change-Id: I51332a8f3ff4ab3087bc6b43a491c6d92b717228
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197118Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      3e428363
    • Matthew Dempsky's avatar
      test: add regress test for #27557 · f346a4c4
      Matthew Dempsky authored
      This commit just adds a regress test for a few of the important corner
      cases that I identified in #27557, which turn out to not be tested
      anywhere.
      
      While here, annotate a few of the existing test cases where we could
      improve escape analysis.
      
      Updates #27557.
      
      Change-Id: Ie57792a538f7899bb17915485fabc86100f469a3
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197137
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      f346a4c4
    • Bryan C. Mills's avatar
      cmd/go/internal/modload: annotate replacements in PackageNotInModuleError · 8189a061
      Bryan C. Mills authored
      Fixes #34085
      
      Change-Id: I3111f5997466ad33f51e80c71f5fb2ccebdcc6e4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/193617
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      8189a061
    • Daniel Martí's avatar
      text/template: don't evaluate '.' as a float64 · 0f7b4e72
      Daniel Martí authored
      When using a '.' constant literal as a reflect.Value variadic argument,
      idealConstant would incorrectly result in a float64. This is because
      rune literals can be represented as a float64, and contain a period,
      which tricked the logic into thinking the literal must have been a
      floating point number.
      
      This also happened with other characters that can be part of a floating
      point number, such as 'e' or 'P'.
      
      To fix these edge cases, exit the case sooner if the literal was a rune,
      since that should always go to the int case instead.
      
      Finally, add test cases that verify that they behave properly. These
      would error before, since eq would receive a mix of int and float64,
      which aren't comparable.
      
      Fixes #34483.
      
      Change-Id: Icfcb7803bfa0cf317a1d1adacacad3d69a57eb42
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196808
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTom Payne <tom@airmap.com>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      0f7b4e72
    • Ariel Mashraki's avatar
      text/template: support all comparable types in eq · 95cbcc5c
      Ariel Mashraki authored
      Extends the built-in eq function to support all Go
      comparable types.
      
      Fixes #33740
      
      Change-Id: I522310e313e251c4dc6a013d33d7c2034fe2ec8e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/193837
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      95cbcc5c
    • Joel Sing's avatar
      cmd/internal/obj/riscv: implement floating point instructions · 434e4caf
      Joel Sing authored
      Add support for assembling various single-precision and double-precision
      floating point instructions.
      
      Based on the riscv-go port.
      
      Updates #27532
      
      Change-Id: Iac1aec9b03bb6cbf116b229daeef944d4df550fa
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196839Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      434e4caf
    • Tobias Klauser's avatar
      runtime: gofmt after CL 192937 · 8f5755e7
      Tobias Klauser authored
      CL 192937 introduced some changes which weren't properly gofmt'ed. Do so
      now.
      
      Change-Id: I2d2d57ea8a79fb41bc4ca59fa23f12198d615fd8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196812
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      8f5755e7
    • Brad Fitzpatrick's avatar
      net/http: propagate Client.Timeout down into Request's context deadline · 7fc2625e
      Brad Fitzpatrick authored
      Fixes #31657
      
      Change-Id: I85e9595d3ea30d410f1f4b787925a6879a72bdf2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175857Reviewed-by: default avatarBenny Siegert <bsiegert@gmail.com>
      Run-TryBot: Benny Siegert <bsiegert@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      7fc2625e