1
0
mirror of https://github.com/golang/go synced 2024-11-24 07:30:10 -07:00
Commit Graph

34294 Commits

Author SHA1 Message Date
Filippo Valsorda
f75158c365 math/big: fix ModSqrt optimized path for x = z
name                   old time/op  new time/op  delta
ModSqrt224_3Mod4-4      153µs ± 2%   154µs ± 1%   ~     (p=0.548 n=5+5)
ModSqrt5430_3Mod4-4     776ms ± 2%   791ms ± 2%   ~     (p=0.222 n=5+5)

Fixes #22265

Change-Id: If233542716e04341990a45a1c2b7118da6d233f7
Reviewed-on: https://go-review.googlesource.com/70832
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2017-10-16 21:41:44 +00:00
Alessandro Arzilli
913fb18e7e runtime/cgo: declare crosscall2 frame using TEXT for amd64 and 386
Use TEXT pseudo-instruction to adjust SP instead of a SUB instruction
so that the assembler knows how to fill in the pcsp table and the frame
description entry correctly.

Updates #21569

Change-Id: I436c840b2af99bbb3042ecd38a7d7c1ab4d7372a
Reviewed-on: https://go-review.googlesource.com/70937
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 21:17:25 +00:00
Ian Lance Taylor
b79e99bfb4 runtime: remove commented out code from ARM Linux boot
The code was commented out by https://golang.org/cl/13234050 in 2013.
Let's just remove it.

Change-Id: I46ae1f07386719e991458e782d236214c40bdce1
Reviewed-on: https://go-review.googlesource.com/70770
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2017-10-16 21:12:48 +00:00
Daniel Martí
bb45bc27b5 cmd/compile: make more use of value switches
Use them to replace if/else chains with at least three comparisons,
where the code becomes clearly simpler.

Passes toolstash -cmp on std cmd.

Change-Id: Ic98aa3905944ddcab5aef5f9d9ba376853263d94
Reviewed-on: https://go-review.googlesource.com/70934
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 19:59:24 +00:00
Cherry Zhang
e0111bb0f4 cmd/compile: remove needwritebarrier from the frontend
The write barrier insertion has moved to the SSA backend's
writebarrier pass. There is still needwritebarrier function
left in the frontend. This function is used in two places:

- fncall, which is called in ascompatet, which is called in
  walking OAS2FUNC. For OAS2FUNC, in order pass we've already
  created temporaries, and there is no write barrier for the
  assignments of these temporaries.

- updateHasCall, which updates the HasCall flag of a node. the
  HasCall flag is then used in
  - fncall, mentioned above.
  - ascompatet. As mentioned above, this is an assignment to
    a temporary, no write barrier.
  - reorder1, which is always called with a list produced by
    ascompatte, which is a list of assignments to stack, which
    have no write barrier.
  - vmatch1, which is called in oaslit with r.Op as OSTRUCTLIT,
    OARRAYLIT, OSLICELIT, or OMAPLIT. There is no write barrier
    in those literals.

Therefore, the needwritebarrier function is unnecessary. This
CL removes it.

Passes "toolstash -cmp" on std cmd.

Updates #17583.

Change-Id: I4b87ba8363d6583e4282a9e607a9ec8ce3ab124a
Reviewed-on: https://go-review.googlesource.com/43640
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 18:42:18 +00:00
Cherry Zhang
290de1f880 cmd/asm: reject STREX with same source and destination register on ARM
On ARM, STREX does not permit the same register used as both the
source and the destination. Reject the bad instruction.

The assembler also accepted special cases
	STREX R0, (R1)	as STREX R0, (R1), R0
	STREX (R1), R0	as STREX R0, (R1), R0
both are illegal. Remove this special case as well.

For STREXD, check that the destination is not source, and not
source+1. Also check that the source register is even numbered,
as required by the architecture's manual.

Fixes #22268.

Change-Id: I6bfde86ae692d8f1d35bd0bd7aac0f8a11ce8e22
Reviewed-on: https://go-review.googlesource.com/71190
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 18:30:56 +00:00
Matthew Dempsky
fc5841af9e cmd/compile: remove unnecessary Xoffset assignment
In golang.org/cl/61130, I removed the need for setting Xoffset on
OXCASE Nodes, but missed this assignment.

Passes toolstash-check.

Change-Id: I90ab05add14981b89ee18e73e1cdf2f13e9f9934
Reviewed-on: https://go-review.googlesource.com/66934
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
2017-10-16 18:15:37 +00:00
Tom Bergan
47f4e7a976 net/http: preserve Host header following a relative redirect
If the client sends a request with a custom Host header and receives
a relative redirect in response, the second request should use the
same Host header as the first request. However, if the response is
an abolute redirect, the Host header should not be preserved. See
further discussion on the issue tracker.

Fixes #22233

