1
0
mirror of https://github.com/golang/go synced 2024-11-13 16:10:25 -07:00
Commit Graph

42148 Commits

Author SHA1 Message Date
Cherry Zhang
0f25102858 runtime: print more information on stack overflow
Print the current SP and (old) stack bounds when the stack grows
too large. This helps to identify the problem: whether a large
stack is used, or something else goes wrong.

For #35470.

Change-Id: I34a4064d5c7280978391d835e171b90d06f87222
Reviewed-on: https://go-review.googlesource.com/c/go/+/207351
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-11-27 01:30:32 +00:00
Cherry Zhang
b2482e4817 cmd/internal/obj: mark split-stack prologue nonpreemptible
When there are both a synchronous preemption request (by
clobbering the stack guard) and an asynchronous one (by signal),
the running goroutine may observe the synchronous request first
in stack bounds check, and go to the path of calling morestack.
If the preemption signal arrives at this point before the call to
morestack, the goroutine will be asynchronously preempted,
entering the scheduler. When it is resumed, the scheduler clears
the preemption request, unclobbers the stack guard. But the
resumed goroutine will still call morestack, as it is already on
its way. morestack will, as there is no preemption request,
double the stack unnecessarily. If this happens multiple times,
the stack may grow too big, although only a small amount is
actually used.

To fix this, we mark the stack bounds check and the call to
morestack async-nonpreemptible, starting after the memory
instruction (mostly a load, on x86 CMP with memory).

Not done for Wasm as it does not support async preemption.

Fixes #35470.

Change-Id: Ibd7f3d935a3649b80f47539116ec9b9556680cf2
Reviewed-on: https://go-review.googlesource.com/c/go/+/207350
Reviewed-by: David Chase <drchase@google.com>
2019-11-27 01:30:19 +00:00
Cherry Zhang
c3f149250e cmd/internal/obj, runtime: use register map to mark unsafe points
Currently we use stack map index -2 to mark unsafe points, i.e.
PC ranges that is not safe for async preemption. This has a
problem: it cannot mark CALL instructions, because for stack scan
a valid stack map index is needed.

This CL switches to use register map index for marking unsafe
points instead, which does not conflict with stack scan and can
be applied on CALL instructions. This is necessary as next CL
will mark call to morestack nonpreemptible.

For #35470.

Change-Id: I357bf26c996e1fee1e7eebe4e6bb07d62930d3f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/207349
Reviewed-by: David Chase <drchase@google.com>
2019-11-27 01:29:00 +00:00
Jay Conrod
4a378d712d doc: add section on GOPROXY protocol to module reference doc
Based on 'go help goproxy'.

Updates #33637

Change-Id: I2f3477cfc8f6fb53515604a28a5bc01eb4fe8f48
Reviewed-on: https://go-review.googlesource.com/c/go/+/208777
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-11-26 20:21:04 +00:00
Jay Conrod
27e50a1a25 cmd: update golang.org/x/mod and vendor
This CL pulls in a fix to golang.org/x/mod/modfile. No change needed
to cmd/go.

Fixes #35737

Change-Id: I7ca1bb46d2923b01587042f0f312d3c3df54c425
Reviewed-on: https://go-review.googlesource.com/c/go/+/208977
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-26 19:05:22 +00:00
Cherry Zhang
67f0f83216 runtime: disable async preemption on darwin/arm(64) if no cgo
On darwin, we use libc calls, and cgo is required on ARM and
ARM64 so we have TLS set up to save/restore G during C calls. If
cgo is absent, we cannot save/restore G in TLS, and if a signal
is received during C execution we cannot get the G. Therefore
don't send signals (and hope that we won't receive any signal
during C execution).

This can only happen in the go_bootstrap program (otherwise cgo
is required).

Fixes #35800.

Change-Id: I6c02a9378af02c19d32749a42db45165b578188d
Reviewed-on: https://go-review.googlesource.com/c/go/+/208818
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-26 16:55:36 +00:00
Kyle Shannon
20bf6a495e lib/time: update tz data to 2019c
Updates #22487

