With gccgo, if we generate getg inlined, the backend may cache
the address of the TLS variable, which will become invalid after
a thread switch.
Currently there is no known bug for this. But if we didn't
implement this carefully, we may get subtle bugs. This CL adds a
test that will fail loudly if this is wrong. (See also
https://go.googlesource.com/gofrontend/+/refs/heads/master/libgo/runtime/proc.c#333
and an incorrect attempt CL 185337.)
Note: at least on Linux/AMD64, even with an incorrect
implementation, this only fails if the test is compiled with
-fPIC, which is not the default setting for gccgo test suite. So
some manual work is needed. Maybe we could extend the test suite
to run the runtime test with more settings (e.g. PIC and static).
Change-Id: I459a3b4c31f09b9785c0eca19b7756f80e8ef54c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186357
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/go: tighten the check for pseudo-version base tags
Do not allow a pseudo-version derived from a canonical tag to refer to
the same revision as the tag itself. It's unnecessary (because
canonical tags already have a total ordering) and confusing (the
pseudo-version appears to come after the tag, but actually refers to
the exact same revision).
Updates #32879
Updates #27173
Change-Id: I02befedbe89c8819bdd93e470783ce63fc813193
Reviewed-on: https://go-review.googlesource.com/c/go/+/184720
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Jay Conrod [Fri, 12 Jul 2019 21:31:59 +0000 (17:31 -0400)]
cmd/go: clarify error text for module path mismatch
This error occurs when a module is loaded with one name (for example,
github.com/golang/lint) but declares a different path in its go.mod
(golang.org/x/lint). The current text "unexpected module path" is
confusing. It doesn't explain why the path was unexpected, and it's
not clear what was expected.
With this change, the error text includes the module and version
containing the go.mod file with the error, the declared module path,
and the loaded module path. The paths are vertically aligned so
differences are visually obvious. As with other module version errors,
the shortest chain of requirements is printed.
This change supercedes CL 158477.
Fixes #28489
Change-Id: Ieb07d00bcae182376d7be6aad111c84fbf784354
Reviewed-on: https://go-review.googlesource.com/c/go/+/185985
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Sat, 13 Jul 2019 19:20:43 +0000 (12:20 -0700)]
cmd/link: put shlib ".type" functions in internal ABI
These functions are compiler generated, and as such are only available
in the internal ABI. Doing this avoids generating an alias symbol.
Doing that avoids confusion between unmangled and mangled type symbols.
Fixes #30768
Change-Id: I197a5ba6403aac11989ffa951dbe35bd0506de91
Reviewed-on: https://go-review.googlesource.com/c/go/+/186077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
cmd/go/internal/modfetch: always check for a go.mod file when fetching from version control
If the module path declared in the go.mod file does not match the path
we are trying to resolve, a build using that module is doomed to fail.
Since we know that the module path does not match in the underlying
repo, we also know that the requested module does not exist at the
requested version.
Therefore, we should reject that version in Stat with a “not exist”
error — sooner rather than later — so that modload.Query will continue
to check other candidate paths (for example, with a major-version
suffix added or removed).
Fixes #33099
Change-Id: I43c980f78ed75fa6ace90f237cc3aad46c22d83a
Reviewed-on: https://go-review.googlesource.com/c/go/+/186237
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Session resumption is not a reliable TLS behavior: the server can decide
to reject a session ticket for a number of reasons, or no reason at all.
This makes this non-hermetic test extremely brittle.
It's currently broken on the builders for both TLS 1.2 and TLS 1.3, and
I could reproduce the issue for TLS 1.3 only. As I was debugging it, it
started passing entirely on my machine.
In practice, it doesn't get us any coverage as resumption is already
tested with the recorded exchange tests, and TestVerifyHostname still
provides a smoke test checking that we can in fact talk TLS.
cmd/go: check for source files in relative paths before attempting to determine the package path
This is a more minimial fix for the immediate symptom of 32917 and
30590, but does not improve 'list -e' behavior or error
messages resulting from other package loading issues.
Fixes #32917
Fixes #30590
Change-Id: I6088d14d864410159ebf228d9392d186322fd2a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/185417
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Jay Conrod [Fri, 12 Jul 2019 15:31:29 +0000 (11:31 -0400)]
cmd/go: improve module version query documentation
Add "upgrade" and "patch" to 'go help modules' section 'Module queries'.
Also explicitly call out the fact that @v2 will select the latest
version starting with v2, not the branch named v2, since this is a
common source of confusion.
Fixes #33010
Change-Id: I2fe27543b81a160fb6f6b8e8444a7a35f3661433
Reviewed-on: https://go-review.googlesource.com/c/go/+/185979
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Thu, 11 Jul 2019 18:57:31 +0000 (11:57 -0700)]
cmd/cgo: do not rewrite call if there are more args than parameters
We already skipped rewriting the call if there were fewer args than
parameters. But we can also get a cgo crash if there are more args,
if at least one of the extra args uses a name qualified with "C.".
Skip the rewrite, since the build will fail later anyhow.
Fixes #33061
Change-Id: I62ff3518b775b502ad10c2bacf9102db4c9a531c
Reviewed-on: https://go-review.googlesource.com/c/go/+/185797
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
This test sometimes times out when the machine is busy.
The reason behind is still a bit blurry. But it seems to comes from
the fact that on AIX, once a listen is performed a socket, every
connection will be accepted even before an accept is made (which only
occurs when a machine is busy). On Linux, a socket is created as a
"passive socket" which seems to wait for the accept before allowing
incoming connections.
Updates #29685
Change-Id: I41b053b7d5f5b4420b72d6a217be72e41220d769
Reviewed-on: https://go-review.googlesource.com/c/go/+/185717
Run-TryBot: Clément Chigot <clement.chigot@atos.net> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Francesco Renzi [Tue, 9 Jul 2019 13:43:10 +0000 (14:43 +0100)]
strings: document that order of pairs matters in NewReplacer
Update NewReplacer documentation to specify that in the case of
multiple matches at the same position, the matching old/new
pair that appears first in NewReplacer arguments takes precedence.
Fixes #32699
Change-Id: I9d0616d28e5cd8c9bfa301be201f2b0ebf361dff
Reviewed-on: https://go-review.googlesource.com/c/go/+/185099 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Than McIntosh [Wed, 10 Jul 2019 13:12:24 +0000 (09:12 -0400)]
test: new testcase for gccgo compiler bug
Updates #33013
Change-Id: I3db062b37860bb0c6c99a553408b47cf0313531e
Reviewed-on: https://go-review.googlesource.com/c/go/+/185517
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
test: add a test for gccgo bug in handling break statement in a select
Gccgo CL 184998 added optimizations for one- and two-case select
statements. But it didn't handle break statement in the select
case correctly. The fix is CL 185519. This CL adds a test.
Change-Id: Ide1b199f106172b41dd77c1f6e0d662fccdd8cc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/185520
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/cgo: fix check for conversion of ptr to struct field
According to the documentation "When passing a pointer to a field in a
struct, the Go memory in question is the memory occupied by the field,
not the entire struct.". checkAddr states that this should also work
with type conversions, which is implemented in isType. However,
ast.StarExpr must be enclosed in ast.ParenExpr according to the go spec
(see example below), which is not considered in the checks.
Example:
// struct Si { int i; int *p; }; void f(struct I *x) {}
import "C"
type S {
p *int
i C.struct_Si
}
func main() {
v := &S{new(int)}
C.f((*C.struct_I)(&v.i)) // <- panic
}
This example will cause cgo to emit a cgoCheck that checks the whole
struct S instead of just S.i causing the panic "cgo argument has Go
pointer to Go pointer".
This patch fixes this situation by adding support for ast.ParenExpr to
isType and adds a test, that fails without the fix.
Fixes #32970.
Change-Id: I15ea28c98f839e9fa708859ed107a2e5f1483133
Reviewed-on: https://go-review.googlesource.com/c/go/+/185098
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 28 Jun 2019 19:01:16 +0000 (15:01 -0400)]
net/http: fix Transport.MaxConnsPerHost limits & idle pool races
There were at least three races in the implementation of the pool of
idle HTTP connections before this CL.
The first race is that HTTP/2 connections can be shared for many
requests, but each requesting goroutine would take the connection out
of the pool and then immediately return it before using it; this
created unnecessary, tiny little race windows during which another
goroutine might dial a second connection instead of reusing the first.
This CL changes the idle pool to just leave the HTTP/2 connection in
the pool permanently (until there is reason to close it), instead of
doing the take-it-out-put-it-back dance race.
The second race is that “is there an idle connection?” and
“register to wait for an idle connection” were implemented as two
separate steps, in different critical sections. So a client could end
up registered to wait for an idle connection and be waiting or perhaps
dialing, not having noticed the idle connection sitting in the pool
that arrived between the two steps.
The third race is that t.getIdleConnCh assumes that the inability to
send on the channel means the client doesn't need the result, when it
could mean that the client has not yet entered the select.
That is, the main dial does:
idleConnCh := t.getIdleConnCh(cm)
select {
case v := <-dialc:
...
case pc := <-idleConnCh
...
...
}
But then tryPutIdleConn does:
waitingDialer := t.idleConnCh[key] // what getIdleConnCh(cm) returned
select {
case waitingDialer <- pconn:
// We're done ...
return nil
default:
if waitingDialer != nil {
// They had populated this, but their dial won
// first, so we can clean up this map entry.
delete(t.idleConnCh, key)
}
}
If the client has returned from getIdleConnCh but not yet reached the
select, tryPutIdleConn will be unable to do the send, incorrectly
conclude that the client does not care anymore, and put the connection
in the idle pool instead, again leaving the client dialing unnecessarily
while a connection sits in the idle pool.
(It's also odd that the success case does not clean up the map entry,
and also that the map has room for only a single waiting goroutine for
a given host.)
None of these races mattered too much before Go 1.11: at most they
meant that connections were not reused quite as promptly as possible,
or a few more than necessary would be created. But Go 1.11 added
Transport.MaxConnsPerHost, which limited the number of connections
created for a given host. The default is 0 (unlimited), but if a user
did explicitly impose a low limit (2 is common), all these misplaced
conns could easily add up to the entire limit, causing a deadlock.
This was causing intermittent timeouts in TestTransportMaxConnsPerHost.
The addition of the MaxConnsPerHost support added its own races.
For example, here t.incHostConnCount could increment the count
and return a channel ready for receiving, and then the client would
not receive from it nor ever issue the decrement, because the select
need not evaluate these two cases in order:
select {
case <-t.incHostConnCount(cmKey):
// count below conn per host limit; proceed
case pc := <-t.getIdleConnCh(cm):
if trace != nil && trace.GotConn != nil {
trace.GotConn(httptrace.GotConnInfo{Conn: pc.conn, Reused: pc.isReused()})
}
return pc, nil
...
}
Obviously, unmatched increments are another way to get to a deadlock.
TestTransportMaxConnsPerHost deadlocked approximately 100% of
the time with a small random sleep added between incHostConnCount
and the select:
ch := t.incHostConnCount(cmKey):
time.Sleep(time.Duration(rand.Intn(10))*time.Millisecond)
select {
case <-ch
// count below conn per host limit; proceed
case pc := <-t.getIdleConnCh(cm):
...
}
The limit also did not properly apply to HTTP/2, because of the
decrement being attached to the underlying net.Conn.Close
and net/http not having access to the underlying HTTP/2 conn.
The alternate decrements for HTTP/2 may also have introduced
spurious decrements (discussion in #29889). Perhaps those
spurious decrements or other races caused the other intermittent
non-deadlock failures in TestTransportMaxConnsPerHost,
in which the HTTP/2 phase created too many connections (#31982).
This CL replaces the buggy, racy code with new code that is hopefully
neither buggy nor racy.
Daniel Martí [Tue, 2 Jul 2019 21:56:41 +0000 (23:56 +0200)]
encoding/json: obey SetEscapeHTML in all MarshalJSON cases
It wasn't obeyed in the case where the MarshalJSON method uses a pointer
receiver, and the encoder grabs the address of a value to find that
method. addrMarshalerEncoder is the function that does this work, but it
ignored opts.escapeHTML.
Here's the before and after of the added test case, which was failing
before the fix. Now the two cases are correct and consistent.
Austin Clements [Thu, 27 Jun 2019 22:14:03 +0000 (18:14 -0400)]
cmd/cgo: accept weak dynamic imports
cgo produces dynamic imports for Go binaries by scanning the dynamic
imports table of a binary produced by the system C compiler and
linker. Currently, since it uses elf.File.ImportedSymbols, it only
reads global symbols. Unfortunately, recent versions of lld emit weak
symbol imports for several pthread symbols, which means the cgo tool
doesn't emit dynamic imports for them, which ultimately causes linking
of cgo binaries to fail.
Fix this by using elf.File.DynamicSymbols instead and filtering down
to both global and weak symbols.
Fixes #31912.
Change-Id: If346a7eca6733e3bfa2cccf74a9cda02a3e81d38
Reviewed-on: https://go-review.googlesource.com/c/go/+/184100
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Thu, 27 Jun 2019 22:03:15 +0000 (18:03 -0400)]
debug/elf: add version information to all dynamic symbols
Currently, File.ImportedSymbols is the only API that exposes the GNU
symbol version information for dynamic symbols. Unfortunately, it also
filters to specific types of symbols, and only returns symbol names.
The cgo tool is going to need symbol version information for more
symbols. In order to support this and make the API more orthogonal,
this CL adds version information to the Symbol type and updates
File.DynamicSymbols to fill this in. This has the downside of
increasing the size of Symbol, but seems to be the most natural API
for exposing this. I also explored 1) adding a method to get the
version information for the i'th dynamic symbol, but we don't use
symbol indexes anywhere else in the API, and it's not clear if this
index would be 0-based or 1-based, and 2) adding a
DynamicSymbolVersions method that returns a slice of version
information that parallels the DynamicSymbols slice, but that's less
efficient to implement and harder to use.
For #31912.
Change-Id: I69052ac3894f7af2aa9561f7085275130e0cf717
Reviewed-on: https://go-review.googlesource.com/c/go/+/184099
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
- fix link for `Time.Format`
- fix closing tag for `go get`
- add links for `runtime.Caller`, `runtime.Callers`
- remove link for `TypedArrayOf` since it has been removed (CL 177537)
Jay Conrod [Mon, 1 Jul 2019 19:25:04 +0000 (15:25 -0400)]
cmd/go: restore @latest behavior and support @upgrade in 'go get'
'go get path@latest' may now downgrade a module required at a
pre-release or pseudo-version newer than the latest released
version. This restores the 1.12 behavior and the ability to easily
roll back from a temporary development version.
'go get path@upgrade' is like @latest but will not downgrade.
If no version suffix is specified ('go get path'), @upgrade is
implied.
Fixes #32846
Change-Id: Ibec0628292ab1c484716a5add0950d7a7ee45f47
Reviewed-on: https://go-review.googlesource.com/c/go/+/184440
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
This CL adds a test for gccgo bug #32901: not all the type
descriptors are registered and thus deduplicated with types
created by reflection. It needs a few levels of indirect imports
to trigger this bug.
Updates #32901.
Change-Id: Idbd89bedd63fea746769f2687f3f31c9767e5ec0
Reviewed-on: https://go-review.googlesource.com/c/go/+/184718 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: I99e76c0c12050289be5b353595eb21fbabe7c01e
Reviewed-on: https://go-review.googlesource.com/c/go/+/184597 Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/go: fix TestScript/mod_sumdb_golang to avoid assumptions about @v/list
I accidentally fetched an invalid version of rsc.io/quote from
proxy.golang.org, which the proxy then cached and now includes in
https://proxy.golang.org/rsc.io/quote/@v/list.
That causes 'go get rsc.io/quote` to resolve to a different version
depending on whether the proxy is used.
Adjust the test to fetch an explicit version instead, since the choice
of 'latest' is mostly irrelevant to the checksum database logic that
the test is intended to verify.
Updates #32805
Fixes #32900
Change-Id: I075b1f62e8c71545d0fb2dd4bd77ba525fc2a36d
Reviewed-on: https://go-review.googlesource.com/c/go/+/184719
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Than McIntosh [Wed, 26 Jun 2019 00:24:15 +0000 (20:24 -0400)]
test: add testcase for gccgo compile failure
Test case that caused a compiler crash in gofrontend, related to
exporting inlinable function bodies.
Updates #32778
Change-Id: Iacf1753825d5359da43e5e281189876d4c3dd3c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/183851 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Keith Randall [Wed, 26 Jun 2019 02:24:34 +0000 (22:24 -0400)]
cmd/compile: make duplicate anonymous interface output deterministic
Taking over CL 162240, the original CL hasn't been making progress.
I just took the parts that fix the immediate issue. I left the
signatslice changes out, I don't think they are necessary.
Fixes #30202
Change-Id: I5b347605f0841dd925d5a73150b8bf269fa82464
Reviewed-on: https://go-review.googlesource.com/c/go/+/183852
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Russ Cox [Sat, 29 Jun 2019 13:26:19 +0000 (09:26 -0400)]
sync: document implementation of Once.Do
It's not correct to use atomic.CompareAndSwap to implement Once.Do,
and we don't, but why we don't is a question that has come up
twice on golang-dev in the past few months.
Add a comment to help others with the same question.
Ian Lance Taylor [Fri, 28 Jun 2019 18:20:15 +0000 (11:20 -0700)]
runtime: use a pipe to wake up signal_recv on Darwin
The implementation of semaphores, and therefore notes, used on Darwin
is not async-signal-safe. The runtime has one case where a note needs
to be woken up from a signal handler: the call to notewakeup in sigsend.
That notewakeup call is only called on a single note, and it doesn't
need the full functionality of notes: nothing ever does a timed wait on it.
So change that one note to use a different implementation on Darwin,
based on a pipe. This lets the wakeup code use the write call, which is
async-signal-safe.
Fixes #31264
Change-Id: If705072d7a961dd908ea9d639c8d12b222c64806
Reviewed-on: https://go-review.googlesource.com/c/go/+/184169
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Russ Cox [Fri, 28 Jun 2019 00:54:14 +0000 (20:54 -0400)]
crypto/tls: deflake localPipe in tests
The localPipe implementation assumes that every successful net.Dial
results in exactly one successful listener.Accept. I don't believe this
is guaranteed by essentially any operating system. For this test, we're
seeing flakes on dragonfly (#29583).
But see also #19519, flakes due to the same assumption on FreeBSD
and macOS in package net's own tests.
This CL rewrites localPipe to try a few times to get a matching pair
of connections on the dial and accept side.
Russ Cox [Fri, 28 Jun 2019 00:30:28 +0000 (20:30 -0400)]
net: deflake TestVariousDeadlines
TestVariousDeadlines starts a client and server.
The client dials the server, sets a timeout on the connection,
reads from it, gets a timeout error, closes the connection.
The server writes an infinite stream of a's to each connection
it accepts.
The test was trying to run these in lockstep:
run a client dial+read+timeout+close,
wait for server to accept+write+error out on write to closed connection,
repeat.
On FreeBSD 11.2 and less frequently on macOS we see
the test timeout waiting for the server to do its half of
the lockstep dance.
I believe the problem is that the client can do its step
of the dance with such a short timeout that the read,
timeout, and close happens before the server ever returns
from the accept(2) system call. For the purposes of testing
the client-side read timeout, this is fine. But I suspect
that under some circumstances, the "TCP-accepted"
connection does not translate into a "socket-layer-accepted"
connection that triggers a return from accept(2).
That is, the Go server never sees the connection at all.
And the test sits there waiting for it to acknowledge
being done with a connection it never started with.
Fix the problem by not trying to lockstep with the server.
This definitely fixes the flake, since the specific line that
was calling t.Fatal is now deleted.
This exposes a different flake, seen on a trybot run for an
early version of this CL, in which the client's io.Copy does
not stop within the time allotted. The problem now is that
there is no guarantee that a read beyond the deadline with
available data returns an error instead of the available data,
yet the test assumes this guarantee, and in fact the opposite
is usually true - we don't bother checking the deadline unless
the read needs to block. That is, deadlines don't cut off a
flood of available data, yet this test thinks they do.
This CL therefore also changes the server not to send an
infinite flood of data - don't send any data at all - so that
the read deadline is guaranteed to be exercised.
Russ Cox [Fri, 28 Jun 2019 05:42:08 +0000 (01:42 -0400)]
runtime: fix pprof cpu profile corruption on arm/mips/mipsle
CL 42652 changed the profile handler for mips/mipsle to
avoid recording a profile when in atomic functions, for fear
of interrupting the 32-bit simulation of a 64-bit atomic with
a lock. The profile logger itself uses 64-bit atomics and might
deadlock (#20146).
The change was to accumulate a count of dropped profile events
and then send the count when the next ordinary event was sent:
CL 117057 extended this behavior to include GOARCH == "arm".
Unfortunately, the inserted cpuprof.addLostAtomic64 differs from
the original cpuprof.add in that it neglects to acquire the lock
protecting the profile buffer.
This has caused a steady stream of flakes on the arm builders
for the past 12 months, ever since CL 117057 landed.
This CL moves the lostAtomic count into the profile buffer and
then lets the existing addExtra calls take care of it, instead of
duplicating the locking logic.
Dmitri Shuralyov [Thu, 27 Jun 2019 00:07:14 +0000 (20:07 -0400)]
cmd/doc: provide working directory to build.Import calls
The current cmd/doc implementation uses go/build.Import in a few
places to check whether a package is findable and importable.
go/build has limited support for finding packages in modules,
but to do so, build.Import requires knowing the source directory
to use when performing the lookup (so it can find the go.mod file).
Otherwise, it only looks inside the GOPATH workspace.
Start passing the current working directory to build.Import calls,
so that it can correctly look for packages in modules when in cmd/doc
is executed in module mode.
Before this change, cmd/doc in module mode could mistakenly find and
use a package in the GOPATH workspace, instead of the current module.
Since the result of os.Getwd is needed in even more places, assign it
to a local variable in parseArgs now.
Fixes #28992
Updates #26504
Change-Id: I7571618e18420d2d3b3890cc69ade2d97b1962bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/183991
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Diogo Pinela [Thu, 27 Jun 2019 21:19:15 +0000 (22:19 +0100)]
doc: fix spec links in Go 1.13 release notes
When reading tip.golang.org/doc/go1.13.html, the spec links in
the "Changes to the language" section should point to the updated
spec, not the old one.
Robert Griesemer [Thu, 27 Jun 2019 20:26:51 +0000 (13:26 -0700)]
text/scanner: remove AllowDigitSeparator flag again
The scanner was changed to accept the new Go number literal syntax
of which separators are a part. Making them opt-in is inconsistent
with the rest of the changes. For comparison, the strconv package
also accepts the new number literals including separators with the
various conversion routines, if no explicit number base is given.
Jay Conrod [Thu, 27 Jun 2019 19:28:08 +0000 (15:28 -0400)]
go/build: don't check if srcDir in GOPATH when deciding to use modules
go/build.Context.Import loads package information using 'go list' when
in module mode. It does this when GO111MODULE is not "off", there is a
go.mod file in any parent directory, and neither the path nor the
source directory are in GOROOT. Import no longer checks whether the
source directory is in GOPATH if GO111MODULE=auto or unset.
Also fixed subdirectory checks that did not handle relative source
directory paths. mod_gobuild_import should have failed when we changed
the meaning of GO111MODULE=auto but didn't because of this.
Fixes #32799
Change-Id: Ic5210b7e00cb58f91ea9455b67b49d5aed4eec63
Reviewed-on: https://go-review.googlesource.com/c/go/+/184098
Run-TryBot: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills [Wed, 26 Jun 2019 19:00:01 +0000 (15:00 -0400)]
misc/cgo/errors: align code snippets in ptr_test.go
Change-Id: Ic3e2819617375df653116d21d7361a46085250d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/183986 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Wed, 26 Jun 2019 19:57:25 +0000 (15:57 -0400)]
doc/go1.13: make cmd/go paragraphs more concise
Looking at the live release notes on tip.golang.org, the Modules
section is much more verbose than the other sections.
To some extent that's to be expected, but too much detail in the
release notes might discourage folks from consulting the actual
documentation. Ensure that topics have clear links and omit
unnecessary details.
Change-Id: I1ccbc1697fccaf7ca7094c606bd11696c46d87f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/183987
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Wed, 26 Jun 2019 18:35:05 +0000 (14:35 -0400)]
sync: only check for successful PopHeads in long mode
In TestPoolDequeue it's surprisingly common for the queue to stay
nearly empty the whole time and for a racing PopTail to happen in the
window between the producer doing a PushHead and doing a PopHead. In
short mode, there are only 100 PopTail attempts. On linux/amd64, it's
not uncommon for this to fail 50% of the time. On linux/arm64, it's
not uncommon for this to fail 100% of the time, causing the test to
fail.
This CL fixes this by only checking for a successful PopTail in long
mode. Long mode makes 200,000 PopTail attempts, and has never been
observed to fail.
Austin Clements [Wed, 26 Jun 2019 17:20:05 +0000 (13:20 -0400)]
sync: make TestPoolDequeue termination condition more robust
TestPoolDequeue creates P-1 consumer goroutines and 1 producer
goroutine. Currently, if a consumer goroutine pops the last value from
the dequeue, it sets a flag that stops all consumers, but the producer
also periodically pops from the dequeue and doesn't set this flag.
Hence, if the producer were to pop the last element, the consumers
will continue to run and the test won't terminate. This CL fixes this
by also setting the termination flag in the producer.
I believe it's impossible for this to happen right now because the
producer only pops after pushing an element for which j%10==0 and the
last element is either 999 or 1999999, which means it should never try
to pop after pushing the last element. However, we shouldn't depend on
this reasoning.
Austin Clements [Wed, 26 Jun 2019 15:11:45 +0000 (11:11 -0400)]
sync: fix pool wrap-around test
TestPoolDequeue in long mode does a little more than 1<<21 pushes.
This was originally because the head and tail indexes were 21 bits and
the intent was to test wrap-around behavior. However, in the final
version they were both 32 bits, so the test no longer tested
wrap-around.
It would take too long to reach 32-bit wrap around in a test, so
instead we initialize the poolDequeue with indexes that are already
nearly at their limit. This keeps the knowledge of the maximum index
in one place, and lets us test wrap-around even in short mode.
Bryan C. Mills [Tue, 25 Jun 2019 21:13:21 +0000 (17:13 -0400)]
cmd/go/internal/modfetch: halt proxy fallback if the proxy returns a non-404/410 response for @latest
The @latest proxy endpoint is optional. If a proxy returns a 404 for
it, and returns an @v/list with no matching versions, then we should
allow module lookup to try other module paths. However, if the proxy
returns some other error (say, a 403 or 505), then the result of the
lookup is ambiguous, and we should report the actual error rather than
"no matching versions for query".
(This fix was prompted by discussion with Dmitri on CL 183619.)
Updates #32715
Updates #26334
Change-Id: I6d510a5ac24d48d9bc5037c3c747ac50695c663f
Reviewed-on: https://go-review.googlesource.com/c/go/+/183845
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Cherry Zhang [Tue, 25 Jun 2019 18:48:04 +0000 (14:48 -0400)]
cmd/compile, runtime: use R20, R21 in ARM64's Duff's devices
Currently we use R16 and R17 for ARM64's Duff's devices.
According to ARM64 ABI, R16 and R17 can be used by the (external)
linker as scratch registers in trampolines. So don't use these
registers to pass information across functions.
It seems unlikely that calling Duff's devices would need a
trampoline in normal cases. But it could happen if the call
target is out of the 128 MB direct jump limit.
The choice of R20 and R21 is kind of arbitrary. The register
allocator allocates from low-numbered registers. High numbered
registers are chosen so it is unlikely to hold a live value and
forces a spill.
Jay Conrod [Tue, 25 Jun 2019 21:25:01 +0000 (17:25 -0400)]
cmd/go: 'go get' should not delete binaries when run from $GOBIN
When 'go install' is run without arguments in a directory containing a
main package, it deletes an executable file with the same name as the
package (presumably created by 'go build' previously).
'go get' in module mode executes the same code after updating and
downloading modules. However, the special case was misfiring because
we passed an empty list of patterns to InstallPackages.
Fixes #32766
Change-Id: I19aca64ee1fb5a216777dd7d559e8e6a45b3e90c
Reviewed-on: https://go-review.googlesource.com/c/go/+/183846
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Tobias Klauser [Tue, 25 Jun 2019 12:06:14 +0000 (14:06 +0200)]
syscall: fix nil pointer dereference in Getdirentries on 32-bit freebsd 12
Don't attempt to dereference basep if it's nil, just pass it to
getdirentries_freebsd12 as is.
Ported from x/sys/unix CL 183223
Change-Id: Id1c4e0eb6ff36dd39524da8194fed9a5957bce61
Reviewed-on: https://go-review.googlesource.com/c/go/+/183797 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Tue, 25 Jun 2019 22:46:23 +0000 (15:46 -0700)]
go/types: avoid race condition with dot-imported objects
It would be nice to have a test, but it requires running
this under the race detector which is a bit complicated
to set up; yet the fix is trivial. Verified manually that
it doesn't trip the race detector.
Fixes #32154.
Change-Id: I20bd746a07945c802f0476a1d8b1dfd83c87dae8
Reviewed-on: https://go-review.googlesource.com/c/go/+/183849
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Fri, 14 Jun 2019 02:11:25 +0000 (19:11 -0700)]
cmd/compile: don't mark argument array as noalg
It ends up making two similar types, [N]uint8 of both
alg and noalg varieties. Comparsions between the two then
don't come out equal when they should.
In particular, the type *[N]uint8 has an Elem pointer which
must point to one of the above two types; it can't point to both.
Thus allocating a *[N]uint8 and dereferencing it might be a
different type than a [N]uint8.
The fix is easy. Making a small test for this is really hard. It
requires that both a argless defer and the test be imported by a
common parent package. This is why a main binary doesn't see this
issue, but a test does (as Agniva noticed), because there's a wrapper
package that imports both the test and the defer.
Types like [N]uint8 don't really need to be marked noalg anyway,
as the generated code (if any) will be shared among all
vanilla memory types of the same size.
LE Manh Cuong [Mon, 17 Jun 2019 04:07:39 +0000 (11:07 +0700)]
cmd/go: validate path in mod init path
When mod init with given module path, validate that module path is a
valid import path.
Note that module.CheckImportPath is used, because module.CheckPath
verifies that module path is something that "go get" can fetch, which is
strictly stronger condition than "a valid module path".
Updates #28389
Fixes #32644
Change-Id: Ia60f218dd7d79186f87be723c28a96d6cb63017e
Reviewed-on: https://go-review.googlesource.com/c/go/+/182560
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Fri, 21 Jun 2019 19:13:10 +0000 (15:13 -0400)]
cmd/go/internal/modfetch: filter pseudo-versions from proxy /list endpoints
The /list files in the module cache include pseudo-versions, but the
documentation for (*modfetch).Repo.Versions explicitly states that
they are not included in the output of that method.
Fixes #32715
Change-Id: Ieba1500b91f52b5fa689e70e16dbe3ad40de20f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/183402
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Ben Shi [Mon, 24 Jun 2019 02:17:40 +0000 (02:17 +0000)]
runtime: fix a register save/restore bug in sigtramp of arm-darwin
In sigtramp of sys_darwin_arm.s, the callee-save register R4 is
saved to the stack, but later R2 is also saved to the save position.
That CL fixes the unexpected lost of the value in R4.
fixes #32744
Change-Id: Ifaeb99f11e4abf0c79bec9da67e0db97c358010c
Reviewed-on: https://go-review.googlesource.com/c/go/+/183517
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
kawakami [Sat, 22 Jun 2019 20:30:24 +0000 (05:30 +0900)]
cmd/cgo: fix inappropriate array copy
Ensure that during rewriting of expressions that take the address of
an array, that we properly recognize *ast.IndexExpr as an operation
to create a pointer variable and thus assign the proper addressOf
and deference operators as "&" and "*" respectively.
This fixes a regression from CL 142884.
Fixed #32579
Change-Id: I3cb78becff4f8035d66fc5536e5b52857eacaa3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/183458
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Mon, 24 Jun 2019 14:16:12 +0000 (10:16 -0400)]
cmd/go/internal/modfetch: treat a missing go.mod file as a “not exist” error
If we have found a repository at the requested version but it does not
contain a go.mod file in an appropriate subdirectory, then the module
with the given path does not exist at that version. Therefore, we
should report it with an error equivalent to os.ErrNotExist so that
modload.Query will continue to check other possible module paths.
Updates #27173
Change-Id: Ica73f4bb97f58e611a7f7d38183ee52fef5ee69a
Reviewed-on: https://go-review.googlesource.com/c/go/+/183618
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Ian Lance Taylor [Sat, 22 Jun 2019 19:56:49 +0000 (12:56 -0700)]
test: update blank1.go for changed gofrontend error messages
Adjusting gofrontend error messages for GCC standards causes the
messages expected by this test to be adjusted slightly: the gofrontend
code now quotes the _ identifier.
Change-Id: I55ee2ae70b4da3bf7a421ceea80b254dd17601a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/183477
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Bryan C. Mills [Tue, 11 Jun 2019 19:49:44 +0000 (15:49 -0400)]
cmd/go: validate pseudo-versions against module paths and revision metadata
Previously, most operations involving pseudo-versions allowed any
arbitrary combination of version string and date, and would resolve to
the underlying revision (typically a Git commit hash) as long as that
revision existed.
There are a number of problems with that approach:
• The pseudo-version participates in minimal version selection. If its
version prefix is inaccurate, the pseudo-version may appear to have
higher precedence that the releases that follow it, effectively
“pinning” the module to that commit. For release tags, module
authors are the ones who make the decision about release tagging;
they should also have control over the pseudo-version precedence
within their module.
• The commit date within the pseudo-version provides a total order
among pseudo-versions. If it is not accurate, the pseudo-version
will sort into the wrong place relative to other commits with the
same version prefix.
To address those problems, this change restricts the pseudo-versions
that the 'go' command accepts, rendering some previously
accepted-but-not-canonical versions invalid. A pseudo-version is now
valid only if all of:
1. The tag from which the pseudo-version derives points to the named
revision or one of its ancestors as reported by the underlying VCS
tool, or the pseudo-version is not derived from any tag (that is,
has a "vX.0.0-" prefix before the date string and uses the lowest
major version appropriate to the module path).
2. The date string within the pseudo-version matches the UTC timestamp
of the revision as reported by the underlying VCS tool.
3. The short name of the revision within the pseudo-version (such as a
Git hash prefix) is the same as the short name reported by the
underlying cmd/go/internal/modfetch/codehost.Repo. Specifically, if
the short name is a SHA-1 prefix, it must use the same number of
hex digits (12) as codehost.ShortenSHA1.
4. The pseudo-version includes a '+incompatible' suffix only if it is
needed for the corresponding major version, and only if the
underlying module does not have a go.mod file.
We believe that all releases of the 'go' tool have generated
pseudo-versions that meet these constraints. However, a few
pseudo-versions edited by hand or generated by third-party tools do
not. If we discover invalid-but-benign pseudo-versions in widely-used
existing dependencies, we may choose to add a whitelist for those
specific path/version combinations.
―
To work around invalid dependencies in leaf modules, users may add a
'replace' directive from the invalid version to its valid equivalent.
Note that the go command's go.mod parser automatically resolves commit
hashes found in 'replace' directives to the appropriate
pseudo-versions, so in most cases one can write something like:
and then run any 'go' command (such as 'go list' or 'go mod tidy') to
resolve it to an appropriate pseudo-version. Note that the invalid
version will still be used in minimal version selection, so this use
of 'replace' directives is an incomplete workaround.
―
One of the common use cases for higher-than-tagged pseudo-versions is
for projects that do parallel development on release branches. For
example, if a project cuts a 'v1.2' release branch at v1.2.0, they may
want future commits on the main branch to show up as pre-releases for
v1.3.0 rather than for v1.2.1 — especially if v1.2.1 is already tagged
on the release branch. (On the other hand, a backport of a patch to
the v1.2 branch should not show up as a pre-release for v1.3.0.)
To address this use-case, module authors can make use of our existing
support for pseudo-versions derived from pre-release tags: if the
author adds an explicit pre-release tag (such as 'v1.3.0-devel') to
the first commit after the branch, then the pseudo-versions for that
commit and its descendents will be derived from that tag and will sort
appropriately in version selection.