Change-Id: I8796e2fbc1c89b3445e651f739d5d0c82e727c14
Reviewed-on: https://go-review.googlesource.com/70792
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-16 17:44:26 +00:00
griesemer
645c661a54 cmd/compile/internal/syntax: factor out list parsing
Instead of repeating the same list parsing pattern for parenthesized
of braced comma or semicolon-separated lists, introduce a single list
parsing function that can be parametrized and which takes a closure
to parse list elements.

This ensures the same error handling and recovery logic is used across
all lists and simplifies the code.

No semantic change.

Change-Id: Ia738d354d6c2e0c3d84a5f1c7269a6eb95685edc
Reviewed-on: https://go-review.googlesource.com/70492
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 17:20:25 +00:00
griesemer
f8f0d6c4de cmd/compile/internal/syntax: match argument and parameter parsing (cleanup)
No semantic change. Move functionality not related to argument
out of the argument parsing function, and thus match parameter
parsing. Also, use a better function name.

Change-Id: Ic550875251d64e6fe1ebf91c11d33a9e4aec9fdd
Reviewed-on: https://go-review.googlesource.com/70491
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 17:18:32 +00:00
griesemer
4b7325c7e3 cmd/compile/internal/syntax: cleanups around parser tracing
These changes affect the parser only when the internal trace
constant is set.

- factored our printing code used for tracing
- streamlined advance function and added trace output

The parser's trace output now more clearly prints what tokens
are skipped and which is the next token in case of an error.

Example trace:

    4: . . . . . . . . . . call (
    4: . . . . . . . . . . . expr (
    4: . . . . . . . . . . . . unaryExpr (
    4: . . . . . . . . . . . . . pexpr (
    4: . . . . . . . . . . . . . . operand name (
    4: . . . . . . . . . . . . . . )
    4: . . . . . . . . . . . . . . call (
    4: . . . . . . . . . . . . . . )
    4: . . . . . . . . . . . . . )
    4: . . . . . . . . . . . . )
    4: . . . . . . . . . . . )
    4: . . . . . . . . . . . syntax error: expecting comma or )
    4: . . . . . . . . . . . skip ;
    6: . . . . . . . . . . . skip name
    6: . . . . . . . . . . . skip :=
    6: . . . . . . . . . . . skip literal
    6: . . . . . . . . . . . skip ;
    7: . . . . . . . . . . . skip }
    7: . . . . . . . . . . . skip ;
    9: . . . . . . . . . . . skip func
    9: . . . . . . . . . . . skip name
    9: . . . . . . . . . . . skip (
    9: . . . . . . . . . . . next )
    9: . . . . . . . . . . )

For #22164.

Change-Id: I4a233696b1f989ee3287472172afaf92cf424565
Reviewed-on: https://go-review.googlesource.com/70490
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-16 17:18:08 +00:00
Ian Lance Taylor
5ddd3d588c runtime: fix use of STREX in various exitThread implementations
STREX does not permit using the same register for the value to store
and the place where the result is returned. Also the code was wrong
anyhow if the first store failed.

Fixes #22248

Change-Id: I96013497410058514ffcb771c76c86faa1ec559b
Reviewed-on: https://go-review.googlesource.com/70911
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2017-10-16 17:15:39 +00:00
Kunpei Sakai
001fe1d57a net/http/httputil: extract duplicate code as removeConnectionHeaders
Change-Id: I50389752dcbf5d058ce11256a414be7955cdb77f
Reviewed-on: https://go-review.googlesource.com/71070
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
2017-10-16 17:09:55 +00:00
soluchok
eb695819a5 net/http: fix panic when status without description for proxied HTTPS responses
Check to ensure that Status is set
when parsing a proxied HTTPS response
that a CONNECT proxy-authorization.

Fixes #21701

Change-Id: Id91700b83425420101e0b0d46e12aaf5d20fd3a3
Reviewed-on: https://go-review.googlesource.com/59990
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Tom Bergan <tombergan@google.com>
2017-10-16 16:46:20 +00:00
Michael Hudson-Doyle
302f0d1646 cmd/link: replace SCONTAINER with an attribute bit
This is much easier than replacing SSUB so split it out from my other CL.

Change-Id: If01e4005da5355895404456320a2156bde4ec09a
Reviewed-on: https://go-review.googlesource.com/71050
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 06:40:37 +00:00
Michael Hudson-Doyle
1341104ae2 cmd/link: replace SHIDDEN bit in SymKind with a bit of Attribute
This is https://go-review.googlesource.com/42025 but with some more fixes --
hidden symbols implicitly passed "Type == 0 || Type == SXREF" checks. (This
sort of thing is part of why I wanted to make this change)

