Michael Anthony Knyszek [Thu, 20 Dec 2018 20:21:45 +0000 (20:21 +0000)]
[release-branch.go1.11] runtime: skip TestLockOSThreadAvoidsStatePropagation if one can't unshare
This change splits a testprog out of TestLockOSThreadExit and makes it
its own test. Then, this change makes the testprog exit prematurely with
a special message if unshare fails with EPERM because not all of the
builders allow the user to call the unshare syscall.
Also, do some minor cleanup on the TestLockOSThread* tests.
Fixes #29366.
Change-Id: Id8a9f6c4b16e26af92ed2916b90b0249ba226dbe
Reviewed-on: https://go-review.googlesource.com/c/155437
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 429bae715876c69853bb63db1733f580e293c916)
Reviewed-on: https://go-review.googlesource.com/c/go/+/167707
Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Tobias Klauser [Thu, 6 Dec 2018 19:58:26 +0000 (20:58 +0100)]
[release-branch.go1.11] crypto/x509: explicitly cast printf format argument
After CL 128056 the build fails on darwin/386 with
src/crypto/x509/root_cgo_darwin.go:218:55: warning: values of type 'SInt32' should not be used as format arguments; add an explicit cast to 'int' instead [-Wformat]
go build crypto/x509: C compiler warning promoted to error on Go builders
Fix the warning by explicitly casting the argument to an int as
suggested by the warning.
Filippo Valsorda [Mon, 4 Feb 2019 23:08:43 +0000 (18:08 -0500)]
[release-branch.go1.11] crypto/x509: consider parents by Subject if AKID has no match
If a certificate somehow has an AKID, it should still chain successfully
to a parent without a SKID, even if the latter is invalid according to
RFC 5280, because only the Subject is authoritative.
This reverts to the behavior before #29233 was fixed in 770130659. Roots
with the right subject will still be shadowed by roots with the right
SKID and the wrong subject, but that's been the case for a long time, and
is left for a more complete fix in Go 1.13.
Cherry Zhang [Wed, 2 Jan 2019 17:27:55 +0000 (12:27 -0500)]
[release-branch.go1.11] cmd/compile: fix deriving from x+d >= w on overflow in prove pass
In the case of x+d >= w, where d and w are constants, we are
deriving x is within the bound of min=w-d and max=maxInt-d. When
there is an overflow (min >= max), we know only one of x >= min
or x <= max is true, and we derive this by excluding the other.
When excluding x >= min, we did not consider the equal case, so
we could incorrectly derive x <= max when x == min.
Filippo Valsorda [Mon, 6 Aug 2018 22:38:18 +0000 (18:38 -0400)]
[release-branch.go1.11] crypto/x509: fix root CA extraction on macOS (no-cgo path)
Certificates without any trust settings might still be in the keychain
(for example if they used to have some, or if they are intermediates for
offline verification), but they are not to be trusted. The only ones we
can trust unconditionally are the ones in the system roots store.
Moreover, the verify-cert invocation was not specifying the ssl policy,
defaulting instead to the basic one. We have no way of communicating
different usages in a CertPool, so stick to the WebPKI use-case as the
primary one for crypto/x509.
Updates #24652
Fixes #26039
Change-Id: Ife8b3d2f4026daa1223aa81fac44aeeb4f96528a
Reviewed-on: https://go-review.googlesource.com/c/128116 Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@golang.org>
(cherry picked from commit aa2415807781ba84bf917c62cb983dc1a44f2ad1)
Reviewed-on: https://go-review.googlesource.com/c/162861
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
Filippo Valsorda [Mon, 6 Aug 2018 19:41:34 +0000 (15:41 -0400)]
[release-branch.go1.11] crypto/x509: fix root CA extraction on macOS (cgo path)
The cgo path was not taking policies into account, using the last
security setting in the array whatever it was. Also, it was not aware of
the defaults for empty security settings, and for security settings
without a result type. Finally, certificates restricted to a hostname
were considered roots.
The API docs for this code are partial and not very clear, so this is a
best effort, really.
Updates #24652
Updates #26039
Change-Id: I8fa2fe4706f44f3d963b32e0615d149e997b537d
Reviewed-on: https://go-review.googlesource.com/c/128056
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@golang.org>
(cherry picked from commit f6be1cf109a2be59b96d1fa913adfa1fbc628579)
Reviewed-on: https://go-review.googlesource.com/c/162860 Reviewed-by: Andrew Bonventre <andybons@golang.org>
Yuval Pavel Zholkover [Fri, 1 Feb 2019 11:51:31 +0000 (13:51 +0200)]
[release-branch.go1.11] cmd/cgo: ignore unrecognized GCC warning group pragmas
CL 159859 causes build failure with old clang versions (3.4.1) on FreeBSD 10.3/10.4.
Update #29962
Reviewed-on: https://go-review.googlesource.com/c/160777
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 6f4dc1ccf9735013fdb7cd044bda29d19bebb906)
Ian Lance Taylor [Mon, 28 Jan 2019 20:31:55 +0000 (12:31 -0800)]
[release-branch.go1.11] cmd/cgo: disable GCC 9 warnings triggered by cgo code
GCC 9 has started emitting warnings when taking the address of a field
in a packed struct may cause a misaligned pointer. We use packed
structs in cgo to ensure that our field layout matches the C
compiler's layout. Our pointers are always aligned, so disable the warning
Updates #29962
Fixes #29967
Change-Id: I7e290a7cf694a2c2958529e340ebed9fcd62089c
Reviewed-on: https://go-review.googlesource.com/c/159859
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit f2a416b90ac68596ea05b97cefa8c72e7416e98f)
Reviewed-on: https://go-review.googlesource.com/c/160449
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Filippo Valsorda [Tue, 22 Jan 2019 21:02:41 +0000 (16:02 -0500)]
[release-branch.go1.11-security] crypto/elliptic: reduce subtraction term to prevent long busy loop
If beta8 is unusually large, the addition loop might take a very long
time to bring x3-beta8 back positive.
This would lead to a DoS vulnerability in the implementation of the
P-521 and P-384 elliptic curves that may let an attacker craft inputs
to ScalarMult that consume excessive amounts of CPU.
[release-branch.go1.11] cmd/go: respect gcflags, ldflags in 'go test'
Fixes bug introduced by https://golang.org/cl/129059 where
gcflags='all=...' and ldflags='all=...' would not be applied to some
packages built by 'go test'.
LoadImport used to set gcflags/ldflags for the Package objects it
created, in https://golang.org/cl/129059 this code was factored out to
setToolFlags. The codepath of `go build` was updated to call
setToolFlags appropriatley, but the codepath of `go test -c` wasn't,
resulting in gcflags/ldflags being applied inconsistently when building
tests.
This commit changes TestPackagesFor to call setToolFlags on the package
objects it creates.
Fixes #28346
Change-Id: Idcbec0c989ee96ec066207184611f08818873e8d
Reviewed-on: https://go-review.googlesource.com/c/136275
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
(cherry picked from commit 374546d800124e9ab4d51b75e335a71f866f3ef8)
Reviewed-on: https://go-review.googlesource.com/c/156377
Run-TryBot: Alessandro Arzilli <alessandro.arzilli@gmail.com>
This rule is problematic. 1<<(32-uint32(d)) <= int32(c) meant to
say that it is true if c is greater than the largest possible
value of the right shift. But when d==1, 1<<(32-1) is negative
and results in the wrong comparison.
Rewrite the rules in a more direct way.
Updates #29402.
Fixes #29442.
Change-Id: I5940fc9538d9bc3a4bcae8aa34672867540dc60e
Reviewed-on: https://go-review.googlesource.com/c/155798
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 6a64efc25004175e198e75191e215a7b1a08a2fa)
Reviewed-on: https://go-review.googlesource.com/c/155799 Reviewed-by: David Chase <drchase@google.com>
Michael Anthony Knyszek [Fri, 7 Dec 2018 00:07:43 +0000 (00:07 +0000)]
[release-branch.go1.11] runtime: don't clear lockedExt on locked M when G exits
When a locked M has its G exit without calling UnlockOSThread, then
lockedExt on it was getting cleared. Unfortunately, this meant that
during P handoff, if a new M was started, it might get forked (on
most OSes besides Windows) from the locked M, which could have kernel
state attached to it.
To solve this, just don't clear lockedExt. At the point where the
locked M has its G exit, it will also exit in accordance with the
LockOSThread API. So, we can safely assume that it's lockedExt state
will no longer be used. For the case of the main thread where it just
gets wedged instead of exiting, it's probably better for it to keep
the locked marker since it more accurately represents its state.
Fixed #28986.
Change-Id: I7d3d71dd65bcb873e9758086d2cbcb9a06429b0f
Reviewed-on: https://go-review.googlesource.com/c/155117
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Robert Griesemer [Mon, 17 Dec 2018 19:33:42 +0000 (11:33 -0800)]
[release-branch.go1.11] cmd/compile: generate interface method expression wrapper for error.Error
A prior optimization (https://golang.org/cl/106175) removed the
generation of unnecessary method expression wrappers, but also
eliminated the generation of the wrapper for error.Error which
was still required.
Cherry Zhang [Sat, 17 Nov 2018 03:53:04 +0000 (22:53 -0500)]
[release-branch.go1.11] cmd/compile: use correct store types in softfloat
When using softfloat, floating point ops are rewritten to integer
ops. The types of store ops were not rewritten. This may lower
to floating point stores, which are problematic. This CL fixes
this by rewriting the store types as well.
This fixes test/fixedbugs/issue28688.go on Wasm. Softfloat mode
is not used by default on Wasm, and it is not needed as Wasm spec
supports floating points. But it is nice to have the correct
types.
Ian Lance Taylor [Wed, 5 Dec 2018 22:04:27 +0000 (14:04 -0800)]
[release-branch.go1.11] cmd/cgo: preserve type information across loadDWARF loop
CL 122575 and its successors introduced a loop calling loadDWARF,
whereas before we only called it once. Pass a single typeConv to each
call, rather than creating a new one in loadDWARF itself. Change the
maps from dwarf.Type to use string keys rather than dwarf.Type keys,
since when the DWARF is reloaded the dwarf.Type pointers will be
different. These changes permit typeConv.Type to return a consistent
value for a given DWARF type, avoiding spurious type conversion errors
due to typedefs loaded after the first loop iteration.
Updates #27340
Fixes #27395
Change-Id: Ic33467bbfca4c54e95909621b35ba2a58216d96e
Reviewed-on: https://go-review.googlesource.com/c/152762
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 6d4358705301e06e71b99977b77ae2c0a6b16b86)
Reviewed-on: https://go-review.googlesource.com/c/154277
Run-TryBot: Filippo Valsorda <filippo@golang.org>
David Chase [Tue, 4 Dec 2018 15:00:16 +0000 (10:00 -0500)]
[release-branch.go1.11] cmd/compile: check for negative upper bound to IsSliceInBounds
IsSliceInBounds(x, y) asserts that y is not negative, but
there were cases where this is not true. Change code
generation to ensure that this is true when it's not obviously
true. Prove phase cleans a few of these out.
With this change the compiler text section is 0.06% larger,
that is, not very much. Benchmarking still TBD, may need
to wait for access to a benchmarking box (next week).
Also corrected run.go to handle '?' in -update_errors output.
Fixes #28799.
Change-Id: Ia8af90bc50a91ae6e934ef973def8d3f398fac7b
Reviewed-on: https://go-review.googlesource.com/c/152477
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit ea6259d5e9d57f247b7d877d4d04602b74ae5155)
Reviewed-on: https://go-review.googlesource.com/c/153638
Bryan C. Mills [Wed, 12 Dec 2018 03:46:55 +0000 (22:46 -0500)]
[release-branch.go1.11] cmd/go/internal/modfetch: skip symlinks in (*coderepo).Zip
Tested manually.
Before:
$ go mod init golang.org/issue/scratch
go: creating new go.mod: module golang.org/issue/scratch
$ go1.11.2 mod download github.com/rogpeppe/test2@latest
go: finding github.com/rogpeppe/test2 v0.0.11
$ find $GOPATH -name goodbye
/tmp/tmp.Y8a8UzX3zD/_gopath/pkg/mod/github.com/rogpeppe/test2@v0.0.11/tests/goodbye
$ cat $(find $GOPATH -name goodbye)
hello
After:
$ go mod init golang.org/issue/scratch
go: creating new go.mod: module golang.org/issue/scratch
$ go mod download github.com/rogpeppe/test2@latest
go: finding github.com/rogpeppe/test2 v0.0.11
$ find $GOPATH -name goodbye
$ find $GOPATH -name hello
/tmp/tmp.Zo0jhfLaRs/_gopath/pkg/mod/github.com/rogpeppe/test2@v0.0.11/tests/hello
A proper regression test would require one of:
• a new entry in the vcs-test server (feasible but tedious, and not easily updated by open-source contributors), or
• a way to set up an HTTPS proxy in a script_test, or
• a way to explicitly populate the module cache from the contents of a local repository (#28835).
Fixes #29191
Updates #28835
Change-Id: I72702a7e791f8815965f0f87c82a30df4d6f0151
Reviewed-on: https://go-review.googlesource.com/c/153819
Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>
(cherry picked from commit 561923fa7a7d47bba99556aaa61e40dd38708773)
Reviewed-on: https://go-review.googlesource.com/c/153822
Bryan C. Mills [Fri, 14 Dec 2018 02:42:33 +0000 (21:42 -0500)]
[release-branch.go1.11] cmd/go/internal/get: move wildcard-trimming to before CheckImportPath
Previously, RepoRootForImportPath trimmed certain "..." wildcards from
package patterns (even though its name suggests that the argument must
be an actual import path). It trimmed at the first path element that
was literally "..." (although wildcards in general may appear within a
larger path element), and relied on a subsequent check in
RepoRootForImportPath to catch confusing resolutions.
However, that causes 'go get' with wildcard patterns in fresh paths to
fail as of CL 154101: a wildcard pattern is not a valid import path,
and fails the path check. (The existing Test{Vendor,Go}Get* packages
in go_test.go and vendor_test.go catch the failure, but they are all
skipped when the "-short" flag is set — including in all.bash — and we
had forgotten to run them separately.)
We now trim the path before any element that contains a wildcard, and
perform the path check (and repo resolution) on only that prefix. It
is possible that the expanded path after fetching the repo will be
invalid, but a repository can contain directories that are not valid
import paths in general anyway.
Bryan C. Mills [Fri, 30 Nov 2018 19:04:35 +0000 (14:04 -0500)]
[release-branch.go1.11-security] cmd/go: reject 'get' of paths containing leading dots or unsupported characters
On some platforms, directories beginning with dot are treated as
hidden files, and filenames containing unusual characters can be
confusing for users to manipulate (and delete).
Change-Id: Ia1f5a65b9cff4eeb51cc4dba3ff7c7afabc343f2
Reviewed-on: https://team-review.git.corp.google.com/c/368442 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Filippo Valsorda [Wed, 5 Dec 2018 03:23:22 +0000 (22:23 -0500)]
[release-branch.go1.11-security] crypto/x509: limit number of signature checks for each verification
That number grows quadratically with the number of intermediate
certificates in certain pathological cases (for example if they all have
the same Subject) leading to a CPU DoS. Set a fixed budget that should
fit all real world chains, given we only look at intermediates provided
by the peer.
The algorithm can be improved, but that's left for follow-up CLs:
* the cache logic should be reviewed for correctness, as it seems to
override the entire chain with the cached one
* the equality check should compare Subject and public key, not the
whole certificate
* certificates with the right SKID but the wrong Subject should not
be considered, and in particular should not take priority over
certificates with the right Subject
Change-Id: Ib257c12cd5563df7723f9c81231d82b882854213
Reviewed-on: https://team-review.git.corp.google.com/c/370475 Reviewed-by: Andrew Bonventre <andybons@google.com>
(cherry picked from commit 09d57361bc99cbbfb9755ee30ddcb42ff5a9d7d6)
Reviewed-on: https://team-review.git.corp.google.com/c/372858 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Ian Lance Taylor [Tue, 27 Nov 2018 23:40:39 +0000 (15:40 -0800)]
[release-branch.go1.11] cmd/go: don't fail if requested Go version is later than current one
This is a partial backport of CL 147278 from tip to the Go 1.11 branch.
Change the behavior when the go.mod file requests a Go version that is
later than the current one. Previously cmd/go would give a fatal error
in this situation. With this change it attempts the compilation, and
if (and only if) the compilation fails it adds a note saying that the
requested Go version is newer than the known version. This is as
described in https://golang.org/issue/28221.
Updates #28221
Change-Id: Iea03ca574b6b1a046655f2bb2e554126f877fb66
Reviewed-on: https://go-review.googlesource.com/c/151358
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Tue, 20 Nov 2018 23:55:02 +0000 (15:55 -0800)]
[release-branch.go1.11] cmd/cgo: use field alignment when setting field offset
The old code ignored the field alignment, and only looked at the field
offset: if the field offset required padding, cgo added padding. But
while that approach works for Go (at least with the gc toolchain) it
doesn't work for C code using packed structs. With a packed struct the
added padding may leave the struct at a misaligned position, and the
inserted alignment, which cgo is not considering, may introduce
additional, unexpected, padding. Padding that ignores alignment is not
a good idea when the struct is not packed, and Go structs are never
packed. So don't ignore alignment.
Updates #28896
Fixes #28916
Change-Id: Ie50ea15fa6dc35557497097be9fecfecb11efd8a
Reviewed-on: https://go-review.googlesource.com/c/150602
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit fbdaa965634be842647195ee2d610dc363c760d2)
Reviewed-on: https://go-review.googlesource.com/c/151778
TryBot-Result: Gobot Gobot <gobot@golang.org>
Robert Griesemer [Mon, 5 Nov 2018 23:51:11 +0000 (15:51 -0800)]
[release-branch.go1.11] go/types: avoid certain problems with recursive alias type declarations
It is possible to create certain recursive type declarations involving
alias types which cause the type-checker to produce an (invalid) type
for the alias because it is not yet available. By type-checking alias
declarations in a 2nd phase, the problem is mitigated a bit since it
requires more convoluted alias declarations for the problem to appear.
Also re-enable testing of fixedbugs/issue27232.go again (which was the
original cause for this change).
Updates #28576.
Fixes #28972.
Change-Id: If6f9656a95262e6575b01c4a003094d41551564b
Reviewed-on: https://go-review.googlesource.com/c/147597 Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-on: https://go-review.googlesource.com/c/151500
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
Robert Griesemer [Sat, 3 Nov 2018 06:28:26 +0000 (23:28 -0700)]
[release-branch.go1.11] cmd/compile: reintroduce work-around for cyclic alias declarations
This change re-introduces (temporarily) a work-around for recursive
alias type declarations, originally in https://golang.org/cl/35831/
(intended as fix for #18640). The work-around was removed later
for a more comprehensive cycle detection check. That check
contained a subtle error which made the code appear to work,
while in fact creating incorrect types internally. See #25838
for details.
By re-introducing the original work-around, we eliminate problems
with many simple recursive type declarations involving aliases;
specifically cases such as #27232 and #27267. However, the more
general problem remains.
This CL also fixes the subtle error (incorrect variable use when
analyzing a type cycle) mentioned above and now issues a fatal
error with a reference to the relevant issue (rather than crashing
later during the compilation). While not great, this is better
than the current status. The long-term solution will need to
address these cycles (see #25838).
As a consequence, several old test cases are not accepted anymore
by the compiler since they happened to work accidentally only.
This CL disables parts or all code of those test cases. The issues
are: #18640, #23823, and #24939.
One of the new test cases (fixedbugs/issue27232.go) exposed a
go/types issue. The test case is excluded from the go/types test
suite and an issue was filed (#28576).
Keith Randall [Mon, 12 Nov 2018 23:49:09 +0000 (15:49 -0800)]
runtime: when using explicit argmap, also use arglen
When we set an explicit argmap, we may want only a prefix of that
argmap. Argmap is set when the function is reflect.makeFuncStub or
reflect.methodValueCall. In this case, arglen specifies how much of
the args section is actually live. (It could be either all the args +
results, or just the args.)
Elias Naur [Sun, 4 Nov 2018 08:36:25 +0000 (09:36 +0100)]
[release-branch.go1.11] runtime: avoid arm64 8.1 atomics on Android
The kernel on some Samsung S9+ models reports support for arm64 8.1
atomics, but in reality only some of the cores support them. Go
programs scheduled to cores without support will crash with SIGILL.
This change unconditionally disables the optimization on Android.
A better fix is to precisely detect the offending chipset.
Nikhil Benesch [Fri, 9 Nov 2018 05:55:13 +0000 (00:55 -0500)]
[release-branch.go1.11] runtime: never call into race detector with retaken P
cgocall could previously invoke the race detector on an M whose P had
been retaken. The race detector would attempt to use the P-local state
from this stale P, racing with the thread that was actually wired to
that P. The result was memory corruption of ThreadSanitizer's internal
data structures that presented as hard-to-understand assertion failures
and segfaults.
Reorder cgocall so that it always acquires a P before invoking the race
detector, and add a test that stresses the interaction between cgo and
the race detector to protect against future bugs of this kind.
Fixes #28690.
Change-Id: Ide93f96a23490314d6647547140e0a412a97f0d4
Reviewed-on: https://go-review.googlesource.com/c/148717
Run-TryBot: Dmitry Vyukov <dvyukov@google.com> Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
(cherry picked from commit e496e612b7f45a09209f8f4e1c7c1d0db378dc18)
Reviewed-on: https://go-review.googlesource.com/c/148902
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Thu, 11 Oct 2018 00:19:29 +0000 (17:19 -0700)]
[release-branch.go1.11] go/types: use correct receiver types for embedded interface methods
Interface methods don't declare a receiver (it's implicit), but after
type-checking the respective *types.Func objects are marked as methods
by having a receiver. For interface methods, the receiver base type used
to be the interface that declared the method in the first place, even if
the method also appeared in other interfaces via embedding. A change in
the computation of method sets for interfaces for Go1.10 changed that
inadvertently, with the consequence that sometimes a method's receiver
type ended up being an interface into which the method was embedded.
The exact behavior also depended on file type-checking order, and because
files are sometimes sorted by name, the behavior depended on file names.
This didn't matter for type-checking (the typechecker doesn't need the
receiver), but it matters for clients, and for printing of methods.
This change fixes interface method receivers at the end of type-checking
when we have all relevant information.
Daniel Theophanes [Sun, 30 Sep 2018 05:10:43 +0000 (22:10 -0700)]
[release-branch.go1.11] database/sql: correctly report MaxIdleClosed stat
Previously the MaxIdleClosed counter was incremented when added
to the free connection list, rather then when it wasn't added
to the free connection list. Flip this logic to correct.
Fixes #28325
Change-Id: I405302c14fb985369dab48fbe845e5651afc4ccf
Reviewed-on: https://go-review.googlesource.com/c/138578
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 7db509e682891f3bc501c7b23e32e02c64893557)
Reviewed-on: https://go-review.googlesource.com/c/146697
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Hana Kim [Mon, 17 Sep 2018 18:46:50 +0000 (14:46 -0400)]
[release-branch.go1.11] cmd/trace: don't drop sweep slice details
For sweep events, we used to modify the ViewerEvent returned from
ctx.emitSlice later in order to embed more details about the sweep
operation. The trick no longer works after the change
https://golang.org/cl/92375 and caused a regression.
ctx.emit method encodes the ViewerEvent, so any modification to the
ViewerEvent object after ctx.emit returns will not be reflected.
Refactor ctx.emitSlice, so ctx.makeSlice can be used when producing
slices for SWEEP. ctx.emit* methods are meant to truely emit
ViewerEvents.
Alex Brainman [Tue, 30 Oct 2018 08:40:57 +0000 (19:40 +1100)]
[release-branch.go1.11] internal/poll: advance file position in windows sendfile
Some versions of Windows (Windows 10 1803) do not set file
position after TransmitFile completes. So just use Seek
to set file position before returning from sendfile.
Russ Cox [Wed, 24 Oct 2018 16:01:13 +0000 (12:01 -0400)]
[release-branch.go1.11] cmd/go, cmd/link: silence bogus Apple Xcode warning
Certain installations of Xcode are affected by a bug that causes
them to print an inconsequential link-time warning that looks like:
ld: warning: text-based stub file /System/Library/Frameworks//Security.framework/Security.tbd and library file /System/Library/Frameworks//Security.framework/Security are out of sync. Falling back to library file for linking.
This has nothing to do with Go, and we've sent this repro case
to Apple:
$ pkgutil --pkg-info=com.apple.pkg.CLTools_Executables | grep version
version: 10.0.0.0.1.1535735448
$ clang --version
Apple LLVM version 10.0.0 (clang-1000.10.44.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
$ cat > issue.c
int main() { return 0; }
^D
$ clang issue.c -framework CoreFoundation
ld: warning: text-based stub file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking.
$
Even if Apple does release a fixed Xcode, many people are seeing
this useless warning, and we might as well make it go away.
Fixes #26073.
Change-Id: Ifc17ba7da1f6b59e233c11ebdab7241cb6656324
Reviewed-on: https://go-review.googlesource.com/c/144112 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
(cherry picked from commit 66bb8ddb956c5ee55b471a019fac2c6817c08ef5)
Reviewed-on: https://go-review.googlesource.com/c/145458
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jason Keene [Thu, 13 Sep 2018 02:35:58 +0000 (20:35 -0600)]
[release-branch.go1.11] cmd/go: ensure git attributes are set
This change disables the export-subst and export-ignore attributes when
creating zip files for modules. This is done to prevent the ziphash for
a given repo/revision from differing based on variables such as git
version or size of repo. The full rational for this change is detailed
here:
Cherry Zhang [Wed, 3 Oct 2018 02:04:45 +0000 (22:04 -0400)]
[release-branch.go1.11] cmd/compile: fix type of OffPtr in some optimization rules
In some optimization rules the type of generated OffPtr was
incorrectly set to the type of the pointee, instead of the
pointer. When the OffPtr value is spilled, this may generate
a spill of the wrong type, e.g. a floating point spill of an
integer (pointer) value. On Wasm, this leads to invalid
bytecode.
Richard Musiol [Tue, 2 Oct 2018 17:11:14 +0000 (19:11 +0200)]
[release-branch.go1.11] misc/wasm: add mention of polyfill for Edge support
Edge supports WebAssembly but not TextEncoder or TextDecoder.
This change adds a comment pointing to a polyfill that could
be used. The polyfill is not added by default, because we want to
let the user decide if/how to include the polyfill.
Fixes #27295
Change-Id: I375f58f2168665f549997b368428c398dfbbca1c
Reviewed-on: https://go-review.googlesource.com/139037 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit cfb603b0b5fb9c1e72be665b2d65743ddf18c779)
Reviewed-on: https://go-review.googlesource.com/139057 Reviewed-by: Richard Musiol <neelance@gmail.com>
Matthew Waters [Mon, 24 Sep 2018 10:08:54 +0000 (06:08 -0400)]
[release-branch.go1.11] net: concatenate multiple TXT strings in single TXT record
When go resolver was changed to use dnsmessage.Parser, LookupTXT
returned two strings in one record as two different records. This change
reverts back to concatenating multiple strings in a single
TXT record.
Change-Id: I46a975ed37d5e570afe2e228d3edf74949e08ad7
Reviewed-on: https://go-review.googlesource.com/138580 Reviewed-by: Michael Munday <mike.munday@ibm.com>
Reviewed-on: https://go-review.googlesource.com/138583
Run-TryBot: Keith Randall <khr@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 25 Sep 2018 22:54:11 +0000 (15:54 -0700)]
[release-branch.go1.11] reflect: ensure correct scanning of return values
During a call to a reflect-generated function or method (via
makeFuncStub or methodValueCall), when should we scan the return
values?
When we're starting a reflect call, the space on the stack for the
return values is not initialized yet, as it contains whatever junk was
on the stack of the caller at the time. The return space must not be
scanned during a GC.
When we're finishing a reflect call, the return values are
initialized, and must be scanned during a GC to make sure that any
pointers in the return values are found and their referents retained.
When the GC stack walk comes across a reflect call in progress on the
stack, it needs to know whether to scan the results or not. It doesn't
know the progress of the reflect call, so it can't decide by
itself. The reflect package needs to tell it.
This CL adds another slot in the frame of makeFuncStub and
methodValueCall so we can put a boolean in there which tells the
runtime whether to scan the results or not.
This CL also adds the args length to reflectMethodValue so the
runtime can restrict its scanning to only the args section (not the
results) if the reflect package says the results aren't ready yet.
Do a delicate dance in the reflect package to set the "results are
valid" bit. We need to make sure we set the bit only after we've
copied the results back to the stack. But we must set the bit before
we drop reflect's copy of the results. Otherwise, we might have a
state where (temporarily) no one has a live copy of the results.
That's the state we were observing in issue #27695 before this CL.
The bitmap used by the runtime currently contains only the args.
(Actually, it contains all the bits, but the size is set so we use
only the args portion.) This is safe for early in a reflect call, but
unsafe late in a reflect call. The test issue27695.go demonstrates
this unsafety. We change the bitmap to always include both args
and results, and decide at runtime which portion to use.
issue27695.go only has a test for method calls. Function calls were ok
because there wasn't a safepoint between when reflect dropped its copy
of the return values and when the caller is resumed. This may change
when we introduce safepoints everywhere.
This truncate-to-only-the-args was part of CL 9888 (in 2015). That
part of the CL fixed the problem demonstrated in issue27695b.go but
introduced the problem demonstrated in issue27695.go.
TODO, in another CL: simplify FuncLayout and its test. stack return
value is now identical to frametype.ptrdata + frametype.gcdata.
Update #27867
Change-Id: I2d49b34e34a82c6328b34f02610587a291b25c5f
Reviewed-on: https://go-review.googlesource.com/137440
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Reviewed-on: https://go-review.googlesource.com/138582
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 25 Sep 2018 21:32:44 +0000 (14:32 -0700)]
[release-branch.go1.11] reflect: use correct write barrier operations for method funcs
Fix the code to use write barriers on heap memory, and no
write barriers on stack memory.
These errors were discovered as part of fixing #27695. They may
have something to do with that issue, but hard to be sure.
The core cause is different, so this fix is a separate CL.
Update #27867
Change-Id: Ib005f6b3308de340be83c3d07d049d5e316b1e3c
Reviewed-on: https://go-review.googlesource.com/137438 Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit e35a41261b19589f40d32bd66274c23ab4b9b32e)
Reviewed-on: https://go-review.googlesource.com/138581
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Gudger [Thu, 6 Sep 2018 06:53:36 +0000 (23:53 -0700)]
[release-branch.go1.11] net: fail fast for DNS rcode success with no answers of requested type
DNS responses which do not contain answers of the requested type return
errNoSuchHost, the same error as rcode name error. Prior to
golang.org/cl/37879, both cases resulted in no additional name servers
being consulted for the question. That CL changed the behavior for both
cases. Issue #25336 was filed about the rcode name error case and
golang.org/cl/113815 fixed it. This CL fixes the no answers of requested
type case as well.
Updates #27525
Fixes #27537
Change-Id: I52fadedcd195f16adf62646b76bea2ab3b15d117
Reviewed-on: https://go-review.googlesource.com/133675
Run-TryBot: Ian Gudger <igudger@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 94f48ddb96c4dfc919ae024f64df19d764f5fb5b)
Reviewed-on: https://go-review.googlesource.com/138175
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Mon, 17 Sep 2018 19:25:36 +0000 (12:25 -0700)]
[release-branch.go1.11] runtime: ignore races between close and len/cap
They aren't really races, or at least they don't have any
observable effect. The spec is silent on whether these are actually
races or not.
Fix this problem by not using the address of len (or of cap)
as the location where channel operations are recorded to occur.
Use a random other field of hchan for that.
I'm not 100% sure we should in fact fix this. Opinions welcome.
Fixes #27778
Change-Id: Ib4efd4b62e0d1ef32fa51e373035ef207a655084
Reviewed-on: https://go-review.googlesource.com/135698 Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
(cherry picked from commit 83dfc3b001245f0b725afdc94c0b540fe1952d21)
Reviewed-on: https://go-review.googlesource.com/138179
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Johan Brandhorst [Fri, 24 Aug 2018 11:10:01 +0000 (12:10 +0100)]
[release-branch.go1.11] net/http: ensure null body in Fetch response is not read
The Fetch API returns a null body if there is no response body,
on browsers that support streaming the response body. This
change ensures we check for both undefined and null bodies
before attempting to read the body.
Fixes #27424
Change-Id: I0da86b61284fe394418b4b431495e715a037f335
Reviewed-on: https://go-review.googlesource.com/131236 Reviewed-by: Richard Musiol <neelance@gmail.com>
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit ce536837d8e53f1bf0c7ef450d4580d19f7d6f52)
Reviewed-on: https://go-review.googlesource.com/136915
Dmitri Shuralyov [Wed, 12 Sep 2018 17:58:18 +0000 (13:58 -0400)]
[release-branch.go1.11] doc/go1.11, cmd/go: elaborate on new GOFLAGS environment variable
In Go 1.11, cmd/go gained support for the GOFLAGS environment variable.
It was added and described in detail in CL 126656.
Mention it in the Go 1.11 release notes, link to the cmd/go documentation,
and add more details there.
Fixes #27387.
Change-Id: Ifc35bfe3e0886a145478d36dde8e80aedd8ec68e
Reviewed-on: https://go-review.googlesource.com/135035 Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Rob Pike <r@golang.org>
(cherry picked from commit 58c6afe075d74261dd67750e0aab5a1b8460839f)
Reviewed-on: https://go-review.googlesource.com/135496 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Alberto Donizetti [Wed, 22 Aug 2018 12:01:22 +0000 (14:01 +0200)]
[release-branch.go1.11] cmd/compile: prevent overflow in walkinrange
In the compiler frontend, walkinrange indiscriminately calls Int64()
on const CTINT nodes, even though Int64's return value is undefined
for anything over 2⁶³ (in practise, it'll return a negative number).
This causes the introduction of bad constants during rewrites of
unsigned expressions, which make the compiler reject valid Go
programs.
This change introduces a preliminary check that Int64() is safe to
call on the consts on hand. If it isn't, walkinrange exits without
doing any rewrite.
Rebecca Stambler [Thu, 30 Aug 2018 15:33:19 +0000 (11:33 -0400)]
[release-branch.go1.11] go/types: handle nil pointer when panic is written outside of a function
The current implementation crashes when someone writes a panic outside of
a function, which makes sense since that is broken code. This fix allows
one to type-check broken code.
Fixes #27497
Change-Id: I81b90dbd918162a20c60a821340898eaf02e648d
Reviewed-on: https://go-review.googlesource.com/132235 Reviewed-by: Alan Donovan <adonovan@google.com>
(cherry picked from commit c99687f87aed84342cfe92ae78924f791237c6f6)
Reviewed-on: https://go-review.googlesource.com/133395 Reviewed-by: Robert Griesemer <gri@golang.org>
[release-branch.go1.11] crypto/x509: allow ":" in Common Name hostnames
At least one popular service puts a hostname which contains a ":"
in the Common Name field. On the other hand, I don't know of any name
constrained certificates that only work if we ignore such CNs.
Updates #24151
Change-Id: I2d813e3e522ebd65ab5ea5cd83390467a869eea3
Reviewed-on: https://go-review.googlesource.com/134076
Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 03c703697f321f66d28d6223457622c5879ba37f)
Reviewed-on: https://go-review.googlesource.com/134078 Reviewed-by: Andrew Bonventre <andybons@golang.org>
Keith Randall [Wed, 5 Sep 2018 21:36:20 +0000 (14:36 -0700)]
[release-branch.go1.11] runtime: in semasleep, subtract time spent so far from timeout
When pthread_cond_timedwait_relative_np gets a spurious wakeup
(due to a signal, typically), we used to retry with the same
relative timeout. That's incorrect, we should lower the timeout
by the time we've spent in this function so far.
In the worst case, signals come in and cause spurious wakeups
faster than the timeout, causing semasleep to never time out.
Also fix nacl and netbsd while we're here. They have similar issues.
Fixes #27521
Change-Id: I6601e120e44a4b8ef436eef75a1e7c8cf1d39e39
Reviewed-on: https://go-review.googlesource.com/133655
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 2bf1370f4369d75f4fffffc6fc05722bce13481b)
Reviewed-on: https://go-review.googlesource.com/134096
Giovanni Bajo [Fri, 31 Aug 2018 00:15:26 +0000 (02:15 +0200)]
[release-branch.go1.11] cmd/compile: in prove, fix fence-post implications for unsigned domain
Fence-post implications of the form "x-1 >= w && x > min ⇒ x > w"
were not correctly handling unsigned domain, by always checking signed
limits.
This bug was uncovered once we taught prove that len(x) is always
>= 0 in the signed domain.
In the code being miscompiled (s[len(s)-1]), prove checks
whether len(s)-1 >= len(s) in the unsigned domain; if it proves
that this is always false, it can remove the bound check.
Notice that len(s)-1 >= len(s) can be true for len(s) = 0 because
of the wrap-around, so this is something prove should not be
able to deduce.
But because of the bug, the gate condition for the fence-post
implication was len(s) > MinInt64 instead of len(s) > 0; that
condition would be good in the signed domain but not in the
unsigned domain. And since in CL105635 we taught prove that
len(s) >= 0, the condition incorrectly triggered
(len(s) >= 0 > MinInt64) and things were going downfall.
Andrei Tudor Călin [Thu, 30 Aug 2018 04:55:05 +0000 (06:55 +0200)]
[release-branch.go1.11] net: refactor readerAtEOF splice test
Refactor TestSplice/readerAtEOF to handle cases where we disable
splice on older kernels better.
If splice is disabled, net.splice and poll.Splice do not get to
observe EOF on the reader, because poll.Splice returns immediately
with EINVAL. The test fails unexpectedly, because the splice operation
is reported as not handled.
This change refactors the test to handle the aforementioned case
correctly, by not calling net.splice directly, but using a higher
level check.
Fixes #27355.
Change-Id: I0d5606b4775213f2dbbb84ef82ddfc3bab662a31
Reviewed-on: https://go-review.googlesource.com/132096
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit bd49b3d580731d8f391e40fb9e2f17301651cede)
Reviewed-on: https://go-review.googlesource.com/132281 Reviewed-by: Andrew Bonventre <andybons@golang.org>