Change-Id: I32dcd604af84e2000196d64bf69c67f81535290a
Reviewed-on: https://go-review.googlesource.com/c/go/+/208797
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
2019-11-26 12:03:08 +00:00
skanehira
87805c92fd os: fix broken comment's link
Change-Id: Icf6cb06dfdde00de1db5e57b243d7e60a9e4e7ac
GitHub-Last-Rev: 45003b0656
GitHub-Pull-Request: golang/go#35834
Reviewed-on: https://go-review.googlesource.com/c/go/+/208837
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-25 23:37:03 +00:00
Fazlul Shahriar
5de3b97554 cmd/link: fix TestStrictDup in module mode
Fixes #35790

Change-Id: Ie9df103c5c21b34a378821dfad60804ce4b0cad4
Reviewed-on: https://go-review.googlesource.com/c/go/+/208517
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-25 21:15:11 +00:00
Jason A. Donenfeld
7adafc985a cmd/go: prefix calls to Windows rmdir with cmd.exe
rmdir is a built-in of cmd.exe. It's also an alias in powershell.exe. We
want always the cmd.exe on, so specify it explicitly.

Fixes #35813

Change-Id: I89723e993ee26a20b42d03b8a725ff10ccf30505
Reviewed-on: https://go-review.googlesource.com/c/go/+/208639
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-25 19:02:50 +00:00
Bryan C. Mills
6d5f8b11d5 misc/cgo/testshared: do not write to GOROOT
Instead of installing shared libraries to GOROOT/pkg, clone the
necessary files into a new GOROOT and run there.

Given that we now have a build cache, ideally we should not need to
install into GOROOT/pkg at all, but we can't fix that during the 1.14
code freeze.

Updates #28387
Updates #28553
Updates #30316

Change-Id: I83084a8ca29a5dffcd586c7fccc3f172cac57cc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/208482
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-11-25 17:05:01 +00:00
Bryan C. Mills
e16f64c094 misc: log 'ok' from 'go run' tests on success
Otherwise, these tests produce no output, which can make the overall
output of all.bash a bit tricky to decipher.

Updates #30316
Updates #29062

Change-Id: I33b9e070fd28b9f21ece128e9e603a982c08b7cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/208483
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-11-25 16:43:27 +00:00
Bryan C. Mills
476395cb3e misc: remove use of relative directories in overlayDir functions
It turns out that the relative-path support never worked in the first
place.

It had been masked by the fact that we ~never invoke overlayDir with
an absolute path, which caused filepath.Rel to always return an error,
and overlayDir to always fall back to absolute paths.

Since the absolute paths seem to be working fine (and are simpler),
let's stick with those. As far as I can recall, the relative paths
were only a space optimization anyway.

Updates #28387
Updates #30316

Change-Id: Ie8cd28f3c41ca6497ace2799f4193d7f5dde7a37
Reviewed-on: https://go-review.googlesource.com/c/go/+/208481
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-11-25 16:26:15 +00:00
diaxu01
01f15b62b3 cmd/internal/obj/arm64: add NOOP description in Go assembly syntax
This Patch describes NOOP in Go assembly syntax and gives Go assembly
example and corresponding GNU assembly example.

Change-Id: I9db659cc5e3dc6b1f1450f2064255af8872d4b1c
Reviewed-on: https://go-review.googlesource.com/c/go/+/207400
Run-TryBot: eric fang <eric.fang@arm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-25 15:08:18 +00:00
Dan Scales
9940c77682 runtime: add go:nosplit to cgo_mmap.go:mmap() and sys_darwin.go:mmap()
cgo_mmap.go:mmap() is called by mem_linux.go:sysAlloc(), a low-level memory
allocation function. mmap() should be nosplit, since it is called in a lot of
low-level parts of the runtime and callers often assume it won't acquire any
locks.

As an example there is a potential deadlock involving two threads if mmap is not nosplit:

