Rui Ueyama [Thu, 27 Mar 2014 21:35:07 +0000 (17:35 -0400)]
misc/emacs: do not highlight built-in function if not followed by '('
Name of built-in function is not reserved word in Go, and you can
use it as variable name. "new" is often used as local variable, for
instance.
This patch is to apply font-lock-builtin-face only when built-in
function name is followed by '(', so that it doesn't highlight
non-function variable that happen to have the same name as built-in
function.
Daniel Morsing [Thu, 27 Mar 2014 20:23:16 +0000 (20:23 +0000)]
cmd/cgo: enforce typing of 0-sized types
cgo represents all 0-sized and unsized types internally as [0]byte. This means that pointers to incomplete types would be interchangable, even if given a name by typedef.
Rui Ueyama [Thu, 27 Mar 2014 19:22:52 +0000 (15:22 -0400)]
misc/emacs: handle backslash in raw string in Emacs 23
Go-mode in Emacs 23 does not recognize a backslash followed
by a backquote as end of raw string literal, as it does not
support syntax-propertize-function which Go-mode uses to
remove special meaning from backslashes in ``.
This patch provides a fallback mechanism to do the same thing
using font-lock-syntactic-keywords, which is supported by
Emacs 23.
Russ Cox [Thu, 27 Mar 2014 18:06:15 +0000 (14:06 -0400)]
runtime: enable 'bad pointer' check during garbage collection of Go stack frames
This is the same check we use during stack copying.
The check cannot be applied to C stack frames, even
though we do emit pointer bitmaps for the arguments,
because (1) the pointer bitmaps assume all arguments
are always live, not true of outputs during the prologue,
and (2) the pointer bitmaps encode interface values as
pointer pairs, not true of interfaces holding integers.
For the rest of the frames, however, we should hold ourselves
to the rule that a pointer marked live really is initialized.
The interface scanning already implicitly checks this
because it interprets the type word as a valid type pointer.
This may slow things down a little because of the extra loads.
Or it may speed things up because we don't bother enqueuing
nil pointers anymore. Enough of the rest of the system is slow
right now that we can't measure it meaningfully.
Enable for now, even if it is slow, to shake out bugs in the
liveness bitmaps, and then decide whether to turn it off
for the Go 1.3 release (issue 7650 reminds us to do this).
The new m->traceback field lets us force printing of fp=
values on all goroutine stack traces when we detect a
bad pointer. This makes it easier to understand exactly
where in the frame the bad pointer is, so that we can trace
it back to a specific variable and determine what is wrong.
Russ Cox [Thu, 27 Mar 2014 18:05:57 +0000 (14:05 -0400)]
cmd/gc: liveness-related bug fixes
1. On entry to a function, only zero the ambiguously live stack variables.
Before, we were zeroing all stack variables containing pointers.
The zeroing is pretty inefficient right now (issue 7624), but there are also
too many stack variables detected as ambiguously live (issue 7345),
and that must be addressed before deciding how to improve the zeroing code.
(Changes in 5g/ggen.c, 6g/ggen.c, 8g/ggen.c, gc/pgen.c)
Fixes #7647.
2. Make the regopt word-based liveness analysis preserve the
whole-variable liveness property expected by the garbage collection
bitmap liveness analysis. That is, if the regopt liveness decides that
one word in a struct needs to be preserved, make sure it preserves
the entire struct. This is particularly important for multiword values
such as strings, slices, and interfaces, in which all the words need
to be present in order to understand the meaning.
(Changes in 5g/reg.c, 6g/reg.c, 8g/reg.c.)
Fixes #7591.
3. Make the regopt word-based liveness analysis treat a variable
as having its address taken - which makes it preserved across
all future calls - whenever n->addrtaken is set, for consistency
with the gc bitmap liveness analysis, even if there is no machine
instruction actually taking the address. In this case n->addrtaken
is incorrect (a nicer way to put it is overconservative), and ideally
there would be no such cases, but they can happen and the two
analyses need to agree.
(Changes in 5g/reg.c, 6g/reg.c, 8g/reg.c; test in bug484.go.)
Fixes crashes found by turning off "zero everything" in step 1.
4. Remove spurious VARDEF annotations. As the comment in
gc/pgen.c explains, the VARDEF must immediately precede
the initialization. It cannot be too early, and it cannot be too late.
In particular, if a function call sits between the VARDEF and the
actual machine instructions doing the initialization, the variable
will be treated as live during that function call even though it is
uninitialized, leading to problems.
(Changes in gc/gen.c; test in live.go.)
Fixes crashes found by turning off "zero everything" in step 1.
5. Do not treat loading the address of a wide value as a signal
that the value must be initialized. Instead depend on the existence
of a VARDEF or the first actual read/write of a word in the value.
If the load is in order to pass the address to a function that does
the actual initialization, treating the load as an implicit VARDEF
causes the same problems as described in step 4.
The alternative is to arrange to zero every such value before
passing it to the real initialization function, but this is a much
easier and more efficient change.
(Changes in gc/plive.c.)
Fixes crashes found by turning off "zero everything" in step 1.
6. Treat wide input parameters with their address taken as
initialized on entry to the function. Otherwise they look
"ambiguously live" and we will try to emit code to zero them.
(Changes in gc/plive.c.)
Fixes crashes found by turning off "zero everything" in step 1.
7. An array of length 0 has no pointers, even if the element type does.
Without this change, the zeroing code complains when asked to
clear a 0-length array.
(Changes in gc/reflect.c.)
Russ Cox [Thu, 27 Mar 2014 18:05:31 +0000 (14:05 -0400)]
cmd/dist: zero output variables on entry to goc2c functions
Zeroing the outputs makes sure that during function calls
in those functions we do not let the garbage collector
treat uninitialized values as pointers.
The garbage collector may still see uninitialized values
if a preemption occurs during the function prologue,
before the zeroing has had a chance to run.
This reduces the number of 'bad pointer' messages when
that runtime check is enabled, but it doesn't fix all of them,
so the check is still disabled.
It will also avoid leaks, although I doubt any of these were
particularly serious.
Russ Cox [Thu, 27 Mar 2014 18:05:14 +0000 (14:05 -0400)]
regexp/syntax: remove InstLast
This was added by the one-pass CL (post Go 1.2)
so it can still be removed.
Removing because surely there will be new operations
added later, and we can't change the constant value
once we define it, so "last" is a bad concept to expose.
Erik Westrup [Wed, 26 Mar 2014 22:23:31 +0000 (15:23 -0700)]
cmd/go: Use exported CgoLDFlags when compiler=gccgo
If you compile a program that has cgo LDFLAGS directives, those are exported to an environment variable to be used by subsequent compiler tool invocations. The linking phase when using the gccgo toolchain did not consider the envvar CGO_LDFLAGS's linking directives resulting in undefined references when using cgo+gccgo.
Dmitriy Vyukov [Wed, 26 Mar 2014 15:06:15 +0000 (19:06 +0400)]
runtime: eliminate false retention due to m->moreargp/morebuf
m->moreargp/morebuf were not cleared in case of preemption and stack growing,
it can lead to persistent leaks of large memory blocks.
It seems to fix the sync.Pool finalizer failures. I've run the test 500'000 times
w/o a single failure; previously it would fail dozens of times.
Dmitriy Vyukov [Wed, 26 Mar 2014 15:05:48 +0000 (19:05 +0400)]
runtime: support channel-based mutex in race detector
Update channel race annotations to support change in
cl/75130045: doc: allow buffered channel as semaphore without initialization
The new annotations are added only for channels with capacity 1.
Strictly saying it's possible to construct a counter-example that
will produce a false positive with capacity > 1. But it's hardly can
lead to false positives in real programs, at least I would like to see such programs first.
Any additional annotations also increase probability of false negatives,
so I would prefer to add them lazily.
Dmitriy Vyukov [Wed, 26 Mar 2014 11:11:36 +0000 (15:11 +0400)]
runtime: fix yet another race in bgsweep
Currently it's possible that bgsweep finishes before all spans
have been swept (we only know that sweeping of all spans has *started*).
In such case bgsweep may fail wake up runfinq goroutine when it needs to.
finq may still be nil at this point, but some finalizers may be queued later.
Make bgsweep to wait for sweeping to *complete*, then it can decide
whether it needs to wake up runfinq for sure.
Update #7533
Dmitriy Vyukov [Wed, 26 Mar 2014 11:03:58 +0000 (15:03 +0400)]
runtime: minor improvement of string scanning
If we set obj, then it will be enqueued for marking at the end of the scanning loop.
This is not necessary, since we've already marked it.
This can wait for 1.4 if you wish.
Dave Cheney [Wed, 26 Mar 2014 04:30:55 +0000 (15:30 +1100)]
cmd/go: ensure external test files are presented to the linker first
Fixes #7627.
CL 61970044 changed the order in which .a files are passed to gccgo's link phase. However by reversing the order it caused gccgo to complain if both internal (liba.a) and external (liba_test.a) versions of a package were presented as the former would not contain all the necessary symbols, and the latter would duplicate symbols already defined.
This change ensures that all 'fake' targets remain at the top of the final link order which should be fine as a package compiled as an external test is a superset of its internal sibling.
Looking at how gcToolchain links tests I think this change now accurately mirrors those actions which present $WORK/_test before $WORK in the link order.
Rob Pike [Wed, 26 Mar 2014 02:56:16 +0000 (13:56 +1100)]
doc/go1.3.html: new release document outline
Almost all TODOS, but the structure is there and it has the details
from go1.3.txt, which is hereby deleted.
David Barnett [Wed, 26 Mar 2014 02:51:16 +0000 (13:51 +1100)]
misc/vim: Disable automatic line wrapping by textwidth.
If someone configures a 'textwidth' in go files, vim will by default insert
newlines into long lines as you type, which breaks syntax and doesn't really
make sense for go code. This fixes the default.
Mikio Hara [Mon, 24 Mar 2014 17:56:37 +0000 (02:56 +0900)]
net: deflake TestTCPConcurrentAccept
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.
Mikio Hara [Mon, 24 Mar 2014 17:56:10 +0000 (02:56 +0900)]
net: avoid multiple calling of syscall connect on Unix variants
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.
Brad Fitzpatrick [Tue, 25 Mar 2014 22:19:58 +0000 (15:19 -0700)]
net/http: disable recently-introduced flaky test on Windows
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.
Keith Randall [Tue, 25 Mar 2014 21:11:34 +0000 (14:11 -0700)]
runtime: redo stack map entries to avoid false retention
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.
Ian Lance Taylor [Tue, 25 Mar 2014 20:22:19 +0000 (13:22 -0700)]
runtime: accurately record whether heap memory is reserved
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.
Brad Fitzpatrick [Tue, 25 Mar 2014 17:59:09 +0000 (10:59 -0700)]
net/http: don't re-use Transport connections if we've seen an EOF
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)
Brad Fitzpatrick [Tue, 25 Mar 2014 17:58:35 +0000 (10:58 -0700)]
crypto/tls: make Conn.Read return (n, io.EOF) when EOF is next in buffer
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.
Nigel Tao [Tue, 25 Mar 2014 02:32:18 +0000 (13:32 +1100)]
database/sql: add "defer rows.Close()" to the example code.
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.
Russ Cox [Tue, 25 Mar 2014 01:22:16 +0000 (21:22 -0400)]
runtime: use VEH, not SEH, for windows/386 exception handling
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
Rob Pike [Tue, 25 Mar 2014 00:25:20 +0000 (11:25 +1100)]
math/cmplx: define Pow(0, x) for problematic values of x.
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.
Russ Cox [Mon, 24 Mar 2014 23:11:21 +0000 (19:11 -0400)]
doc: allow buffered channel as semaphore without initialization
This rule not existing has been the source of many discussions
on golang-dev and on issues. We have stated publicly that it is
true, but we have never written it down. Write it down.
Rui Ueyama [Mon, 24 Mar 2014 18:48:34 +0000 (11:48 -0700)]
bufio: fix bug that ReadFrom stops before EOF or error
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.
David du Colombier [Fri, 21 Mar 2014 18:26:47 +0000 (19:26 +0100)]
cmd/ld: fix warnings on Plan 9
warning: src/cmd/ld/macho.c:595 sign-extended character constant
warning: src/cmd/ld/macho.c:595 sign-extended character constant
warning: src/cmd/ld/symtab.c:63 sign-extended character constant
warning: src/cmd/ld/symtab.c:63 sign-extended character constant
Rob Pike [Fri, 21 Mar 2014 02:59:30 +0000 (13:59 +1100)]
doc/go_faq.html: update description of stack management
They aren't segmented any more, at least with gc.
Also improve the comparison of goroutines and threads.
Fixes #7373.
Rémy Oudompheng [Thu, 20 Mar 2014 21:22:37 +0000 (22:22 +0100)]
cmd/6g, cmd/8g: skip CONVNOP nodes in bgen.
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.
Rui Ueyama [Thu, 20 Mar 2014 05:00:34 +0000 (16:00 +1100)]
base64: fix bug that decoder fails to detect corruption
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.
Rob Pike [Wed, 19 Mar 2014 21:51:06 +0000 (08:51 +1100)]
fmt: make %F a synonym for %f
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.
Dmitriy Vyukov [Wed, 19 Mar 2014 13:04:51 +0000 (17:04 +0400)]
runtime: fix stack split detection around fork
If runtime_BeforeFork splits stack, it will unsplit it
with spoiled g->stackguard. It leads to check failure in oldstack:
Rick Arnold [Tue, 18 Mar 2014 02:03:03 +0000 (13:03 +1100)]
build: fix race in doc/articles/wiki test
The original test would open a local port and then immediately close it
and use the port number in subsequent tests. Between the port being closed
and reused by the later process, it could be opened by some other program
on the machine.
Changed the test to run the server process directly and have it save the
assigned port to a text file to be used by client processes.
Rob Pike [Tue, 18 Mar 2014 00:25:04 +0000 (11:25 +1100)]
fmt: document GoStringer and explain application of formats to compound objects
%q quotes each element of a string slice; this was never explained in the docs.
Fixes #7015.
Catalin Patulea [Mon, 17 Mar 2014 22:47:16 +0000 (15:47 -0700)]
net/http/fcgi: fix handling of request ID reuse
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.