When Host is not valid per RFC 3986, the behavior of Hostname and Port
was wildly unpredictable, to the point that Host could have a suffix
that didn't appear in neither Hostname nor Port.
This is a security issue when applications are applying checks to Host
and expecting them to be meaningful for the contents of Hostname.
To reduce disruption, this change only aims to guarantee the following
two security-relevant invariants.
* Host is either Hostname or [Hostname] with Port empty, or
Hostname:Port or [Hostname]:Port.
* Port is only decimals.
The second invariant is the one that's most likely to cause disruption,
but I believe it's important, as it's conceivable an application might
do a suffix check on Host and expect it to be meaningful for the
contents of Hostname (if the suffix is not a valid port).
There are three ways to ensure it.
1) Reject invalid ports in Parse. Note that non-numeric ports are
already rejected if and only if the host starts with "[".
2) Consider non-numeric ports as part of Hostname, not Port.
3) Allow non-numeric ports, and hope they only flow down to net/http,
which will reject them (#14353).
This change adopts both 1 and 2. We could do only the latter, but then
these invalid hosts would flow past port checks, like in
http_test.TestTransportRejectsAlphaPort. Non-numeric ports weren't fully
supported anyway, because they were rejected after IPv6 literals, so
this restores consistency. We could do only the former, but at this
point 2) is free and might help with manually constructed Host values
(or if we get something wrong in Parse).
Note that net.SplitHostPort and net.Dial explicitly accept service names
in place of port numbers, but this is an URL package, and RFC 3986,
Section 3.2.3, clearly specifies ports as a number in decimal.
net/http uses a mix of net.SplitHostPort and url.Parse that would
deserve looking into, but in general it seems that it will still accept
service names in Addr fields as they are passed to net.Listen, while
rejecting them in URLs, which feels correct.
This leaves a number of invalid URLs to reject, which however are not
security relevant once the two invariants above hold, so can be done in
Go 1.14: IPv6 literals without brackets (#31024), invalid IPv6 literals,
hostnames with invalid characters, and more.
Tested with 200M executions of go-fuzz and the following Fuzz function.
u, err := url.Parse(string(data))
if err != nil {
return 0
}
h := u.Hostname()
p := u.Port()
switch u.Host {
case h + ":" + p:
return 1
case "[" + h + "]:" + p:
return 1
case h:
fallthrough
case "[" + h + "]":
if p != "" {
panic("unexpected Port()")
}
return 1
}
panic("Host is not a variant of [Hostname]:Port")
Fixes CVE-2019-14809
Updates #29098
Change-Id: I7ef40823dab28f29511329fa2d5a7fb10c3ec895
Reviewed-on: https://go-review.googlesource.com/c/go/+/189258
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 61bb56ad63)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526408
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
(cherry picked from commit 3226f2d492)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526409
Apply the following unpublished golang.org/x/net commit.
commit b1cc14aba47abf96f96818003fa4caad3a4b4e86
Author: Filippo Valsorda <filippo@golang.org>
Date: Sun Aug 11 02:12:18 2019 -0400
[release-branch.go1.11] http2: limit number of control frames in server send queue
An attacker could cause servers to queue an unlimited number of PING
ACKs or RST_STREAM frames by soliciting them and not reading them, until
the program runs out of memory.
Limit control frames in the queue to a few thousands (matching the limit
imposed by other vendors) by counting as they enter and exit the scheduler,
so the protection will work with any WriteScheduler.
Once the limit is exceeded, close the connection, as we have no way to
communicate with the peer.
Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
(cherry picked from commit 589ad6cc5321fb68a90370348a241a5da0a2cc80)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526070
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Fixes CVE-2019-9512 and CVE-2019-9514
Updates #33606
Change-Id: Iecedf1cc63ec7a1cd75661ec591d91ebc911cc64
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/526072
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
For later versions of MacOS, the dynamic loader is more picky about
enforcing restrictions on __DWARF MachO load commands/segments,
triggering aborts of the form
dyld: malformed mach-o image: segment __DWARF has vmsize < filesize
for Go programs that use cgo on Darwin. The error is being triggered
because the Go linker is setting "vmsize" in the DWARF segment entry
to zero as a way to signal that the DWARF doesn't need to be mapped
into memory at runtime (which we need to continue to do).
This patch changes the initial protection on the __DWARF segment to
zero, which dyld seems to be happy with (this is used for other similar
non-loadable sections such as __LLVM).
Updates #32696
Change-Id: I9a73449c6d26c172f3d70361719943af381f37e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/182958
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/183399
While many other call sites have been moved to using the proper
higher-level system loading, these areas were left out. This prevents
DLL directory injection attacks. This includes both the runtime load
calls (using LoadLibrary prior) and the implicitly linked ones via
cgo_import_dynamic, which we move to our LoadLibraryEx. The goal is to
only loosely load kernel32.dll and strictly load all others.
Meanwhile we make sure that we never fallback to insecure loading on
older or unpatched systems.
This is CVE-2019-9634.
Fixes#30989
Updates #14959
Updates #28978
Updates #30642
Change-Id: I401a13ed8db248ab1bb5039bf2d31915cac72b93
Reviewed-on: https://go-review.googlesource.com/c/go/+/165798
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
(cherry picked from commit 9b6e9f0c8c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/175378
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
New versions of clang can generate multiple sections named ".text"
when using vague C++ linkage. This is valid ELF, but would cause the
Go linker to report an error when using internal linking:
symbol PACKAGEPATH(.text) listed multiple times
Avoid the problem by renaming section symbol names if there is a name
collision.
Change-Id: I41127e95003d5b4554aaf849177b3fe000382c02
Reviewed-on: https://go-review.googlesource.com/c/go/+/172697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 3235f7c072)
Reviewed-on: https://go-review.googlesource.com/c/go/+/172702
The current wasm write barrier implementation incorrectly implements
the "deletion" part of the barrier. It correctly greys the new value
of the pointer, but rather than also greying the old value of the
pointer, it greys the object containing the slot (which, since the old
value was just overwritten, is not going to contain the old value).
This can lead to unmarked, reachable objects.
Often, this is masked by other marking activity, but one specific
sequence that can lead to an unmarked object because of this bug is:
1. Initially, GC is off, object A is reachable from just one pointer
in the heap.
2. GC starts and scans the stack of goroutine G.
3. G copies the pointer to A on to its stack and overwrites the
pointer to A in the heap. (Now A is reachable only from G's stack.)
4. GC finishes while A is still reachable from G's stack.
With a functioning deletion barrier, step 3 causes A to be greyed.
Without a functioning deletion barrier, nothing causes A to be greyed,
so A will be freed even though it's still reachable from G's stack.
This CL fixes the wasm write barrier.
Fixes#30872.
Change-Id: I8a74ee517facd3aa9ad606e5424bcf8f0d78e754
Reviewed-on: https://go-review.googlesource.com/c/go/+/167743
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit d9db9e32e9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/167746
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 429bae7158)
Reviewed-on: https://go-review.googlesource.com/c/go/+/167707
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
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.
Fixes#30444
Change-Id: Icb6bd622a543e9bc5f669fd3d7abd418b4a8e579
Reviewed-on: https://go-review.googlesource.com/c/152958
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit ec0077c54d)
Reviewed-on: https://go-review.googlesource.com/c/164240
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
Updates #30079Fixes#30081
Change-Id: If8ab0179aca86cb74caa926d1ef93fb5e416b4bb
Reviewed-on: https://go-review.googlesource.com/c/161097
Reviewed-by: Adam Langley <agl@golang.org>
(cherry picked from commit 95e5b07cf5)
Reviewed-on: https://go-review.googlesource.com/c/163739
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
Updates #29502.
Fixes#29503.
Change-Id: Ia9f7d814264b1a3ddf78f52e2ce23377450e6e8a
Reviewed-on: https://go-review.googlesource.com/c/156019
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 2e217fa726)
Reviewed-on: https://go-review.googlesource.com/c/163724
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 #24652Fixes#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 aa24158077)
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>
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 f6be1cf109)
Reviewed-on: https://go-review.googlesource.com/c/162860
Reviewed-by: Andrew Bonventre <andybons@golang.org>
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.
This fixes CVE-2019-6486.
Change-Id: Ia969e8b5bf5ac4071a00722de9d5e4d856d8071a
Reviewed-on: https://team-review.git.corp.google.com/c/399777
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
(cherry picked from commit 746d6abe2dfb9ce7609f8e1e1a8dcb7e221f423e)
Reviewed-on: https://team-review.git.corp.google.com/c/401142
Reviewed-by: Filippo Valsorda <valsorda@google.com>
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 374546d800)
Reviewed-on: https://go-review.googlesource.com/c/156377
Run-TryBot: Alessandro Arzilli <alessandro.arzilli@gmail.com>
(SGTconst [c] (SRLconst _ [d])) && 0 <= int32(c) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= int32(c) -> (MOVWconst [1])
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 6a64efc250)
Reviewed-on: https://go-review.googlesource.com/c/155799
Reviewed-by: David Chase <drchase@google.com>
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>
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.
Change-Id: Ib5e19e19fa9491b15c2f60320f8724cace5cefb5
Reviewed-on: https://go-review.googlesource.com/c/149965
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 63a3993a33)
Reviewed-on: https://go-review.googlesource.com/c/151344
Reviewed-by: Filippo Valsorda <filippo@golang.org>
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 #27340Fixes#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 6d43587053)
Reviewed-on: https://go-review.googlesource.com/c/154277
Run-TryBot: Filippo Valsorda <filippo@golang.org>
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 ea6259d5e9)
Reviewed-on: https://go-review.googlesource.com/c/153638
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 561923fa7a)
Reviewed-on: https://go-review.googlesource.com/c/153822
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.
Fixes#29248
Change-Id: I70fb2f7fc6603b7d339fd6c02e8cdeacfc93fc4b
Reviewed-on: https://go-review.googlesource.com/c/154108
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit 47fb1fbd55)
Reviewed-on: https://go-review.googlesource.com/c/154110
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This should be a no-op, but produces deterministic (and more correct)
behavior if we have accidentally failed to sanitize one of the inputs.
Change-Id: I1271d0ffd01a691ec8c84906c4e02d9e2be19c72
Reviewed-on: https://team-review.git.corp.google.com/c/372705
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
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>
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>
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>
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 #28896Fixes#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 fbdaa96563)
Reviewed-on: https://go-review.googlesource.com/c/151778
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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).
Updates #18640.
Updates #23823.
Updates #24939.
Updates #25838.
Updates #28576.
Fixes#27232.
Fixes#27267.
Fixes#27383.
Change-Id: I6c2d10da98bfc6f4f445c755fcaab17fc7b214c5
Reviewed-on: https://go-review.googlesource.com/c/147286
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
(cherry picked from commit e6305380a0)
Reviewed-on: https://go-review.googlesource.com/c/151339
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
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.)
Fixes#28752
Change-Id: Idf060607f15a298ac591016994e58e22f7f92d83
Reviewed-on: https://go-review.googlesource.com/c/149217
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit 0098f8aeac)
Reviewed-on: https://go-review.googlesource.com/c/149457
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.
Fixes#28586
Change-Id: I35a1273e5660603824d30ebef2ce7e429241bf1f
Reviewed-on: https://go-review.googlesource.com/c/147377
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/149557
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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 e496e612b7)
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>
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.
Fixes#28249
Updates #28005
Change-Id: I96c120fb0e517d7f8a14b8530f0273674569d5ea
Reviewed-on: https://go-review.googlesource.com/c/141358
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-on: https://go-review.googlesource.com/c/146660
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
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.
Fixes#27717
Updates #27711
Change-Id: I0b733ebbbfd4facd8714db0535809ec3cab0833d
Reviewed-on: https://go-review.googlesource.com/135775
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit e57f24ab39)
Reviewed-on: https://go-review.googlesource.com/c/146698
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
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 66bb8ddb95)
Reviewed-on: https://go-review.googlesource.com/c/145458
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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>
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 e35a41261b)
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>
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 #27525Fixes#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 94f48ddb96)
Reviewed-on: https://go-review.googlesource.com/138175
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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 83dfc3b001)
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>
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.
Fixes#27246
Change-Id: I2017073cae65468a521ff3262d4ea8ab0d7098d9
Reviewed-on: https://go-review.googlesource.com/130735
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 42cc4ca30a)
Reviewed-on: https://go-review.googlesource.com/131596
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
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>
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
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.
Fixes#27378
Change-Id: I3dbcb1955ac5a66a0dcbee500f41e8d219409be5
Reviewed-on: https://go-review.googlesource.com/132495
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 09ea3c08e8)
Reviewed-on: https://go-review.googlesource.com/132575
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 bd49b3d580)
Reviewed-on: https://go-review.googlesource.com/132281
Reviewed-by: Andrew Bonventre <andybons@golang.org>
The wiki page has recently been created, and at this time it's
just a stub. It's expected that support for WebAssembly will be
evolving over time, and the wiki page can be kept updated with
helpful information, how to get started, tips and tricks, etc.
Use present tense because it's expected that there will be more
general information added by the time Go 1.11 release happens.
Also add link to https://webassembly.org/ in first paragraph.
Change-Id: I139c2dcec8f0d7fd89401df38a3e12960946693f
Reviewed-on: https://go-review.googlesource.com/131078
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 6e76aeba0b)
Reviewed-on: https://go-review.googlesource.com/131096
It is possible to enter the parent-walking directory loop in a way that
it will loop forever - if mdir is empty, and d reaches ".". To avoid
this, make sure that the 'd = filepath.Dir(d)' step only happens if the
parent directory is actually different than the current directory.
This fixes some of the tests like TestImport/golang.org_x_net_context,
which were never finishing before.
While at it, also fix TestImport/golang.org_x_net, which seems to have
the wrong expected error. The root of the x/net repo doesn't have a
go.mod file, nor is part of a module itself, so it seems like the
expected error should reflect that.
After these two changes, 'go test cmd/go/internal/modload' passes on my
linux/amd64 machine.
Fixes#27080.
Change-Id: Ie8bab0f9fbc9f447844cbbc64117420d9087db1b
Reviewed-on: https://go-review.googlesource.com/129778
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 692307aa83)
Reviewed-on: https://go-review.googlesource.com/130275
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
In CL 48510 the gcmAble interface was changed to include the tag size.
The BoringCrypto aesCipher implementation wasn't updated, causing a
failed type assertion and consequently a performance degradation.
Change-Id: Ie5cff9ef242218d60f82795f3eb6760a57fe06f5
Reviewed-on: https://go-review.googlesource.com/127821
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Now that the standard library behavior in reading from the randomness
source is not reliable thanks to randutil.MaybeReadByte, we don't need
to emulate its behavior.
Also, since boring.RandReader is never deterministic, add an early exit
to randutil.MaybeReadByte.
Change-Id: Ie53e45ee64af635595181f71abd3c4340c600907
Reviewed-on: https://go-review.googlesource.com/117555
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Conflicts due to randutil.MaybeReadByte (kept at the top for patch
maintainability and consistency):
src/crypto/ecdsa/ecdsa.go
src/crypto/rsa/pkcs1v15.go
src/crypto/rsa/rsa.go
Change-Id: I03a2de541e68a1bbdc48590ad7c01fbffbbf4a2b
This patch used to be in crypto/internal/cipherhw.AESGCMSupport which
was removed from the tree. It was meant and documented to affect only
crypto/tls, so move the logic there.
Change-Id: I36ed4f08a5fe2abaab18907910899ae0297d1611
Reviewed-on: https://go-review.googlesource.com/114816
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Conflicts due to crypto/internal/cipherhw removal:
src/crypto/aes/cipher_amd64.go
src/crypto/internal/cipherhw/cipherhw_amd64.go
src/go/build/deps_test.go
This removes the AESGCMSupport patch, as there is no equivalent place
for it. The logic will be added back in the next change.
Change-Id: I8169069ff732b6cd0b56279c073cf5e0dd36959d
Go 1.10 expects hash.Hash implementations to have these. Make it so.
Tested by src/hash/marshal_test.go.
Change-Id: I9df366e31fe20e79385d5dbde7060b01b68c54df
Reviewed-on: https://go-review.googlesource.com/82139
Reviewed-by: Adam Langley <agl@golang.org>
This is a git merge of master into dev.boringcrypto.
The branch was previously based on release-branch.go1.9,
so there are a handful of spurious conflicts that would
also arise if trying to merge master into release-branch.go1.9
(which we never do). Those have all been resolved by taking
the original file from master, discarding any Go 1.9-specific
edits.
all.bash passes on darwin/amd64, which is to say without
actually using BoringCrypto.
Go 1.10-related fixes to BoringCrypto itself will be in a followup CL.
This CL is just the merge.
Change-Id: I4c97711fec0fb86761913dcde28d25c001246c35
CL 36932 (speed up fastrandn) made it faster but introduced
bad interference with some properties of fastrand itself, making
fastrandn not very random in certain ways. In particular, certain
selects are demonstrably unfair.
For Go 1.10 the new faster fastrandn has induced a new fastrand,
which in turn has caused other follow-on bugs that are still being
discovered and fixed.
For Go 1.9.2, just go back to the barely slower % implementation
that we used in Go 1.8 and earlier. This should restore fairness in
select and any other problems caused by the clever fastrandn.
The test in this CL is copied from CL 62530.
Fixes#22253.
Change-Id: Ibcf948a7bce981452e05c90dbdac122043f6f813
Reviewed-on: https://go-review.googlesource.com/70991
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
If we have
y = <int16> (MOVBQSX x)
z = <int32> (MOVWQSX y)
We used to use this rewrite rule:
(MOVWQSX x:(MOVBQSX _)) -> x
But that resulted in replacing z with a value whose type
is only int16. Then if z is spilled and restored, it gets
zero extended instead of sign extended.
Instead use the rule
(MOVWQSX (MOVBQSX x)) -> (MOVBQSX x)
The result is has the correct type, so it can be spilled
and restored correctly. It might mean that a few more extension
ops might not be eliminated, but that's the price for correctness.
Fixes#21963
Change-Id: I6ec82c3d2dbe43cc1fee6fb2bd6b3a72fca3af00
Reviewed-on: https://go-review.googlesource.com/65290
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/70986
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The assembler barfs on large offsets. Make sure that all the
instructions that need to have their offsets in an int32
1) check on any rule that computes offsets for such instructions
2) change their aux fields so the check builder checks it.
The assembler also silently misassembled offsets between 1<<31
and 1<<32. Add a check in the assembler to barf on those as well.
Fixes#21655
Change-Id: Iebf24bf10f9f37b3ea819ceb7d588251c0f46d7d
Reviewed-on: https://go-review.googlesource.com/59630
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/70981
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
After the number of lost extra events are written to the the cpuprof log,
the number of lost extra events should be set to zero, or else, the next
time time addExtra is logged, lostExtra will be overcounted. This change
resets lostExtra after its value is written to the log.
Fixes#21836
Change-Id: I8a6ac9c61e579e7a5ca7bdb0f3463f8ae8b9f864
Reviewed-on: https://go-review.googlesource.com/63270
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/70974
Run-TryBot: Russ Cox <rsc@golang.org>
More generally I'm concerned about these tests using
$GOROOT/src/cmd/go as scratch space, especially
combined wtih tg.parallel() - it's easy to believe some other
test might inadvertently also try to write x.exe about the
same time. This CL only solves the "didn't clean up x.exe"
problem and leaves for another day the "probably shouldn't
write to cmd/go at all" problem.
Fixes#22266.
Change-Id: I651534d70e2d360138e0373fb4a316081872550b
Reviewed-on: https://go-review.googlesource.com/71410
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/71530
After we detect errors, the AST is in a precarious state and more
likely to trip useless ICE failures. Instead let the user fix any
existing errors and see if the ICE persists. This makes Fatalf more
consistent with how panics are handled by hidePanic.
While here, also fix detection for release versions: release version
strings begin with "go" ("go1.8", "go1.9.1", etc), not "release".
Fixes#22252.
Change-Id: I1c400af62fb49dd979b96e1bf0fb295a81c8b336
Reviewed-on: https://go-review.googlesource.com/70850
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/70985
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Database drivers should be called from a single goroutine to ease
driver's design. If a driver chooses to handle context
cancels internally it may do so.
The sql package violated this agreement when calling Next or
NextResultSet. It was possible for a concurrent rollback
triggered from a context cancel to call a Tx.Rollback (which
takes a driver connection lock) while a Rows.Next is in progress
(which does not tack the driver connection lock).
The current internal design of the sql package is each call takes
roughly two locks: a closemu lock which prevents an disposing of
internal resources (assigning nil or removing from lists)
and a driver connection lock that prevents calling driver code from
multiple goroutines.
Fixes#21117
Change-Id: Ie340dc752a503089c27f57ffd43e191534829360
Reviewed-on: https://go-review.googlesource.com/65731
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/71510
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Current code assumes that SetFileCompletionNotificationModes
is safe to call even if we know that it is not safe to use
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag. It appears (see issue #22149),
SetFileCompletionNotificationModes crashes when we call it without
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag.
Do not call SetFileCompletionNotificationModes in that situation.
We are allowed to do that, because SetFileCompletionNotificationModes
is just an optimisation.
Fixes#22149
Change-Id: I0ad3aff4eabd8c27739417a62c286b1819ae166a
Reviewed-on: https://go-review.googlesource.com/69870
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/70989
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
On an iPhone 6 running iOS 11, the TestDialerDualStackFDLeak test
started failing with dial durations just above the limit:
FAIL: TestDialerDualStackFDLeak (0.21s)
dial_test.go:90: got 101.154ms; want <= 95ms
Bump the timeout on iOS.
For the iOS builder.
Change-Id: Id42b471e7cf7d0c84f6e83ed04b395fa1a2d449d
Reviewed-on: https://go-review.googlesource.com/66491
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/70987
Run-TryBot: Russ Cox <rsc@golang.org>
Currently, the priority of checks in (gcTrigger).test() puts the
gcpercent<0 test above gcTriggerCycle, which is used for runtime.GC().
This is an unintentional change from 1.8 and before, where
runtime.GC() triggered a GC even if GOGC=off.
Fix this by rearranging the priority so the gcTriggerCycle test
executes even if gcpercent < 0.
Fixes#22023.
Change-Id: I109328d7b643b6824eb9d79061a9e775f0149575
Reviewed-on: https://go-review.googlesource.com/65994
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-on: https://go-review.googlesource.com/70979
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
When a MOVDstorezero (8 bytes) is used the offset field
in the instruction must be a multiple of 4. This situation
had been corrected in the rules for other types of stores
but not for the zero case.
This also removes some of the special MOVDstorezero cases since
they can be handled by the general LowerZero case.
Updates made to the ssa test for lowering zero moves to
include cases where the target is not aligned to at least 4.
Fixes#21947
Change-Id: I7cceceb1be4898c77cd3b5e78b58dce0a7e28edd
Reviewed-on: https://go-review.googlesource.com/64970
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/70978
Run-TryBot: Russ Cox <rsc@golang.org>
internal/poll package assumes that only net sockets use runtime
netpoller on windows. We get memory corruption if other file
handles are passed into runtime poller. Make FD.Init receive
and use useNetpoller argument, so FD.Init caller is explicit
about using runtime netpoller.
Fixes#21172
Change-Id: I60e2bfedf9dda9b341eb7a3e5221035db29f5739
Reviewed-on: https://go-review.googlesource.com/65810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/71132
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
The previous code seems to have an off-by-1 in it somewhere, the
consequence being that we didn't properly preserve all of the old
buffer contents that we intended to.
After spending a while looking at the existing window-shifting logic,
I wasn't able to understand exactly how it was supposed to work or
where the issue was, so I rewrote it to be (at least IMO) more
obviously correct.
Fixes#21938.
Change-Id: I1ed7bbc1e1751a52ab5f7cf0411ae289586dc345
Reviewed-on: https://go-review.googlesource.com/64830
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-on: https://go-review.googlesource.com/70977
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler replaces any path of the form /path/to/goroot/src/net/port.go
with GOROOT/src/net/port.go so that the same object file is
produced if the GOROOT is moved. It was skipping this transformation
for any absolute path into the GOROOT that came from //line directives,
such as those generated by cmd/cgo.
Fixes#21373Fixes#21720Fixes#21825
Change-Id: I2784c701b4391cfb92e23efbcb091a84957d61dd
Reviewed-on: https://go-review.googlesource.com/63693
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-on: https://go-review.googlesource.com/70975
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 36428 changed the way nanotime works so on Darwin and Windows it
now depends on runtime.startNano, which is computed at runtime.init
time. Unfortunately, the `runtimeInitTime = nanotime()` initialization
happened *before* runtime.init, so on these platforms runtimeInitTime
is set incorrectly. The one (and only) consequence of this is that the
start time printed in gctrace lines is bogus:
gc 1 18446653480.186s 0%: 0.092+0.47+0.038 ms clock, 0.37+0.15/0.81/1.8+0.15 ms cpu, 4->4->1 MB, 5 MB goal, 8 P
To fix this, this commit moves the runtimeInitTime initialization to
shortly after runtime.init, at which point nanotime is safe to use.
This also requires changing the condition in newproc1 that currently
uses runtimeInitTime != 0 simply to detect whether or not the main M
has started. Since runtimeInitTime could genuinely be 0 now, this
introduces a separate flag to newproc1.
Fixes#21554.
Change-Id: Id874a4b912d3fa3d22f58d01b31ffb3548266d3b
Reviewed-on: https://go-review.googlesource.com/58690
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/70848
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
golang.org/cl/36941 enabled loading of all trusted certs on darwin
for the non-cgo execSecurityRoots.
The corresponding cgo version golang.org/cl/36942 for systemRootsPool
has not been merged yet.
This tests fails reliably on some darwin systems:
--- FAIL: TestSystemRoots (1.28s)
root_darwin_test.go:31: cgo sys roots: 353.552363ms
root_darwin_test.go:32: non-cgo sys roots: 921.85297ms
root_darwin_test.go:44: got 169 roots
root_darwin_test.go:44: got 455 roots
root_darwin_test.go:73: insufficient overlap between cgo and non-cgo roots; want at least 227, have 168
FAIL
FAIL crypto/x509 2.445s
Updates #16532
Updates #21416
Change-Id: I52c2c847651fb3621fdb6ab858ebe8e28894c201
Reviewed-on: https://go-review.googlesource.com/57830
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-on: https://go-review.googlesource.com/70847
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
PlainAuth originally refused to send passwords to non-TLS servers
and was documented as such.
In 2013, issue #5184 was filed objecting to the TLS requirement,
despite the fact that it is spelled out clearly in RFC 4954.
The only possibly legitimate use case raised was using PLAIN auth
for connections to localhost, and the suggested fix was to let the
server decide: if it advertises that PLAIN auth is OK, believe it.
That approach was adopted in CL 8279043 and released in Go 1.1.
Unfortunately, this is exactly wrong. The whole point of the TLS
requirement is to make sure not to send the password to the wrong
server or to a man-in-the-middle. Instead of implementing this rule,
CL 8279043 blindly trusts the server, so that if a man-in-the-middle
says "it's OK, you can send me your password," PlainAuth does.
And the documentation was not updated to reflect any of this.
This CL restores the original TLS check, as required by RFC 4954
and as promised in the documentation for PlainAuth.
It then carves out a documented exception for connections made
to localhost (defined as "localhost", "127.0.0.1", or "::1").
Cherry-pick of CL 68170.
Change-Id: I1d3729bbd33aa2f11a03f4c000e6bb473164957b
Reviewed-on: https://go-review.googlesource.com/68210
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
All the finalizer-enabled C wrappers must be careful to use
runtime.KeepAlive to ensure the C wrapper object (a Go object)
lives through the end of every C call using state that the
wrapper's finalizer would free.
This CL makes the wrappers appropriately careful.
The test proves that this is the bug I was chasing in a
separate real program, and that the KeepAlives fix it.
I did not write a test of every possible operation.
Change-Id: I627007e480f16adf8396e7f796b54e5525d9ea80
Reviewed-on: https://go-review.googlesource.com/64870
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There's no way for a *.syso file to be compiled to work both in
normal mode and in msan mode. Assume they are compiled in
normal mode and drop them in msan mode.
Packages with syso files currently fail in -msan mode because
the syso file calls out to a routine like memcmp which then
falsely reports uninitialized memory. After this CL, they will fail
in -msan with link errors, because the syso will not be used.
But then it will at least be possible for package authors to write
fallback code in the package that avoids the syso in -msan mode,
so that the package with the syso can at least run in both modes.
Without this CL, that's not possible.
See #21884.
Change-Id: I77340614c4711325032484e65fa9c3f8332741d5
Reviewed-on: https://go-review.googlesource.com/63917
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The syso is not compiled with -fsanitize=memory, so don't try to use it.
Otherwise the first time it calls out to memcmp, memcmp complains
that it is being asked to compare uninitialized memory.
Change-Id: I85ab707cfbe64eded8e110d4d6b40d1b75f50541
Reviewed-on: https://go-review.googlesource.com/63916
Reviewed-by: Adam Langley <agl@golang.org>
I've now debugged multiple mysterious "inability to communicate"
bugs that manifest as a silent unexplained authentication failure but are
really crypto.AEAD.Open being invoked with badly aligned buffers.
In #21624 I suggested using a panic as the consequence of bad alignment,
so that this kind of failure is loud and clearly different from, say, a
corrupted or invalid message signature. Adding the panic here made
my failure very easy to track down, once I realized that was the problem.
I don't want to debug another one of these.
Also using this CL as an experiment to get data about the impact of
maybe applying this change more broadly in the master branch.
Change-Id: Id2e2d8e980439f8acacac985fc2674f7c96c5032
Reviewed-on: https://go-review.googlesource.com/63915
Reviewed-by: Adam Langley <agl@golang.org>
This matches the standard GenerateKey and more importantly Precompute,
so that if you generate a key and then store it, read it back, call Precompute
on the new copy, and then do reflect.DeepEqual on the two copies, they
will match. Before this CL, the original key had CRTValues == nil and the
reconstituted key has CRTValues != nil (but len(CRTValues) == 0).
Change-Id: I1ddc64342a50a1b65a48d827e4d564f1faab1945
Reviewed-on: https://go-review.googlesource.com/63914
Reviewed-by: Adam Langley <agl@golang.org>
When using the go command, test binaries end in .test,
but when using Bazel, test binaries conventionally end in _test.
Change-Id: Ic4cac8722fd93ae316169f87b321f68e0b71f0c3
Reviewed-on: https://go-review.googlesource.com/63913
Reviewed-by: Adam Langley <agl@golang.org>
In routines like GenerateKey, where bits from the randomness source have a
visible effect on the output, we bypass BoringCrypto if given a non-standard
randomness source (and also assert that this happens only during tests).
In the decryption paths, the randomness source is only for blinding and has
no effect on the output, so we unconditionally invoke BoringCrypto, letting it
use its own randomness source as it sees fit. This in turn lets us verify that
the non-BoringCrypto decryption function is never called, not even in tests.
Unfortunately, while the randomness source has no visible effect on the
decrypt operation, the decrypt operation does have a visible effect on
the randomness source. If decryption doesn't use the randomness source,
and it's a synthetic stream, then a future operation will read a different
position in the stream and may produce different output. This happens
in tests more often than you'd hope.
To keep behavior of those future operations unchanged while still
ensuring that the original decrypt is never called, this CL adds a
simulation of the blinding preparation, to discard the right amount
from the random source before invoking BoringCrypto.
Change-Id: If2f87b856c811b59b536187c93efa99a97721419
Reviewed-on: https://go-review.googlesource.com/63912
Reviewed-by: Adam Langley <agl@golang.org>
This is documented to work (in hash.Hash's definition)
and existing code assumes it works. Add a test.
Change-Id: I63546f3b2d66222683a4f268a4eaff835fd836fe
Reviewed-on: https://go-review.googlesource.com/63911
Reviewed-by: Adam Langley <agl@golang.org>
hmac.New returns a hash.Hash, which defines Sum as:
// Sum appends the current hash to b and returns the resulting slice.
// It does not change the underlying hash state.
Sum(b []byte) []byte
I've now seen two different pieces of code that make
use of the assumption that Sum has no effect on the
internal state, so make it so.
Test in next CL in stack, so that it can be cherry-picked
to master.
Change-Id: Iad84ab3e2cc12dbecef25c3fc8f2662d157b0d0b
Reviewed-on: https://go-review.googlesource.com/63910
Reviewed-by: Adam Langley <agl@golang.org>
The standard Go crypto/rsa allows signatures to be shorter
than the RSA modulus and assumes leading zeros.
BoringCrypto does not, so supply the leading zeros explicitly.
This fixes the golang.org/x/crypto/openpgp tests.
Change-Id: Ic8b18d6beb0e02992a0474f5fdb2b73ccf7098cf
Reviewed-on: https://go-review.googlesource.com/62170
Reviewed-by: Adam Langley <agl@golang.org>
Did not consider these fields being embedded or adopted
into structs defined in other packages, but that's possible too.
Refine the import path check to account for that.
Fixes 'go test -short golang.org/x/crypto/ssh' but also
adds a new test in internal/boring for the same problem.
Change-Id: Ied2d04fe2b0ac3b0a34f07bc8dfc50fc203abb9f
Reviewed-on: https://go-review.googlesource.com/62152
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This is terrible but much simpler, cleaner, and more effective
than all the alternatives I have come up with.
Lots of code assumes that reflect.DeepEqual is meaningful
on rsa.PublicKey etc, because previously they consisted only of
exported meaningful fields.
Worse, there exists code that assumes asn1.Marshal can be
passed an rsa.PublicKey, because that struct has historically
matched exactly the form that would be needed to produce
the official ASN.1 DER encoding of an RSA public key.
Instead of tracking down and fixing all of that code
(and probably more), we can limit the BoringCrypto-induced
damage by ensliting the compiler to hide the new field
from reflection. Then nothing can get at it and nothing can
be disrupted by it.
Kill two birds with one cannon ball.
I'm very sorry.
Change-Id: I0ca4d6047c7e98f880cbb81904048c1952e278cc
Reviewed-on: https://go-review.googlesource.com/60271
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Test is in a separate CL for easier cherry-picking to master branch.
Change-Id: Ia4a9032892d2896332010fe18a3216f8c4a58d1c
Reviewed-on: https://go-review.googlesource.com/59770
Reviewed-by: Adam Langley <agl@golang.org>
The DWARF code is mishandling the case when the host object files
define multiple (distinct) symbols with the same name. They are mapped
to the same DWARF debug symbol, which then appears on the dwarfp
list multiple times, which then breaks the code that processes the list.
Detect duplicates and skip them, because that's trivial, instead of fixing
the underlying problem.
See #21566.
Change-Id: Ib5a34c891d7c15f4c7bb6239d8f31a1ec767b8bc
Reviewed-on: https://go-review.googlesource.com/57943
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
[This is a cherry-pick of CL 54090 to the 1.9 release branch.]
gc.Sysfunc must not be called concurrently.
We set up runtime routines used by the backend
prior to doing any backend compilation.
I missed the 387 ones; fix that.
Sysfunc should have been unexported during 1.9.
I will rectify that in a subsequent CL.
Fixes#21352
Change-Id: I485bb1867b46d8e5cf64bc820b8963576dc16174
Reviewed-on: https://go-review.googlesource.com/55970
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Using atomic.Value causes vet errors in code copying
PublicKey or PrivateKey structures. I don't think the errors
are accurate, but it's easier to work around them than
to change vet or change atomic.Value.
See #21504.
Change-Id: I3a3435c1fc664cc5166c81674f6f7c58dab35f21
Reviewed-on: https://go-review.googlesource.com/56671
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Because profile labels are copied from the goroutine into the tag
buffer by the signal handler, there's a carefully-crafted set of race
detector annotations to create the necessary happens-before edges
between setting a goroutine's profile label and retrieving it from the
profile tag buffer.
Given the constraints of the signal handler, we have to approximate
the true synchronization behavior. Currently, that approximation is
too weak.
Ideally, runtime_setProfLabel would perform a store-release on
&getg().labels and copying each label into the profile would perform a
load-acquire on &getg().labels. This would create the necessary
happens-before edges through each individual g.labels object.
Since we can't do this in the signal handler, we instead synchronize
on a "labelSync" global. The problem occurs with the following
sequence:
1. Goroutine 1 calls setProfLabel, which does a store-release on
labelSync.
2. Goroutine 2 calls setProfLabel, which does a store-release on
labelSync.
3. Goroutine 3 reads the profile, which does a load-acquire on
labelSync.
The problem is that the load-acquire only synchronizes with the *most
recent* store-release to labelSync, and the two store-releases don't
synchronize with each other. So, once goroutine 3 touches the label
set by goroutine 1, we report a race.
The solution is to use racereleasemerge. This is like a
read-modify-write, rather than just a store-release. Each RMW of
labelSync in runtime_setProfLabel synchronizes with the previous RMW
of labelSync, and this ultimately carries forward to the load-acquire,
so it synchronizes with *all* setProfLabel operations, not just the
most recent.
Change-Id: Iab58329b156122002fff12cfe64fbeacb31c9613
Reviewed-on: https://go-review.googlesource.com/57190
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The code was adding race.Errors to t.raceErrors before checking
Failed, but Failed was using t.raceErrors+race.Errors. We don't want
to change Failed, since that would affect tests themselves, so modify
the harness to not unnecessarily change t.raceErrors.
Updates #19851Fixes#21338
Change-Id: I483f27c68c340928f1cbdef160abc0a5716efb5d
Reviewed-on: https://go-review.googlesource.com/57151
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Cherry-pick CL 56890.
Normally 64-bit div/mod is turned into runtime calls on 32-bit
arch, but the front end leaves power-of-two constant division
and hopes the SSA backend turns into a shift or AND. The SSA rule is
(Mod64u <t> n (Const64 [c])) && isPowerOfTwo(c) -> (And64 n (Const64 <t> [c-1]))
But isPowerOfTwo returns true only for positive int64, which leaves
out 1<<63 unhandled. Add a special case for 1<<63.
Fixes#21517.
Change-Id: Ic91f86fd5e035a8bb64b937c15cb1c38fec917d6
Reviewed-on: https://go-review.googlesource.com/57070
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If we substitute a SHA1 implementation where the entirety of the
reading of the buffer is done in assembly (or C called from cgo),
then the race detector cannot observe the race.
Change to crc32 with a fake polynomial, in the hope that it will
always be handled by Go code, not optimized assembly or cgo calls.
Change-Id: I34e90b14ede6bc220ef686f6aef16b8e464b5cde
Reviewed-on: https://go-review.googlesource.com/56510
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Right now the package doesn't do anything useful, but it will.
This CL is about the machinery for building goboringcrypto_linux_amd64.syso
and then running the self-test and checking FIPS_mode from Go init.
Change-Id: I4ec0f5efaa88ccfb506b9818d24a7f1cbcc5a7d6
Reviewed-on: https://go-review.googlesource.com/55472
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
579120323f runtime: mapassign_* should use typedmemmove to update keys
380525598c all: remove some manual hyphenation
f096b5b340 runtime: mark activeModules nosplit/nowritebarrier
3e3da54633 math/bits: fix example for OnesCount64
9b1e7cf2ac math/bits: add examples for OnesCount functions
b01db023b1 misc/cgo/testsanitizers: also skip tsan11/tsan12 when using GCC
a279b53a18 reflect: document how DeepEqual handles cycles
909f409a8d doc: mention handling of moved GOROOT in 1.9 release notes
58ad0176ca doc: use better wording to explain type-aware completion
92dac21d29 doc: replace paid with commercial
9bb98e02de doc/1.9: add CL 43712, ReverseProxy of HTTP/2 trailers to the release notes.
78d74fc2cd doc: clarify that Gogland is for paid IntelliJ platform IDEs
5495047223 doc/1.9: fix broken html link in CL 53030/53210
890e0e862f doc: fix bad link in go1.9 release notes
be596f049a doc/1.9: fix stray html in CL 53030
0173631d53 encoding/binary: add examples for varint functions
ac0ccf3cd2 doc/1.9: add CL 36696 for crypto/x509 to the release notes
cc402c2c4d doc: hide blog content for golang.google.cn
f396fa4285 internal/poll: don't add non-sockets to runtime poller
664cd26c89 cmd/vet: don't exit with failure on type checking error
a8730cd93a doc: hide video and share if being served from CN
b63db76c4a testsanitizers: check that tsan program runs, skip tsan10 on gcc
193eda7291 time: skip ZoneAbbr test in timezones with no abbreviation
6f08c935a9 cmd/go: show examples with empty output in go test -list
f20944de78 cmd/compile: set/unset base register for better assembly print
623e2c4603 runtime: map bitmap and spans during heap initialization
780249eed4 runtime: fall back to small mmaps if we fail to grow reservation
31b2c4cc25 .github: add .md extension to SUPPORT file
ac29f30dbb plugin: mention that there are known bugs with plugins
45a4609c0a cmd/dist: skip moved GOROOT on Go's Windows builders when not sharding tests
e157fac02d test: add README
835dfef939 runtime/pprof: prevent a deadlock that SIGPROF might create on mips{,le}
df91b8044d doc: list editor options by name, not plugin name
3d9475c04b doc: cleanup editor page
b9661a14ea doc: add Atom to editor guide
ee392ac10c cmd/compile: consider exported flag in namedata
Change-Id: I3a48493e8c05d97cb3b61635503ef0ccd646e5cb
We lazily map the bitmap and spans areas as the heap grows. However,
right now we're very slightly too lazy. Specifically, the following
can happen on 32-bit:
1. mallocinit fails to allocate any heap arena, so
arena_used == arena_alloc == arena_end == bitmap.
2. There's less than 256MB between the end of the bitmap mapping and
the next mapping.
3. On the first allocation, mheap.sysAlloc sees that there's not
enough room in [arena_alloc, arena_end) because there's no room at
all. It gets a 256MB mapping from somewhere *lower* in the address
space than arena_used and sets arena_alloc and arena_end to this
hole.
4. Since the new arena_alloc is lower than arena_used, mheap.sysAlloc
doesn't bother to call mheap.setArenaUsed, so we still don't have a
bitmap mapping or a spans array mapping.
5. mheap.grow, which called mheap.sysAlloc, attempts to fill in the
spans array and crashes.
Fix this by mapping the metadata regions for the initial arena_used
when the heap is initialized, rather than trying to wait for an
allocation. This maintains the intended invariant that the structures
are always mapped for [arena_start, arena_used).
Fixes#21044.
Cherry-pick of CL 51714. Fixes#21234.
Change-Id: I4422375a6e234b9f979d22135fc63ae3395946b0
Reviewed-on: https://go-review.googlesource.com/52191
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Right now, if it's possible to grow the arena reservation but
mheap.sysAlloc fails to get 256MB more of memory, it simply fails.
However, on 32-bit we have a fallback path that uses much smaller
mmaps that could take in this situation, but fail to.
This commit fixes mheap.sysAlloc to use a common failure path in case
it can't grow the reservation. On 32-bit, this path includes the
fallback.
Ideally, mheap.sysAlloc would attempt smaller reservation growths
first, but taking the fallback path is a simple change for Go 1.9.
Updates #21044 (fixes one of two issues).
Cherry-pick of CL 51713. Updates #21234.
Change-Id: I1e0035ffba986c3551479d5742809e43da5e7c73
Reviewed-on: https://go-review.googlesource.com/52190
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is possible to have an unexported name with a nil package,
for an embedded field whose type is a pointer to an unexported type.
We must encode that fact in the type..namedata symbol name,
to avoid incorrectly merging an unexported name with an exported name.
Fixes#21120
Change-Id: I2e3879d77fa15c05ad92e0bf8e55f74082db5111
Reviewed-on: https://go-review.googlesource.com/50710
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-on: https://go-review.googlesource.com/50970
Reviewed-by: Chris Broadfoot <cbro@golang.org>
2017-07-24 18:12:06 +00:00
260 changed files with 10735 additions and 2016 deletions
In general, systems that need consistent formatting of Go source code should
use a specific version of the <code>gofmt</code> binary.
See the <ahref="/pkg/go/format/">go/format</a> package godoc for more
See the <ahref="/pkg/go/format/">go/format</a> package documentation for more
information.
</p>
<h3id="run">Run</h3>
<p>
<!-- CL 109341 -->
The <ahref="/cmd/go/"><code>go</code> <code>run</code></a>
command now allows a single import path, a directory name or a
pattern matching a single package.
This allows <code>go</code> <code>run</code> <code>pkg</code> or <code>go</code> <code>run</code> <code>dir</code>, most importantly <code>go</code> <code>run</code> <code>.</code>
# Copied from https://raw.githubusercontent.com/docker-library/golang/master/1.9-rc/stretch/go-wrapper
# Copied into Docker images.
set -e
usage() {
base="$(basename "$0")"
cat <<EOUSAGE
usage: $base command [args]
This script assumes that is is run from the root of your Go package (for
example, "/go/src/app" if your GOPATH is set to "/go").
In Go 1.4, a feature was introduced to supply the canonical "import path" for a
given package in a comment attached to a package statement
(https://golang.org/s/go14customimport).
This script allows us to take a generic directory of Go source files such as
"/go/src/app" and determine that the canonical "import path" of where that code
expects to live and reference itself is "github.com/jsmith/my-cool-app". It
will then ensure that "/go/src/github.com/jsmith/my-cool-app" is a symlink to
"/go/src/app", which allows us to build and run it under the proper package
name.
For compatibility with versions of Go older than 1.4, the "import path" may also
be placed in a file named ".godir".
Available Commands:
$base download
$base download -u
(equivalent to "go get -d [args] [godir]")
$base install
$base install -race
(equivalent to "go install [args] [godir]")
$base run
$base run -app -specific -arguments
(assumes "GOPATH/bin" is in "PATH")
EOUSAGE
}
# make sure there is a subcommand specified
if [ "$#" -eq 0 ]; then
usage >&2
exit 1
fi
# "shift" so that "$@" becomes the remaining arguments and can be passed along to other "go" subcommands easily
cmd="$1"
shift
goDir="$(go list -e -f '{{.ImportComment}}' 2>/dev/null || true)"
if [ -z "$goDir" -a -s .godir ]; then
goDir="$(cat .godir)"
fi
dir="$(pwd -P)"
if [ "$goDir" ]; then
goPath="${GOPATH%%:*}" # this just grabs the first path listed in GOPATH, if there are multiple (which is the detection logic "go get" itself uses, too)
goDirPath="$goPath/src/$goDir"
mkdir -p "$(dirname "$goDirPath")"
if [ ! -e "$goDirPath" ]; then
ln -sfv "$dir" "$goDirPath"
elif [ ! -L "$goDirPath" ]; then
echo >&2 "error: $goDirPath already exists but is unexpectedly not a symlink!"
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