trace.bufLock acquired, then stackpool[order].item.mu, then mheap_.lock
  - can happen for traceEvents that are not invoked on the system stack and cause
    a traceFlush, which causes a sysAlloc, which calls mmap(), which may cause a
    stack split. mheap_.lock
mheap_.lock acquired, then trace.bufLock
  - can happen when doing a trace in reclaimChunk (which holds the mheap_ lock)

Also, sysAlloc() has a comment that it is nosplit because it may be invoked
without a valid G, in which case its callee mmap() should also be nosplit.

Similarly, sys_darwin.go:mmap() is called by mem_darwin.go:sysAlloc(), and should
be nosplit for the same reasons.

Extra gomote testing:  linux/arm64, darwin/amd64

Change-Id: Ia4d10cec5cf1e186a0fe5aab2858c6e0e5b80fdc
Reviewed-on: https://go-review.googlesource.com/c/go/+/207844
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-25 03:41:16 +00:00
Than McIntosh
b81dd1c0e7 cmd/link: disable new testpoint on mips pending investigation
Skip TestMinusRSymsWithSameName testpoint on MIPS for the time being
since it triggers failures on that arch. Will re-enable once the
problems are fixed.

Updates #35779.

Change-Id: I3e6650158ab04a2be77e3db5a5194df3bbb0859e
Reviewed-on: https://go-review.googlesource.com/c/go/+/208557
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-24 15:06:38 +00:00
Tobias Klauser
6f7b96f6cb syscall: follow convention for generated code comments in linux/riscv64 files
Use generated code comments following the convention
https://golang.org/s/generatedcode.

Follow-up for CL 204659

Change-Id: I7a3467ff9477488616b17f059245368d3d82d7a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/208417
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-23 11:00:41 +00:00
Richard Musiol
c2e2296dd7 syscall/js: handle interleaved functions correctly
Because of concurrent goroutines it is possible for multiple event
handlers to return at the same time. This was not properly supported
and caused the wrong goroutine to continue, which in turn caused
memory corruption.

This change adds a stack of events so it is always clear which is the
innermost event that needs to return next.

Fixes #35256

Change-Id: Ia527da3b91673bc14e84174cdc407f5c9d5a3d09
Reviewed-on: https://go-review.googlesource.com/c/go/+/204662
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-23 09:35:22 +00:00
Than McIntosh
688aa74857 cmd/link: add new linker testpoint for "ld -r" host object
This adds a new test that builds a small Go program with linked
against a *.syso file that is the result of an "ld -r" link. The
sysobj in question has multiple static symbols in the same section
with the same name, which triggered a bug in the loader in -newobj
mode.

Updates #35779.

Change-Id: Ibe1a75662dc1d49c4347279e55646ee65a81508e
Reviewed-on: https://go-review.googlesource.com/c/go/+/208478
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-23 00:06:55 +00:00
Than McIntosh
f29e53d664 cmd/link: fix bug with -newobj and "ld -r" ELF host objects
When the ELF host object loader encounters a static/hidden symbol, it
creates a sym.Symbol for it but does not enter it into the sym.Symbols
lookup table. Under -newobj mode, this was not happening correctly; we
were adding the sym via loader.LookupOrCreate, which resulted in
collisions when it encountered symbols with the same name + version +
section (this can happen for "ld -r" objects).

Fixes #35779.

Change-Id: I36d40fc1efc03fc1cd8ae6b76cb6a0d2a957389c
Reviewed-on: https://go-review.googlesource.com/c/go/+/208479
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-23 00:06:41 +00:00
Dmitri Shuralyov
6ba1bf393a net/http: rename tests for Redirect and StripPrefix
Before May 2018, I mistakenly thought the _suffix naming convention¹
used by examples also applied to tests. Thanks to a code review comment²
from Ian Lance Taylor, I have since learned that is not true.
This trivial change fixes some collateral damage from my earlier
misunderstanding, resulting in improved test naming consistency.

