Brad Fitzpatrick [Wed, 24 Jun 2015 09:53:24 +0000 (11:53 +0200)]
net/http: bound the number of bytes read seeking EOF in Handler's Body.Close
If a client sent a POST with a huge request body, calling
req.Body.Close in the handler (which is implicit at the end of a
request) would end up consuming it all.
Put a cap on that, using the same threshold used elsewhere for similar
cases.
Fixes #9662
Change-Id: I26628413aa5f623a96ef7c2609a8d03c746669e5
Reviewed-on: https://go-review.googlesource.com/11412 Reviewed-by: Andrew Gerrand <adg@golang.org>
Todd Neal [Wed, 24 Jun 2015 12:48:35 +0000 (07:48 -0500)]
fmt: handle negative width/prec when supplied as an argument
Negative width arguments now left align the way a minus-width in the
format string aligns. The minus in the format string overrides the sign
of the argument as in C.
Precision behavior is modified to include an error if the argument is
negative. This differs from a negative precision in a format string
which just terminates the format.
Additional checks for large magnitude widths and precisions are added to
make the runtime behavior (failure, but with different error messages),
more consistent between format string specified width/precision and
argument specified width/precision.
Fixes #11376
Change-Id: I8c7ed21088e9c18128a45d4c487c5ab9fafd13ef
Reviewed-on: https://go-review.googlesource.com/11405 Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
Matthew Dempsky [Tue, 23 Jun 2015 18:36:57 +0000 (11:36 -0700)]
net/http: sync Get and Head's documentation
Instead of ambiguously referring to "the Client's CheckRedirect
function" in Head, describe the default behavior like for Get as users
aren't expected to change DefaultClient.CheckRedirect.
While here, use consistent punctuation for the Get and Head Client
method documentation.
Brad Fitzpatrick [Wed, 24 Jun 2015 11:57:33 +0000 (13:57 +0200)]
net/http: don't always require certFile, keyFile in Server.ListenAndServerTLS
The ListenAndServerTLS function still requires the certFile and
keyFile, but the Server.ListenAndServerTLS method doesn't need to
require the certFile and keyFile if the Server.TLSConfig.Certificates
are already populated.
Fixes #8599
Change-Id: Id2e3433732f93e2619bfd78891f775d89f1d651e
Reviewed-on: https://go-review.googlesource.com/11413 Reviewed-by: Andrew Gerrand <adg@golang.org>
Didier Spezia [Sat, 20 Jun 2015 11:25:59 +0000 (11:25 +0000)]
go/format: fix //line corner case when formatting statements
The code formatting mechanism can be applied to partial Go code,
such as a list of statements. The statements are wrapped into a
function definition (to be parsed fine), and unwrapped after formatting.
When the statements contain //line annotations, it may fail,
because not all comments are flushed by the printer before the final '}'.
Formatting "\ta()\n//line :1" results in "\ta() }\n\n//line", which
is wrong.
Tweaked the wrapping/unwrapping code to make sure comments are flushed
before the '}'.
Fixes #11276
Change-Id: Id15c80279b0382ee9ed939cca1647f525c4929f5
Reviewed-on: https://go-review.googlesource.com/11282
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Dmitry Vyukov [Thu, 7 May 2015 09:33:45 +0000 (12:33 +0300)]
runtime/race: make test more robust
The test is flaky on builders lately. I don't see any issues other than
usage of very small sleeps. So increase the sleeps. Also take opportunity
to refactor the code.
On my machine this change significantly reduces failure rate with GOMAXPROCS=2.
I can't reproduce the failure with GOMAXPROCS=1.
Russ Cox [Wed, 24 Jun 2015 12:39:44 +0000 (08:39 -0400)]
net: make LookupIP("1.2.3.4") behavior consistent
To date, the behavior has depended on whether we're using cgo and
in turn what the host resolver does. Most host resolvers will "resolve"
IP addresses, but the non-cgo pure Go path has not.
This CL makes resolution of IP addresses always work, even if we're not using cgo
and even if the host resolver does not "resolve" IP addresses.
Austin Clements [Tue, 23 Jun 2015 15:35:21 +0000 (11:35 -0400)]
runtime: fix heap bitmap repeating with large scalar tails
When heapBitsSetType repeats a source bitmap with a scalar tail
(typ.ptrdata < typ.size), it lays out the tail upon reaching the end
of the source bitmap by simply increasing the number of bits claimed
to be in the incoming bit buffer. This causes later iterations to read
the appropriate number of zeros out of the bit buffer before starting
on the next repeat of the source bitmap.
Currently, however, later iterations of the loop continue to read bits
from the source bitmap *regardless of the number of bits currently in
the bit buffer*. The bit buffer can only hold 32 or 64 bits, so if the
scalar tail is large and the padding bits exceed the size of the bit
buffer, the read from the source bitmap on the next iteration will
shift the incoming bits into oblivion when it attempts to put them in
the bit buffer. When the buffer does eventually shift down to where
these bits were supposed to be, it will contain zeros. As a result,
words that should be marked as pointers on later repetitions are
marked as scalars, so the garbage collector does not trace them. If
this is the only reference to an object, it will be incorrectly freed.
Fix this by adding logic to drain the bit buffer down if it is large
instead of reading more bits from the source bitmap.
Austin Clements [Tue, 23 Jun 2015 18:04:09 +0000 (14:04 -0400)]
runtime: document memory ordering for h_spans
h_spans can be accessed concurrently without synchronization from
other threads, which means it needs the appropriate memory barriers on
weakly ordered machines. It happens to already have the necessary
memory barriers because all accesses to h_spans are currently
protected by the heap lock and the unlocks happen in exactly the
places where release barriers are needed, but it's easy to imagine
that this could change in the future. Document the fact that we're
depending on the barrier implied by the unlock.
Rob Pike [Mon, 22 Jun 2015 22:40:40 +0000 (08:40 +1000)]
cmd/asm: fix shifts again, this time for sure
There are two conditions to worry about:
1) The shift count cannot be negative. Since the evaluator uses unsigned
arithmetic throughout, this means checking that the high bit of
the shift count is always off, which is done by converting to int64
and seeing if the result is negative.
2) For right shifts, the value cannot be negative. We don't want a
high bit in the value because right shifting a value depends on the
sign, and for clarity we always want unsigned shifts.
Next step is to build some testing infrastructure for the parser.
Rob Pike [Mon, 22 Jun 2015 20:23:04 +0000 (06:23 +1000)]
cmd/asm: fix handling of negative shifts.
The change that "fixed" LSH was incorrect, and the fix for RSH was poor.
Make both use a correct, simple test: if the 64-bit value as a signed
integer is negative, it's an error.
Really fixes #11278.
Change-Id: I72cca03d7ad0d64fd649fa33a9ead2f31bd2977b
Reviewed-on: https://go-review.googlesource.com/11325 Reviewed-by: Andrew Gerrand <adg@golang.org>
Rick Hudson [Mon, 22 Jun 2015 18:05:00 +0000 (14:05 -0400)]
runtime: remove race and increase precision in pointer validation.
This CL removes the single and racy use of mheap.arena_end outside
of the bookkeeping done in mHeap_init and mHeap_Alloc.
There should be no way for heapBitsForSpan to see a pointer to
an invalid span. This CL makes the check for this more precise by
checking that the pointer is between mheap_.arena_start and
mheap_.arena_used instead of mheap_.arena_end.
Austin Clements [Mon, 22 Jun 2015 15:18:23 +0000 (11:18 -0400)]
runtime: one more Map{Bits,Spans} before arena_used update
In order to avoid a race with a concurrent write barrier or garbage
collector thread, any update to arena_used must be preceded by mapping
the corresponding heap bitmap and spans array memory. Otherwise, the
concurrent access may observe that a pointer falls within the heap
arena, but then attempt to access unmapped memory to look up its span
or heap bits.
Commit d57c889 fixed all of the places where we updated arena_used
immediately before mapping the heap bitmap and spans, but it missed
the one place where we update arena_used and depend on later code to
update it again and map the bitmap and spans. This creates a window
where the original race can still happen. This commit fixes this by
mapping the heap bitmap and spans before this arena_used update as
well. This code path is only taken when expanding the heap reservation
on 32-bit over a hole in the address space, so these extra mmap calls
should have negligible impact.
Austin Clements [Fri, 19 Jun 2015 16:29:42 +0000 (12:29 -0400)]
runtime: document relaxed access to arena_used
The unsynchronized accesses to mheap_.arena_used in the concurrent
part of the garbage collector look like a problem waiting to happen.
In fact, they are safe, but the reason is somewhat subtle and
undocumented. This commit documents this reasoning.
Russ Cox [Fri, 19 Jun 2015 21:04:56 +0000 (17:04 -0400)]
net/url: add RawPath field, a hint at the desired encoding of Path
Historically we have declined to try to provide real support for URLs
that contain %2F in the path, but they seem to be popping up more
often, especially in (arguably ill-considered) REST APIs that shoehorn
entire paths into individual path elements.
The obvious thing to do is to introduce a URL.RawPath field that
records the original encoding of Path and then consult it during
URL.String and URL.RequestURI. The problem with the obvious thing
is that it breaks backward compatibility: if someone parses a URL
into u, modifies u.Path, and calls u.String, they expect the result
to use the modified u.Path and not the original raw encoding.
Split the difference by treating u.RawPath as a hint: the observation
is that there are many valid encodings of u.Path. If u.RawPath is one
of them, use it. Otherwise compute the encoding of u.Path as before.
If a client does not use RawPath, the only change will be that String
selects a different valid encoding sometimes (the original passed
to Parse).
This ensures that, for example, HTTP requests use the exact
encoding passed to http.Get (or http.NewRequest, etc).
Also add new URL.EscapedPath method for access to the actual
escaped path. Clients should use EscapedPath instead of
reading RawPath directly.
All the old workarounds remain valid.
Fixes #5777.
Might help #9859.
Fixes #7356.
Fixes #8767.
Fixes #8292.
Fixes #8450.
Fixes #4860.
Fixes #10887.
Fixes #3659.
Fixes #8248.
Fixes #6658.
Reduces need for #2782.
Russ Cox [Fri, 19 Jun 2015 18:43:09 +0000 (14:43 -0400)]
cmd/go: fix test for issue 8181
The test was translated from shell incorrectly,
and it depended on having hg installed, which
may not be the case.
Moved repo to GitHub, updated code, and fixed
go list ... command to be expected to succeed.
Fixes test for #8181.
Change-Id: I7f3e8fb20cd16cac5ed24de6fd952003bc5e08d4
Reviewed-on: https://go-review.googlesource.com/11301 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jeff R. Allen [Thu, 18 Jun 2015 12:28:54 +0000 (14:28 +0200)]
net/textproto: skip zero-length keys
A header of ": value" results in an empty key. Do not add
it to the headers, because RFC7230 (section 3.2) says that
field-names are tokens, which are one or more characters.
Rob Pike [Sat, 20 Jun 2015 10:28:46 +0000 (20:28 +1000)]
cmd/doc: add test for constructor, fix build
Most important: skip test on darwin/arm64 for unclear reasons.
First cut at the test missed this feature of go doc: when asking for
the docs for a type, include any function that looks like it constructs
a that type as a return value.
Change-Id: I124e7695e5d365e2b12524b541a9a4e6e0300fbc
Reviewed-on: https://go-review.googlesource.com/11295 Reviewed-by: Rob Pike <r@golang.org>
Ian Lance Taylor [Fri, 19 Jun 2015 20:48:06 +0000 (13:48 -0700)]
syscall: skip non-root user namespace test if kernel forbids
Some Linux kernels apparently have a sysctl that prohibits
nonprivileged processes from creating user namespaces. If we see a
failure for that reason, skip the test.
Rob Pike [Fri, 19 Jun 2015 22:12:10 +0000 (08:12 +1000)]
cmd/doc: fix test on nacl
nacl is really giving a hard time. avoid all external dependencies in the test.
Worked with trybots, failed in the build. No explanation, but this should fix it.
TBR=rsc
Change-Id: Icb644286dbce88f17ee3d96ad90efba34a80a92d
Reviewed-on: https://go-review.googlesource.com/11291 Reviewed-by: Rob Pike <r@golang.org>
doc: mention moderation delay in contributing docs
This sometime worries new contributors.
Hopefully mentioning it here will help.
Fixes #11300.
Change-Id: Ica7f10d749731704ac6a2c39c7dcba389996011e
Reviewed-on: https://go-review.googlesource.com/11236 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Tue, 9 Jun 2015 19:09:12 +0000 (12:09 -0700)]
cmd/go: add preliminary support for vendor directories
When GO15VENDOREXPERIMENT=1 is in the environment,
this CL changes the resolution of import paths according to
the Go 1.5 vendor proposal:
If there is a source directory d/vendor, then,
when compiling a source file within the subtree rooted at d,
import "p" is interpreted as import "d/vendor/p" if that exists.
When there are multiple possible resolutions,
the most specific (longest) path wins.
The short form must always be used: no import path can
contain “/vendor/” explicitly.
Import comments are ignored in vendored packages.
The goal of these changes is to allow authors to vendor (copy) external
packages into their source trees without any modifications to the code.
This functionality has been achieved in tools like godep, nut, and gb by
requiring GOPATH manipulation. This alternate directory-based approach
eliminates the need for GOPATH manipulation and in keeping with the
go command's use of directory layout-based configuration.
The flag allows experimentation with these vendoring semantics once
Go 1.5 is released, without forcing them on by default. If the experiment
is deemed a success, the flag will default to true in Go 1.6 and then be
removed in Go 1.7.
For more details, see the original proposal by Keith Rarick at
https://groups.google.com/d/msg/golang-dev/74zjMON9glU/dGhnoi2IMzsJ.
Change-Id: I2c6527e777d14ac6dc43c53e4b3ff24f3279216e
Reviewed-on: https://go-review.googlesource.com/10923 Reviewed-by: Andrew Gerrand <adg@golang.org>
Russ Cox [Tue, 9 Jun 2015 19:08:59 +0000 (12:08 -0700)]
cmd/compile: add -importmap option
The -importmap option takes an argument of the form old=new
and specifies that import "old" should be interpreted as if it said
import "new". The option may be repeated to specify multiple mappings.
This option is here to support the go command's new -vendor flag.
Change-Id: I31b4ed4249b549982a720bf61bb230462b33c59b
Reviewed-on: https://go-review.googlesource.com/10922 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Mon, 15 Jun 2015 16:30:23 +0000 (12:30 -0400)]
runtime: ensure GC sees type-safe memory on weak machines
Currently its possible for the garbage collector to observe
uninitialized memory or stale heap bitmap bits on weakly ordered
architectures such as ARM and PPC. On such architectures, the stores
that zero newly allocated memory and initialize its heap bitmap may
move after a store in user code that makes the allocated object
observable by the garbage collector.
To fix this, add a "publication barrier" (also known as an "export
barrier") before returning from mallocgc. This is a store/store
barrier that ensures any write done by user code that makes the
returned object observable to the garbage collector will be ordered
after the initialization performed by mallocgc. No barrier is
necessary on the reading side because of the data dependency between
loading the pointer and loading the contents of the object.
Fixes one of the issues raised in #9984.
Change-Id: Ia3d96ad9c5fc7f4d342f5e05ec0ceae700cd17c8
Reviewed-on: https://go-review.googlesource.com/11083 Reviewed-by: Rick Hudson <rlh@golang.org> Reviewed-by: Dmitry Vyukov <dvyukov@google.com> Reviewed-by: Minux Ma <minux@golang.org> Reviewed-by: Martin Capitanio <capnm9@gmail.com> Reviewed-by: Russ Cox <rsc@golang.org>
Nigel Tao [Fri, 19 Jun 2015 05:39:11 +0000 (15:39 +1000)]
image/gif: re-enable some invalid-palette tests.
These tests were broken by https://go-review.googlesource.com/#/c/11227/
which fixed the LZW encoder to reject invalid input.
For TestNoPalette, the LZW encoder with a litWidth of 2 now rejects an
input byte of 128, so we change 128 to 3, as 3 <= (1<<2 - 1).
For TestPixelOutsidePaletteRange, the LZW encoder similarly rejects an
input byte of 255. Prior to golang.org/cl/11227, the encoder (again with
a litWidth of 2) accepted the 255 input byte, but masked it with (1<<2 -
1), so that the 255 test case was effectively the same as the 3 test
case. After that LZW CL, the 255 input byte is simply invalid, so we
remove it as a test case. The test still tests pixels outside of the
palette range, since 3 >= the length of the global palette, which is 2.
Change-Id: I50be9623ace016740e34801549c15f83671103eb
Reviewed-on: https://go-review.googlesource.com/11273 Reviewed-by: David Symonds <dsymonds@golang.org>
Alex Brainman [Mon, 27 Apr 2015 07:32:23 +0000 (17:32 +1000)]
runtime: rename cgocall_errno and asmcgocall_errno into cgocall and asmcgocall
Change-Id: I5917bea8bb35b0e725dcc56a68f3a70137cfc180
Reviewed-on: https://go-review.googlesource.com/9387 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Andrey Petrov [Thu, 18 Jun 2015 19:43:01 +0000 (21:43 +0200)]
doc: clarify duplicate symbol condition in cgo
Spell out what will happen if a declaration and definition is included
in the same file, should help people who run into duplicate symbol
errors and search for relevant keywords.
This edit is based on opening issue #11263 erroneously.
Change-Id: I0645a9433b8668d2ede9b9a3f6550d802c26388b
Reviewed-on: https://go-review.googlesource.com/11247 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Nigel Tao [Thu, 18 Jun 2015 22:26:02 +0000 (08:26 +1000)]
image/gif: (temporarily) disable broken tests.
The compress/lzw encoder now rejects too-large input bytes, as of
https://go-review.googlesource.com/#/c/11227/, so we can't generate bad
GIFs programatically.
Change-Id: I0b32ce8e1f1776cd6997869db61e687430464e45
Reviewed-on: https://go-review.googlesource.com/11270 Reviewed-by: Nigel Tao <nigeltao@golang.org>
Jeff R. Allen [Wed, 17 Jun 2015 12:07:13 +0000 (14:07 +0200)]
compress/lzw: mention relation between litWidth and input bytes
Add sentences to the docs explaining the limit on input
bytes implicit in the choice of litWidth, and the fact that
compress and decompress litWidth must match.
Fixes #11142.
Change-Id: I20cfb4df35739f7bfeb50b92c78249df3d47942c
Reviewed-on: https://go-review.googlesource.com/11063 Reviewed-by: Nigel Tao <nigeltao@golang.org>
Rick Hudson [Mon, 1 Jun 2015 22:16:03 +0000 (18:16 -0400)]
runtime: reduce latency by aggressively ending mark phase
Some latency regressions have crept into our system over the past few
weeks. This CL fixes those by having the mark phase more aggressively
blacken objects so that the mark termination phase, a STW phase, has less
work to do. Three approaches were taken when the mark phase believes
it has no more work to do, ie all the work buffers are empty.
If things have gone well the mark phase is correct and there is
in fact little or no work. In that case the following items will
take very little time. If the mark phase is wrong this CL will
ferret that work out and give the mark phase a chance to deal with
it concurrently before mark termination begins.
When the mark phase first appears to be out of work, it does three things:
1) It switches from allocating white to allocating black to reduce the
number of unmarked objects reachable only from stacks.
2) It flushes and disables per-P GC work caches so all work must be in
globally visible work buffers.
3) It rescans the global roots---the BSS and data segments---so there
are fewer objects to blacken during mark termination. We do not rescan
stacks at this point, though that could be done in a later CL.
After these steps, it again drains the global work buffers.
On a lightly loaded machine the garbage benchmark has reduced the
number of GC cycles with latency > 10 ms from 83 out of 4083 cycles
down to 2 out of 3995 cycles. Maximum latency was reduced from
60+ msecs down to 20 ms.
Michael Matloob [Tue, 30 Dec 2014 00:59:55 +0000 (16:59 -0800)]
cmd/compile: provide better error when method called without receiver
When a method is called using the Type.Method(receiver, args...) syntax
without the receiver, or enough arguments, provide the more helpful
error message "not enough arguments in call to method expression
Type.Method" instead of the old message "not enough arguments in call
to Type.Method".
Shenghou Ma [Wed, 20 May 2015 01:24:31 +0000 (21:24 -0400)]
time: correct unrepresentable Unix time comment
It's easy for someone who wants a time bigger than any
valid time to reach for time.Unix(1<<63-1, 0), so it
makes sense to explicit say such value is not valid.
Davies Liu [Fri, 19 Dec 2014 06:45:55 +0000 (22:45 -0800)]
hash/crc32: speedup crc32 of IEEE using slicingBy8
The Slicing-By-8 [1] algorithm has much performance improvements than
current approach. This patch only uses it for IEEE, which is the most
common case in practice.
There is the benchmark on Mac OS X 10.9:
benchmark old MB/s new MB/s speedup
BenchmarkIEEECrc1KB 349.40 353.03 1.01x
BenchmarkIEEECrc4KB 351.55 934.35 2.66x
BenchmarkCastagnoliCrc1KB 7037.58 7392.63 1.05x
This algorithm need 8K lookup table, so it's enabled only for block
larger than 4K.
Peter Waldschmidt [Sat, 18 Apr 2015 09:30:30 +0000 (05:30 -0400)]
encoding/json: Remove extra allocation in scanner.
When the scanner receives a non-whitespace character in stateEndTop,
it creates an error message and caches it to return on the next
transition. nextValue() uses the scanner to sub-scan for a value
inside a larger JSON structure. Since stateEndTop is triggered
*after* the ending byte, whatever character immediately follows the
sub-value gets pulled into the scanner's state machine as well.
Even though it is not used and doesn't cause an error, it does
cause the state machine to allocate an error that will never be used.
The fix is to probe the state machine with whitespace after
scanEndObject or scanEndArray to see if the next character would
result in a scanEnd state transition. If so, we can return right
away without processing the next character and avoid triggering
an allocation.
There were two issues.
1. Delayed EvGoSysExit could have been emitted during TraceStart,
while it had not yet emitted EvGoInSyscall.
2. Delayed EvGoSysExit could have been emitted during next tracing session.
Alex Brainman [Wed, 17 Jun 2015 06:48:02 +0000 (16:48 +1000)]
runtime: remove cgocall and asmcgocall
In preparation for rename of cgocall_errno into cgocall and
asmcgocall_errno into asmcgocall in the fllowinng CL.
rsc requested CL 9387 to be split into two parts. This is first part.
Change-Id: I7434f0e4b44dd37017540695834bfcb1eebf0b2f
Reviewed-on: https://go-review.googlesource.com/11166 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Mikio Hara [Thu, 18 Jun 2015 00:38:04 +0000 (09:38 +0900)]
net: fix build on android
Change-Id: Ib6d0b2947748dec98cad2e6abb6812cac46a9897
Reviewed-on: https://go-review.googlesource.com/11220 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 18 Jun 2015 00:02:40 +0000 (17:02 -0700)]
go/internal/gccgoimporter: enable tests on Plan9
Work-around issue #11265 and re-enable tests for Plan9.
Change-Id: I3aabb674a149b8eb936f948dd4cda5fd81454646
Reviewed-on: https://go-review.googlesource.com/11194
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Tue, 16 Jun 2015 23:28:57 +0000 (16:28 -0700)]
go/internal/gccgoimporter: unmodified copy of x/tools/go/gccgoimporter
This change will brake the build. The immediately following change
contains the necessary adjustments to make it work again. We're
doing this in two steps to expose the manual changes applied.
Change-Id: I225947da23e190b12e12cbd0c5e6e91628de7f53
Reviewed-on: https://go-review.googlesource.com/11151 Reviewed-by: Alan Donovan <adonovan@google.com>