Jean de Klerk [Mon, 1 Jun 2020 18:19:05 +0000 (12:19 -0600)]
[release-branch.go1.14] testing: capture testname on --- PASS and --- FAIL lines
This fixes an issue raised at https://github.com/golang/go/issues/38458#issuecomment-635617670
in which --- PASS and --- FAIL lines would not trigger --- CONT lines
of other tests.
Updates #38458.
For #39308.
Change-Id: I0d8cc54d682a370d0a6ea6816a11b2e462a92efe
Reviewed-on: https://go-review.googlesource.com/c/go/+/235997
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/242058 Reviewed-by: Jean de Klerk <deklerk@google.com> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Tobias Klauser [Sat, 23 May 2020 22:09:20 +0000 (00:09 +0200)]
[release-branch.go1.14] cmd/go: fix parallel chatty tests on solaris-amd64 builder
The parallel chatty tests added in CL 229085 fail on the
solaris-amd64-oraclerel builder, because a +NN:NN offset time zone is
used. Allow for the `+` character in the corresponding regex to fix
these tests. Also move the '-' to the end of the character class, so it
is not interpreted as the range 9-T.
Updates #38458.
For #39308.
Change-Id: Iec9ae82ba45d2490176f274f0dc6812666eae718
Reviewed-on: https://go-review.googlesource.com/c/go/+/234978
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/242059
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Jean de Klerk [Mon, 4 May 2020 20:06:34 +0000 (14:06 -0600)]
[release-branch.go1.14] testing: reformat test chatty output
In #24929, we decided to stream chatty test output. It looks like,
foo_test.go:138: TestFoo/sub-1: hello from subtest 1
foo_test.go:138: TestFoo/sub-2: hello from subtest 2
In this CL, we refactor the output to be grouped by === CONT lines, preserving
the old test-file-before-log-line behavior:
=== CONT TestFoo/sub-1
foo_test.go:138 hello from subtest 1
=== CONT TestFoo/sub-2
foo_test.go:138 hello from subtest 2
This should remove a layer of verbosity from tests, and make it easier to group
together related lines. It also returns to a more familiar format (the
pre-streaming format), whilst still preserving the streaming feature.
Updates #38458.
Fixes #39308.
Change-Id: Iaef94c580d69cdd541b2ef055aa004f50d72d078
Reviewed-on: https://go-review.googlesource.com/c/go/+/229085
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/242057 Reviewed-by: Jean de Klerk <deklerk@google.com> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
Emmanuel T Odeke [Fri, 24 Jan 2020 02:18:39 +0000 (18:18 -0800)]
[release-branch.go1.14] database/sql: backport 5 Tx rollback related CLs
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.
Note:
Due to the restrictions that we cannot retroactively
introduce API changes to Go1.14.6 that weren't in Go1.14, 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".
Filippo Valsorda [Fri, 19 Jun 2020 02:45:52 +0000 (22:45 -0400)]
[release-branch.go1.14-security] crypto/x509: respect VerifyOptions.KeyUsages on Windows
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.
[release-branch.go1.14-security] net/http: synchronize "100 Continue" write and Handler writes
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.
Keith Randall [Thu, 18 Jun 2020 19:51:35 +0000 (12:51 -0700)]
[release-branch.go1.14] reflect: zero stack slots before writing to them with write barriers
reflect.assignTo writes to the target using write barriers. Make sure
that the memory it is writing to is zeroed, so the write barrier does
not read pointers from uninitialized memory.
Fixes #39698
Change-Id: Ia64b2cacc193bffd0c1396bbce1dfb8182d4905b
Reviewed-on: https://go-review.googlesource.com/c/go/+/238760
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 3dec253783e1211989102ac6abd34cddbf8ba0e6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/238861
Keith Randall [Thu, 25 Jun 2020 03:59:18 +0000 (20:59 -0700)]
[release-branch.go1.14] cmd/compile: remove check that Zero's arg has the correct base type
It doesn't have to. The type in the aux field is authoritative.
There are cases involving casting from interface{} where pointers
have a placeholder pointer type (because the type is not known when
the IData op is generated).
The check was introduced in CL 13447.
Fixes #39849
Change-Id: Id77a57577806a271aeebd20bea5d92d08ee7aa6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/239817
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 3b2f67a59702e4881625cb967f853ef56b0c4828)
Reviewed-on: https://go-review.googlesource.com/c/go/+/239997
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
[release-branch.go1.14] cmd/link: detect trampoline of deferreturn call
This is a backport of CL 234105. This is not a clean cherry-pick,
as CL 234105 is for the new linker, whereas we still use the old
linker here. This CL backports the logic.
The runtime needs to find the PC of the deferreturn call in a few
places. So for functions that have defer, we record the PC of
deferreturn call in its funcdata.
For very large binaries, the deferreturn call could be made
through a trampoline. The current code of finding deferreturn PC
fails in this case. This CL handles the trampoline as well.
Fixes #39991.
Updates #39049.
Change-Id: I929be54d6ae436f5294013793217dc2a35f080d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234105
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/240917
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Joel Sing <joel@sing.id.au>
Daniel Martí [Sun, 14 Jun 2020 21:09:18 +0000 (22:09 +0100)]
[release-branch.go1.14] encoding/json: revert "avoid work when unquoting strings, take 2"
This reverts golang.org/cl/190659 and golang.org/cl/226218, minus the
regression tests in the latter.
The original work happened in golang.org/cl/151157, which was reverted
in golang.org/cl/190909 due to a crash found by fuzzing.
We tried a second time in golang.org/cl/190659, which shipped with Go
1.14. A bug was found, where strings would be mangled in certain edge
cases. The fix for that was golang.org/cl/226218, which was backported
into Go 1.14.4.
Unfortunately, a second regression was just reported in #39555, which is
a similar case of strings getting mangled when decoding under certain
conditions. It would be possible to come up with another small patch to
fix that edge case, but instead, let's just revert the entire
optimization, as it has proved to do more harm than good. Moreover, it's
hard to argue or prove that there will be no more such regressions.
However, all the work wasn't for nothing. First, we learned that the way
the decoder unquotes tokenized strings isn't simple; initially, we had
wrongly assumed that each string was unquoted exactly once and in order.
Second, we have gained a number of regression tests which will be useful
to prevent the same mistakes in the future, including the test cases we
add in this CL.
For #39555.
Fixes #39585.
Change-Id: I66a6919c2dd6d9789232482ba6cf3814eaa70f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/237838
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
(cherry picked from commit 11389baf2ea0b5e920959b0aa8d406d8090a0a93)
Reviewed-on: https://go-review.googlesource.com/c/go/+/241081 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
This teaches vet to recognize %O in a fmt.Printf format string.
O has been supported since the 1.13 release, but vet would warn about it.
Fixes #39288.
For #29986.
Change-Id: Ia7817ee60ae6beac32cc402c0c68afa917e4ef0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/236199
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitri Shuralyov [Fri, 12 Jun 2020 02:02:27 +0000 (22:02 -0400)]
[release-branch.go1.14] cmd/compile/internal/ssa: skip TestNexting with old Delve on linux/386
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 #39562.
Change-Id: I395cb1f076e59dd3a3feb53e1dcdce5101e9a0f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/237619 Reviewed-by: David Chase <drchase@google.com>
Ivan Trubach [Thu, 12 Dec 2019 13:33:42 +0000 (13:33 +0000)]
[release-branch.go1.14] cmd/doc: fix merging comments in -src mode
These changes fix go doc -src mode that vomits comments from random files if
filesystem does not sort files by name. The issue was with parse.ParseDir
using the Readdir order of files, which varies between platforms and filesystem
implementations. Another option is to merge comments using token.FileSet.Iterate
order in cmd/doc, but since ParseDir is mostly used in go doc, I’ve opted for
smaller change because it’s unlikely to break other uses or cause any perfomance
issues.
Example (macOS APFS): `go doc -src net.ListenPacket`
Fixes #38993
Change-Id: I7f9f368c7d9ccd9a2cbc48665f2cb9798c7b3a3f
GitHub-Last-Rev: 654fb450421266a0bb64518016944db22bd681e3
GitHub-Pull-Request: golang/go#36104
Reviewed-on: https://go-review.googlesource.com/c/go/+/210999
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
(cherry picked from commit 585e31df63f6879c03b285711de6f9dcba1f2cb0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/235579
Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org>
Daniel Martí [Fri, 27 Mar 2020 23:56:09 +0000 (23:56 +0000)]
[release-branch.go1.14] encoding/json: don't mangle strings in an edge case when decoding
The added comment contains some context. The original optimization
assumed that each call to unquoteBytes (or unquote) followed its
corresponding call to rescanLiteral. Otherwise, unquoting a literal
might use d.safeUnquote from another re-scanned literal.
Unfortunately, this assumption is wrong. When decoding {"foo": "bar"}
into a map[T]string where T implements TextUnmarshaler, the sequence of
calls would be as follows:
Note that the call to UnmarshalText happens in literalStore, which
repeats the work to unquote the input string literal. But, since that
happens after we've re-scanned "bar", we're using the wrong safeUnquote
field value.
In the added test case, the second string had a non-zero number of safe
bytes, and the first string had none since it was all non-ASCII. Thus,
"safely" unquoting a number of the first string's bytes could cut a rune
in half, and thus mangle the runes.
A rather simple fix, without a full revert, is to only allow one use of
safeUnquote per call to unquoteBytes. Each call to rescanLiteral when
we have a string is soon followed by a call to unquoteBytes, so it's no
longer possible for us to use the wrong index.
Also add a test case from #38126, which is the same underlying bug, but
affecting the ",string" option.
Before the fix, the test would fail, just like in the original two issues:
--- FAIL: TestUnmarshalRescanLiteralMangledUnquote (0.00s)
decode_test.go:2443: Key "开源" does not exist in map: map[开���:12345开源]
decode_test.go:2458: Unmarshal unexpected error: json: invalid use of ,string struct tag, trying to unmarshal "\"aaa\tbbb\"" into string
Fixes #38106.
For #38105.
For #38126.
Change-Id: I761e54924e9a971a4f9eaa70bbf72014bb1476e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/226218
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
(cherry picked from commit 55361a26177b3faf151a1d35467db5d403b51f22)
Reviewed-on: https://go-review.googlesource.com/c/go/+/233057
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Daniel Martí [Tue, 31 Mar 2020 11:20:15 +0000 (12:20 +0100)]
[release-branch.go1.14] encoding/json: properly encode strings with ",string" again
golang.org/cl/193604 fixed one bug when one encodes a string with the
",string" option: if SetEscapeHTML(false) is used, we should not be
using HTML escaping for the inner string encoding. The CL correctly
fixed that.
The CL also tried to speed up this edge case. By avoiding an entire new
call to Marshal, the new Issue34127 benchmark reduced its time/op by
45%, and lowered the allocs/op from 3 to 2.
However, that last optimization wasn't correct:
Since Go 1.2 every string can be marshaled to JSON without error
even if it contains invalid UTF-8 byte sequences. Therefore
there is no need to use Marshal again for the only reason of
enclosing the string in double quotes.
JSON string encoding isn't just about adding quotes and taking care of
invalid UTF-8. We also need to escape some characters, like tabs and
newlines.
The new code failed to do that. The bug resulted in the added test case
failing to roundtrip properly; before our fix here, we'd see an error:
invalid use of ,string struct tag, trying to unmarshal "\"\b\f\n\r\t\"\\\"" into string
If you pay close attention, you'll notice that the special characters
like tab and newline are only encoded once, not twice. When decoding
with the ",string" option, the outer string decode works, but the inner
string decode fails, as we are now decoding a JSON string with unescaped
special characters.
The fix we apply here isn't to go back to Marshal, as that would
re-introduce the bug with SetEscapeHTML(false). Instead, we can use a
new encode state from the pool - it results in minimal performance
impact, and even reduces allocs/op further. The performance impact seems
fair, given that we need to check the entire string for characters that
need to be escaped.
name old time/op new time/op delta
Issue34127-8 89.7ns ± 2% 100.8ns ± 1% +12.27% (p=0.000 n=8+8)
name old alloc/op new alloc/op delta
Issue34127-8 40.0B ± 0% 32.0B ± 0% -20.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
Issue34127-8 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
Instead of adding another standalone test, we convert an existing
"string tag" test to be table-based, and add another test case there.
One test case from the original CL also had to be amended, due to the
same problem - when escaping '<' due to SetEscapeHTML(true), we need to
end up with double escaping, since we're using ",string".
Fixes #38178.
For #38173.
Change-Id: I2b0df9e4f1d3452fff74fe910e189c930dde4b5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/226498
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
(cherry picked from commit b1a48af7e8ee87cc46e1bbb07f81ac4853e0f27b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/233037 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Michael Pratt [Thu, 7 May 2020 22:13:21 +0000 (18:13 -0400)]
[release-branch.go1.14] runtime: disable preemption in startTemplateThread
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:
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.
Ian Lance Taylor [Wed, 20 May 2020 03:23:58 +0000 (20:23 -0700)]
[release-branch.go1.14] syscall: preserve Windows file permissions for O_CREAT|O_TRUNC
On Windows, calling syscall.Open(file, O_CREAT|O_TRUNC, 0) for a file
that already exists would change the file to be read-only.
That is not how the Unix syscall.Open behaves, so avoid it on
Windows by calling CreateFile twice if necessary.
For #38225
Fixes #39158
Change-Id: I70097fca8863df427cc8a97b9376a9ffc69c6318
Reviewed-on: https://go-review.googlesource.com/c/go/+/234534
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
(cherry picked from commit 567556d78657326c99b8fa84ec2a5ee511a0941b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/234686 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
[release-branch.go1.14] go/doc: fix detection of whole file examples
After CL 211357 (commit 499dc1c),
hasTests and numDecl were not updated properly for function
declarations with parameters, which affected the whole file
example detection logic. This caused examples like
In CL 187657, I refactored constant conversion logic without realizing
that conversions between int/float and complex types are allowed for
constants (assuming the constant values are representable by the
destination type), but are never allowed for non-constant expressions.
This CL expands convertop to take an extra srcConstant parameter to
indicate whether the source expression is a constant; and if so, to
allow any numeric-to-numeric conversion. (Conversions of values that
cannot be represented in the destination type are rejected by
evconst.)
Fixes #38123.
For #38117.
Change-Id: Id7077d749a14c8fd910be38da170fa5254819f2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/226197
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
(cherry picked from commit 34314280e46da1558bc7f9cd7e8a9ed610cf417b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/232719
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Michael Anthony Knyszek [Fri, 24 Apr 2020 14:46:28 +0000 (14:46 +0000)]
[release-branch.go1.14] runtime: make the scavenger's pacing logic more defensive
This change adds two bits of logic to the scavenger's pacing. Firstly,
it checks to make sure we scavenged at least one physical page, if we
released a non-zero amount of memory. If we try to release less than one
physical page, most systems will release the whole page, which could
lead to memory corruption down the road, and this is a signal we're in
this situation.
Secondly, the scavenger's pacing logic now checks to see if the time a
scavenging operation takes is measured to be exactly zero or negative.
The exact zero case can happen if time update granularity is too large
to effectively capture the time the scavenging operation took, like on
Windows where the OS timer frequency is generally 1ms. The negative case
should not happen, but we're being defensive (against kernel bugs, bugs
in the runtime, etc.). If either of these cases happen, we fall back to
Go 1.13 behavior: assume the scavenge operation took around 10µs per
physical page. We ignore huge pages in this case because we're in
unknown territory, so we choose to be conservative about pacing (huge
pages could only increase the rate of scavenging).
Currently, the scavenger is broken on Windows because the granularity of
time measurement is around 1 ms, which is too coarse to measure how fast
we're scavenging, so we often end up with a scavenging time of zero,
followed by NaNs and garbage values in the pacing logic, which usually
leads to the scavenger sleeping forever.
For #38617.
Fixes #38856.
Change-Id: Iaaa2a4cbb21338e1258d010f7362ed58b7db1af7
Reviewed-on: https://go-review.googlesource.com/c/go/+/229997
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit c7915376ce3cdd172bf71ca4127c67f196b8e43e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/232743
Keith Randall [Thu, 2 Apr 2020 19:00:50 +0000 (12:00 -0700)]
[release-branch.go1.14] runtime/race: update some .syso files
Update race detector syso files for some platforms.
There's still 2 more to do, but they might take a while so I'm
mailing the ones I have now.
Note: some arm64 tests did not complete successfully due to out
of memory errors, but I suspect the .syso is correct.
For #14481
For #37485 (I think?)
For #37355
Fixes #38321
Change-Id: I7e7e707a1fd7574855a538ba89dc11acc999c760
Reviewed-on: https://go-review.googlesource.com/c/go/+/226981
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 041bcb32b59b37ff55ad527083b7bd4ffa4c1a3d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/231222 Reviewed-by: Keith Randall <khr@golang.org>
Michael Anthony Knyszek [Wed, 22 Apr 2020 21:36:11 +0000 (21:36 +0000)]
[release-branch.go1.14] runtime: ensure allocToCache updates searchAddr in a valid way
Currently allocToCache assumes it can move the search address past the
block it allocated the cache from, which violates the property that
searchAddr should always point to mapped memory (i.e. memory represented
by pageAlloc.inUse).
This bug was already fixed once for pageAlloc.alloc in the Go 1.14
release via CL 216697, but that changed failed to take into account
allocToCache.
For #38605.
Fixes #38606.
Change-Id: Id08180aa10d19dc0f9f551a1d9e327a295560dff
Reviewed-on: https://go-review.googlesource.com/c/go/+/229577
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 287d1ec96c1271de532c6b1160cd9cbbe717ee34)
Reviewed-on: https://go-review.googlesource.com/c/go/+/230377 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Munday [Tue, 14 Apr 2020 14:46:26 +0000 (15:46 +0100)]
[release-branch.go1.14] cmd/compile: fix deallocation of live value copies in regalloc
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 #38443.
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 382fe3e2498f2066400e7e7007aa9903440e339d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/230357
Rémy Oudompheng [Thu, 5 Mar 2020 06:59:00 +0000 (07:59 +0100)]
[release-branch.go1.14] math/big: correct off-by-one access in divBasic
The divBasic function computes the quotient of big nats u/v word by word.
It estimates each word qhat by performing a long division (top 2 words of u
divided by top word of v), looks at the next word to correct the estimate,
then perform a full multiplication (qhat*v) to catch any inaccuracy in the
estimate.
In the latter case, "negative" values appear temporarily and carries
must be carefully managed, and the recursive division refactoring
introduced a case where qhat*v has the same length as v, triggering an
out-of-bounds write in the case it happens when computing the top word
of the quotient.
Jay Conrod [Fri, 28 Feb 2020 21:31:19 +0000 (16:31 -0500)]
[release-branch.go1.14] cmd/go: make module zip extraction more robust
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 #37800
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 093049b3709eda7537ece92a2991918cf53782d6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/223147
Austin Clements [Wed, 26 Feb 2020 20:12:33 +0000 (15:12 -0500)]
[release-branch.go1.14] runtime: fix rounding in materializeGCProg
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:
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.
Michael Pratt [Thu, 26 Mar 2020 19:10:21 +0000 (15:10 -0400)]
[release-branch.go1.14] runtime/pprof: increment fake overflow record PC
gentraceback generates PCs which are usually following the CALL
instruction. For those that aren't, it fixes up the PCs so that
functions processing the output can unconditionally decrement the PC.
runtime_expandInlineFrames does this unconditional decrement when
looking up the function. However, the fake stack frame generated for
overflow records fails to meet the contract, and decrementing the PC
results in a PC in the previous function. If that function contains
inlined call, runtime_expandInlineFrames will not short-circuit and will
panic trying to look up a PC that doesn't exist.
Note that the added test does not fail at HEAD. It will only fail (with
a panic) if the function preceeding lostProfileEvent contains inlined
function calls. At the moment (on linux/amd64), that is
runtime/pprof.addMaxRSS, which does not.
Fixes #38118
Change-Id: Iad0819f23c566011c920fd9a5b1254719228da0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/225661 Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 4a8b9bd2646a5b297197ffd1c412ef6afebe5c0d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/226077
Tobias Klauser [Sun, 29 Mar 2020 22:38:09 +0000 (00:38 +0200)]
[release-branch.go1.14] cmd/cgo, misc/cgo: only cache anonymous struct typedefs with parent name
CL 181857 broke the translation of certain C types using cmd/cgo -godefs
because it stores each typedef, array and qualified type with their
parent type name in the translation cache.
Fix this by only considering the parent type for typedefs of anonymous
structs which is the only case where types might become ambiguous.
Fixes #37622
Change-Id: I301a749ec89585789cb0d213593bb8b7341beb88
Reviewed-on: https://go-review.googlesource.com/c/go/+/226341
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit a265c2c448497fcee1633d2e2b912da52ea22d3c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/226497
[release-branch.go1.14] os/exec: use environment variables for user token when present
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 #35314
Fixes #37471
Change-Id: I4c1722e90b91945eb6980d5c5928183269b50487
GitHub-Last-Rev: 32216b7291418f9285147a93ed6d0ba028f94ef2
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>
(cherry picked from commit e0c3ded337e95ded40eb401e7d9e74716e3a445f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/226281
Michael Anthony Knyszek [Wed, 18 Mar 2020 15:09:40 +0000 (15:09 +0000)]
[release-branch.go1.14] runtime: ensure minTriggerRatio never exceeds maxTriggerRatio
Currently, the capping logic for the GC trigger ratio is such that if
gcpercent is low, we may end up setting the trigger ratio far too high,
breaking the promise of SetGCPercent and GOGC has a trade-off knob (we
won't start a GC early enough, and we will use more memory).
This change modifies the capping logic for the trigger ratio by scaling
the minTriggerRatio with gcpercent the same way we scale
maxTriggerRatio.
For #37927.
Fixes #37928.
Change-Id: I2a048c1808fb67186333d3d5a6bee328be2f35da
Reviewed-on: https://go-review.googlesource.com/c/go/+/223937
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
(cherry picked from commit d1ecfcc1e8baa0bb3a9fb504e8c14125a69139ba)
Reviewed-on: https://go-review.googlesource.com/c/go/+/225637 Reviewed-by: David Chase <drchase@google.com>
Bryan C. Mills [Thu, 26 Mar 2020 02:24:44 +0000 (22:24 -0400)]
[release-branch.go1.14] cmd/go: do not append to the global cfg.OrigEnv slice
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 #38083
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 bfb1342a40216cba0ff5ae3a1b102823b7603068)
Reviewed-on: https://go-review.googlesource.com/c/go/+/225659
Michael Anthony Knyszek [Wed, 25 Mar 2020 16:41:20 +0000 (16:41 +0000)]
[release-branch.go1.14] runtime: prevent preemption while timer is in timerModifying
Currently if a goroutine is preempted while owning a timer in the
timerModifying state, it could self-deadlock. When the goroutine is
preempted and calls into the scheduler, it could call checkTimers. If
checkTimers encounters the timerModifying timer and calls runtimer on
it, then runtimer will spin, waiting for that timer to leave the
timerModifying state, which it never will.
So far we got lucky that for the most part that there were no preemption
points while timerModifying is happening, however CL 221077 seems to
have introduced one, leading to sporadic self-deadlocks.
This change disables preemption explicitly while a goroutines holds a
timer in timerModifying. Since only checkTimers (and thus runtimer) is
called from the scheduler, this is sufficient to prevent
preemption-based self-deadlocks.
For #38070
Fixes #38072
Change-Id: Idbfac310889c92773023733ff7e2ff87e9896f0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit e8be350d78f3fd21b0fab4cc6909c03fe21f1640)
Reviewed-on: https://go-review.googlesource.com/c/go/+/225521
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Alex Brainman [Sun, 16 Feb 2020 01:01:02 +0000 (12:01 +1100)]
[release-branch.go1.14] runtime: ignore error returned by PowerRegisterSuspendResumeNotification
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 #37149
Fixes #37699
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 d467f3bbc9c76805ae16ab1924c28ec3be487875)
Reviewed-on: https://go-review.googlesource.com/c/go/+/224586
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Dan Scales [Fri, 20 Mar 2020 16:31:20 +0000 (09:31 -0700)]
[release-branch.go1.14] runtime: fix code so defer record is not added to g0 defer list during panic
newdefer() actually adds the new defer to the current g's defer chain. That
happens even if we are on the system stack, in which case the g will be the g0
stack. For open-coded defers, we call newdefer() (only during panic processing)
while on the system stack, so the new defer is unintentionally added to the
g0._defer defer list. The code later correctly adds the defer to the user g's
defer list.
The g0._defer list is never used. However, that pointer on the g0._defer list can
keep a defer struct alive that is intended to be garbage-collected (smaller defers
use a defer pool, but larger-sized defer records are just GC'ed). freedefer() does
not zero out pointers when it intends that a defer become garbage-collected. So,
we can have the pointers in a defer that is held alive by g0._defer become invalid
(in particular d.link). This is the cause of the bad pointer bug in this issue
The fix is to change newdefer (only used in two places) to not add the new defer
to the gp._defer list. We just do it after the call with the correct gp pointer.
(As mentioned above, this code was already there after the newdefer in
addOneOpenDeferFrame.) That ensures that defers will be correctly
garbage-collected and eliminate the bad pointer.
This fix definitely fixes the original repro. I added a test and tried hard to
reproduce the bug (based on the original repro code), but awasn't actually able to
cause the bug. However, the test is still an interesting mix of heap-allocated,
stack-allocated, and open-coded defers.
For #37688
Fixes #37968
Fixes #37688
Change-Id: I1a481b9d9e9b9ba4e8726ef718a1f4512a2d6faf
Reviewed-on: https://go-review.googlesource.com/c/go/+/224581
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 825ae71e567593d3a28b7dddede8745701273c52)
Reviewed-on: https://go-review.googlesource.com/c/go/+/225279
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dan Scales <danscales@google.com>
Cherry Zhang [Tue, 17 Mar 2020 00:08:00 +0000 (20:08 -0400)]
[release-branch.go1.14] runtime: don't send preemption signal if there is a signal pending
If multiple threads call preemptone to preempt the same M, it may
send many signals to the same M such that it hardly make
progress, causing live-lock problem. Only send a signal if there
isn't already one pending.
Ian Lance Taylor [Thu, 12 Mar 2020 04:51:09 +0000 (21:51 -0700)]
[release-branch.go1.14] runtime: don't crash on mlock failure
Instead, note that mlock has failed, start trying the mitigation of
touching the signal stack before sending a preemption signal, and,
if the program crashes, mention the possible problem and a wiki page
describing the issue (https://golang.org/wiki/LinuxKernelSignalVectorBug).
Tested on a kernel in the buggy version range, but with the patch,
by using `ulimit -l 0`.
For #37436
Fixes #37807
Change-Id: I072aadb2101496dffd655e442fa5c367dad46ce8
Reviewed-on: https://go-review.googlesource.com/c/go/+/223121
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit b851e51160bc8ed412e229152b430b75e7ce56f9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/223417
TryBot-Result: Gobot Gobot <gobot@golang.org>
Dan Scales [Thu, 5 Mar 2020 20:46:04 +0000 (12:46 -0800)]
[release-branch.go1.14] runtime: fix problem with repeated panic/recover/re-panics and open-coded defers
In the open-code defer implementation, we add defer struct entries to the defer
chain on-the-fly at panic time to represent stack frames that contain open-coded
defers. This allows us to process non-open-coded and open-coded defers in the
correct order. Also, we need somewhere to be able to store the 'started' state of
open-coded defers. However, if a recover succeeds, defers will now be processed
inline again (unless another panic happens). Any defer entry representing a frame
with open-coded defers will become stale once we run the corresponding defers
inline and exit the associated stack frame. So, we need to remove all entries for
open-coded defers at recover time.
The current code was only removing the top-most open-coded defer from the defer
chain during recovery. However, with recursive functions that do repeated
panic-recover-repanic, multiple stale entries can accumulate on the chain. So, we
just adjust the loop to process the entire chain. Since this is at panic/recover
case, it is fine to scan through the entire chain (which should usually have few
elements in it, since most defers are open-coded).
The added test fails with a SEGV without the fix, because it tries to run a stale
open-code defer entry (and the stack has changed).
Updates #37664.
Fixes #37782.
Change-Id: I8e3da5d610b5e607411451b66881dea887f7484d
Reviewed-on: https://go-review.googlesource.com/c/go/+/222420
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit fae87a2223e1fa959a20017742455200fe3c35f1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/222818
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Keith Randall [Fri, 6 Mar 2020 22:01:26 +0000 (14:01 -0800)]
[release-branch.go1.14] runtime: make typehash match compiler generated hashes exactly
If typehash (used by reflect) does not match the built-in map's hash,
then problems occur. If a map is built using reflect, and then
assigned to a variable of map type, the hash function can change. That
causes very bad things.
This issue is rare. MapOf consults a cache of all types that occur in
the binary before making a new one. To make a true new map type (with
a hash function derived from typehash) that map type must not occur in
the binary anywhere. But to cause the bug, we need a variable of that
type in order to assign to it. The only way to make that work is to
use a named map type for the variable, so it is distinct from the
unnamed version that MapOf looks for.
Michael Pratt [Fri, 28 Feb 2020 19:16:41 +0000 (14:16 -0500)]
[release-branch.go1.14] runtime/pprof: expand final stack frame to avoid truncation
When generating stacks, the runtime automatically expands inline
functions to inline all inline frames in the stack. However, due to the
stack size limit, the final frame may be truncated in the middle of
several inline frames at the same location.
As-is, we assume that the final frame is a normal function, and emit and
cache a Location for it. If we later receive a complete stack frame, we
will first use the cached Location for the inlined function and then
generate a new Location for the "caller" frame, in violation of the
pprof requirement to merge inlined functions into the same Location.
As a result, we:
1. Nondeterministically may generate a profile with the different stacks
combined or split, depending on which is encountered first. This is
particularly problematic when performing a diff of profiles.
2. When split stacks are generated, we lose the inlining information.
We avoid both of these problems by performing a second expansion of the
last stack frame to recover additional inline frames that may have been
lost. This expansion is a bit simpler than the one done by the runtime
because we don't have to handle skipping, and we know that the last
emitted frame is not an elided wrapper, since it by definition is
already included in the stack.
Fixes #37447
Change-Id: If3ca2af25b21d252cf457cc867dd932f107d4c61
Reviewed-on: https://go-review.googlesource.com/c/go/+/221577
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
(cherry picked from commit fadbf7404d2b1aca63993e289448fcc3b6a23107)
Reviewed-on: https://go-review.googlesource.com/c/go/+/222762
Jay Conrod [Thu, 5 Mar 2020 16:11:47 +0000 (11:11 -0500)]
[release-branch.go1.14] cmd/go: make go test -json report failures for panicking/exiting tests
'go test -json' should report that a test failed if the test binary
did not exit normally with status 0. This covers panics, non-zero
exits, and abnormal terminations.
These tests don't print a final result when run with -test.v (which is
used by 'go test -json'). The final result should be "PASS" or "FAIL"
on a line by itself. 'go test' prints "FAIL" in this case, but
includes error information.
test2json was changed in CL 192104 to report that a test passed if it
does not report a final status. This caused 'go test -json' to report
that a test passed after a panic or non-zero exit.
With this change, test2json treats "FAIL" with error information the
same as "FAIL" on a line by itself. This is intended to be a minimal
fix for backporting, but it will likely be replaced by a complete
solution for #29062.
Change-Id: I63dad8b554353197ae0f29fa2a84f17bffa58557
GitHub-Last-Rev: 5297df32200ea5b52b2e7b52c8ee022d37e44111
GitHub-Pull-Request: golang/go#37661
Reviewed-on: https://go-review.googlesource.com/c/go/+/222037 Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 2b0f481278cc093e9f61945592257e6d651a169c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/222317
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Hana (Hyang-Ah) Kim [Wed, 19 Feb 2020 03:41:20 +0000 (22:41 -0500)]
[release-branch.go1.14] cmd/trace: update to use WebComponents V0 polyfill
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 #37343.
Change-Id: I0f369b15349dd0f4718c261ec23dfab6a47ace2f
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 75ea964b3f6073076e1a86a0de2be9a2f159da24)
Reviewed-on: https://go-review.googlesource.com/c/go/+/220323
As far as I can tell, there is no public documentation on this topic,
which cost me several days of debugging.
I am possibly unusual in that I run binaries in production with the
race detector turned on, but I think that others who do the same may
want to be aware of the risk.
Updates #26813.
Updates #37233.
Change-Id: I1f8111bd01d0000596e6057b7cb5ed017d5dc655
Reviewed-on: https://go-review.googlesource.com/c/go/+/220586 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
(cherry picked from commit ba093c4562e7464e95a4bde6505d270b71ed623f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/221019
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
vovapi [Thu, 20 Feb 2020 19:29:22 +0000 (19:29 +0000)]
[release-branch.go1.14] hash/maphash: don't discard data on random seed init
Hash initializes seed on the first usage of seed or state with initSeed.
initSeed uses SetSeed which discards accumulated data.
This causes hash to return different sums for the same data in the first use
and after reset.
This CL fixes this issue by separating the seed set from data discard.
Dmitri Shuralyov [Fri, 21 Feb 2020 19:24:31 +0000 (14:24 -0500)]
[release-branch.go1.14] CONTRIBUTORS: second round of updates for Go 1.14
This update was automatically generated using the updatecontrib command:
cd gotip
go run golang.org/x/build/cmd/updatecontrib
With minor manual changes based on publicly available information
to canonicalize letter case and formatting for a few names.
Actions taken (relative to CONTRIBUTORS at origin/master):
Added Aaron Bieber <deftly@gmail.com>
Added Adam Williams <pwnfactory@gmail.com>
Added Ayke van Laethem <aykevanlaethem@gmail.com>
Added Bradford Lamson-Scribner <brad.lamson@gmail.com>
Added Brian Falk <falk@logicparty.org>
Added Chen Zhihan <energiehund@gmail.com>
Added Christopher Loessl <cloessl+github@gmail.com>
Added Frederik Zipp <fzipp@gmx.de>
Added Fujimoto Kyosuke <kyoro.f@gmail.com>
Added GitHub User jopbrown (6345470) <msshane2008@gmail.com>
Added GitHub User yah01 (12216890) <kagaminehuan@gmail.com>
Added Hiromichi Ema <ema.hiro@gmail.com>
Added Jamal Carvalho <jamal.a.carvalho@gmail.com>
Added Jason Baker <jason-baker@users.noreply.github.com>
Added Kanta Ebihara <kantaebihara@gmail.com>
Added Kirill Tatchihin <kirabsuir@gmail.com>
Added Kévin Dunglas <dunglas@gmail.com>
Added Mariano Cano <mariano@smallstep.com>
Added Sergey Ivanov <ser1325@gmail.com>
Added Thomas Symborski <thomas.symborski@gmail.com>
Added Tomohiro Kusumoto <zabio1192@gmail.com>
Added Xingqang Bai <bxq2011hust@qq.com>
Used GitHub User jopbrown (6345470) form for jopbrown <msshane2008@gmail.com> https://github.com/golang/exp/commit/0405dc7 [exp]
Used GitHub User yah01 (12216890) form for yah01 <kagaminehuan@gmail.com> https://github.com/golang/go/commit/ee55dd6b64 [go]
Used GitHub name "Hiromichi Ema" for emahiro <ema.hiro@gmail.com> https://github.com/golang/tools/commit/b6336cbc [tools]
Used GitHub name "Jamal Carvalho" for Gopher <jamal.a.carvalho@gmail.com> https://github.com/golang/gddo/commit/31dd61d [gddo]
Used GitHub name "Xingqang Bai" for bxq2011hust <bxq2011hust@qq.com> https://github.com/golang/go/commit/79ccbe1b67 [go]
Updates #12042
Change-Id: I13f8ab37f8b38f8f5d0ff71c939ad39d0bc4f985
Reviewed-on: https://go-review.googlesource.com/c/go/+/220363 Reviewed-by: Alexander Rakoczy <alex@golang.org>
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/220368
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitri Shuralyov [Fri, 21 Feb 2020 20:16:40 +0000 (15:16 -0500)]
[release-branch.go1.14] all: merge master into release-branch.go1.14
1cd724acb6 doc/go1.14: highlight the addition of hash/maphash package a0cf2c872f doc/go1.14: remove TODO comment for CL 200439 a9ea91d571 cmd/link, runtime: skip holes in func table 88e564edb1 doc/go1.14: add missing period at sentence end 6917529cc6 testing: remove obsolete comment in testing.(*T) docs
Dmitri Shuralyov [Tue, 18 Feb 2020 15:03:22 +0000 (10:03 -0500)]
doc/go1.14: highlight the addition of hash/maphash package
Given that it's a package that did not exist before, was a proposal
in issue #28322, got accepted and implemented for 1.14, it seems to
be more than a minor change to the library. Highlight it accordingly.
Also specify the results are 64-bit integers, as done in CL 219340.
Updates #36878
Updates #28322
Change-Id: Idefe63d4c47a02cdcf8be8ab08c40cdb94ff2098
Reviewed-on: https://go-review.googlesource.com/c/go/+/219877
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Dmitri Shuralyov [Tue, 18 Feb 2020 18:28:35 +0000 (18:28 +0000)]
doc/go1.14: remove TODO comment for CL 200439
Based on https://golang.org/issue/36878#issuecomment-587533153
and https://golang.org/issue/36878#issuecomment-587549692,
this is not a CL that needs to be mentioned in the release notes.
Updates #36878
Change-Id: Icaa9153da7481a1d3ebabc237411539dd770cef2
Reviewed-on: https://go-review.googlesource.com/c/go/+/219898
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Sun, 16 Feb 2020 21:18:04 +0000 (16:18 -0500)]
cmd/link, runtime: skip holes in func table
On PPC64 when external linking, for large binaries we split the
text section to multiple sections, so the external linking may
insert trampolines between sections. These trampolines are within
the address range covered by the func table, but not known by Go.
This causes runtime.findfunc to return a wrong function if the
given PC is from such trampolines.
In this CL, we generate a marker between text sections where
there could potentially be a hole in the func table. At run time,
we skip the hole if we see such a marker.
Fixes #37216.
Change-Id: I95ab3875a84b357dbaa65a4ed339a19282257ce0
Reviewed-on: https://go-review.googlesource.com/c/go/+/219717 Reviewed-by: David Chase <drchase@google.com>
Dmitri Shuralyov [Thu, 13 Feb 2020 19:58:56 +0000 (14:58 -0500)]
[release-branch.go1.14] all: merge master into release-branch.go1.14
3eab754cd0 runtime: correct caller PC/SP offsets in walltime1/nanotime1 123f7dd3e1 runtime: zero upper bit of Y registers in asyncPreempt on darwin/amd64 a0c9fb6bd3 hash/maphash: mention the results are 64-bit integers e237df5b53 runtime: fix fallback logic for aeshash on 32/64 bit 363bcd0068 cmd/go: eliminate empty '()' when passing -mod=readonly explicitly to 'go list' 7385947825 cmd/go/internal/modcmd: remove dead function addModFlag 5ce8005990 cmd/go/internal/web: fix a typo d0050e2871 go/build: populate partial package information in importGo 1c241d2879 hash/maphash: mention that hash values do not persist in package docs 25da21ddc9 crypto/elliptic: document the Name and names of each curve ab5d9f5831 doc/go1.14: add a couple minor crypto release notes 6a8164a254 go/doc: clarify that NewFromFiles caller must filter by GOOS/GOARCH dff55c1f76 doc: move doc/modules.md to x/website cfe2ab42e7 doc/go1.14: rearrange in alphabetical order ca8bf63809 doc/go1.14: add link to TempFile in io/ioutil a528215693 doc/go1.14: fix inconsistent markup a6b03c64b2 runtime/race: update reference to compiler-rt sources 60d437f994 runtime: avoid double notewakeup in netpoll stub code b8061825e5 doc: fill in 'go mod init' section of module documentation cb16d26bd6 doc: fill in 'go mod download' section of module documentation 08d41dbb10 doc: fill in 'go list -m' section in module documentation ff091b5fa0 doc: fill in 'Module-aware commands' section in module documentation c7c525a79d doc: add section on module paths to module documentation 153a9e8033 doc: add section on go.mod file syntax 1a37095062 Revert "cmd/link: code cleanup in macho_combine_dwarf.go" 494dd1dddc cmd/link: code cleanup in macho_combine_dwarf.go
Cherry Zhang [Tue, 11 Feb 2020 20:09:39 +0000 (15:09 -0500)]
runtime: correct caller PC/SP offsets in walltime1/nanotime1
In walltime1/nanotime1, we save the caller's PC and SP for stack
unwinding. The code does that assumed zero frame size. Now that
the frame size is not zero, correct the offset. Rewrite it in a
way that doesn't depend on hard-coded frame size.
Cherry Zhang [Tue, 11 Feb 2020 23:54:30 +0000 (18:54 -0500)]
runtime: zero upper bit of Y registers in asyncPreempt on darwin/amd64
Apparently, the signal handling code path in darwin kernel leaves
the upper bits of Y registers in a dirty state, which causes many
SSE operations (128-bit and narrower) become much slower. Clear
the upper bits to get to a clean state.
We do it at the entry of asyncPreempt, which is immediately
following exiting from the kernel's signal handling code, if we
actually injected a call. It does not cover other exits where we
don't inject a call, e.g. failed preemption, profiling signal, or
other async signals. But it does cover an important use case of
async signals, preempting a tight numerical loop, which we
introduced in this cycle.
Running the benchmark in issue #37174:
name old time/op new time/op delta
Fast-8 90.0ns ± 1% 46.8ns ± 3% -47.97% (p=0.000 n=10+10)
Slow-8 188ns ± 5% 49ns ± 1% -73.82% (p=0.000 n=10+9)
There is no more slowdown due to preemption signals.
Kanta Ebihara [Tue, 11 Feb 2020 23:17:30 +0000 (08:17 +0900)]
cmd/go/internal/web: fix a typo
dependenicies -> dependencies
Change-Id: I0b8f06c04cf397c6330ffb43ac3ae5c2f7cf3138
Reviewed-on: https://go-review.googlesource.com/c/go/+/219157 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitri Shuralyov [Mon, 10 Feb 2020 00:51:22 +0000 (19:51 -0500)]
go/build: populate partial package information in importGo
This is a followup to CL 199840 and CL 203820. Cumulatively, they caused
a previously known bug to trigger more often while also nearly fixing it.
This change is a small fixup to CL 199840 that resolves the known bug
and prevents it from causing an additional regression in Go 1.14.
Part 1
The intention in CL 199840 was to return the same error that 'go list'
reported when the package wasn't located, so an early return was added.
However, to determine whether the package was located or not, p.Dir was
unintentionally checked instead of dir.
p is initialized to &Package{ImportPath: path} at top of Context.Import,
and its Dir field is never set before that line in importGo is reached.
So return errors.New(errStr) was always executed whenever errStr != "".
Originally, in CL 125296, the "go list" invocation did not include an
'-e' flag, so it would return a non-zero exit code on packages where
build constraints exclude all Go files, and importGo would return an
error like "go/build: importGo import/path: unexpected output: ...".
CL 199840 added an '-e' flag to the "go list" invocation, but checking
the wrong dir variable caused partial package information to never get
populated, and thus issue #31603 continued to occur, although with a
different error message (which ironically included the location of the
package that was supposedly "not found").
Now that the right dir is checked, issue #31603 is fixed.
Part 2
importGo checks whether it can use the go command to find the directory
of a package. In Go 1.13.x and earlier, one of the conditions to use the
go command was that the source directory must be provided.
CL 203820 made a change such that knowing the source directory was
no longer required:
// To invoke the go command,
-// we must know the source directory,
// ...
That meant build.Import invocations where srcDir is the empty string:
build.Import(path, "", build.FindOnly)
Started using the go command to find the directory of the package, and
started to run into issue #31603 as well. That's the #37153 regression.
Since this change fixes issue #31603, it also fixes issue #37153.
Part 3
There is one more thing. Delete the debugImportGo constant, it's unused.
Filippo Valsorda [Mon, 10 Feb 2020 19:48:58 +0000 (14:48 -0500)]
doc/go1.14: add a couple minor crypto release notes
These were left out of CL 216759 because they are trivial, but I was
advised to be thorough.
Updates #36878
Change-Id: Id4fd3a84866a82265e3f89abfdad6e3d231b507c
Reviewed-on: https://go-review.googlesource.com/c/go/+/218918 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dmitri Shuralyov [Fri, 7 Feb 2020 16:15:11 +0000 (11:15 -0500)]
go/doc: clarify that NewFromFiles caller must filter by GOOS/GOARCH
The most well known and important build constraints to take into
account when rendering package documentation are the GOOS/GOARCH
values. Make it more clear in the NewFromFiles documentation that
they are a part of all build constraints that the caller is
responsible for filtering out.
Also suggest the "go/build".Context.MatchFile method for performing
file matching. The logic to perform build context file matching is
subtle and has many rules that aren't well known (for example,
taking the gc or gccgo compiler into account). It is currently the
only exported API in the standard library that implements this logic,
and it would be unfortunate if people attempt to re-create it because
they don't realize it is already available.
Updates #23864
Change-Id: I3c5901e7081acf79125b2d429ec3aa3b58416ed7
Reviewed-on: https://go-review.googlesource.com/c/go/+/218477 Reviewed-by: Robert Griesemer <gri@golang.org>