This bug was introduced in golang.org/cl/18217,
while trying to fix#13777.
Originally I wanted to just disable inlining for the case
being handled incorrectly, but it's fairly difficult to detect
and much easier just to fix. Since the case being handled
incorrectly was inlined correctly in Go 1.5, not inlining it
would also be somewhat of a regression.
So just fix it.
Test case copied from Ian's CL 19520.
The mistake to worry about in this CL would be relaxing
the condition too much (we now print the note more often
than we did yesterday). To confirm that we'd catch this mistake,
I checked that changing (!fmtbody || !t.Funarg) to (true) does
cause fixedbugs/issue13777.go to fail. And putting it back
to what is written in this CL makes that test pass again
as well as the new fixedbugs/issue14331.go.
So I believe that the new condition is correct for both constraints.
Fixes#14331.
Change-Id: I91f75a4d5d07c53af5caea1855c780d9874b8df6
Reviewed-on: https://go-review.googlesource.com/19514
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestInterfaceAddrsWithNetsh invokes Windows netsh command passing
it a particular interface name. This approach somehow does not work
on some computers (see issue for details). Change that to call netsh
without specifying any interface name. This provides output for all
interfaces available. So we can achieve same goal parsing this output.
Also makes test faster because we only need to invoke netsh once.
Fixes#14130.
Change-Id: I7911692ca64e372af1e1f9d6acb718c67071de67
Reviewed-on: https://go-review.googlesource.com/19441
Reviewed-by: Volker Dobler <dr.volker.dobler@gmail.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
We used to include panic calls in tracebacks; however, when
runtime.panic was renamed to runtime.gopanic in the conversion of the
runtime to Go, we missed the special case in showframe that includes
panic calls even though they're in package runtime.
Fix the function name check in showframe (and, while we're here, fix
the other check for "runtime.panic" in runtime/pprof). Since the
"runtime.gopanic" name doesn't match what users call panic and hence
isn't very user-friendly, make traceback rewrite it to just "panic".
Updates #5832, #13857. Fixes#14315.
Change-Id: I8059621b41ec043e63d5cfb4cbee479f47f64973
Reviewed-on: https://go-review.googlesource.com/19492
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Go 1.6 significantly improves pause times for large heaps, but it
improves them in many other situations as well, such as when goroutine
churn is high, allocation rate is high, or when there are many
finalizers. Hence, make the statement about pause times a bit more
general.
Change-Id: Ic034b1c904c39dd1d966ee7fa96ca8bbb3614e53
Reviewed-on: https://go-review.googlesource.com/19504
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently we use "Section's" as the plural of the debug/elf Section
struct. Change this to "Sections" because it's not possessive and
doesn't seem to fall in to any special cases were the apostrophe is
acceptable.
Change-Id: Id5d3abbd748502a67ead3f483182ee7729db94a2
Reviewed-on: https://go-review.googlesource.com/19505
Reviewed-by: Russ Cox <rsc@golang.org>
Type switches need write barriers if the written-to
variable is heap allocated.
For the added needwritebarrier call, the right arg doesn't
really matter, I just pass something that will never disqualify
the write barrier. The left arg is the one that matters.
Fixes#14306
Change-Id: Ic2754167cce062064ea2eeac2944ea4f77cc9c3b
Reviewed-on: https://go-review.googlesource.com/19481
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The code in mem_bsd.go expects that when mmap fails it will return a
positive errno value. This fixes the Solaris implementation of mmap to
work as expected.
Change-Id: Id1c34a9b916e8dc955ced90ea2f4af8321d92265
Reviewed-on: https://go-review.googlesource.com/19477
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The caller of mmap expects it to return a positive errno value, but the
linux-arm64 and nacl-386 system calls returned a negative errno value.
Correct them to negate the errno value.
The caller of mincore expects it to return a negative errno value (yes,
this is inconsistent), but the linux-mips64x and linux-ppc64x system
call returned a positive errno value. Correct them to negate the errno
value.
Add a test that mmap returns errno with the correct sign. Brad added a
test for mincore's errno value in https://golang.org/cl/19457.
Fixes#14297.
Change-Id: I2b93f32e679bd1eae1c9aef9ae7bcf0ba39521b5
Reviewed-on: https://go-review.googlesource.com/19455
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
httptest.Server was rewritten during Go 1.6, but
CloseClientConnections was accidentally made async in the rewrite and
not caught due to lack of tests.
Restore the Go 1.5 behavior and add tests.
Fixes#14290
Updates #14291
Change-Id: I14f01849066785053ccca2373931bc82d78c0a13
Reviewed-on: https://go-review.googlesource.com/19432
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
1) go/types.dir: Correctly return "." if there is no path.
2) go/internal/gcimporter.FindPkg: work-around for build.Import
(build.Import doesn't produce expected result if srcDir is
relative). See also issue 14282.
Fixes#14215.
Change-Id: Ia3721f9ad8a1115d2595fe99b04baaf30d5765f2
Reviewed-on: https://go-review.googlesource.com/19393
Reviewed-by: Russ Cox <rsc@golang.org>
The Windows 7 getmac command may report the physical address of an adapter
as "Disabled" or "N/A". Handle these two cases to make the tests more
robust when building on Windows with manually disabled adapters or turned
off hardware.
Addresses issue #14130.
Change-Id: I0c2f8554b4b6810568e4e60ed53857599401f296
Reviewed-on: https://go-review.googlesource.com/19411
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
For now, don't enable http2 when Transport.TLSConfig != nil.
See background in #14275.
Also don't enable http2 when ExpectContinueTimeout is specified for
now, in case somebody depends on that functionality. (It is not yet
implemented in http2, and was only just added to net/http too in Go
1.6, so nobody would be setting it yet).
Updates #14275
Updates #13851
Change-Id: I192d555f5fb0a567bd89b6ad87175bbdd7891ae3
Reviewed-on: https://go-review.googlesource.com/19424
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Given GOPATH=p1:p2 and source code of just the right form,
the go command could previously end up invoking the compiler
with -I p2 -I p1 or the linker with -L p2 -L p1, so that
compiled packages in p2 incorrectly shadowed packages in p1.
If foo were in both p1 and p2 and the compilation of bar
were such that the -I and -L options were inverted in this way,
then
GOPATH=p2 go install foo
GOPATH=p1:p2 go install bar
would get the p2 copy of foo instead of the (expected) p1 copy of foo.
This manifested in real usage in a few different ways, but in all
the root cause was that the -I or -L option sequence did not
match GOPATH.
Make it match GOPATH.
Fixes#14176 (second report).
Fixes#14192.
Related but less common issue #14271 not fixed.
Change-Id: I9c0f69042bb2bf92c9fc370535da2c60a1187d30
Reviewed-on: https://go-review.googlesource.com/19385
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
And update two imports in cmd/internal/objfile/disasm.go.
This makes GO15VENDOREXPERIMENT=0 ./make.bash work.
For Go 1.7 we will move it back.
Fixes#14236.
Change-Id: I429c9af4baff8496f83d113b1b03b90e309f4f48
Reviewed-on: https://go-review.googlesource.com/19384
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This error only affects the compilation of the standard library,
but I discovered that if you import "notexist" from the standard
library then you get both an error about notexist not existing
and an error about notexist being a non-standard package
(because the non-existant package is in fact not a standard package).
Silence the second error.
Change-Id: Ib4c1523e89844260fde90de3459ec1e752df8f25
Reviewed-on: https://go-review.googlesource.com/19383
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ListenAndServeTLS doesn't require cert and key file names if the
server's TLSConfig has a cert configured. This code was never updated
when the GetCertificate hook was added to *tls.Config, however.
Fixes#14268
Change-Id: Ib282ebb05697edd37ed8ff105972cbd1176d900b
Reviewed-on: https://go-review.googlesource.com/19381
Reviewed-by: Russ Cox <rsc@golang.org>
The operation where this manifested in a crash was % (only defined on integers).
However, the existing code was sloppy in that it didn't retain the integer form
after a value (e.g., 3.0) was accepted as representable in integer form (3 for
the example). We would have seen a crash in such cases for / as well except
that there was code to fix it for just that case.
Remove the special code for / and fix more generally by retaining the integer
form for all operations if applicable.
Fixes#14229.
Change-Id: I8bef769e6299839fade27c6e8b5ff29ad6521d0d
Reviewed-on: https://go-review.googlesource.com/19300
Reviewed-by: Alan Donovan <adonovan@google.com>
The test sends two HTTP/1.1 pipelined requests. The first is
completedly by the second, and as such triggers an immediate call to the
CloseNotify channel. The second calls the CloseNotify channel after the
overall connection is closed.
The test was passing fine on gc because the code would enter the select
loop before running the handler, so the send on gotReq would always be
seen first. On gccgo the code would sometimes enter the select loop
after the handler had already finished, meaning that the select could
choose between gotReq and sawClose. If it picked sawClose, it would
never close the overall connection, and the httptest server would hang.
The same hang could be induced with gc by adding a time.Sleep
immediately before the select loop.
Deflake the test by 1) don't close the overall connection until both
requests have been seen; 2) don't exit the loop until both closes have
been seen.
Fixes#14231.
Change-Id: I9d20c309125422ce60ac545f78bcfa337aec1c7d
Reviewed-on: https://go-review.googlesource.com/19281
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When using a stack-allocated buffer for the result, don't
expose the uninitialized portion of it by restricting its
capacity to its length.
The other option is to zero the portion between len and cap.
That seems like more work, but might be worth it if the caller
then appends some stuff to the result. But this close to 1.6,
I'm inclined to do the simplest fix possible.
Fixes#14232
Change-Id: I21c50d3cda02fd2df4d60ba5e2cfe2efe272f333
Reviewed-on: https://go-review.googlesource.com/19231
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The plan9.bell-labs.com site has fallen into disrepair.
We'll instead use the site maintained by contributor David du Colombier.
Fixes#14233
Change-Id: I0c702e5d3b091cccd42b288ea32f34d507a4733d
Reviewed-on: https://go-review.googlesource.com/19240
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
libcurl sends this (despite never being standardized), and the Google
GFE rejects it with a 400 bad request (but only when over http2?).
So nuke it.
Change-Id: I3fc95523d50f33a0e23bb26b9195f70ab0aed0f4
Reviewed-on: https://go-review.googlesource.com/19184
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Previous flakes:
https://build.golang.org/log/223365dedb6b6aa0cfdf5afd0a50fd433a16badehttps://build.golang.org/log/edbea4cd3f24e707ef2ae8378559bb0fcc453c22
Dmitry says in email about this:
> The stack trace points to it pretty clearly. Done can indeed unblock
> Wait first and then panic. I guess we need to recover after first
> Done as well.
And it looks like TestWaitGroupMisuse2 was already hardened against
this. Do the same in TestWaitGroupMisuse3.
Change-Id: I317800c7e46f13c97873f0873c759a489dd5f47d
Reviewed-on: https://go-review.googlesource.com/19183
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The loading of zversion.go was expecting it to be in
package runtime, but it moved to runtime/internal/sys.
Worse, the load was not checking the error.
Update the path, check the error, add a test.
Fixes#14176.
Change-Id: I203c40afe1448875581415d5e42c29f09b14545d
Reviewed-on: https://go-review.googlesource.com/19180
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
RFC Errata 4522 (http://www.rfc-editor.org/errata_search.php?eid=4522)
notes that RFC 2616 had a typo in a list of headers that the
httputil.ReverseProxy code copied. Fix the typo in our code.
Fixes#14174
Change-Id: Ifc8f18fd58a6508a02a23e54ff3c473f03e521d3
Reviewed-on: https://go-review.googlesource.com/19133
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This corrects a regression from Go 1.5 introduced by CL 18317.
Fixes#14185.
Change-Id: Ic3215714846d9f28809cd04e3eb3664b599244f4
Reviewed-on: https://go-review.googlesource.com/19151
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently it's possible for the scheduler to deadlock with the right
confluence of locked Gs, assists, and scheduling of background mark
workers. Broadly, this happens because handoffp is stricter than
findrunnable, and if the only work for a P is GC work, handoffp will
put the P into idle, rather than starting an M to execute that P. One
way this can happen is as follows:
0. There is only one user G, which we'll call G 1. There is more than
one P, but they're all idle except the one running G 1.
1. G 1 locks itself to an M using runtime.LockOSThread.
2. GC starts up and enters mark 1.
3. G 1 performs a GC assist, which completes mark 1 without being
fully satisfied. Completing mark 1 causes all background mark
workers to park. And since the assist isn't fully satisfied, it
parks as well, waiting for a background mark worker to satisfy its
remaining assist debt.
4. The assist park enters the scheduler. Since G 1 is locked to the M,
the scheduler releases the P and calls handoffp to hand the P to
another M.
5. handoffp checks the local and global run queues, which are empty,
and sees that there are idle Ps, so rather than start an M, it puts
the P into idle.
At this point, all of the Gs are waiting and all of the Ps are idle.
In particular, none of the GC workers are running, so no mark work
gets done and the assist on the main G is never satisfied, so the
whole process soft locks up.
Fix this by making handoffp start an M if there is GC work. This
reintroduces a key invariant: that in any situation where findrunnable
would return a G to run on a P, handoffp for that P will start an M to
run work on that P.
Fixes#13645.
Tested by running 2,689 iterations of `go tool dist test -no-rebuild
runtime:cpu124` across 10 linux-amd64-noopt VMs with no failures.
Without this change, the failure rate was somewhere around 1%.
Performance change is negligible.
name old time/op new time/op delta
XBenchGarbage-12 2.48ms ± 2% 2.48ms ± 1% -0.24% (p=0.000 n=92+93)
name old time/op new time/op delta
BinaryTree17-12 2.86s ± 2% 2.87s ± 2% ~ (p=0.667 n=19+20)
Fannkuch11-12 2.52s ± 1% 2.47s ± 1% -2.05% (p=0.000 n=18+20)
FmtFprintfEmpty-12 51.7ns ± 1% 51.5ns ± 3% ~ (p=0.931 n=16+20)
FmtFprintfString-12 170ns ± 1% 168ns ± 1% -0.65% (p=0.000 n=19+19)
FmtFprintfInt-12 160ns ± 0% 160ns ± 0% +0.18% (p=0.033 n=17+19)
FmtFprintfIntInt-12 265ns ± 1% 273ns ± 1% +2.98% (p=0.000 n=17+19)
FmtFprintfPrefixedInt-12 235ns ± 1% 239ns ± 1% +1.99% (p=0.000 n=16+19)
FmtFprintfFloat-12 315ns ± 0% 315ns ± 1% ~ (p=0.250 n=17+19)
FmtManyArgs-12 1.04µs ± 1% 1.05µs ± 0% +0.87% (p=0.000 n=17+19)
GobDecode-12 7.93ms ± 0% 7.85ms ± 1% -1.03% (p=0.000 n=16+18)
GobEncode-12 6.62ms ± 1% 6.58ms ± 1% -0.60% (p=0.000 n=18+19)
Gzip-12 322ms ± 1% 320ms ± 1% -0.46% (p=0.009 n=20+20)
Gunzip-12 42.5ms ± 1% 42.5ms ± 0% ~ (p=0.751 n=19+19)
HTTPClientServer-12 69.7µs ± 1% 70.0µs ± 2% ~ (p=0.056 n=19+19)
JSONEncode-12 16.9ms ± 1% 16.7ms ± 1% -1.13% (p=0.000 n=19+19)
JSONDecode-12 61.5ms ± 1% 61.3ms ± 1% -0.35% (p=0.001 n=20+17)
Mandelbrot200-12 3.94ms ± 0% 3.91ms ± 0% -0.67% (p=0.000 n=20+18)
GoParse-12 3.71ms ± 1% 3.70ms ± 1% ~ (p=0.244 n=17+19)
RegexpMatchEasy0_32-12 101ns ± 1% 102ns ± 2% +0.54% (p=0.037 n=19+20)
RegexpMatchEasy0_1K-12 349ns ± 0% 350ns ± 0% +0.33% (p=0.000 n=17+18)
RegexpMatchEasy1_32-12 84.5ns ± 2% 84.2ns ± 1% -0.43% (p=0.048 n=19+20)
RegexpMatchEasy1_1K-12 510ns ± 1% 513ns ± 2% +0.58% (p=0.002 n=18+20)
RegexpMatchMedium_32-12 132ns ± 1% 134ns ± 1% +0.95% (p=0.000 n=20+20)
RegexpMatchMedium_1K-12 40.1µs ± 1% 39.6µs ± 1% -1.39% (p=0.000 n=20+20)
RegexpMatchHard_32-12 2.08µs ± 0% 2.06µs ± 1% -0.95% (p=0.000 n=18+18)
RegexpMatchHard_1K-12 62.2µs ± 1% 61.9µs ± 1% -0.42% (p=0.001 n=19+20)
Revcomp-12 537ms ± 0% 536ms ± 0% ~ (p=0.076 n=20+20)
Template-12 71.3ms ± 1% 69.3ms ± 1% -2.75% (p=0.000 n=20+20)
TimeParse-12 361ns ± 0% 360ns ± 1% ~ (p=0.056 n=19+19)
TimeFormat-12 353ns ± 0% 352ns ± 0% -0.23% (p=0.000 n=17+18)
[Geo mean] 62.6µs 62.5µs -0.17%
Change-Id: I0fbbbe4d7d99653ba5600ffb4394fa03558bc4e9
Reviewed-on: https://go-review.googlesource.com/19107
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates x/net/http2 to git rev 644ffc for three CLs since the last update:
http2: don't add *Response to activeRes in Transport on Headers.END_STREAM
https://golang.org/cl/19134
http2: add mechanism to send undeclared Trailers mid handler
https://golang.org/cl/19131
http2: remove unused variable
https://golang.org/cl/18936
The first in the list above is the main fix that's necessary. The
other are two are in the git history but along for the cmd/bundle
ride. The middle CL is well-tested, small (mostly comments),
non-tricky, and almost never seen (since nobody really uses Trailers).
The final CL is just deleting an unused global variable.
Fixes#14084 again (with more tests)
Change-Id: Iac51350acee9c51d32bf7779d57e9d5a5482b928
Reviewed-on: https://go-review.googlesource.com/19135
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Tested by hand with a runtime/cgo modified to return an mmap failure
after 10 calls.
This is an interim patch. For 1.7 we should fix mmap properly to avoid
using the same value as both a pointer and an errno value.
Fixes#14149.
Change-Id: I8f2bbd47d711e283001ba73296f1c34a26c59241
Reviewed-on: https://go-review.googlesource.com/19084
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