¹ https://golang.org/pkg/testing/#hdr-Examples
² https://go-review.googlesource.com/c/go/+/112935/1/src/path/filepath/path_test.go#1075

Change-Id: I555f60719629eb64bf2f096aa3dd5e00851827cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/207446
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-22 23:23:13 +00:00
Brad Fitzpatrick
980f0c00bb net/http: make Transport.IdleConnTimeout consider wall (not monotonic) time
Both laptops closing their lids and cloud container runtimes
suspending VMs both faced the problem where an idle HTTP connection
used by the Transport could be cached for later reuse before the
machine is frozen, only to wake up many minutes later to think that
their HTTP connection was still good (because only a second or two of
monotonic time passed), only to find out that the peer hung up on them
when they went to write.

HTTP/1 connection reuse is inherently racy like this, but no need for
us to step into a trap if we can avoid it. Also, not everybody sets
Request.GetBody to enable re-tryable POSTs. And we can only safely
retry requests in some cases.

So with this CL, before reusing an old connection, double check the walltime.

Testing was done both with a laptop (closing the lid for a bit) and
with QEMU, running "stop" and "cont" commands in the monitor and
sending QMP guest agent commands to update its wall clock after the
"cont":

echo '{"execute":"guest-set-time"}' | socat STDIN UNIX-CONNECT:/var/run/qemu-server/108.qga

In both cases, I was running
https://gist.github.com/bradfitz/260851776f08e4bc4dacedd82afa7aea and
watching that the RemoteAddr changed after resume.

It's kinda difficult to write an automated test for. I gave a lightning talk on
using pure emulation user mode qemu for such tests:

   https://www.youtube.com/watch?v=69Zy77O-BUM
   https://docs.google.com/presentation/d/1rAAyOTCsB8GLbMgI0CAbn69r6EVWL8j3DPl4qc0sSlc/edit?usp=sharing
   https://github.com/google/embiggen-disk/blob/master/integration_test.go

... that would probably be a good direction if we want an automated
test here. But I don't have time to do that now.

Updates #29308 (HTTP/2 remains)

Change-Id: I03997e00491f861629d67a0292da000bd94ed5ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/204797
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-11-22 22:24:38 +00:00
Bryan C. Mills
9f3c2b6d16 cmd/go/internal/work: reduce code duplication in buildModeInit by using sys.BuildModeSupported
Updates #34347

Change-Id: I6ea02d4737999bf24f5335508b5ed2352b498122
Reviewed-on: https://go-review.googlesource.com/c/go/+/208458
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-11-22 21:09:43 +00:00
Jay Conrod
8324acadfe cmd/go: fix and re-enable build_trimpath test
The test was comparing a binary built from a list of files to a test
build from a named package. That should not (and did not) work. The
test now compares two binaries built the same way in different
directories.

Also add a portion of the test for GOPATH and fix the gccgo portion of
the test (verified manually).

Fixes #35435

Change-Id: I2535a0011c9d97d2274e5550ae277302dbb91e6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/208234
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-22 19:17:28 +00:00
Bryan C. Mills
941ac9ce70 cmd/go: do not panic when computing Shlib for a package with no Target
In module mode, a non-main package lacks an install target.

The location of the .shlib corresponding to a given target is stored
in a .shlibname file alongside its install target, so in module mode
a non-main package also lacks a .shlibname file.

This also implies that such a package cannot be installed with
'go install -buildmode=linkshared', but that is a problem
for another day.

Fixes #35759
Updates #34347

Change-Id: Id3e0e068266d5fb9b061a59e70f9a65985d4973b
Reviewed-on: https://go-review.googlesource.com/c/go/+/208233
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-11-22 19:03:50 +00:00
Bryan C. Mills
28314cf124 cmd/go: add a 'buildmode' condition for script tests
In CL 208233 I am fixing a panic that occurs only with a specific
build mode. I want that test to run on all platforms that support that
build mode, but the logic for determining support is somewhat
involved.