Change-Id: I2273ee98570fd7f2dd8a799c692a2083c014235e
Reviewed-on: https://go-review.googlesource.com/42330
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-16 06:40:13 +00:00
Javier Segura
7128ed0501 bytes: add examples of Equal and IndexByte
Change-Id: Ibf3179d0903eb443c89b6d886802c36f8d199898
Reviewed-on: https://go-review.googlesource.com/70933
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-16 03:34:28 +00:00
Daniel Martí
270a789c52 cmd/compile: simplify some declarations
Reduce the scope of some. Also remove vars that were simply the index or
the value in a range statement. While at it, remove a var that was
exactly the length of a slice.

Also replaced 'bad' with a more clear 'errored' of type bool, and
renamed a single-char name with a comment to a name that is
self-explanatory.

And removed a few unnecessary Index calls within loops.

Passes toolstash -cmp on std cmd.

Change-Id: I26eee5f04e8f7e5418e43e25dca34f89cca5c80a
Reviewed-on: https://go-review.googlesource.com/70930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2017-10-15 19:24:40 +00:00
Rob Pike
e5b7335705 fmt: clarify wording of * flag
The complainant is confused by the ambiguity of 'next' in the
phrase 'next operand'. It seems clear enough to me that things
are always read left to right when formatting, but to calm the
waters we add a clarifying parenthetical.

Fixes #22275

Change-Id: I82418c1e987db736f4bee0faa53fe715c9cde8f5
Reviewed-on: https://go-review.googlesource.com/71010
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2017-10-15 06:03:34 +00:00
Andreas Auernhammer
fb46b9ea20 crypto/elliptic: don't unmarshal invalid encoded points
ANSI X9.62 specifies that Unmarshal should fail if the a given coordinate is
not smaller than the prime of the elliptic curve. This change makes Unmarshal
ANSI X9.62 compliant and explicitly documents that the Marshal/Unmarshal only
supports uncompressed points.

Fixes #20482

Change-Id: I161a73da8279cae505c9ba0b3022021709fe8145
Reviewed-on: https://go-review.googlesource.com/44312
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-15 02:24:19 +00:00
Jed Denlea
8b220d8ef1 image/gif: write fewer, bigger blocks
The indexed bitmap of a frame is encoded into a GIF by first LZW
compression, and then packaged by a simple block mechanism.  Each block
of up-to-256 bytes starts with one byte, which indicates the size of the
block (0x01-0xff). The sequence of blocks is terminated by a 0x00.

While the format supports it, there is no good reason why any particular
image should be anything but a sequence of 255-byte blocks with one last
block less than 255-bytes.

The old blockWriter implementation would not buffer between Write()s,
meaning if the lzw Writer needs to flush more than one chunk of data via
a Write, multiple short blocks might exist in the middle of a stream.

Separate but related, the old implementation also forces lzw.NewWriter
to allocate a bufio.Writer because the blockWriter is not an
io.ByteWriter itself.  But, even though it doesn't effectively buffer
data between Writes, it does make extra copies of sub-blocks during the
course of writing them to the GIF's writer.

Now, the blockWriter shall continue to use the encoder's [256]byte buf,
but use it to effectively buffer a series of WriteByte calls from the
lzw Writer.  Once a WriteByte fills the buffer, the staged block is
Write()n to the underlying GIF writer.  After the lzw Writer is Closed,
the blockWriter should also be closed, which will flush any remaining
block along with the block terminator.

BenchmarkEncode indicates slight improvements:

name      old time/op    new time/op    delta
Encode-8    7.71ms ± 0%    7.38ms ± 0%   -4.27%  (p=0.008 n=5+5)

name      old speed      new speed      delta
Encode-8   159MB/s ± 0%   167MB/s ± 0%   +4.46%  (p=0.008 n=5+5)

name      old alloc/op   new alloc/op   delta
Encode-8    84.1kB ± 0%    80.0kB ± 0%   -4.94%  (p=0.008 n=5+5)

name      old allocs/op  new allocs/op  delta
Encode-8      9.00 ± 0%      7.00 ± 0%  -22.22%  (p=0.008 n=5+5)

Change-Id: I9eb9367d41d7c3d4d7f0adc9b720fc24fb50006a
Reviewed-on: https://go-review.googlesource.com/68351
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2017-10-14 05:51:20 +00:00
Matthew Dempsky
f3d4ff7ddc cmd/compile: omit ICE diagnostics after normal error messages
After we detect errors, the AST is in a precarious state and more
likely to trip useless ICE failures. Instead let the user fix any
existing errors and see if the ICE persists.  This makes Fatalf more
consistent with how panics are handled by hidePanic.

While here, also fix detection for release versions: release version
strings begin with "go" ("go1.8", "go1.9.1", etc), not "release".

Fixes #22252.

Change-Id: I1c400af62fb49dd979b96e1bf0fb295a81c8b336
Reviewed-on: https://go-review.googlesource.com/70850
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-14 01:00:31 +00:00
Cherry Zhang
e01eac371a cmd/compile: mark LoweredGetCallerPC rematerializeable
The caller's PC is always available in the frame. We can just
load it when needed, no need to spill.

