Lynn Boger [Tue, 4 Dec 2018 20:15:27 +0000 (15:15 -0500)]
runtime: fix runtime-gdb.py when switching sp value
After a recent change to runtime-gdb_test.go the ppc64le builder
has had intermittent failures. The failures occur when trying to
invoke the goroutineCmd function to display the backtrace for
a selected goroutine. There is nothing wrong with the testcase
but it seems to intermittently leave goroutines in a state
where an error can occur.
The error message indicates that the problem occurs when trying
to change the sp back to the original after displaying the
stacktrace for the goroutine.
gdb.error: Attempt to assign to an unmodifiable value.
After some searching I found that this error message can happen
if the sp register is changed when on a frame that is not the
top-most frame. To fix the problem, frame 0 is selected before
changing the value of sp. This fixes the problem in my
reproducer environment, and hopefully will fix the problem on
the builder.
Michael Anthony Knyszek [Thu, 6 Dec 2018 21:51:51 +0000 (21:51 +0000)]
runtime: enable preemption of mark termination goroutine
A mark worker goroutine may attempt to preempt the mark termination
goroutine to scan its stack while the mark termination goroutine is
trying to preempt that worker to flush its work buffer, in rare
cases.
This change makes it so that, like a worker goroutine, the mark
termination goroutine stack is preemptible while it is on the
system stack, attempting to preempt others.
Bryan C. Mills [Fri, 7 Dec 2018 15:14:42 +0000 (10:14 -0500)]
doc: mention the use of replacements to resolve imports for 1.12
Updates #26241
Change-Id: I8ffac13d9cc1ee4d4de8fcd2042a7fa60fca567b
Reviewed-on: https://go-review.googlesource.com/c/153157 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Thu, 6 Dec 2018 14:03:59 +0000 (09:03 -0500)]
doc: announce the end of support for binary-only packages
Updates #28152
Change-Id: If859221afc683b392f649e79d7ff0a06125cbe10
Reviewed-on: https://go-review.googlesource.com/c/152918 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Fri, 7 Dec 2018 15:25:50 +0000 (07:25 -0800)]
cmd/internal/obj/s390x: don't crash on invalid instruction
I didn't bother with a test as there doesn't seem to be an existing
framework for testing assembler failures, and tests for invalid code
aren't all that interesting.
Fixes #26700
Change-Id: I719410d83527802a09b9d38625954fdb36a3c0f7
Reviewed-on: https://go-review.googlesource.com/c/153177
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Robert Griesemer [Thu, 6 Dec 2018 21:32:50 +0000 (13:32 -0800)]
math: document sign bit correspondence for floating-point/bits conversions
Fixes #27736.
Change-Id: Ibda7da7ec6e731626fc43abf3e8c1190117f7885
Reviewed-on: https://go-review.googlesource.com/c/153057 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tobias Klauser [Thu, 6 Dec 2018 19:58:26 +0000 (20:58 +0100)]
crypto/x509: explicitly cast printf format argument
After CL 128056 the build fails on darwin/386 with
src/crypto/x509/root_cgo_darwin.go:218:55: warning: values of type 'SInt32' should not be used as format arguments; add an explicit cast to 'int' instead [-Wformat]
go build crypto/x509: C compiler warning promoted to error on Go builders
Fix the warning by explicitly casting the argument to an int as
suggested by the warning.
Change-Id: Icb6bd622a543e9bc5f669fd3d7abd418b4a8e579
Reviewed-on: https://go-review.googlesource.com/c/152958
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Wed, 5 Dec 2018 14:52:58 +0000 (09:52 -0500)]
cmd/go/internal/modload: use replacements to resolve missing imports
If the replacements specify one or more versions, we choose the latest
(for consistency with the QueryPackage path, with resolves the latest
version from upstream).
Otherwise, we synthesize a pseudo-version with a zero timestamp and an
appropriate major version.
Bryan C. Mills [Wed, 5 Dec 2018 21:33:56 +0000 (16:33 -0500)]
doc: 1.12 release notes for cmd/go
Change-Id: I1a0bedc9fbd42e138eb68af8365115339e377856
Reviewed-on: https://go-review.googlesource.com/c/152742 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Wed, 5 Dec 2018 22:42:47 +0000 (14:42 -0800)]
cmd/compile/internal/syntax: remove unused field in (scanner) source
The source.offs field was intended for computing line offsets which
may allow a tiny optimization (see TODO in source.go). We haven't
done the optimization, so for now just remove the field to avoid
confusion. It's trivially added if needed.
While at it, also:
- Fix comment for ungetr2.
- Make sure sentinel is present even if reading from the io.Reader failed.
Change-Id: Ib056c6478030b3fe5fec29045362c8161ff3d19e
Reviewed-on: https://go-review.googlesource.com/c/152763
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Andrew Bonventre [Wed, 5 Dec 2018 18:22:44 +0000 (13:22 -0500)]
doc/go1.12: add more release notes for various packages
Change-Id: Ie11cf7d8204860f5a61ab650589d44072d6b131c
Reviewed-on: https://go-review.googlesource.com/c/152740 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Filippo Valsorda [Mon, 6 Aug 2018 22:38:38 +0000 (18:38 -0400)]
crypto/x509: re-enable TestSystemRoots
Now that the cgo and no-cgo paths should be correct and equivalent,
re-enable the TestSystemRoots test without any margin of error (which
was tripping anyway when users had too many of a certain edge-case).
As a last quirk, the verify-cert invocation will validate certificates
that aren't roots, but are signed by valid roots. Ignore them.
Filippo Valsorda [Mon, 6 Aug 2018 22:38:18 +0000 (18:38 -0400)]
crypto/x509: fix root CA extraction on macOS (no-cgo path)
Certificates without any trust settings might still be in the keychain
(for example if they used to have some, or if they are intermediates for
offline verification), but they are not to be trusted. The only ones we
can trust unconditionally are the ones in the system roots store.
Moreover, the verify-cert invocation was not specifying the ssl policy,
defaulting instead to the basic one. We have no way of communicating
different usages in a CertPool, so stick to the WebPKI use-case as the
primary one for crypto/x509.
Updates #24652
Change-Id: Ife8b3d2f4026daa1223aa81fac44aeeb4f96528a
Reviewed-on: https://go-review.googlesource.com/c/128116 Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@golang.org>
Filippo Valsorda [Mon, 6 Aug 2018 19:41:34 +0000 (15:41 -0400)]
crypto/x509: fix root CA extraction on macOS (cgo path)
The cgo path was not taking policies into account, using the last
security setting in the array whatever it was. Also, it was not aware of
the defaults for empty security settings, and for security settings
without a result type. Finally, certificates restricted to a hostname
were considered roots.
The API docs for this code are partial and not very clear, so this is a
best effort, really.
Updates #24652
Change-Id: I8fa2fe4706f44f3d963b32e0615d149e997b537d
Reviewed-on: https://go-review.googlesource.com/c/128056
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@golang.org>
Austin Clements [Wed, 5 Dec 2018 20:23:26 +0000 (15:23 -0500)]
cmd/compile: omit write barriers for slice clears of go:notinheap pointers
Currently,
for i := range a {
a[i] = nil
}
will compile to have write barriers even if a is a slice of pointers
to go:notinheap types. This happens because the optimization that
transforms this into a memclr only asks it a's element type has
pointers, and not if it specifically has heap pointers.
Fix this by changing arrayClear to use HasHeapPointer instead of
types.Haspointers. We probably shouldn't have both of these functions,
since a pointer to a notinheap type is effectively a uintptr, but
that's not going to change in this CL.
Austin Clements [Wed, 5 Dec 2018 20:21:17 +0000 (15:21 -0500)]
cmd/compile: mark memclrHasPointers calls as write barriers
There are two places where the compiler generates memclrHasPointers
calls. These are effectively write barriers, but the compiler doesn't
currently record them as such in the function. As a result code like
for i := range a {
a[i] = nil
}
inserts a write barrier for the assignment to a[i], but the compiler
doesn't report this. Hence, it's not reported in the -d=wb output, and
it's not checked against //go:nowritebarrier annotations.
This updates master to fix the ppc64 objdump. There were many cases where
the Go objdump was generating opcodes that didn't exist in the Go assembler,
or generated operands in the wrong order. The goal is to generate a Go
objdump that is acceptable to the Go assembler, or as close as possible.
An additional change will be needed for the Go objdump tool to make
use of this.
Andrew Bonventre [Tue, 4 Dec 2018 21:22:26 +0000 (16:22 -0500)]
doc/go1.12: add some package release notes
Change-Id: I845eab3c98a3d472c71310de4e0475045eb59d4e
Reviewed-on: https://go-review.googlesource.com/c/152619 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Wed, 5 Dec 2018 13:15:37 +0000 (14:15 +0100)]
cmd/go/internal/lockedfile: fix filelock.Unlock() called twice
filelock.Unlock() was called twice for fcntl implementation if an error
occurs during file.{,R}Lock(): once in the error handler, once in
filelock.lock().
Change-Id: I5ad84e8ef6b5e51d79e0a7a0c51f465276cd0c57
Reviewed-on: https://go-review.googlesource.com/c/152717
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Hiroshi Ioka [Tue, 4 Dec 2018 23:01:07 +0000 (08:01 +0900)]
cmd/cgo: reject names that are likely to be mangled C name
Fixes #28721
Change-Id: I00356f3a9b0c2fb21dc9c2237dd5296fcb3b319b
Reviewed-on: https://go-review.googlesource.com/c/152657
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
In the s390x assembly implementation of NIST P-256 curve, utilize faster multiply/square
instructions introduced in the z14. These new instructions are designed for crypto
and are constant time. The algorithm is unchanged except for faster
multiplication when run on a z14 or later. On z13, the original mutiplication
(also constant time) is used.
P-256 performance is critical in many applications, such as Blockchain.
name old time new time delta
BaseMultP256 24396 ns/op 21564 ns/op 1.13x
ScalarMultP256 87546 ns/op 72813 ns/op. 1.20x
Change-Id: I7e6d8b420fac56d5f9cc13c9423e2080df854bac
Reviewed-on: https://go-review.googlesource.com/c/146022 Reviewed-by: Michael Munday <mike.munday@ibm.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Alex Brainman [Tue, 4 Dec 2018 01:05:11 +0000 (12:05 +1100)]
runtime: correct isAbortPC check in isgoexception
The expression passed into isAbortPC call was written specifically
for windows/amd64 and windows/386 runtime.abort implementation.
Adjust the code, so it also works for windows/arm.
Fixes #29050
Change-Id: I3dc8ddd08031f34115396429eff512827264826f
Reviewed-on: https://go-review.googlesource.com/c/152357 Reviewed-by: Ian Lance Taylor <iant@golang.org>
smasher164 [Tue, 4 Dec 2018 11:41:39 +0000 (06:41 -0500)]
cmd/compile: improve error message for non-final variadic parameter
Previously, when a function signature had defined a non-final variadic
parameter, the error message always referred to the type associated with that
parameter. However, if the offending parameter's name was part of an identifier
list with a variadic type, one could misinterpret the message, thinking the
problem had been with one of the other names in the identifer list.
func bar(a, b ...int) {}
clear ~~~~~~~^ ^~~~~~~~ confusing
This change updates the error message and sets the column position to that of
the offending parameter's name, if it exists.
Fixes #28450.
Change-Id: I076f560925598ed90e218c25d70f9449ffd9b3ea
Reviewed-on: https://go-review.googlesource.com/c/152417
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Cherry Zhang [Tue, 4 Dec 2018 20:54:14 +0000 (15:54 -0500)]
cmd/compile: fix unnamed parameter handling in escape analysis
For recursive functions, the parameters were iterated using
fn.Name.Defn.Func.Dcl, which does not include unnamed/blank
parameters. This results in a mismatch in formal-actual
assignments, for example,
func f(_ T, x T)
f(a, b) should result in { _=a, x=b }, but the escape analysis
currently sees only { x=a } and drops b on the floor. This may
cause b to not escape when it should (or a escape when it should
not).
Fix this by using fntype.Params().FieldSlice() instead, which
does include unnamed parameters.
Also add a sanity check that ensures all the actual parameters
are consumed.
Fixes #29000
Change-Id: Icd86f2b5d71e7ebbab76e375b7702f62efcf59ae
Reviewed-on: https://go-review.googlesource.com/c/152617 Reviewed-by: Keith Randall <khr@golang.org>
Daniel Martí [Tue, 4 Dec 2018 16:25:39 +0000 (16:25 +0000)]
cmd/go: revert multi-flag GOFLAGS doc example
This partially reverts https://golang.org/cl/135035.
Reason for revert: multiple -ldflags=-foo flags simply override each
other, since that's the logic for per-package flags. The suggested
'GOFLAGS=-ldflags=-s -ldflags=-w' has never worked for 'go build', and
even breaks 'go test' and 'go vet'.
There should be a way to specify -ldflags='-w -s' via GOFLAGS, which is
being tracked in #29096. For now, just remove the incorrect suggestion.
Fixes #29053.
Change-Id: I9203056f7e5191e894bcd16595a92df2fb704ea7
Reviewed-on: https://go-review.googlesource.com/c/152479 Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Brad Fitzpatrick [Tue, 4 Dec 2018 19:31:52 +0000 (19:31 +0000)]
doc/go1.12: flesh out net, etc
Change-Id: I081400286544d88eec83a8b332b9f7934fd76ae2
Reviewed-on: https://go-review.googlesource.com/c/152539 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alan Donovan [Fri, 2 Nov 2018 15:27:53 +0000 (11:27 -0400)]
go/importer: add ForCompiler, which accepts a token.FileSet
The importer.For function logically requires a FileSet, but did not
when it was first created because export data did not then record
position information. This change adds a new function, ForCompiler,
that has an additional FileSet parameter, and deprecates the For
function.
Before this change, cmd/vet would report confusing spurious
positions for token.Pos values produced by the importer.
The bug is essentially unfixable in cmd/vet.
This CL includes a test that the FileSet is correctly populated.
The changes to cmd/vendor will be applied upstream in a follow-up.
Fixes #28995
Change-Id: I9271bcb1f28e96845c913e15f0304bac93d4d4c4
Reviewed-on: https://go-review.googlesource.com/c/152258
Run-TryBot: Alan Donovan <adonovan@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Robert Griesemer <gri@golang.org>
Agniva De Sarker [Wed, 10 Oct 2018 10:22:03 +0000 (15:52 +0530)]
go/doc: tune factory method association logic
Ignore predeclared types (such as error) in result parameter lists when determining
with which result type a method should be associated with. This change will again
associate common factory functions with the first result type even if there are more
than one result, as long as the others are predeclared types.
Fixes #27928
Change-Id: Ia2aeaed15fc4c8debdeeaf729cc7fbba1612cafb
Reviewed-on: https://go-review.googlesource.com/c/141617
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Clément Chigot [Tue, 4 Dec 2018 08:03:32 +0000 (09:03 +0100)]
syscall, cmd/go/internal/lockedfile: remove Flock syscall for aix/ppc64
AIX doesn't provide flock() syscall, it was previously emulated by fcntl
calls. However, there are some differences between a flock() syscall and
a flock() using fcntl. Therefore, it's safer to remove it and just
provide FcntlFlock.
Thus, lockedfile implementation must be moved to use FcntlFlock on aix/ppc64.
Lynn Boger [Mon, 3 Dec 2018 22:02:20 +0000 (17:02 -0500)]
cmd/go: add missing gccgo checks for buildmodeInit
Some recent failures in gccgo on linux/ppc64 identified
an error in buildmodeInit when buildmode=c-archive.
A fix went into gofrontend, and this is the
corresponding change for master. This change also includes
two other updates related to gccgo in this function that
were in the file from gofrontend but missing from master.
Updates #29046
Change-Id: I9a894e7d728e31fb9e9344cd61d50408df7faf4a
Reviewed-on: https://go-review.googlesource.com/c/152160
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jay Conrod [Wed, 28 Nov 2018 21:33:23 +0000 (16:33 -0500)]
cmd/go: emit go list error for local non-existant packages
In CL 129061, a check was added for patterns that reference
nonexistent local directories. While this prevented unnecessary
network lookups (fixing #26874), it caused "go list -e" to exit with
an error instead of listing packages with error messages.
This change avoids the network lookup and does not exit for these
kinds of packages. Errors are still reported by
internal/load.LoadImport for packages that don't exist.
Fixes #28023
Change-Id: I0a648269e437aed3a95bfb05461a397264f3793f
Reviewed-on: https://go-review.googlesource.com/c/151800
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Robert Griesemer [Mon, 3 Dec 2018 21:46:14 +0000 (13:46 -0800)]
go/types: fix interface receiver type for incremental type-checking
The type checker may be called incrementally (by repeatedly calling
Checker.Files), for instance when adding _test.go files to a set of
already checked files.
The existing code reset a cache of (already computed) interface
information with each Checker.Files call, causing interfaces to be
recomputed in some cases, albeit with different receiver information
(see comments in this CL for details).
Don't reset the interface cache to avoid this problem.
While adding a test case, also factor out some common testing logic.
Fixes #29029.
Change-Id: I2e2d6d6bb839b3a76522fbc4ba7355c71d3bb80b
Reviewed-on: https://go-review.googlesource.com/c/152259 Reviewed-by: Alan Donovan <adonovan@google.com>
Brad Fitzpatrick [Mon, 3 Dec 2018 17:01:18 +0000 (17:01 +0000)]
net/http: document CanonicalHeaderKey from Header
And remove some unnecessary textproto references. (The net/http
package's CanonicalHeaderKey just calls textproto's
CanonicalMIMEHeaderKey)
Fixes #28894
Change-Id: Ibd277893a168368c593147a2677ad6130870cb88
Reviewed-on: https://go-review.googlesource.com/c/152157 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Sat, 1 Dec 2018 21:21:04 +0000 (13:21 -0800)]
cmd/compile: fix static initializer
staticcopy of a struct or array should recursively call itself, not
staticassign.
This fixes an issue where a struct with a slice in it is copied during
static initialization. In this case, the backing array for the slice
is duplicated, and each copy of the slice refers to a different
backing array, which is incorrect. That issue has existed since at
least Go 1.2.
I'm not sure why this was never noticed. It seems like a pretty
obvious bug if anyone modifies the resulting slice.
In any case, we started to notice when the optimization in CL 140301
landed. Here is basically what happens in issue29013b.go:
1) The error above happens, so we get two backing stores for what
should be the same slice.
2) The code for initializing those backing stores is reused.
But not duplicated: they are the same Node structure.
3) The order pass allocates temporaries for the map operations.
For the first instance, things work fine and two temporaries are
allocated and stored in the OKEY nodes. For the second instance,
the order pass decides new temporaries aren't needed, because
the OKEY nodes already have temporaries in them.
But the order pass also puts a VARKILL of the temporaries between
the two instance initializations.
4) In this state, the code is technically incorrect. But before
CL 140301 it happens to work because the temporaries are still
correctly initialized when they are used for the second time. But then...
5) The new CL 140301 sees the VARKILLs and decides to reuse the
temporary for instance 1 map 2 to initialize the instance 2 map 1
map. Because the keys aren't re-initialized, instance 2 map 1
gets the wrong key inserted into it.
Carlo Alberto Ferraris [Sun, 15 Apr 2018 02:34:19 +0000 (11:34 +0900)]
net: enable TCP keepalives by default
This is just the first step in attempting to make all network connection have
timeouts as a "safe default". TCP keepalives only protect against certain
classes of network and host issues (e.g. server/OS crash), but do nothing
against application-level issues (e.g. an application that accepts connections
but then fails to serve requests).
The actual keep-alive duration (15s) is chosen to cause broken connections
to be closed after 2~3 minutes (depending on the OS, see #23549 for details).
We don't make the actual default value part of the public API for a number of
reasons:
- because it's not very useful by itself: as discussed in #23549 the actual
"timeout" after which the connection is torn down is duration*(KEEPCNT+1),
and we use the OS-wide value for KEEPCNT because there's currently no way
to set it from Go.
- because it may change in the future: if users need to rely on a specific
value they should explicitly set this value instead of relying on the default.
Fixes #23459
Change-Id: I348c03be97588d5001e6de0f377e7a93b51957fd
Reviewed-on: https://go-review.googlesource.com/c/107196
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Leon Klingele [Mon, 3 Dec 2018 04:44:48 +0000 (04:44 +0000)]
net/http: add StatusTooEarly (425)
StatusTooEarly can be returned to indicate that a server is unwilling
to accept early data as introduced in TLS 1.3.
The status code was specified in RFC 8470, section 5.2.
Major supported browsers are:
- Firefox as of version 58
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/425#Browser_compatibility
- Chromium as of version 73.0.3628.1
https://chromium.googlesource.com/chromium/src/+/58097ec3823e0f340ab5abfcaec1306e1d954c5a
David Chase [Thu, 1 Nov 2018 19:26:02 +0000 (15:26 -0400)]
cmd/compile: for location lists, handle case where prev block is not a pred
Before this change, location list construction would extend
from the previous (in linear order) block, even if was not a
flow predecessor. This can cause a debugger to tell lies.
Fix accounts for this in block merging code by (crudely)
"changing" all variables live from a previous block if it
is not also a predecessor.
Daniel Martí [Sat, 1 Dec 2018 16:28:27 +0000 (16:28 +0000)]
cmd/compile: add Buffer.Grow to TestIntendedInlining
golang.org/cl/151977 slightly decreased the cost of inlining an extra
call from 60 to 57, since it was a safe change that could help in some
scenarios.
One notable change spotted in that CL is that bytes.Buffer.Grow is now
inlinable, meaning that a fixedbugs test needed updating.
For consistency, add the test case to TestIntendedInlining too,
alongside other commonly used bytes.Buffer methods.
David Chase [Fri, 30 Nov 2018 13:36:00 +0000 (08:36 -0500)]
cmd/compile: decrease inlining call cost from 60 to 57
A Go user made a well-documented request for a slightly
lower threshold. I tested against a selection of other
people's benchmarks, and saw a tiny benefit (possibly noise)
at equally tiny cost, and no unpleasant surprises observed
in benchmarking.
I.e., might help, doesn't hurt, low risk, request was
delivered on a silver platter.
It did, however, change the behavior of one test because
now bytes.Buffer.Grow is eligible for inlining.
Updates #19348.
Change-Id: I85e3088a4911290872b8c6bda9601b5354c48695
Reviewed-on: https://go-review.googlesource.com/c/151977
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Ian Lance Taylor [Fri, 30 Nov 2018 22:21:33 +0000 (14:21 -0800)]
cmd/cgo: use preprocessor macros to avoid prolog redefinitions
Avoid redefinition errors when a Go file uses a cgo comment to
There is no particularly good reason to do this, but there is also no
particularly good reason that it should fail.
Fixes #27019
Change-Id: Icd6f8197a89be4ee6b03ddae675667998a8b4189
Reviewed-on: https://go-review.googlesource.com/c/152079
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ben Shi [Fri, 30 Nov 2018 09:30:36 +0000 (09:30 +0000)]
test/codegen: add arithmetic tests for 386/amd64/arm/arm64
This CL adds several test cases of arithmetic operations for
386/amd64/arm/arm64.
Change-Id: I362687c06249f31091458a1d8c45fc4d006b616a
Reviewed-on: https://go-review.googlesource.com/c/151897
Run-TryBot: Ben Shi <powerman1st@163.com> Reviewed-by: Keith Randall <khr@golang.org>
Elias Naur [Fri, 30 Nov 2018 11:25:18 +0000 (12:25 +0100)]
syscall: avoid "64"-postfixed libSystem syscalls on iOS
The stat(2) man page contain this comment about the 64-bit versions
of the system file functions:
"Platforms that were released after these updates only have the
newer variants available to them. These platforms have the macro
_DARWIN_FEATURE_ONLY_64_BIT_INODE defined."
It turns out that on iOS the _DARWIN_FEATURE_ONLY_64_BIT_INODE is
defined and that even though the "64"-postfixed versions are
accessible they are deemed private. Apps that refer to private
API are not admissible on App Store, and after the Go runtime
started using libSystem instead of direct syscalls, the App Store
submission checks reject apps built with Go tip.
The fix is simple: use the non-postfixed versions on iOS.
getdirentries(2) is not changed; it is not available at all on iOS
and needs replacement.
Updates #28984
Change-Id: Icb8d44e271456acaa1913ba486fcf5b569722fa9
Reviewed-on: https://go-review.googlesource.com/c/151938
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Robert Griesemer [Wed, 28 Nov 2018 22:34:45 +0000 (14:34 -0800)]
cmd/compile: fix constant index bounds check and error message
While here, rename nonnegintconst to indexconst (because that's
what it is) and add Fatalf calls where we are not expecting the
indexconst call to fail, and fixed wrong comparison in smallintconst.
Fixes #23781.
Change-Id: I86eb13081c450943b1806dfe3ae368872f76639a
Reviewed-on: https://go-review.googlesource.com/c/151599
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Michael Anthony Knyszek [Wed, 28 Nov 2018 18:52:35 +0000 (18:52 +0000)]
runtime: fix heap pointer invariant rules in HACKING.md
This change fixes an error in HACKING.md which claims all pointers
which live in unmanaged memory but point to the heap must be marked
as GC roots explicitly by runtime.markroot. This isn't technically
necessary if the pointer is accessible through a global variable.
Change-Id: I632b25272fdb2f789c5259dd1685d517f45fd435
Reviewed-on: https://go-review.googlesource.com/c/151539 Reviewed-by: Rick Hudson <rlh@golang.org>
Ian Lance Taylor [Fri, 30 Nov 2018 20:26:10 +0000 (12:26 -0800)]
go/internal/gccgoimporter: fix test when using gccgo 4.7
TestInstallationImporter checks that it can read the export data for a
list of known standard library packages. It was failing on the SmartOS
builder which has GCC 4.7 installed. Skip packages that did not exist
in GCC 4.7. Most packages are still there and the missing packages are
fairly simple, so this doesn't really affect test quality.
Updates #29006
Change-Id: If7ae6f83d51d40168a9692acb0b99c9bf21f2a4d
Reviewed-on: https://go-review.googlesource.com/c/152077
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Than McIntosh [Fri, 30 Nov 2018 14:50:05 +0000 (09:50 -0500)]
go/internal/gccgoimporter: fix bug reading V1 export data
Fix a bug in the reading of elderly export data. In such export data
when reading type information it's possible to encounter a named type N1
defined as a typedef of some other named type N2 at a point when the
underying type of N1 has not yet been finalized. Handle this case by
generating a fixup, then process fixups at the end of parsing to
set the correct underlying type.
Fixes #29006.
Change-Id: I6a505c897bd95eb161ee04637bb6eebad9f20d52
Reviewed-on: https://go-review.googlesource.com/c/151997
Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Filippo Valsorda [Thu, 29 Nov 2018 06:38:07 +0000 (01:38 -0500)]
crypto/tls: fix client certificates support for legacy servers
signatureSchemesForCertificate was written to be used with TLS 1.3, but
ended up used for TLS 1.2 client certificates in a refactor. Since it
only supported TLS 1.3 signature algorithms, it would lead to no RSA
client certificates being sent to servers that didn't support RSA-PSS.
TestHandshakeClientCertRSAPKCS1v15 was testing *specifically* for this,
but alas the OpenSSL flag -verify accepts an empty certificates list as
valid, as opposed to -Verify...
Bryan C. Mills [Thu, 29 Nov 2018 22:46:14 +0000 (17:46 -0500)]
cmd/go/internal/modfetch: make directories read-only after renaming, not before
The call to os.Rename was failing on the darwin builders, despite having passed in the TryBots.
(I tested this change by running 'go test cmd/go' manually on a darwin gomote.)
Austin Clements [Mon, 26 Nov 2018 19:41:23 +0000 (14:41 -0500)]
runtime: check more work flushing races
This adds several new checks to help debug #27993. It adds a mechanism
for freezing write barriers and gcWork puts during the mark completion
algorithm. This way, if we do detect mark completion, we can catch any
puts that happened during the completion algorithm. Based on build
dashboard failures, this seems to be the window of time when these are
happening.
This also double-checks that all work buffers are empty immediately
upon entering mark termination (much earlier than the current check).
This is unlikely to trigger based on the current failures, but is a
good safety net.
Ian Lance Taylor [Tue, 20 Nov 2018 23:55:02 +0000 (15:55 -0800)]
cmd/cgo: use field alignment when setting field offset
The old code ignored the field alignment, and only looked at the field
offset: if the field offset required padding, cgo added padding. But
while that approach works for Go (at least with the gc toolchain) it
doesn't work for C code using packed structs. With a packed struct the
added padding may leave the struct at a misaligned position, and the
inserted alignment, which cgo is not considering, may introduce
additional, unexpected, padding. Padding that ignores alignment is not
a good idea when the struct is not packed, and Go structs are never
packed. So don't ignore alignment.
Fixes #28896
Change-Id: Ie50ea15fa6dc35557497097be9fecfecb11efd8a
Reviewed-on: https://go-review.googlesource.com/c/150602
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Agniva De Sarker [Tue, 20 Nov 2018 05:43:03 +0000 (11:13 +0530)]
go/doc: convert to unicode quotes for ToText and Synopsis
We refactor the conversion of quotes to their unicode equivalent
to a separate function so that it can be called from ToText and Synopsis.
And we introduce a temp buffer to write the escaped HTML and convert
the unicode quotes back to html escaped entities. This simplifies the logic
and gets rid of the need to track the index of the escaped text.
Fixes #27759
Change-Id: I71cf47ddcd4c6794ccdf2898ac25539388b393c1
Reviewed-on: https://go-review.googlesource.com/c/150377
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Gergely Brautigam [Wed, 28 Nov 2018 18:10:28 +0000 (18:10 +0000)]
runtime: node ordering in mTreap; adjust code to reflect description.
Adjust mTreap ordering logic to reflect the description of mTreap ordering.
Before it was using unsafe.Pointer in order to gather the base address of
a span. This has been changed to use base, which is the startAddress of a
span as the description is telling us in mgclarge.go.
Fixes: golang/go#28805
Change-Id: Ib3cd94a0757e23d135b5d41830f38fc08bcf16a3
GitHub-Last-Rev: 93f749b6700b1e179de16607a18395d5e162ecc1
GitHub-Pull-Request: golang/go#28973
Reviewed-on: https://go-review.googlesource.com/c/151499 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Bryan C. Mills [Thu, 8 Nov 2018 14:46:24 +0000 (09:46 -0500)]
cmd/go/internal/load: remove redundant assignment to BinDir
This assignment became a no-op in CL 36196, where both it and the
preceding assignment were changed to cfg.GOROOTbin (from gorootBin and
gobin respectively).
Bryan C. Mills [Thu, 8 Nov 2018 15:29:40 +0000 (10:29 -0500)]
cmd/go: enable module mode without a main module when GO111MODULE=on
This is as minimal a change as I could comfortably make to enable 'go
get' outside of a module for 1.12.
In general, commands invoked in module mode while outside of a module
operate as though they are in a module with an initially-empty go.mod
file. ('go env GOMOD' reports os.DevNull.)
Commands that operate on the current directory (such as 'go list' and
'go get -u' without arguments) fail: without a module definition, we
don't know the package path. Likewise, commands whose sole purpose is
to write files within the main module (such as 'go mod edit' and 'go
mod vendor') fail, since we don't know where to write their output.
Since the go.sum file for the main module is authoritative, we do not
check go.sum files when operating outside of a module. I plan to
revisit that when the tree opens for 1.13.
We may also want to revisit the behavior of 'go list': it would be
useful to be able to query individual packages (and dependencies of
those packages) within versioned modules, but today we only allow
versioned paths in conjunction with the '-m' flag.
Bryan C. Mills [Thu, 25 Oct 2018 20:42:26 +0000 (16:42 -0400)]
cmd/go/internal/modfetch/codehost: add lockfiles for repos
The lockfile guards calls that may change the repo's filesystem contents.
We don't know how robust VCS implementations are to running
simultaneous commands, and this way we don't need to care: only one
'go' command at a time will modify any given repository.
If we can guarantee that particular VCS implementations are robust
enough across all of the VCS tool versions we support, we may be able
to remove some of this locking to improve parallelism.
Bryan C. Mills [Tue, 23 Oct 2018 19:51:42 +0000 (15:51 -0400)]
cmd/go/internal/{modcmd,modload}: lock edits to go.mod
Use an arbitrary lockfile to serialize edits, and use atomic renames
to actually write the go.mod file so that we never drop version
requirements due to a command failing partway through a write.
Multiple invocations of the 'go' command may read the go.mod file
concurrently, and will see some consistent version even if some other
invocation changes it concurrently.
Multiple commands may attempt to write the go.mod file concurrently.
One writer will succeed and write a consistent, complete go.mod file.
The others will detect the changed contents and fail explicitly: it is
not, in general, possible to resolve two conflicting changes to module
requirements, so we surface the problem to the user rather than trying
to solve the problem heuristically.
Bryan C. Mills [Mon, 15 Oct 2018 19:52:01 +0000 (15:52 -0400)]
cmd/go/internal/modfetch: lock files and directories
We employ the following new locking mechanisms:
• Zip files and list files within the module cache are written using
atomic renames of temporary files, so that GOPROXY servers reading
from the cache will never serve incomplete content.
• A lock file for each module version guards downloading and extraction of
(immutable) module contents.
• A lock file alongside each version list (named 'list.lock')
guards updates to the list.
• A single lock file in the module cache guards updates to all go.sum
files. The go.sum files themselves are written using an atomic
rename to ensure that we never accidentally discard existing sums.
Bryan C. Mills [Tue, 23 Oct 2018 19:39:07 +0000 (15:39 -0400)]
cmd/go/internal/{clean,test}: lock testexpire.txt
Also check to make sure we don't overwrite a newer timestamp with an
older one.
testexpire.txt may be written concurrently, and a partially-written
timestamp may appear much older than the actual intended one. We don't
want to re-run tests that should still be cached.
Bryan C. Mills [Tue, 16 Oct 2018 14:42:58 +0000 (10:42 -0400)]
cmd/go/internal/lockedfile: add package and support library
lockedfile.File passes through to os.File, with Open, Create, and OpenFile
functions that mimic the corresponding os functions but acquire locks
automatically, releasing them when the file is closed.
lockedfile.Sentinel is a simplified wrapper around lockedfile.OpenFile for the
common use-case of files that signal the status of idempotent tasks.
lockedfile.Mutex is a Mutex-like synchronization primitive implemented in terms
of file locks.
lockedfile.Read is like ioutil.Read, but obtains a read-lock.
lockedfile.Write is like ioutil.Write, but obtains a write-lock and can be used
for read-only files with idempotent contents.
Bryan C. Mills [Mon, 5 Nov 2018 20:01:53 +0000 (15:01 -0500)]
vendor/golang_org/x: move to internal/x
Packages in vendor/ directories have a "vendor/" path prefix in GOPATH
mode, but intentionally do not in module mode. Since the import path
is embedded in the compiled output, changing that path invalidates
cache entries and causes cmd/go to try to rebuild (and reinstall) the
vendored libraries, which will fail if the directory containing those
libraries is read-only.
If I understood correctly, this is the approach Russ suggested as an
alternative to https://golang.org/cl/136138.
Than McIntosh [Wed, 28 Nov 2018 15:50:54 +0000 (10:50 -0500)]
go/internal/gccgoimporter: additional V3 export data changes
This patch merges in support for reading the most recent
incarnation of V3 export data (initial inline function bodies),
from the importer portions of https://golang.org/cl/150061 and
https://golang.org/cl/150067.
Updates #28961.
Change-Id: I34e837acbf48b8fd1a4896a1a977d2241adfb28d
Reviewed-on: https://go-review.googlesource.com/c/151557
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
David Chase [Fri, 26 Oct 2018 16:00:07 +0000 (12:00 -0400)]
cmd/compile: begin OpArg and OpPhi location lists at block start
For the entry block, make the "first instruction" be truly
the first instruction. This allows printing of incoming
parameters with Delve.
Also be sure Phis are marked as being at the start of their
block. This is observed to move location list pointers,
and where moved, they become correct.
Leading zero-width instructions include LoweredGetClosurePtr.
Because this instruction is actually architecture-specific,
and it is now tested for in 3 different places, also created
Op.isLoweredGetClosurePtr() to reduce future surprises.