For now, I am duplicating that logic into the cmd/internal/sys
package, which already reports platform support for other build flags.

We can refactor cmd/go/internal/work to use the extracted function in
a followup CL.

Updates #35759

Change-Id: Ibbaedde4d1e8f683c650beedd10849bc27e7a6e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/208457
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2019-11-22 19:03:26 +00:00
Bryan C. Mills
c931f1b6e6 misc/cgo/testshared: make -v output less verbose
Previously, 'go test -v' in this directory would result in a massive
dump of go command output, because the test plumbed -v to 'build -x'.
This change separates them into distinct flags, so that '-v' only
implies the display of default 'go' command output.

Updates #30316

Change-Id: Ifb125f35ec6a0bebe7e8286e7c546d132fb213df
Reviewed-on: https://go-review.googlesource.com/c/go/+/208232
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-22 19:01:42 +00:00
Michael Anthony Knyszek
05511a5c0a runtime: release worldsema before Gosched in STW GC mode
After CL 182657 we no longer hold worldsema across the GC, we hold
gcsema instead.

However in STW GC mode we don't release worldsema before calling Gosched
on the user goroutine (note that user goroutines are disabled during STW
GC) so that user goroutine holds onto it. When the GC is done and the
runtime inevitably wants to "stop the world" again (though there isn't
much to stop) it'll sit there waiting for worldsema which won't be
released until the aforementioned goroutine is scheduled, which it won't
be until the GC is done!

So, we have a deadlock.

The fix is easy: just release worldsema before calling Gosched.

Fixes #34736.

Change-Id: Ia50db22ebed3176114e7e60a7edaf82f8535c1b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/208379
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-22 17:33:48 +00:00
Michael Anthony Knyszek
1c5bd3459b runtime: increase TestPhysicalMemoryUtilization threshold
TestPhysicalMemoryUtilization occasionally fails on some platforms by
only a small margin. The reason for this is that it assumes the
scavenger will always be able to scavenge all the memory that's released
by sweeping, but because of the page cache, there could be free and
unscavenged memory held onto by a P which the scavenger simply cannot
get to.

As a result, if the page cache gets filled completely (512 KiB of free
and unscavenged memory) this could skew a test which expects to
scavenge roughly 8 MiB of memory. More specifically, this is 512 KiB of
memory per P, and if a system is more inclined to bounce around
between Ps (even if there's only one goroutine), this memory can get
"stuck".

Through some experimentation, I found that failures correlated highly
with relatively large amounts of memory ending up in some page cache
(like 60 or 64 pages) on at least one P.

This change changes the test's threshold such that it accounts for the
page cache, and scales up with GOMAXPROCS. Because the test constants
themselves don't change, however, the test must now also bound
GOMAXPROCS such that the threshold doesn't get too high (at which point
the test becomes meaningless).

Fixes #35580.

Change-Id: I6bdb70706de991966a9d28347da830be4a19d3a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/208377
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2019-11-22 16:04:32 +00:00
Jay Conrod
300f5d5b4b cmd/go: add 'go generate' commands to modfile_flag test
Verify that 'go generate' works with -modfile. Also check that
go commands starts with 'go generate' do not inherit -modfile, but
they should still work if -modfile is set in GOFLAGS.

Updates #34506

Change-Id: I5e1f897b4e38e4fdaccc0fbb7a71b8d0e9fc0660
Reviewed-on: https://go-review.googlesource.com/c/go/+/208236
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-11-22 16:03:16 +00:00
Bryan C. Mills
3922c006ad misc/cgo/testcshared: avoid writing to GOROOT in tests
The tests in this package invoked 'go install -i -buildmode=c-shared'
in order to generate an archive as well as multiple C header files.

Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_testcshared_shared, which may not be writable
— for example, if GOROOT is owned by the root user but the test is
being run by a non-root user.

Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.

In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.

Updates #28387
Updates #30316
Updates #35715

