If there is doubt about passing arguments correctly
(as there is in this test), there should be doubt about
getting the results back intact too. Using 0 and 1
(especially 0 for success) makes it easy to get a PASS
accidentally when the return value is not actually
being propagated. Use less common values.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/141110043
runtime: handle nil ptr load/store in arm software floating point
We cannot let a real panic start there, because there is C code
on the stack, and worse, there is an assembly frame with a
saved copy of the registers and we have no idea which ones
are pointers.
Instead, detect the nil ptr load/store and return out of the C
and assembly into a stub that will start the call to sigpanic.
Fixes GOARM=5 build.
LGTM=iant
R=golang-codereviews, iant
CC=dave, golang-codereviews, minux, r
https://golang.org/cl/138130043
sigprof and setcpuprofilerate coordinate the enabling/disabling
of the handler using a Mutex. This has always been a bit dodgy:
setcpuprofilerate must be careful to turn off signals before acquiring
the lock to avoid a deadlock.
Now the lock implementations use onM, and onM isn't okay on the
signal stack. We know how to make it okay, but it's more work than
is probably worth doing.
Since this is super-dodgy anyway, replace the lock with a simple
cas loop. It is only contended if setcpuprofilerate is being called,
and that doesn't happen frequently enough to care about the
raw speed or about using futexes/semaphores.
TBR to fix freebsd/amd64 and dragonfly/amd64 builds.
Happy to make changes in a follow-up CL.
The general kernel system call interface
takes 6 arguments: R0, R1, R2, R3, R4, R5.
Syscall is for calls that only need 3.
The amd64 and 386 versions zero the extra arg registers,
but the arm version does not.
func utimensat calls Syscall with 3 arguments.
The kernel expects a 4th argument.
That turns out to be whatever is in R3 at the time of the call.
CL 137160043 changed various pieces of code and apparently
changed the value left in R3 at the time of utimensat's Syscall.
This causes the kernel to return EINVAL.
Change linux/arm Syscall to zero R3, R4, R5, so that calls will
behave deterministically, even if they pass too few arguments.
Arguably, utimensat could be fixed too, but the predictable
zeroing is certainly worth doing, and once done utimensat's
use of Syscall is fine.
1. If onM is called on a g0 stack, it just calls the given function.
2. If onM is called on a gsignal stack, it calls badonm.
3. If onM is called on a curg stack, it switches to the g0 stack
and then calls the function.
In cases 1 and 2, if the program then crashes (and badonm always does),
we want to see what called onM, but the traceback stops at onM.
In case 3, the traceback must stop at onM, because the g0
stack we are renting really does stop at onM.
The current code stops the traceback at onM to handle 3,
at the cost of making 1 and 2 crash with incomplete traces.
Change traceback to scan past onM but in case 3 make it look
like on the rented g0 stack, onM was called from mstart.
The traceback already knows that mstart is a top-of-stack function.
Alternate fix at CL 132610043 but I think this one is cleaner.
This CL makes 3 the exception, while that CL makes 1 and 2 the exception.
Submitting TBR to try to get better stack traces out of the
freebsd/amd64 builder, but happy to make changes in a
followup CL.
cmd/dist: make textflag.h available in runtime, avoid android/linux conflicts
1) cmd/dist was copying textflag.h to the build include directory,
but only after compiling package runtime. So other packages could
use it, just not runtime. Copy earlier, so that runtime can use it too.
2) We decided for android that anything marked linux is also included
in the build. The generated linux-specific files in cmd/dist must therefore
have explicit +build !android tags, or else you can't have simultaneous
linux/arm and android/arm builds in a single client. The tag was already
there for at least one file, but it was missing from many others.
cmd/api: don't depend on os/user or USER to check api
The -nocgo builder failed because it has cgo disabled
as well as no USER environment variable:
http://build.golang.org/log/2250abb82f5022b72a12997b8ff89fcdeff094c9
# Checking API compatibility.
Error getting current user: user: Current not implemented on linux/amd64
exit status 1
The original conversion in CL 132090043 cut up
the function in an attempt to avoid converting most
of the code to Go. This contorts the control flow.
While debugging the onM signal stack bug,
I reconverted sigqueue.goc in its entirety.
This restores the original control flow, which is
much easier to understand.
The current conversion is correct, it's just complex
and will be hard to maintain. The new one is as
readable as the original code.
I uploaded sigqueue.goc as the initial copy of
sigqueue.go in the CL, so if you view the diffs
of sigqueue.go comparing against patch set 2 [sic]
it will show the actual starting point.
For example:
https://golang.org/cl/136160043/diff2/20001:60001/src/pkg/runtime/sigqueue.go
LGTM=dvyukov, iant
R=golang-codereviews, dvyukov, iant
CC=golang-codereviews, khr, r
https://golang.org/cl/136160043
The arm5 build breakage at CL 139110043 was caused by
calling funcPC on a lessstack defined as a struct{}.
That symbol ended up with a non-4-aligned address,
which caused the memory fault that broke the builders.
The definition of lessstack was fixed in CL 140880043.
Tracking that down suggested that it would be worth
looking for the same bug elsewhere in the directory.
This is the only one I found.
LGTM=bradfitz
R=golang-codereviews, dave, bradfitz
CC=dvyukov, golang-codereviews, iant, khr, r
https://golang.org/cl/134410043
The implementation and use patterns of onM assume
that they run on either the m->curg or m->g0 stack.
Calling onM from m->gsignal has two problems:
(1) When not on g0, onM switches to g0 and then "back" to curg.
If we didn't start at curg, bad things happen.
(2) The use of scalararg/ptrarg to pass C arguments and results
assumes that there is only one onM call at a time.
If a gsignal starts running, it may have interrupted the
setup/teardown of the args for an onM on the curg or g0 stack.
Using scalararg/ptrarg itself would smash those.
We can fix (1) by remembering what g was running before the switch.
We can fix (2) by requiring that uses of onM that might happen
on a signal handling stack must save the old scalararg/ptrarg
and restore them after the call, instead of zeroing them.
The only sane way to do this is to introduce a separate
onM_signalsafe that omits the signal check, and then if you
see a call to onM_signalsafe you know the surrounding code
must preserve the old scalararg/ptrarg values.
(The implementation would be that onM_signalsafe just calls
fn if on the signal stack or else jumps to onM. It's not necessary
to have two whole copies of the function.)
(2) is not a problem if the caller and callee are both Go and
a closure is used instead of the scalararg/ptrarg slots.
For now, I think we can avoid calling onM from code executing
on gsignal stacks, so just reject it.
In the long term, (2) goes away (as do the scalararg/ptrarg slots)
once everything is in Go, and at that point fixing (1) would be
trivial and maybe worth doing just for regularity.
LGTM=iant
R=golang-codereviews, iant
CC=dvyukov, golang-codereviews, khr, r
https://golang.org/cl/135400043
Instead of making asmcgocall call asmcgocall_errno,
make both load args into registers and call a shared
assembly function.
On amd64, this costs 1 word in the asmcgocall_errno path
but saves 3 words in the asmcgocall path, and the latter
is what happens on critical nosplit paths on Windows.
On arm, this fixes build failures: asmcgocall was writing
the arguments for asmcgocall_errno into the wrong
place on the stack. Passing them in registers avoids the
decision entirely.
On 386, this isn't really needed, since the nosplit paths
have twice as many words to work with, but do it for consistency.
net: fix parsing literal IP address in builtin dns stub resolver
This CL fixes a bug introduced by CL 128820043 which is that
builtin dns stub resolver doesn't work well with literal IPv6
address namesever entries in /etc/resolv.conf.
Also simplifies resolv.conf parser and adds more test cases.
Rob Pike [Wed, 3 Sep 2014 22:57:03 +0000 (15:57 -0700)]
text/template: 0xef is an integer, not a floating-point value.
The discriminator in the execution engine was stupid.
Add a test to the parse package too. The problem wasn't there
but the particular case ('e' in a hex integer) was not covered.
runtime: make entersyscall/exitsyscall safe for stack splits
It is fundamentally unsafe to grow the stack once someone
has made a call to syscall.Syscall. That function takes 6 uintptr
arguments, but depending on the call some are pointers.
In fact, some might be pointers to stack values, and we don't know which.
That makes it impossible to copy the stack somewhere else.
Since we want to delete all the stack splitting code, relying only
on stack copying, make sure that Syscall never needs to split the stack.
The only thing Syscall does is:
call entersyscall
make the system call
call exitsyscall
As long as we make sure that entersyscall and exitsyscall
can live in the nosplit region, they won't ask for more stack.
Do this by making entersyscall and exitsyscall set up the
stack guard so that any call to a function with a split check
will cause a crash. Then move non-essential slow-path
work onto the m stack using onM and mark the rest of the
work nosplit. The linker will verify that the chain of nosplits
fits in the total nosplit budget.
LGTM=iant
R=golang-codereviews, iant
CC=dvyukov, golang-codereviews, khr, r
https://golang.org/cl/140950043
Because symtab.c was partially converted before,
the diffs are not terribly useful.
The earlier conversion was trying to refactor or
clean up the code in addition to doing the translation.
It also made a mistake by redefining Func to be something
users could overwrite.
I undid those changes, making symtab.go a more
literal line-for-line translation of symtab.c instead.
LGTM=josharian
R=golang-codereviews, dave, bradfitz, josharian
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/140880043
Rick Hudson [Wed, 3 Sep 2014 16:06:36 +0000 (12:06 -0400)]
runtime: Start and stop individual goroutines at gc safepoints
Code to bring goroutines to a gc safepoint one at a time,
do some work such as scanning, and restart the
goroutine, and then move on to the next goroutine.
Currently this code does not do much useful work
but this infrastructure will be critical to future
concurrent GC work.
Alex Brainman [Wed, 3 Sep 2014 04:17:04 +0000 (14:17 +1000)]
runtime: increase windows prof thread priority sooner
If system is busy burning cpu, it takes long time (about 300ms on
windows builders) to adjust prof thread priority. Once adjusted, prof
thread runs ahead of everyone else, but due to initial slowness, it
does not capture prof snapshots until start-up period is completed.
Change prof thread priority sooner, so it can start captures straight
away.
The code I wrote originally works for trivial functions
that are inlined at a call site in another package,
because that was how I wrote my local test.
Make hex(x) work for non-inlinable functions too.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/140830043
Marko Tiikkaja [Tue, 2 Sep 2014 16:08:41 +0000 (09:08 -0700)]
database/sql: Avoid re-preparing statements when all connections are busy
Previously, if all connections were busy, we would always
re-prepare the statement on the connection we were assigned from
the pool. That meant that if all connections were busy most of the
time, the number of prepared statements for each connection would
keep increasing over time.
Instead, after getting a free connection, check to see if the
statement has already been prepared on it, and reuse the statement
handle if so.
runtime: fix race detector running Go code on g0 of non-main thread
It looks like this has just always been broken:
the race detector handles running Go code on g0 of the main thread
and on g0 of any extra threads created by non-Go code, but it does
not handle running Go code on g0 of non-main threads created by Go.
Handle that.
Should fix the race build failures on the dashboard.
We're running into this now because we are running more
and more Go code on g0.
Alex Brainman [Tue, 2 Sep 2014 01:54:24 +0000 (11:54 +1000)]
time: update generated zoneinfo_abbrs_windows.go
The file in repo has been updated recently, but all these changes
are gone off the web site now. It seems web site gets updated once
in a while, so we'll update our file occasionally.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/140780043
runtime/race: better handling of atomic operations
This change fixes the last known false negative of the race detector --
detection of races between mutating atomic operations and non-atomic operations.
Race runtime already has functions for precise modelling of various atomic operations,
so this change just forwards all atomic ops to race runtime
instead of poor man modeling in sync/atomic package.
Performance is also improved -- full sync/atomic tests run in 60s instead of 85s now.
This is needed for callers to be able to keep track of the
writing position within a zip file. Otherwise it's not
possible to compute the size of headers, and the TOC isn't
written until the very end.
runtime: paste mprof.goc into mprof.go as comments
NO CODE CHANGES HERE
The first conversion was not complete.
This CL doesn't make any actual changes,
but it inserts the missing mprof.goc code
as comments so that the next CL will have
useful diffs.
To make the diffs a bit more useful, removed
all semicolons, ->, and runtime· prefixes as well.
Also corrected order of a few functions in mprof.go
to match original order in mprof.goc.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/134200043
Ian Lance Taylor [Mon, 1 Sep 2014 02:59:43 +0000 (22:59 -0400)]
misc/cgo/testcdefs: comment out test of packed structs
The [568]c compilers no longer support packed structs, so
using them with -cdefs no longer works. Just commenting out
the test, rather than removing it, in case this needs to be
handled. It may be that -cdefs can go away entirely in the
future, in which case so can this directory.
Ian Lance Taylor [Sun, 31 Aug 2014 01:15:55 +0000 (18:15 -0700)]
runtime: fix Linux build
Make the definition of the EpollEvent data field consistent
across architectures, adapt the other use of it in
netpoll_epoll for the new definition, and use uint64 rather
than uintptr.
LGTM=dave
R=rsc, dave
CC=golang-codereviews
https://golang.org/cl/137890043
Dmitriy Vyukov [Sat, 30 Aug 2014 18:38:54 +0000 (22:38 +0400)]
runtime/pprof: make CPU profiling tests more robust
Under the race detector most of the samples go into race runtime,
because of that freebsd race builder constantly fails on this test.
Kelsey Hightower [Sat, 30 Aug 2014 05:19:30 +0000 (22:19 -0700)]
net/http: add BasicAuth method to *http.Request
The net/http package supports setting the HTTP Authorization header
using the Basic Authentication Scheme as defined in RFC 2617, but does
not provide support for extracting the username and password from an
authenticated request using the Basic Authentication Scheme.
Add BasicAuth method to *http.Request that returns the username and
password from authenticated requests using the Basic Authentication
Scheme.
Russ Cox [Sat, 30 Aug 2014 04:56:52 +0000 (00:56 -0400)]
runtime: increase nosplit area to 192
In CL 131450043, which raised it to 160,
I'd raise it to 192 if necessary.
Apparently it is necessary on windows/amd64.
One note for those concerned about the growth:
in the old segmented stack world, we wasted this much
space at the bottom of every stack segment.
In the new contiguous stack world, each goroutine has
only one stack segment, so we only waste this much space
once per goroutine. So even raising the limit further might
still be a net savings.
Dmitriy Vyukov [Sat, 30 Aug 2014 04:40:56 +0000 (08:40 +0400)]
runtime: convert type algorithms to Go
Actually it mostly deletes code -- alg.print and alg.copy go away.
There was only one usage of alg.print for debug purposes.
Alg.copy is used in chan.goc, but Keith replaces them with
memcopy during conversion, so alg.copy is not needed as well.
Converting them would be significant amount of work
for no visible benefit.
Alex A Skinner [Sat, 30 Aug 2014 04:12:28 +0000 (13:12 +0900)]
net: implement query-response fast failover in builtin dns stub resolver
Speed improvements via code cleanup, and changes to make go dns behave more like glibc resolver. See https://groups.google.com/forum/#!topic/golang-dev/lV-0aHqxVeo
Fixes #6579.
Benchmark results on linux/amd64
benchmark old ns/op new ns/op delta
BenchmarkGoLookupIP 48319032572937 -46.75%
BenchmarkGoLookupIPNoSuchHost 101141052419641 -76.08%
BenchmarkGoLookupIPWithBrokenNameServer 200077356245004490730 -74.99%
benchmark old allocs new allocs delta
BenchmarkGoLookupIP 287 288 0.35%
BenchmarkGoLookupIPNoSuchHost 204 102 -50.00%
BenchmarkGoLookupIPWithBrokenNameServer 410 358 -12.68%
benchmark old bytes new bytes delta
BenchmarkGoLookupIP 13181 13271 0.68%
BenchmarkGoLookupIPNoSuchHost 17260 8714 -49.51%
BenchmarkGoLookupIPWithBrokenNameServer 28160 22432 -20.34%