Change-Id: I9c0a525903e574bb4eec9fe53cbeb8c64321166a
Reviewed-on: https://go-review.googlesource.com/70710
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2017-10-14 00:53:20 +00:00
Peter Wu
d1bbdbe760 crypto/tls: replace signatureAndHash by SignatureScheme.
Consolidate the signature and hash fields (SignatureAndHashAlgorithm in
TLS 1.2) into a single uint16 (SignatureScheme in TLS 1.3 draft 21).
This makes it easier to add RSASSA-PSS for TLS 1.2 in the future.

Fields were named like "signatureAlgorithm" rather than
"signatureScheme" since that name is also used throughout the 1.3 draft.

The only new public symbol is ECDSAWithSHA1, other than that this is an
internal change with no new functionality.

Change-Id: Iba63d262ab1af895420583ac9e302d9705a7e0f0
Reviewed-on: https://go-review.googlesource.com/62210
Reviewed-by: Adam Langley <agl@golang.org>
2017-10-13 23:25:03 +00:00
David Crawshaw
c996d07fee cmd/link: use the correct module data on ppc64le
Fixes #22250

Change-Id: I0e39d10ff6f0785cd22b0105de2d839e569db4b7
Reviewed-on: https://go-review.googlesource.com/70810
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-13 22:46:25 +00:00
Austin Clements
e09dbaa1de runtime: schedule fractional workers on all Ps
Currently only a single P can run a fractional mark worker at a time.
This doesn't let us spread out the load, so it gets concentrated on
whatever unlucky P picks up the token to run a fractional worker. This
can significantly delay goroutines on that P.

This commit changes this scheduling rule so each P separately
schedules fractional workers. This can significantly reduce the load
on any individual P and allows workers to self-preempt earlier. It
does have the downside that it's possible for all Ps to be in
fractional workers simultaneously (an effect STW).

Updates #21698.

Change-Id: Ia1e300c422043fa62bb4e3dd23c6232d81e4419c
Reviewed-on: https://go-review.googlesource.com/68574
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
2017-10-13 20:53:22 +00:00
Austin Clements
28e1a8e47a runtime: preempt fractional worker after reaching utilization goal
Currently fractional workers run until preempted by the scheduler,
which means they typically run for 20ms. During this time, all other
goroutines on that P are blocked, which can introduce significant
latency variance.

This modifies fractional workers to self-preempt shortly after
achieving the fractional utilization goal. In practice this means they
preempt much sooner, and the scale of their preemption is on the order
of how often the user goroutine block (so, if the application is
compute-bound, the fractional workers will also run for long times,
but if the application blocks frequently, the fractional workers will
also preempt quickly).

Fixes #21698.
Updates #18534.

Change-Id: I03a5ab195dae93154a46c32083c4bb52415d2017
Reviewed-on: https://go-review.googlesource.com/68573
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
2017-10-13 20:53:13 +00:00
Austin Clements
b783930e63 runtime: simplify fractional mark worker scheduler
We haven't used non-zero gcForcePreemptNS for ages. Remove it and
declutter the code.

Change-Id: Id5cc62f526d21ca394d2b6ca17d34a72959535da
Reviewed-on: https://go-review.googlesource.com/68572
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
2017-10-13 20:53:03 +00:00
Austin Clements
315c28b788 runtime: use only dedicated mark workers at reasonable GOMAXPROCS
When GOMAXPROCS is not small, fractional workers don't add much to
throughput, but they do add to the latency of individual goroutines.
In this case, it makes sense to just use dedicated workers, even if we
can't exactly hit the 25% CPU goal with dedicated workers.

This implements this logic by computing the number of dedicated mark
workers that will us closest to the 25% target. We only fall back to
fractional workers if that would be more than 30% off of the target
(less than 17.5% or more than 32.5%, which in practice happens for
GOMAXPROCS <= 3 and GOMAXPROCS == 6).

Updates #21698.

Change-Id: I484063adeeaa1190200e4ef210193a20e635d552
Reviewed-on: https://go-review.googlesource.com/68571
Reviewed-by: Rick Hudson <rlh@golang.org>
2017-10-13 20:52:55 +00:00
Austin Clements
27923482fa runtime: separate GC background utilization from goal utilization
Currently these are the same constant, but are separate concepts.
Split them into two constants for easier experimentation and better
documentation.

Change-Id: I121854d4fd1a4a827f727c8e5153160c24aacda7
Reviewed-on: https://go-review.googlesource.com/68570
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
2017-10-13 20:52:45 +00:00
Adam Langley
504a305c62 crypto/x509: reformat test struct.
https://golang.org/cl/67270 wasn't `go fmt`ed correctly, according to
the current `go fmt`. However, what `go fmt` did looked odd, so this
change tweaks the test to use a more standard layout.

Whitespace-only; no semantic change.

