1. 20 May, 2018 1 commit
    • Keith Randall's avatar
      runtime: use libc for nanotime on Darwin · cc09212f
      Keith Randall authored
      Use mach_absolute_time and mach_timebase_info to get nanosecond-level
      timing information from libc on Darwin.
      
      The conversion code from Apple's arbitrary time unit to nanoseconds is
      really annoying.  It would be nice if we could replace the internal
      runtime "time" with arbitrary units and put the conversion to nanoseconds
      only in the places that really need it (so it isn't in every nanotime call).
      
      It's especially annoying because numer==denom==1 for all the machines
      I tried.  Makes it hard to test the conversion code :(
      
      Update #17490
      
      Change-Id: I6c5d602a802f5c24e35184e33d5e8194aa7afa86
      Reviewed-on: https://go-review.googlesource.com/110655
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      cc09212f
  2. 19 May, 2018 2 commits
    • Keith Randall's avatar
      runtime: fix darwin 386/amd64 stack switches · e86c2678
      Keith Randall authored
      A few libc_ calls were missing stack switches.
      
      Unfortunately, adding the stack switches revealed a deeper problem.
      systemstack() is fundamentally flawed because when you do
      
          systemstack(func() { ... })
      
      There's no way to mark the anonymous function as nosplit.  At first I
      thought it didn't matter, as that function runs on the g0 stack.  But
      nosplit is still required, because some syscalls are done when stack
      bounds are not set up correctly (e.g. in a signal handler, which runs
      on the g0 stack, but g is still pointing at the g stack).  Instead use
      asmcgocall and funcPC, so we can be nosplit all the way down.
      
      Mid-stack inlining now pushes darwin over the nosplit limit also.
      Leaving that as a TODO.
      Update #23168
      
      This might fix the cause of occasional darwin hangs.
      Update #25181
      
      Update #17490
      
      Change-Id: If9c3ef052822c7679f5a1dd192443f714483327e
      Reviewed-on: https://go-review.googlesource.com/111258Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e86c2678
    • Ali Rizvi-Santiago's avatar
      debug/pe: parse the import directory correctly · e9137299
      Ali Rizvi-Santiago authored
      This parses the import table properly which allows for debug/pe
      to extract import symbols from pecoffs linked with an import
      table in a section named something other than ".idata"
      
      The section names in a pecoff object aren't guaranteed to actually
      mean anything, so hardcoding a search for the ".idata" section
      is not guaranteed to find the import table in all shared libraries.
      This resulted in debug/pe being unable to read import symbols
      from some libraries.
      
      The proper way to locate the import table is to validate the
      number of data directory entries, locate the import entry, and
      then use the va to identify the section containing the import
      table. This patch does exactly this.
      
      Fixes #16103.
      
      Change-Id: I3ab6de7f896a0c56bb86c3863e504e8dd4c8faf3
      GitHub-Last-Rev: ce8077cb154f18ada7a86e152ab03de813937816
      GitHub-Pull-Request: golang/go#25193
      Reviewed-on: https://go-review.googlesource.com/110555
      Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarAlex Brainman <alex.brainman@gmail.com>
      e9137299
  3. 18 May, 2018 5 commits
  4. 17 May, 2018 7 commits
  5. 16 May, 2018 6 commits
  6. 15 May, 2018 6 commits
    • Rob Pike's avatar
      doc/contribute.html: English cleanups · 3868a371
      Rob Pike authored
      Fixes #24487
      
      Change-Id: Ic523e469f7f67f376edd2fca6e07d35bb11b2db9
      Reviewed-on: https://go-review.googlesource.com/113016Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      3868a371
    • Michael Munday's avatar
      cmd/compile: improve error message emitted by debug info generation · 3797f88f
      Michael Munday authored
      Before:
      
      unexpected at 2721:load with unexpected source op v3278unexpected at 2775:load with
      unexpected source op v3281unexpected at 2249:load with unexpected source op
      v3289unexpected at 2875:load with unexpected source op v3278unexpected at 2232:load
      with unexpected source op v286unexpected at 2231:load with unexpected source op
      v3291unexpected at 2784:load with unexpected source op v3289unexpected at 2785:load
      with unexpected source op v3291
      
      After:
      
      debug info generation: v2721: load with unexpected source op: Phi (v3278)
      debug info generation: v2775: load with unexpected source op: Phi (v3281)
      debug info generation: v2249: load with unexpected source op: Phi (v3289)
      debug info generation: v2875: load with unexpected source op: Phi (v3278)
      debug info generation: v2232: load with unexpected source op: Phi (v286)
      debug info generation: v2231: load with unexpected source op: Phi (v3291)
      debug info generation: v2784: load with unexpected source op: Phi (v3289)
      debug info generation: v2785: load with unexpected source op: Phi (v3291)
      
      Updates #25404.
      
      Change-Id: Ib97722848d27ca18bdcd482a610626bc3c6def7d
      Reviewed-on: https://go-review.googlesource.com/113275
      Run-TryBot: Michael Munday <mike.munday@ibm.com>
      Run-TryBot: Heschi Kreinick <heschi@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarHeschi Kreinick <heschi@google.com>
      3797f88f
    • Michael Munday's avatar
      vendor/golang_org/x/crypto: revendor · 212c9479
      Michael Munday authored
      This change updates the vendored copy of golang.org/x/crypto to
      commit 1a580b3eff7814fc9b40602fd35256c63b50f491.
      
      An import of golang.org/x/sys/cpu was replaced with an import of
      internal/cpu as required by
      https://github.com/golang/go/issues/24843#issuecomment-383194779.
      
      The following bash command can be used to replicate this import
      update:
      
      find `pwd` -name '*.go' -exec sed -i 's/golang\.org\/x\/sys\/cpu/internal\/cpu/g' '{}' \;
      
      Change-Id: Ic80d361f940a96c70e4196f594d791c63421d73c
      Reviewed-on: https://go-review.googlesource.com/113175Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      212c9479
    • isharipo's avatar
      mime: do a pre-allocation in encodeWord · 3fb3ca08
      isharipo authored
      The preallocated memory size is comparable to bytes.Buffer bootstraping
      array length (bytes.Buffer was used before rewrite to strings.Builder).
      
      Without preallocation, encodeWord does more than one allocation for
      almost any possible input.
      
      The regression happens because bytes.Buffer did a 80-bytes allocation
      at the beginning of encodeWord while strings.Builder did several
      smaller allocations (started with cap=0).
      
      Comparison with reported regression:
      
        name           old time/op    new time/op    delta
        QEncodeWord-4     781ns ± 1%     593ns ± 1%  -24.08%  (p=0.008 n=5+5)
      
        name           old alloc/op   new alloc/op   delta
        QEncodeWord-4      152B ± 0%       80B ± 0%  -47.37%  (p=0.008 n=5+5)
      
        name           old allocs/op  new allocs/op  delta
        QEncodeWord-4      5.00 ± 0%      2.00 ± 0%  -60.00%  (p=0.008 n=5+5)
      
      Comparison with buffer solution (like before strings.Builder, but
      without sync pool for buffer re-using):
      
        name           old time/op    new time/op    delta
        QEncodeWord-4     595ns ± 1%     593ns ± 1%     ~     (p=0.460 n=5+5)
      
        name           old alloc/op   new alloc/op   delta
        QEncodeWord-4      160B ± 0%       80B ± 0%  -50.00%  (p=0.008 n=5+5)
      
        name           old allocs/op  new allocs/op  delta
        QEncodeWord-4      2.00 ± 0%      2.00 ± 0%     ~     (all equal)
      
      We avoid allocation in buf.String(), as expected.
      
      Fixes #25379
      
      Change-Id: I19763f0e593a27390c1a549b86ce6507b489046b
      Reviewed-on: https://go-review.googlesource.com/113235
      Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
      Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      3fb3ca08
    • Richard Musiol's avatar
      misc/wasm: fix passing large negative integers from JS to Go · 73b59513
      Richard Musiol authored
      This commit addresses a FIXME left in the code of wasm_exec.js to
      properly get the upper 32 bit of a JS number to be stored as an
      64-bit integer. A bitshift operation is not possible, because in
      JavaScript bitshift operations only operate on the lower 32 bits.
      
      Change-Id: I8f627fd604e592682d9d322942a4852db64a7f66
      Reviewed-on: https://go-review.googlesource.com/113076Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      73b59513
    • Richard Musiol's avatar
      misc/wasm: pollute global JS namespace less · db91ee36
      Richard Musiol authored
      This commit changes wasm_exec.js so it only puts the single
      name "go" into the global namespace. Other names became private
      or were turned into a property/method of "go".
      
      Change-Id: I633829dfd3c06936f092c0a14b9978bf855e41fe
      Reviewed-on: https://go-review.googlesource.com/112980Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: 's avatarAgniva De Sarker <agniva.quicksilver@gmail.com>
      db91ee36
  7. 14 May, 2018 13 commits
    • Elias Naur's avatar
      net: skip socket hungry test on iOS · 3027932a
      Elias Naur authored
      The iOS builder recently gained access to the GO_BUILDER_NAME
      environment variable, which in turn enabled some net tests that
      were previously guarded by testenv.Builder() == "". Some such tests
      have been disabled because they don't work; others have increased
      the pressure on open file descriptors, pushing the low iOS limit of
      250.
      
      Since many net tests run with t.Parallel(), the "too many open files"
      error hit many different tests, so instead of playing whack-a-mole,
      lower the file descriptor demand by skipping the most file
      descriptor hungry test, TestTCPSpuriousConnSetupCompletionWithCancel.
      
      Before:
      
      $ GO_BUILDER_NAME=darwin-arm64 GOARCH=arm64 go test -short -v net
      ...
      Socket statistical information:
      ...
      (inet4, stream, default): opened=5245 connected=193 listened=75 accepted=177 closed=5399 openfailed=0 connectfailed=5161 listenfailed=0 acceptfailed=143 closefailed=0
      ...
      
      After:
      
      $ GO_BUILDER_NAME=darwin-arm64 GOARCH=arm64 go test -short -v net
      ...
      Socket statistical information:
      ...
      (inet4, stream, default): opened=381 connected=194 listened=75 accepted=169 closed=547 openfailed=0 connectfailed=297 listenfailed=0 acceptfailed=134 closefailed=0
      ...
      
      Fixes #25365 (Hopefully).
      
      Change-Id: I8343de1b687ffb79001a846b1211df7aadd0535b
      Reviewed-on: https://go-review.googlesource.com/113095
      Run-TryBot: Elias Naur <elias.naur@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarHyang-Ah Hana Kim <hyangah@gmail.com>
      3027932a
    • David du Colombier's avatar
      cmd/compile: skip TestStmtLines on Plan 9 · 218650fa
      David du Colombier authored
      TestStmtLines has been added in CL 102435.
      This test is failing on Plan 9 because executables
      don't have a DWARF symbol table.
      
      Fixes #25387.
      
      Change-Id: I6ae7cba0e8ad4ab569a29ea8920b7849acfb9846
      Reviewed-on: https://go-review.googlesource.com/113115
      Run-TryBot: David du Colombier <0intro@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarDavid Chase <drchase@google.com>
      218650fa
    • David Chase's avatar
      cmd/compile: remove now-irrelevant test · eeff8fa4
      David Chase authored
      This test measures "line churn" which was minimized to help
      improve the debugger experience.  With proper is_stmt markers,
      this is no longer necessary, and it is more accurate (for
      profiling) to allow line numbers to vary willy-nilly.
      
      "Debugger experience" is now better measured by
      cmd/compile/internal/ssa/debug_test.go
      
      This CL made the obsoleting change:
      https://go-review.googlesource.com/c/go/+/102435
      
      Change-Id: I874ab89f3b243b905aaeba7836118f632225a667
      Reviewed-on: https://go-review.googlesource.com/113155
      Run-TryBot: David Chase <drchase@google.com>
      Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      eeff8fa4
    • Diogo Pinela's avatar
      testing: allow marking subtest and subbenchmark functions as Helpers · 15f2cbf4
      Diogo Pinela authored
      Since subtests and subbenchmarks run in a separate goroutine, and thus
      a separate stack, this entails capturing the stack trace at the point
      tb.Run is called. The work of getting the file and line information from
      this stack is only done when needed, however.
      
      Continuing the search into the parent test also requires temporarily
      holding its mutex. Since Run does not hold it while waiting for the
      subtest to complete, there should be no risk of a deadlock due to this.
      
      Fixes #24128
      
      Change-Id: If0bb169f3ac96bd48794624e619ade7edb599f83
      Reviewed-on: https://go-review.googlesource.com/108658
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarMarcel van Lohuizen <mpvl@golang.org>
      15f2cbf4
    • Alberto Donizetti's avatar
      testing: only compute b.N once when passed -count > 1 · 05ca3409
      Alberto Donizetti authored
      When running a benchmark multiple times, instead of re-computing the
      value of b.N each time, use the value found by the first run.
      
      For
      
        go test -bench=. -benchtime 3s -count 2 p_test.go
      
      on the benchmark in the linked issue; before:
      
        BenchmarkBenchmark-4   	     500	  10180593 ns/op
        --- BENCH: BenchmarkBenchmark-4
        	  p_test.go:13: single call took 10.111079ms
        	  p_test.go:13: single call took 1.017298685s
        	  p_test.go:13: single call took 5.090096124s
        BenchmarkBenchmark-4   	     500	  10182164 ns/op
        --- BENCH: BenchmarkBenchmark-4
        	  p_test.go:13: single call took 10.098169ms
        	  p_test.go:13: single call took 1.017712905s
        	  p_test.go:13: single call took 5.090898517s
        PASS
        ok  	command-line-arguments	12.244s
      
      and after:
      
        BenchmarkBenchmark-4   	     500	  10177076 ns/op
        --- BENCH: BenchmarkBenchmark-4
        	  p_test.go:13: single call took 10.091301ms
        	  p_test.go:13: single call took 1.016943125s
        	  p_test.go:13: single call took 5.088376028s
        BenchmarkBenchmark-4   	     500	  10171497 ns/op
        --- BENCH: BenchmarkBenchmark-4
        	  p_test.go:13: single call took 10.140245ms
        	  p_test.go:13: single call took 5.085605921s
        PASS
        ok  	command-line-arguments	11.218s
      
      Fixes #23423
      
      Change-Id: Ie66a8c5ac43881eb8741e14105db28745b4d56d3
      Reviewed-on: https://go-review.googlesource.com/110775Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
      05ca3409
    • Giovanni Bajo's avatar
      cmd/compile: reduce allocations in prove by reusing posets · 3c8545c5
      Giovanni Bajo authored
      In prove, reuse posets between different functions by storing them
      in the per-worker cache.
      
      Allocation count regression caused by prove improvements is down
      from 5% to 3% after this CL.
      
      Updates #25179
      
      Change-Id: I6d14003109833d9b3ef5165fdea00aa9c9e952e8
      Reviewed-on: https://go-review.googlesource.com/110455
      Run-TryBot: Giovanni Bajo <rasky@develer.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarDavid Chase <drchase@google.com>
      3c8545c5
    • Giovanni Bajo's avatar
      cmd/compile: improve undo of poset · 67656ba7
      Giovanni Bajo authored
      prove uses the poset datastructure in a DFS walk, and always undoes
      it back to its pristine status. Before this CL, poset's undo of
      a new node creation didn't fully deallocate the node, which means
      that at the end of prove there was still some allocated memory pending.
      
      This was not a problem until now because the posets used by prove
      were discarded after each function, but it would prevent recycling
      them between functions (as a followup CL does).
      
      Change-Id: I1c1c99c03fe19ad765395a43958cb256f686765a
      Reviewed-on: https://go-review.googlesource.com/112976
      Run-TryBot: Giovanni Bajo <rasky@develer.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarJosh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: 's avatarDavid Chase <drchase@google.com>
      67656ba7
    • David Chase's avatar
      cmd/compile: plumb prologueEnd into DWARF · 7bac2a95
      David Chase authored
      This marks the first instruction after the prologue for
      consumption by debuggers, specifically Delve, who asked
      for it.  gdb appears to ignore it, lldb appears to use it.
      
      The bits for end-of-prologue and beginning-of-epilogue
      are added to Pos (reducing maximum line number by 4x, to
      1048575).  They're added in cmd/internal/obj/<ARCH>.go
      (currently x86 only), so the compiler-proper need not
      deal with them.
      
      The linker currently does nothing with beginning-of-epilogue,
      but the plumbing exists to make it easier in the future.
      
      This also upgrades the line number table to DWARF version 3.
      
      This CL includes a regression in the coverage for
      testdata/i22558.gdb-dbg.nexts, this appears to be a gdb
      artifact but the fix would be in the preceding CL in the
      stack.
      
      Change-Id: I3bda5f46a0ed232d137ad48f65a14835c742c506
      Reviewed-on: https://go-review.googlesource.com/110416
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarAustin Clements <austin@google.com>
      7bac2a95
    • David Chase's avatar
      cmd/compile: assign and preserve statement boundaries. · c2c1822b
      David Chase authored
      A new pass run after ssa building (before any other
      optimization) identifies the "first" ssa node for each
      statement. Other "noise" nodes are tagged as being never
      appropriate for a statement boundary (e.g., VarKill, VarDef,
      Phi).
      
      Rewrite, deadcode, cse, and nilcheck are modified to move
      the statement boundaries forward whenever possible if a
      boundary-tagged ssa value is removed; never-boundary nodes
      are ignored in this search (some operations involving
      constants are also tagged as never-boundary and also ignored
      because they are likely to be moved or removed during
      optimization).
      
      Code generation treats all nodes except those explicitly
      marked as statement boundaries as "not statement" nodes,
      and floats statement boundaries to the beginning of each
      same-line run of instructions found within a basic block.
      
      Line number html conversion was modified to make statement
      boundary nodes a bit more obvious by prepending a "+".
      
      The code in fuse.go that glued together the value slices
      of two blocks produced a result that depended on the
      former capacities (not lengths) of the two slices.  This
      causes differences in the 386 bootstrap, and also can
      sometimes put values into an order that does a worse job
      of preserving statement boundaries when values are removed.
      
      Portions of two delve tests that had caught problems were
      incorporated into ssa/debug_test.go.  There are some
      opportunities to do better with optimized code, but the
      next-ing is not lying or overly jumpy.
      
      Over 4 CLs, compilebench geomean measured binary size
      increase of 3.5% and compile user time increase of 3.8%
      (this is after optimization to reuse a sparse map instead
      of creating multiple maps.)
      
      This CL worsens the optimized-debugging experience with
      Delve; we need to work with the delve team so that
      they can use the is_stmt marks that we're emitting now.
      
      The reference output changes from time to time depending
      on other changes in the compiler, sometimes better,
      sometimes worse.
      
      This CL now includes a test ensuring that 99+% of the lines
      in the Go command itself (a handy optimized binary) include
      is_stmt markers.
      
      Change-Id: I359c94e06843f1eb41f9da437bd614885aa9644a
      Reviewed-on: https://go-review.googlesource.com/102435
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarAustin Clements <austin@google.com>
      c2c1822b
    • Yury Smolsky's avatar
      cmd/go: fix TestBuildIDContainsArchModeEnv · c06f0275
      Yury Smolsky authored
      Changing GOARCH, GOARM, GO386 leads to a stale dependency.
      
      Updates #24436.
      
      Change-Id: I5b5b3fca6401be50fa81fb040bc56356de7555de
      Reviewed-on: https://go-review.googlesource.com/112975
      Run-TryBot: Yury Smolsky <yury@smolsky.by>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      c06f0275
    • Michael Munday's avatar
      sync: deflake TestWaitGroupMisuse2 · b7f3c178
      Michael Munday authored
      We need to yield to the runtime every now and again to avoid
      deadlock. This doesn't show up on most machines because the test
      only runs when you have 5 or more CPUs.
      
      Fixes #20072.
      
      Change-Id: Ibf5ed370e919943395f3418487188df0b2be160b
      Reviewed-on: https://go-review.googlesource.com/112978
      Run-TryBot: Michael Munday <mike.munday@ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      b7f3c178
    • Dmitri Shuralyov's avatar
      path/filepath: make Abs("") return working directory on Windows · 91f07c57
      Dmitri Shuralyov authored
      The current Abs docs say:
      
      	// If the path is not absolute it will be joined with the current
      	// working directory to turn it into an absolute path.
      
      The empty string is not an absolute path, so the docs suggest that the
      empty string should be joined with the current working directory to
      turn it into an absolute path. This was already the case on all
      platforms other than Windows. Per the decision in issue #24441,
      this change makes it work on Windows too.
      
      Since the empty string is not a valid path for the purposes of calling
      os.Stat on it, we can't simply add the empty string test case to
      absTests, which TestAbs uses. It would error when trying to do:
      
      	info, err := os.Stat(path)
      
      I didn't find a good way to modify TestAbs to handle this situation
      without significantly complicating its code and compromising the test.
      So, a separate test is created for testing Abs on empty string input.
      
      Fixes #24441.
      
      Change-Id: I11d8ae2f6e6e358f3e996372ee2a0449093898d2
      Reviewed-on: https://go-review.googlesource.com/112935Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      Reviewed-by: 's avatarAlex Brainman <alex.brainman@gmail.com>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      91f07c57
    • Rob Pike's avatar
      doc/contribute.html: clean up HTML and formatting · a023f3c8
      Rob Pike authored
      Mostly just formatting and minor cleanup:
      
      - regularize HTML (add </p> etc.)
      - remove all errors caught by tidy
      - start all sentences on new line for easy editing
      
      Some wording changes, but there will be more to come.
      It seemed there were already enough edits to send it out.
      
      Update #24487
      
      Change-Id: I613ce206b1e8e3e522ecb0bbcd2acb11c4ff5bae
      Reviewed-on: https://go-review.googlesource.com/113015Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
      a023f3c8