Change-Id: I622426a860828020d98f7040636f374e5c766d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/208119
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-22 15:34:14 +00:00
Bryan C. Mills
c02f3b86b4 misc/cgo/testcarchive: avoid writing to GOROOT in tests
Also add a -testwork flag to facilitate debugging the test itself.

Three of the tests of this package invoked 'go install -i
-buildmode=c-archive' in order to generate an archive as well as
multiple C header files.

Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_shared, which may not be writable — for
example, if GOROOT is owned by the root user but the test is being run
by a non-root user.

Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.

In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.

Updates #28387
Updates #30316
Updates #35715

Change-Id: I3d483f84e22058561efe740aa4885fc3f26137b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/208117
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-22 15:32:23 +00:00
Hana Kim
ea18a1c212 runtime/pprof: avoid crash due to truncated stack traces
The profile proto message builder maintains a location entry cache
that maps a location (possibly involving multiple user frames
that represent inlined function calls) to the location id. We have
been using the first pc of the inlined call sequence as the key of
the cached location entry assuming that, for a given pc, the sequence
of frames representing the inlined call stack is deterministic and
stable. Then, when analyzing the new stack trace, we expected the
exact number of pcs to be present in the captured stack trace upon
the cache hit.

This assumption does not hold, however, in the presence of the stack
trace truncation in the runtime during profiling, and also with the
potential bugs in runtime.

A better fix is to use all the pcs of the inlined call sequece as
the key instead of the first pc. But that is a bigger code change.
This CL avoids the crash assuming the trace was truncated.

Fixes #35538

Change-Id: I8c6bae98bc8b178ee51523c7316f56b1cce6df16
Reviewed-on: https://go-review.googlesource.com/c/go/+/207609
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
2019-11-22 14:32:02 +00:00
two
95be9b7559 reflect: remove obsolete comment about gobitVector
Change-Id: Ie3495a51ac2021a55e7c1ee43a66d07a5bf2757a
GitHub-Last-Rev: b6a6bab3ab
GitHub-Pull-Request: golang/go#35709
Reviewed-on: https://go-review.googlesource.com/c/go/+/207853
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-22 04:26:34 +00:00
David Chase
0e02cfb369 cmd/compile: try harder to not use an empty src.XPos for a bogus line
The fix for #35652 did not guarantee that it was using a non-empty
src position to replace an empty one.  The new code checks again
and falls back to a more certain position.  (The input in question
compiles to a single empty infinite loop, and none of the actual instructions
had any source position at all.  That is a bug, but given the pathology
of this input, not one worth dealing with this late in the release cycle,
if ever.)

Literally:

00000 (5) TEXT "".f(SB), ABIInternal
00001 (5) PCDATA $0, $-2
00002 (5) PCDATA $1, $-2
00003 (5) FUNCDATA $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00004 (5) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
00005 (5) FUNCDATA $2, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
b2
00006 (?) XCHGL AX, AX
b6
00007 (+1048575) JMP 6
00008 (?) END

TODO: Add runtime.InfiniteLoop(), replace infinite loops with a call to
that, and use an eco-friendly runtime.gopark instead.  (This was Cherry's
excellent idea.)

Updates #35652
Fixes #35695

Change-Id: I4b9a841142ee4df0f6b10863cfa0721a7e13b437
Reviewed-on: https://go-review.googlesource.com/c/go/+/207964
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-22 03:06:22 +00:00
Roman Kollár
bbbc6589df net/http: fix Server.ConnContext modifying context for all new connections
Fixes #35750

Change-Id: I65d38cfc5ddd66131777e104c269cc3559b2471d
GitHub-Last-Rev: 953fdfd49b
GitHub-Pull-Request: golang/go#35751
Reviewed-on: https://go-review.googlesource.com/c/go/+/208318
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-21 23:15:28 +00:00
Roberto Clapis
94e9a5e19b text/template: harden JSEscape to also escape ampersand and equal
Ampersand and equal are not dangerous in a JS/JSString context
but they might cause issues if interpolated in HTML attributes.

This change makes it harder to introduce XSS by misusing
escaping.

