Keith Randall [Mon, 3 Jun 2019 17:03:44 +0000 (10:03 -0700)]
runtime: get map of args of unstarted goroutines like we do for defers
Normally, reflect.makeFuncStub records the context value at a known
point in the stack frame, so that the runtime can get the argument map
for reflect.makeFuncStub from that known location.
This doesn't work for defers or goroutines that haven't started yet,
because they haven't allocated a frame or run an instruction yet. The
argument map must be extracted from the context value. We already do
this for defers (the non-nil ctxt arg to getArgInfo), we just need to
do it for unstarted goroutines as well.
When we traceback a goroutine, remember the context value from
g.sched. Use it for the first frame we find.
(We never need it for deeper frames, because we normally don't stop at
the start of reflect.makeFuncStub, as it is nosplit. With this CL we
could allow makeFuncStub to no longer be nosplit.)
Andrew Gerrand [Fri, 31 May 2019 06:58:44 +0000 (16:58 +1000)]
cmd/cover: fix counting of blocks split by goto statements
When adding coverage counters to a block, the block's statement list is
mutated. CL 77150 removed the part where the mutated list is assigned
back to its parent node; this was confusing ast.Walk, which would then
lose its place and stop walking the current block, dropping counters in
the process.
This change has addCounters make a copy of the list before mutating
it, so that the original list doesn't change under Walk's feet.
Fix #32200
Change-Id: Ia3b67d8cee860ceb7caf8748cb7a80ff9c6276e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/179581 Reviewed-by: Rob Pike <r@golang.org>
Jay Conrod [Fri, 31 May 2019 18:14:00 +0000 (14:14 -0400)]
cmd/go: ignore build tags when 'go get' modifies build list
In module mode, 'go get' should not consider build constraints when
loading packages in order to modify the module graph. With this
change, 'go get' considers all build tags to be true except for
"ignore" and malformed build constraint expressions.
When 'go get' builds packages, it still applies build constraints for
the target platform.
Fixes #32345
Change-Id: I6dceae6f10a5185870537de730b36292271ad124
Reviewed-on: https://go-review.googlesource.com/c/go/+/179898 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Tue, 28 May 2019 21:59:23 +0000 (14:59 -0700)]
cmd/compile: don't move nil checks across a VarDef
We need to make sure that there's no possible faulting
instruction between a VarDef and that variable being
fully initialized. If there was, then anything scanning
the stack during the handling of that fault will see
a live but uninitialized variable on the stack.
If we have:
NilCheck p
VarDef x
x = *p
We can't rewrite that to
VarDef x
NilCheck p
x = *p
Particularly, even though *p faults on p==nil, we still
have to do the explicit nil check before the VarDef.
Cherry Zhang [Fri, 31 May 2019 21:11:50 +0000 (17:11 -0400)]
cmd/compile: make sure build works when intrinsics are disabled
Some runtime functions, like getcallerpc/sp, don't have Go or
assembly implementations and have to be intrinsified. Make sure
they are, even if intrinsics are disabled.
This makes "go build -gcflags=all=-d=ssa/intrinsics/off hello.go"
work.
Bryan C. Mills [Fri, 31 May 2019 19:24:49 +0000 (15:24 -0400)]
cmd/go/internal/modfetch: use the resolved version to search for tags in (*codeRepo).convert
Previously, we used the passed-in statVers as the basis for tag search,
but it is not always valid.
Instead, use info.Name, which (by precondition) must be valid.
Updates #32161
Updates #27171
Change-Id: Iaecb5043bdf2fefd26fbe3f8e3714b07d22f580f
Reviewed-on: https://go-review.googlesource.com/c/go/+/179857
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Than McIntosh [Fri, 31 May 2019 16:05:07 +0000 (12:05 -0400)]
cmd/link: revise test case to work on pre-10.14 macos
Rework this recently introduced test case to insure that it works with
older versions of the OS. It was using a new framework library not
available on pre-10.14 to trigger the weak symbol reference; switch to
using a new symbol from an existing library. Tested on MacOS 10.14 and
10.11.
Updates #32233.
Change-Id: I1fe2a9255fca46cb7cdf33ff7fed67bba86fdc22
Reviewed-on: https://go-review.googlesource.com/c/go/+/179837
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Andrew Gerrand [Fri, 31 May 2019 10:59:35 +0000 (20:59 +1000)]
strconv: document handling of NaN and ±Inf
In addition to the example that was added in 203b80ab, mention these
special cases in the doc comment. This change also adjusts the example
to include "+Inf", as it was not otherwise mentioned that the plus
symbol may be present.
Brad Fitzpatrick [Wed, 29 May 2019 21:49:20 +0000 (21:49 +0000)]
net/http: prevent Transport from spamming stderr on server 408 reply
HTTP 408 responses now exist and are seen in the wild (e.g. from
Google's GFE), so make Go's HTTP client not spam about them when seen.
They're normal (now).
Greg Thelen [Sat, 25 May 2019 18:44:44 +0000 (11:44 -0700)]
syscall: use Ctty before fd shuffle
On unix if exec.Command() is given both ExtraFiles and Ctty, and the
Ctty file descriptor overlaps the range of FDs intended for the child,
then cmd.Start() the ioctl(fd,TIOCSCTTY) call fails with an
"inappropriate ioctl for device" error.
When child file descriptors overlap the new child's ctty the ctty will
be closed in the fd shuffle before the TIOCSCTTY. Thus TIOCSCTTY is
used on one of the ExtraFiles rather than the intended Ctty file. Thus
the error.
exec.Command() callers can workaround this by ensuring the Ctty fd is
larger than any ExtraFiles destined for the child.
Fix this by doing the ctty ioctl before the fd shuffle.
Test for this issue by modifying TestTerminalSignal to use more
ExtraFiles. The test fails on linux and freebsd without this change's
syscall/*.go changes. Other platforms (e.g. darwin, aix, solaris) have
the same fd shuffle logic, so the same fix is applied to them. However,
I was only able to test on linux (32 and 64 bit) and freebsd (64 bit).
Manual runs of the test in https://golang.org/issue/29458 start passing
with this patch:
Before:
% /tmp/src/go/bin/go run t
successfully ran child process with ParentExtraFileFdNum=5, ChildExtraFileFd=6, ParentPtyFd=7
panic: failed to run child process with ParentExtraFileFdNum=10, ChildExtraFileFd=11, ParentPtyFd=11: fork/exec /bin/true: inappropriate ioctl for device
After:
% /tmp/src/go/bin/go run t
successfully ran child process with ParentExtraFileFdNum=5, ChildExtraFileFd=6, ParentPtyFd=7
successfully ran child process with ParentExtraFileFdNum=10, ChildExtraFileFd=11, ParentPtyFd=11
Fixes #29458
Change-Id: I99513de7b6073c7eb855f1eeb4d1f9dc0454ef8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/178919
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Johan Brandhorst [Tue, 28 May 2019 20:51:54 +0000 (21:51 +0100)]
net/http: enable WASM fetch where supported
The existing check was introduced to allow tests to pass
on WASM without an environment where the fetch RoundTripper
could run. However, the check now prohibits the use of the
Fetch RoundTripper in all WASM tests, even where the
RoundTripper could run. The new change should only disable
the RoundTripper when used in an environment without fetch.
Matthew Dempsky [Thu, 28 Mar 2019 21:35:49 +0000 (14:35 -0700)]
cmd/compile: fix package initialization ordering
This CL rewrites cmd/compile's package-level initialization ordering
algorithm to be compliant with the Go spec. See documentation in
initorder.go for details.
Incidentally, this CL also improves fidelity of initialization loop
diagnostics by including referenced functions in the emitted output
like go/types does.
Fixes #22326.
Change-Id: I7c9ac47ff563df4d4f700cf6195387a0f372cc7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/170062
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Jason A. Donenfeld [Sun, 26 May 2019 13:28:05 +0000 (15:28 +0200)]
cmd/link: do not generate NT 4 compatibility binaries
Incredibly, the subsystem version numbers in the PE header influence how
win32k handles various syscalls. The first time a win32k syscall is
invoked and the kernel upgrades the thread object to a tagTHREADINFO
with all of the lovely undocumented UI members and such, it sets the
dwExpWinVer member (offset 624 in Windows 10 build 1809) to the result
of RtlGetExpWinVer(PsGetProcessSectionBaseAddress(proc)).
RtlGetExpWinVer, also undocumented, then calls into the undocumented
RtlImageNtHeader function, which returns a fortunately documented
IMAGE_NT_HEADERS structure. It uses the subsystem members in there to
set the dwExpWinVer member of our newly minted tagTHREADINFO object.
Later, functions like SendInput consult this to vary their behaviors and
return values. In fact, littered through out win32k are checks like `if
(gsti->dwExpWinVer >= 0x501) { ... }`.
I don't think Go ever supported NT 4.0. These days the minimum version
is Windows 7, which is 6.1. So, let's set the version numbers in the PE
header at that, which should give us the behavior that MSDN advertises
for various functions, as opposed to bizarre archeological remnants.
Interestingly, I suspect that most people never noticed the brokenness,
because most people trying to do serious Win32 UI stuff wind up linking
in cgo, if not for actually using C, then just to have a larger system
stack so that the stack doesn't get corrupted by various UI functions.
When MingW is used, the PE header gets a later version. But recently
there's been a bug report of some people trying to do more modest UI
manipulation using SendInput in a setting where this cgo hack probably
isn't required, so they ran into the weird historical compatibility
stuff.
Fixes #31685
Change-Id: I54461ce820f6e9df349e37be5ecc5a44c04a3e26
Reviewed-on: https://go-review.googlesource.com/c/go/+/178977
Run-TryBot: Jason Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Brad Fitzpatrick [Tue, 28 May 2019 17:45:22 +0000 (17:45 +0000)]
net/http: quiet some log spam in tests
One of these tests creates a bunch of connections concurrently, then
discovers it doesn't need them all, which then makes the server log
that the client went away midway through the TLS handshake. Perhaps
the server should recognize that as a case not worthy of logging
about, but this is a safer way to eliminate the stderr spam during go
test for now.
The other test's client gives up on its connection and closes it,
similarly confusing the server.
cmd/compile: process blocks containing only dead values in fuseIf
The code in #29218 resulted in an If block containing only its control.
That block was then converted by fuseIf into a plain block;
as a result, that control value was dead.
However, the control value was still present in b.Values.
This prevented further fusing of that block.
This change beefs up the check in fuseIf to allow fusing
blocks that contain only dead values (if any).
In the case of #29218, this enables enough extra
fusing that the control value could be eliminated,
allowing all values in turn to be eliminated.
This change also fuses 34 new blocks during make.bash.
It is not clear that this fixes every variant of #29218,
but it is a reasonable standalone change.
And code like #29218 is rare and fundamentally buggy,
so we can handle new instances if/when they actually occur.
Rob Pike [Fri, 17 May 2019 03:43:51 +0000 (13:43 +1000)]
cmd/doc: always print package clause except for commands
There was an implicit heuristic before about when to print the
package clause or omit it, but it was undocumented and confusing.
Get rid of it and print it always unless asking for the package
docs for a command, which are more of a usage message than a
programming question. This simplifies the processing.
There are several paths to the output, so to put the fix in one
place we place a wrapper before the output buffer than adds the
clause when Write is first called.
The tests don't verify this behavior, but they didn't before either.
Unsure what the right approach is but this will do for now.
Ian Lance Taylor [Wed, 1 May 2019 04:06:01 +0000 (21:06 -0700)]
runtime: remove VDSO fallback test and benchmarks
These tests assume that it is OK to switch between time implementations,
but the clock_gettime call uses CLOCK_MONOTONIC and the fallback call,
gettimeofday, uses CLOCK_REALTIME. Disabling the clock_gettime call means
that calls to nanotime will start returning very different values.
This breaks the new timer code, which assumes that nanotime will return
a consistently increasing value.
This test is not very useful in any case as it doesn't check the results.
Removing this file also removes BenchmarkTimeNow, which is a duplicate
of BenchmarkNow in the time package.
Russ Cox [Fri, 24 May 2019 12:55:30 +0000 (08:55 -0400)]
cmd/go: respect default proxy setting, add direct fallback
Getenv("GOPROXY") says what the environment variable is
(including looking in the go env file), but it doesn't include
the default setting. This code needs to use cfg.GOPROXY
to get the actual default. Fix and test that.
Also, we forgot to include the fallback to direct for when
the proxy serves a 404. Add and test that too.
Also add HTTP fetch information to -x build flag output.
(It does not belong in the -v output, despite the GOPATH go get
command doing this.)
Change-Id: Ieab7ef13cda3e1ad041dbe04921af206e2232c9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/178720
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Bryan C. Mills [Thu, 16 May 2019 13:21:49 +0000 (09:21 -0400)]
cmd/go: when resolving packages, try all module paths before falling back to the next proxy
Since we're mucking with error-propagation in modload.Query* anyway,
simplify the classification logic. Ensure that “module not found”
errors are reported as such in various places, since non-“not found”
errors terminate the module search.
Following the previous CL, this removes more global variables on
Wasm.
PC_B is used mostly for intra-function jumps, and for a function
telling its callee where to start or resume. This usage can be
served by a parameter. The top level loop (wasm_pc_f_loop) uses
PC_B for resuming a function. This value is either set by gogo,
or loaded from the Go stack at function return. Instead of
loading PC_B at each function return, we could make gogo stores
PC_B at the same stack location, and let the top level loop do
the load. This way, we don't need to use global PC_B to
communicate with the top level loop, and we can replace global
PC_B with a parameter.
PC_F is similar. It is even more so in that the only reader is
the top level loop. Let the top level loop read it from the stack,
and we can get rid of PC_F entirely.
PC_F and PC_B are used less entensively as SP, so this CL has
smaller performance gain.
Michael Anthony Knyszek [Fri, 17 May 2019 14:48:04 +0000 (14:48 +0000)]
runtime: ensure mheap lock stack growth invariant is maintained
Currently there's an invariant in the runtime wherein the heap lock
can only be acquired on the system stack, otherwise a self-deadlock
could occur if the stack grows while the lock is held.
This invariant is upheld and documented in a number of situations (e.g.
allocManual, freeManual) but there are other places where the invariant
is either not maintained at all which risks self-deadlock (e.g.
setGCPercent, gcResetMarkState, allocmcache) or is maintained but
undocumented (e.g. gcSweep, readGCStats_m).
This change adds go:systemstack to any function that acquires the heap
lock or adds a systemstack(func() { ... }) around the critical section,
where appropriate. It also documents the invariant on (*mheap).lock
directly and updates repetitive documentation to refer to that comment.
Martin Sucha [Thu, 23 May 2019 18:34:17 +0000 (20:34 +0200)]
strings: clarify example of ContainsAny
I have seen code that literally copied the example like this:
if strings.ContainsAny(s, "1 & 2 & 3") {
The developer apparently thought that this is the way to
specify multiple characters and I noticed this pattern
being used in the example. Let's update the example so
that it's clear how multiple Unicode code points should
be specified.
Richard Musiol [Wed, 15 May 2019 23:03:10 +0000 (01:03 +0200)]
syscall/js: replace TypedArrayOf with CopyBytesToGo/CopyBytesToJS
The typed arrays returned by TypedArrayOf were backed by WebAssembly
memory. They became invalid each time we grow the WebAssembly memory.
This made them very error prone and hard to use correctly.
This change removes TypedArrayOf completely and instead introduces
CopyBytesToGo and CopyBytesToJS for copying bytes between a byte
slice and an Uint8Array. This breaking change is still allowed for
the syscall/js package.
bill_ofarrell [Thu, 16 May 2019 16:45:52 +0000 (12:45 -0400)]
crypto/ecdsa: implement ecdsa on s390x for P256/P384/P521 using KDSA instruction
Utilize KDSA when available. This guarantees constant time operation on all three curves mentioned,
and is faster than conventional assembly. The IBM Z model(s) that support KDSA as used in this CL
are not yet publicly available, and so we are unable to release performance data at this time.
Change-Id: I85360dcf90fe42d2bf32afe3f638e282de10a518
Reviewed-on: https://go-review.googlesource.com/c/go/+/174437
Run-TryBot: Michael Munday <mike.munday@ibm.com> Reviewed-by: Michael Munday <mike.munday@ibm.com>
Brad Fitzpatrick [Thu, 23 May 2019 14:01:16 +0000 (14:01 +0000)]
SECURITY.md: add security file
This is now recognized and recommended by GitHub.
Fixes #32201
Change-Id: Iafb5ef1b2bee5f021a711b0b758aaf6a74758c5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/178697 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad Fitzpatrick [Thu, 23 May 2019 19:06:16 +0000 (19:06 +0000)]
test: skip a test on failing test on nacl/386
This test was designed for #15609 and didn't consider nacl. It's not
worth adding new +build-guarded assembly files in issue15609.dir for
nacl, especially as nacl is going away.
Fixes #32206
Change-Id: Ic5bd48b4f790a1f7019100b8a72d4688df75512f
Reviewed-on: https://go-review.googlesource.com/c/go/+/178698
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Caleb Spare [Sat, 12 Jan 2019 22:29:08 +0000 (14:29 -0800)]
strconv: fix rounding in FormatFloat fallback path
Float formatting uses a multiprecision fallback path where Grisu3
algorithm fails. This has a bug during the rounding phase: the
difference between the decimal value and the upper bound is examined
byte-by-byte and doesn't properly handle the case where the first
divergence has a difference of 1.
For instance (using an example from #29491), for the number 498484681984085570, roundShortest examines the three decimal values:
After examining the 16th digit, we know that rounding d up will fall
within the bounds unless all remaining digits of d are 9 and all
remaining digits of upper are 0:
d: ...855xx
upper: ...856xx
However, the loop forgets that d and upper have already diverged and
then on the next iteration sees that the 17th digit of d is actually
lower than the 17th digit of upper and decides that we still can't round
up:
Fix it by remembering when we've seen divergence in previous digits.
This CL also fixes another bug in the same loop: for some inputs, the
decimal value d or the lower bound may have fewer digits than the upper
bound, yet the iteration through the digits starts at i=0 for each of
them. For instance, given the float64 value 1e23, we have
but the loop starts by comparing '9' to '1' rather than '0' to '1'.
I haven't found any cases where this second bug causes incorrect output
because when the digit comparison fails on the first loop iteration the
upper bound always has more nonzero digits (i.e., the expression
'i+1 < upper.nd' is always true).
Russ Cox [Tue, 21 May 2019 13:03:26 +0000 (09:03 -0400)]
cmd/go: default to GOPROXY=https://proxy.golang.org and GOSUMDB=sum.golang.org
This CL changes the default module download and module verification mechanisms
to use the Go module mirror and Go checksum database run by Google.
See https://proxy.golang.org/privacy for the services' privacy policy.
(Today, that URL is a redirect to Google's standard privacy policy,
which covers these services as well. If we publish a more specific
privacy policy just for these services, that URL will be updated to
display or redirect to it.)
See 'go help modules' and 'go help modules-auth' for details (added in this CL).
To disable the mirror and checksum database for non-public modules:
go env -w GONOPROXY=*.private.net,your.com/*
go env -w GONOSUMDB=*.private.net,your.com/*
(If you are using a private module proxy then you'd only do the second.)
If you run into problems with the behavior of the go command when using
the Go module mirror or the Go checksum database, please file issues at
https://golang.org/issue/new, so that we can address them for the
Go 1.13 release.
Mickey Reiss [Thu, 23 May 2019 05:15:49 +0000 (05:15 +0000)]
bufio: Fix typo in scan.go documentation
Apologies for the the nitpicky PR. I believe there is a minor typo in the documentation of `MaxScanTokenSize`, which confused me for a moment when I went to search for the referenced method, `Scan.Buffer`. Thanks!
Ariel Mashraki [Sun, 10 Feb 2019 20:44:03 +0000 (22:44 +0200)]
text/template: add a slice function to the predefined global functions
The new slice function returns the result of slicing its first argument by
the following arguments. Thus {{slice x 1 3}} is, in Go syntax, x[1:3].
Each sliced item must be a string, slice, or array.
Closed #30153
RELNOTE=yes
Change-Id: I63188c422848cee3d383a64dc4d046e3a1767c63
Reviewed-on: https://go-review.googlesource.com/c/go/+/161762 Reviewed-by: Rob Pike <r@golang.org>
LE Manh Cuong [Fri, 17 May 2019 10:25:07 +0000 (17:25 +0700)]
test/fixedbugs: fix some tests will not be run
Currently, some tests under test/fixedbugs never run:
$ for d in test/fixedbugs/*.dir; do
! test -f "${d%.dir}.go" && echo "$d"
done
test/fixedbugs/issue15071.dir
test/fixedbugs/issue15609.dir
test/fixedbugs/issue29612.dir
Because they missed the corresponding ".go" file, so "go run run.go"
will skip them.
Add missing ".go" files for those tests to make sure they will be
collected and run.
While at it, add another action "runindir", which does "go run ."
inside the t.goDirName then check the output.
Change-Id: I88000b3663a6a615d90c1cf11844ea0377403e3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/177798
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad Fitzpatrick [Tue, 21 May 2019 19:02:00 +0000 (19:02 +0000)]
cmd/dist: support using cross-compiled std test binaries for slow builders
We want the builders to be able to cross-compile test binaries for a
few of the super slow builders that require either slow hardware or
slow full CPU emulation.
Updates golang/go#31217
Change-Id: I8d33b18efaf788f6f131354b2917ac9738ca975e
Reviewed-on: https://go-review.googlesource.com/c/go/+/178399
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Matthew Dempsky [Wed, 22 May 2019 18:06:09 +0000 (11:06 -0700)]
cmd/compile: fix capture-by-reference of return parameters
As an optimization, function literals capture variables by value when
they're not assigned and their address has not been taken. Because
result parameters are implicitly assigned through return statements
(which do not otherwise set the "assigned" flag), result parameters
are explicitly handled to always capture by reference.
However, the logic was slightly mistaken because it was only checking
if the variable in the immediately enclosing context was a return
parameter, whereas in a multiply-nested function literal it would
itself be another closure variable (PAUTOHEAP) rather than a return
parameter (PPARAMOUT).
The fix is to simply test the outermost variable, like the rest of the
if statement's tests were already doing.
Filippo Valsorda [Wed, 22 May 2019 15:10:06 +0000 (11:10 -0400)]
crypto/x509: include roots with empty or multiple policies on macOS
To a fifth reading of the relevant docs, it looks like
1) a constraint dictionary with no policy applies to all of them;
2) multiple applying constraint dictionaries should have their results OR'd;
3) untrusted certificates in the keychain should be used for chain building.
This fixes 1), approximates 2) and punts on 3).
Fixes #30672
Fixes #30471
Change-Id: Ibbaabf0b77d267377c0b5de07abca3445c2c2302
Reviewed-on: https://go-review.googlesource.com/c/go/+/178539 Reviewed-by: Adam Langley <agl@golang.org>
Filippo Valsorda [Tue, 21 May 2019 18:54:54 +0000 (14:54 -0400)]
crypto/x509: fix value ownership in isSSLPolicy on macOS
CFDictionaryGetValueIfPresent does not take ownership of the value, so
releasing the properties dictionary before passing the value to CFEqual
can crash. Not really clear why this works most of the time.
See https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html
Fixes #28092
Hopefully fixes #30763
Change-Id: I5ee7ca276b753a48abc3aedfb78b8af68b448dd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/178537 Reviewed-by: Adam Langley <agl@golang.org>
Russ Cox [Thu, 16 May 2019 00:47:25 +0000 (20:47 -0400)]
misc/cgo/test: consolidate tests into fewer cgo source files
Each different file that does import "C" must be compiled
and analyzed separately by cgo. Having fewer files import "C"
reduces the cost of building the test. This is especially important
because this test is built and run four different times (with different
settings) during all.bash.
go test -c in this directory used to take over 20 seconds on my laptop.
Now it takes under 5 seconds.
Keith Randall [Tue, 21 May 2019 05:01:12 +0000 (01:01 -0400)]
runtime: revert init order changes
First, remove the randomization of initialization order.
Then, revert to source code order instead of sorted package path order.
This restores the behavior that was in 1.12.
A larger change which will implement the suggestion in #31636 will
wait for 1.14. It's too complicated for 1.13 at this point (it has
tricky interactions with plugins).
Fixes #31636
Change-Id: I35b48e8cc21cf9f93c0973edd9193d2eac197628
Reviewed-on: https://go-review.googlesource.com/c/go/+/178297
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Tue, 21 May 2019 12:24:27 +0000 (08:24 -0400)]
all: remove PEM-encoded private keys from tests
Gerrit is complaining about pushes that affect these files
and forcing people to use -o nokeycheck, which defeats
the point of the check. Hide the keys from this kind of scan
by marking them explicitly as testing keys.
This is a little annoying but better than training everyone
who ever edits one of these test files to reflexively override
the Gerrit check.
The only remaining keys explicitly marked as private instead
of testing are in examples, and there's not much to do
about those. Hopefully they are not edited as much.
Russ Cox [Thu, 16 May 2019 14:00:10 +0000 (10:00 -0400)]
misc/cgo/errors: consolidate test work
Build a single binary containing all the TestPointerChecks
instead of building many small binaries,
each with its own cgo+compile+link invocation.
This cuts 'go test -run=TestPointerChecks'
from 6.7r 35.5u 26.1s to 2.1r 2.1u 1.4s.
Move as many cgo checks as possible into fewer test files
for TestReportsTypeErrors too.
This cuts 'go test -run=TestReportsTypeErrors'
from 2.1r 6.7u 6.7s to 1.5r 2.5u 2.5s.
After this change, all.bash runs in ~4:30 on my laptop.
For #26473.
Change-Id: I3787448b03689a1f62dd810957ab6013bb75582f
Reviewed-on: https://go-review.googlesource.com/c/go/+/177599
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
LE Manh Cuong [Wed, 15 May 2019 19:28:47 +0000 (02:28 +0700)]
cmd/compile: fix typecheck type alias makes wrong export symbol metadata
typecheck type alias always replaces the original definition of the symbol.
This is wrong behavior because if the symbol's definition is replaced by a
local type alias, it ends up being written to compiled file as an alias,
instead of the original type.
To fix, only replace the definition of symbol with global type alias.
Russ Cox [Thu, 16 May 2019 04:12:34 +0000 (00:12 -0400)]
test: skip cross-arch codegen tests in all.bash
The test/codegen tests check all architectures
mentioned in the test file, but this requires
building at least the runtime for that architecture.
This CL changes the test to only check the local
architecture, leaving checking of other architectures
to the relevant builders, as usual.
This cuts 'go run run.go codegen' by 12r 78u 21s.
After this change, all.bash runs in ~4:40 on my laptop.
For #26473.
Change-Id: Ia0354d1aff2df2949f838528c8171410bc42dc8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/177577
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
LE Manh Cuong [Fri, 3 May 2019 17:24:53 +0000 (00:24 +0700)]
cmd/compile: clarify the difference between types.Sym and obj.LSym
Both types.Sym and obj.LSym have the field Name, and that field is
widely used in compiler source. It can lead to confusion that when to
use which one.
So, adding documentation for clarifying the difference between them,
eliminate the confusion, or at least, make the code which use them
clearer for the reader.
See https://github.com/golang/go/issues/31252#issuecomment-481929174
Change-Id: I31f7fc6e4de4cf68f67ab2e3a385a7f451c796f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/175019 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Michael Anthony Knyszek [Tue, 14 May 2019 19:59:57 +0000 (19:59 +0000)]
runtime: overhaul TestPhysicalMemoryUtilization
Currently, this test allocates many objects and relies on heap-growth
scavenging to happen unconditionally on heap-growth. However with the
new pacing system for the scavenging, this is no longer true and the
test is flaky.
So, this change overhauls TestPhysicalMemoryUtilization to check the
same aspect of the runtime, but in a much more robust way.
Firstly, it sets up a much more constrained scenario: only 5 objects are
allocated total with a maximum worst-case (i.e. the test fails) memory
footprint of about 16 MiB. The test is now aware that scavenging will
only happen if the heap growth causes us to push way past our scavenge
goal, which is based on the heap goal. So, it makes the holes in the
test much bigger and the actual retained allocations much smaller to
keep the heap goal at the heap's minimum size. It does this twice to
create exactly two unscavenged holes. Because the ratio between the size
of the "saved" objects and the "condemned" object is so small, two holes
are sufficient to create a consistent test.
Then, the test allocates one enormous object (the size of the 4 other
objects allocated, combined) with the intent that heap-growth scavenging
should kick in and scavenge the holes. The heap goal will rise after
this object is allocated, so it's very important we do all the
scavenging in a single allocation that exceeds the heap goal because
otherwise the rising heap goal could foil our test.
Finally, we check memory use relative to HeapAlloc as before. Since the
runtime should scavenge the entirety of the remaining holes,
theoretically there should be no more free and unscavenged memory.
However due to other allocations that may happen during the test we may
still see unscavenged memory, so we need to have some threshold. We keep
the current 10% threshold which, while arbitrary, is very conservative
and should easily account for any other allocations the test makes.
Before, we also had to ensure the allocations we were making looked
large relative to the size of a heap arena since newly-mapped memory was
considered unscavenged, and so that could significantly skew the test.
However, thanks to the fix for #32012 we were able to reduce memory use
to 16 MiB in the worst case.
Shulhan [Fri, 10 May 2019 15:38:56 +0000 (22:38 +0700)]
internal/envcmd: print GO111MODULE when executing "go env"
If we look at the issues in the past releases that are related
to go command that involved modules, its usually mention or ask about
the value of GO111MODULE, either in separate line or in separate
comment.
There are quite long time range before GO111MODULE will be removed
(unused). The next release is still default to auto [1], and until Go
1.13 unsupported (two releases after that) there is about one and half
years after that.
Since the change is not that big (one line) [2], maybe temporary adding
it to "go env" give more clarity and benefit in issue reporting rather
than not.
[1] https://github.com/golang/go/issues/31857
Fixes #29656
Change-Id: I609ad6664774018e4f4147ec6158485172968e16
Reviewed-on: https://go-review.googlesource.com/c/go/+/176837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Brad Fitzpatrick [Sun, 19 May 2019 17:06:24 +0000 (17:06 +0000)]
net/http/httptest: update docs, remove old inaccurate sentence
The "After it is called, changing rw.Header will not affect
rw.HeaderMap" claim predates the Result method which changed how the
Recorder should be used.
Fixes #32144
Fixes #32136
Change-Id: I95bdfa5ac489ce7b0202824bb5663f4da188e8a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/178058 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Brad Fitzpatrick [Sun, 19 May 2019 17:02:01 +0000 (17:02 +0000)]
cmd/go/internal/work: fix a couple typos
Change-Id: I357669d8c9bc004031b17f057803c9b152edefee
Reviewed-on: https://go-review.googlesource.com/c/go/+/178057 Reviewed-by: Ian Lance Taylor <iant@golang.org>