Change-Id: Id820352e7c9e68189ee485c8a9bfece75ca4f9cb
Reviewed-on: https://go-review.googlesource.com/69031
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@golang.org>
2017-10-13 18:29:40 +00:00
Ben Schwartz
f5cd3868d5 net/http: HTTPS proxies support
net/http already supports http proxies. This CL allows it to establish
a connection to the http proxy over https. See more at:
https://www.chromium.org/developers/design-documents/secure-web-proxy

Fixes golang/go#11332

Change-Id: If0e017df0e8f8c2c499a2ddcbbeb625c8fa2bb6b
Reviewed-on: https://go-review.googlesource.com/68550
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
2017-10-13 18:20:45 +00:00
Daniel Theophanes
897080d5cb database/sql: prevent race in driver by locking dc in Next
Database drivers should be called from a single goroutine to ease
driver's design. If a driver chooses to handle context
cancels internally it may do so.

The sql package violated this agreement when calling Next or
NextResultSet. It was possible for a concurrent rollback
triggered from a context cancel to call a Tx.Rollback (which
takes a driver connection lock) while a Rows.Next is in progress
(which does not tack the driver connection lock).

The current internal design of the sql package is each call takes
roughly two locks: a closemu lock which prevents an disposing of
internal resources (assigning nil or removing from lists)
and a driver connection lock that prevents calling driver code from
multiple goroutines.

Fixes #21117

Change-Id: Ie340dc752a503089c27f57ffd43e191534829360
Reviewed-on: https://go-review.googlesource.com/65731
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 18:11:41 +00:00
David Crawshaw
350b74bc4b cmd/link: zero symtab fields correctly
CL 69370 introduced a hasmain field to moduledata after the
modulehashes slice. However that code was relying on the zeroing
code after it to cover modulehashes if len(Shlibs) == 0. The
hasmain field gets in the way of that. So clear modulehashes
explicitly in that case.

Found when looking at #22250. Not sure if it's related.

Change-Id: I81050cb4554cd49e9f245d261ef422f97d026df4
Reviewed-on: https://go-review.googlesource.com/70730
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 17:48:54 +00:00
Daniel Martí
0e4de78d13 net: fix data race in TestClosingListener
In https://golang.org/cl/66334, the test was changed so that the second
Listen would also be closed. However, it shouldn't have reused the same
ln variable, as that can lead to a data race with the background loop
that accepts connections.

Simply define a new Listener, since we don't need to overwrite the first
variable.

I was able to reproduce the data race report locally about 10% of the
time by reducing the sleep from a millisecond to a nanosecond. After the
fix, it's entirely gone after 1000 runs.

Fixes #22226.

Change-Id: I7c639f9f2ee5098eac951a45f42f97758654eacd
Reviewed-on: https://go-review.googlesource.com/70230
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 15:19:59 +00:00
Martin Möhrmann
743117a85e cmd/compile: simplify slice/array range loops for some element sizes
In range loops over slices and arrays besides a variable to track the
index an extra variable containing the address of the current element
is used. To compute a pointer to the next element the elements size is
added to the address.

On 386 and amd64 an element of size 1, 2, 4 or 8 bytes can by copied
from an array using a MOV instruction with suitable addressing mode
that uses the start address of the array, the index of the element and
element size as scaling factor. Thereby, for arrays and slices with
suitable element size we can avoid keeping and incrementing an extra
variable to compute the next elements address.

Shrinks cmd/go by 4 kilobytes.

