1. 08 May, 2017 2 commits
    • Marvin Stenger's avatar
      bytes: skip inline test by default · 8c49c06b
      Marvin Stenger authored
      The test "TestTryGrowByResliceInlined" introduced in c08ac367 broke the
      noopt builder as it fails when inlining is disabled.
      Since there are currently no other options at hand for checking
      inlined-ness other than looking at emited symbols of the compilation,
      we for now skip the problem causing test by default and only run
      it on one specific builder ("linux-amd64").
      Also see CL 42813, which introduced the test and contains comments
      suggesting this temporary solution.
      
      Change-Id: I3978ab0831da04876cf873d78959f821c459282b
      Reviewed-on: https://go-review.googlesource.com/42820Reviewed-by: default avatarJosh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      8c49c06b
    • Alex Brainman's avatar
      internal/poll: remove allocation in windows FD.Writev · ddcb975f
      Alex Brainman authored
      Use closure parameter instead of external variable to
      remove 1 allocation.
      
      I tried to add test, but it is difficult to add something simple
      and not flake here. I did test this with:
      
      diff --git a/src/net/writev_test.go b/src/net/writev_test.go
      index 4c05be4..e417d68 100644
      --- a/src/net/writev_test.go
      +++ b/src/net/writev_test.go
      @@ -99,6 +99,15 @@ func TestBuffers_WriteTo(t *testing.T) {
       	}
       }
      
      +func TestBuffers_WriteToAllocs(t *testing.T) {
      +	allocs := testing.AllocsPerRun(10, func() {
      +		testBuffer_writeTo(t, 10, false)
      +	})
      +	if allocs > 0 {
      +		t.Fatalf("got %v; want 0", allocs)
      +	}
      +}
      +
       func testBuffer_writeTo(t *testing.T, chunks int, useCopy bool) {
       	oldHook := poll.TestHookDidWritev
       	defer func() { poll.TestHookDidWritev = oldHook }()
      
      It makes allocation count go down by 1 after the fix.
      
      Before:
      
      C:\>u:\test -test.v -test.run=WriteToAllocs
      === RUN   TestBuffers_WriteToAllocs
      --- FAIL: TestBuffers_WriteToAllocs (0.05s)
              writev_test.go:107: got 66; want 0
      FAIL
      
      and after:
      
      C:\>u:\test -test.v -test.run=WriteToAllocs
      === RUN   TestBuffers_WriteToAllocs
      --- FAIL: TestBuffers_WriteToAllocs (0.04s)
              writev_test.go:107: got 65; want 0
      FAIL
      
      Thanks to @MichaelMonashev for report and the fix.
      
      Fixes #19222
      
      Change-Id: I0f73cd9e2c8bbaa0653083f81f3ccb83b5ea84e1
      Reviewed-on: https://go-review.googlesource.com/42893Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      ddcb975f
  2. 07 May, 2017 5 commits
    • Elias Naur's avatar
      cmd/link/internal/ld: don't link with -no_pie on darwin/arm64 · 45d42fdc
      Elias Naur authored
      Ever since CL 33301 linking darwin/arm64 excutables has resulted in
      warnings like:
      
      ld: warning: -no_pie ignored for arm64
      
      Remove -no_pie on darwin/arm64.
      
      Change-Id: I9f7685351fa8cce29795283e1a24fc7a6753d698
      Reviewed-on: https://go-review.googlesource.com/42815
      Run-TryBot: Elias Naur <elias.naur@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      45d42fdc
    • Kevin Burke's avatar
      os, cmd/link: fix typos · 9058b9ae
      Kevin Burke authored
      Also switch "stating" to "statting" to describe applying os.Stat to
      a resource; the former is more confusable than the latter.
      
      Change-Id: I9d8e3506bd383f8f1479c05948c03b8c633dc4af
      Reviewed-on: https://go-review.googlesource.com/42855Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      9058b9ae
    • Marvin Stenger's avatar
      bytes: optimize Buffer's Write, WriteString, WriteByte, and WriteRune · c08ac367
      Marvin Stenger authored
      In the common case, the grow method only needs to reslice the internal
      buffer. Making another function call to grow can be expensive when Write
      is called very often with small pieces of data (like a byte or rune).
      Thus, we add a tryGrowByReslice method that is inlineable so that we can
      avoid an extra call in most cases.
      
      name                       old time/op    new time/op    delta
      WriteByte-4                  35.5µs ± 0%    17.4µs ± 1%   -51.03%  (p=0.000 n=19+20)
      WriteRune-4                  55.7µs ± 1%    38.7µs ± 1%   -30.56%  (p=0.000 n=18+19)
      BufferNotEmptyWriteRead-4     304µs ± 5%     283µs ± 3%    -6.86%  (p=0.000 n=19+17)
      BufferFullSmallReads-4       87.0µs ± 5%    66.8µs ± 2%   -23.26%  (p=0.000 n=17+17)
      
      name                       old speed      new speed      delta
      WriteByte-4                 115MB/s ± 0%   235MB/s ± 1%  +104.19%  (p=0.000 n=19+20)
      WriteRune-4                 221MB/s ± 1%   318MB/s ± 1%   +44.01%  (p=0.000 n=18+19)
      
      Fixes #17857
      
      Change-Id: I08dfb10a1c7e001817729dbfcc951bda12fe8814
      Reviewed-on: https://go-review.googlesource.com/42813Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      c08ac367
    • Damien Lespiau's avatar
      cmd/asm: enable MOVSD in the encoding end-to-end test · 23c5db9b
      Damien Lespiau authored
      MOVSD is properly handled but its encoding test wasn't enabled. Enable
      it.
      
      For reference this was found with a little tool I wrote [1] to explore
      which instructions are missing or not tested in the go obj package and
      assembler:
      
      "which SSE2 instructions aren't tested? And don't list instructions
      which can take MMX operands"
      
      $ x86db-gogen list --extension SSE2 --not-tested --not-mmx
      CLFLUSH mem           [m:  np 0f ae /7] WILLAMETTE,SSE2
      MOVSD   xmmreg,xmmreg [rm: f2 0f 10 /r] WILLAMETTE,SSE2
      MOVSD   xmmreg,xmmreg [mr: f2 0f 11 /r] WILLAMETTE,SSE2
      MOVSD   mem64,xmmreg  [mr: f2 0f 11 /r] WILLAMETTE,SSE2
      MOVSD   xmmreg,mem64  [rm: f2 0f 10 /r] WILLAMETTE,SSE2
      
      (CLFLUSH was introduced with SSE2, but has its own CPUID bit)
      
      [1] https://github.com/dlespiau/x86db
      
      Change-Id: Ic3af3028cb8d4f02e53fdebb9b30fb311f4ee454
      Reviewed-on: https://go-review.googlesource.com/42814Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      23c5db9b
    • Alex Brainman's avatar
      os: reimplement windows os.Stat · 53003621
      Alex Brainman authored
      Currently windows Stat uses combination of Lstat and Readlink to
      walk symlinks until it reaches file or directory. Windows Readlink
      is implemented via Windows DeviceIoControl(FSCTL_GET_REPARSE_POINT, ...)
      call, but that call does not work on network shares or inside of
      Docker container (see issues #18555 ad #19922 for details).
      
      But Raymond Chen suggests different approach:
      https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/
      - he suggests to use Windows I/O manager to dereferences the
      symbolic link.
      
      This appears to work for all normal symlinks, but also for network
      shares and inside of Docker container.
      
      This CL implements described procedure.
      
      I also had to adjust TestStatSymlinkLoop, because the test is
      expecting Stat to return syscall.ELOOP for symlink with a loop.
      But new Stat returns Windows error of ERROR_CANT_RESOLVE_FILENAME
      = 1921 instead. I could map ERROR_CANT_RESOLVE_FILENAME into
      syscall.ELOOP, but I suspect the former is broader than later.
      And ERROR_CANT_RESOLVE_FILENAME message text of "The name of
      the file cannot be resolved by the system." sounds fine to me.
      
      Fixes #10935
      Fixes #18555
      Fixes #19922
      
      Change-Id: I979636064cdbdb9c7c840cf8ae73fe2c24499879
      Reviewed-on: https://go-review.googlesource.com/41834Reviewed-by: default avatarHarshavardhana <hrshvardhana@gmail.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      53003621
  3. 06 May, 2017 3 commits
  4. 05 May, 2017 8 commits
    • Robert Griesemer's avatar
      go/types: remove invalid documentation and assertion on package names · 2eeaba41
      Robert Griesemer authored
      NewPackage required through documentation that the package name not
      be blank (which wasn't true since each time we check a new package
      we create one with a blank name (api.go:350). NewPackage also asserted
      that a package name not be "_". While it is invalid for a package name
      to be "_", one could conceivably create a package named "_" through
      export data manipulation. Furthermore, it is ok to import a package
      with package path "_" as long as the package itself is not named "_".
      
      - removed misleading documentation
      - removed unnecessary assertion
      - added safety checks when we actually do the import
      
      Fixes #20231.
      
      Change-Id: I1eb1ab7b5e3130283db715374770cf05d749d159
      Reviewed-on: https://go-review.googlesource.com/42852
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAlan Donovan <adonovan@google.com>
      2eeaba41
    • Robert Griesemer's avatar
      go/importer: report import path if package is not found · 0e751829
      Robert Griesemer authored
      Fixes #20230.
      
      Change-Id: I2e9b9e9d2540eb66c8411ac7910962933bc2c0e9
      Reviewed-on: https://go-review.googlesource.com/42870
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJosh Bleecher Snyder <josharian@gmail.com>
      0e751829
    • Josh Bleecher Snyder's avatar
      cmd/compile: add Func.SetNilCheckDisabled · 53e62aba
      Josh Bleecher Snyder authored
      Generated hash and eq routines don't need nil checks.
      Prior to this CL, this was accomplished by
      temporarily incrementing the global variable disable_checknil.
      However, that increment lasted only the lifetime of the
      call to funccompile. After CL 41503, funccompile may
      do nothing but enqueue the function for compilation,
      resulting in nil checks being generated.
      
      Fix this by adding an explicit flag to a function
      indicating whether nil checks should be disabled
      for that function.
      
      While we're here, allow concurrent compilation
      with the -w and -W flags, since that was needed
      to investigate this issue.
      
      Fixes #20242
      
      Change-Id: Ib9140c22c49e9a09e62fa3cf350f5d3eff18e2bd
      Reviewed-on: https://go-review.googlesource.com/42591
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMarvin Stenger <marvin.stenger94@gmail.com>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      53e62aba
    • Carlos Eduardo Seo's avatar
      cmd/internal/obj/ppc64, cmd/link/internal/ppc64: Change function alignment to 16 · 09b71d56
      Carlos Eduardo Seo authored
      The Power processor manual states that "Branches not from the last instruction
      of an aligned quadword and not to the first instruction of an aligned quadword
      cause inefficiencies in the IBuffer". This changes the function alignment from 8
      to 16 bytes to comply with that.
      
      Fixes #18963
      
      Change-Id: Ibce9bf8302110a86c6ab05948569af9ffdfcf4bb
      Reviewed-on: https://go-review.googlesource.com/36390
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
      09b71d56
    • Samuel Tan's avatar
      html/template: allow safe usage of predefined escapers in pipelines · 3a2fee03
      Samuel Tan authored
      Allow the predefined escapers "html", "urlquery", and "js" to be used
      in pipelines when they have no potential to affect the correctness or
      safety of the escaped pipeline output. Specifically:
      - "urlquery" may be used if it is the last command in the pipeline.
      - "html" may be used if it is the last command in the pipeline, and
        the pipeline does not occur in an unquoted HTML attribute value
        context.
      - "js" may be used in any pipeline, since it does not affect the
        merging of contextual escapers.
      
      This change will loosens the restrictions on predefined escapers
      introduced in golang.org/cl/37880, which will hopefully ease the
      upgrade path for existing template users.
      
      This change brings back the escaper-merging logic, and associated
      unit tests, that were removed in golang.org/cl/37880. However, a
      few notable changes have been made:
      - "_html_template_nospaceescaper" is no longer considered
        equivalent to "html", since the former escapes spaces, while
        the latter does not (see #19345). This change should not silently
        break any templates, since pipelines where this substituion will
        happen will already trigger an explicit error.
      - An "_eval_args_" internal directive has been added to
        handle pipelines containing a single explicit call to a
        predefined escaper, e.g. {{html .X}} (see #19353).
      
      Also, the HTMLEscape function called by the predefined
      text/template "html" function now escapes the NULL character as
      well. This effectively makes it as secure as the internal
      html/template HTML escapers (see #19345). While this change is
      backward-incompatible, it will only affect illegitimate uses
      of this escaper, since the NULL character is always illegal in
      valid HTML.
      
      Fixes #19952
      
      Change-Id: I9b5570a80a3ea284b53901e6a1f842fc59b33d3a
      Reviewed-on: https://go-review.googlesource.com/40936Reviewed-by: default avatarRuss Cox <rsc@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      3a2fee03
    • Dieter Plaetinck's avatar
      template: warn about interleaved nature of writes · 1acff5fe
      Dieter Plaetinck authored
      Execute incurs separate writes for each "step", e.g. each
      variable that needs to be printed, and the final newline.
      While it is correct to state that templates can be executed
      concurrently, there is a more subtle nuance that is easily missed:
      when writing to the same writer, the writes from concurrent execute
      calls can be interleaved, leading to unexpected output.
      
      Change-Id: I0abbd7960d8a8d15e109a8a3eeff3b43b852bbbf
      Reviewed-on: https://go-review.googlesource.com/37444Reviewed-by: default avatarRob Pike <r@golang.org>
      1acff5fe
    • David Crawshaw's avatar
      cmd/link: stop passing unused read_only_relocs · 27a10f7d
      David Crawshaw authored
      The external darwin linker has been printing:
      
      	ld: warning: -read_only_relocs cannot be used with x86_64
      
      for a long time. Now that it is printed by CL 33301, we may as
      well get rid of it.
      
      Fixes #20246
      
      Change-Id: I1147cf1ff197fdfda228a1349f13627bcf9fc72f
      Reviewed-on: https://go-review.googlesource.com/42730
      Run-TryBot: David Crawshaw <crawshaw@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTodd Neal <todd@tneal.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      27a10f7d
    • Alex Brainman's avatar
      cmd/link: do not read .bss sections in ldpe · 507f4d5f
      Alex Brainman authored
      For .bss section symbol ldelf does not set P (raw symbol data).
      Make ldpe do the same.
      
      Change-Id: Ib3d558456f505ee568d0972465fa9b08b5794a87
      Reviewed-on: https://go-review.googlesource.com/42631Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      Run-TryBot: Ian Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      507f4d5f
  5. 04 May, 2017 10 commits
  6. 03 May, 2017 8 commits
  7. 02 May, 2017 4 commits