1
0
mirror of https://github.com/golang/go synced 2024-11-17 12:14:47 -07:00
Commit Graph

44134 Commits

Author SHA1 Message Date
Xiangdong Ji
e8f5a33191 cmd/compile: fix incorrect rewriting to if condition
Some ARM64 rewriting rules convert 'comparing to zero' conditions of if
statements to a simplified version utilizing CMN and CMP instructions to
branch over condition flags, in order to save one Add or Sub caculation.

Such optimizations lead to wrong branching in case an overflow/underflow
occurs when executing CMN or CMP.

Fix the issue by introducing new block opcodes that don't honor the
overflow/underflow flag, in the following categories:

  Block-Op        Meaning                   ARM condition codes
  1. LTnoov        less than                 MI
  2. GEnoov        greater than or equal     PL
  3. LEnoov        less than or equal        MI || EQ
  4. GTnoov        greater than              NEQ & PL

The backend generates two consecutive branch instructions for 'LEnoov'
and 'GTnoov' to model their expected behavior. A slight change to 'gc'
and amd64/386 backends is made to unify the code generation.

Add a test 'TestCondRewrite' as justification, it covers 32 incorrect rules
identified on arm64, more might be needed on other arches, like 32-bit arm.

Add two benchmarks profiling the aforementioned category 1&2 and category
3&4 separetely, we expect the first two categories will show performance
improvement and the second will not result in visible regression compared with
the non-optimized version.

This change also updates TestFormats to support using %#x.

Examples exhibiting where does the issue come from:
  1: 'if x + 3 < 0' might be converted to:
  before:
    CMN $3, R0
    BGE <else branch> // wrong branch is taken if 'x+3' overflows
  after:
    CMN $3, R0
    BPL <else branch>

  2: 'if y - 3 > 0' might be converted to:
  before:
    CMP $3, R0
    BLE <else branch> // wrong branch is taken if 'y-3' underflows
  after:
    CMP $3, R0
    BMI <else branch>
    BEQ <else branch>

Benchmark data from different kinds of arm64 servers, 'old' is the non-optimized
version (not the parent commit), generally the optimization version outperforms.

S1:
name                    old time/op  new time/op  delta
CondRewrite/SoloJump  13.6ns ± 0%  12.9ns ± 0%  -5.15%  (p=0.000 n=10+10)
CondRewrite/CombJump  13.8ns ± 1%  12.9ns ± 0%  -6.32%  (p=0.000 n=10+10)

S2:
name                     old time/op  new time/op  delta
CondRewrite/SoloJump  11.6ns ± 0%  10.9ns ± 0%  -6.03%  (p=0.000 n=10+10)
CondRewrite/CombJump  11.4ns ± 0%  10.8ns ± 1%  -5.53%  (p=0.000 n=10+10)

S3:
name                     old time/op  new time/op  delta
CondRewrite/SoloJump  7.36ns ± 0%  7.50ns ± 0%  +1.79%  (p=0.000 n=9+10)
CondRewrite/CombJump  7.35ns ± 0%  7.75ns ± 0%  +5.51%  (p=0.000 n=8+9)

S4:
name                      old time/op  new time/op  delta
CondRewrite/SoloJump-224  11.5ns ± 1%  10.9ns ± 0%  -4.97%  (p=0.000 n=10+10)
CondRewrite/CombJump-224  11.9ns ± 0%  11.5ns ± 0%  -2.95%  (p=0.000 n=10+10)

S5:
name                     old time/op  new time/op  delta
CondRewrite/SoloJump  10.0ns ± 0%  10.0ns ± 0%  -0.45%  (p=0.000 n=9+10)
CondRewrite/CombJump  9.93ns ± 0%  9.77ns ± 0%  -1.53%  (p=0.000 n=10+9)

Go1 perf. data:

name                     old time/op    new time/op    delta
BinaryTree17              6.29s ± 1%     6.30s ± 1%    ~     (p=1.000 n=5+5)
Fannkuch11                5.40s ± 0%     5.40s ± 0%    ~     (p=0.841 n=5+5)
FmtFprintfEmpty          97.9ns ± 0%    98.9ns ± 3%    ~     (p=0.937 n=4+5)
FmtFprintfString          171ns ± 3%     171ns ± 2%    ~     (p=0.754 n=5+5)
FmtFprintfInt             212ns ± 0%     217ns ± 6%  +2.55%  (p=0.008 n=5+5)
FmtFprintfIntInt          296ns ± 1%     297ns ± 2%    ~     (p=0.516 n=5+5)
FmtFprintfPrefixedInt     371ns ± 2%     374ns ± 7%    ~     (p=1.000 n=5+5)
FmtFprintfFloat           435ns ± 1%     439ns ± 2%    ~     (p=0.056 n=5+5)
FmtManyArgs              1.37µs ± 1%    1.36µs ± 1%    ~     (p=0.730 n=5+5)
GobDecode                14.6ms ± 4%    14.4ms ± 4%    ~     (p=0.690 n=5+5)
GobEncode                11.8ms ±20%    11.6ms ±15%    ~     (p=1.000 n=5+5)
Gzip                      507ms ± 0%     491ms ± 0%  -3.22%  (p=0.008 n=5+5)
Gunzip                   73.8ms ± 0%    73.9ms ± 0%    ~     (p=0.690 n=5+5)
HTTPClientServer          116µs ± 0%     116µs ± 0%    ~     (p=0.686 n=4+4)
JSONEncode               21.8ms ± 1%    21.6ms ± 2%    ~     (p=0.151 n=5+5)
JSONDecode                104ms ± 1%     103ms ± 1%  -1.08%  (p=0.016 n=5+5)
Mandelbrot200            9.53ms ± 0%    9.53ms ± 0%    ~     (p=0.421 n=5+5)
GoParse                  7.55ms ± 1%    7.51ms ± 1%    ~     (p=0.151 n=5+5)
RegexpMatchEasy0_32       158ns ± 0%     158ns ± 0%    ~     (all equal)
RegexpMatchEasy0_1K       606ns ± 1%     608ns ± 3%    ~     (p=0.937 n=5+5)
RegexpMatchEasy1_32       143ns ± 0%     144ns ± 1%    ~     (p=0.095 n=5+4)
RegexpMatchEasy1_1K       927ns ± 2%     944ns ± 2%    ~     (p=0.056 n=5+5)
RegexpMatchMedium_32     16.0ns ± 0%    16.0ns ± 0%    ~     (all equal)
RegexpMatchMedium_1K     69.3µs ± 2%    69.7µs ± 0%    ~     (p=0.690 n=5+5)
RegexpMatchHard_32       3.73µs ± 0%    3.73µs ± 1%    ~     (p=0.984 n=5+5)
RegexpMatchHard_1K        111µs ± 1%     110µs ± 0%    ~     (p=0.151 n=5+5)
Revcomp                   1.91s ±47%     1.77s ±68%    ~     (p=1.000 n=5+5)
Template                  138ms ± 1%     138ms ± 1%    ~     (p=1.000 n=5+5)
TimeParse                 787ns ± 2%     785ns ± 1%    ~     (p=0.540 n=5+5)
TimeFormat                729ns ± 1%     726ns ± 1%    ~     (p=0.151 n=5+5)

Updates #38740
Change-Id: I06c604874acdc1e63e66452dadee5df053045222
Reviewed-on: https://go-review.googlesource.com/c/go/+/233097
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
2020-05-29 15:39:54 +00:00
Alberto Donizetti
65f514edfb math: fix dead link to springerlink (now link.springer)
Change-Id: Ie5fd026af45d2e7bc371a38d15dbb52a1b4958cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/235717
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-05-29 14:33:50 +00:00
Volker Dobler
1519bc4457 net/http: clarify that AddCookie only sanitizes the Cookie being added
AddCookie properly encodes a cookie and appends it to the Cookie header
field but does not modify or sanitize what the Cookie header field
contains already. If a user manualy sets the Cookie header field to
something not conforming to RFC 6265 then a cookie added via AddCookie
might not be retrievable.

Fixes #38437

Change-Id: I232b64ac489b39bb962fe4f7dbdc2ae44fcc0514
Reviewed-on: https://go-review.googlesource.com/c/go/+/235141
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-29 09:21:54 +00:00
Daniel Martí
8f4151ea67 encoding/xml: only initialize nil struct fields when decoding
fieldInfo.value used to initialize nil anonymous struct fields if they
were encountered. This behavior is wanted when decoding, but not when
encoding. When encoding, the value should never be modified, and these
nil fields should be skipped entirely.

To fix the bug, add a bool argument to the function which tells the
code whether we are encoding or decoding.

Finally, add a couple of tests to cover the edge cases pointed out in
the original issue.

Fixes #27240.