AMD64:
name                   old time/op    new time/op    delta
BinaryTree17              2.66s ± 7%     2.54s ± 0%  -4.53%  (p=0.000 n=10+8)
Fannkuch11                3.02s ± 1%     3.02s ± 1%    ~     (p=0.579 n=10+10)
FmtFprintfEmpty          45.6ns ± 1%    42.2ns ± 1%  -7.46%  (p=0.000 n=10+10)
FmtFprintfString         69.8ns ± 1%    70.4ns ± 1%  +0.84%  (p=0.041 n=10+10)
FmtFprintfInt            80.1ns ± 1%    79.0ns ± 1%  -1.35%  (p=0.000 n=10+10)
FmtFprintfIntInt          127ns ± 1%     125ns ± 1%  -1.00%  (p=0.007 n=10+9)
FmtFprintfPrefixedInt     158ns ± 2%     152ns ± 1%  -4.11%  (p=0.000 n=10+10)
FmtFprintfFloat           218ns ± 1%     214ns ± 1%  -1.61%  (p=0.000 n=10+10)
FmtManyArgs               508ns ± 1%     504ns ± 1%  -0.93%  (p=0.001 n=9+10)
GobDecode                6.76ms ± 1%    6.78ms ± 1%    ~     (p=0.353 n=10+10)
GobEncode                5.84ms ± 1%    5.77ms ± 1%  -1.31%  (p=0.000 n=10+9)
Gzip                      223ms ± 1%     218ms ± 1%  -2.39%  (p=0.000 n=10+10)
Gunzip                   40.3ms ± 1%    40.4ms ± 3%    ~     (p=0.796 n=10+10)
HTTPClientServer         73.5µs ± 0%    73.3µs ± 0%  -0.28%  (p=0.000 n=10+9)
JSONEncode               12.7ms ± 1%    12.6ms ± 8%    ~     (p=0.173 n=8+10)
JSONDecode               57.5ms ± 1%    56.1ms ± 2%  -2.40%  (p=0.000 n=10+10)
Mandelbrot200            3.80ms ± 1%    3.86ms ± 6%    ~     (p=0.579 n=10+10)
GoParse                  3.25ms ± 1%    3.23ms ± 1%    ~     (p=0.052 n=10+10)
RegexpMatchEasy0_32      74.4ns ± 1%    76.9ns ± 1%  +3.39%  (p=0.000 n=10+10)
RegexpMatchEasy0_1K       243ns ± 2%     248ns ± 1%  +1.86%  (p=0.000 n=10+8)
RegexpMatchEasy1_32      71.0ns ± 2%    72.8ns ± 1%  +2.55%  (p=0.000 n=10+10)
RegexpMatchEasy1_1K       370ns ± 1%     383ns ± 0%  +3.39%  (p=0.000 n=10+9)
RegexpMatchMedium_32      107ns ± 0%     113ns ± 1%  +5.33%  (p=0.000 n=6+10)
RegexpMatchMedium_1K     35.0µs ± 1%    36.0µs ± 1%  +3.13%  (p=0.000 n=10+10)
RegexpMatchHard_32       1.65µs ± 1%    1.69µs ± 1%  +2.23%  (p=0.000 n=10+9)
RegexpMatchHard_1K       49.8µs ± 1%    50.6µs ± 1%  +1.59%  (p=0.000 n=10+10)
Revcomp                   398ms ± 1%     396ms ± 1%  -0.51%  (p=0.043 n=10+10)
Template                 63.4ms ± 1%    60.8ms ± 0%  -4.11%  (p=0.000 n=10+9)
TimeParse                 318ns ± 1%     322ns ± 1%  +1.10%  (p=0.005 n=10+10)
TimeFormat                323ns ± 1%     336ns ± 1%  +4.15%  (p=0.000 n=10+10)

Updates: #15809.

Change-Id: I55915aaf6d26768e12247f8a8edf14e7630726d1
Reviewed-on: https://go-review.googlesource.com/38061
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2017-10-13 14:52:04 +00:00
Frank Somers
af40cbe83c runtime: use vDSO on linux/386 to improve time.Now performance
This change adds support for accelerating time.Now by using
the __vdso_clock_gettime fast-path via the vDSO on linux/386
if it is available.

When the vDSO path to the clocks is available, it is typically
5x-10x faster than the syscall path (see benchmark extract
below).  Two such calls are made for each time.Now() call
on most platforms as of go 1.9.

- Add vdso_linux_386.go, containing the ELF32 definitions
  for use by vdso_linux.go, the maximum array size, and
  the symbols to be located in the vDSO.

- Modify runtime.walltime and runtime.nanotime to check for
  and use the vDSO fast-path if available, or fall back to
  the existing syscall path.

- Reduce the stack reservations for runtime.walltime and
  runtime.monotime from 32 to 16 bytes. It appears the syscall
  path actually only needed 8 bytes, but 16 is now needed to
  cover the syscall and vDSO paths.

- Remove clearing DX from the syscall paths as clock_gettime
  only takes 2 args (BX, CX in syscall calling convention),
  so there should be no need to clear DX.

The included BenchmarkTimeNow was run with -cpu=1 -count=20
on an "Intel(R) Celeron(R) CPU J1900 @ 1.99GHz", comparing
released go 1.9.1 vs this change. This shows a gain in
performance on linux/386 (6.89x), and that no regression
occurred on linux/amd64 due to this change.

Kernel: linux/i686, GOOS=linux GOARCH=386
   name      old time/op  new time/op  delta
   TimeNow   978ns ± 0%   142ns ± 0%  -85.48%  (p=0.000 n=16+20)

Kernel: linux/x86_64, GOOS=linux GOARCH=amd64
   name      old time/op  new time/op  delta
   TimeNow   125ns ± 0%   125ns ± 0%   ~       (all equal)

Gains are more dramatic in virtualized environments,
presumably due to the overhead of virtualizing the syscall.

Fixes #22190

Change-Id: I2f83ce60cb1b8b310c9ced0706bb463c1b3aedf8
Reviewed-on: https://go-review.googlesource.com/69390
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 14:41:04 +00:00
Tobias Klauser
bf237f534c syscall: correct type for timeout argument to Select on linux/{arm64,mips64x}
syscall.Select uses SYS_PSELECT6 on arm64 and mipx64x, however this
syscall expects its 5th argument to be of type Timespec (with seconds
and nanoseconds) instead of type Timeval (with seconds and microseconds)
This leads to the timeout being too short by a factor of 1000.

