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
Shenghou Ma [Wed, 5 Mar 2014 19:40:55 +0000 (14:40 -0500)]
cmd/ld: don't include padding length in size of the note section
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)
Russ Cox [Wed, 5 Mar 2014 16:10:40 +0000 (11:10 -0500)]
runtime: handle Go calls C calls Go panic correctly on windows/386
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.
Joel Sing [Wed, 5 Mar 2014 13:08:03 +0000 (00:08 +1100)]
net: disable "udp" to IPv6 unicast address loopback test on dragonfly
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.
Joel Sing [Wed, 5 Mar 2014 13:07:16 +0000 (00:07 +1100)]
net: fix non-blocking connect handling on dragonfly
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.
Mike Andrews [Tue, 4 Mar 2014 21:43:26 +0000 (13:43 -0800)]
net/smtp: set ServerName in StartTLS, as now required by crypto/tls
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.
Brad Fitzpatrick [Tue, 4 Mar 2014 19:55:35 +0000 (11:55 -0800)]
net/http: fix test failure on some Windows machines
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.
Russ Cox [Tue, 4 Mar 2014 18:53:08 +0000 (13:53 -0500)]
cmd/ld: clear unused ctxt before morestack
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.
Brad Fitzpatrick [Tue, 4 Mar 2014 17:58:54 +0000 (09:58 -0800)]
crypto/cipher: fix AEAD.Open documentation nit
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).
Russ Cox [Tue, 4 Mar 2014 14:46:40 +0000 (09:46 -0500)]
runtime: fix finalizer flakiness
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
Russ Cox [Tue, 4 Mar 2014 04:33:27 +0000 (23:33 -0500)]
runtime: fix traceback on Windows
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.
Robert Griesemer [Tue, 4 Mar 2014 04:07:34 +0000 (20:07 -0800)]
spec: clarify what is considered a function call for len/cap special case
gccgo considers built-in function calls returning a constant not as function call (issue 7386)
go/types considers any call (regular or built-in) as a function call
The wording and examples clarify that only "function calls" that are issued
at run-time (and thus do not result in a constant result) are considered
function calls in this case.
gc is inconsistent (issue 7385)
gccgo already interprets the spec accordingly and issue 7386 is moot.
go/types considers all calls (constant or not) as function calls (issue 7457).
Fixes #7387.
Fixes #7386.
LGTM=r, rsc, iant
R=r, rsc, iant, ken
CC=golang-codereviews
https://golang.org/cl/66860046
Brad Fitzpatrick [Tue, 4 Mar 2014 02:58:28 +0000 (18:58 -0800)]
net/http: fix location of StateHijacked and StateActive
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
Mikio Hara [Tue, 4 Mar 2014 00:26:01 +0000 (09:26 +0900)]
syscall: regenerate z-files on FreeBSD 10
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
Brad Fitzpatrick [Mon, 3 Mar 2014 19:25:57 +0000 (11:25 -0800)]
net/http: in Client, consume small redirect bodies before making next request
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.
Adam Langley [Mon, 3 Mar 2014 14:01:44 +0000 (09:01 -0500)]
crypto/tls: split connErr to avoid read/write races.
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
Richard Crowley [Sun, 2 Mar 2014 04:32:42 +0000 (20:32 -0800)]
net/http: ensure ConnState for StateNew fires before Server.Serve returns
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.
Dave Cheney [Sat, 1 Mar 2014 21:30:45 +0000 (08:30 +1100)]
sync/atomic: skip broken tests on freebsd/arm and netbsd/arm
Update #7338
The nil deref tests are currently failing on the *bsd/arm platforms. In an effort to avoid the build deteriorating further I would like to skip these tests on freebsd/arm and netbsd/arm.
Dave Cheney [Sat, 1 Mar 2014 00:13:29 +0000 (11:13 +1100)]
runtime: small Native Client fixes
cgocall.c: define the CBARGS macro for GOARCH_amd64p32. I don't think the value of this macro will ever be used under nacl/amd64p32 but it is required to compile even if cgo is not used.
Adam Langley [Fri, 28 Feb 2014 14:40:12 +0000 (09:40 -0500)]
crypto/tls: add DialWithDialer.
While reviewing uses of the lower-level Client API in code, I found
that in many cases, code was using Client only because it needed a
timeout on the connection. DialWithDialer allows a timeout (and
other values) to be specified without resorting to the low-level API.
Brad Fitzpatrick [Fri, 28 Feb 2014 02:29:00 +0000 (18:29 -0800)]
net/http: add optional Server.ConnState callback
Update #4674
This allows for all sorts of graceful shutdown policies,
without picking a policy (e.g. lameduck period) and without
adding lots of locking to the server core. That policy and
locking can be implemented outside of net/http now.
Russ Cox [Fri, 28 Feb 2014 01:37:00 +0000 (20:37 -0500)]
all: final merge of NaCl tree
This CL replays the following one CL from the rsc-go13nacl repo.
This is the last replay CL: after this CL the main repo will have
everything the rsc-go13nacl repo did. Changes made to the main
repo after the rsc-go13nacl repo branched off probably mean that
NaCl doesn't actually work after this CL, but all the code is now moved
over and just needs to be redebugged.
---
cmd/6l, cmd/8l, cmd/ld: support for Native Client
See golang.org/s/go13nacl for design overview.
This CL is publicly visible but not CC'ed to golang-dev,
to avoid distracting from the preparation of the Go 1.2
release.
This CL and the others will be checked into my rsc-go13nacl
clone repo for now, and I will send CLs against the main
repo early in the Go 1.3 development.
Nigel Tao [Thu, 27 Feb 2014 23:37:21 +0000 (10:37 +1100)]
image/jpeg: fix progressive decoding when the DC components are split
over multiple scans. Previously, the Go code assumed that DC was
synonymous with interleaved and AC with non-interleaved.
Fixes #6767.
The test files were generated with libjpeg's cjpeg program, version 9a,
with the following patch, since cjpeg is hard-coded to output
interleaved DC.
Keith Randall [Thu, 27 Feb 2014 22:20:15 +0000 (14:20 -0800)]
runtime: move stack shrinking until after sweepgen is incremented.
Before GC, we flush all the per-P allocation caches. Doing
stack shrinking mid-GC causes these caches to fill up. At the
end of gc, the sweepgen is incremented which causes all of the
data in these caches to be in a bad state (cached but not yet
swept).
Move the stack shrinking until after sweepgen is incremented,
so any caching that happens as part of shrinking is done with
already-swept data.
David du Colombier [Thu, 27 Feb 2014 08:22:02 +0000 (09:22 +0100)]
runtime: fix build on Plan 9
warning: src/pkg/runtime/mem_plan9.c:72 param declared and not used: n
src/pkg/runtime/mem_plan9.c:73 name not declared: nbytes
src/pkg/runtime/mem_plan9.c:73 bad in naddr: NAME nbytes<>+0(SB)
Keith Randall [Thu, 27 Feb 2014 07:28:44 +0000 (23:28 -0800)]
runtime: grow stack by copying
On stack overflow, if all frames on the stack are
copyable, we copy the frames to a new stack twice
as large as the old one. During GC, if a G is using
less than 1/4 of its stack, copy the stack to a stack
half its size.
TODO
- Do something about C frames. When a C frame is in the
stack segment, it isn't copyable. We allocate a new segment
in this case.
- For idempotent C code, we can abort it, copy the stack,
then retry. I'm working on a separate CL for this.
- For other C code, we can raise the stackguard
to the lowest Go frame so the next call that Go frame
makes triggers a copy, which will then succeed.
- Pick a starting stack size?
The plan is that eventually we reach a point where the
stack contains only copyable frames.
Rémy Oudompheng [Thu, 27 Feb 2014 07:07:50 +0000 (08:07 +0100)]
cmd/gc: do not nop-convert equivalent but different interface types.
The cached computed interface tables are indexed by the interface
types, not by the unnamed underlying interfaces
To preserve the invariants expected by interface comparison, an
itab generated for an interface type must not be used for a value
of a different interface type even if the representation is identical.
Robert Griesemer [Wed, 26 Feb 2014 21:39:49 +0000 (13:39 -0800)]
go/printer: refine handling of one-line functions
Functions that "fit" on one line and were on one
line in the original source are not broken up into
two lines anymore simply because they contain a comment.
- Fine-tuned use of separating blanks after /*-style comments, so:
( /* extra blank after this comment */ )
(a int /* no extra blank after this comment*/)
- Factored out comment state (from printer state) into commentInfo.
- No impact on $GOROOT/src, misc formatting.
Fixes #5543.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/68630043
Robert Griesemer [Wed, 26 Feb 2014 17:54:01 +0000 (09:54 -0800)]
go/parser: report error if ParseExpr argument contains extra tokens
This partly addresses issue 6099 where a gofmt rewrite is behaving
unexpectedly because the provided rewrite term is not a valid expression
but is silently consumed anyway.