Thanks to t1ddl3r <t1ddl3r@gmail.com> for reporting this common
misuse scenario.

Fixes #35665

Change-Id: Ice6416477bba4cb2ba2fe2cfdc20e027957255c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/207637
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Mike Samuel <mikesamuel@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
2019-11-21 22:20:17 +00:00
Filippo Valsorda
f4a8bf1283 crypto/tls: remove leftover extensionNextProtoNeg constant
NPN was removed in CL 174329.

Change-Id: Ic63ad53e7e24872e28673d590727e0300f435619
Reviewed-on: https://go-review.googlesource.com/c/go/+/208224
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-21 19:27:16 +00:00
Jay Conrod
2434869858 cmd/go: report an error for 'go list -m ...' outside a module
Previously, we just reported an error for "all". Now we report an
error for any pattern that matches modules in the build list. The
build list can only contain the module "command-line-arguments", so
these patterns are not meaningful.

Fixes #35728

Change-Id: Ibc736491ec9164588f9657c09d1b9683b33cf1de
Reviewed-on: https://go-review.googlesource.com/c/go/+/208222
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-21 19:09:24 +00:00
Cherry Zhang
37715cce69 runtime: relax TestAsyncPreempt
In TestAsyncPreempt, the function being tested for preemption,
although still asynchronously preemptible, may have only samll
ranges of PCs that are preemtible. In an unlucky run, it may
take quite a while to have a signal that lands on a preemptible
instruction. The test case is kind of an extreme. Relax it to
make it more preemptible.

In the original version, the first closure has more work to do,
and it is not a leaf function, and the second test case is a
frameless leaf function. In the current version, the first one
is also a frameless leaf function (the atomic is intrinsified).
Add some calls to it. It is still not preemptible without async
preemption.

Fixes #35608.

Change-Id: Ia4f857f2afc55501c6568d7507b517e3b4db191c
Reviewed-on: https://go-review.googlesource.com/c/go/+/208221
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-21 16:56:47 +00:00
jinmiaoluo
c7e73ef60a net/url: update net/url split comment
Fixes #35735

Change-Id: I4618ffcd1bcf9a7506468b9a1443bc4a2f7f3138
GitHub-Last-Rev: edaf780d3d
GitHub-Pull-Request: golang/go#35736
Reviewed-on: https://go-review.googlesource.com/c/go/+/208297
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-21 16:01:14 +00:00
Andrew
8bbfc51d9a all: base64-encode binaries that will cause Apple notarization to fail
Starting with macOS 10.15 (Catalina), Apple now requires all software
distributed outside of the App Store to be notarized. Any binaries we
distribute must abide by a strict set of requirements like code-signing
and having a minimum target SDK of 10.9 (amongst others).

Apple’s notarization service will recursively inspect archives looking to
find notarization candidate binaries. If it finds a binary that does not
meet the requirements or is unable to decompress an archive, it will
reject the entire distribution. From cursory testing, it seems that the
service uses content sniffing to determine file types, so changing
the file extension will not work.

There are some binaries and archives included in our distribution that
are being detected by Apple’s service as potential candidates for
notarization or decompression. As these are files used by tests and some
are intentionally invalid, we don’t intend to ever make them compliant.

As a workaround for this, we base64-encode any binaries or archives that
Apple’s notarization service issues a warning for, as these warnings will
become errors in January 2020.

Updates #34986

Change-Id: I106fbb6227b61eb221755568f047ee11103c1680
Reviewed-on: https://go-review.googlesource.com/c/go/+/208118
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-21 14:55:12 +00:00
Bryan C. Mills
39a9cb4b5d misc/cgo/testplugin: avoid writing to GOROOT
One of the 'go build' commands executed by this test passed the '-i'
flag, which caused the 'go' command to attempt to install transitive
standard-library dependencies to GOROOT/pkg/$GOOS_$GOARCH_dynlink.

That failed if GOROOT/pkg was not writable (for example, if GOROOT was
owned by the root user, but the user running the test was not root).