This CL fixes this by adjusting the timeout argument accordingly,
similarly to how glibc does it for architectures where neither
SYS_SELECT nor SYS__NEWSELECT are available.

Fixes #22246

Change-Id: I33a183b0b87c2dae4a77a2d00f8615169fad48dd
Reviewed-on: https://go-review.googlesource.com/70590
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 14:01:17 +00:00
Wei Xiao
531e6c06c4 cmd/asm: refine Go assembly for ARM64
Some ARM64-specific instructions (such as SIMD instructions) are not supported.
This patch adds support for the following:
1. Extended register, e.g.:
     ADD	Rm.<ext>[<<amount], Rn, Rd
     <ext> can have the following values:
       UXTB, UXTH, UXTW, UXTX, SXTB, SXTH, SXTW and SXTX
2. Arrangement for SIMD instructions, e.g.:
     VADDP	Vm.<T>, Vn.<T>, Vd.<T>
     <T> can have the following values:
       B8, B16, H4, H8, S2, S4 and D2
3. Width specifier and element index for SIMD instructions, e.g.:
     VMOV	Vn.<T>[index], Rd // MOV(to general register)
     <T> can have the following values:
       S and D
4. Register List, e.g.:
     VLD1	(Rn), [Vt1.<T>, Vt2.<T>, Vt3.<T>]
5. Register offset variant, e.g.:
     VLD1.P	(Rn)(Rm), [Vt1.<T>, Vt2.<T>] // Rm is the post-index register
6. Go assembly for ARM64 reference manual
     new added instructions are required to have according explanation items in
     the manual and items for existed instructions will be added incrementally

