Russ Cox [Fri, 1 Jun 2018 19:47:29 +0000 (15:47 -0400)]
[release-branch.go1.9] cmd/go: fix 'go get' compatibility for direct download of vgo-aware module
CL 109340 added “minimal module-awareness for legacy operation.”
One part of that is reinterpreting imports inside code trees with go.mod files
as using semantic import versioning, and converting them back to
legacy import paths by stripping the major version element
(for example, interpreting import "x.com/foo/v2/bar" as import "x.com/foo/bar").
This rewrite was not being applied during "go get", with the effect that once
you had the target code downloaded already, everything was fine,
but it didn't download and build successfully the first time.
[release-branch.go1.9] cmd/go: add minimal module-awareness for legacy operation
We want authors to be able to publish code that works with both
the current standard go command and the planned new go command
support for modules. If authors have tagged their code v2 or later,
semantic import versioning means the import paths must include a
v2 path element after the path prefix naming the module.
One option for making this convention compatible with original go get
is to move code into a v2 subdirectory of the root.
That makes sense for some authors, but many authors would prefer
not to move all the code into a v2 subdirectory for a transition and
then move it back up once we everyone has a module-aware go command.
Instead, this CL teaches the old (non-module-aware) go command
a tiny amount about modules and their import paths, to expand
the options for authors who want to publish compatible packages.
If an author has a v2 of a package, say my/thing/v2/sub/pkg,
in the my/thing repo's sub/pkg subdirectory (no v2 in the file system path),
then old go get continues to import that package as my/thing/sub/pkg.
But when go get is processing code in any module (code in a tree with
a go.mod file) and encounters a path like my/thing/v2/sub/pkg,
it will check to see if my/thing/go.mod says "module my/thing/v2".
If so, the go command will read the import my/thing/v2/sub/pkg
as if it said my/thing/sub/pkg, which is the correct "old" import path
for the package in question.
This CL will be back-ported to Go 1.10 and Go 1.9 as well.
Once users have updated to the latest Go point releases containing
this new logic, authors will be able to update to using modules
within their own repos, including using semantic import paths
with vN path elements, and old go get will still be able to consume
those repositories.
This CL also makes "go get" ignore meta go-import lines using
the new "mod" VCS type. This allows a package to specify both
a "mod" type and a "git" type, to present more efficient module
access to module-aware go but still present a Git repo to the old
"go get".
Fixes #24751.
Fixes #25069.
This backport to Go 1.9 also had to pick up p.Internal.RawImports
from CL 74750 and CL 74356 and use it to prepare an updated
set of -importmap arguments for the compiler. (The old code only
understood vendor-related rewriting of import paths.)
Backport fixes #25140.
Change-Id: I378955613a0d63834d4f50f121f4db7e4d87dc0a
Reviewed-on: https://go-review.googlesource.com/115298
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Andrew Bonventre <andybons@golang.org>
Martin Möhrmann [Fri, 4 May 2018 04:54:18 +0000 (06:54 +0200)]
[release-branch.go1.9] strings: fix encoding of \u0080 in map
Fix encoding of PAD (U+0080) which has the same value as utf8.RuneSelf
being incorrectly encoded as \x80 in strings.Map due to using <= instead
of a < comparison operator to check one byte encodings for utf8.
Fixes golang/go#25573
Change-Id: Ib6c7d1f425a7ba81e431b6d64009e713d94ea3bc
Reviewed-on: https://go-review.googlesource.com/111286
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 8c62fc0ca3c96ecbd3a6e81546aa8c53e32ff500)
Reviewed-on: https://go-review.googlesource.com/114636
Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
Alex Brainman [Sat, 5 May 2018 05:28:56 +0000 (15:28 +1000)]
[release-branch.go1.9] crypto/x509: copy and use adjusted syscall.CertChainPolicyPara
As discussed in issue #21376, it is unsafe to have
syscall.CertChainPolicyPara.ExtraPolicyPara uintptr -
it has to be a pointer type. So copy syscall.CertChainPolicyPara
into crypto/tls package, make ExtraPolicyPara unsafe.Pointer,
and use new struct instead of syscall.CertChainPolicyPara.
Ian Lance Taylor [Fri, 20 Apr 2018 21:43:17 +0000 (14:43 -0700)]
[release-branch.go1.9] misc/cgo/testcshared: use file descriptor 30 for TestUnexportedSymbols
Backport of CL 108537 to 1.9 release branch.
We were using file descriptor 100, which requires the Linux kernel to
grow the fdtable size. That step may sometimes require a long time,
causing the test to fail. Switch to file descriptor 30, which should
not require growing the fdtable.
Updates #23784
Fixes #25278
Change-Id: I19ea6ab1724ec1807643d5111c44631e20be76b0
Reviewed-on: https://go-review.googlesource.com/111996
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It's used on Solaris to import symbols from shared libraries, e.g., in
golang.org/x/sys/unix and golang.org/x/net/internal/socket.
We could use a different directive but that would require build tags
in all the places that use it.
Fixes golang/go#23939
Change-Id: I47fcf72a6d2862e304204705979c2056c2f78ec5
Reviewed-on: https://go-review.googlesource.com/94018
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 41d3d153eb76f000e9f6e9edf68ed441d15081cb)
Reviewed-on: https://go-review.googlesource.com/110077 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Wed, 28 Mar 2018 00:06:53 +0000 (17:06 -0700)]
[release-branch.go1.9] cmd/go: add more C compiler/linker options to whitelist
Fixes #23937
Change-Id: Ie63d91355d1a724d0012d99d457d939deeeb8d3e
Reviewed-on: https://go-review.googlesource.com/102818
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-on: https://go-review.googlesource.com/103156
Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 6 Feb 2018 17:44:34 +0000 (09:44 -0800)]
[release-branch.go1.9] cmd/compile: use unsigned loads for multi-element comparisons
When loading multiple elements of an array into a single register,
make sure we treat them as unsigned. When treated as signed, the
upper bits might all be set, causing the shift-or combo to clobber
the values higher in the register.
Fixes #23719.
Change-Id: Ic87da03e9bd0fe2c60bb214b99f846e4e9446052
Reviewed-on: https://go-review.googlesource.com/92335
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-on: https://go-review.googlesource.com/103115
Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
A very small number of old browsers consider content as HTML
even when it is explicitly stated in the Content-Type header
that it is not. If content served is based on user-supplied
input, then an XSS is possible. Introduce three mitigations:
+ Don't reflect user input in error strings
+ Set a Content-Disposition header when requesting a resource
that should never be displayed in a browser window
+ Set X-Content-Type-Options: nosniff on all responses
Ian Lance Taylor [Thu, 15 Feb 2018 23:57:13 +0000 (15:57 -0800)]
[release-branch.go1.9] cmd/go: restrict meta imports to valid schemes
Before this change, when using -insecure, we permitted any meta import
repo root as long as it contained "://". When not using -insecure, we
restrict meta import repo roots to be valid URLs. People may depend on
that somehow, so permit meta import repo roots to be invalid URLs, but
require them to have valid schemes per RFC 3986.
Fixes #23867
Change-Id: Iac666dfc75ac321bf8639dda5b0dba7c8840922d
Reviewed-on: https://go-review.googlesource.com/94603 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/102776
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Tue, 6 Feb 2018 04:12:50 +0000 (23:12 -0500)]
[release-branch.go1.9] cmd/go: accept only limited compiler and linker flags in #cgo directives
Both gcc and clang accept an option -fplugin=code.so to load
a plugin from the ELF shared object file code.so.
Obviously that plugin can then do anything it wants
during the build. This is contrary to the goal of "go get"
never running untrusted code during the build.
(What happens if you choose to run the result of
the build is your responsibility.)
Disallow this behavior by only allowing a small set of
known command-line flags in #cgo CFLAGS directives
(and #cgo LDFLAGS, etc).
The new restrictions can be adjusted by the environment
variables CGO_CFLAGS_ALLOW, CGO_CFLAGS_DISALLOW,
and so on. See the documentation.
In addition to excluding cgo-defined flags, we also have to
make sure that when we pass file names on the command
line, they don't look like flags. So we now refuse to build
packages containing suspicious file names like -x.go.
A wrinkle in all this is that GNU binutils uniformly accept
@foo on the command line to mean "if the file foo exists,
then substitute its contents for @foo in the command line".
So we must also reject @x.go, flags and flag arguments
beginning with @, and so on.
Fixes #23673, CVE-2018-6574.
Change-Id: I59e7c1355155c335a5c5ae0d2cf8fa7aa313940a
Reviewed-on: https://team-review.git.corp.google.com/212507 Reviewed-by: Ian Lance Taylor <iant@google.com>
Keith Randall [Tue, 21 Nov 2017 15:14:11 +0000 (07:14 -0800)]
[release-branch.go1.9] cmd/compile: fix mapassign_fast* routines for pointer keys
The signature of the mapassign_fast* routines need to distinguish
the pointerness of their key argument. If the affected routines
suspend part way through, the object pointed to by the key might
get garbage collected because the key is typed as a uint{32,64}.
This is not a problem for mapaccess or mapdelete because the key
in those situations do not live beyond the call involved. If the
object referenced by the key is garbage collected prematurely, the
code still works fine. Even if that object is subsequently reallocated,
it can't be written to the map in time to affect the lookup/delete.
Fixes #22781
Change-Id: I0bbbc5e9883d5ce702faf4e655348be1191ee439
Reviewed-on: https://go-review.googlesource.com/79018
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-on: https://go-review.googlesource.com/88635
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Ian Lance Taylor [Fri, 10 Nov 2017 17:48:39 +0000 (09:48 -0800)]
[release-branch.go1.9] runtime: call amd64 VDSO entry points on large stack
NOTE: This elides changes to src/runtime/sys_linux_386.s since that
requires another change (golang.org/cl/69390) which we don’t want
to backport.
If the Linux kernel was built with CONFIG_OPTIMIZE_INLINING=n and was
built with hardening options turned on, GCC will insert a stack probe
in the VDSO function that requires a full page of stack space.
The stack probe can corrupt memory if another thread is using it.
Avoid sporadic crashes by calling the VDSO on the g0 or gsignal stack.
While we're at it, align the stack as C code expects. We've been
getting away with a misaligned stack, but it's possible that the VDSO
code will change in the future to break that assumption.
Benchmarks show a 11% hit on time.Now, but it's only 6ns.
Austin Clements [Fri, 1 Dec 2017 21:21:03 +0000 (16:21 -0500)]
[release-branch.go1.9] runtime: use MAP_ANON in sigstack check
MAP_ANON is the deprecated but more portable spelling of
MAP_ANONYMOUS. Use MAP_ANON to un-break the Darwin 10.10 builder.
Updates #22930.
Change-Id: Iedd6232b94390b3b2a7423c45cdcb25c1a5b3323
Reviewed-on: https://go-review.googlesource.com/88316
Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Fri, 1 Dec 2017 03:09:35 +0000 (22:09 -0500)]
[release-branch.go1.9] runtime: restore the Go-allocated signal stack in unminit
Currently, when we minit on a thread that already has an alternate
signal stack (e.g., because the M was an extram being used for a cgo
callback, or to handle a signal on a C thread, or because the
platform's libc always allocates a signal stack like on Android), we
simply drop the Go-allocated gsignal stack on the floor.
This is a problem for Ms on the extram list because those Ms may later
be reused for a different thread that may not have its own alternate
signal stack. On tip, this manifests as a crash in sigaltstack because
we clear the gsignal stack bounds in unminit and later try to use
those cleared bounds when we re-minit that M. On 1.9 and earlier, we
didn't clear the bounds, so this manifests as running more than one
signal handler on the same signal stack, which could lead to arbitrary
memory corruption.
This CL fixes this problem by saving the Go-allocated gsignal stack in
a new field in the m struct when overwriting it with a system-provided
signal stack, and then restoring the original gsignal stack in
unminit.
This CL is designed to be easy to back-port to 1.9. It won't quite
cherry-pick cleanly, but it should be sufficient to simply ignore the
change in mexit (which didn't exist in 1.9).
Now that we always have a place to stash the original signal stack in
the m struct, there are some simplifications we can make to the signal
stack handling. We'll do those in a later CL.
Fixes #22930.
Change-Id: I55c5a6dd9d97532f131146afdef0b216e1433054
Reviewed-on: https://go-review.googlesource.com/88315
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
When the user context which passed in (*DB)BeginTx is canceled or
timeout, the current implementation could cause db transaction leak
in some extreme scenario.
Goroutine 1:
Call (*DB) BeginTx begins a transaction with a userContext.
In (*DB)BeginTx, a new goroutine (*Tx)awaitDone
which monitor context and rollback tx if needed will be created
Goroutine 2(awaitDone):
block on tx.ctx.Done()
Goroutine 1:
Execute some insert or update sqls on the database
Goroutine 1:
Commit the transaction, (*Tx)Commit set
the atomic variable tx.done to 1
Goroutine 3(maybe global timer):
Cancel userContext which be passed in Tx
Goroutine 1:
(*Tx)Commit checks tx.ctx.Done().
Due to the context has been canceled, it will return
context.Canceled or context.DeadlineExceeded error immediately
and abort the real COMMIT operation of transaction
Goroutine 2:
Release with tx.ctx.Done() signal, execute (*Tx)rollback.
However the atomic variable tx.done is 1 currently,
it will return ErrTxDone error immediately and
abort the real ROLLBACK operation of transaction
Alberto Donizetti [Tue, 21 Nov 2017 13:16:04 +0000 (14:16 +0100)]
[release-branch.go1.9] math/big: protect against aliasing in nat.divLarge
In nat.divLarge (having signature (z nat).divLarge(u, uIn, v nat)),
we check whether z aliases uIn or v, but aliasing is currently not
checked for the u parameter.
Unfortunately, z and u aliasing each other can in some cases cause
errors in the computation.
The q return parameter (which will hold the result's quotient), is
unconditionally initialized as
q = z.make(m + 1)
When cap(z) ≥ m+1, z.make() will reuse z's backing array, causing q
and z to share the same backing array. If then z aliases u, setting q
during the quotient computation will then corrupt u, which at that
point already holds computation state.
To fix this, we add an alias(z, u) check at the beginning of the
function, taking care of aliasing the same way we already do for uIn
and v.
Fixes #22830
Change-Id: I3ab81120d5af6db7772a062bb1dfc011de91f7ad
Reviewed-on: https://go-review.googlesource.com/78995
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-on: https://go-review.googlesource.com/88322
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Austin Clements [Tue, 14 Nov 2017 23:08:32 +0000 (15:08 -0800)]
[release-branch.go1.9] runtime: fix gctrace STW CPU time and CPU fraction
The CPU time reported in the gctrace for STW phases is simply
work.stwprocs times the wall-clock duration of these phases. However,
work.stwprocs is set to gcprocs(), which is wrong for multiple
reasons:
1. gcprocs is intended to limit the number of Ms used for mark
termination based on how well the garbage collector actually
scales, but the gctrace wants to report how much CPU time is being
stolen from the application. During STW, that's *all* of the CPU,
regardless of how many the garbage collector can actually use.
2. gcprocs assumes it's being called during STW, so it limits its
result to sched.nmidle+1. However, we're not calling it during STW,
so sched.nmidle is typically quite small, even if GOMAXPROCS is
quite large.
Fix this by setting work.stwprocs to min(ncpu, GOMAXPROCS). This also
fixes the overall GC CPU fraction, which is based on the computed CPU
times.
KeepAlive needs to introduce a use of the spill of the
value it is keeping alive. Without that, we don't guarantee
that the spill dominates the KeepAlive.
This bug was probably introduced with the code to move spills
down to the dominator of the restores, instead of always spilling
just after the value itself (CL 34822).
Fixes #22458.
Change-Id: I94955a21960448ffdacc4df775fe1213967b1d4c
Reviewed-on: https://go-review.googlesource.com/74210 Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/88318
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Russ Cox [Sun, 15 Oct 2017 02:47:24 +0000 (22:47 -0400)]
[release-branch.go1.9] runtime: use simple, more robust fastrandn
CL 36932 (speed up fastrandn) made it faster but introduced
bad interference with some properties of fastrand itself, making
fastrandn not very random in certain ways. In particular, certain
selects are demonstrably unfair.
For Go 1.10 the new faster fastrandn has induced a new fastrand,
which in turn has caused other follow-on bugs that are still being
discovered and fixed.
For Go 1.9.2, just go back to the barely slower % implementation
that we used in Go 1.8 and earlier. This should restore fairness in
select and any other problems caused by the clever fastrandn.
The test in this CL is copied from CL 62530.
Fixes #22253.
Change-Id: Ibcf948a7bce981452e05c90dbdac122043f6f813
Reviewed-on: https://go-review.googlesource.com/70991
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
But that resulted in replacing z with a value whose type
is only int16. Then if z is spilled and restored, it gets
zero extended instead of sign extended.
Instead use the rule
(MOVWQSX (MOVBQSX x)) -> (MOVBQSX x)
The result is has the correct type, so it can be spilled
and restored correctly. It might mean that a few more extension
ops might not be eliminated, but that's the price for correctness.
Keith Randall [Mon, 28 Aug 2017 19:57:52 +0000 (12:57 -0700)]
[release-branch.go1.9] cmd/compile: avoid generating large offsets
The assembler barfs on large offsets. Make sure that all the
instructions that need to have their offsets in an int32
1) check on any rule that computes offsets for such instructions
2) change their aux fields so the check builder checks it.
The assembler also silently misassembled offsets between 1<<31
and 1<<32. Add a check in the assembler to barf on those as well.
Fixes #21655
Change-Id: Iebf24bf10f9f37b3ea819ceb7d588251c0f46d7d
Reviewed-on: https://go-review.googlesource.com/59630
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/70981
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Michael Matloob [Tue, 12 Sep 2017 16:22:22 +0000 (12:22 -0400)]
[release-branch.go1.9] runtime: in cpuProfile.addExtra, set p.lostExtra to 0 after flush
After the number of lost extra events are written to the the cpuprof log,
the number of lost extra events should be set to zero, or else, the next
time time addExtra is logged, lostExtra will be overcounted. This change
resets lostExtra after its value is written to the log.
Hiroshi Ioka [Thu, 31 Aug 2017 04:49:43 +0000 (13:49 +0900)]
[release-branch.go1.9] cmd/cgo: support large unsigned macro again
The approach of https://golang.org/cl/43476 turned out incorrect.
The problem is that the sniff introduced by the CL only work for simple
expression. And when it fails it fallback to uint64, not int64, which
breaks backward compatibility.
In this CL, we use DWARF for guessing kind instead. That should be more
reliable than previous approach. And importanly, it fallbacks to int64 even
if it fails to guess kind.
Fixes #21708
Change-Id: I39a18cb2efbe4faa9becdcf53d5ac68dba180d47
Reviewed-on: https://go-review.googlesource.com/60510
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/60810 Reviewed-by: Hiroshi Ioka <hirochachacha@gmail.com> Reviewed-by: Chris Broadfoot <cbro@golang.org>
Reviewed-on: https://go-review.googlesource.com/70970
Run-TryBot: Russ Cox <rsc@golang.org>
Russ Cox [Wed, 25 Oct 2017 15:13:23 +0000 (11:13 -0400)]
[release-branch.go1.9] runtime: avoid monotonic time zero on systems with low-res timers
Otherwise low-res timers cause problems at call sites that expect to
be able to use 0 as meaning "no time set" and therefore expect that
nanotime never returns 0 itself. For example, sched.lastpoll == 0
means no last poll.
Russ Cox [Tue, 17 Oct 2017 19:10:59 +0000 (15:10 -0400)]
[release-branch.go1.9] cmd/go: clean up x.exe properly in TestImportMain
More generally I'm concerned about these tests using
$GOROOT/src/cmd/go as scratch space, especially
combined wtih tg.parallel() - it's easy to believe some other
test might inadvertently also try to write x.exe about the
same time. This CL only solves the "didn't clean up x.exe"
problem and leaves for another day the "probably shouldn't
write to cmd/go at all" problem.
Fixes #22266.
Change-Id: I651534d70e2d360138e0373fb4a316081872550b
Reviewed-on: https://go-review.googlesource.com/71410
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/71530
Matthew Dempsky [Fri, 13 Oct 2017 21:47:45 +0000 (14:47 -0700)]
[release-branch.go1.9] cmd/compile: omit ICE diagnostics after normal error messages
After we detect errors, the AST is in a precarious state and more
likely to trip useless ICE failures. Instead let the user fix any
existing errors and see if the ICE persists. This makes Fatalf more
consistent with how panics are handled by hidePanic.
While here, also fix detection for release versions: release version
strings begin with "go" ("go1.8", "go1.9.1", etc), not "release".
Daniel Theophanes [Sat, 23 Sep 2017 22:30:46 +0000 (15:30 -0700)]
[release-branch.go1.9] database/sql: prevent race in driver by locking dc in Next
Database drivers should be called from a single goroutine to ease
driver's design. If a driver chooses to handle context
cancels internally it may do so.
The sql package violated this agreement when calling Next or
NextResultSet. It was possible for a concurrent rollback
triggered from a context cancel to call a Tx.Rollback (which
takes a driver connection lock) while a Rows.Next is in progress
(which does not tack the driver connection lock).
The current internal design of the sql package is each call takes
roughly two locks: a closemu lock which prevents an disposing of
internal resources (assigning nil or removing from lists)
and a driver connection lock that prevents calling driver code from
multiple goroutines.
Fixes #21117
Change-Id: Ie340dc752a503089c27f57ffd43e191534829360
Reviewed-on: https://go-review.googlesource.com/65731 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/71510
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Alex Brainman [Wed, 11 Oct 2017 07:15:25 +0000 (18:15 +1100)]
[release-branch.go1.9] internal/poll: do not call SetFileCompletionNotificationModes if it is broken
Current code assumes that SetFileCompletionNotificationModes
is safe to call even if we know that it is not safe to use
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag. It appears (see issue #22149),
SetFileCompletionNotificationModes crashes when we call it without
FILE_SKIP_COMPLETION_PORT_ON_SUCCESS flag.
Do not call SetFileCompletionNotificationModes in that situation.
We are allowed to do that, because SetFileCompletionNotificationModes
is just an optimisation.
Fixes #22149
Change-Id: I0ad3aff4eabd8c27739417a62c286b1819ae166a
Reviewed-on: https://go-review.googlesource.com/69870 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/70989
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Ian Lance Taylor [Tue, 10 Oct 2017 21:10:28 +0000 (14:10 -0700)]
[release-branch.go1.9] cmd/go: correct directory used in checkNestedVCS test
This error was not used when using git because nested git is permitted.
Add test using Mercurial, so that at least we have a test, even though
the test is not run by default.
Fixes #22157
Fixes #22201
Change-Id: If521f3c09b0754e00e56fa3cd0364764a57a43ad
Reviewed-on: https://go-review.googlesource.com/69670
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/70984
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Adam Langley [Fri, 6 Oct 2017 19:46:22 +0000 (12:46 -0700)]
[release-branch.go1.9] crypto/x509: reject intermediates with unknown critical extensions.
In https://golang.org/cl/9390 I messed up and put the critical extension
test in the wrong function. Thus it only triggered for leaf certificates
and not for intermediates or roots.
In practice, this is not expected to have a security impact in the web
PKI.
Change-Id: I4f2464ef2fb71b5865389901f293062ba1327702
Reviewed-on: https://go-review.googlesource.com/69294
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/70983
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Mon, 2 Oct 2017 22:47:41 +0000 (15:47 -0700)]
[release-branch.go1.9] cmd/compile: fix merge rules for panic calls
Use entire inlining call stack to decide whether two panic calls
can be merged. We used to merge panic calls when only the leaf
line numbers matched, but that leads to places higher up the call
stack being merged incorrectly.
[release-branch.go1.9] runtime: make runtime.GC() trigger GC even if GOGC=off
Currently, the priority of checks in (gcTrigger).test() puts the
gcpercent<0 test above gcTriggerCycle, which is used for runtime.GC().
This is an unintentional change from 1.8 and before, where
runtime.GC() triggered a GC even if GOGC=off.
Fix this by rearranging the priority so the gcTriggerCycle test
executes even if gcpercent < 0.
[release-branch.go1.9] cmd/compile: fix regression in PPC64.rules move zero
When a MOVDstorezero (8 bytes) is used the offset field
in the instruction must be a multiple of 4. This situation
had been corrected in the rules for other types of stores
but not for the zero case.
This also removes some of the special MOVDstorezero cases since
they can be handled by the general LowerZero case.
Updates made to the ssa test for lowering zero moves to
include cases where the target is not aligned to at least 4.
Alex Brainman [Mon, 25 Sep 2017 08:54:14 +0000 (18:54 +1000)]
[release-branch.go1.9] internal/poll: be explicit when using runtime netpoller
internal/poll package assumes that only net sockets use runtime
netpoller on windows. We get memory corruption if other file
handles are passed into runtime poller. Make FD.Init receive
and use useNetpoller argument, so FD.Init caller is explicit
about using runtime netpoller.
Fixes #21172
Change-Id: I60e2bfedf9dda9b341eb7a3e5221035db29f5739
Reviewed-on: https://go-review.googlesource.com/65810 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/71132
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
The previous code seems to have an off-by-1 in it somewhere, the
consequence being that we didn't properly preserve all of the old
buffer contents that we intended to.
After spending a while looking at the existing window-shifting logic,
I wasn't able to understand exactly how it was supposed to work or
where the issue was, so I rewrote it to be (at least IMO) more
obviously correct.
Fixes #21938.
Change-Id: I1ed7bbc1e1751a52ab5f7cf0411ae289586dc345
Reviewed-on: https://go-review.googlesource.com/64830
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-on: https://go-review.googlesource.com/70977
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
[release-branch.go1.9] reflect: fix pointer past-the-end in Call with zero-sized return value
If a function with nonzero frame but zero-sized return value is
Call'd, we may write a past-the-end pointer in preparing the
return Values. Fix by return the zero value for zero-sized
return value.
[release-branch.go1.9] log: fix data race on log.Output
There was unprotected access to Logger.flag in log.Output which
could lead to data race in cases when log.SetFlags called simultaneously.
For example, "hot" switching on/off debug-mode for Logger by log.SetFlags
while application still writing logs.
David Crawshaw [Wed, 13 Sep 2017 23:04:25 +0000 (19:04 -0400)]
[release-branch.go1.9] cmd/compile: replace GOROOT in //line directives
The compiler replaces any path of the form /path/to/goroot/src/net/port.go
with GOROOT/src/net/port.go so that the same object file is
produced if the GOROOT is moved. It was skipping this transformation
for any absolute path into the GOROOT that came from //line directives,
such as those generated by cmd/cgo.
Austin Clements [Thu, 24 Aug 2017 19:06:26 +0000 (15:06 -0400)]
[release-branch.go1.9] runtime: capture runtimeInitTime after nanotime is initialized
CL 36428 changed the way nanotime works so on Darwin and Windows it
now depends on runtime.startNano, which is computed at runtime.init
time. Unfortunately, the `runtimeInitTime = nanotime()` initialization
happened *before* runtime.init, so on these platforms runtimeInitTime
is set incorrectly. The one (and only) consequence of this is that the
start time printed in gctrace lines is bogus:
gc 1 18446653480.186s 0%: 0.092+0.47+0.038 ms clock, 0.37+0.15/0.81/1.8+0.15 ms cpu, 4->4->1 MB, 5 MB goal, 8 P
To fix this, this commit moves the runtimeInitTime initialization to
shortly after runtime.init, at which point nanotime is safe to use.
This also requires changing the condition in newproc1 that currently
uses runtimeInitTime != 0 simply to detect whether or not the main M
has started. Since runtimeInitTime could genuinely be 0 now, this
introduces a separate flag to newproc1.
Tom Bergan [Mon, 28 Aug 2017 18:09:37 +0000 (11:09 -0700)]
[release-branch.go1.9] doc/1.9: add mention of net/http.LocalAddrContextKey
Fixes #21603
Reviewed-on: https://go-review.googlesource.com/59530 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/59670 Reviewed-by: Chris Broadfoot <cbro@golang.org> Reviewed-by: Tom Bergan <tombergan@google.com>
Change-Id: Ie9732d57948593dc0306a4a649664eedb3de370c
Reviewed-on: https://go-review.googlesource.com/68232 Reviewed-by: Chris Broadfoot <cbro@golang.org>
Russ Cox [Wed, 4 Oct 2017 17:24:49 +0000 (13:24 -0400)]
[release-branch.go1.9] net/smtp: fix PlainAuth to refuse to send passwords to non-TLS servers
PlainAuth originally refused to send passwords to non-TLS servers
and was documented as such.
In 2013, issue #5184 was filed objecting to the TLS requirement,
despite the fact that it is spelled out clearly in RFC 4954.
The only possibly legitimate use case raised was using PLAIN auth
for connections to localhost, and the suggested fix was to let the
server decide: if it advertises that PLAIN auth is OK, believe it.
That approach was adopted in CL 8279043 and released in Go 1.1.
Unfortunately, this is exactly wrong. The whole point of the TLS
requirement is to make sure not to send the password to the wrong
server or to a man-in-the-middle. Instead of implementing this rule,
CL 8279043 blindly trusts the server, so that if a man-in-the-middle
says "it's OK, you can send me your password," PlainAuth does.
And the documentation was not updated to reflect any of this.
This CL restores the original TLS check, as required by RFC 4954
and as promised in the documentation for PlainAuth.
It then carves out a documented exception for connections made
to localhost (defined as "localhost", "127.0.0.1", or "::1").
[release-branch.go1.9] cmd/compile: remove gc.Sysfunc calls from 387 backend
[This is a cherry-pick of CL 54090 to the 1.9 release branch.]
gc.Sysfunc must not be called concurrently.
We set up runtime routines used by the backend
prior to doing any backend compilation.
I missed the 387 ones; fix that.
Sysfunc should have been unexported during 1.9.
I will rectify that in a subsequent CL.
Austin Clements [Thu, 17 Aug 2017 22:40:07 +0000 (18:40 -0400)]
[release-branch.go1.9] runtime: fix false positive race in profile label reading
Because profile labels are copied from the goroutine into the tag
buffer by the signal handler, there's a carefully-crafted set of race
detector annotations to create the necessary happens-before edges
between setting a goroutine's profile label and retrieving it from the
profile tag buffer.
Given the constraints of the signal handler, we have to approximate
the true synchronization behavior. Currently, that approximation is
too weak.
Ideally, runtime_setProfLabel would perform a store-release on
&getg().labels and copying each label into the profile would perform a
load-acquire on &getg().labels. This would create the necessary
happens-before edges through each individual g.labels object.
Since we can't do this in the signal handler, we instead synchronize
on a "labelSync" global. The problem occurs with the following
sequence:
1. Goroutine 1 calls setProfLabel, which does a store-release on
labelSync.
2. Goroutine 2 calls setProfLabel, which does a store-release on
labelSync.
3. Goroutine 3 reads the profile, which does a load-acquire on
labelSync.
The problem is that the load-acquire only synchronizes with the *most
recent* store-release to labelSync, and the two store-releases don't
synchronize with each other. So, once goroutine 3 touches the label
set by goroutine 1, we report a race.
The solution is to use racereleasemerge. This is like a
read-modify-write, rather than just a store-release. Each RMW of
labelSync in runtime_setProfLabel synchronizes with the previous RMW
of labelSync, and this ultimately carries forward to the load-acquire,
so it synchronizes with *all* setProfLabel operations, not just the
most recent.
Change-Id: Iab58329b156122002fff12cfe64fbeacb31c9613
Reviewed-on: https://go-review.googlesource.com/57190
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Fri, 18 Aug 2017 19:58:08 +0000 (12:58 -0700)]
[release-branch.go1.9] testing: don't fail all tests after racy test failure
The code was adding race.Errors to t.raceErrors before checking
Failed, but Failed was using t.raceErrors+race.Errors. We don't want
to change Failed, since that would affect tests themselves, so modify
the harness to not unnecessarily change t.raceErrors.
Updates #19851
Fixes #21338
Change-Id: I483f27c68c340928f1cbdef160abc0a5716efb5d
Reviewed-on: https://go-review.googlesource.com/57151
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Cherry Zhang [Fri, 18 Aug 2017 12:37:58 +0000 (08:37 -0400)]
[release-branch.go1.9] cmd/compile: add rules handling unsigned div/mod by constant 1<<63
Cherry-pick CL 56890.
Normally 64-bit div/mod is turned into runtime calls on 32-bit
arch, but the front end leaves power-of-two constant division
and hopes the SSA backend turns into a shift or AND. The SSA rule is
(Mod64u <t> n (Const64 [c])) && isPowerOfTwo(c) -> (And64 n (Const64 <t> [c-1]))
But isPowerOfTwo returns true only for positive int64, which leaves
out 1<<63 unhandled. Add a special case for 1<<63.