This patch reinstates a fix for PowerPC with regard to making VDSO calls
while receiving a signal, and subsequently crashing. The crash happens
because certain VDSO calls can modify the r30 register, which is where g
is stored. This change was reverted for PowerPC because r30 is supposed
to be a non-volatile register. This is true, but that only makes a
guarantee across function calls, but not "within" a function call. This
patch was seemingly fine before because the Linux kernel still had hand
rolled assembly VDSO function calls, however with a recent change to C
function calls it seems the compiler used can generate instructions
which temporarily clobber r30. This means that when we receive a signal
during one of these calls the value of r30 will not be the g as the
runtime expects, causing a segfault.
You can see from this assembly dump how the register is clobbered during
the call:
(the following is from a 5.13rc2 kernel)
```
Dump of assembler code for function __cvdso_clock_gettime_data:
0x00007ffff7ff0700 <+0>: cmplwi r4,15
0x00007ffff7ff0704 <+4>: bgt 0x7ffff7ff07f0 <__cvdso_clock_gettime_data+240>
0x00007ffff7ff0708 <+8>: li r9,1
0x00007ffff7ff070c <+12>: slw r9,r9,r4
0x00007ffff7ff0710 <+16>: andi. r10,r9,2179
0x00007ffff7ff0714 <+20>: beq 0x7ffff7ff0810 <__cvdso_clock_gettime_data+272>
0x00007ffff7ff0718 <+24>: rldicr r10,r4,4,59
0x00007ffff7ff071c <+28>: lis r9,32767
0x00007ffff7ff0720 <+32>: std r30,-16(r1)
0x00007ffff7ff0724 <+36>: std r31,-8(r1)
0x00007ffff7ff0728 <+40>: add r6,r3,r10
0x00007ffff7ff072c <+44>: ori r4,r9,65535
0x00007ffff7ff0730 <+48>: lwz r8,0(r3)
0x00007ffff7ff0734 <+52>: andi. r9,r8,1
0x00007ffff7ff0738 <+56>: bne 0x7ffff7ff07d0 <__cvdso_clock_gettime_data+208>
0x00007ffff7ff073c <+60>: lwsync
0x00007ffff7ff0740 <+64>: mftb r30 <---- RIGHT HERE
=> 0x00007ffff7ff0744 <+68>: ld r12,40(r6)
```
What I believe is happening is that the kernel changed the PowerPC VDSO
calls to use standard C calls instead of using hand rolled assembly. The
hand rolled assembly calls never touched r30, so this change was safe to
roll back. That does not seem to be the case anymore as on the 5.13rc2
kernel the compiler *is* generating assembly which modifies r30, making
this change again unsafe and causing a crash when the program receives a
signal during these calls (which will happen often due to async
preempt). This change happened here:
https://lwn.net/ml/linux-kernel/235e5571959cfa89ced081d7e838ed5ff38447d2.1601365870.git.christophe.leroy@csgroup.eu/.
I realize this was reverted due to unexplained hangs in PowerPC
builders, but I think we should reinstate this change and investigate
those issues separately:
f4ca3c1e0aFixes#46857
Change-Id: Ib18d7bbfc80a1a9cb558f0098878d41081324b52
GitHub-Last-Rev: c3002bcfca
GitHub-Pull-Request: golang/go#46767
Reviewed-on: https://go-review.googlesource.com/c/go/+/328110
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
(cherry picked from commit 16e82be454)
Reviewed-on: https://go-review.googlesource.com/c/go/+/334411
Run-TryBot: Carlos Amedee <carlos@golang.org>
Instead of hard failing on a single bad record, filter the bad records
and return anything valid. This only applies to the methods which can
return multiple records, LookupMX, LookupNS, LookupSRV, and LookupAddr.
When bad results are filtered out, also return an error, indicating
that this filtering has happened.
Updates #46241
Updates #46979Fixes#47012
Change-Id: I6493e0002beaf89f5a9795333a93605abd30d171
Reviewed-on: https://go-review.googlesource.com/c/go/+/332549
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 296ddf2a93)
Reviewed-on: https://go-review.googlesource.com/c/go/+/333331
Run-TryBot: Roland Shoemaker <roland@golang.org>
The compiler machinery that generates "map.zero" symbols marks them as
RODATA and DUPOK, which is problematic when a given application has
multiple map zero symbols (from different packages) with varying
sizes: the dupok path in the loader assumes that if two symbols have
the same name, it is safe to pick any of the versions. In the case of
map.zero, the link needs to select the largest symbol, not an
arbitrary sym.
This patch changes the linker's dupok symbol loading path to detect
this problem, and in situations where we're loading a dupok symbol
whose name is the same as an existing symbol but whose size is large,
select the new dup over the old. Note: this fix differs from the one
used in 1.16/1.17, which uses content-addressable symbols instead.
Fixes#46656.
Change-Id: Iabd2feef01d448670ba795c7eaddc48c191ea276
Reviewed-on: https://go-review.googlesource.com/c/go/+/326211
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit aa5540cd82)
Reviewed-on: https://go-review.googlesource.com/c/go/+/326213
When testing if a flag (e.g. "-no-pie") is supported by the
external linker, pass arch-specific flags (like "-marm").
In particular, on the ARM builder, if CGO_LDFLAGS=-march=armv6
is set, the C toolchain fails to build if -marm is not passed.
# cc -march=armv6 1.c
1.c: In function 'main':
1.c:3:1: sorry, unimplemented: Thumb-1 hard-float VFP ABI
int main() {
^~~
This makes the Go linker think "-no-pie" is not supported when it
actually is.
Passing -marm makes it work.
Fixes#46684.
Change-Id: I4e8b71f08818993cbbcb2494b310c68d812d6b50
Reviewed-on: https://go-review.googlesource.com/c/go/+/278592
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit a318d56c1e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/326711
Run-TryBot: Carlos Amedee <carlos@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
This backports the test from CL 319669, but — because of extensive
changes to the module loader during the Go 1.16 and 1.17 cycles — the
implementation is entirely different. (This implementation is based on
the addGoStmt function already present in init.go.)
Fixes#46143
Updates #46142
Change-Id: Ib7a0a159e53cbe476be6aa9a050add10cc750dec
Reviewed-on: https://go-review.googlesource.com/c/go/+/319710
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
For some reason, the go.mod file added to this test in CL 147281 lists
'go 1.20' instead of the version that was actually current when the
go.mod file was added.
That causes the test's behavior to change under lazy loading, because
1.20 is above the threshold to trigger lazy-loading invariants (1.17).
This backports CL 314049 to Go 1.15 in order to fix a spurious test
failure in a subsequent change.
For #46143
Updates #46142
Updates #36460
Change-Id: I92400996cb051ab30e99bfffafd91ff32a1e7087
Reviewed-on: https://go-review.googlesource.com/c/go/+/314049
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/319709
Reviewed-by: Jay Conrod <jayconrod@google.com>
When creating programs with large text sections on ppc64le,
trampolines are needed for calls that are too far; however
they are not created if the code is generated such that the TOC
register r2 is initialized and maintained in the code because
then the external linker can create the trampolines. Previously
the function DynlinkingGo was used to determine this but in the
case where plugins are used, this could return true even though
r2 is not valid.
To fix this problem I've added a new function r2Valid which returns
true when the build options indicate that the r2 is
initialized and maintained. Because of the ways that
DynlinkingGo is used I wanted to maintain its previous
behavior.
Fixes#46002
Change-Id: I6d902eba6ad41757aa6474948b79acdbd479cb38
Reviewed-on: https://go-review.googlesource.com/c/go/+/315289
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
(cherry picked from commit 9ed736ac2a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/317974
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The timerpMask optimization updates a mask of Ps (potentially)
containing timers in pidleget / pidleput. For correctness, it depends on
the assumption that new timers can only be added to a P's own heap.
addtimer violates this assumption if it is preempted after computing pp.
That G may then run on a different P, but adding a timer to the original
P's heap.
Avoid this by disabling preemption while pp is in use.
Other uses of doaddtimer should be OK:
* moveTimers: always moves to the current P's heap
* modtimer, cleantimers, addAdjustedTimers, runtimer: does not add net
new timers to the heap while locked
For #44868Fixes#45731
Change-Id: I4a5d080865e854931d0a3a09a51ca36879101d72
Reviewed-on: https://go-review.googlesource.com/c/go/+/300610
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/313129
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
If the current time is computed from extend string
and the zone file contains multiple zones with the
same name, the lookup by name might find incorrect
zone.
This happens for example with the slim Europe/Dublin
time zone file in the embedded zip. This zone file
has last transition in 1996 and rest is covered by
extend string.
tzset returns IST as the zone name to use, but there
are two records with IST name. Lookup by name finds
the wrong one. We need to check offset and isDST too.
In case we can't find an existing zone, we allocate
a new zone so that we use correct offset and isDST.
I have renamed zone variable to zones as it shadowed
the zone type that we need to allocate the cached zone.
Backport note: this change also incorporates portions of
CL 264077.
For #45370Fixes#45384
Change-Id: I43d416d009e20878261156c821a5784e2407ed1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/307212
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
When using cgo, some of the frames can be provided by cgoTraceback, a
cgo-provided function to generate C tracebacks. Unlike Go tracebacks,
cgoTraceback has no particular guarantees that it produces valid
tracebacks.
If one of the (invalid) frames happens to put the PC in the alignment
region at the end of a function (filled with int 3's on amd64), then
Frames.Next will find a valid funcInfo for the PC, but pcdatavalue will
panic because PCDATA doesn't cover this PC.
Tolerate this case by doing a non-strict PCDATA lookup. We'll still show
a bogus frame, but at least avoid throwing.
For #44971Fixes#45302
Change-Id: I9eed728470d6f264179a7615bd19845c941db78c
Reviewed-on: https://go-review.googlesource.com/c/go/+/301369
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit e4a4161f1f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/305890
We need to be careful that when doing value graph surgery, we not
re-substitute a value that has already been substituted. That can lead
to confusing a previous iteration's value with the current iteration's
value.
The simple fix in this CL just aborts the optimization if it detects
intertwined phis (a phi which is the argument to another phi). It
might be possible to keep the optimization with a more complicated
CL, but:
1) This CL is clearly safe to backport.
2) There were no instances of this abort triggering in
all.bash, prior to the test introduced in this CL.
Fixes#45187
Change-Id: I2411dca03948653c053291f6829a76bec0c32330
Reviewed-on: https://go-review.googlesource.com/c/go/+/304251
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 771c57e68e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/304529
Tx acquires tx.closemu W-lock and then acquires stmt.closemu.W-lock
to fully close the transaction and associated prepared statement.
Stmt query and execution run in reverse ways - acquires
stmt.closemu.R-lock and then acquires tx.closemu.R-lock to grab tx
connection, which may cause deadlock.
Prevent the lock is held around tx.closePrepared to ensure no
deadlock happens.
Includes a test fix from CL 266097.
Fixes#42884
Updates #40985
Updates #42259
Change-Id: Id52737660ada3cebdfff6efc23366cdc3224b8e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/250178
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
(cherry picked from commit d4c1ad8829)
Reviewed-on: https://go-review.googlesource.com/c/go/+/284513
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
We used to clear GOPATH in all the build scripts.
Clearing GOPATH is misleading at best, since you just end up
with the default GOPATH (%USERPROFILE%\go on Windows).
Unless that's your GOROOT, in which case you end up with a
fatal error from the go command (#43938).
run.bash changed to setting GOPATH=/dev/null, which has no
clear analogue on Windows.
run.rc still clears GOPATH.
Change them all to set GOPATH to a non-existent directory
/nonexist-gopath or c:\nonexist-gopath.
For #45238.
Fixes#45239.
Change-Id: I51edd66d37ff6a891b0d0541d91ecba97fbbb03d
Reviewed-on: https://go-review.googlesource.com/c/go/+/288818
Trust: Russ Cox <rsc@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
(cherry picked from commit bb6efb9609)
Reviewed-on: https://go-review.googlesource.com/c/go/+/304773
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Trust: Carlos Amedee <carlos@golang.org>
Currently, in the trampoline generation pass we expect packages
are laid out in dependency order, so a cross-package jump always
has a known target address so we can check if a trampoline is
needed. With linknames, there can be cycles in the package
dependency graph, making this algorithm no longer work. For them,
as the target address is unkown we conservatively generate a
trampoline. This may generate unnecessary trampolines (if the
packages turn out laid together), but package cycles are extremely
rare so this is fine.
Updates #44639.
Fixes#44748.
Change-Id: I2dc2998edacbda27d726fc79452313a21d07787a
Reviewed-on: https://go-review.googlesource.com/c/go/+/292490
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit 098504c73f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/298030
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes#44872
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298352
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 302a400316)
Reviewed-on: https://go-review.googlesource.com/c/go/+/299830
'go mod tidy' and 'go mod vendor' normally report errors when a
package can't be imported, even if the import appears in a file that
wouldn't be compiled by the current version of Go. These errors are
common for packages introduced in higher versions of Go, like "embed"
in 1.16.
This change causes 'go mod tidy' and 'go mod vendor' to ignore
missing package errors if the import path appears to come from the
standard library because it lacks a dot in the first path element.
NOTE: This change is not a clean cherry-pick of CL 298749 because
parts of modload were substantially rewritten after 1.15.
Fixes#44792
Updates #27063
Change-Id: I61d6443e77ab95fd8c0d1514f57ef4c8885a77cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/298749
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
(cherry picked from commit 56d52e6611)
Reviewed-on: https://go-review.googlesource.com/c/go/+/298950
Issue #41600 fixed the issue when a second request canceled a connection
while the first request was still in roundTrip.
This uncovered a second issue where a request was being canceled (in
roundtrip) but the connection was put back into the idle pool for a
subsequent request.
The fix is the similar except its now in readLoop instead of roundTrip.
A persistent connection is only added back if it successfully removed
the cancel function; otherwise we know the roundTrip has started
cancelRequest.
Fixes#42935.
Updates #42942.
Change-Id: Ia56add20880ccd0c1ab812d380d8628e45f6f44c
Reviewed-on: https://go-review.googlesource.com/c/go/+/274973
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 854a2f8e01)
Reviewed-on: https://go-review.googlesource.com/c/go/+/297910
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
One of escape analysis's responsibilities is to summarize whether/how
each function parameter flows to the heap so we can correctly
incorporate those flows into callers' escape analysis data flow
graphs.
As an optimization, we separately record when parameters flow to
result parameters, so that we can more precisely analyze parameter
flows based on how the results are used at the call site. However, if
a named result parameter itself needs to be heap allocated, this
optimization isn't safe and the parameter needs to be recorded as
flowing to heap rather than flowing to result.
Escape analysis used to get this correct because it conservatively
rewalked the data-flow graph multiple times. So even though it would
incorrectly record the result parameter flow, it would separately find
a flow to the heap. However, CL 196811 (specifically, case 3)
optimized the walking logic to reduce unnecessary rewalks causing us
to stop finding the extra heap flow.
This CL fixes the issue by correcting location.leakTo to be sensitive
to sink.escapes and not record result-flows when the result parameter
escapes to the heap.
Fixes#44658.
Change-Id: I48742ed35a6cab591094e2d23a439e205bd65c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/297289
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/297291
The third argument to GetQueuedCompletionStatus is a pointer to a
uintptr, not a uint32. Users of this functions have therefore been
corrupting their memory every time they used it. Either that memory
corruption was silent (dangerous), or their programs didn't work so they
chose a different API to use.
This fixes the problem by passing through an intermediate buffer.
Updates #44538.
Fixes#44592.
Change-Id: Icacd71f705b36e41e52bd8c4d74898559a27522f
Reviewed-on: https://go-review.googlesource.com/c/go/+/296151
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
These replacement rules assume that TST and TEQ set V. But TST and
TEQ do not set V. This is a problem because instructions like LT are
actually checking for N!=V. But with TST and TEQ not setting V, LT
doesn't do anything meaningful. It's possible to construct trivial
miscompilations from this, such as:
package main
var x = [4]int32{-0x7fffffff, 0x7fffffff, 2, 4}
func main() {
if x[0] > x[1] {
panic("fail 1")
}
if x[2]&x[3] < 0 {
panic("fail 2") // Fails here
}
}
That first comparison sets V, via the CMP that subtracts the values
causing the overflow. Then the second comparison operation thinks that
it uses the result of TST, when it actually uses the V from CMP.
Before this fix:
TST R0, R1
BLT loc_6C164
After this fix:
TST R0, R1
BMI loc_6C164
The BMI instruction checks the N flag, which TST sets. This commit
fixes the issue by using [LG][TE]noov instead of vanilla [LG][TE], and
also adds a test case for the direct issue.
Updates #42876.
Fixes#42930.
Change-Id: I13c62c88d18574247ad002b671b38d2d0b0fc6fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/282432
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In net/http package, the ServeContent/ServeFile doesn't check the I/O
timeout error from chunkWriter or *net.TCPConn, which means that both
HTTP status and headers might be missing when WriteTimeout happens. If
the poll.SendFile() doesn't check the *poll.FD state before sending
data, the client will only receive the response body with status and
report "malformed http response/status code".
This patch is to enable netpollcheckerr before sendfile, which should
align with normal *poll.FD.Write() and Splice().
For #43822Fixes#44294
Change-Id: I32517e3f261bab883a58b577b813ef189214b954
Reviewed-on: https://go-review.googlesource.com/c/go/+/285914
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
(cherry picked from commit f0d23c9dbb)
Reviewed-on: https://go-review.googlesource.com/c/go/+/296530
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
On current Linux kernels copy_file_range does not correctly handle
files in certain special file systems, such as /proc. For those file
systems it fails to copy any data and returns zero. This breaks Go's
io.Copy for those files.
Fix the problem by assuming that if copy_file_range returns 0 the
first time it is called on a file, that that file is not supported.
In that case fall back to just using read. This will force an extra
system call when using io.Copy to copy a zero-sized normal file,
but at least it will work correctly.
For #36817
For #44272Fixes#44273
Change-Id: I02e81872cb70fda0ce5485e2ea712f219132e614
Reviewed-on: https://go-review.googlesource.com/c/go/+/291989
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit 30641e36aa)
Reviewed-on: https://go-review.googlesource.com/c/go/+/292289
Before this CL, the following sequence was possible:
* GC scavenger starts and sets up scavenge.timer
* GC calls readyForScavenger, but sysmon is sleeping
* program calls runtime.GOMAXPROCS to shrink number of processors
* procresize destroys a P, the one that scavenge.timer is on
* (*pp).destroy calls moveTimers, which gets to the scavenger timer
* scavenger timer is timerWaiting, and moveTimers clears t.pp
* sysmon wakes up and calls wakeScavenger
* wakeScavengers calls stopTimer on scavenger.timer, still timerWaiting
* stopTimer calls deltimer which loads t.pp, which is still nil
* stopTimer tries to increment deletedTimers on nil t.pp, and crashes
The point of vulnerability is the time that t.pp is set to nil by
moveTimers and the time that t.pp is set to non-nil by moveTimers,
which is a few instructions at most. So it's not likely and in
particular is quite unlikely on x86. But with a more relaxed memory
model the area of vulnerability can be somewhat larger. This appears
to tbe the cause of two builder failures in a few months on linux-mips.
This CL fixes the problem by making moveTimers change the status from
timerWaiting to timerMoving while t.pp is clear. That will cause
deltimer to wait until the status is back to timerWaiting, at which
point t.pp has been set again.
For #43712Fixes#43833
Change-Id: I66838319ecfbf15be66c1fac88d9bd40e2295852
Reviewed-on: https://go-review.googlesource.com/c/go/+/284775
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit d2d155d1ae)
Reviewed-on: https://go-review.googlesource.com/c/go/+/287092
Run-TryBot: Carlos Amedee <carlos@golang.org>
The code in the Go object file reader was casting a pointer to mmaped
memory into a large array prior to performing a read of the
relocations section:
return (*[1<<20]Reloc)(unsafe.Pointer(&r.b[off]))[:n:n]
For very large object files, this artificial array isn't large enough
(that is, there are more than 1048576 relocs to read), so update the
code to use a larger artifical array size.
Fixes#43214.
Updates #41621.
Change-Id: Ic047c8aef4f8a3839f2e7e3594bce652ebd6bd5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/278492
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Trust: Than McIntosh <thanm@google.com>
(cherry picked from commit f4e7a6b905)
Reviewed-on: https://go-review.googlesource.com/c/go/+/278673
The resource symbol may have been copied to the mmap'd
output buffer. If so, certain conditions can cause that
mmap'd output buffer to be munmap'd before we get a chance
to use it. To avoid any issues we copy the data to the heap
when the resource symbol exists.
Fixes#42384
Change-Id: I32ef5420802d7313a3d965b8badfbcfb9f0fba4a
GitHub-Last-Rev: 7b0f43011d
GitHub-Pull-Request: golang/go#42427
Reviewed-on: https://go-review.googlesource.com/c/go/+/268018
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Trust: Carlos Amedee <carlos@golang.org>
Previously, if CC was a path without separators (like gcc or clang),
we'd look it up in PATH in cmd/go using internal/execabs.LookPath,
then pass the resolved path to cgo in CC.
This caused a regression: if the directory in PATH containing CC has a
space, cgo splits it and interprets it as multiple arguments.
With this change, cmd/go no longer resolves CC before invoking
cgo. cgo does the path lookup on each invocation. This reverts the
security fix CL 284780, but that was redundant with the addition of
internal/execabs (CL 955304), which still protects us.
NOTE: This CL includes a related test fix from CL 286292.
Fixes#43860
Change-Id: I65d91a1e303856df8653881eb6e2e75a3bf95c49
Reviewed-on: https://go-review.googlesource.com/c/go/+/285873
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit a2cef9b544)
Reviewed-on: https://go-review.googlesource.com/c/go/+/285954
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Current optimization: When we copy a->b and then b->c, we might as well
copy a->c instead of b->c (then b might be dead and go away).
*Except* if a is a volatile location (might be clobbered by a call).
In that case, we really do want to copy a immediately, because there
might be a call before we can do the a->c copy.
User calls can't happen in between, because the rule matches up the
memory states. But calls inserted for memory barriers, particularly
runtime.typedmemmove, can.
(I guess we could introduce a register-calling-convention version
of runtime.typedmemmove, but that seems a bigger change than this one.)
Fixes#43575
Change-Id: Ifa518bb1a6f3a8dd46c352d4fd54ea9713b3eb1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/282492
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 304f769ffc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/282558
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Introduces a wrapper around os/exec, internal/execabs, for use in
all commands. This wrapper prevents exec.LookPath and exec.Command from
running executables in the current directory.
All imports of os/exec in non-test files in cmd/ are replaced with
imports of internal/execabs.
This issue was reported by RyotaK.
Fixes CVE-2021-3115
Change-Id: I0423451a6e27ec1e1d6f3fe929ab1ef69145c08f
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/955304
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-by: Katie Hockman <katiehockman@google.com>
(cherry picked from commit 44f09a6990ccf4db601cbf8208c89ac4e888f884)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/955308
This patch fixes two independent bugs in p224Contract, the function that
performs the final complete reduction in the P-224 field. Incorrect
outputs due to these bugs were observable from a high-level
P224().ScalarMult() call.
The first bug was in the calculation of out3GT. That mask was supposed
to be all ones if the third limb of the value is greater than the third
limb of P (out[3] > 0xffff000). Instead, it was also set if they are
equal. That meant that if the third limb was equal, the value was always
considered greater than or equal to P, even when the three bottom limbs
were all zero. There is exactly one affected value, P - 1, which would
trigger the subtraction by P even if it's lower than P already.
The second bug was more easily hit, and is the one that caused the known
high-level incorrect output: after the conditional subtraction by P, a
potential underflow of the lowest limb was not handled. Any values that
trigger the subtraction by P (values between P and 2^224-1, and P - 1
due to the bug above) but have a zero lowest limb would produce invalid
outputs. Those conditions apply to the intermediate representation
before the subtraction, so they are hard to trace to precise inputs.
This patch also adds a test suite for the P-224 field arithmetic,
including a custom fuzzer that automatically explores potential edge
cases by combining limb values that have various meanings in the code.
contractMatchesBigInt in TestP224Contract finds the second bug in less
than a second without being tailored to it, and could eventually find
the first one too by combining 0, (1 << 28) - 1, and the difference of
(1 << 28) and (1 << 12).
The incorrect P224().ScalarMult() output was found by the
elliptic-curve-differential-fuzzer project running on OSS-Fuzz and
reported by Philippe Antoine (Catena cyber).
Fixes CVE-2021-3114
Change-Id: I50176602d544de3da854270d66a293bcaca57ad7
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/947792
Reviewed-by: Katie Hockman <katiehockman@google.com>
(cherry picked from commit 5fa534e9c7eaeaf875e53b98eac9342b0855b283)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/955175
Immediately after a forward Seek, the offset we're writing to is
beyond len(buf)+len(heap):
|<--- buf --->|<--- heap --->|
^
off
If we do a copyHeap at this point, the new heapPos should not be
0:
|<---------- buf ----------->|<-heap->|
^
off
Recompute it.
Updates #42082Fixes#42948
Change-Id: Icb3e4e1c7bf7d1fd3d76a2e0d7dfcb319c661534
Reviewed-on: https://go-review.googlesource.com/c/go/+/270942
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Trust: Cherry Zhang <cherryyz@google.com>
MOVLconst must have a properly sign-extended auxint constant.
The bit operations in these rules don't enforce that invariant.
The easiest fix is just to turn on properly typed auxint fields
(which is what fixed this issue at tip).
Fixes#42753
Change-Id: I264245fad45067a6ade65326f7fe681feb5f3739
Reviewed-on: https://go-review.googlesource.com/c/go/+/272028
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The previous s value could cause a crash
for certain inputs.
Will check in tests and documentation improvements later.
Thanks to the Go Ethereum team and the OSS-Fuzz project for reporting this.
Thanks to Rémy Oudompheng and Robert Griesemer for their help
developing and validating the fix.
Fixes CVE-2020-28362
Change-Id: Ibbf455c4436bcdb07c84a34fa6551fb3422356d3
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/899974
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
(cherry picked from commit 28015462c2a83239543dc2bef651e9a5f234b633)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/901065
A hand-edited object file can have a symbol name that uses newline and
other normally invalid characters. The cgo tool will generate Go files
containing symbol names, unquoted. That can permit those symbol names
to inject Go code into a cgo-generated file. If that Go code uses the
//go:cgo_ldflag pragma, it can cause the C linker to run arbitrary
code when building a package. If you build an imported package we
permit arbitrary code at run time, but we don't want to permit it at
package build time. This CL prevents this in two ways.
In cgo, reject invalid symbols that contain non-printable or space
characters, or that contain anything that looks like a Go comment.
In the go tool, double check all //go:cgo_ldflag directives in
generated code, to make sure they follow the existing LDFLAG restrictions.
Thanks to Chris Brown and Tempus Ex for reporting this.
Fixes CVE-2020-28366
Change-Id: Ia1ad8f3791ea79612690fa7d26ac451d0f6df7c1
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/895832
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 6bc814dd2bbfeaafa41d314dd4cc591b575dfbf6)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/901056
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Backport of part of https://golang.org/cl/261877 to support the slim
tzdata format. As of tzdata 2020b, the default is to use the slim format.
We need to support that format so that Go installations continue to
work when tzdata is updated.
Relevant part of the CL description:
The reason for the failed tests was that when caching location data, the
extended time format past the end of zone transitions was not
considered. The respective change was introduced in (*Location).lookup
by CL 215539.
For #42138
Change-Id: I37f52a0917b2c6e3957e6b4612c8ef104c736e65
Reviewed-on: https://go-review.googlesource.com/c/go/+/264301
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
storeType splits compound stores up into a scalar parts and a pointer parts.
The scalar part happens unconditionally, and the pointer part happens
under the guard of a write barrier check.
Types which are declared as pointers, but are represented as scalars because
they might have "bad" values, were not handled correctly here. They ended
up not getting stored in either set.
Fixes#42151
Change-Id: I46f6600075c0c370e640b807066247237f93c7ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/264300
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 933721b8c7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/265719
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
During Go 1.15 development, a fix was added to the toolchain for issue
information. The 1.15 line tables were slightly malformed in the way
that they used the DWARF "end sequence" operator, resulting in
incorrect line table info for the final instruction in the final
function of a compilation unit.
This problem was fixed in https://golang.org/cl/235739, which made it
into Go 1.15. It now appears that while the fix works OK for linux, in
certain cases it causes issues with the Darwin linker (the "address
not in any section" ld64 error reported in issue #40974).
During Go 1.16 development, the fix in https://golang.org/cl/235739
was revised so as to fix another related problem (described in issue #39757);
the newer fix does not trigger the problem in the Darwin linker however.
This CL back-ports the changes in https://golang.org/cl/239286 to the
1.15 release branch, so as to fix the Darwin linker error.
Updates #38192.
Updates #39757.
Fixes#40974.
Change-Id: I9350fec4503cd3a76b97aaea0d8aed1511662e29
Reviewed-on: https://go-review.googlesource.com/c/go/+/258422
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Than McIntosh <thanm@google.com>
On current macOS versions a program that receives a signal during an
execve can fail with a SIGILL signal. This appears to be a macOS
kernel bug. It has been reported to Apple.
This CL partially works around the problem by using execLock to not
send preemption signals during execve. Of course some other stray
signal could occur, but at least we can avoid exacerbating the problem.
We can't simply disable signals, as that would mean that the exec'ed
process would start with all signals blocked, which it likely does not
expect.
For #41702Fixes#41704
Change-Id: I91b0add967b315671ddcf73269c4d30136e579b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/262438
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 64fb6ae95f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/262717
The signature of call16 is currently missing the "typ" parameter. This
CL fixes this. This wasn't caught by vet because call16 is defined by
macro expansion (see #17544), and we didn't notice the mismatch with
the other call* functions because call16 is defined only on 32-bit
architectures and lives alone in stubs32.go.
Unfortunately, this means its GC signature is also wrong: the "arg"
parameter is treated as a scalar rather than a pointer, so GC won't
trace it and stack copying won't adjust it. This turns out to matter
in exactly one case right now: on 32-bit architectures (which are the
only architectures where call16 is defined), a stack-allocated defer
of a function with a 16-byte or smaller argument frame including a
non-empty result area can corrupt memory if the deferred function
grows the stack and is invoked during a panic. Whew. All other current
uses of reflectcall pass a heap-allocated "arg" frame (which happens
to be reachable from other stack roots, so tracing isn't a problem).
Curiously, in 2016, the signatures of all call* functions were wrong
in exactly this way. CL 31654 fixed all of them in stubs.go, but
missed the one in stubs32.go.
Updates #41795.
Fixes#41797.
Change-Id: I31e3c0df201f79ee5707eeb8dc4ff0d13fc10ada
Reviewed-on: https://go-review.googlesource.com/c/go/+/259338
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/259598
//go:notinheap
type T int
type U T
We already correctly propagate the notinheap-ness of T to U. But we
have an assertion in the typechecker that if there's no explicit
//go:notinheap associated with U, then report an error. Get rid of
that error so that implicit propagation is allowed.
Adjust the tests so that we make sure that uses of types like U
do correctly report an error when U is used in a context that might
cause a Go heap allocation.
Update #41432
Change-Id: I1692bc7cceff21ebb3f557f3748812a40887118d
Reviewed-on: https://go-review.googlesource.com/c/go/+/255637
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
(cherry picked from commit 22053790fa)
Reviewed-on: https://go-review.googlesource.com/c/go/+/255697
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
They can't reasonably be allocated on the heap. Not a huge deal, but
it has an interesting and useful side effect.
After CL 249917, the compiler and runtime treat pointers to
go:notinheap types as uintptrs instead of real pointers (no write
barrier, not processed during stack scanning, ...). That feature is
exactly what we want for cgo to fix#40954. All the cases we have of
pointers declared in C, but which might actually be filled with
non-pointer data, are of this form (JNI's jobject heirarch, Darwin's
CFType heirarchy, ...).
Fixes#40954
Change-Id: I44a3b9bc2513d4287107e39d0cbbd0efd46a3aae
Reviewed-on: https://go-review.googlesource.com/c/go/+/250940
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/255321
The alias doesn't need to be marked go:notinheap. It gets its
notinheap-ness from the target type.
Without this change, the type alias test in the notinheap.go file
generates these two errors:
notinheap.go:62: misplaced compiler directive
notinheap.go:63: type nih must be go:notinheap
The first is a result of go:notinheap pragmas not applying
to type alias declarations.
The second is the result of then trying to match the notinheap-ness
of the alias and the target type.
Add a few more go:notinheap tests while we are here.
Update #40954
Change-Id: I067ec47698df6e9e593e080d67796fd05a1d480f
Reviewed-on: https://go-review.googlesource.com/c/go/+/250939
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/255337
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Right now we just prevent such types from being on the heap. This CL
makes it so they cannot appear on the stack either. The distinction
between heap and stack is pretty vague at the language level (e.g. it
is affected by -N), and we don't need the flexibility anyway.
Once go:notinheap types cannot be in either place, we don't need to
consider pointers to such types to be pointers, at least according to
the garbage collector and stack copying. (This is the big win of this
CL, in my opinion.)
The distinction between HasPointers and HasHeapPointer no longer
exists. There is only HasPointers.
This CL is cleanup before possible use of go:notinheap to fix#40954.
Update #13386
Change-Id: Ibd895aadf001c0385078a6d4809c3f374991231a
Reviewed-on: https://go-review.googlesource.com/c/go/+/255320
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The second argument of StorepNoWB must be forced to escape.
The current Go code does not explicitly enforce that property.
By implementing in assembly, and not using go:noescape, we
force the issue.
Test is in CL 249761. Issue #40975.
This CL is needed for CL 249917, which changes how go:notinheap
works and breaks the previous StorepNoWB wasm code.
I checked for other possible errors like this. This is the only
go:notinheap that isn't in the runtime itself.
Included test from CL 249761.
Update #41432
Change-Id: I43400a806662655727c4a3baa8902b63bdc9fa57
Reviewed-on: https://go-review.googlesource.com/c/go/+/249962
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
(cherry picked from commit c0602603b2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/260878
Trust: Keith Randall <khr@golang.org>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The IndexString implementation in the bytealg package requires that
the string passed into it be in the range '2 <= len(s) <= MaxLen'
where MaxLen may be any value (including 0).
CL 156998 added calls to bytealg.IndexString where MaxLen was not
first checked. This led to an illegal instruction on s390x with
the vector facility disabled.
This CL guards the calls to bytealg.IndexString with a MaxLen check.
If the check fails then the code now falls back to the pre CL 156998
implementation (a loop over the runes in the string).
Since the MaxLen check is now in place the generic implementation is
no longer called so I have returned it to its original unimplemented
state.
In future we may want to drop MaxLen to prevent this kind of
confusion.
Fixes#41595.
Change-Id: I81d88cf8c5ae143a8f5f460d18f8269cb6c0f28c
Reviewed-on: https://go-review.googlesource.com/c/go/+/256921
Trust: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
In https://golang.org/cl/221397 we made commands like "go version -v"
error, since both of the command's flags only make sense when arguments
follow them. Without arguments, the command only reports Go's own
version, and the flags are most likely a mistake.
However, the script below is entirely reasonable:
export GOFLAGS=-v # make all Go commands verbose
go version
go build
After the previous CL, "go version" would error. Instead, only error if
the flag was passed explicitly, and not via GOFLAGS.
The patch does mean that we won't error on "GOFLAGS=-v go version -v",
but that very unlikely false negative is okay. The error is only meant
to help the user not misuse the flags, anyway - it's not a critical
error of any sort.
To reuse inGOFLAGS, we move it to the base package and export it there,
since it's where the rest of the GOFLAGS funcs are.
Fixes#41464.
Change-Id: I74003dd25d94bacf9ac507b5cad778fd65233321
Reviewed-on: https://go-review.googlesource.com/c/go/+/254157
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit de0957dc08)
Reviewed-on: https://go-review.googlesource.com/c/go/+/255498
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Currently activeStackChans is set before a goroutine blocks on a channel
operation in an unlockf passed to gopark. The trouble is that the
unlockf is called *after* the G's status is changed, and the G's status
is what is used by a concurrent mark worker (calling suspendG) to
determine that a G has successfully been suspended. In this window
between the status change and unlockf, the mark worker could try to
shrink the G's stack, and in particular observe that activeStackChans is
false. This observation will cause the mark worker to *not* synchronize
with concurrent channel operations when it should, and so updating
pointers in the sudog for the blocked goroutine (which may point to the
goroutine's stack) races with channel operations which may also
manipulate the pointer (read it, dereference it, update it, etc.).
Fix the problem by adding a new atomically-updated flag to the g struct
called parkingOnChan, which is non-zero in the race window above. Then,
in isShrinkStackSafe, check if parkingOnChan is zero. The race is
resolved like so:
* Blocking G sets parkingOnChan, then changes status in gopark.
* Mark worker successfully suspends blocking G.
* If the mark worker observes parkingOnChan is non-zero when checking
isShrinkStackSafe, then it's not safe to shrink (we're in the race
window).
* If the mark worker observes parkingOnChan as zero, then because
the mark worker observed the G status change, it can be sure that
gopark's unlockf completed, and gp.activeStackChans will be correct.
The risk of this change is low, since although it reduces the number of
places that stack shrinking is allowed, the window here is incredibly
small. Essentially, every place that it might crash now is replaced with
no shrink.
This change adds a test, but the race window is so small that it's hard
to trigger without a well-placed sleep in park_m. Also, this change
fixes stackGrowRecursive in proc_test.go to actually allocate a 128-byte
stack frame. It turns out the compiler was destructuring the "pad" field
and only allocating one uint64 on the stack.
For #40641.
Fixes#40643.
Change-Id: I7dfbe7d460f6972b8956116b137bc13bc24464e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/247050
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit eb3c6a93c3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/256300
Reviewed-by: Austin Clements <austin@google.com>
Taking the live variable set from the last return point is problematic.
See #40629 for details, but there may not be a return point, or it may
be before the final defer.
Additionally, keeping track of the last call as a *Value doesn't quite
work. If it is dead-code eliminated, the storage for the Value is reused
for some other random instruction. Its live variable information,
if it is available at all, is wrong.
Instead, just mark all the open-defer argument slots as live
throughout the function. (They are already zero-initialized.)
Fixes#40742
Change-Id: Ie456c7db3082d0de57eaa5234a0f32525a1cce13
Reviewed-on: https://go-review.googlesource.com/c/go/+/247522
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
(cherry picked from commit 32a84c99e1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/248621
Trust: Dmitri Shuralyov <dmitshur@golang.org>
While debugging #40771, I realized that the chatty printer should only
ever print to a single io.Writer (normally os.Stdout). The other
Writer implementations in the chain write to local buffers, but if we
wrote a test's output to a local buffer, then we did *not* write it to
stdout and we should not store it as the most recently logged test.
Because the chatty printer should only ever print to one place, it
shouldn't receive an io.Writer as an argument — rather, it shouldn't
be used at all for destinations other than the main output stream.
On the other hand, when we flush the output buffer to stdout in the
top-level flushToParent call, it is important that we not allow some
other test's output to intrude between the test summary header and the
remainder of the test's output. cmd/test2json doesn't know how to
parse such an intrusion, and it's confusing to humans too.
No test because I couldn't reproduce the user-reported error without
modifying the testing package. (This behavior seems to be very
sensitive to output size and/or goroutine scheduling.)
Fixes#40881
Updates #40771
Updates #38458
Change-Id: Ic19bf1d535672b096ba1c8583a3b74aab6d6d766
Reviewed-on: https://go-review.googlesource.com/c/go/+/249026
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 51c0bdc6d1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/252637
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
The 387 port needs to load a floating-point control word from a
global location to implement float32 arithmetic.
When compiling with -pie, loading that control word clobbers an
integer register. If that register had something important in it, boom.
Fix by using LEAL to materialize the address of the global location
first. LEAL with -pie works because the destination register is
used as the scratch register.
387 support is about to go away (#40255), so this will need to be
backported to have any effect.
No test. I have one, but it requires building with -pie, which
requires cgo. Our testing infrastructure doesn't make that easy.
Not worth it for a port which is about to vanish.
Fixes#41620
Change-Id: I140f9fc8fdce4e74a52c2c046e2bd30ae476d295
Reviewed-on: https://go-review.googlesource.com/c/go/+/257277
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
(cherry picked from commit ea106cc07a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/257207
The 1.15 compiler is not quite smart enough to see that the byte slice
passed to ignoringEINTR does not escape. This ripples back up to user
code which would see a byte slice passed to os.(*File).Write escape,
which did not happen in 1.14.
Rather than backport some moderately complex compiler fixes, rewrite
the code slightly so that the 1.15 compiler is able to see that the
slice does not escape.
This is not a backport from tip, where the code is already different.
The test for this will be on tip, where we will most likely change the
compiler to understand this kind of code.
Fixes#41543
For #41474
Change-Id: I6c78164229fea7794e7edba512bfd7034a0b91c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/256418
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Both ReadMemStatsSlow and CheckScavengedBits iterate over the page
allocator's chunks but don't actually check if they exist. During the
development process the chunks index became sparse, so now this was a
possibility. If the runtime tests' heap is sparse we might end up
segfaulting in either one of these functions, though this will generally
be very rare.
The pattern here to return nil for a nonexistent chunk is also useful
elsewhere, so this change introduces tryChunkOf which won't throw, but
might return nil. It also updates the documentation of chunkOf.
For #41296.
Fixes#41317.
Change-Id: Id5ae0ca3234480de1724fdf2e3677eeedcf76fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/253777
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 34835df048)
Reviewed-on: https://go-review.googlesource.com/c/go/+/253917
The optimization that replaces inline markers with pre-existing
instructions assumes that 'Prog' values produced by the compiler are
still reachable after the assembler has run. This was not true on
s390x where the assembler was removing NOP instructions from the
linked list of 'Prog' values. This led to broken inlining data
which in turn caused an infinite loop in the runtime traceback code.
Fix this by stopping the s390x assembler backend removing NOP
values. It does not make any difference to the output of the
assembler because NOP instructions are 0 bytes long anyway.
Note: compiler check omitted from backport to reduce risk of change.
Fixes#40693.
Change-Id: I4eb570de13165cde342d5fb2ee3218945ddf4b52
Reviewed-on: https://go-review.googlesource.com/c/go/+/248478
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When external linking, for large binaries, the external linker
may insert a trampoline for the write barrier call, which looks
0000000005a98cc8 <__long_branch_runtime.gcWriteBarrier>:
5a98cc8: 86 01 82 3d addis r12,r2,390
5a98ccc: d8 bd 8c e9 ld r12,-16936(r12)
5a98cd0: a6 03 89 7d mtctr r12
5a98cd4: 20 04 80 4e bctr
It clobbers R12 (and CTR, which is never live across a call).
As at compile time we don't know whether the binary is big and
what link mode will be used, I think we need to mark R12 as
clobbered for write barrier call. For extra safety (future-proof)
we mark caller-saved register that cannot be used for function
arguments, which includes R11, as potentially clobbered as well.
Updates #40851.
Fixes#40868.
Change-Id: Iedd901c5072f1127cc59b0a48cfeb4aaec81b519
Reviewed-on: https://go-review.googlesource.com/c/go/+/248917
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit b58d297416)
Reviewed-on: https://go-review.googlesource.com/c/go/+/249019
Previously, the assembler removed NOPs from the Prog list in
obj9.go. NOPs shouldn't be removed if they were added as
an inline mark, as described in the issue below.
Fixes#40767
Once the NOPs were left in the Prog list, some instructions
were flagged as invalid because they had an operand which was
not represented in optab. In order to preserve the previous
assembler behavior, entries were added to optab for those
operand cases. They were not flagged as errors before because
the NOP instructions were removed before the code to check the
valid opcode/operand combinations.
Change-Id: Iae5145f94459027cf458e914d7c5d6089807ccf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/247842
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Paul Murphy <murp@ibm.com>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 7d7bd5abc7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/248381
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
If we are parsing a test output, and the test does not end in the
usual PASS or FAIL line (say, because it panicked), then we need the
exit status of the test binary in order to determine whether the test
passed or failed. If we don't have that status available, we shouldn't
guess arbitrarily — instead, we should omit the final "pass" or "fail"
action entirely.
(In practice, we nearly always DO have the final status, such as when
running 'go test' or 'go tool test2json some.exe'.)
Updates #40132Fixes#40805
Change-Id: Iae482577361a6033395fe4a05d746b980e18c3de
Reviewed-on: https://go-review.googlesource.com/c/go/+/248624
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
(cherry picked from commit 1b86bdbdc3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/248725
This CL ensures that responses served via CGI and FastCGI
have a Content-Type header based on the content of the
response if not explicitly set by handlers.
If the implementers of the handler did not explicitly
specify a Content-Type both CGI implementations would default
to "text/html", potentially causing cross-site scripting.
Thanks to RedTeam Pentesting GmbH for reporting this.
Fixes CVE-2020-24553
Change-Id: I82cfc396309b5ab2e8d6e9a87eda8ea7e3799473
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/823217
Reviewed-by: Russ Cox <rsc@google.com>
(cherry picked from commit 23d675d07fdc56aafd67c0a0b63d5b7e14708ff0)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/835311
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The service handler needs to handle CTRL+C-like events -- including
those sent by the service manager itself -- using the default Windows
implementation if no signal handler from Go is already listening to
those events. Ordinarily, the signal handler would call exit(2), but we
actually need to allow this to be passed onward to the service handler.
So, we detect if we're in a service and skip calling exit(2) in that
case, just like we do for shared libraries.
Updates #40167.
Updates #40074.
Fixes#40412.
Change-Id: Ia77871737a80e1e94f85b02d26af1fd2f646af96
Reviewed-on: https://go-review.googlesource.com/c/go/+/244959
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
checkptr has code to recognize &^ expressions, but it didn't take into
account that "p &^ x" gets rewritten to "p & ^x" during walk, which
resulted in false positive diagnostics.
This CL changes walkexpr to mark OANDNOT expressions with Implicit
when they're rewritten to OAND, so that walkCheckPtrArithmetic can
still recognize them later.
It would be slightly more idiomatic to instead mark the OBITNOT
expression as Implicit (as it's a compiler-generated Node), but the
OBITNOT expression might get constant folded. It's not worth the extra
complexity/subtlety of relying on n.Right.Orig, so we set Implicit on
the OAND node instead.
To atone for this transgression, I add documentation for nodeImplicit.
Updates #40917.
Fixes#40934.
Change-Id: I386304171ad299c530e151e5924f179e9a5fd5b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/249477
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
(cherry picked from commit e94544cf01)
Reviewed-on: https://go-review.googlesource.com/c/go/+/249879
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
ba9e108899 cmd: update golang.org/x/xerrors
027d7241ce encoding/binary: read at most MaxVarintLen64 bytes in ReadUvarint
6f08e89ec3 cmd/go: fix error stacks when there are scanner errors
f235275097 net/http: fix cancelation of requests with a readTrackingBody wrapper
f92337422e runtime/race: fix ppc64le build
e49b2308a5 runtime/race: rebuild some .syso files to remove getauxval dependency
10523c0efb doc/go1.15: fix a few trivial inconsistencies
7388956b76 cmd/cgo: fix mangling of enum and union types
b56791cdea runtime: validate candidate searchAddr in pageAlloc.find
10374e2435 testing: fix quotation marks
7f86080476 cmd/compile: don't addLocalInductiveFacts if there is no direct edge from if block to phi block
54e75e8f9d crypto/ed25519: remove s390x KDSA implementation
6b4dcf19fa runtime: hold sched.lock over globrunqputbatch in runqputbatch
85afa2eb19 runtime: ensure startm new M is consistently visible to checkdead
c4fed25553 cmd/compile: add floating point load+op operations to addressing modes pass
19a932ceb8 cmd/link: don't mark shared library symbols reachable unconditionally
8696ae82c9 syscall: use correct file descriptor in dup2 fallback path
9591515f51 runtime, sync: add copyright headers to new files
074f2d800f doc/go1.15: surface the crypto/x509 CommonName deprecation note
Change-Id: I0bfcff1fc2de723960909d9dda718fee6abc2912
RSA key types have a finalizer that will free the underlying C value
when the Go one is garbage collected. It's important that the finalizer
doesn't run while a cgo call is using the underlying C value, so they
require runtime.KeepAlive calls after each use.
This is error prone, so replace it with a closure that provides access
to the underlying C value and then automatically calls KeepAlive.
AES, HMAC, and ECDSA also need KeepAlives, but they have much fewer call
sites, so avoid the complexity for now.
Change-Id: I6d6f38297cd1cf384a1639974d9739a939cbdbcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/221822
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Use OPENSSL_malloc for set0 functions as OPENSSL_free now catches us
using the libc malloc and aborts.
While at it, move the runtime.KeepAlive to the location of the key use.
Fixes#30158
Change-Id: I968a98d8974ca5f220e822841beb6c34290eefe9
Reviewed-on: https://go-review.googlesource.com/c/go/+/218000
Reviewed-by: Katie Hockman <katie@golang.org>
Move the import in cipher_suites.go up where it's less likely to ever
conflict again, and remove the equivalent import from common.go, again
to reduce the likeliness of future conflicts.
Change-Id: Ib05daba7ba6ce81f665a44185b53a6e083f7c693
Updated TestBoringServerSignatureAndHash to expect RSA-PSS to work with
TLS 1.2, and hence with FIPS mode.
Change-Id: I358271b2e4804733cf61dc132fa0c5f39c2bff19
Signing-side signature algorithm selection moved to
selectSignatureScheme, so add FIPS logic there.
Change-Id: I827e7296d01ecfd36072e2139e74603ef42c6b24
This step was added in CL 188738 to work around the issue
golang.org/issue/33443. That issue has now been resolved,
so this step is no longer needed and can be removed.
Updates #33443
Change-Id: I0c9257ab61d53f3a47556882f7dfc8fc119be849
Reviewed-on: https://go-review.googlesource.com/c/go/+/189942
Reviewed-by: Filippo Valsorda <filippo@golang.org>
The inliner seems to have gotten a bit too smart in 1.12 and it made
sha1.boringNewSHA1 disappear. Replace it with the proper
crypto/internal/boring/sig.BoringCrypto signature. Also, switch the
negative signature to sha256.(*digest), since SHA-256 is used for sure
by cmd/go. Not using crypto/internal/boring/sig.StandardCrypto just to
be safe, in case the crypto/internal/boring/sig mechanism breaks.
Also, had to fight #30833 and #30515 to get
golang.org/x/build/cmd/release to build in modules mode.
Change-Id: I46f1471582fd77daae47d00baab975109902052d
Reviewed-on: https://go-review.googlesource.com/c/go/+/169517
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Also, make the docker build script pull the latest base image so we are
not bundling an outdated system.
Change-Id: I6c8ee8ba89101232d635fc2e58f4cfc818d139ba
Reviewed-on: https://go-review.googlesource.com/c/152920
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
The bootstrap stage 1 compiler was defaulting to the language version
used by the bootstrap compiler itself, typically 1.4. Normally this
doesn't matter since the bootstrap code has to build with 1.4 anyhow,
but it broke the boringcrypto branch which uses cgo during the
bootstrap, as cgo now generates code that uses type aliases.
Change-Id: I8a8312bb9ca4befaf65c00a8d71a78566075c2f7
Reviewed-on: https://go-review.googlesource.com/c/149459
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 69397422c0)
Reviewed-on: https://go-review.googlesource.com/c/149485
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Merge at CL 144340, in order to cherry-pick CL 149459 next to it, which
fixes a BoringCrypto specific breakage in the toolchain.
Change-Id: I30aeac344bbff279449e27876dc8f9c406e55e43
Also, document the fact that we cut releases only from the versioned
branches, and use the correct x/net branch.
Had to build this passing -skip_tests to release because the buildlet
was timing out (see below), but the builders on the dashboard are green.
2018/09/28 19:14:50 linux-amd64: Start.
2018/09/28 19:14:50 linux-amd64: Creating buildlet.
2018/09/28 19:15:28 linux-amd64: Pushing source to buildlet.
2018/09/28 19:15:37 linux-amd64: Writing VERSION file.
2018/09/28 19:15:38 linux-amd64: Cleaning goroot (pre-build).
2018/09/28 19:15:38 linux-amd64: Building.
2018/09/28 19:46:20 Buildlet https://farmer.golang.org:443 failed three heartbeats; final error: timeout waiting for headers
2018/09/28 19:46:20 linux-amd64: Error: Buildlet https://farmer.golang.org:443 failed heartbeat after 10.007631241s; marking dead; err=timeout waiting for headers
Change-Id: I9d982df693075f96d44aa6f163533253c8ae2914
Reviewed-on: https://go-review.googlesource.com/138555
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Filippo Valsorda <filippo@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
2093 changed files with 115432 additions and 104743 deletions
@@ -666,13 +660,16 @@ For example, you should not set <code>$GOHOSTARCH</code> to
<code>arm</code> on an x86 system.
</p>
<li><code>$GO386</code> (for <code>386</code> only, defaults to <code>sse2</code>)
<li><code>$GO386</code> (for <code>386</code> only, default is auto-detected
if built on either <code>386</code> or <code>amd64</code>, <code>387</code> otherwise)
<p>
This variable controls how gc implements floatingpoint computations.
This controls the code generated by gc to use either the 387 floating-point unit
(set to <code>387</code>) or SSE2 instructions (set to <code>sse2</code>) for
floating point computations.
</p>
<ul>
<li><code>GO386=softfloat</code>: use software floating point operations; should support all x86 chips (Pentium MMX or later).</li>
<li><code>GO386=sse2</code>: use SSE2 for floating point operations; has better performance but only available on Pentium 4/Opteron/Athlon 64 or later.</li>
<li><code>GO386=387</code>: use x87 for floating point operations; should support all x86 chips (Pentium MMX or later).</li>
<li><code>GO386=sse2</code>: use SSE2 for floating point operations; has better performance than 387, but only available on Pentium 4/Opteron/Athlon 64 or later.</li>
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.