For more information about the refinement background, please refer to the
discussion (https://groups.google.com/forum/#!topic/golang-dev/rWgDxCrL4GU)

This patch only adds syntax and doesn't break any assembly that already exists.

Change-Id: I34e90b7faae032820593a0e417022c354a882008
Reviewed-on: https://go-review.googlesource.com/41654
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-13 13:41:19 +00:00
Jed Denlea
31cd20a70e image/gif: try harder to use global color table
The GIF format allows for an image to contain a global color table which
might be used for some or every frame in an animated GIF.  This palette
contains 24-bit opaque RGB values.  An individual frame may use the
global palette and enable transparency by picking one number to be
transparent, instead of the color value in the palette.

image/gif decodes a GIF, which contains an []*image.Paletted that holds
each frame.  When decoded, if a frame has a transparent color and uses
the global palette, a copy of the global []color.Color is made, and the
transparency color index is replaced with color.RGBA{}.

When encoding a GIF, each frame's palette is encoded to the form it
might exist in a GIF, up to 768 bytes "RGBRGBRGBRGB...". If a frame's
encoded palette is equal to the encoded global color table, the frame
will be encoded with the flag set to use the global color table,
otherwise the frame's palette will be included.

So, if the color in the global color table that matches the transparent
index of one frame wasn't black (and it frequently is not), reencoding a
GIF will likely result in a larger file because each frame's palette
will have to be encoded inline.

This commit takes a frame's transparent color index into account when
comparing an individual image.Paletted's encoded color table to the
global color table.

Fixes #22137

Change-Id: I5460021da6e4d7ce19198d5f94a8ce714815bc08
Reviewed-on: https://go-review.googlesource.com/68313
Reviewed-by: Nigel Tao <nigeltao@golang.org>
2017-10-13 04:28:53 +00:00
David Chase
e45e490296 cmd/compile: attempt to deflake debug_test.go
Excluded when -short because it still runs relatively long,
but deflaked.

Removed timeouts from normal path and ensured that they were
not needed and that reference files did not change.

Use "tbreak" instead of "break" with gdb to reduce chance
of multiple hits on main.main.  (Seems not enough, but a
move in the right direction).

By default, testing ignores repeated lines that occur when
nexting.  This appears to sometimes be timing-dependent and
is the observed source of flakiness in testing so far.
Note that these can also be signs of a bug in the generated
debugging output, but it is one of the less-confusing bugs
that can occur.

By default, testing with gdb uses compilation with
inlining disabled to prevent dependence on library code
(it's a bug that library code is seen while Nexting, but
the bug is current behavior).

Also by default exclude all source files outside /testdata
to prevent accidental dependence on library code.  Note that
this is currently only applicable to dlv because (for the
debugging information we produce) gdb does not indicate a
change in the source file for inlined code.

Added flags -i and -r to make gdb testing compile with
inlining and be sensitive to repeats in the next stream.
This is for developer-testing and so we can describe these
problems in bug reports.

Updates #22206.

Change-Id: I9a30ebbc65aa0153fe77b1858cf19743bdc985e4
Reviewed-on: https://go-review.googlesource.com/69930
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 03:25:23 +00:00
Tim Cooper
245e386e4c reflect: allow Copy to a byte array or byte slice from a string
This somewhat mirrors the special case behavior of the copy built-in.

Fixes #22215

Change-Id: Ic353003ad3de659d3a6b4e9d97295b42510f3bf7
Reviewed-on: https://go-review.googlesource.com/70431
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 02:35:56 +00:00
Frank Somers
c14dcfda6b runtime: factor amd64 specifics from vdso_linux.go
This is a preparation step for adding vDSO support on linux/386.

This change relocates the elf64 and amd64 specifics from
vdso_linux.go to a new vdso_linux_amd64.go.

This should enable vdso_linux.go to be used for vDSO
support on linux architectures other than amd64.

- Relocate the elf64X structure definitions appropriate to amd64,
  and change their names to elfX so that the code in vdso_linux.go
  is ELFnn-agnostic.

- Relocate the sym_keys and corresponding __vdso_* variables
  appropriate to amd64.

- Provide an amd64-specific constant for the maximum byte size of
  an array, and use this in vdso_linux.go to compute constants for
  sizing the elf structure arrays traversed in the loaded vDSO.

Change-Id: I1edb4e4ec9f2d79b7533aa95fbd09f771fa4edef
Reviewed-on: https://go-review.googlesource.com/69391
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 02:04:20 +00:00
David Crawshaw
c58b98b2d6 cmd/link, runtime: put hasmain bit in moduledata
Currently we look to see if the main.main symbol address is in the
module data text range. This requires access to the main.main
symbol, which usually the runtime has, but does not when building
a plugin.

To avoid a dynamic relocation to main.main (which I haven't worked
out how to have the linker generate on darwin), stop using the
symbol. Instead record a boolean in the moduledata if the module
has the main function.

Fixes #22175

Change-Id: If313a118f17ab499d0a760bbc2519771ed654530
Reviewed-on: https://go-review.googlesource.com/69370
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 01:13:33 +00:00
David Crawshaw
d06815ba3f cmd/link: split PE loader into its own package
For #22095

Change-Id: I8f48fce571b69a7e8edf2ad7733ffdfd38676e63
Reviewed-on: https://go-review.googlesource.com/70310
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-12 21:35:43 +00:00
Tobias Klauser
fd9fac2c7c cmd/compile: add two error position tests for the typechecker
Follow CL 41477 and add two more line position tests for yyerror calls
in the typechecker which are currently not tested.

Update #19683

Change-Id: Iacd865195a3bfba87d8c22655382af267aba47a9
Reviewed-on: https://go-review.googlesource.com/70251
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
2017-10-12 20:50:20 +00:00
Matthew Dempsky
53bbddd527 cmd/compile: intrinsify runtime/internal/sys.Ctz{32,64} on ppc64
These functions are identical to math/bits.TrailingZeros{32,64}, which
are already intrinsified on ppc64.

Change-Id: If7ee57e7afe53154874f4b66bacdb6237806128a
Reviewed-on: https://go-review.googlesource.com/70350
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2017-10-12 20:01:16 +00:00
Ian Lance Taylor
56dec8dde2 test: add test case that gccgo miscompiled
Error was

main.go:7:11: error: import error at 162: expected ‘<type ’

Change-Id: Iacfe4bfa003d7708a21ebc89ad1ab2d4a3b041a8
Reviewed-on: https://go-review.googlesource.com/70290
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-12 19:02:09 +00:00
Austin Clements
97920373fa cmd/link: generate PC ranges for compilation unit DIEs
When we split separate packages into separate compilation units, we
lost PC range information because it was no longer contiguous. This
brings it back by constructing proper per-package PC range tables.

Change-Id: Id0ab5187e08ac5d13b3d3794977bfc857a56224f
Reviewed-on: https://go-review.googlesource.com/69974
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
2017-10-12 18:56:26 +00:00
Austin Clements
d4dda76b5f cmd/link: one DWARF compilation unit per package
Currently, the linker generates one huge DWARF compilation unit for
the entire Go binary. This commit creates a separate compilation unit
and line table per Go package.

We temporarily lose compilation unit PC range information, since it's
now discontiguous, so harder to emit. We'll bring it back in the next
commit.

Beyond being "more traditional", this has various technical
advantages:

* It should speed up line table lookup, since that requires a
  sequential scan of the line table. With this change, a debugger can
  first locate the per-package line table and then scan only that line
  table.

* Once we emit compilation unit PC ranges again, this should also
  speed up various other debugger reverse PC lookups.

* It puts us in a good position to move more DWARF generation into the
  compiler, which could produce at least the CU header, per-function
  line table fragments, and per-function frame unwinding info that the
  linker could simply paste together.

* It will let us record a per-package compiler command-line flags
  (#22168).

Change-Id: Ibac642890984636b3ef1d4b37fe97f4453c2cc84
Reviewed-on: https://go-review.googlesource.com/69973
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-10-12 18:56:23 +00:00