As far as I can tell the '-i' flag is not necessary in this test.
Prior to the introduction of the build cache it may have been an
optimization, but now that the build cache is required the '-i' flag
only adds extra work.

Updates #30316

Change-Id: Ib60080a008c1941aa92b5bdd5a194d89fd6202aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/208120
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-20 22:38:34 +00:00
Bryan C. Mills
be0471880d misc/cgo/fortran: avoid writing to $PWD
The bash script that drives this test needs to know whether the
fortran compiler works, but it doesn't actually care about the
generated binary. Write that binary to /dev/null.

Updates #28387
Updates #30316

Change-Id: I4f86da1aeb939fc205f467511fc69235a6a9af26
Reviewed-on: https://go-review.googlesource.com/c/go/+/208124
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-20 22:37:23 +00:00
Eric Rutherford
9852b4b093 path: minor changes to improve documentation for Join
Reworking the comments in path to call out how leading
empty elements are treated. Also updating filepath.Join
since it shared much of the wording from path.Join.

Updates #35655

Change-Id: I5b15c5d36e9d19831ed39e6bcc7f2fd6c1330033
Reviewed-on: https://go-review.googlesource.com/c/go/+/207797
Reviewed-by: Rob Pike <r@golang.org>
2019-11-20 20:51:13 +00:00
Filippo Valsorda
fbcfa6a532 encoding/base64: document that Strict mode still ignores newlines
An application that wants to reject non-canonical encodings is likely to
care about other sources of malleability.

Change-Id: I1d3a5b281d2631ca78df3f89b957a02687a534d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/188858
Reviewed-by: Katie Hockman <katie@golang.org>
2019-11-20 19:43:21 +00:00
Austin Clements
b89b4623eb runtime: support preemption on windows/{386,amd64}
This implements preemptM on Windows using SuspendThead and
ResumeThread.

Unlike on POSIX platforms, preemptM on Windows happens synchronously.
This means we need a make a few other tweaks to suspendG:

1. We need to CAS the G back to _Grunning before doing the preemptM,
   or there's a good chance we'll just catch the G spinning on its
   status in the runtime, which won't be preemptible.

2. We need to rate-limit preemptM attempts. Otherwise, if the first
   attempt catches the G at a non-preemptible point, the busy loop in
   suspendG may hammer it so hard that it never makes it past that
   non-preemptible point.

Updates #10958, #24543.

Change-Id: Ie53b098811096f7e45d864afd292dc9e999ce226
Reviewed-on: https://go-review.googlesource.com/c/go/+/204340
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-20 17:13:59 +00:00
Austin Clements
6fd467ee29 runtime: ensure thread handle is valid in profileloop1
On Windows, there is currently a race between unminit closing the
thread's handle and profileloop1 suspending the thread using its
handle. If another handle reuses the same handle value, this can lead
to unpredictable results.

To fix this, we protect the thread handle with a lock and duplicate it
under this lock in profileloop1 before using it.

This is going to become a much bigger problem with non-cooperative
preemption (#10958, #24543), which uses the same basic mechanism as
profileloop1.

Change-Id: I9d62b83051df8c03f3363344438e37781a69ce16
Reviewed-on: https://go-review.googlesource.com/c/go/+/207779
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
2019-11-20 17:13:48 +00:00
Austin Clements
91bb1d734e runtime: move m.thread to mOS
This field is only used on Windows.

Change-Id: I12d4df09261f8e7ad54c2abd7beda669af28c8e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/207778
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
2019-11-20 17:13:36 +00:00
Clément Chigot
9c6f6409ad runtime: disable GDB tests on AIX with -short
Since the new page allocator, AIX's GDB has trouble running Go programs.
It does work but it can be really slow. Therefore, they are disable when
tests are run with -short.

Updates: #35710

Change-Id: Ibfc4bd2cd9714268f1fe172aaf32a73612e262d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/207919
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-20 15:16:17 +00:00