This CL ensures that ReadUvarint consumes only a limited
amount of input (instead of an unbounded amount).
On some inputs, ReadUvarint could read an arbitrary number
of bytes before deciding to return an overflow error.
After this CL, ReadUvarint returns that same overflow
error sooner, after reading at most MaxVarintLen64 bytes.
Fix authored by Robert Griesemer and Filippo Valsorda.
Thanks to Diederik Loerakker, Jonny Rhea, Raúl Kripalani,
and Preston Van Loon for reporting this.
Fixes CVE-2020-16845
Change-Id: Ie0cb15972f14c38b7cf7af84c45c4ce54909bb8f
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/812099
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/812324
Manually backported the subject CLs, because of lack of
Gerrit "forge-author" permissions, but also because the prior
cherry picks didn't apply cleanly, due to a tight relation chain.
The backport comprises of:
* CL 174122
* CL 216197
* CL 223963
* CL 216240
* CL 216241
Note:
Due to the restrictions that we cannot retroactively
introduce API changes to Go1.13.13 that weren't in Go1.13, the Conn.Validator
interface (from CL 174122, CL 223963) isn't exposed, and drivers will just be
inspected, for if they have an IsValid() bool method implemented.
For a description of the content of each CL:
* CL 174122:
database/sql: process all Session Resets synchronously
Adds a new interface, driver.ConnectionValidator, to allow
drivers to signal they should not be used again,
separatly from the session resetter interface.
This is done now that the session reset is done
after the connection is put into the connection pool.
Previous behavior attempted to run Session Resets
in a background worker. This implementation had two
problems: untested performance gains for additional
complexity, and failures when the pool size
exceeded the connection reset channel buffer size.
* CL 216197:
database/sql: check conn expiry when returning to pool, not when handing it out
With the original connection reuse strategy, it was possible that
when a new connection was requested, the pool would wait for an
an existing connection to return for re-use in a full connection
pool, and then it would check if the returned connection was expired.
If the returned connection expired while awaiting re-use, it would
return an error to the location requestiong the new connection.
The existing call sites requesting a new connection was often the last
attempt at returning a connection for a query. This would then
result in a failed query.
This change ensures that we perform the expiry check right
before a connection is inserted back in to the connection pool
for while requesting a new connection. If requesting a new connection
it will no longer fail due to the connection expiring.
* CL 216240:
database/sql: prevent Tx statement from committing after rollback
It was possible for a Tx that was aborted for rollback
asynchronously to execute a query after the rollback had completed
on the database, which often would auto commit the query outside
of the transaction.
By W-locking the tx.closemu prior to issuing the rollback
connection it ensures any Tx query either fails or finishes
on the Tx, and never after the Tx has rolled back.
* CL 216241:
database/sql: on Tx rollback, retain connection if driver can reset session
Previously the Tx would drop the connection after rolling back from
a context cancel. Now if the driver can reset the session,
keep the connection.
* CL 223963
database/sql: add test for Conn.Validator interface
This addresses comments made by Russ after
https://golang.org/cl/174122 was merged. It addes a test
for the connection validator and renames the interface to just
"Validator".
Updates #31480
Updates #32530
Updates #32942
Updates #34775Fixes#40205
Change-Id: I6d7307180b0db0bf159130d91161764cf0f18b58
Reviewed-on: https://go-review.googlesource.com/c/go/+/242522
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
When using the platform verifier on Windows (because Roots is nil) we
were always enforcing server auth EKUs if DNSName was set, and none
otherwise. If an application was setting KeyUsages, they were not being
respected.
Started correctly surfacing IncompatibleUsage errors from the system
verifier, as those are the ones applications will see if they are
affected by this change.
Also refactored verify_test.go to make it easier to add tests for this,
and replaced the EKULeaf chain with a new one that doesn't have a SHA-1
signature.
Thanks to Niall Newman for reporting this.
Fixes#39360
Fixes CVE-2020-14039
Change-Id: If5c00d615f2944f7d57007891aae1307f9571c32
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/774414
Reviewed-by: Katie Hockman <katiehockman@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/793509
Reviewed-by: Filippo Valsorda <valsorda@google.com>
The expectContinueReader writes to the connection on the first
Request.Body read. Since a Handler might be doing a read in parallel or
before a write, expectContinueReader needs to synchronize with the
ResponseWriter, and abort if a response already went out.
The tests will land in a separate CL.
Fixes#34902
Fixes CVE-2020-15586
Change-Id: Icdd8dd539f45e8863762bd378194bb4741e875fc
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/793350
Reviewed-by: Filippo Valsorda <valsorda@google.com>
(cherry picked from commit b5e504f4a07c572744b228fa1b10e3989c4c44f3)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/793499
This teaches vet to recognize the new formats %#b and %O on integers,
%x on float/complex. They have been supported since the 1.13 release,
but vet would warn about it.
Also update cmd/vet testdata accordingly, as was done in CL 202083.
Fixes#39287.
For #29986.
Change-Id: Ia7817ee60ae6beac32cc402c0c68afa917e4ef0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/238018
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Support for linux/386 was added to Delve in version 1.4.1, but the
version of Delve currently installed on the linux-386-longtest
builder is 1.2.0. That isn't new enough, which causes the test
to fail. Skip it on that builder until it can be made to work.
The only reason it used to pass on the linux-386-longtest builder
before is because that builder was misconfigured to run tests for
linux/amd64. This was resolved in CL 234520.
Also improve internal documentation and the text of skip reasons.
For #39309.
Fixes#39561.
Change-Id: I395cb1f076e59dd3a3feb53e1dcdce5101e9a0f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/237617
Reviewed-by: David Chase <drchase@google.com>
A Rat is represented via a quotient a/b where a and b are Int values.
To make it possible to use an uninitialized Rat value (with a and b
uninitialized and thus == 0), the implementation treats a 0 denominator
as 1.
Rat.Num and Rat.Denom return pointers to these values a and b. Because
b may be 0, Rat.Denom used to first initialize it to 1 and thus produce
an undesirable side-effect (by changing the Rat's denominator).
This CL changes Denom to return a new (not shared) *Int with value 1
in the rare case where the Rat was not initialized. This eliminates
the side effect and returns the correct denominator value.
While this is changing behavior of the API, the impact should now be
minor because together with (prior) CL https://golang.org/cl/202997,
which initializes Rats ASAP, Denom is unlikely used to access the
denominator of an uninitialized (and thus 0) Rat. Any operation that
will somehow set a Rat value will ensure that the denominator is not 0.
Fixes#36689.
For #33792.
For #3521.
Change-Id: I0bf15ac60513cf52162bfb62440817ba36f0c3fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/203059
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/233323
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
A Rat is represented via a quotient a/b where a and b are Int values.
To make it possible to use an uninitialized Rat value (with a and b
uninitialized and thus == 0), the implementation treats a 0 denominator
as 1.
For each operation we check if the denominator is 0, and then treat
it as 1 (if necessary). Operations that create a new Rat result,
normalize that value such that a result denominator 1 is represened
as 0 again.
This CL changes this behavior slightly: 0 denominators are still
interpreted as 1, but whenever we (safely) can, we set an uninitialized
0 denominator to 1. This simplifies the code overall.
Also: Improved some doc strings.
Preparation for addressing issue #33792.
For #36689.
For #33792.
Change-Id: I3040587c8d0dad2e840022f96ca027d8470878a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/202997
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/233322
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
When a locked M wants to start a new M, it hands off to the template
thread to actually call clone and start the thread. The template thread
is lazily created the first time a thread is locked (or if cgo is in
use).
stoplockedm will release the P (_Pidle), then call handoffp to give the
P to another M. In the case of a pending STW, one of two things can
happen:
1. handoffp starts an M, which does acquirep followed by schedule, which
will finally enter _Pgcstop.
2. handoffp immediately enters _Pgcstop. This only occurs if the P has
no local work, GC work, and no spinning M is required.
If handoffp starts an M, and must create a new M to do so, then newm
will simply queue the M on newmHandoff for the template thread to do the
clone.
When a stop-the-world is required, stopTheWorldWithSema will start the
stop and then wait for all Ps to enter _Pgcstop. If the template thread
is not fully created because startTemplateThread gets stopped, then
another stoplockedm may queue an M that will never get created, and the
handoff P will never leave _Pidle. Thus stopTheWorldWithSema will wait
forever.
A sequence to trigger this hang when STW occurs can be visualized with
two threads:
T1 T2
------------------------------- -----------------------------
LockOSThread LockOSThread
haveTemplateThread == 0
startTemplateThread
haveTemplateThread = 1
newm haveTemplateThread == 1
preempt -> schedule g.m.lockedExt++
gcstopm -> _Pgcstop g.m.lockedg = ...
park g.lockedm = ...
return
... (any code)
preempt -> schedule
stoplockedm
releasep -> _Pidle
handoffp
startm (first 3 handoffp cases)
newm
g.m.lockedExt != 0
Add to newmHandoff, return
park
Note that the P in T2 is stuck sitting in _Pidle. Since the template
thread isn't running, the new M will not be started complete the
transition to _Pgcstop.
To resolve this, we disable preemption around the assignment of
haveTemplateThread and the creation of the template thread in order to
guarantee that if handTemplateThread is set then the template thread
will eventually exist, in the presence of stops.
For #38931Fixes#38932
Change-Id: I50535fbbe2f328f47b18e24d9030136719274191
Reviewed-on: https://go-review.googlesource.com/c/go/+/232978
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 11b3730a02)
Reviewed-on: https://go-review.googlesource.com/c/go/+/234888
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When deallocating the input register to a phi so that the phi
itself could be allocated to that register the code was also
deallocating all copies of that phi input value. Those copies
of the value could still be live and if they were the register
allocator could reuse them incorrectly to hold speculative
copies of other phi inputs. This causes strange bugs.
No test because this is a very obscure scenario that is hard
to replicate but CL 228060 adds an assertion to the compiler
that does trigger when running the std tests on linux/s390x
without this CL applied. Hopefully that assertion will prevent
future regressions.
Fixes#38442.
Change-Id: Id975dadedd731c7bb21933b9ea6b17daaa5c9e1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/228061
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 382fe3e249)
Reviewed-on: https://go-review.googlesource.com/c/go/+/230358
materializeGCProg allocates a temporary buffer for unrolling a GC
program. Unfortunately, when computing the size of the buffer, it
rounds *down* the number of bytes needed to store bitmap before
rounding up the number of pages needed to store those bytes. The fact
that it rounds up to pages usually mitigates the rounding down, but
the type from #37470 exists right on the boundary where this doesn't
work:
type Sequencer struct {
htable [1 << 17]uint32
buf []byte
}
On 64-bit, this GC bitmap is exactly 8 KiB of zeros, followed by three
one bits. Hence, this needs 8193 bytes of storage, but the current
math in materializeGCProg rounds *down* the three one bits to 8192
bytes. Since this is exactly pageSize, the next step of rounding up to
the page size doesn't mitigate this error, and materializeGCProg
allocates a buffer that is one byte too small. runGCProg then writes
one byte past the end of this buffer, causing either a segfault (if
you're lucky!) or memory corruption.
Updates #37470.
Fixes#37483.
Change-Id: Iad24c463c501cd9b1dc1924bc2ad007991a094a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/224418
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, we extract module zip files to temporary directories, then
atomically rename them into place. On Windows, this can fail with
ERROR_ACCESS_DENIED if another process (antivirus) has files open
before the rename. In CL 220978, we repeated the rename operation in a
loop over 500 ms, but this didn't solve the problem for everyone.
A better solution will extract module zip files to their permanent
locations in the cache and will keep a ".partial" marker file,
indicating when a module hasn't been fully extracted (CL 221157).
This approach is not safe if current versions of Go access the module
cache concurrently, since the module directory is detected with a
single os.Stat.
In the interim, this CL makes two changes:
1. Flaky file system operations are repeated over 2000 ms to reduce
the chance of this error occurring.
2. cmd/go will now check for .partial files created by future
versions. If a .partial file is found, it will lock the lock file,
then remove the .partial file and directory if needed.
After some time has passed and Go versions lacking this CL are no
longer supported, we can start extracting module zip files in place.
Updates #37802
Change-Id: I467ee11aa59a90b63cf0e3e761c4fec89d57d3b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/221820
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 093049b370)
Reviewed-on: https://go-review.googlesource.com/c/go/+/223146
These non-short Windows test failures were resolved fully in CL 206144.
Both TestScript/build_trimpath and TestScript/version tests can be fixed
by backporting the changes to test scripts only, so that is done here.
Fixing TestScript/mod_list_dir requires backporting non-test changes in
addition to the test script changes, which is unlikely to be appropriate
this late in Go 1.13 release cycle. A failing test can cover up other
regressions, so skip this known failing test to fix the builder.
For #36181.
Change-Id: I4f140bd373554eb4664f04638666dee77986ec3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/223782
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This test was fixed by changing cmd/doc behavior in CL 204442.
Backporting that non-test code change is unlikely to be appropriate
this late in Go 1.13 release cycle. A failing test can cover up other
regressions, so skip this known failing test to fix the builder.
For #35236.
For #36181.
Change-Id: I07e795e75d7e37bc96ab68607d5d5cc9254342f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/223780
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Builds upon the changes from #32000 which supported sourcing environment
variables for a new process from the environment of a Windows user token
when supplied.
But due to the logic of os/exec, the Env field of a process was
always non-nil when it reached that change.
This change moves the logic up to os/exec, specifically when
os.ProcAttr is being built for the os.StartProcess call, this
ensures that if a user token has been supplied and no Env slice has
been provided on the command it will be sourced from the user's
environment.
If no token is provided, or the program is compiled for any other
platform than Windows, the default environment will be sourced from
syscall.Environ().
For #35314Fixes#37433
Change-Id: I4c1722e90b91945eb6980d5c5928183269b50487
GitHub-Last-Rev: 32216b7291
GitHub-Pull-Request: golang/go#37402
Reviewed-on: https://go-review.googlesource.com/c/go/+/220587
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/226280
Appending to a global slice is only safe if its length is already
equal to its capacity. That property is not guaranteed for slices in
general, and empirically does not hold for this one.
This is a minimal fix to make it easier to backport.
A more robust cleanup of the base.EnvForDir function will be sent in a
subsequent CL.
Fixes#38082
Updates #38077
Change-Id: I731d5bbd0e516642c2cf43e713eeea15402604e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/225577
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
(cherry picked from commit bfb1342a40)
Reviewed-on: https://go-review.googlesource.com/c/go/+/225660
It appears that PowerRegisterSuspendResumeNotification is not supported
when running inside Docker - see issues #35447, #36557 and #37149.
Our current code relies on error number to determine Docker environment.
But we already saw PowerRegisterSuspendResumeNotification return
ERROR_FILE_NOT_FOUND, ERROR_INVALID_PARAMETERS and ERROR_ACCESS_DENIED
(see issues above). So this approach is not sustainable.
Just ignore PowerRegisterSuspendResumeNotification returned error.
For #37149Fixes#37230
Change-Id: I2beba9d45cdb8c1efac5e974e747827a6261915a
Reviewed-on: https://go-review.googlesource.com/c/go/+/219657
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
(cherry picked from commit d467f3bbc9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/224585
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This test's existence was predicated upon assumptions about the full
range of known data types and known data into those types. However,
we've learned from Microsoft that there are several undocumented secret
registry types that are in use by various parts of Windows, and we've
learned from inspection that many Microsoft uses of registry types don't
strictly adhere to the recommended value size. It's therefore foolhardy
to make any assumptions about what goes in and out of the registry, and
so this test, as well as its "blacklist", are meaningless.
For #35084.
Fixes#37826.
Change-Id: I6c3fe5fb0e740e88858321b3b042c0ff1a23284e
Reviewed-on: https://go-review.googlesource.com/c/go/+/203604
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
(cherry picked from commit 0d3092ffa7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/223237
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Old trace viewer stopped working with Chrome M80+ because the
old trace viewer heavily depended on WebComponents V0 which are deprecated.
Trace viewer recently migrated to use WebComponents V0 polyfill
(crbug.com/1036492). This CL brings in the newly updated trace_viewer_full.html
(sync'd @ 9508452e)
and updates the javascript snippet included in the /trace endpoint
to use the polyfill.
This brings in webcomponents.min.js copied from
https://chromium.googlesource.com/catapult/+/9508452e18f130c98499cb4c4f1e1efaedee8962/third_party/polymer/components/webcomponentsjs/webcomponents.min.js
That is necessary because the /trace endpoint needs to import
the vulcanized trace_viewer_full.html.
It's possible that some features are not working correctly with
this polyfill. In that case, report the issue to crbug.com/1036492.
There will be a warning message in the UI (yellow banner above the timeline)
which can be hidden by clicking the 'hide' button.
This allows to render the trace in browsers other than chrome in theory,
but I observed some buttons and functions still don't work outside
chrome.
Updates #34374.
Fixes#37342.
Change-Id: Ib575f756f5e6b22ad904ede6e4d224a995ebe259
Reviewed-on: https://go-review.googlesource.com/c/go/+/219997
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit 75ea964b3f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/220321
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Regression tests for #24161 use a macro to conditionally compile some
stub definitions. The macro tests that the minimum macOS version is
less than 10.12.
We get duplicate definitions when building this test with
CGO_CFLAGS=-mmacosx-version-min=10.x where 10.x < 10.12. With this
change, we use a different macro, __MAC_OS_X_VERSION_MAX_ALLOWED__,
which tests the SDK version instead of the minimum macOS version. This
checks whether these definitions are present in headers.
After this change, 'go tool dist test cgo_test' should pass with
CGO_FLAGS=-mmacosx-version-min=10.10.
Updates #36846
Updates #35459
Change-Id: I88d63601c94b0369c73c38d216a2d41ba7d4e579
Reviewed-on: https://go-review.googlesource.com/c/go/+/216243
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 1f9f88b95e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/217059
Run-TryBot: Carlos Amedee <carlos@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Running the dwarf tests with CGO_CFLAGS set
with certain values would cause the test to fail. all.bash
would fail when CGO_CFLAGS was set to '-mmacosx-version-min=10.10'
because the --macosx-version-min flag is incompatible with some dwarf
tests. The change guards against using an unintended flag in the unit test.
Updates #36846
Updates #35459
Change-Id: Idc9b354aba44fdab424cb0081a4b3ea7a6d0f8e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/216177
Run-TryBot: Carlos Amedee <carlos@golang.org>
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 e948d2b73e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/217057
Running the 'TestBuildForTvOS' test with CGO_CFLAGS set
with certain values would cause the test to fail. all.bash
would fail when CGO_CFLAGS was set to '-mmacosx-version-min=10.10'
because the --macosx-version-min flag is incompatible with tvOS.
The change guards against using an unintended flag in the unit test.
Updates #36846
Updated #35459
Change-Id: Ifc43f3ebfb23d37aabeaac2ea9efae5b877991bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/215957
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit ace25f82df)
Reviewed-on: https://go-review.googlesource.com/c/go/+/218598
cryptobyte: fix panic due to malformed ASN.1 inputs on 32-bit archs
When int is 32 bits wide (on 32-bit architectures like 386 and arm), an
overflow could occur, causing a panic, due to malformed ASN.1 being
passed to any of the ASN1 methods of String.
Tested on linux/386 and darwin/amd64.
This fixes CVE-2020-7919 and was found thanks to the Project Wycheproof
test vectors.
Change-Id: I8c9696a8bfad1b40ec877cd740dba3467d66ab54
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/645211
Reviewed-by: Katie Hockman <katiehockman@google.com>
Reviewed-by: Adam Langley <agl@google.com>
x/crypto/cryptobyte is used in crypto/x509 for parsing certificates.
Malformed certificates might cause a panic during parsing on 32-bit
architectures (like arm and 386).
Change-Id: I840feb54eba880dbb96780ef7adcade073c4c4e3
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/647741
Reviewed-by: Katie Hockman <katiehockman@google.com>
If a pointer write is not atomic, if the GC is running
concurrently, it may observe a partially updated pointer, which
may point to unallocated or already dead memory. Most pointer
writes, like the store instructions generated by the compiler,
are already atomic. But we still need to be careful in places
like memmove. In memmove, we don't know which bits are pointers
(or too expensive to query), so we ensure that all aligned
pointer-sized units are written atomically.
Fixes#36361.
Updates #36101.
Change-Id: I1b3ca24c6b1ac8a8aaf9ee470115e9a89ec1b00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/212626
Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit ffbc02761a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/213683
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Systems where PowerRegisterSuspendResumeNotification returns ERROR_
FILE_NOT_FOUND are also systems where nanotime() is on "program time"
rather than "real time". The chain for this is:
powrprof.dll!PowerRegisterSuspendResumeNotification ->
umpdc.dll!PdcPortOpen ->
ntdll.dll!ZwAlpcConnectPort("\\PdcPort") ->
syscall -> ntoskrnl.exe!AlpcpConnectPort
Opening \\.\PdcPort fails with STATUS_OBJECT_NAME_NOT_FOUND when pdc.sys
hasn't been initialized. Pdc.sys also provides the various hooks for
sleep resumption events, which means if it's not loaded, then our "real
time" timer is actually on "program time". Finally STATUS_OBJECT_NAME_
NOT_FOUND is passed through RtlNtStatusToDosError, which returns ERROR_
FILE_NOT_FOUND. Therefore, in the case where the function returns ERROR_
FILE_NOT_FOUND, we don't mind, since the timer we're using will
correspond fine with the lack of sleep resumption notifications. This
applies, for example, to Docker users.
Updates #35447
Updates #35482Fixes#35746
Change-Id: I9e1ce5bbc54b9da55ff7a3918b5da28112647eee
Reviewed-on: https://go-review.googlesource.com/c/go/+/211280
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Not all closed issues in a given minor milestone are included in that
release, only the ones that have been labeled as CherryPickApproved are.
Update the links to the GitHub issue tracker to include a filter on the
CherryPickApproved label, so that the default view shows only the
backports that were included in a given release. This should more useful
to most people than seeing all backports (considered and approved).
Do this only for Go 1.9.1 and newer releases, as that is when we started
using the CherryPickCandidate and CherryPickApproved labels.
Updates #35988Fixes#36003
Change-Id: I51e07c1bc3ab9c4a5744e8f668c5470adf78bffe
Reviewed-on: https://go-review.googlesource.com/c/go/+/210117
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If a compilation has multiple text sections, code in
textOff must compare the offset argument against the range
for each text section to determine which one it is in.
The comparison looks like this:
if uintptr(off) >= sectaddr && uintptr(off) <= sectaddr+sectlen
If the off value being compared is equal to sectaddr+sectlen then it
is not within the range of the text section but after it. The
comparison should be just '<'.
Fixes#35211
Change-Id: I114633fd734563d38f4e842dd884c6c239f73c95
Reviewed-on: https://go-review.googlesource.com/c/go/+/203817
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 0ae9389609)
Reviewed-on: https://go-review.googlesource.com/c/go/+/203819
Run-TryBot: Carlos Amedee <carlos@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Starting with macOS 10.15 (Catalina), Apple now requires all software
distributed outside of the App Store to be notarized. Any binaries we
distribute must abide by a strict set of requirements like code-signing
and having a minimum target SDK of 10.9 (amongst others).
Apple’s notarization service will recursively inspect archives looking to
find notarization candidate binaries. If it finds a binary that does not
meet the requirements or is unable to decompress an archive, it will
reject the entire distribution. From cursory testing, it seems that the
service uses content sniffing to determine file types, so changing
the file extension will not work.
There are some binaries and archives included in our distribution that
are being detected by Apple’s service as potential candidates for
notarization or decompression. As these are files used by tests and some
are intentionally invalid, we don’t intend to ever make them compliant.
As a workaround for this, we base64-encode any binaries or archives that
Apple’s notarization service issues a warning for, as these warnings will
become errors in January 2020.
Updates #34986Fixes#35748
Change-Id: I106fbb6227b61eb221755568f047ee11103c1680
Reviewed-on: https://go-review.googlesource.com/c/go/+/208118
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 8bbfc51d9a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/208219
Reviewed-by: Alexander Rakoczy <alex@golang.org>
When assessing whether A <= B, the poset's OrderedOrEqual has a passing
condition which permits A <= B, but is not sufficient to infer that A <= B.
This CL removes that incorrect passing condition.
Having identified that A and B are in the poset, the method will report that
A <= B if any of these three conditions are true:
(1) A and B are the same node in the poset.
- This means we know that A == B.
(2) There is a directed path, strict or not, from A -> B
- This means we know that, at least, A <= B, but A < B is possible.
(3) There is a directed path from B -> A, AND that path has no strict edges.
- This means we know that B <= A, but do not know that B < A.
In condition (3), we do not have enough information to say that A <= B, rather
we only know that B == A (which satisfies A <= B) is possible. The way I
understand it, a strict edge shows a known, strictly-ordered relation (<) but
the lack of a strict edge does not show the lack of a strictly-ordered relation.
The difference is highlighted by the example in #34802, where a bounds check is
incorrectly removed by prove, such that negative indexes into a slice
succeed:
n := make([]int, 1)
for i := -1; i <= 0; i++ {
fmt.Printf("i is %d\n", i)
n[i] = 1 // No Bounds check, program runs, assignment to n[-1] succeeds!!
}
When prove is checking the negative/failed branch from the bounds check at n[i],
in the signed domain we learn (0 > i || i >= len(n)). Because prove can't learn
the OR condition, we check whether we know that i is non-negative so we can
learn something, namely that i >= len(n). Prove uses the poset to check whether
we know that i is non-negative. At this point the poset holds the following
relations as a directed graph:
-1 <= i <= 0
-1 < 0
In poset.OrderedOrEqual, we are testing for 0 <= i. In this case, condition (3)
above is true because there is a non-strict path from i -> 0, and that path
does NOT have any strict edges. Because this condition is true, the poset
reports to prove that i is known to be >= 0. Knowing, incorrectly, that i >= 0,
prove learns from the failed bounds check that i >= len(n) in the signed domain.
When the slice, n, was created, prove learned that len(n) == 1. Because i is
also the induction variable for the loop, upon entering the loop, prove previously
learned that i is in [-1,0]. So when prove attempts to learn from the failed
bounds check, it finds the new fact, i > len(n), unsatisfiable given that it
previously learned that i <= 0 and len(n) = 1.
Fixes#34807
Change-Id: I235f4224bef97700c3aa5c01edcc595eb9f13afc
Reviewed-on: https://go-review.googlesource.com/c/go/+/200759
Run-TryBot: Zach Jones <zachj1@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/201060
Run-TryBot: Alexander Rakoczy <alex@golang.org>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/575398
Reviewed-by: Filippo Valsorda <valsorda@google.com>
The partially ordered set uses a method named 'dominates' to determine whether
two nodes are partially ordered. Dominates does a depth-first search of the
DAG, beginning at the source node, and returns true as soon as it finds a path
to the target node. In the context of the forest-of-DAGs that makes up the
poset, dominates is not necessarily checking dominance, but is checking
reachability. See the issue tracker for a more detailed discussion of the
difference.
Fortunately, reachability is logically correct everywhere dominates is currently
used in poset.go. Reachability within a DAG is sufficient to establish the
partial ordering (source < target).
This CL changes the name of the method (dominates -> reaches) and updates
all the comments in the file accordingly.
Updates #34807
Change-Id: Ia3a34f7b14b363801d75b05099cfc686035f7d96
Reviewed-on: https://go-review.googlesource.com/c/go/+/192617
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/201059
Run-TryBot: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/575397
Reviewed-by: Filippo Valsorda <valsorda@google.com>
This is a revert of CL 174437 and equivalent to CL 201360.
The size of the params block passed into the KDSA instruction is
incorrect and this appears to result in out-of-bounds writes
that cause a panic in the crypto/x509 tests when run on a machine
that supports KDSA.
Remove this assembly for now. We can revisit the use of the KDSA
instruction in a future release.
Fixes#34928.
Change-Id: I7ad2fe9714b47ad04abc25f18aa235b9d2aef062
Reviewed-on: https://go-review.googlesource.com/c/go/+/201361
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When assessing whether A <= B, the poset's OrderedOrEqual has a passing
condition which permits A <= B, but is not sufficient to infer that A <= B.
This CL removes that incorrect passing condition.
Having identified that A and B are in the poset, the method will report that
A <= B if any of these three conditions are true:
(1) A and B are the same node in the poset.
- This means we know that A == B.
(2) There is a directed path, strict or not, from A -> B
- This means we know that, at least, A <= B, but A < B is possible.
(3) There is a directed path from B -> A, AND that path has no strict edges.
- This means we know that B <= A, but do not know that B < A.
In condition (3), we do not have enough information to say that A <= B, rather
we only know that B == A (which satisfies A <= B) is possible. The way I
understand it, a strict edge shows a known, strictly-ordered relation (<) but
the lack of a strict edge does not show the lack of a strictly-ordered relation.
The difference is highlighted by the example in #34802, where a bounds check is
incorrectly removed by prove, such that negative indexes into a slice
succeed:
n := make([]int, 1)
for i := -1; i <= 0; i++ {
fmt.Printf("i is %d\n", i)
n[i] = 1 // No Bounds check, program runs, assignment to n[-1] succeeds!!
}
When prove is checking the negative/failed branch from the bounds check at n[i],
in the signed domain we learn (0 > i || i >= len(n)). Because prove can't learn
the OR condition, we check whether we know that i is non-negative so we can
learn something, namely that i >= len(n). Prove uses the poset to check whether
we know that i is non-negative. At this point the poset holds the following
relations as a directed graph:
-1 <= i <= 0
-1 < 0
In poset.OrderedOrEqual, we are testing for 0 <= i. In this case, condition (3)
above is true because there is a non-strict path from i -> 0, and that path
does NOT have any strict edges. Because this condition is true, the poset
reports to prove that i is known to be >= 0. Knowing, incorrectly, that i >= 0,
prove learns from the failed bounds check that i >= len(n) in the signed domain.
When the slice, n, was created, prove learned that len(n) == 1. Because i is
also the induction variable for the loop, upon entering the loop, prove previously
learned that i is in [-1,0]. So when prove attempts to learn from the failed
bounds check, it finds the new fact, i > len(n), unsatisfiable given that it
previously learned that i <= 0 and len(n) = 1.
Fixes#34807
Change-Id: I235f4224bef97700c3aa5c01edcc595eb9f13afc
Reviewed-on: https://go-review.googlesource.com/c/go/+/200759
Run-TryBot: Zach Jones <zachj1@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/201060
Run-TryBot: Alexander Rakoczy <alex@golang.org>
The partially ordered set uses a method named 'dominates' to determine whether
two nodes are partially ordered. Dominates does a depth-first search of the
DAG, beginning at the source node, and returns true as soon as it finds a path
to the target node. In the context of the forest-of-DAGs that makes up the
poset, dominates is not necessarily checking dominance, but is checking
reachability. See the issue tracker for a more detailed discussion of the
difference.
Fortunately, reachability is logically correct everywhere dominates is currently
used in poset.go. Reachability within a DAG is sufficient to establish the
partial ordering (source < target).
This CL changes the name of the method (dominates -> reaches) and updates
all the comments in the file accordingly.
Updates #34807
Change-Id: Ia3a34f7b14b363801d75b05099cfc686035f7d96
Reviewed-on: https://go-review.googlesource.com/c/go/+/192617
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/201059
Run-TryBot: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
For Go 1.13 we introduced Header.Clone and it returns
nil if a nil Header is cloned. Unfortunately, though,
this exported Header.Clone nil behavior differed from
the old Go 1.12 and earlier internal header clone
behavior which always returned non-nil Headers.
This CL fixes the places where that distinction mattered.
Fixes#34882
Change-Id: Id19dea2272948c8dd10883b18ea7f7b8b33ea8eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/200977
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 9969c72080)
Reviewed-on: https://go-review.googlesource.com/c/go/+/201040
While understanding why syscall.Read is 2x slower on darwin/amd64, I found
out that, contrary to popular belief, the slowdown is not due to the migration
to use libSystem.dylib instead of direct SYSCALLs, i.e., CL 141639 (and #17490),
but due to a subtle change introduced in CL 141639.
Previously, syscall.Read used syscall.Syscall(SYS_READ), whose preamble called
runtime.entersyscall, but after CL 141639, syscall.Read changes to call
runtime.syscall_syscall instead, which in turn calls runtime.entersyscallblock
instead of runtime.entersyscall. And the entire 2x slow down can be attributed
to this change.
I think this is unnecessary as even though syscalls like Read might block, it
does not always block, so there is no need to handoff P proactively for each
Read. Additionally, we have been fine with not handing off P for each Read
prior to Go 1.12, so we probably don't need to change it. This changes restores
the pre-Go 1.12 behavior, where syscall preamble uses runtime.entersyscall,
and we rely on sysmon to take P back from g blocked in syscalls.
Updates #34712
Change-Id: If76e97b5a7040cf1c10380a567c4f5baec3121ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/197938
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit c1635ad8f0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/200103
Run-TryBot: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In CL 197059, I suppressed errors if the target package was already found.
However, that does not cover the case of passing a '/v2' module path to
'go get' when the module does not contain a package at its root.
This CL is a minimal fix for that case, intended to be backportable to 1.13.
(Longer term, I intend to rework the version-validation check to treat
all mismatched paths as ErrNotExist.)
Updates #34746Fixes#34747
Change-Id: Ia963c2ea00fae424812b8f46a4d6c2c668252147
Reviewed-on: https://go-review.googlesource.com/c/go/+/199839
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 421d35cf69)
Reviewed-on: https://go-review.googlesource.com/c/go/+/199997
Passing ambiguous patterns, ending in `.go`, to `go list` results in them
being interpreted as Go files despite potentially being package references.
This can then result in errors on other package references.
The parsing logic is modified to check for a locally present file
corresponding to any pattern ending in `.go`. If no such file is present
the pattern is considered to be a package reference.
We're also adding a variety of non-regression tests that fail with the
original parsing code but passes after applying the fix.
Updates #34653Fixes#34694
Change-Id: I073871da0dfc5641a359643f95ac14608fdca09b
GitHub-Last-Rev: 5abc200103
GitHub-Pull-Request: golang/go#34663
Reviewed-on: https://go-review.googlesource.com/c/go/+/198459
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 33683f1d64df0cef2c598a84b741abb5af8abe5e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/198957
Reviewed-by: Jay Conrod <jayconrod@google.com>
Inline scavenging causes significant performance regressions in tail
latency for k8s and has relatively little benefit for RSS footprint.
We disabled inline scavenging in Go 1.12.5 (CL 174102) as well, but
we thought other changes in Go 1.13 had mitigated the issues with
inline scavenging. Apparently we were wrong.
This CL switches back to only doing foreground scavenging on heap
growth, rather than doing it when allocation tries to allocate from
scavenged space.
Fixes#34556
Change-Id: I1f5df44046091f0b4f89fec73c2cde98bf9448cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/183857
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit eb96f8a574)
Reviewed-on: https://go-review.googlesource.com/c/go/+/198486
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
This change makes it so that the scavenge goal is defined primarily in
terms of heap_inuse at the end of the last GC rather than next_gc. The
reason behind this change is that next_gc doesn't take into account
fragmentation, and we can fall into situation where the scavenger thinks
it should have work to do but there's no free and unscavenged memory
available.
In order to ensure the scavenge goal still tracks next_gc, we multiply
heap_inuse by the ratio between the current heap goal and the last heap
goal, which describes whether the heap is growing or shrinking, and by
how much.
Finally, this change updates the documentation for scavenging and
elaborates on why the scavenge goal is defined the way it is.
Fixes#34149
Change-Id: I8deaf87620b5dc12a40ab8a90bf27932868610da
Reviewed-on: https://go-review.googlesource.com/c/go/+/193040
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 9b30811280)
Reviewed-on: https://go-review.googlesource.com/c/go/+/198487
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Currently, we map and grow the heap a whole arena (64MB) at a time.
Unfortunately, in order to fix#32828, we need to switch from
scavenging inline with allocation back to scavenging on heap growth,
but heap-growth scavenging happens in large jumps because we grow the
heap in large jumps.
In order to prepare for better heap-growth scavenging, this CL
separates mapping more space for the heap from actually "growing" it
(tracking the new space with spans). Instead, growing the heap keeps
track of the "current arena" it's growing into. It track that with new
spans as needed, and only maps more arena space when the current arena
is inadequate. The effect to the user is the same, but this will let
us scavenge on much smaller increments of heap growth.
There are two slightly subtleties to this change:
1. If an allocation requires mapping a new arena and that new arena
isn't contiguous with the current arena, we don't want to lose the
unused space in the current arena, so we have to immediately track
that with a span.
2. The mapped space must be accounted as released and idle, even
though it isn't actually tracked in a span.
For #32828, since this makes heap-growth scavenging far more
effective, especially at small heap sizes. For example, this change is
necessary for TestPhysicalMemoryUtilization to pass once we remove
inline scavenging.
Updates #34556
Change-Id: I300e74a0534062467e4ce91cdc3508e5ef9aa73a
Reviewed-on: https://go-review.googlesource.com/c/go/+/189957
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit f18109d7e3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/198485
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Starting in Windows 8, the wait functions don't take into account
suspend time, even though the monotonic counters do. This results in
timer buckets stalling on resume. Therefore, this commit makes it so
that on resume, we return from the wait functions and recalculate the
amount of time left to wait.
This is a cherry pick of CL 191957 and its cleanup, CL 198417.
Updates #31528Fixes#34130
Change-Id: I0db02cc72188cb620954e87a0180e0a3c83f4a56
Reviewed-on: https://go-review.googlesource.com/c/go/+/193607
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
There are currently two edges in the lock cycle graph caused by
scavenge.lock: with sched.lock and mheap_.lock. These edges appear
because of the call to ready() and stack growths respectively.
Furthermore, there's already an invariant in the code wherein
mheap_.lock must be acquired before scavenge.lock, hence the cycle.
The fix to this is to bring scavenge.lock higher in the lock cycle
graph, such that sched.lock and mheap_.lock are only acquired once
scavenge.lock is already held.
To faciliate this change, we move scavenger waking outside of
gcSetTriggerRatio such that it doesn't have to happen with the heap
locked. Furthermore, we check scavenge generation numbers with the heap
locked by using gopark instead of goparkunlock, and specify a function
which aborts the park should there be any skew in generation count.
Fixes#34150.
Change-Id: I3519119214bac66375e2b1262b36ce376c820d12
Reviewed-on: https://go-review.googlesource.com/c/go/+/191977
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 62e4156552)
Reviewed-on: https://go-review.googlesource.com/c/go/+/197501
Reviewed-by: Austin Clements <austin@google.com>
RFC 7230 is clear about headers with a space before the colon, like
X-Answer : 42
being invalid, but we've been accepting and normalizing them for compatibility
purposes since CL 5690059 in 2012.
On the client side, this is harmless and indeed most browsers behave the same
to this day. On the server side, this becomes a security issue when the
behavior doesn't match that of a reverse proxy sitting in front of the server.
For example, if a WAF accepts them without normalizing them, it might be
possible to bypass its filters, because the Go server would interpret the
header differently. Worse, if the reverse proxy coalesces requests onto a
single HTTP/1.1 connection to a Go server, the understanding of the request
boundaries can get out of sync between them, allowing an attacker to tack an
arbitrary method and path onto a request by other clients, including
authentication headers unknown to the attacker.
This was recently presented at multiple security conferences:
https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn
net/http servers already reject header keys with invalid characters.
Simply stop normalizing extra spaces in net/textproto, let it return them
unchanged like it does for other invalid headers, and let net/http enforce
RFC 7230, which is HTTP specific. This loses us normalization on the client
side, but there's no right answer on the client side anyway, and hiding the
issue sounds worse than letting the application decide.
Fixes CVE-2019-16276
Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
(cherry picked from commit 1280b868e82bf173ea3e988be3092d160ee66082)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/558935
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
CL 140357 caused HTTP/2 connections to be put in the idle pool, but
failed to properly guard the trace.GotConn call in getConn. dialConn
returns a minimal persistConn with conn == nil for HTTP/2 connections.
This persistConn was then returned from queueForIdleConn and caused the
httptrace.GotConnInfo passed into GotConn to have a nil Conn field.
HTTP/2 connections call GotConn themselves so leave it for HTTP/2 to call
GotConn as is done directly below.
Fixes#34285
Change-Id: If54bfaf6edb14f5391463f908efbef5bb8a5d78e
GitHub-Last-Rev: 2b7d66a1ce
GitHub-Pull-Request: golang/go#34283
Reviewed-on: https://go-review.googlesource.com/c/go/+/195237
Reviewed-by: Michael Fraenkel <michael.fraenkel@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 582d5194fa)
Reviewed-on: https://go-review.googlesource.com/c/go/+/196579
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
If a generated test main package transitively depends on a main
package, the main package will now always be rebuilt as a library and
will not be compiled with '-p main'.
This expands the fix for #30907, which only applied to packages with
the BuildInfo set (main packages built in module mode). Linking
multiple packages with BuildInfo caused link errors, but it appears
these errors apply to some symbols in GOPATH mode.
Fixes#34223
Change-Id: Ic1e53437942269a950dd7e45d163707922c92edd
Reviewed-on: https://go-review.googlesource.com/c/go/+/195279
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 24781a1faf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/195281
In CL 173017, I changed the package-to-module query logic to query all
possible module paths in parallel in order to reduce latency. (For
long package paths, most such paths will not exist and will fail with
little overhead.)
The module resolution algorithm treats various kinds of non-existence
as “soft errors”, to be reported only if package resolution fails, but
treats any remaining errors as hard errors that should fail the query.
Unfortunately, that interacted badly with the +incompatible version
validation added in CL 181881, causing a regression in the 'direct'
fetch path for modules using the “major branch” layout¹ with a post-v1
version on the repository's default branch. Because we did not
interpret a mismatched module path as “no such module”, a go.mod file
specifying the path 'example.com/foo/v2' would cause the search for
module 'example.com/foo' to error out. (That regression was not caught
ahead of time due to a lack of test coverage for 'go get' on a package
within a /vN module.)
The promotion of hard errors during parallel search also made the 'go'
command less tolerant of servers that advertise 'go-import' tags for
nonexistent repositories. CL 194561 mitigated that problem for HTTP
servers that return code 404 or 410 for a nonexistent repository, but
unfortunately a few servers in common use (notably GitLab and
pre-1.9.3 releases of Gitea) do not.
This change mitigates both of those failure modes by ignoring
“miscellaneous” errors from shorter module paths if the requested
package pattern was successfully matched against a module with a
longer path.
¹https://research.swtch.com/vgo-module#from_repository_to_modules
Updates #34383
Updates #34094Fixes#34497Fixes#34215
Change-Id: If37dc422e973eba13f3a3aeb68bc7b96e2d7f73d
Reviewed-on: https://go-review.googlesource.com/c/go/+/197059
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 a3426f2571)
Reviewed-on: https://go-review.googlesource.com/c/go/+/197063
'go get' accepts arguments of the form path@version, and it passes
them through search.CleanPatterns before querying proxies. With this
change, CleanPatterns preserves text after '@' and will strip trailing
slashes from the patn.
Previously, we did not strip trailing slashes when a version was
present, which caused proxy base URL validation to fail. Module paths
that end with ".go" (for example, github.com/nats-io/nats.go) use
trailing slashes to prevent 'go build' and other commands from
interpreting packages as source file names, so this caused unnecessary
problems for them.
Fixes#34243
Change-Id: Id3730c52089e52f1cac446617c20132a3021a808
Reviewed-on: https://go-review.googlesource.com/c/go/+/194600
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 8875fb97c5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/194687
Created the dev.boringcrypto.go1.13 branch at the latest
dev.boringcrypto commit that did not include commits to master made
after the release-branch.go1.13 branch point by running
git push origin 5a1705286e:refs/heads/dev.boringcrypto.go1.13
Had to make the same merge as CL 193137 because of the cherry-pick
in CL 191998.
Change-Id: I88c1749ad4f2ec59b7e4b3344261e9becae70056
This partly reverts CL 173417 as it incorrectly documented that Compact
performed HTML escaping and the output was safe to embed inside HTML
<script> tags. This has never been true.
Although Compact does escape U+2028 and U+2029, it doesn't escape <, >
or &. Compact is thus only performing a subset of HTML escaping and it's
output is not safe to embed inside HTML <script> tags.
A more complete fix would be for Compact to either never perform any
HTML escaping, as it was prior to CL 10883045, or to actually perform
the same HTML escaping as HTMLEscape. Neither change is likely safe
enough for go1.13.
Fixes#34006
Updates #30357
Change-Id: I912f0fe9611097d988048b28228c4a5b985080ba
GitHub-Last-Rev: aebababc92
GitHub-Pull-Request: golang/go#33427
Reviewed-on: https://go-review.googlesource.com/c/go/+/188717
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 79669dc705)
Reviewed-on: https://go-review.googlesource.com/c/go/+/192747
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
'git ls-remote' started recognizing the '--' separator at some point
after 2.7.4, but git defaults to version 2.7.4 on Ubuntu 16.04 LTS,
which remains supported by Ubuntu until April 2021.
We added '--' tokens to most VCS commands as a defensive measure in
CL 181237, but it isn't strictly necessary here because the 'scheme'
argument to our template is chosen from a predefined list: we can
safely drop it to retain compatibility.
Cherry-picked from CL 191978.
Updates #33836Fixes#33880
Change-Id: Ibb53366b95f8029b587e0b7646a439330d759ac7
Reviewed-on: https://go-review.googlesource.com/c/go/+/191973
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I'm trying to keep the code changes minimal for backporting to Go 1.13,
so it is still possible for a handful of entries to leak,
but the leaks are now O(1) instead of O(N) in the steady state.
Longer-term, I think it would be a good idea to coalesce idleMu with
connsPerHostMu and clear entries out of both queues as soon as their
goroutines are done waiting.
Cherry-picked from CL 191964.
Updates #33849
Updates #33850Fixes#33878
Change-Id: Ia66bc64671eb1014369f2d3a01debfc023b44281
Reviewed-on: https://go-review.googlesource.com/c/go/+/191964
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 94bf9a8d4a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191967
Update golang.org/x/net to v0.0.0-20190813141303-74dc4d7220e7 to import
the following security fix.
commit 74dc4d7220e7acc4e100824340f3e66577424772
Author: Filippo Valsorda <filippo@golang.org>
Date: Sun Aug 11 02:12:18 2019 -0400
http2: limit number of control frames in server send queue
An attacker could cause servers to queue an unlimited number of PING
ACKs or RST_STREAM frames by soliciting them and not reading them, until
the program runs out of memory.
Limit control frames in the queue to a few thousands (matching the limit
imposed by other vendors) by counting as they enter and exit the scheduler,
so the protection will work with any WriteScheduler.
Once the limit is exceeded, close the connection, as we have no way to
communicate with the peer.
Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
This change was generated with cmd/go and cmd/bundle:
$ go get -u golang.org/x/net
$ go mod tidy
$ go mod vendor
$ go generate net/http
Fixes CVE-2019-9512 and CVE-2019-9514
Fixes#33606
Change-Id: I464baf96175006aa101d65d3b0f6494f28a626ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/190137
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
(cherry picked from commit 145e193131)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191618
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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
294 changed files with 10266 additions and 3302 deletions
allow finer control over the <ahref="/pkg/context#Context"><code>Context</code></a> values provided to requests and connections.
allow finer control over the <ahref="/pkg/context/#Context"><code>Context</code></a> values provided to requests and connections.
</p>
<p><!-- CL 167781 -->
<ahref="/pkg/net/http#DetectContentType"><code>http.DetectContentType</code></a> now correctly detects RAR signatures, and can now also detect RAR v5 signatures.
<ahref="/pkg/net/http/#DetectContentType"><code>http.DetectContentType</code></a> now correctly detects RAR signatures, and can now also detect RAR v5 signatures.
</p>
<p><!-- CL 173658 -->
@@ -836,7 +862,8 @@ godoc
</p>
<p><!-- CL 179457 -->
<ahref="/pkg/net/http/#Transport"><code>Transport</code></a> now silently ignores a <code>408 "Request Timeout"</code> response.
The <ahref="/pkg/net/http/#Transport"><code>Transport</code></a> no longer logs errors when servers
gracefully shut down idle connections using a <code>"408 Request Timeout"</code> response.
# Copied from https://raw.githubusercontent.com/docker-library/golang/master/1.9-rc/stretch/go-wrapper
# Copied into Docker images.
set -e
usage() {
base="$(basename "$0")"
cat <<EOUSAGE
usage: $base command [args]
This script assumes that is is run from the root of your Go package (for
example, "/go/src/app" if your GOPATH is set to "/go").
In Go 1.4, a feature was introduced to supply the canonical "import path" for a
given package in a comment attached to a package statement
(https://golang.org/s/go14customimport).
This script allows us to take a generic directory of Go source files such as
"/go/src/app" and determine that the canonical "import path" of where that code
expects to live and reference itself is "github.com/jsmith/my-cool-app". It
will then ensure that "/go/src/github.com/jsmith/my-cool-app" is a symlink to
"/go/src/app", which allows us to build and run it under the proper package
name.
For compatibility with versions of Go older than 1.4, the "import path" may also
be placed in a file named ".godir".
Available Commands:
$base download
$base download -u
(equivalent to "go get -d [args] [godir]")
$base install
$base install -race
(equivalent to "go install [args] [godir]")
$base run
$base run -app -specific -arguments
(assumes "GOPATH/bin" is in "PATH")
EOUSAGE
}
# make sure there is a subcommand specified
if [ "$#" -eq 0 ]; then
usage >&2
exit 1
fi
# "shift" so that "$@" becomes the remaining arguments and can be passed along to other "go" subcommands easily
cmd="$1"
shift
goDir="$(go list -e -f '{{.ImportComment}}' 2>/dev/null || true)"
if [ -z "$goDir" -a -s .godir ]; then
goDir="$(cat .godir)"
fi
dir="$(pwd -P)"
if [ "$goDir" ]; then
goPath="${GOPATH%%:*}" # this just grabs the first path listed in GOPATH, if there are multiple (which is the detection logic "go get" itself uses, too)
goDirPath="$goPath/src/$goDir"
mkdir -p "$(dirname "$goDirPath")"
if [ ! -e "$goDirPath" ]; then
ln -sfv "$dir" "$goDirPath"
elif [ ! -L "$goDirPath" ]; then
echo >&2 "error: $goDirPath already exists but is unexpectedly not a symlink!"
t.Skip("known Windows test failure on release-branch.go1.13; fix is in CL 204442 but requires non-test code changes unlikely to be appropriate for backporting this late")
[windows] skip # known Windows test failure on release-branch.go1.13; fix is in CL 206144 but requires non-test code changes unlikely to be appropriate for backporting this late
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.