Change-Id: Ic97ae4bfe5f2062c8518e03d1dec07c3875e18f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/196809
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-05-28 22:48:53 +00:00
Daniel Martí
107ebb1781 Revert "encoding/json: reuse values when decoding map elements"
This reverts golang.org/cl/179337.

Reason for revert: broke a few too many reasonably valid Go programs.
The previous behavior was perhaps less consistent, but the docs were
never very clear about when the decoder merges with existing values,
versus replacing existing values altogether.

Fixes #39149.

Change-Id: I1c1d857709b8398969fe421aa962f6b62f91763a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234559
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
2020-05-28 22:17:10 +00:00
Bryan C. Mills
86ed0955bf os: in Symlink, stat the correct target path for drive-relative targets on Windows
Previously, when the target (“old”) path passed to os.Symlink was a
“root-relative” Windows path,¹ we would erroneously prepend
destination (“new”) path when determining which path to Stat,
resulting in an invalid path which was then masked by the lack of
error propagation for the Stat call (#39183).

If the link target is a directory (rather than a file), that would
result in the symlink being created without the
SYMBOLIC_LINK_FLAG_DIRECTORY flag, which then fails in os.Open.

¹https://docs.microsoft.com/en-us/windows/win32/fileio/creating-symbolic-links

Updates #39183

Change-Id: I04f179cd2b0c44f984f34ec330acad2408aa3a20
Reviewed-on: https://go-review.googlesource.com/c/go/+/235317
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-28 21:41:10 +00:00
Dmitri Shuralyov
5a00adf1ea doc/go1.15: document no language changes
There are no language changes in Go 1.15, so document that.

For #37419.

Change-Id: I1e96e58b701f1758d64c79881dfa0b1109836b83
Reviewed-on: https://go-review.googlesource.com/c/go/+/235580
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-05-28 21:13:51 +00:00
Elias Naur
2711127974 syscall: avoid dup2 in forkAndExecInChild1 on Android
Android O and newer blocks the dup2 syscall.

Change-Id: Ibca01fc72ef114deeef6c0450a8b81a556ed0530
Reviewed-on: https://go-review.googlesource.com/c/go/+/235537
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-28 19:51:59 +00:00
Cherry Zhang
6bf2eea62a cmd/compile: always use StackMapDontCare as register map index when reg map is not used
When go115ReduceLiveness is true (so we don't emit actual
register maps), use StackMapDontCare consistently for the
register map index, so RegMapValid is always false.

This fixes a compiler crash when doing -live=2 debug print.

Fixes #39251.

Change-Id: Ice087af491fa69c413f8ee59f923b72d592c0643
Reviewed-on: https://go-review.googlesource.com/c/go/+/235418
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2020-05-28 14:43:19 +00:00
Bryan C. Mills
0d20a49282 run.bat: use ..\bin\go instead of "go" to install std and cmd
The paths for the other "go" commands in this file were fixed in CL 223741,
but this one was missed (and run.bat is not caught by the builders).

Change-Id: Iba1efddc7d2fbe6af39c39d643508decc954bbc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/234758
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2020-05-27 19:54:12 +00:00
Russ Cox
e3491c4603 net/http: handle body rewind in HTTP/2 connection loss better
In certain cases the HTTP/2 stack needs to resend a request.
It obtains a fresh body to send by calling req.GetBody.
This call was missing from the path where the HTTP/2
round tripper returns ErrSkipAltProtocol, meaning fall back
to HTTP/1.1. The result was that the HTTP/1.1 fallback
request was sent with no body at all.

This CL changes that code path to rewind the body before
falling back to HTTP/1.1. But rewinding the body is easier
said than done. Some requests have no GetBody function,
meaning the body can't be rewound. If we need to rewind and
can't, that's an error. But if we didn't read anything, we don't
need to rewind. So we have to track whether we read anything,
with a new ReadCloser wrapper. That in turn requires adding
to the couple places that unwrap Body values to look at the
underlying implementation.

This CL adds the new rewinding code in the main retry loop
as well.

The new rewindBody function also takes care of closing the
old body before abandoning it. That was missing in the old
rewind code.

Thanks to Aleksandr Razumov for CL 210123
and to Jun Chen for CL 234358, both of which informed
this CL.

Fixes #32441.

Change-Id: Id183758526c087c6b179ab73cf3b61ed23a2a46a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234894
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-27 16:56:56 +00:00
Michael Pratt
748533e3a1 runtime: check gdb exit status and log output
All GDB tests currently ignore non-zero exit statuses. When tests
flakes, we don't even know if GDB exited successfully or not.

Add checks for non-zero exits, which are not expected.

Furthermore, always log the output from GDB. The tests are currently
inconsistent about whether they always log, or only on error.

Updates #39021

Change-Id: I7af1d795fc2fdf58093cb2731d616d4aa44e9996
Reviewed-on: https://go-review.googlesource.com/c/go/+/235282
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-27 15:53:56 +00:00
Cherry Zhang
902d8de79e cmd/link: actually close the output file
When the output file is mmap'd, OutBuf.Close currently munmap the
file but doesn't actually close the file descriptor. This CL
makes it actually close the FD.

Change-Id: I053c5592ae95497228c50ce6a267b3b48f0af6d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/235417
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
2020-05-27 15:26:40 +00:00
Paschalis Tsilias
c0e8e405c0 cmd/go: clean -cache -n should not delete cache
Uses the `cfg.BuildN` flag to avoid deleting inside the `if cleanCache`
block. Introduces a test in src/cmd/go/testdata/script.

Fixes #39250

Change-Id: I857c441b1d7aa7c68cfd646d6833e6eaca5b18d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/235140
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2020-05-27 13:37:56 +00:00
Jay Conrod
b2ce3931d8 cmd/go: report error for empty GOPROXY list
If GOPROXY is "", we set it to the default value,
"https://proxy.golang.org,direct". However, if GOPROXY is a non-empty
string that doesn't contain any URLs or keywords, we treat it as
either "off" or "noproxy", which can lead to some strange errors.

This change reports an error for this kind of GOPROXY value.

For #39180

Change-Id: If2e6e39d6f74c708e5ec8f90e9d4880e0e91894f
Reviewed-on: https://go-review.googlesource.com/c/go/+/234857
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-05-26 22:28:45 +00:00
Tobias Klauser
4abec2a480 runtime, time: gofmt
Change-Id: Ib36a5f239db5af497aae122eba049c15d0d4c4a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/235139
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-26 22:06:26 +00:00
Katie Hockman
b5bf2f0682 crypto/x509: allow setting MaxPathLen to -1 without IsCA
This fixes a bug in CL 228777 which disallowed
a MaxPathLen of -1 without IsCA, even though the
x509.Certificate documentation indicates that
MaxPathLen of -1 is considered "unset".

Updates #38216

Change-Id: Ib7240e00408d060f27567be8b820d0eee239256f
Reviewed-on: https://go-review.googlesource.com/c/go/+/235280
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2020-05-26 21:29:55 +00:00
Paul D. Weber
e5a6a94aeb cmd/link/internal/ld/lib.go: use lld on Android
Set linker explicitly to lld because the default does not work on NDK
versions r19c, r20, r20b and r21. NDK 18b (or earlier) based builds
will need to specify -fuse-ld=gold.

Fixes #38838

Change-Id: Ib75f71fb9896b843910f41bd12aa1e36868fa9b3
GitHub-Last-Rev: eeaa171604
GitHub-Pull-Request: golang/go#39217
Reviewed-on: https://go-review.googlesource.com/c/go/+/235017
Reviewed-by: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-26 19:10:28 +00:00
Joel Sing
20160b37c6 runtime, syscall: correct openbsd/arm and openbsd/arm64 syscalls for OpenBSD 6.7
Add two no op instructions following svc on openbsd/arm64 and swi on openbsd/arm.

All except some of the most recent arm64 processors have a speculative execution
flaw that occurs across a syscall boundary, which cannot be mitigated in the
kernel. In order to protect against this leak a speculation barrier needs to be
placed after an svc or swi instruction.

In order to avoid the performance impact of these instructions, the OpenBSD 6.7
kernel returns execution two instructions past the svc or swi call. For now two
hardware no ops are added, which allows syscalls to work with both 6.6 and 6.7.
These should be replaced with real speculation barriers once OpenBSD 6.8 is
released.

Updates #36435

Change-Id: I06153cb0998199242cca8761450e53599c3e7de4
Reviewed-on: https://go-review.googlesource.com/c/go/+/234381
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-26 17:02:46 +00:00
Elias Naur
bcda68447b cmd/link/internal/ld: consider alternative linkers in linkerFlagSupported
CL 235017 is about to change the default Android linker to lld. lld doesn't
support the --compress-debug-sections=zlib-gnu flag, but linkerFlagSupported
doesn't take any alternative linkers specified with -fuse-ld into account.

Updates #38838

Change-Id: I5f7422c06d40dedde2e4b070fc48398e8f822190
Reviewed-on: https://go-review.googlesource.com/c/go/+/235157
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-25 21:01:14 +00:00
Tobias Klauser
f65ad0dda7 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.

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>
2020-05-24 20:49:55 +00:00
Richard Musiol
828bb0c123 syscall/js: improve documentation of Func.Release
Fixes #38152

Change-Id: I807f49e23cc33e1c9b64029c7504b5a1f81a6bab
Reviewed-on: https://go-review.googlesource.com/c/go/+/235138
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2020-05-24 17:46:52 +00:00
matsuyoshi
8194187a2d os: use same link in UserCacheDir/UserConfigDir doc
Change-Id: I94c385243c37589f56aadaa30336b400adf31308
Reviewed-on: https://go-review.googlesource.com/c/go/+/234959
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-24 01:19:35 +00:00
Roland Shoemaker
b68fa57c59 encoding/asn1: document what Unmarshal returns in rest
Specifically, this change documents the behavior of Unmarshal when a
SEQUENCE contains trailing elements.

For context Unmarshal treats trailing elements of a SEQUENCE that do not
have matching struct fields as valid, as this is how ASN.1 structures
are typically extended. This can be somewhat confusing as you might
expect those elements to be appended to rest, but rest is really only
for trailing data unrelated to the structure being parsed (i.e. if you
append a second sequence to b, it would be returned in rest).

Fixes #35680

Change-Id: Ia2c68b2f7d8674d09e859b4b7f9aff327da26fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/233537
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2020-05-23 06:47:06 +00:00
Russ Cox
9acdc705e7 time: simplify Duration.String example
The existing example is needlessly complex.
You have to know that t.Sub returns a Duration
and also have to mentally subtract the two times
to understand what duration should be printed.

Rewrite to focus on just the Duration.String operation.

Change-Id: I00765b6019c07a6ff03022625b556c2b9ba87c09
Reviewed-on: https://go-review.googlesource.com/c/go/+/234893
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-05-22 11:28:55 +00:00
Austin Clements
ea2de3346f runtime: detect and report zombie slots during sweeping
A zombie slot is a slot that is marked, but isn't allocated. This can
indicate a bug in the GC, or a bad use of unsafe.Pointer. Currently,
the sweeper has best-effort detection for zombie slots: if there are
more marked slots than allocated slots, then there must have been a
zombie slot. However, this is imprecise since it only compares totals
and it reports almost no information that may be helpful to debug the
issue.

Add a precise check that compares the mark and allocation bitmaps and
reports detailed information if it detects a zombie slot.

No appreciable effect on performance as measured by the sweet
benchmarks:

name                                old time/op  new time/op  delta
BiogoIgor                            15.8s ± 2%   15.8s ± 2%    ~     (p=0.421 n=24+25)
BiogoKrishna                         15.6s ± 2%   15.8s ± 5%    ~     (p=0.082 n=22+23)
BleveIndexBatch100                   4.90s ± 3%   4.88s ± 2%    ~     (p=0.627 n=25+24)
CompileTemplate                      204ms ± 1%   205ms ± 0%  +0.22%  (p=0.010 n=24+23)
CompileUnicode                      77.8ms ± 2%  78.0ms ± 1%    ~     (p=0.236 n=25+24)
CompileGoTypes                       729ms ± 0%   731ms ± 0%  +0.26%  (p=0.000 n=24+24)
CompileCompiler                      3.52s ± 0%   3.52s ± 1%    ~     (p=0.152 n=25+25)
CompileSSA                           8.06s ± 1%   8.05s ± 0%    ~     (p=0.192 n=25+24)
CompileFlate                         132ms ± 1%   132ms ± 1%    ~     (p=0.373 n=24+24)
CompileGoParser                      163ms ± 1%   164ms ± 1%  +0.32%  (p=0.003 n=24+25)
CompileReflect                       453ms ± 1%   455ms ± 1%  +0.39%  (p=0.000 n=22+22)
CompileTar                           181ms ± 1%   181ms ± 1%  +0.20%  (p=0.029 n=24+21)
CompileXML                           244ms ± 1%   244ms ± 1%    ~     (p=0.065 n=24+24)
CompileStdCmd                        15.8s ± 2%   15.7s ± 2%    ~     (p=0.059 n=23+24)
FoglemanFauxGLRenderRotateBoat       13.4s ±11%   12.8s ± 0%    ~     (p=0.377 n=25+24)
FoglemanPathTraceRenderGopherIter1   18.6s ± 0%   18.6s ± 0%    ~     (p=0.696 n=23+24)
GopherLuaKNucleotide                 28.7s ± 4%   28.6s ± 5%    ~     (p=0.700 n=25+25)
MarkdownRenderXHTML                  250ms ± 1%   248ms ± 1%  -1.01%  (p=0.000 n=24+24)
[Geo mean]                           1.60s        1.60s       -0.11%

(https://perf.golang.org/search?q=upload:20200517.6)

For #38702.

Change-Id: I8af1fefd5fbf7b9cb665b98f9c4b73d1d08eea81
Reviewed-on: https://go-review.googlesource.com/c/go/+/234100
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-05-21 21:36:40 +00:00
Bryan C. Mills
9f4aeb36e2 all: use a hermetic "go" tool in standard-library tests
The go/build package uses the "go" tool from the user's environment,
but its tests should not assume that that tool is in any particular
state, let alone appropriate for running the test.

Instead, explicitly use testenv.GoTool, adding it to $PATH in a
TestMain when necessary.

Fixes #39199
Fixes #39198

Change-Id: I56618a55ced473e75dd96eeb3a8f7084e2e64d02
Reviewed-on: https://go-review.googlesource.com/c/go/+/234880
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
2020-05-21 21:17:48 +00:00
Michael Pratt
11b3730a02 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:

  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.

Fixes #38931

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>
2020-05-21 21:01:39 +00:00
Jean de Klerk
0a90ecad07 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.

Fixes #38458

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>
2020-05-21 18:28:05 +00:00
Michael Anthony Knyszek
c847589ad0 runtime: synchronize StartTrace and StopTrace with sysmon
Currently sysmon is not stopped when the world is stopped, which is
in general a difficult thing to do. The result of this is that when
tracing starts and the value of trace.enabled changes, it's possible
for sysmon to fail to emit an event when it really should. This leads to
traces which the execution trace parser deems inconsistent.

Fix this by putting all of sysmon's work behind a new lock sysmonlock.
StartTrace and StopTrace both acquire this lock after stopping the world
but before performing any work in order to ensure sysmon sees the
required state change in tracing. This change is expected to slow down
StartTrace and StopTrace, but will help ensure consistent traces are
generated.

Updates #29707.
Fixes #38794.

Change-Id: I64c58e7c3fd173cd5281ffc208d6db24ff6c0284
Reviewed-on: https://go-review.googlesource.com/c/go/+/234617
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
2020-05-21 14:48:50 +00:00
Cherry Zhang
39ea0ea05d cmd/link: fix size calculation for file space preallocation on darwin
On darwin, we preallocate file storage space with fcntl
F_ALLOCATEALL in F_PEOFPOSMODE mode. This is specified as
allocating from the physical end of the file. So the size we give
it should be the increment, instead of the total size.

Fixes #39044.

Change-Id: I10c7ee8d51f237b4a7604233ac7abc6f91dcd602
Reviewed-on: https://go-review.googlesource.com/c/go/+/234481
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
2020-05-21 14:48:43 +00:00
Than McIntosh
fed33d76bc cmd/compile: delay inlinable method compilation for -c=1
When the concurrent back end is not enabled, it is possible to have a
scenario where: we compile a specific inlinable non-pointer-receiver
method T.M, then at some point later on in the compilation we visit a
type that triggers generation of a pointer-receiver wrapper (*T).M,
which then results in an inline of T.M into (*T).M. This introduces
subtle differences in the DWARF as compared with when the concurrent
back end is enabled (in the concurrent case, by the time we run the
SSA back end on T.M is is marked as being inlined, whereas in the
non-current case it is not marked inlined).

As a fix, at the point where we would normally compile a given
function in the xtop list right away, if the function is a method AND
is inlinable AND hasn't been inlined, then delay its compilation until
compileFunctions (so as to make sure that when we do compile it, all
possible inlining has been complete). In addition, make sure that
the abstract function symbol for the inlined function gets recorded
correctly.

Fixes #38068.

Change-Id: I57410ab5658bd4ee5b4b80750518e9b20fd6ba52
Reviewed-on: https://go-review.googlesource.com/c/go/+/234178
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2020-05-21 14:45:26 +00:00
Jay Conrod
0cfe1fb878 cmd/go: rank errUseProxy lower when handling proxy errors
modfetch.TryProxies ranks errors returned by GOPROXY entries by
usefulness. It returns the error of the highest rank from the last
proxy. Errors from "direct" and "noproxy" are most useful, followed by
errors other than ErrNotExist, followed by ErrNotExist.

This change ranks errUseProxy with ErrNotExist even though it's
reported by "noproxy". There is almost always a more useful message
than "path does not match GOPRIVATE/GONOPROXY".

Fixes #39180

Change-Id: Ifa5b96462d7bf411e6d2d951888465c839d42471
Reviewed-on: https://go-review.googlesource.com/c/go/+/234687
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-21 14:31:37 +00:00
Cherry Zhang
c53b2bdb35 runtime: add a barrier after a new span is allocated
When copying a stack, we
1. allocate a new stack,
2. adjust pointers pointing to the old stack to pointing to the
   new stack.

If the GC is running on another thread concurrently, on a machine
with weak memory model, the GC could observe the adjusted pointer
(e.g. through gp._defer which could be a special heap-to-stack
pointer), but not observe the publish of the new stack span. In
this case, the GC will see the adjusted pointer pointing to an
unallocated span, and throw. Fixing this by adding a publication
barrier between the allocation of the span and adjusting pointers.

One testcase for this is TestDeferHeapAndStack in long mode. It
fails reliably on linux-mips64le-mengzhuo builder without the fix,
and passes reliably after the fix.

Fixes #35541.

Change-Id: I82b09b824fdf14be7336a9ee853f56dec1b13b90
Reviewed-on: https://go-review.googlesource.com/c/go/+/234478
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2020-05-21 14:31:36 +00:00
Ian Lance Taylor
567556d786 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.

Fixes #38225

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>
2020-05-20 21:52:34 +00:00
Yasuhiro Matsumoto
aeab403174 cmd/go: use temporary file for output of gcc command on Windows
On Windows, some of gcc command (like msys2 native) output NUL as a file.

Fixes #36000

Change-Id: I02c632fa2d710a011d79f24d5beee4bc57ad994e
Reviewed-on: https://go-review.googlesource.com/c/go/+/233977
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-20 21:52:17 +00:00
David Chase
daf70d6c16 cmd/go: remove GOAMD64 environment variable
This removes the GOAMD64 environment variable and its documentation.
The value is instead supplied by a compiled-in constant.

Note that function alignment is also dependent on the value of
the (removed) flag; it is 32 for aligned jumps, 16 if not.
When the flag-dependent logic is removed, it will be 32.

Updates #35881.

Change-Id: Ic41c0b9833d2e8a31fa3ce8067d92aa2f165bf72
Reviewed-on: https://go-review.googlesource.com/c/go/+/231600
Reviewed-by: Austin Clements <austin@google.com>
2020-05-20 19:21:43 +00:00
Bryan C. Mills
f7f9c8f2fb runtime: allocate fewer bytes during TestEINTR
This will hopefully address the occasional "runtime: out of memory"
failures observed on the openbsd-arm-jsing builder:
https://build.golang.org/log/c296d866e5d99ba401b18c1a2ff3e4d480e5238c

Also make the "spin" and "winch" loops concurrent instead of
sequential to cut down the test's running time.

Finally, change Block to coordinate by closing stdin instead of
sending SIGINT. The SIGINT handler wasn't necessarily registered by
the time the signal was sent.

Updates #20400
Updates #39043

Change-Id: Ie12fc75b87e33847dc25a12edb4126db27492da6
Reviewed-on: https://go-review.googlesource.com/c/go/+/234538
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-20 15:57:15 +00:00
Michael Anthony Knyszek
dfd613e0e4 runtime: don't use (addrRange).subtract in removeGreaterEqual
Currently in (*addrRanges).removeGreaterEqual we use
(addrRange).subtract with a range from specified address to "infinity"
which is supposed to be maxOffAddr. However, maxOffAddr is necessarily
an inclusive bound on the address space, because on many platforms an
exclusive bound would overflow back to 0.

On some platforms like mips and mipsle, the address space is smaller
than what's representable in a pointer, so if there's a range which hits
the top of the address space (such as in the pageAlloc tests), the limit
doesn't overflow, but maxOffAddr is inclusive, so any attempt to prune
this range with (*addrRange).removeGreaterEqual causes a failure, since
the range passed to subtract is contained within the address range which
touches the top of the address space.

Another problem with using subtract here is that addr and
maxOffAddr.addr() may not be in the same segment which could cause
makeAddrRange to panic. While this unlikely to happen, on some platforms
such as Solaris it is possible.

Fix these issues by not using subtract at all. Create a specific
implementation of (addrRange).removeGreaterEqual which side-steps all of
this by not having to worry about the top of the address space at all.

Fixes #39128.

Change-Id: Icd5b587b1a3d32a5681fb76cec4c001401f5756f
Reviewed-on: https://go-review.googlesource.com/c/go/+/234457
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-05-20 15:15:16 +00:00
Michael Hudson-Doyle
4ec4a792f6 cmd/go: accept smart quotes when checking for missing gold in TestNoteReading
Fixes #39157

Change-Id: Ia983f5e66698519cd19c1565cfb80e86d08fdfc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/234380
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-05-20 03:40:22 +00:00
Bryan C. Mills
13617380ca testing: clean up remaining TempDir issues from CL 231958
Updates #38850

Change-Id: I33f48762f5520eb0c0a841d8ca1ccdd65ecc20c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/234583
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-19 19:55:14 +00:00
Bryan C. Mills
2dbbc867db crypto/x509: save the temp dir in TestReadUniqueDirectoryEntries
In CL 231958, TempDir was changed to create a new temp directory on
each allocation, on the theory that it is easy to save in a variable
for callers that want the same directory repeatedly. Apply that
transformation here.

Updates #38850

Change-Id: Ibb014095426c33038e0a2c95303579cf95d5c3ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/234582
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-05-19 18:43:29 +00:00
Roger Peppe
6d6e4827c0 testing: return unique directory inside same base root for TempDir
We use a single parent directory for all temporary directories
created by a test so they're all kept together.

Fixes #38850

Change-Id: If8edae10c5136efcbcf6fd632487d198b9e3a868
Reviewed-on: https://go-review.googlesource.com/c/go/+/231958
Reviewed-by: Russ Cox <rsc@golang.org>
2020-05-19 17:38:32 +00:00
Tobias Klauser
8fa468d511 doc/go1.15: consolidate notes regarding package testing
Fold the descriptions of testing.T.Deadline and TestMain related changes
into the existing section for package testing.

Also link T.Deadline to its godoc.

Change-Id: I732c45fb879305099cb8a51a77ef11fba1b2f1e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/234557
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-05-19 15:34:47 +00:00
Ian Lance Taylor
c88f6989e1 doc: require macOS 10.12 or later
For #23011

Change-Id: I3137820af975301e251b0a23a9349c544b42c70b
Reviewed-on: https://go-review.googlesource.com/c/go/+/234521
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2020-05-19 04:10:43 +00:00
Katie Hockman
185c3d4610 crypto/tls: remove version check when unmarshaling sessionState
This was causing issues when fuzzing with
TestMarshalUnmarshal since the test would
occassionally set the version to VersionTLS13,
which would fail when unmarshaling. The check
doesn't add much in practice, and there is no
harm in removing it to de-flake the test.

Fixes #38902

Change-Id: I0906c570e9ed69c85fdd2c15f1b52f9e372c62e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/234486
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2020-05-18 23:09:31 +00:00
Ian Lance Taylor
0d39dba6de doc/go1.15: mention vet warning for impossible type assertions
For #4483

Change-Id: Iab76baf50b79eda1e3acfd662d0e7830c7962f5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/234518
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-05-18 23:05:20 +00:00
Ian Lance Taylor
b0bc18d5bc doc/go1.15: mention vet warning for string(x)
For #32479

Change-Id: I974709d471021d370aa9bdc5f24b02afa8bd9b54
Reviewed-on: https://go-review.googlesource.com/c/go/+/234517
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-05-18 23:04:48 +00:00
Katie Hockman
3b0882e838 crypto/tls: persist the createdAt time when re-wrapping session tickets
Change-Id: I33fcde2d544943fb04c2599810cf7fb773aeba1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/234483
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-05-18 20:53:36 +00:00
Jay Conrod
afd477f2ba cmd: update golang.org/x/mod to v0.3.0 (same commit)
v0.3.0 is a tag on 859b3ef565e2, the version that was already being
used. This change is a no-op, except for letting us use a release
version instead of a pseudo-version.

For #36905

Change-Id: I70b8ce2a3f1451f5602c469501362d7a6a673b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/234002
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-05-18 13:11:11 +00:00