Some platform that implements inp_localgroup-like shared internet
protocol control block group looks a bit sensitive about transport
layer protocol's address:port reuse. Sometimes it rejects a TCP SYN
packet using TCP RST, and sometimes silence.
For now, until test case refactoring, we admit few Dial failures on
TestTCPConcurrentAccept as a workaround.
Update #7400
Update #7541
LGTM=jsing
R=jsing
CC=golang-codereviews
https://golang.org/cl/75920043
The previous fix CL 69340044 still leaves a possibility of it.
This CL prevents the kernel, especially DragonFly BSD, from
performing unpredictable asynchronous connection establishment
on stream-based transport layer protocol sockets.
Update #7541
Update #7474
LGTM=jsing
R=jsing
CC=golang-codereviews
https://golang.org/cl/75930043
Disable it until it's debugged so it doesn't hide other real
problems on Windows. The test was known to be unreliable
anyway (which is why it only needed 1 of 20 runs to pass), but
apparently it never passes on Windows. Figure out why later.
Update #7634
LGTM=alex.brainman
R=adg, alex.brainman
CC=golang-codereviews
https://golang.org/cl/80110043
Change two-bit stack map entries to encode:
0 = dead
1 = scalar
2 = pointer
3 = multiword
If multiword, the two-bit entry for the following word encodes:
0 = string
1 = slice
2 = iface
3 = eface
That way, during stack scanning we can check if a string
is zero length or a slice has zero capacity. We can avoid
following the contained pointer in those cases. It is safe
to do so because it can never be dereferenced, and it is
desirable to do so because it may cause false retention
of the following block in memory.
Slice feature turned off until issue 7564 is fixed.
Update #7549
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/76380043
The existing code did not have a clear notion of whether
memory has been actually reserved. It checked based on
whether in 32-bit mode or 64-bit mode and (on GNU/Linux) the
requested address, but it confused the requested address and
the returned address.
LGTM=rsc
R=rsc, dvyukov
CC=golang-codereviews, michael.hudson
https://golang.org/cl/79610043
This the second part of making persistent HTTPS connections to
certain servers (notably Amazon) robust.
See the story in part 1: https://golang.org/cl/76400046/
This is the http Transport change that notes whether our
net.Conn.Read has ever seen an EOF. If it has, then we use
that as an additional signal to not re-use that connection (in
addition to the HTTP response headers)
Fixes#3514
LGTM=rsc
R=agl, rsc
CC=golang-codereviews
https://golang.org/cl/79240044
Update #3514
An io.Reader is permitted to return either (n, nil)
or (n, io.EOF) on EOF or other error.
The tls package previously always returned (n, nil) for a read
of size n if n bytes were available, not surfacing errors at
the same time.
Amazon's HTTPS frontends like to hang up on clients without
sending the appropriate HTTP headers. (In their defense,
they're allowed to hang up any time, but generally a server
hangs up after a bit of inactivity, not immediately.) In any
case, the Go HTTP client tries to re-use connections by
looking at whether the response headers say to keep the
connection open, and because the connection looks okay, under
heavy load it's possible we'll reuse it immediately, writing
the next request, just as the Transport's always-reading
goroutine returns from tls.Conn.Read and sees (0, io.EOF).
But because Amazon does send an AlertCloseNotify record before
it hangs up on us, and the tls package does its own internal
buffering (up to 1024 bytes) of pending data, we have the
AlertCloseNotify in an unread buffer when our Conn.Read (to
the HTTP Transport code) reads its final bit of data in the
HTTP response body.
This change makes that final Read return (n, io.EOF) when
an AlertCloseNotify record is buffered right after, if we'd
otherwise return (n, nil).
A dependent change in the HTTP code then notes whether a
client connection has seen an io.EOF and uses that as an
additional signal to not reuse a HTTPS connection. With both
changes, the majority of Amazon request failures go
away. Without either one, 10-20 goroutines hitting the S3 API
leads to such an error rate that empirically up to 5 retries
are needed to complete an API call.
LGTM=agl, rsc
R=agl, rsc
CC=golang-codereviews
https://golang.org/cl/76400046
The nproc and ndone fields are uint32. This makes the type
consistent.
LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews
https://golang.org/cl/79340044
Strictly speaking, it's not necessary in example_test.go, as the
Rows.Close docs say that "If Next returns false, the Rows are closed
automatically". However, if the for loop breaks or returns early, it's
not obvious that you'll leak unless you explicitly call Rows.Close.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews, rsc
https://golang.org/cl/79330043
Structured Exception Handling (SEH) was the first way to handle
exceptions (memory faults, divides by zero) on Windows.
The S might as well stand for "stack-based": the implementation
interprets stack addresses in a few different ways, and it gets
subtly confused by Go's management of stacks. It's also something
that requires active maintenance during cgo switches, and we've
had bugs in that maintenance in the past.
We have recently come to believe that SEH cannot work with
Go's stack usage. See http://golang.org/issue/7325 for details.
Vectored Exception Handling (VEH) is more like a Unix signal
handler: you set it once for the whole process and forget about it.
This CL drops all the SEH code and replaces it with VEH code.
Many special cases and 7 #ifdefs disappear.
VEH was introduced in Windows XP, so Go on windows/386 will
now require Windows XP or later. The previous requirement was
Windows 2000 or later. Windows 2000 immediately preceded
Windows XP, so Windows 2000 is the only affected version.
Microsoft stopped supporting Windows 2000 in 2010.
See http://golang.org/s/win2000-golang-nuts for details.
Fixes#7325.
LGTM=alex.brainman, r
R=golang-codereviews, alex.brainman, stephen.gutekanst, dave
CC=golang-codereviews, iant, r
https://golang.org/cl/74790043
This has come up twice now. Redirect future questions
to the explanation in the issue tracker.
LGTM=iant, r
R=r, iant
CC=golang-codereviews
https://golang.org/cl/79550043
Currently it's always zero, but that is inconsistent with math.Pow
and also plain wrong.
This is a proposal for how it should be defined.
Fixes#7583.
LGTM=rsc
R=golang-codereviews, iant, gobot, rsc
CC=golang-codereviews
https://golang.org/cl/76940044
ReadFrom should not return until it receives a non-nil error
or too many contiguous (0, nil)s from a given reader.
Currently it immediately returns if it receives one (0, nil).
Fixes#7611.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/76400048
bi is a slice and not an array, so bi[:] does not make much sense.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/79280043
It's a little bit waste to check if r is not a surrogate
code point because RuneError is not a surrogate code point.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/79230043
"min" and "max" in "case '{'" clause are fresh variables.
The variables defined in the outer scope never get value
other than 0.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/78750044
Currently Scan ignores an error returned from source if the number
of bytes source has read is 0.
Fixes#7594.
LGTM=gri
R=golang-codereviews, bradfitz, gri
CC=golang-codereviews
https://golang.org/cl/78120043
Revision 3ae4607a43ff introduced CONVNOP layers
to fix type checking issues arising from comparisons.
The added complexity made 8g run out of registers
when compiling an equality function in go.net/ipv6.
A similar issue occurred in test/sizeof.go on
amd64p32 with 6g.
Fixes#7405.
LGTM=khr
R=rsc, dave, iant, khr
CC=golang-codereviews
https://golang.org/cl/78100044
Encoding.Decode() failed to detect trailing garbages if input contains "==" followed by garbage smaller than 3 bytes (for example, it failed to detect "x" in "AA==x"). This patch fixes the bug and adds a few tests.
LGTM=nigeltao
R=golang-codereviews, bradfitz, nigeltao
CC=golang-codereviews
https://golang.org/cl/75340044
Removes most uses of the REP prefix, which has a high startup cost.
LGTM=iant
R=golang-codereviews, iant, khr
CC=golang-codereviews
https://golang.org/cl/77920043
Rationale:
It already is for scanning.
It is accepted for complexes already, but doesn't work.
It's analogous to %G and %E.
C accepts it too, and we try to be roughly compatible.
Fixes#7518.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/77580044
CL 77580046 caused a data race issue with tests that assumes ReadAt
does not mutate receiver. This patch partially revert CL 77580046
to fix it.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/77900043
These test cases are redundant because TestSimpleFold tests
all possible rotations of test data, so no need to add
rotated strings.
Also updated the comment as it's guaranteed that SimpleFold
returns values in increasing order.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/77730043
UnreadRune should return an error if previous operation is not
ReadRune.
Fixes#7579.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/77580046
Also move generated code into a separate file,
because it's difficult to work with the file otherwise.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews
https://golang.org/cl/76080044
A too large float constant is an error.
A too small float constant is rounded to zero.
Fixes#7419
Update #6902
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/76730046
utf8.RuneLen returns -1 for an invalid rune. In that case we
need to extend the internal buffer at least by 3 for \uFFFD.
Fixes#7577.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/77420044
See testing.FailNow for further information.
Also avoid nil pointer derefernce in TestTransportMaxPerHostIdleConns.
LGTM=dave
R=golang-codereviews, dave
CC=golang-codereviews
https://golang.org/cl/76470043
"nn" can never be zero for any input "p", so no check is needed.
This change should improve readability a bit.
LGTM=nigeltao
R=golang-codereviews, bradfitz, nigeltao
CC=golang-codereviews
https://golang.org/cl/76610045
%q quotes each element of a string slice; this was never explained in the docs.
Fixes#7015.
LGTM=josharian
R=golang-codereviews, josharian
CC=golang-codereviews
https://golang.org/cl/77140044
Request ID reuse is allowed by the FastCGI spec [1]. In particular nginx uses
the same request ID, 1, for all requests on a given connection. Because
serveRequest does not remove the request from conn.requests, this causes it to
treat the second request as a duplicate and drops the connection immediately
after beginRequest. This manifests with nginx option 'fastcgi_keep_conn on' as
the following message in nginx error log:
2014/03/17 01:39:13 [error] 730#0: *109 recv() failed (104: Connection reset by peer) while reading response header from upstream, client: x.x.x.x, server: example.org, request: "GET / HTTP/1.1", upstream: "fastcgi://127.0.0.1:9001", host: "example.org"
Because handleRecord and serveRequest run in different goroutines, access to
conn.requests must now be synchronized.
[1] http://www.fastcgi.com/drupal/node/6?q=node/22#S3.3
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/76800043
I believe the original author of this code just forgot to check for error here.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/76760043
It was using the wrong offset and returned random values
making "runoutput" compiler tests crash.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/76250043
They were rejected by NaCl due to AES instructions and
accesses to %gs:0x8, caused by wrong tlsoffset value.
LGTM=iant
R=rsc, dave, iant
CC=golang-codereviews
https://golang.org/cl/76050044
It's possible that bgsweep constantly does not catch up for some reason,
in this case runfinq was not woken at all.
R=rsc
CC=golang-codereviews
https://golang.org/cl/75940043
The problem was that spans end up in wrong lists after split
(e.g. in h->busy instead of h->central->empty).
Also the span can be non-swept before split,
I don't know what it can cause, but it's safer to operate on swept spans.
Fixes#7544.
R=golang-codereviews, rsc
CC=golang-codereviews, khr
https://golang.org/cl/76160043
Currently processes crash with obscure message.
Say that it's "out of memory".
LGTM=rsc
R=golang-codereviews
CC=golang-codereviews, khr, rsc
https://golang.org/cl/75820045
Old versions of DTrace (as those shipped in OS X and FreeBSD)
don't support unicode characters in symbol names. Replace '·'
to '.' to make DTrace happy.
Fixes#7493
LGTM=aram, rsc
R=aram, rsc, gobot, iant
CC=golang-codereviews
https://golang.org/cl/72280043
The Solaris network poller uses event ports, which are
level-triggered. As such, it has to re-arm itself after each
wakeup. The arming mechanism (which runs in its own thread) raced
with the closing of a file descriptor happening in a different
thread. When a network file descriptor is about to be closed,
the network poller is awaken to give it a chance to remove its
association with the file descriptor. Because the poller always
re-armed itself, it raced with code that closed the descriptor.
This change makes the network poller check before re-arming if
the file descriptor is about to be closed, in which case it will
ignore the re-arming request. It uses the per-PollDesc lock in
order to serialize access to the PollDesc.
This change also adds extensive documentation describing the
Solaris implementation of the network poller.
Fixes#7410.
LGTM=dvyukov, iant
R=golang-codereviews, bradfitz, iant, dvyukov, aram.h, gobot
CC=golang-codereviews
https://golang.org/cl/69190044
debug/elf does the same thing, use []byte{} for
any missing sections.
Fixes#7510
LGTM=rsc
R=golang-codereviews, iant
CC=golang-codereviews, rsc
https://golang.org/cl/75230043
Mark free memory blocks as unused.
On amd64 it allows the process to eat all 128 GB of heap
without killing the machine.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/74070043
This is especially important for SetPanicOnCrash,
but also useful for e.g. nil deref in mallocgc.
Panics on such crashes can't lead to anything useful,
only to deadlocks, hangs and obscure crashes.
This is a copy of broken but already LGTMed
https://golang.org/cl/68540043/
TBR=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/75320043
When we copy stack, we check only new size of the top segment.
This is incorrect, because we can have other segments below it.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rsc
https://golang.org/cl/73980045
Calling runtime·cgocall could trigger a GC in the child while
gclock was held by the parent.
Fixes#7511
LGTM=bradfitz, dvyukov, dave
R=golang-codereviews, bradfitz, dvyukov, dave
CC=golang-codereviews, rsc
https://golang.org/cl/75210044
The lowering to runtime calls introduces hidden pointers to the
arguments of select clauses. When implicit conversions were
involved it could end up with incompatible pointers. Since the
pointed-to types have the same representation, we can introduce a
forced conversion.
Fixes#6847.
LGTM=rsc
R=rsc, iant, khr
CC=golang-codereviews
https://golang.org/cl/72380043
It was using a REP STOSQ but putting in CX the number of 32-bit
words to clear.
LGTM=dave
R=rsc, dave
CC=golang-codereviews
https://golang.org/cl/75240043
Under some circumstances, gccgoToolchain's ld can pass the path of
build outputs that have been deleted to the link command.
Fixes#7303.
LGTM=rsc
R=golang-codereviews, dave, michael.hudson, rsc
CC=golang-codereviews
https://golang.org/cl/61970044
The compilers expect to not be interrupted by floating
point exceptions. On Plan 9, every process starts with
interrupts enabled for invalid operation, stack overflow,
and divide by zero exceptions.
LGTM=rsc
R=rsc, 0intro
CC=golang-codereviews
https://golang.org/cl/72750043
Previously, we wrote "kill" to the process control file
to kill a program. This is problematic because it doesn't
let the program gracefully exit.
This matters especially if the process we're killing is a
Go program. On Unix, sending SIGKILL to a Go program will
automatically kill all runtime threads. On Plan 9, there
are no threads so when the program wants to exit it has to
somehow signal all of the runtime processes. It can't do
this if we mercilessly kill it by writing to it's control
file.
Instead, we now send it a note to invoke it's note handler
and let it perform any cleanup before exiting.
LGTM=rsc
R=rsc, 0intro
CC=golang-codereviews
https://golang.org/cl/74440044
On Plan 9, the kernel disallows the use of floating point
instructions while handling a note. Previously, we worked
around this by using a simple loop in place of memmove.
When I added that work-around, I verified that all paths
from the note handler didn't end up calling memmove. Now
that memclr is using SSE instructions, the same process
will have to be done again.
Instead of doing that, however, this CL just punts and
uses unoptimized functions everywhere on Plan 9.
LGTM=rsc
R=rsc, 0intro
CC=golang-codereviews
https://golang.org/cl/73830044
Acid can't produce a stack trace without .frame symbols.
Of course, it can only unwind through linear stacks but
this is still better than nothing. (I wrote an acid func
to do the full unwind a long time ago but lost it and
haven't worked up the courage to write it again).
Note that these will only be present in the native symbol
table for Plan 9 binaries.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/72450045
syscall.naclWrite was missing from sys_nacl_386.s
This gets ./make.bash passing, but doesn't pass validation. I'm not sure if this is the fault of this change, or validation was broken anyway.
LGTM=rsc
R=minux.ma, rsc
CC=golang-codereviews
https://golang.org/cl/74510043
1. Fix the bug that shrinkstack returns memory to heap.
This causes growslice to misbehave (it manually initialized
blocks, and in efence mode shrinkstack's free leads to
partially-initialized blocks coming out of growslice.
Which in turn causes GC to crash while treating the garbage
as Eface/Iface.
2. Enable efence for stack segments.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews, khr
https://golang.org/cl/74080043
The garbage collector uses type information to guide the
traversal of the heap. If it sees a field that should be a string,
it marks the object pointed at by the string data pointer as
visited but does not bother to look at the data, because
strings contain bytes, not pointers.
If you save s[len(s):] somewhere, though, the string data pointer
actually points just beyond the string data; if the string data
were exactly the size of an allocated block, the string data
pointer would actually point at the next block. It is incorrect
to mark that next block as visited and not bother to look at
the data, because the next block may be some other type
entirely.
The fix is to ignore strings with zero length during collection:
they are empty and can never become non-empty: the base
pointer will never be used again. The handling of slices already
does this (but using cap instead of len).
This was not a bug in Go 1.2, because until January all string
allocations included a trailing NUL byte not included in the
length, so s[len(s):] still pointed inside the string allocation
(at the NUL).
This bug was causing the crashes in test/run.go. Specifically,
the parsing of a regexp in package regexp/syntax allocated a
[]syntax.Inst with rounded size 1152 bytes. In fact it
allocated many such slices, because during the processing of
test/index2.go it creates thousands of regexps that are all
approximately the same complexity. That takes a long time, and
test/run works on other tests in other goroutines. One such
other test is chan/perm.go, which uses an 1152-byte source
file. test/run reads that file into a []byte and then calls
strings.Split(string(src), "\n"). The string(src) creates an
1152-byte string - and there's a very good chance of it
landing next to one of the many many regexp slices already
allocated - and then because the file ends in a \n,
strings.Split records the tail empty string as the final
element in the slice. A garbage collection happens at this
point, the collection finds that string before encountering
the []syntax.Inst data it now inadvertently points to, and the
[]syntax.Inst data is not scanned for the pointers that it
contains. Each syntax.Inst contains a []rune, those are
missed, and the backing rune arrays are freed for reuse. When
the regexp is later executed, the runes being searched for are
no longer runes at all, and there is no match, even on text
that should match.
On 64-bit machines the pointer in the []rune inside the
syntax.Inst is larger (along with a few other pointers),
pushing the []syntax.Inst backing array into a larger size
class, avoiding the collision with chan/perm.go's
inadvertently sized file.
I expect this was more prevalent on OS X than on Linux or
Windows because those managed to run faster or slower and
didn't overlap index2.go with chan/perm.go as often. On the
ARM systems, we only run one errorcheck test at a time, so
index2 and chan/perm would never overlap.
It is possible that this bug is the root cause of other crashes
as well. For now we only know it is the cause of the test/run crash.
Many thanks to Dmitriy for help debugging.
Fixes#7344.
Fixes#7455.
LGTM=r, dvyukov, dave, iant
R=golang-codereviews, dave, r, dvyukov, delpontej, iant
CC=golang-codereviews, khr
https://golang.org/cl/74250043
debuglive >= 1 is not the condition under which we
start recording messages (we avoid printing for
init functions even if debuglive is set).
LGTM=bradfitz, iant
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, khr
https://golang.org/cl/74390043
For now Note, futexsleep and futexwakeup are designed for threads,
not for processes. The explicit use of UMTX_OP_WAIT_UINT_PRIVATE and
UMTX_OP_WAKE_PRIVATE can avoid unnecessary traversals of VM objects,
to hit undiscovered bugs related to VM system on SMP/SMT/NUMA
environment.
Update #7496
LGTM=iant
R=golang-codereviews, gobot, iant, bradfitz
CC=golang-codereviews
https://golang.org/cl/72760043
Solaris doesn't have struct ip_mreqn, instead it uses struct ip_mreq
and struct group_req with struct sockaddr_storage.
Also fixes incorrect SockaddrDatalink.
Update #7399
LGTM=aram, iant
R=golang-codereviews, aram, gobot, iant
CC=golang-codereviews
https://golang.org/cl/73920043
This CL is a reformulation of CL 73110043 containing only the minimum required to get the nacl builds compiling.
LGTM=bradfitz
R=rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/74220043
The comment for 'Clean' function is prepended with spaces instead of
a single tab, resulting in visually misaligned comment in the generated
documentation.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/73840043
The change to signal_amd64.c from CL 15790043 was not merged correctly.
This CL reapplies the change, renaming the file to signal_amd64x.c and adds the appropriate build tags.
LGTM=iant, bradfitz
R=rsc, iant, bradfitz
CC=golang-codereviews
https://golang.org/cl/72790043
The byte that r is or'd into is already 0x7, so the failure to zero r only
impacts the generated machine code if the register is > 7.
Fixes#7044.
LGTM=dave, minux.ma, rsc
R=dave, minux.ma, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/73730043
Spans are now private to threads, and the loop
is removed from all other functions.
Remove it from marknogc for consistency.
LGTM=khr, rsc
R=golang-codereviews, bradfitz, khr
CC=golang-codereviews, khr, rsc
https://golang.org/cl/72520043
Not many windows users have perl installed. They can just use
standard go tools instead. Also mkerrors_windows.sh script
removed - we don't add any new "unix" errors to windows
syscall package anymore.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/41060044
Thanks to Ian for spotting these.
runtime.h: define uintreg correctly.
stack.c: address warning caused by the type of uintreg being 32 bits on amd64p32.
Commentary (mainly for my own use)
nacl/amd64p32 defines a machine with 64bit registers, but address space is limited to a 4gb window (the window is placed randomly inside the full 48 bit virtual address space of a process). To cope with this 6c defines _64BIT and _64BITREG.
_64BITREG is always defined by 6c, so both GOARCH=amd64 and GOARCH=amd64p32 use 64bit wide registers.
However _64BIT itself is only defined when 6c is compiling for amd64 targets. The definition is elided for amd64p32 environments causing int, uint and other arch specific types to revert to their 32bit definitions.
LGTM=iant
R=iant, rsc, remyoudompheng
CC=golang-codereviews
https://golang.org/cl/72860046
gc does not report this as an error, but go/types does.
(I suspect that constructing a closure counts as a reference
to &all in gc's implementation).
This is not a tool bug, since the spec doesn't require
implementations to implement this check, but it does
illustrate that dialect variations are always a nuisance.
LGTM=rsc, bradfitz
R=bradfitz
CC=golang-codereviews, gri, rsc
https://golang.org/cl/73850043
Previously, passing a long duration to ParseDuration could result in
random, even negative, values.
LGTM=r
R=golang-codereviews, bradfitz, r
CC=golang-codereviews
https://golang.org/cl/72120043
CL 69340044 requires that syscall.SO_ERROR be defined on all unix like platforms. Add SO_ERROR to the list of dummy constants in sycall/net_nacl.go.
LGTM=bradfitz
R=iant, rsc, mikioh.mikioh, bradfitz
CC=golang-codereviews
https://golang.org/cl/73100043
If we report a leak, make sure we've waited long enough to be sure.
The new sleep regimen waits 1.05 seconds before failing; the old
one waited 0.005 seconds.
(The single linux/amd64 failure in this test feels more like a
timing problem than a leak. I don't want to spend time on it unless
we're sure.)
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/72630043
We provide amd64p32 implementations for md5 and sha1 so we need to exclude amd64p32 from the generic implementations in those packages.
Fixes build once CL 72360044 lands.
LGTM=agl, remyoudompheng
R=rsc, bradfitz, agl, remyoudompheng
CC=golang-codereviews
https://golang.org/cl/72460043
This fixes the following amd64p32 issue:
pkg/time/format.go:724: internal compiler error: twobitwalktype1: invalid initial alignment, Time
caused by the pointer zone ending on a 32-bit-aligned boundary.
LGTM=rsc
R=rsc, dave
CC=golang-codereviews
https://golang.org/cl/72270046
This code being buggy is the only explanation I can come up
with for issue 7325. It's probably not, but the only alternative
is a Windows kernel bug. Comment this out to see what breaks
or gets fixed.
Update #7325
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/72590044
From the trace it appears that stackalloc is being
called with 0x1800 which is 6k = 4k + (StackSystem=2k).
Make StackSystem 4k too, to make stackalloc happy.
It's already 4k on windows/amd64.
TBR=khr
CC=golang-codereviews
https://golang.org/cl/72600043
It was using MOVL to pass a 64-bit argument
(concatenated framesize and argsize) to morestack11.
LGTM=dave, rsc
R=dave, rsc, iant
CC=golang-codereviews
https://golang.org/cl/72360044
There are at least 3 bugs:
1. g->stacksize accounting is broken during copystack/shrinkstack
2. stktop->free is not properly maintained during copystack/shrinkstack
3. stktop->free logic is broken:
we can have stktop->free==FixedStack,
and we will free it into stack cache,
but it actually comes from heap as the result of non-copying segment shrink
This shows as at least spurious races on race builders (maybe something else as well I don't know).
The idea behind the refactoring is to consolidate stacksize and
segment origin logic in stackalloc/stackfree.
Fixes#7490.
LGTM=rsc, khr
R=golang-codereviews, rsc, khr
CC=golang-codereviews
https://golang.org/cl/72440043
Recursive panics leave dangling Panic structs in g->panic stack.
At best it leads to a Defer leak and incorrect output on a subsequent panic.
At worst it arbitrary corrupts heap.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/72480043
One reason the sync.Pool finalizer test can fail is that
this function's ef1 contains uninitialized data that just
happens to point at some of the old pool. I've seen this cause
retention of a single pool cache line (32 elements) on arm.
Really we need liveness information for C functions, but
for now we can be more careful about data in long-lived
C functions that block.
LGTM=bradfitz, dvyukov
R=golang-codereviews, bradfitz, dvyukov
CC=golang-codereviews, iant, khr
https://golang.org/cl/72490043
Replaces CL 70000043.
Switch to the amd64p32 linker model if we are building under nacl/amd64p32.
No need to introduce linkarchinit() as 6a contains its own main() function.
LGTM=rsc
R=rsc, minux.ma
CC=golang-codereviews
https://golang.org/cl/72020043
Replaces CL 70000043.
Introduce linkarchinit() from cmd/ld.
For cmd/6g, switch to the amd64p32 linker model if we are building under nacl/amd64p32.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/71330045
Instead, split the underlying storage in half and
free just half of it.
Shrinking without copying lets us reclaim storage used
by a previously profligate Go routine that has now blocked
inside some C code.
To shrink in place, we need all stacks to be a power of 2 in size.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/69580044
This is a test case for CL 34680044.
Fixes#6333.
LGTM=bradfitz
R=golang-codereviews, bradfitz, minux.ma
CC=golang-codereviews
https://golang.org/cl/71230049
Two memory allocator bug fixes.
- efence is not maintaining the proper heap metadata
to make eventual memory reuse safe, so use SysFault.
- now that our heap PageSize is 8k but most hardware
uses 4k pages, SysAlloc and SysReserve results must be
explicitly aligned. Do that in a few more call sites and
document this fact in malloc.h.
Fixes#7448.
LGTM=iant
R=golang-codereviews, josharian, iant
CC=dvyukov, golang-codereviews
https://golang.org/cl/71750048
Replaces CL 70000043.
Introduce linkarchinit() from cmd/ld.
For cmd/6c, switch to the amd64p32 linker model if we are building under nacl/amd64p32.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/72010043
I've just needed the G status on fault to debug runtime bug.
For some reason we print everything except header here.
Make it more informative and consistent.
R=rsc
CC=golang-codereviews
https://golang.org/cl/67870056
Implement custom assembly thunks for hot race calls (memory accesses and function entry/exit).
The thunks extract caller pc, verify that the address is in heap or global and switch to g0 stack.
Before:
ok regexp 3.692s
ok compress/bzip2 9.461s
ok encoding/json 6.380s
After:
ok regexp 2.229s (-40%)
ok compress/bzip2 4.703s (-50%)
ok encoding/json 3.629s (-43%)
For comparison, normal non-race build:
ok regexp 0.348s
ok compress/bzip2 0.304s
ok encoding/json 0.661s
Race build:
ok regexp 2.229s (+540%)
ok compress/bzip2 4.703s (+1447%)
ok encoding/json 3.629s (+449%)
Also removes some race-related special cases from cgocall and scheduler.
In long-term it will allow to remove cyclic runtime/race dependency on cmd/cgo.
Fixes#4249.
Fixes#7460.
Update #6508
Update #6688
R=iant, rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/55100044
Tested GOARM=6 on Raspberry Pi, and I found only a few tests that
use sub-normal numbers fails. I have a patch to NetBSD kernel pending
that fixes this issue (NetBSD kernel doesn't allow us to disable the
Flush-to-Zero feature).
LGTM=jsing
R=golang-codereviews, jsing
CC=golang-codereviews
https://golang.org/cl/70730043
Essentialy for running tests without a working cmd/go.
While we're at it, also fix a typo.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/70640043
During the glob decoding process interface values are set to concrete
values after a test for assignability. If the assignability test fails
a slightly vague error message is produced. While technically accurate
the error message does not clearly describe the problem.
Rewrite the error message to include the usage of the word assignable,
which makes it clear the concrete value type is not assignable to the
interface value type.
Fixes#6467.
LGTM=r
R=golang-codereviews, rsc, r
CC=golang-codereviews
https://golang.org/cl/71590043
Recently NetBSD starts to enforce this, and refuses to execute
the program if n is larger than the sum of entry sizes.
Before:
$ readelf -n ../bin/go.old
Notes at offset 0x00000bd0 with length 0x00000019:
Owner Data size Description
NetBSD 0x00000004 NT_VERSION (version)
readelf: Warning: corrupt note found at offset 18 into core notes
readelf: Warning: type: 0, namesize: 00000000, descsize: 00000000
$ readelf -n ../bin/go
Notes at offset 0x00000bd0 with length 0x00000018:
Owner Data size Description
NetBSD 0x00000004 NT_VERSION (version)
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/70710043
Add CgoFiles to the covered files when building
with cover support.
LGTM=rsc
R=golang-codereviews, gobot, r, rsc
CC=golang-codereviews
https://golang.org/cl/34680044
Apparently, the Windows routines sometimes fail to generate output.
Copy the Unix stdio-based implementations instead.
Suggested by Pietro Gagliardi in CL 65280043 but that CL
seems to have been abandoned.
Fixes#7242.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/71550044
32-bit Windows uses "structured exception handling" (SEH) to
handle hardware faults: that there is a per-thread linked list
of fault handlers maintained in user space instead of
something like Unix's signal handlers. The structures in the
linked list are required to live on the OS stack, and the
usual discipline is that the function that pushes a record
(allocated from the current stack frame) onto the list pops
that record before returning. Not to pop the entry before
returning creates a dangling pointer error: the list head
points to a stack frame that no longer exists.
Go pushes an SEH record in the top frame of every OS thread,
and that record suffices for all Go execution on that thread,
at least until cgo gets involved.
If we call into C using cgo, that called C code may push its
own SEH records, but by the convention it must pop them before
returning back to the Go code. We assume it does, and that's
fine.
If the C code calls back into Go, we want the Go SEH handler
to become active again, not whatever C has set up. So
runtime.callbackasm1, which handles a call from C back into
Go, pushes a new SEH record before calling the Go code and
pops it when the Go code returns. That's also fine.
It can happen that when Go calls C calls Go like this, the
inner Go code panics. We allow a defer in the outer Go to
recover the panic, effectively wiping not only the inner Go
frames but also the C calls. This sequence was not popping the
SEH stack up to what it was before the cgo calls, so it was
creating the dangling pointer warned about above. When
eventually the m stack was used enough to overwrite the
dangling SEH records, the SEH chain was lost, and any future
panic would not end up in Go's handler.
The bug in TestCallbackPanic and friends was thus creating a
situation where TestSetPanicOnFault - which causes a hardware
fault - would not find the Go fault handler and instead crash
the binary.
Add checks to TestCallbackPanicLocked to diagnose the mistake
in that test instead of leaving a bad state for another test
case to stumble over.
Fix bug by restoring SEH chain during deferred "endcgo"
cleanup.
This bug is likely present in Go 1.2.1, but since it depends
on Go calling C calling Go, with the inner Go panicking and
the outer Go recovering the panic, it seems not important
enough to bother fixing before Go 1.3. Certainly no one has
complained.
Fixes#7470.
LGTM=alex.brainman
R=golang-codereviews, alex.brainman
CC=golang-codereviews, iant, khr
https://golang.org/cl/71440043
Regenerate z-files for DragonFly BSD 3.6.
F_DUP_FD_CLOEXEC is now supported, so remove the zero value constant
from types_dragonfly.go so that we use the generated value from the
z-files.
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/70080047
The format of the DragonFly BSD syscalls.master file has changed
slightly - update mksysnum_dragonfly.pl to match.
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/71460044
Disable the "udp" to IPv6 unicast address on the loopback interface
test under DragonFly BSD. This currently returns a local address of
0.0.0.1, rather than an IPv6 address with zone identifier.
Update #7473
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/71500044
Performing multiple connect system calls on a non-blocking socket
under DragonFly BSD does not necessarily result in errors from earlier
connect calls being returned, particularly if we are connecting to
localhost. Instead, once netpoll tells us that the socket is ready,
get the SO_ERROR socket option to see if the connection succeeded
or failed.
Fixes#7474
LGTM=mikioh.mikioh
R=mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/69340044
Added test cases and expanded test harness to handle token end
positions.
Also: Make sure token end positions are never outside the valid
position range, as was possible in case of parse errors.
Fixes#7458.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/70190046
the crypto/tls revision d3d43f270632 (CL 67010043, requiring ServerName or InsecureSkipVerify) breaks net/smtp,
since it seems impossible to do SMTP via TLS anymore. i've tried to fix this by simply using a tls.Config with
ServerName, instead of a nil *tls.Config. without this fix, doing SMTP with TLS results in error "tls: either
ServerName or InsecureSkipVerify must be specified in the tls.Config".
testing: the new method TestTlsClient(...) sets up a skeletal smtp server with tls capability, and test client
injects a "fake" certificate allowing tls to work on localhost; thus, the modification to SendMail(...) enabling
this.
Fixes#7437.
LGTM=bradfitz
R=golang-codereviews, josharian, bradfitz
CC=golang-codereviews
https://golang.org/cl/70380043
The arm puts the text flags in a different place
than the other architectures. This needs to be
cleaned up.
TBR=minux
CC=golang-codereviews
https://golang.org/cl/71260043
The network connection dies differently from the server's
perspective on (some) Windows when the client goes away. Match
on the common prefix (common to Unix and Windows) instead of
the network error part.
Fixes#7456
LGTM=josharian
R=golang-codereviews, josharian
CC=alex.brainman, golang-codereviews, iant
https://golang.org/cl/70010050
For non-closure functions, the context register is uninitialized
on entry and will not be used, but morestack saves it and then the
garbage collector treats it as live. This can be a source of memory
leaks if the context register points at otherwise dead memory.
Avoid this by introducing a parallel set of morestack functions
that clear the context register, and use those for the non-closure functions.
I hope this will help with some of the finalizer flakiness, but it probably won't.
Fixes#7244.
LGTM=dvyukov
R=khr, dvyukov
CC=golang-codereviews
https://golang.org/cl/71030044
I have one machine where this 25 test run is flaky
and fails ("21 >= 21"), but 50 works everywhere.
LGTM=josharian
R=josharian
CC=golang-codereviews
https://golang.org/cl/67870053
It mentioned true and false for error values. Instead, just
don't mention the error semantics, as they match normal Go
conventions (if error is non-nil, the other value is
meaningless). We generally only document error values when
they're interesting (where non-nil, non-nil is valid, or the
error value can be certain known values or types).
Fixes#7464
LGTM=agl
R=agl
CC=golang-codereviews
https://golang.org/cl/68440044
The flakiness appears to be just in tests, not in the actual code.
Specifically, the many tests call runtime.GC once and expect that
the finalizers will be running in the background when GC returns.
Now that the sweep phase is concurrent with execution, however,
the finalizers will not be run until sweep finishes, which might
be quite a bit later. To force sweep to finish, implement runtime.GC
by calling the actual collection twice. The second will complete the
sweep from the first.
This was reliably broken after a few runs before the CL and now
passes tens of runs:
while GOMAXPROCS=2 ./runtime.test -test.run=Finalizer -test.short \
-test.timeout=300s -test.cpu=$(perl -e 'print ("1,2,4," x 100) . "1"')
do true; done
Fixes#7328.
LGTM=dvyukov
R=dvyukov, dave
CC=golang-codereviews
https://golang.org/cl/71080043
The exception handler runs on the ordinary g stack,
and the stack copier is now trying to do a traceback
across it. That's never been needed before, so it was
unimplemented. Implement it, in all its ugliness.
Fixes windows/amd64 build.
TBR=khr
CC=golang-codereviews
https://golang.org/cl/71030043
1) Move StateHijacked callback earlier, to make it
panic-proof. A Hijack followed by a panic didn't previously
result in ConnState getting fired for StateHijacked. Move it
earlier, to the time of hijack.
2) Don't fire StateActive unless any bytes were read off the
wire while waiting for a request. This means we don't
transition from New or Idle to Active if the client
disconnects or times out. This was documented before, but not
implemented properly.
This CL is required for an pending fix for Issue 7264
LGTM=josharian
R=josharian
CC=golang-codereviews
https://golang.org/cl/69860049
Unfortunately FreeBSD 10 has changed its syscall arguments for
some reasons but as per request at golang-dev this CL does not
generate some structures, syscall numbers and constants as
compatible to FreeBSD 10 as follows:
Structure: Stat_t, IfData, IfMsghdr are based on 8-STABLE
Syscall number: Capsicum API is based on 9-STABLE
Constant: IFT_CARP, SIOCAIFADDR, SIOCSIFPHYADDR are based on 9-STABLE
Update #7193
FreeBSD 10 breaking changes:
r205792: Rename st_*timespec fields to st_*tim for POSIX 2008
compliance.
http://svnweb.freebsd.org/base?view=revision&revision=205792
r254804: Restructure the mbuf pkthdr to make it fit for upcoming
capabilities and features.
http://svnweb.freebsd.org/base?view=revision&revision=254804
r255219: Change the cap_rights_t type from uint64_t to a structure
that we can extend in the future in a backward compatible (API and
ABI) way.
http://svnweb.freebsd.org/base?view=revision&revision=255219
LGTM=iant
R=golang-codereviews, rsc, minux.ma, gobot, iant
CC=golang-codereviews
https://golang.org/cl/56770044
In Go 1.2, closing a request body without reading to EOF
causes the underlying TCP connection to not be reused. This
client code following redirects was never updated when that
happened.
This was part of a previous CL but moved to its own CL at
Josh's request. Now with test too.
LGTM=josharian
R=josharian
CC=golang-codereviews
https://golang.org/cl/70800043
Currently a write error will cause future reads to return that same error.
However, there may have been extra information from a peer pending on
the read direction that is now unavailable.
This change splits the single connErr into errors for the read, write and
handshake. (Splitting off the handshake error is needed because both read
and write paths check the handshake error.)
Fixes#7414.
LGTM=bradfitz, r
R=golang-codereviews, r, bradfitz
CC=golang-codereviews
https://golang.org/cl/69090044
warning: src/cmd/ld/pcln.c:184 more arguments than format INT
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/69870047
The addition of Server.ConnState provides all the necessary
hooks to stop a Server gracefully, but StateNew previously
could fire concurrently with Serve exiting (as it does when
its net.Listener is closed). This previously meant one
couldn't use a WaitGroup incremented in the StateNew hook
along with calling Wait after Serve. Now you can.
Update #4674
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/70410044
