Rob Pike [Mon, 29 Jan 2018 00:28:17 +0000 (11:28 +1100)]
cmd/asm: fix crash on bad symbol for TEXT
Was missing a check in validSymbol.
Fixes #23580.
Can wait for go1.11. Probably safe but the crash is only for
invalid input, so not worth the risk.
Change-Id: I51f88c5be35a8880536147d1fe5c5dd6798c29de
Reviewed-on: https://go-review.googlesource.com/90398 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Sun, 4 Feb 2018 09:25:55 +0000 (09:25 +0000)]
go/types: use consistent receiver names
Inconsistent names are quite obvious on the godoc HTML rendering:
type Array
func NewArray(elem Type, len int64) *Array
func (a *Array) Elem() Type
func (a *Array) Len() int64
func (t *Array) String() string
func (t *Array) Underlying() Type
Fix all the String and Underlying methods to be consistent with their
types. This makes these two lists of methods less consistent, but that's
not visible to the user.
This also makes the inconsistent receiver names rule in golint happy.
Change-Id: I7c84d6bae1235887233a70d5f7f61a224106e952
Reviewed-on: https://go-review.googlesource.com/91736 Reviewed-by: Robert Griesemer <gri@golang.org>
Robert Griesemer [Thu, 18 Jan 2018 05:42:51 +0000 (21:42 -0800)]
cmd/compile/internal/syntax: implement comment reporting in scanner
R=go1.11
In order to collect comments in the AST and for error testing purposes,
the scanner needs to not only recognize and skip comments, but also be
able to report them if so desired. This change adds a mode flag to the
scanner's init function which controls the scanner behavior around
comments.
In the common case where comments are not needed, there must be no
significant overhead. Thus, comments are reported via a handler upcall
rather than being returned as a _Comment token (which the parser would
have to filter out with every scanner.next() call).
Because the handlers for error messages, directives, and comments all
look the same (they take a position and text), and because directives
look like comments, and errors never start with a '/', this change
simplifies the scanner's init call to only take one (error) handler
instead of 2 or 3 different handlers with identical signature. It is
trivial in the handler to determine if we have an error, directive,
or general comment.
Finally, because directives are comments, when reporting directives
the full comment text is returned now rather than just the directive
text. This simplifies the implementation and makes the scanner API
more regular. Furthermore, it provides important information about
the comment style used by a directive, which may matter eventually
when we fully implement /*line file:line:col*/ directives.
Change-Id: I2adbfcebecd615e4237ed3a832b6ceb9518bf09c
Reviewed-on: https://go-review.googlesource.com/88215 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This implements parsing of /*line file:line*/ and /*line file:line:col*/
directives and also extends the optional column format to regular //line
directives, per #22662.
For a line directive to be recognized, its comment text must start with
the prefix "line " which is followed by one of the following:
:line
:line:col
filename:line
filename:line:col
with at least one : present. The line and col values must be unsigned
decimal integers; everything before is considered part of the filename.
Valid line directives are:
//line :123
//line :123:8
//line foo.go:123
//line C:foo.go:123 (filename is "C:foo.go")
//line C:foo.go:123:8 (filename is "C:foo.go")
/*line ::123*/ (filename is ":")
No matter the comment format, at the moment all directives act as if
they were in //line comments, and column information is ignored.
To be addressed in subsequent CLs.
For #22662.
Change-Id: I1a2dc54bacc94bc6cdedc5229ee13278971f314e
Reviewed-on: https://go-review.googlesource.com/86037 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Tue, 16 Jan 2018 23:28:57 +0000 (15:28 -0800)]
go/parser: improved error recovery after missing type
R=go1.11
This CL also introduces a new TODO in parser.go. To be
addressed in a separate CL to make this easier to review.
Also: Make parser's test harness easier to use by ignoring
auto-inserted (invisible) semicolons when computing error
positions. Adjusted testdata/commas.src accordingly.
Fixes #23434.
Change-Id: I050592d11d5f984f71185548394c000eea509205
Reviewed-on: https://go-review.googlesource.com/87898 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Fri, 12 Jan 2018 05:52:27 +0000 (21:52 -0800)]
go/scanner: don't eat \r in comments if that shortens the comment
For consistent formatting across platforms we strip \r's from
comments. This happens in the go/scanner which already does
this for raw string literals where it is mandated by the spec.
But if a (sequence of) \r appears in a regular (/*-style) comment
between a * and a /, removing that (sequence of) \r shortens that
text segment to */ which terminates the comment prematurely.
Don't do it.
As an aside, a better approach would be to not touch comments,
(and raw string literals for that matter) in the scanner and
leave the extra processing to clients. That is the approach
taken by the cmd/compile/internal/syntax package. However, we
probably can't change the semantics here too much, so just do
the minimal change that doesn't produce invalid comments. It's
an esoteric case after all.
Fixes #11151.
Change-Id: Ib4dcb52094f13c235e840c9672e439ea65fef961
Reviewed-on: https://go-review.googlesource.com/87498 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Robert Griesemer [Sat, 23 Dec 2017 00:29:36 +0000 (16:29 -0800)]
go/parser: simplify code to read from an io.Reader (cleanup)
ioutil.ReadAll didn't exist when we wrote that parser code
originally (in 2009). Now it does, so use it. This may also
make that code path slightly more efficient.
Also, now that we are guaranteed to have a fast path for reading
from an io.Reader (and thus an io.ReadCloser), simplify setup
code for parser.ParseFile calls in srcimporter.Importer.ParseFiles.
Remove the associated TODO since we cannot reproduce any significant
performance differences when running go test -run ImportStdLib for
the case where we used to read directly from a file (even before the
change to the parser).
Fixes #19281.
Change-Id: I816459d092bb9e27fdc85089b8f21d57ec3fd79a
Reviewed-on: https://go-review.googlesource.com/85395 Reviewed-by: Alan Donovan <adonovan@google.com>
Austin Clements [Tue, 6 Feb 2018 23:00:13 +0000 (18:00 -0500)]
runtime: remove legacy comments and code from arm morestack
CL 137410043 deleted support for split stacks, which means morestack
no longer needed to save its caller's frame or argument size or its
caller's argument pointer. However, this commit failed to update the
comment or delete the line that computed the caller's argument
pointer. Clean these up now.
Austin Clements [Wed, 24 Jan 2018 22:17:38 +0000 (17:17 -0500)]
cmd/internal/obj/mips: support NOFRAME
This passes toolstash -cmp with one exception: assembly functions that
were declared with a frame size of -4 (or -8) used to record
locals=0xfffffffffffffffc in the object file and now record
locals=0x0. This doesn't affect anything.
Austin Clements [Wed, 24 Jan 2018 22:17:38 +0000 (17:17 -0500)]
cmd/internal/obj/arm64: support NOFRAME
In addition, this makes the arm64 prologue code generation much closer
to the pattern used on other platforms.
This passes toolstash -cmp with one exception: assembly functions that
were declared with a frame size of -8 used to record
locals=0xfffffffffffffff8 in the object file and now record
locals=0x0. This doesn't affect anything.
Austin Clements [Wed, 24 Jan 2018 22:17:38 +0000 (17:17 -0500)]
cmd/internal/obj/arm: support NOFRAME
This adds support on arm for the NOFRAME symbol attribute used by
ppc64 and s390x in preference to using a frame size of -4. This is
modeled on ppc64's implementation of NOFRAME.
Austin Clements [Thu, 25 Jan 2018 16:41:41 +0000 (11:41 -0500)]
cmd/compile: eliminate NoFramePointer
The NoFramePointer function flag is no longer used, so this CL
eliminates it. This cleans up some confusion between the compiler's
NoFramePointer flag and obj's NOFRAME flag. NoFramePointer was
intended to eliminate the saved base pointer on x86, but it was
translated into obj's NOFRAME flag. On x86, NOFRAME does mean to omit
the saved base pointer, but on ppc64 and s390x it has a more general
meaning of omitting *everything* from the frame, including the saved
LR and ppc64's "fixed frame". Hence, on ppc64 and s390x there are far
fewer situations where it is safe to set this flag.
Austin Clements [Thu, 25 Jan 2018 16:35:27 +0000 (11:35 -0500)]
cmd/internal/obj/x86: adjust SP correctly for tail calls
Currently, tail calls on x86 don't adjust the SP on return, so it's
important that the compiler produce a zero-sized frame and disable the
frame pointer. However, these constraints aren't necessary. For
example, on other architectures it's generally necessary to restore
the saved LR before a tail call, so obj simply makes this work.
Likewise, on x86, there's no reason we can't simply make this work.
Hence, this CL adjusts the compiler to use the same tail call
convention for x86 that we use on LR machines by producing a RET with
a target, rather than a JMP with a target. In fact, obj already
understands this convention for x86 except that it's buggy with
non-zero frame sizes. So we also fix this bug obj. As a result of
these fixes, the compiler no longer needs to mark wrappers as
NoFramePointer since it's now perfectly fine to save the frame
pointer.
In fact, this eliminates the only use of NoFramePointer in the
compiler, which will enable further cleanups.
This also fixes what is very nearly, but not quite, a code generation
bug. NoFramePointer becomes obj.NOFRAME in the object file, which on
ppc64 and s390x means to omit the saved LR. Hence, on these
architectures, NoFramePointer (and NOFRAME) is only safe to set on
leaf functions. However, on *most* architectures, wrappers aren't
necessarily leaf functions because they may call DUFFZERO. We're saved
on ppc64 and s390x only because the compiler doesn't have the rules to
produce DUFFZERO calls on these architectures. Hence, this only works
because the set of LR architectures that implement NOFRAME is disjoint
from the set where the compiler produces DUFFZERO operations. (I
discovered this whole mess when I attempted to add NOFRAME support to
arm.)
Robert Griesemer [Thu, 21 Dec 2017 22:35:21 +0000 (14:35 -0800)]
go/types: use check.lookup consistently where possible (cleanup)
This CL converts the last call to scope.LookupParent with no position
information to a check.lookup call that respects position information
provided by Eval (there's one more LookupParent call that cannot be
converted, see the respective comment in the code).
In this case, the lookup is needed to determine the variable on the
LHS of an assignment, for adjustment of its `used` information.
Outside a types.Eval call, i.e., during normal type-checking, there
is no difference between this new code and the old code.
While in a types.Eval call, it's important to use the correct position
to look up the relevant variable. If token.NoPos were used, one might
find another variable with the same name, declared later in the scope.
Caveat: Types.Eval only accepts expressions, and it's currently not
possible to evaluate assignments (except via function literals, but
then the scope is different). That is, this change is a fix for a
potential future bug, and for now a no-op.
Change-Id: I28db1fe1202c07e3f7b3fadfd185728afb9b2ae7
Reviewed-on: https://go-review.googlesource.com/85199 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Thu, 21 Dec 2017 22:15:20 +0000 (14:15 -0800)]
go/types: correctly determine if panic call refers to built-in
R=go1.11
The terminating statement check for functions that declare result
parameters was using the wrong scope to look up calls to `panic`
which in esoteric cases lead to a false positive.
Instead of looking up a panic call again at a time when correct
scope information would have to be recomputed, collect calls to
predeclared panic in a set when type-checking that call.
Fixes #23218.
Change-Id: I35eaf010e5cb8e43696efba7d77cefffb6f3deb2
Reviewed-on: https://go-review.googlesource.com/85198 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Tue, 19 Dec 2017 23:45:50 +0000 (15:45 -0800)]
go/types: perform delayed tests even for types.Eval
R=go1.11
types.Eval historically never evaluated any delayed tests, which
included verification of validity of map keys, but also function
literal bodies.
Now, embedded interfaces are also type-checked in a delayed fashion,
so it becomes imperative to do all delayed checks for eval (otherwise
obviously incorrect type expressions are silently accepted).
Enabling the delayed tests also removes the restriction that function
literals were not type-checked.
Also fixed a bug where eval wouldn't return a type-checking error
because check.handleBailout was using the wrong err variable.
Added tests that verify that method set computation is using the
right types when evaluating interfaces with embedded types.
For #18395.
For #22992.
Change-Id: I574fa84568b5158bca4b4ccd4ef5abb616fbf896
Reviewed-on: https://go-review.googlesource.com/84898 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Thu, 14 Dec 2017 05:37:13 +0000 (21:37 -0800)]
go/types: don't associate methods with alias type names
R=go1.11
The existing code associated methods with receiver base type
names before knowing if a type name denoted a locally defined
type. Sometimes, methods would be incorrectly associated with
alias type names and consequently were lost down the road.
This change first collects all methods with non-blank names
and in a follow-up pass resolves receiver base type names to
valid non-alias type names with which the methods are then
associated.
Fixes #23042.
Change-Id: I7699e577b70aadef6a2997e882beb0644da89fa3
Reviewed-on: https://go-review.googlesource.com/83996 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Mon, 11 Dec 2017 23:30:39 +0000 (15:30 -0800)]
go/types: delay type-checking of function literals
R=go1.11
Functions (at the package level) were collected and their bodies
type-checked after all other package-level objects were checked.
But function literals where type-checked right away when they were
encountered so that they could see the correct, partially populated
surrounding scope, and also to mark variables of the surrounding
function as used.
This approach, while simple, breaks down in esoteric cases where
a function literal appears inside the declaration of an object
that its body depends on: If the body is type-checked before the
object is completely set up, the literal may use incomplete data
structures, possibly leading to spurious errors.
This change postpones type-checking of function literals to later;
after the current expression or statement, but before any changes
to the enclosing scope (so that the delayed type-checking sees the
correct scope contents).
The new mechanism is general and now is also used for other
(non-function) delayed checks.
Fixes #22992.
Change-Id: Ic95f709560858b4bdf8c645be70abe4449f6184d
Reviewed-on: https://go-review.googlesource.com/83397 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Tue, 21 Nov 2017 23:53:39 +0000 (15:53 -0800)]
go/types: correctly compute method set of some recursive interfaces
R=go1.11.
The existing algorithm for type-checking interfaces breaks down in
complex cases of recursive types, e.g.:
package issue21804
type (
_ interface{ m(B) }
A interface{ a(D) }
B interface{ A }
C interface{ B }
D interface{ C }
)
var _ A = C(nil)
The underlying problem is that the method set of C is computed by
following a chain of embedded interfaces at a point when the method
set for one of those interfaces is not yet complete. A more general
problem is largely avoided by topological sorting of interfaces
depending on their dependencies on embedded interfaces (but not
method parameters).
This change fixes this problem by fundamentally changing how
interface method sets are computed: Instead of determining them
based on recursively type-checking embedded interfaces, the new
approach computes the method sets of interfaces separately,
based on syntactic structure and name resolution; and using type-
checked types only when readily available (e.g., for local types
which can at best refer to themselves, and imported interfaces).
This approach avoids cyclic dependencies arising in the method
sets by separating the collection of embedded methods (which
fundamentally cannot have cycles in correct programs) and type-
checking of the method's signatures (which may have arbitrary
cycles).
As a consequence, type-checking interfaces can rely on the
pre-computed method sets which makes the code simpler: Type-
checking of embedded interface types doesn't have to happen
anymore during interface construction since we already have
all methods and now is delayed to the end of type-checking.
Also, the topological sort of global interfaces is not needed
anymore.
Fixes #18395.
Change-Id: I0f849ac9568e87a32c9c27bbf8fab0e2bac9ebb1
Reviewed-on: https://go-review.googlesource.com/79575 Reviewed-by: Alan Donovan <adonovan@google.com>
Andrew Bonventre [Mon, 12 Feb 2018 20:05:15 +0000 (15:05 -0500)]
[release-branch.go1.10] all: merge master into release-branch.go1.10
* 74b56022a1 doc: note that x509 cert parsing rejects some more certs now
* c52e27e68d CONTRIBUTING: remove Pull Request bit
* 829b64c1ea cmd/fix: fix cast check
* ee59f6dff2 doc: minor wording improvement to the diagnostics guide
* c6e7330ebd all: remove PULL_REQUEST_TEMPLATE from .github
* d814c2be9b doc: remove Pull Request note in README.md
* 104445e314 doc: document Go 1.9.4 and Go 1.8.7
Brad Fitzpatrick [Sun, 11 Feb 2018 16:20:38 +0000 (16:20 +0000)]
CONTRIBUTING: remove Pull Request bit
Also remove the "Also, please do not post patches on the issue
tracker" part, since that didn't seem to reduce the number of patches
inlined into bug reports. And now that we accept PRs, people will
probably try that first. We'll see.
Fixes #23779
Updates #18517
Change-Id: I449e0afd7292718e57d9d428494799c78296a0d2
Reviewed-on: https://go-review.googlesource.com/93335 Reviewed-by: Andrew Bonventre <andybons@golang.org>
Russ Cox [Thu, 1 Feb 2018 19:07:21 +0000 (14:07 -0500)]
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 #23672, CVE-2018-6574.
Change-Id: I59e7c1355155c335a5c5ae0d2cf8fa7aa313940a
Reviewed-on: https://team-review.git.corp.google.com/209949 Reviewed-by: Ian Lance Taylor <iant@google.com>
Russ Cox [Wed, 7 Feb 2018 14:33:20 +0000 (09:33 -0500)]
all: merge master into release-branch.go1.10, for go1.10rc2
* b2d3d6e6 cmd/link/internal/loadelf: fix logic for computing ELF flags on ARM
* c07095cd cmd/cgo: revert CL 49490 "fix for function taking pointer typedef"
* 23e8e197 cmd/compile: use unsigned loads for multi-element comparisons
* 85bdd05c cmd/go: rebuild as needed for tests of packages that add methods
* fd7331a8 text/template: revert CL 66410 "add break, continue actions in ranges"
* f54f780d cmd/vet: unexported interface{} fields on %s are ok
* a0222ec5 cmd/internal/obj/arm64: fix assemble add/adds/sub/subs/cmp/cmn(extended register) bug
* 59523176 cmd/go: only run -race test if -race works
* 4558321e doc/editors: remove feature matrix for various editors/IDEs
* e6756ec1 cmd/go: ignore coverpkg match on sync/atomic in atomic coverage mode
* 10d096fe cmd/go: fix import config debugging flag
* f598ad58 go/internal/gccgoimporter: remove old and exp gccgo packages in test
* 2a8229d9 misc/cgo/test: get uintptr, not pointer, from dlopen
* 851e98f0 spec: remove need for separate Function production (cleanup)
* cbe1a61e net: fix the kernel state name for TCP listen queue on FreeBSD
* 6f37fee3 cmd/go: fix TestNoCache on Plan 9
* e5186895 runtime: restore RSB for sigpanic call on mips64x
* 3ff41cdf runtime: suppress "unexpected return pc" any time we're in cgo
* d929e40e syscall: use SYS_GETDENTS64 on linux/mips64{,le}
* 43288467 test: add test for gccgo bug 23545
* 19150303 cmd/go: if unable to initialize cache, just disable it
* ebe38b86 runtime: fail silently if we unwind over sigpanic into C code
* 5c2be42a runtime: don't unwind past asmcgocall
* 03e10bd9 os/signal: skip TestTerminalSignal if posix_openpt fails with EACCES
* d30591c1 cmd/vendor/github.com/google/pprof: cherry-pick fix to cope with $HOME not being writable
* bcc86d5f doc: add GOMIPS to source installation docs
* 926f2787 cmd/fix: cleanup directories created during typecheck
* 32a08d09 bootstrap.bash: only fetch git revision if we need it
* 14f8027a cmd/vet: extra args if any formats are indexed are ok
* 4072608b cmd/vet: %s is valid for an array of stringer
* 1f85917f cmd/vet: **T is not Stringer if *T has a String method
* 8c1f21d9 cmd/vet: disable complaint about 0 flag in print
* d529aa93 doc: fix the closing tag in contribute.html
* f8610bbd doc: fix two small mistakes in 1.10 release notes
* 5af1e7d7 cmd/go: skip external tests on plan9/arm
* 00587e89 doc: fix spelling mistake
* 3ee8c3cc os: document inheritance of thread state over exec
* b5b35be2 cmd/compile: don't inline functions that call recover
* 651ddbdb database/sql: buffers provided to Rows.Next should not be modified by drivers
* 7350297e doc: remove Sarah Adams from conduct working group contacts
Michael Hudson-Doyle [Wed, 7 Feb 2018 02:46:26 +0000 (15:46 +1300)]
cmd/link/internal/loadelf: fix logic for computing ELF flags on ARM
The linker contains complicated logic for figuring out which float ABI to
indicate it is using on (32 bit) ARM systems: it parses a special section in
host object files to look for a flag indicating use of the hard float ABI. When
loadelf got split into its own package a bug was introduced: if the last host
object file does not contain a float ABI related tag, the ELF header's flag was
set to 0, rather than using the value from the last object file which contained
an ABI tag. Fix the code to only change the value used for the ELF header if a
tag was found.
This fixes an extremely confusing build failure on Ubuntu's armhf builders.
Change-Id: I0845d68d082d1383e4cae84ea85164cdc6bcdddb
Reviewed-on: https://go-review.googlesource.com/92515
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 6 Feb 2018 17:44:34 +0000 (09:44 -0800)]
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.
Russ Cox [Tue, 6 Feb 2018 04:57:41 +0000 (23:57 -0500)]
cmd/go: rebuild as needed for tests of packages that add methods
If A's external test package imports B, which imports A,
and A's (internal) test code also adds something to A that
invalidates anything in the export data from a build of A
without its test code, then strictly speaking we need to
rebuild B against the test-augmented version of A before
using it to build A's external test package.
We've been skating by without doing this for a very long time,
but I knew we'd need to handle it better eventually,
I planned for it in the new build cache simplifications,
and the code was ready. Now that we have a real-world
test case that needs it, turn on the "proper rebuilding" code.
It doesn't really matter how much things slow down, since
a real-world test cases that caused an internal compiler error
before is now handled correctly, but it appears to be small:
I wasn't able to measure an effect on "go test -a -c fmt".
And of course most builds won't use -a and will be cached well.
Fixes #6204.
Fixes #23701.
Change-Id: I2cd60cf400d1928428979ab05831f48ff7cee6ca
Reviewed-on: https://go-review.googlesource.com/92215
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Mon, 5 Feb 2018 23:50:29 +0000 (15:50 -0800)]
text/template: revert CL 66410 "add break, continue actions in ranges"
The new break and continue actions do not work in html/template, and
fixing them requires thinking about security issues that seem too
tricky at this stage of the release. We will try again for 1.11.
Original CL description:
text/template: add break, continue actions in ranges
Adds the two range control actions "break" and "continue". They act the
same as the Go keywords break and continue, but are simplified in that
only the innermost range statement can be broken out of or continued.
Fixes #20531
Updates #20531
Updates #23683
Change-Id: Ia7fd3c409163e3bcb5dc42947ae90b15bdf89853
Reviewed-on: https://go-review.googlesource.com/92155
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Daniel Martí [Mon, 29 Jan 2018 11:14:31 +0000 (11:14 +0000)]
cmd/vet: unexported interface{} fields on %s are ok
For example, the following program is valid:
type T struct {
f interface{}
}
func main() {
fmt.Printf("%s", T{"foo"}) // prints {foo}
}
Since the field is of type interface{}, we might have any value in it.
For example, if we had T{3}, fmt would complain. However, not knowing
what the type under the interface is, we must be conservative.
However, as shown in #17798, we should issue an error if the field's
type is statically known to implement the error or fmt.Stringer
interfaces. In those cases, the user likely wanted the %s format to call
those methods. Keep the vet error in those cases.
While at it, add more field type test cases, such as custom error types,
and interfaces that extend the error interface.
Fixes #23563.
Change-Id: I063885955555917c59da000391b603f0d6dce432
Reviewed-on: https://go-review.googlesource.com/90516
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
The current code encodes the wrong option value in the binary.
The fix reconstructs the function opxrrr() that does not encode the option
value into the binary value when arguments is sign or zero-extended register.
Andrew Bonventre [Mon, 5 Feb 2018 17:10:22 +0000 (12:10 -0500)]
doc/editors: remove feature matrix for various editors/IDEs
The speed of feature development for these products outpaces the
standard Go 6-month release cycle tied to this page. The cost of
maintaining this list is becoming a burden as we make every
attempt at being impartial. As of this writing, we believe feature
lists belong on the pages of the editors/IDEs themselves.
Robert Griesemer [Thu, 1 Feb 2018 22:08:45 +0000 (14:08 -0800)]
spec: remove need for separate Function production (cleanup)
The EBNF production
Function = Signature FunctionBody .
was used in FunctionDecl, MethodDecl, and FunctionLit, but only
for the latter it shortened the syntax slightly.
This change "inlines" Function which simplifies FunctionDecl and
MethodDecl and gets rid of the Function production.
This has no impact on the specified language. Also, the Function
production is never referred to by the prose, so it's safe to
remove it from the spec.
Finally, neither go/ast nor go/parser have a representation of
this production via a corresponding node or parse function, so
no possibly valuable documentation is lost, either.
Change-Id: Ia2875d31c6ec2d2079081ef481e50bad4f43c694
Reviewed-on: https://go-review.googlesource.com/91515 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org>
David du Colombier [Wed, 31 Jan 2018 21:56:39 +0000 (22:56 +0100)]
cmd/go: fix TestNoCache on Plan 9
CL 91097 added TestNoCache. However, this
test is failing on Plan 9 because the HOME
environment variable doesn't contain the
home directory where the Go cache is located.
This change fixes the TestNoCache test
by using the home environment variable
instead of HOME on Plan 9.
Fixes #23644.
Change-Id: Icfb7a7a4c2852f159c93032b4081411628a2787f
Reviewed-on: https://go-review.googlesource.com/91216
Run-TryBot: David du Colombier <0intro@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Wed, 31 Jan 2018 19:34:36 +0000 (14:34 -0500)]
runtime: restore RSB for sigpanic call on mips64x
preparePanic must set all registers expected by Go runtime conventions
in case the sigpanic is being injected into C code. However, on
mips64x it fails to restore RSB (R28). As a result, if C code modifies
RSB and then raises a signal that turns into a sigpanic call, sigpanic
may crash when it attempts to lock runtime.debuglock (the first global
it references).
Fix this by restoring RSB in the signal context using the same
convention as main and sigtramp.
Austin Clements [Wed, 31 Jan 2018 16:55:22 +0000 (11:55 -0500)]
runtime: suppress "unexpected return pc" any time we're in cgo
Currently, gentraceback suppresses the "unexpected return pc" error
for sigpanic's caller if the M was running C code.
However, there are various situations where a sigpanic is injected
into C code that can cause traceback to unwind *past* the sigpanic
before realizing that it's in trouble (the traceback beyond the
sigpanic will be wrong).
Rather than try to fix these issues for Go 1.10, this CL simply
disables complaining about unexpected return PCs if we're in cgo
regardless of whether or not they're from the sigpanic frame. Go 1.9
never complained about unexpected return PCs when printing, so this is
simply a step closer to the old behavior.
This should fix the openbsd-386 failures on the dashboard, though this
issue could affect any architecture.
Fixes #23640.
Change-Id: I8c32c1ee86a70d2f280661ed1f8caf82549e324b
Reviewed-on: https://go-review.googlesource.com/91136
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tobias Klauser [Wed, 31 Jan 2018 12:59:00 +0000 (13:59 +0100)]
syscall: use SYS_GETDENTS64 on linux/mips64{,le}
The getdents64 syscall is only available for mips64/mips64le starting
with Linux kernel 3.10. Since mips64le requires at least 4.8 according
to [1] (regarding #16848) using it should be fine.
[1] https://golang.org/wiki/MinimumRequirements
This CL changes the binary layout of type Dirent for mips64/mips64le,
but not the public API. But since the currently used layout doesn't
match the struct linux_dirent returned by the getdents syscall this
should be fine as well.
Austin Clements [Tue, 30 Jan 2018 21:03:51 +0000 (16:03 -0500)]
runtime: fail silently if we unwind over sigpanic into C code
If we're running C code and the code panics, the runtime will inject a
call to sigpanic into the C code just like it would into Go code.
However, the return PC from this sigpanic will be in C code. We used
to silently abort the traceback if we didn't recognize a return PC, so
this went by quietly. Now we're much louder because in general this is
a bad thing. However, in this one particular case, it's fine, so if
we're in cgo and are looking at the return PC of sigpanic, silence the
debug output.
Fixes #23576.
Change-Id: I03d0c14d4e4d25b29b1f5804f5e9ccc4f742f876
Reviewed-on: https://go-review.googlesource.com/90896
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Tue, 30 Jan 2018 21:01:33 +0000 (16:01 -0500)]
runtime: don't unwind past asmcgocall
asmcgocall switches to the system stack and aligns the SP, so
gentraceback both can't unwind over it when it appears on the system
stack (it'll read some uninitialized stack slot as the return PC).
There's also no point in unwinding over it, so don't.
Updates #23576.
Change-Id: Idfcc9599c7636b80dec5451cb65ae892b4611981
Reviewed-on: https://go-review.googlesource.com/90895
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Mohit Agarwal [Tue, 30 Jan 2018 12:58:50 +0000 (18:28 +0530)]
cmd/fix: cleanup directories created during typecheck
Executing
$ go tool dist test -run=^go_test:cmd/fix$
leaves a number of directories (fix_cgo_typecheck*) in TMPDIR.
Change-Id: Ia5bdc2f7d884333771d50365063faf514ebf6eae
Reviewed-on: https://go-review.googlesource.com/90795
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Mon, 29 Jan 2018 09:24:51 +0000 (09:24 +0000)]
cmd/vet: %s is valid for an array of stringer
vet was quiet for []stringer, but not for [N]stringer. The source of the
problem was how the recursive call used .Elem().Underlying() for arrays,
but .Elem() for slices. In the first case, the named type is dropped,
thus losing all information of attached methods.
Be consistent across slices and arrays, by dropping the Underlying call
that is causing trouble. Add regression tests too, including cases where
the element type does not implement fmt.Stringer.
Fixes #23552.
Change-Id: I0fde07d101f112d5768be0a79207ef0b3dc45f2e
Reviewed-on: https://go-review.googlesource.com/90455
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Daniel Martí [Mon, 29 Jan 2018 09:50:50 +0000 (09:50 +0000)]
cmd/vet: **T is not Stringer if *T has a String method
vet recorded what types had String methods defined on them, but it did
not record whether the receivers were pointer types. That information is
important, as the following program is valid:
Rob Pike [Tue, 30 Jan 2018 02:49:48 +0000 (13:49 +1100)]
cmd/vet: disable complaint about 0 flag in print
The problem is that vet complains about 0 as a Printf flag in some
situations where fmt allows it but probably shouldn't. The two
need to be brought in line, but it's too late in the release cycle.
The situation is messy and should be resolved properly in 1.11. This
CL is a simple fix to disable a spurious complaint for 1.10 that will be
resolved in a more thorough way in 1.11.
The workaround is just to be silent about flag 0, as suggested in
issue 23605.
Fixes #23605
Update #23498
Change-Id: Ice1a4f4d86845d70c1340a0a6430d74e5de9afd4
Reviewed-on: https://go-review.googlesource.com/90695
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alberto Donizetti [Mon, 29 Jan 2018 10:59:53 +0000 (11:59 +0100)]
doc: fix two small mistakes in 1.10 release notes
Change-Id: If928b14f6cd89b1d78745e8bc7a081ccbd7fd38f
Reviewed-on: https://go-review.googlesource.com/90515 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Kevin Burke [Thu, 25 Jan 2018 19:26:54 +0000 (11:26 -0800)]
doc: fix spelling mistake
Change-Id: I7bf1a93d3377acccdd3a34c5dcef7863310496e0
Reviewed-on: https://go-review.googlesource.com/89955 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Austin Clements [Thu, 25 Jan 2018 22:22:41 +0000 (17:22 -0500)]
cmd/compile: don't inline functions that call recover
recover determines whether it's being called by a deferred frame by
matching its caller's argument frame pointer with the one recorded in
the panic object. That means its caller needs a valid and unique
argument frame pointer, so it must not be inlined.
Richard Miller [Wed, 24 Jan 2018 16:17:05 +0000 (16:17 +0000)]
os: use the syscall version of Getwd for Plan 9
In Plan 9, each OS thread has its own independent working directory,
so the Go runtime for Plan 9 needs to coordinate Chdir and Getwd
operations to keep the working directory consistent for all goroutines.
The function os.Getwd in Plan 9 should always call syscall.Getwd
to retrieve the common working directory. Failure to do this was
the cause of (at least some of) the intermittent failures in the
Plan 9 builders with a seemingly spurious "file does not exist"
message, when a thread's working directory had been removed in
another thread.
Change-Id: Ifb834ad025ee39578234ad3b04d08bc98e939291
Reviewed-on: https://go-review.googlesource.com/89575 Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Russ Cox [Tue, 23 Jan 2018 01:07:58 +0000 (20:07 -0500)]
cmd/go: fix -coverpkg=all with dot imports
If you use -coverpkg=all you get coverage for all packages in the build.
Go 1.9 used a global counter for all the GoCover variables, so that they
were distinct for the entire build. The global counter caused problems
with caching, so we switched to a per-package counter. But now the
GoCover_0 in one package may be dot-imported into another and
conflict with the GoCover_0 in that other package.
Reestablish (overwhelmingly likely) global uniqueness of GoCover
variables by appending an _xxxxxxxxxxxx suffix, where the x's are
the prefix of the SHA256 hash of the import path. The point is only
to avoid accidents, not to defeat people determined to break the tools.
Fixes #23432.
Change-Id: I3088eceebbe35174f2eefe8d558b7c8b59d3eeac
Reviewed-on: https://go-review.googlesource.com/89135 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Kevin Burke [Wed, 24 Jan 2018 06:19:34 +0000 (22:19 -0800)]
go/types: fix spelling mistake in comment
Change-Id: If8609dd7c4bdc261056804759ec254f8af0156df
Reviewed-on: https://go-review.googlesource.com/89417 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Hana Kim [Tue, 23 Jan 2018 20:02:45 +0000 (15:02 -0500)]
runtime/pprof: scale mutex profile with sampling rate
pprof expects the samples are scaled and reflects unsampled numbers.
The legacy profile parser uses the sampling period in the output
and multiplies all values with the period.
Apply the same scaling when we output the mutex profile
in the pprof proto format.
Block profile shares the same code, but how to infer unsampled
values is unclear. Legacy profile parser doesn't do anything special
so we do nothing for block profile here.
Tested by checking the profiles reported with debug=0 (proto format)
are similar to the profiles computed from legacy format profile
when the profile rate is a non-trivial number (e.g. 2) manually.
Change-Id: Iaa33f92051deed67d8be43ddffc7c1016db566ca
Reviewed-on: https://go-review.googlesource.com/89295 Reviewed-by: Peter Weinberger <pjw@google.com>
Keith Randall [Tue, 23 Jan 2018 21:06:21 +0000 (13:06 -0800)]
cmd/compile: don't let spills clobber arguments
The compiler allows code to have multiple differently-typed views of a
single argument. For instance, if we have
func f(x float64) {
y := *(*int64)(unsafe.Pointer(&x))
...
}
Then in SSA we get two OpArg ops, one with float64 type and one with
int64 type.
The compiler will try to reuse argument slots for spill slots. It
checks that the argument slot is dead by consulting an interference
graph.
When building the interference graph, we normally ignore cross-type
edges because the values on either end of that edge can't be allocated
to the same slot. (This is just a space-saving optimization.) This
rule breaks down when one of the values is an argument, because of the
multiple views described above. If we're spilling a float64, it is not
enough that the float64 version of x is dead; the int64 version of x
has to be dead also.
Remove the optimization of not recording interference edges if types
don't match. That optimization is incorrect if one of the values
connected by the edge is an argument.
Fixes #23522
Change-Id: I361f85d80fe3bc7249014ca2c3ec887c3dc30271
Reviewed-on: https://go-review.googlesource.com/89335
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>