> The stack trace points to it pretty clearly. Done can indeed unblock
> Wait first and then panic. I guess we need to recover after first
> Done as well.
And it looks like TestWaitGroupMisuse2 was already hardened against
this. Do the same in TestWaitGroupMisuse3.
Russ Cox [Tue, 2 Feb 2016 14:19:47 +0000 (09:19 -0500)]
cmd/go: fix rebuild after installation of new Go release
The loading of zversion.go was expecting it to be in
package runtime, but it moved to runtime/internal/sys.
Worse, the load was not checking the error.
Brad Fitzpatrick [Mon, 1 Feb 2016 15:58:34 +0000 (15:58 +0000)]
net/http/httputil: fix spelling of Trailer hop-by-hop header per errata
RFC Errata 4522 (http://www.rfc-editor.org/errata_search.php?eid=4522)
notes that RFC 2616 had a typo in a list of headers that the
httputil.ReverseProxy code copied. Fix the typo in our code.
Austin Clements [Mon, 1 Feb 2016 19:06:51 +0000 (14:06 -0500)]
runtime: start an M when handing off a P when there's GC work
Currently it's possible for the scheduler to deadlock with the right
confluence of locked Gs, assists, and scheduling of background mark
workers. Broadly, this happens because handoffp is stricter than
findrunnable, and if the only work for a P is GC work, handoffp will
put the P into idle, rather than starting an M to execute that P. One
way this can happen is as follows:
0. There is only one user G, which we'll call G 1. There is more than
one P, but they're all idle except the one running G 1.
1. G 1 locks itself to an M using runtime.LockOSThread.
2. GC starts up and enters mark 1.
3. G 1 performs a GC assist, which completes mark 1 without being
fully satisfied. Completing mark 1 causes all background mark
workers to park. And since the assist isn't fully satisfied, it
parks as well, waiting for a background mark worker to satisfy its
remaining assist debt.
4. The assist park enters the scheduler. Since G 1 is locked to the M,
the scheduler releases the P and calls handoffp to hand the P to
another M.
5. handoffp checks the local and global run queues, which are empty,
and sees that there are idle Ps, so rather than start an M, it puts
the P into idle.
At this point, all of the Gs are waiting and all of the Ps are idle.
In particular, none of the GC workers are running, so no mark work
gets done and the assist on the main G is never satisfied, so the
whole process soft locks up.
Fix this by making handoffp start an M if there is GC work. This
reintroduces a key invariant: that in any situation where findrunnable
would return a G to run on a P, handoffp for that P will start an M to
run work on that P.
Fixes #13645.
Tested by running 2,689 iterations of `go tool dist test -no-rebuild
runtime:cpu124` across 10 linux-amd64-noopt VMs with no failures.
Without this change, the failure rate was somewhere around 1%.
Performance change is negligible.
name old time/op new time/op delta
XBenchGarbage-12 2.48ms ± 2% 2.48ms ± 1% -0.24% (p=0.000 n=92+93)
The first in the list above is the main fix that's necessary. The
other are two are in the git history but along for the cmd/bundle
ride. The middle CL is well-tested, small (mostly comments),
non-tricky, and almost never seen (since nobody really uses Trailers).
The final CL is just deleting an unused global variable.
Russ Cox [Fri, 29 Jan 2016 17:18:32 +0000 (12:18 -0500)]
cmd/go: avoid a few symlink-induced errors in internal and vendor checks
This CL expands symlinks only when an error would be reported otherwise.
Since the expansions are only on error paths, anything that worked yesterday
should still work after this CL.
This CL fixes a regression from Go 1.5 in "go run", or else we'd probably
postpone it.
Changing only the error paths is meant as a way to reduce the risk of
making this change so late in the release cycle, but it may actually be
the right strategy for symlinks in general.
Russ Cox [Fri, 29 Jan 2016 15:16:24 +0000 (10:16 -0500)]
cmd/vet: report uncalled functions in Printf %v
Given, say, var f *os.File, a new vet check in CL 14122 diagnoses:
fmt.Printf("%s\n", f.Name)
fmt.Println(f.Name)
but not
fmt.Printf("%v\n", f.Name)
In all three cases the error is that the argument should be f.Name().
Diagnosing Println but not Printf %v seems oddly inconsistent,
so I changed %v to have the check too. In fact, all verbs now have
the check except %p and %T.
Fixes Dave Cheney's confusion when trying to write an example
of the new vet check advertised in the Go 1.6 release notes.
Rahul Chaudhry [Fri, 29 Jan 2016 00:33:35 +0000 (16:33 -0800)]
unsafe: fix typo in documentation of valid Pointer->uintptr->Pointer conversions
Change-Id: Ib669d5241372326a46361ee096570e960b7a957f
Reviewed-on: https://go-review.googlesource.com/19082 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Thu, 28 Jan 2016 06:00:59 +0000 (22:00 -0800)]
runtime: align stack in sigfwd for darwin/386
We might be forwarding to a C signal handler. C code expects the stack
to be aligned. Should fix darwin/386 build: the testcarchive tests were
hanging as the program got an endless series of SIGSEGV signals.
Change-Id: Ia02485d3736a3c40e12259f02d25f842cf8e4d29
Reviewed-on: https://go-review.googlesource.com/19025
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ian Lance Taylor [Wed, 27 Jan 2016 22:07:25 +0000 (14:07 -0800)]
runtime: handle kindString in cgoCheckArg
It's awkward to get a string value in cgoCheckArg, but SWIG testing
revealed that it is possible. The new handling of extra files in the
ptr.go test emulates what SWIG does with an exported function that
returns a string.
Change-Id: I453717f867b8a49499576c28550e7c93053a0cf8
Reviewed-on: https://go-review.googlesource.com/19020
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Dmitry Vyukov [Wed, 27 Jan 2016 18:22:28 +0000 (19:22 +0100)]
runtime/race: run tests with GOMAXPROCS=1
We set GOMAXPROCS=1 to prevent test flakiness.
There are two sources of flakiness:
1. Some tests rely on particular execution order.
If the order is different, race does not happen at all.
2. Ironically, ThreadSanitizer runtime contains a logical race condition
that can lead to false negatives if racy accesses happen literally at the same time.
Tests used to work reliably in the good old days of GOMAXPROCS=1.
So let's set it for now. A more reliable solution is to explicitly annotate tests
with required execution order by means of a special "invisible" synchronization primitive
(that's what is done for C++ ThreadSanitizer tests). This is issue #14119.
This reduces flakes on RaceAsFunc3 test from 60/3000 to 1/3000.
Richard Miller [Wed, 27 Jan 2016 19:10:11 +0000 (19:10 +0000)]
runtime: remove redundant empty function call from Breakpoint on arm
CL 18964 included an extra patch (sorry, my first experience of
git-codereview) which defined the conventional breakpoint instruction
used by Plan 9 on arm, but also introduced a benign but unneeded
call to runtime.emptyfunc. This CL removes the redundant call again.
This completes the series of CLs which add support for Plan 9 on arm.
Russ Cox [Wed, 27 Jan 2016 15:05:18 +0000 (10:05 -0500)]
cmd/go: refine definition of 'standard' import paths to include vendored code
The vendored copy of golang.org/x/net/http/hpack was being treated
as not standard, which in turn was making it not subject to the mtime
exception for rebuilding the standard library in a release, which in turn
was making net/http look out of date.
One fix and three tests:
- Fix the definition of standard.
- Test that everything in $GOROOT/src/ is standard during 'go test cmd/go'.
(In general there can be non-standard things in $GOROOT/src/, but this
test implies that you can do that or you can run 'go test cmd/go',
but not both. That's fine.)
- Test that 'go list std cmd' shows our vendored code.
- Enforce that no standard package can depend on a non-standard one.
Also fix a few error printing nits.
Fixes #13713.
Change-Id: I1f943f1c354174c199e9b52075c11ee44198e81b
Reviewed-on: https://go-review.googlesource.com/18978 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Brad Fitzpatrick [Tue, 26 Jan 2016 19:57:19 +0000 (19:57 +0000)]
net/http: add protections against misuse of ServeFile
Martin Lenord pointed out that bad patterns have emerged in online
examples of how to use ServeFile, where people pass r.URL.Path[1:] to
ServeFile. This is unsafe. Document that it's unsafe, and add some
protections.
Brad Fitzpatrick [Wed, 27 Jan 2016 00:09:50 +0000 (00:09 +0000)]
runtime: add more debug info to flaky TestNumGoroutine
This has been flaking on the new OpenBSD 5.8 builders lately:
https://storage.googleapis.com/go-build-log/808270e7/openbsd-amd64-gce58_61ce2663.log
(as one example)
Austin Clements [Tue, 26 Jan 2016 19:44:58 +0000 (14:44 -0500)]
runtime: make p.gcBgMarkWorker a guintptr
Currently p.gcBgMarkWorker is a *g. Change it to a guintptr. This
eliminates a write barrier during the subtle mark worker parking dance
(which isn't known to be causing problems, but may).
Austin Clements [Tue, 26 Jan 2016 22:26:55 +0000 (17:26 -0500)]
runtime: acquire stack lock in traceEvent
traceEvent records system call events after a G has already entered
_Gsyscall, which means the garbage collector could be installing stack
barriers in the G's stack during the traceEvent. If traceEvent
attempts to capture the user stack during this, it may observe a
inconsistent stack barriers and panic. Fix this by acquiring the stack
lock around the stack walk in traceEvent.
Austin Clements [Wed, 20 Jan 2016 03:45:37 +0000 (22:45 -0500)]
runtime: attach mark workers to P after they park
Currently mark workers attach to their designated Ps before parking,
either during initialization or after performing a phase transition.
However, in both of these cases, it's possible that the mark worker is
running on a different P than the one it attaches to. This is a
problem, because as soon as the worker attaches to a P, that P's
scheduler can execute the worker. If the worker hasn't yet parked on
the P it's actually running on, this means the worker G will be
running in two places at once. The most visible consequence of this is
that once the first instance of the worker does park, it will clear
g.m and the second instance will crash shortly when it tries to use
g.m.
Fix this by moving the attach to the gopark callback. At this point,
the G is genuinely stopped and the callback is running on the system
stack, so it's safe for another P's scheduler to pick up the worker G.
Russ Cox [Tue, 26 Jan 2016 20:26:09 +0000 (15:26 -0500)]
cmd/internal/obj/arm64: adjust literal pool flush for span-dependent jump enlargement
The current code delays the literal pool until the very last moment,
but based on the assumption that span-dependent jumps are as
short as possible. If they need to be enlarged in a later round, that
very last moment may be too late. Flush a little early to prevent that.
Fixes #13579.
Change-Id: I759b5db5c43a977bf2b940872870cbbc436ad141
Reviewed-on: https://go-review.googlesource.com/18972 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dave Cheney <dave@cheney.net>
Run-TryBot: Russ Cox <rsc@golang.org>
Adam Langley [Mon, 11 Jan 2016 02:30:27 +0000 (18:30 -0800)]
crypto/rsa: expand on documentation and add some examples.
In some cases the documentation for functions in this package was
lacking from the beginning and, in order cases, the documentation didn't
keep pace as the package grew.
This change somewhat addresses that.
Updates #13711.
Change-Id: I25b2bb1fcd4658c5417671e23cf8e644d08cb9ab
Reviewed-on: https://go-review.googlesource.com/18486 Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Brad Fitzpatrick [Tue, 26 Jan 2016 22:56:08 +0000 (22:56 +0000)]
doc: mention the need for a C compiler for cgo support
Fixes #13954
Change-Id: I4c01e9bb3fb08e8b9fa14d4c59b7ea824ba3f0c9
Reviewed-on: https://go-review.googlesource.com/18937 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Austin Clements [Fri, 15 Jan 2016 19:23:43 +0000 (14:23 -0500)]
runtime/pprof: retry failed tests with longer duration
Currently we run profiling tests for around 200ms in short mode.
However, even on platforms with good profiling, these tests are
inherently flaky, especially on loaded systems like the builders.
To mitigate this, modify the profiling test harness so that if a test
fails in a way that could indicate there just weren't enough samples,
it retries with a longer duration.
This requires some adjustment to the profile checker to distinguish
"fatal" and "retryable" errors. In particular, we no longer consider
it a fatal error to get a profile with zero samples (which we
previously treated as a parse error). We replace this with a retryable
check that the total number of samples is reasonable.
Brad Fitzpatrick [Tue, 26 Jan 2016 18:55:35 +0000 (18:55 +0000)]
net/http: document TimeFormat more
Fixes #14103
Change-Id: I89963643eccc902b809e04b7a14153acb0d242e1
Reviewed-on: https://go-review.googlesource.com/18933 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Mon, 25 Jan 2016 19:44:02 +0000 (11:44 -0800)]
cmd/link: add -extar option to set ar program for c-archive
People who want to use -buildmode=c-archive in unusual cross-compilation
setups will need something like this. It could also be done via (yet
another) environment variable but I use -extar by analogy with the
existing -extld.
Change-Id: I354cfabc4c470603affd13cd946997b3a24c0e6c
Reviewed-on: https://go-review.googlesource.com/18913 Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Mikio Hara [Tue, 26 Jan 2016 07:29:47 +0000 (16:29 +0900)]
net/http: fix nit in test
Change-Id: I8c647e709d93a76636e04375609fceadf3754aa1
Reviewed-on: https://go-review.googlesource.com/18954 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Mikio Hara [Tue, 26 Jan 2016 07:35:52 +0000 (16:35 +0900)]
cmd/go/testdata: fix nits in test
Change-Id: I85fa5e672a476098f8711dcbb5b20ea1a3fa630d
Reviewed-on: https://go-review.googlesource.com/18953 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Mon, 25 Jan 2016 23:22:03 +0000 (15:22 -0800)]
runtime: don't check sigaltstack on darwin/{arm,arm64}
Use of the alternate signal stack on darwin/{arm,arm64} is reportedly
buggy, and the runtime function sigaltstack does nothing. So don't
check the sigaltstack result to decide how to handle the signal stack.
Brad Fitzpatrick [Wed, 20 Jan 2016 19:00:32 +0000 (19:00 +0000)]
net/http/httputil: clarify docs on the Dump functions
Also don't nil out the Request or Response Body on error. Just leave
it in its previous broken state. The docs now say it's undefined, but
it always was.
Fixes #14036
Change-Id: I7fe175a36cbc01b4158f4dffacd8733b2ffa9999
Reviewed-on: https://go-review.googlesource.com/18726 Reviewed-by: Rob Pike <r@golang.org>
Ian Lance Taylor [Mon, 25 Jan 2016 17:54:39 +0000 (09:54 -0800)]
runtime/pprof: document SetCPUProfile with c-archive/c-shared
When using c-archive/c-shared, the signal handler for SIGPROF will not
be installed, which means that runtime/pprof.StartCPUProfile won't work.
There is no really good solution here, as the main program may want to
do its own profiling. For now, just document that runtime/pprof doesn't
work as expected, but that it will work if you use Notify to install the
Go signal handler.
Fixes #14043.
Change-Id: I7ff7a01df6ef7f63a7f050aac3674d640a246fb4
Reviewed-on: https://go-review.googlesource.com/18911
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org>
Ian Lance Taylor [Fri, 22 Jan 2016 05:56:38 +0000 (21:56 -0800)]
runtime: always install new signal stack on NetBSD and DragonFly
On NetBSD and DragonFly a newly created thread inherits the signal stack
of the creating thread. That means that in a cgo program a C thread
created using pthread_create will get the signal stack of the creating
thread, most likely a Go thread. This will then lead to chaos if two
signals occur simultaneously.
We can't fix the general case. But we can fix the case of a C thread
that calls a Go function, by installing a new signal stack and then
dropping it when we return to C. That will break the case of a C thread
that calls sigaltstack and then calls Go, because we will drop the C
thread's alternate signal stack as we return from Go. Still, this is
the 1.5 behavior. And what else can we do?
Russ Cox [Sun, 24 Jan 2016 06:33:16 +0000 (01:33 -0500)]
cmd/asm: reject foo(SB)(AX) instead of silently treating as foo(SB)
Add test for assembly errors, to verify fix.
Make sure invalid instruction errors are printed just once
(was printing them once per span iteration, so typically twice).
Fixes #13282.
Change-Id: Id5f66f80a80b3bc4832e00084b0a91f1afec7f8f
Reviewed-on: https://go-review.googlesource.com/18858 Reviewed-by: Rob Pike <r@golang.org>
Russ Cox [Sun, 24 Jan 2016 16:02:19 +0000 (11:02 -0500)]
misc/cgo/test: fix test on darwin/386 with cgo enabled
Apparently the darwin/386 builder does not enable cgo.
This failure turned up running
GOARCH=386 GOHOSTARCH=386 ./all.bash
on my Mac.
Change-Id: Ia2487c4fd85d4b0f9f564880f22d9fde379946c3
Reviewed-on: https://go-review.googlesource.com/18859 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the standard names, for discoverability.
Use the standard register arguments, for correctness.
Implement all possible arguments, for completeness.
Enable the corresponding tests now that everything is standard.
Update the uses in package runtime.
Russ Cox [Fri, 22 Jan 2016 03:48:29 +0000 (22:48 -0500)]
cmd/asm: report more than one instruction encoding error
Also, remove output file if there are encoding errors.
The extra reports are convenient.
Removing the output file is very important.
Noticed while testing.
Change-Id: I0fab17d4078f93c5a0d6d1217d8d9a63ac789696
Reviewed-on: https://go-review.googlesource.com/18845 Reviewed-by: Rob Pike <r@golang.org>
Russ Cox [Tue, 5 Jan 2016 14:48:45 +0000 (09:48 -0500)]
cmd/asm: simplify golden test maintenance
Instead of two parallel files that look almost identical,
mark the expected differences in the original file.
The annotations being added here keep the tests passing,
but they also make clear a number of printing or parsing
errors that were not as easily seen when the data was
split across two files.
Fix a few diagnostic problems in cmd/internal/obj as well.
Mikio Hara [Sat, 23 Jan 2016 04:28:14 +0000 (13:28 +0900)]
net: enable TestLookupDotsWithRemoteSource on builders
Change-Id: I2609660b10a16ec2a256fc9c8e046ba4ae67963f
Reviewed-on: https://go-review.googlesource.com/18880 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dominik Honnef [Sat, 23 Jan 2016 03:57:21 +0000 (04:57 +0100)]
doc: missing words and letters in release notes
Change-Id: Ica7f2a000eb1d89d5b02cb8c6f1596ddc04bfb26
Reviewed-on: https://go-review.googlesource.com/18890 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Thu, 14 Jan 2016 01:14:03 +0000 (20:14 -0500)]
unsafe: document valid uses of Pointer
Add docs for valid uses of Pointer.
Then document change made for #13372 in CL 18584.
Fixes #8994.
Change-Id: Ifba71e5aeafd11f684aed0b7ddacf3c8ec07c580
Reviewed-on: https://go-review.googlesource.com/18640 Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Rick Hudson <rlh@golang.org>