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>
R=go1.11.
Now that we have a syntax error test harness, we can add the
proper tests for the recent parser fixes.
For #20800.
For #20789.
For #23385.
For #23434.
A test for #20789 already exists in test/fixedbugs, but this
is the better location for that test. But leaving the existing
one where it is as well.
Change-Id: I5937b9b63bafd1efab467a00344302e717976171
Reviewed-on: https://go-review.googlesource.com/88336
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
R=go1.11
A common error is to write '=' instead of '==' inside the condition
of a simple 'if' statement:
if x = 0 { ... }
Highlight the fact that we have an assignment in the error message
to prevent further confusion.
Fixes#23385.
Change-Id: I1552050fd6da927bd12a1be0977bd2e98eca5885
Reviewed-on: https://go-review.googlesource.com/87316
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
R=go1.11
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>
R=go1.11
This is a follow up for #11377 which reported that an error like
/tmp/xx.go:9:6: expected '(', found 'IDENT' F1
shouldn't print 'IDENT', as it's just an internal detail.
The relevant change wasn't made in the original fix, so here it is.
For #11377.
Change-Id: Ib76957d86b88e3e63646fbe4abf03a3b9d045139
Reviewed-on: https://go-review.googlesource.com/87900
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
R=go1.11
No semantic change.
For #23434.
Change-Id: Iafdb062b0ebe6cd6e51f9a98b62b1d10f1bacc5c
Reviewed-on: https://go-review.googlesource.com/87899
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
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>
R=go1.11
To fix this, this CL borrows code from the new syntax
package which has a better tuned parser at this point.
Fixes#11377.
Change-Id: Ib9212c945903d6f62abcc59ef5a5767d4ef36981
Reviewed-on: https://go-review.googlesource.com/87495
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
R=go1.11
Once approved, this change must be ported to golang.org/x/tools/go/gcimporter15.
Fixes#13829.
Change-Id: I26a0094d2bfd38b97f2b64bae84b9f428fc9cdf1
Reviewed-on: https://go-review.googlesource.com/85318
Reviewed-by: Alan Donovan <adonovan@google.com>
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.
Change-Id: I65725d3d42c86e8adb6645d5aa80c305d473363d
Reviewed-on: https://go-review.googlesource.com/92437
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -4/-8 with the NOFRAME flag in mips and
mips64 assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-[84]/\1|NOFRAME,\2$0/' $(find -name '*_mips*.s')
Plus a manual fix to mkduff.go.
The go binary is identical on both architectures before and after this
change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efb
Reviewed-on: https://go-review.googlesource.com/92044
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -8 with the NOFRAME flag in arm64 assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-8/\1|NOFRAME,\2$0/' $(find -name '*_arm64.s')
Plus a manual fix to mkduff.go.
The go binary is identical before and after this change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efa
Reviewed-on: https://go-review.googlesource.com/92043
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -4 with the NOFRAME flag in arm assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-4/\1|NOFRAME,\2$0/' $(find -name '*_arm.s')
Plus three manual comment changes found by:
grep '\$-4' $(find -name '*_arm.s')
The go binary is identical before and after this change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227ef9
Reviewed-on: https://go-review.googlesource.com/92042
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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.
Change-Id: I0d15e81770e54222ae329ce4496da06016736771
Reviewed-on: https://go-review.googlesource.com/92041
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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.
Change-Id: I0d15e81770e54222ae329ce4496da06016736770
Reviewed-on: https://go-review.googlesource.com/92040
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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.
This passes toolstash -cmp.
Change-Id: I0d15e81770e54222ae329ce4496da0601673677f
Reviewed-on: https://go-review.googlesource.com/92039
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
"-8" is not a sensible frame size on arm and we're about to start
rejecting it. Replace it with -4.
Likewise, "-4" is not a sensible frame size on arm64 and we're about
to start rejecting it. Replace it with -8.
Finally, clean up some places we're weirdly inconsistent about using 0
versus -8.
Change-Id: If85e229993d5f7f1f0cfa9852b4e294d053bd784
Reviewed-on: https://go-review.googlesource.com/92038
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For leaf functions with zero-sized frames, there's no point in doing a
stack check, so omit it.
This aligns arm64 with other architectures.
Change-Id: I1fb483d62f1736af10c5110815d3f5a875a46d7f
Reviewed-on: https://go-review.googlesource.com/92037
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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.
Change-Id: If68991310b4d00638128c296bdd57f4ed731b46d
Reviewed-on: https://go-review.googlesource.com/92036
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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.)
Change-Id: Icc589aeb86beacb850d0a6a80bd3024974a33947
Reviewed-on: https://go-review.googlesource.com/92035
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
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>
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>
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>
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>
R=go1.11
Also: Moved Checker.pos field into context where it belongs.
This is a cleanup/code factoring.
For #22992.
Change-Id: If9d4f0af537cb181f73735e709ebc8258b2a1378
Reviewed-on: https://go-review.googlesource.com/83017
Reviewed-by: Alan Donovan <adonovan@google.com>
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>
Need 2-result cast so we can check the result correctly.
Fixes#23762
Change-Id: Icac3a5415156fe918988f369d6022a9a29c14089
Reviewed-on: https://go-review.googlesource.com/93078
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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>
CL 49490 fixed a warning when compiling the C code generated by cgo,
but it introduced typedef conflicts in Go code that cgo is supposed to
avoid.
Original CL description:
cmd/cgo: fix for function taking pointer typedef
Fixes#19832
Updates #19832Fixes#23720
Change-Id: I22a732db31be0b4f7248c105277ab8ee44ef6cfb
Reviewed-on: https://go-review.googlesource.com/92455
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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>
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>
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>
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.
Add the relevant test cases and negative tests.
Fixes#23501
Change-Id: Ie5850ead2ad08d9a235a5664869aac5051762f1f
Reviewed-on: https://go-review.googlesource.com/88876
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Otherwise we get into a dependency loop as we try to apply coverage
analysis to sync/atomic when the coverage analysis itself requires
sync/atomic.
Fixes#23694
Change-Id: I3a74ef3881ec5c6197ed348acc7f9e175417f6c7
Reviewed-on: https://go-review.googlesource.com/91875
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Was improperly bypassed in a couple places.
Change-Id: I13426b3efe68b9e67324c283540d0ef7b81b3d41
Reviewed-on: https://go-review.googlesource.com/91636
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The old and exp packages listed in gccgoinstallation_test.go have been
removed from gccgo. Remove them from the test.
Fixes#20932
Change-Id: I04a5148e18dccef332904b836c42098b55f2516c
Reviewed-on: https://go-review.googlesource.com/91656
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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.
Fixes#23641.
Change-Id: Ib47e83df89e2a3eece10f480e4e91ce9e4424388
Reviewed-on: https://go-review.googlesource.com/91156
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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.
Fixes#23624
Change-Id: Iaa7306fa6e4442ad2fed41c60b37627a7314f117
Reviewed-on: https://go-review.googlesource.com/91055
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
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>
This happens in a chroot and so causes failures when packaging Go 1.10 for
Debian/Ubuntu.
Change-Id: I817038c237e584ce185b2168f8c7a10b9ef27b43
Reviewed-on: https://go-review.googlesource.com/90875
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
For example, the following program is valid:
func main() {
fmt.Printf("%[1]d", 1, 2, 3)
}
If any of the formats are indexed, fmt will not complain about unused
extra arguments. See #22867 for more detail.
Make vet follow the same logic, to avoid erroring on programs that would
run without fmt complaining.
Fixes#23564.
Change-Id: Ic9dede5d4c37d1cd4fa24714216944897b5bb7cc
Reviewed-on: https://go-review.googlesource.com/90495
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>
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>
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:
type T string
func (t *T) String() string {
return fmt.Sprint(&t) // prints address
}
Teach vet that, if *T is Stringer, **T is not.
Fixes#23550.
Change-Id: I1062e60e6d82e789af9cca396546db6bfc3541e8
Reviewed-on: https://go-review.googlesource.com/90417
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>
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>
CL 13166, CL 13342 and CL 33425 skipped external tests
on freebsd/arm, linux/arm and linux/mips.
This CL does the same for plan9/arm to reduce test time
on plan9/arm and prevent the Go builder to time out.
Change-Id: I16fcc5d8010a354f480673b8c4a8a11dbc833557
Reviewed-on: https://go-review.googlesource.com/90416
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
With this fix, test/recover.go passes with -l=4.
Fixes#23557.
Change-Id: I1f32a624c49e387cfc67893a0829bb248d69c3d4
Reviewed-on: https://go-review.googlesource.com/90035
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously we allowed drivers to modify the row buffer used to scan
values when closing Rows. This is no longer acceptable and can lead
to data races.
Fixes#23519
Change-Id: I91820a6266ffe52f95f40bb47307d375727715af
Reviewed-on: https://go-review.googlesource.com/89936
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Provide a fresh conversion table for TestConversions as it gets
modified on each test.
Change-Id: I6e2240d0c3455451271a6879e994b82222c3d44c
Reviewed-on: https://go-review.googlesource.com/89595
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
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>
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>
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.
0138a3cd6d/profile/legacy_profile.go (L815)
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>
Remove leading space at the beginning of error strings,
so the strings are consistent between isExist, isNotExist
and isPermission functions.
Here is a list of error strings returned on the most common
file servers on Plan 9:
match cwfs fossil ramfs
"exists" "create/wstat -- file exists" "file already exists" "file exists"
"is a directory" "is a directory" "file is a directory"
"does not exist" "file does not exist" "file does not exist"
"not found" "directory entry not found"
"has been removed" "file has been removed"
"permission denied" "access permission denied" "permission denied" "permission denied"
"no parent" is an error returned by lib9p when removing a file without parent.
Change-Id: I2362ed4b6730b8bec7a707a1052bd1ad8921cd97
Reviewed-on: https://go-review.googlesource.com/89315
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
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>
- Don't complain about invalid constant type if the type is
invalid already (we do this in other places as well). This
is useful to do in general, and even more so if we have
invalid types due to import "C".
- Type-check the lhs of an assignment even if we bail out early
due to an error on the rhs. This was simply an oversight. We
already have machinery in place to "use" expressions; in this
case we just have to also make sure we don't overcount "uses"
of variables on the lhs.
- Fix overcount uses correction in assignments: Only do it if
the variable in question is declared inside the same package
to avoid possible race conditions when type-checking exported
variables concurrently.
Fixes#22090.
Change-Id: I4c1b59f9ce38970e7129fedc5f6023908386e4f1
Reviewed-on: https://go-review.googlesource.com/88375
Reviewed-by: Alan Donovan <adonovan@google.com>
Currently, startpanic_m (which prepares for an unrecoverable panic)
goes out of its way to make it possible to allocate during panic
handling by allocating an mcache if there isn't one.
However, this is both potentially dangerous and unnecessary.
Allocating an mcache is a generally complex thing to do in an already
precarious situation. Specifically, it requires obtaining the heap
lock, and there's evidence that this may be able to deadlock (#23360).
However, it's also unnecessary because we never allocate from the
unrecoverable panic path.
This didn't use to be the case. The call to allocmcache was introduced
long ago, in CL 7388043, where it was in preparation for separating Ms
and Ps and potentially running an M without an mcache. At the time,
after calling startpanic, the runtime could call String and Error
methods on panicked values, which could do anything including
allocating. That was generally unsafe even at the time, and CL 19792
fixed this be pre-printing panic messages before calling startpanic.
As a result, we now no longer allocate after calling startpanic.
This CL not only removes the allocmcache call, but goes a step further
to explicitly disallow any allocation during unrecoverable panic
handling, even in situations where it might be safe. This way, if
panic handling ever does an allocation that would be unsafe in unusual
circumstances, we'll know even if it happens during normal
circumstances.
This would help with debugging #23360, since the deadlock in
allocmcache is currently masking the real failure.
Beyond all.bash, I manually tested this change by adding panics at
various points in early runtime init, signal handling, and the
scheduler to check unusual panic situations.
Change-Id: I85df21e2b4b20c6faf1f13fae266c9339eebc061
Reviewed-on: https://go-review.googlesource.com/88835
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, if a _SigPanic signal arrives in a throwsplit context,
nothing is stopping the runtime from injecting a call to sigpanic that
may attempt to grow the stack. This will fail and, in turn, mask the
real problem.
Fix this by checking for throwsplit in the signal handler itself
before injecting the sigpanic call.
Updates #21431, where this problem is likely masking the real problem.
Change-Id: I64b61ff08e8c4d6f6c0fb01315d7d5e66bf1d3e2
Reviewed-on: https://go-review.googlesource.com/87595
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The current code misassembles VLD1/VST1 instruction with non-zero
offset. The offset is dropped silently without any error message.
The cause of the misassembling is the current code treats argument
(Rn)(Rm) as ZOREG type.
The fix changes the matching rules and considers (Rn)(Rm) as ROFF
type. The fix will report error information when assembles VLD1/VST1
(R8)(R13), [V1.16B].
The fix enables the ARM64Errors test.
Fixes#23448
Change-Id: I3dd518b91e9960131ffb8efcb685cb8df84b70eb
Reviewed-on: https://go-review.googlesource.com/87956
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Except for removing the DRAFT marker, I think these are now ready to go.
Change-Id: I20604f5b135616189a24990db463c7bb5e7d48f1
Reviewed-on: https://go-review.googlesource.com/88975
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This error is returned by os.Mkdir when the directory already exists.
This change fixes some upspin tests.
Change-Id: I9ad5aefebb32dff577726d537b4f3826d79868eb
Reviewed-on: https://go-review.googlesource.com/88656
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When there is no go directory inside the swiglib directory then swig
was installed without Go support. Tests in misc/swig will fail when
swig is installed without Go support.
Add additional checks for the presence of a go directory in the directory
reported by 'swig -go -swiglib' to determine if misc/swig tests should
be run.
This avoids all.bash failing when swig but not swig-go is installed
using macports.
Tested on darwin with swig and with and without swig-go installed
using macports.
Fixes#23469
Change-Id: I173201221554982ea0d9f2bea70a3cb85b297cec
Reviewed-on: https://go-review.googlesource.com/88776
Reviewed-by: David Chase <drchase@google.com>
In earlier versions of Go the "go vet" command would run on regular
source files and test files. That was lost in CL74750. Bring it back.
This required moving a chunk of code from internal/test to
internal/load. The diff looks big but the code is unchanged.
Fixes#23395
Change-Id: Ie9ec183337e8db81c5fc421d118a22b351b5409e
Reviewed-on: https://go-review.googlesource.com/87636
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, if anything goes wrong when printing a traceback, we simply
cut off the traceback without any further diagnostics. Unfortunately,
right now, we have a few issues that are difficult to debug because
the traceback simply cuts off (#21431, #23484).
This is an attempt to improve the debuggability of traceback failure
by printing a diagnostic message plus a hex dump around the failed
traceback frame when something goes wrong.
The failures look like:
goroutine 5 [running]:
runtime: unexpected return pc for main.badLR2 called from 0xbad
stack: frame={sp:0xc42004dfa8, fp:0xc42004dfc8} stack=[0xc42004d800,0xc42004e000)
000000c42004dea8: 0000000000000001 0000000000000001
000000c42004deb8: 000000c42004ded8 000000c42004ded8
000000c42004dec8: 0000000000427eea <runtime.dopanic+74> 000000c42004ded8
000000c42004ded8: 000000000044df70 <runtime.dopanic.func1+0> 000000c420001080
000000c42004dee8: 0000000000427b21 <runtime.gopanic+961> 000000c42004df08
000000c42004def8: 000000c42004df98 0000000000427b21 <runtime.gopanic+961>
000000c42004df08: 0000000000000000 0000000000000000
000000c42004df18: 0000000000000000 0000000000000000
000000c42004df28: 0000000000000000 0000000000000000
000000c42004df38: 0000000000000000 000000c420001080
000000c42004df48: 0000000000000000 0000000000000000
000000c42004df58: 0000000000000000 0000000000000000
000000c42004df68: 000000c4200010a0 0000000000000000
000000c42004df78: 00000000004c6400 00000000005031d0
000000c42004df88: 0000000000000000 0000000000000000
000000c42004df98: 000000c42004dfb8 00000000004ae7d9 <main.badLR2+73>
000000c42004dfa8: <00000000004c6400 00000000005031d0
000000c42004dfb8: 000000c42004dfd0 !0000000000000bad
000000c42004dfc8: >0000000000000000 0000000000000000
000000c42004dfd8: 0000000000451821 <runtime.goexit+1> 0000000000000000
000000c42004dfe8: 0000000000000000 0000000000000000
000000c42004dff8: 0000000000000000
main.badLR2(0x0)
/go/src/runtime/testdata/testprog/badtraceback.go:42 +0x49
For #21431, #23484.
Change-Id: I8718fc76ced81adb0b4b0b4f2293f3219ca80786
Reviewed-on: https://go-review.googlesource.com/89016
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
On an android/amd64 emulator, $HOME points to / which is not writable.
Ignore the error in the pprof driver test.
With this, androidtest.sh on android/amd64 and android/386 passes.
Upstream pull request https://github.com/google/pprof/pull/295.
Change-Id: If919d7f44530a977fd044631ad01bac87d32deaa
Reviewed-on: https://go-review.googlesource.com/88817
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Fix breakage introduced in CL 70530.
Change-Id: I87f3da6b20554d4f405a1143b0d894c5953b63aa
Reviewed-on: https://go-review.googlesource.com/88516
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Introduce GOANDROID_ADB_FLAGS for additional flags to adb invocations.
With GOANDROID_ADG_FLAGS, the Android builders can distinguish between
emulator and device builds.
Change-Id: I11729926a523ee27f6a3795cb2cfb64a9454f0a5
Reviewed-on: https://go-review.googlesource.com/88795
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
When casting between *C.CFTypeRef and *unsafe.Pointer, we used to be
able to do the cast directly. Now with C.CFTypeRef being a uintptr
instead of an unsafe.Pointer, we need an intermediate cast.
Add the insertion of the intermediate cast to the cftype fix module.
Fixes#23091
Change-Id: I891be2f4a08cfd7de1cc4c6ab841b1e0d8c388a6
Reviewed-on: https://go-review.googlesource.com/88175
Reviewed-by: Robert Griesemer <gri@golang.org>
This change expands the documentation for Verify to mention the name
constraints and EKU behaviour.
Change-Id: Ifc80faa6077c26fcc1d2a261ad1d14c00fd13b23
Reviewed-on: https://go-review.googlesource.com/87300
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
cgo uses the presence of these functions to determine whether
a given type is in the CFTypeRef hierarchy and thus should be
a uintptr instead of a pointer. But if the *GetTypeID functions
aren't used by the user code, then they won't be present in the
cgo output, and thus cmd/fix won't see them.
Use the simpler rule that anything ending in *Ref should be
rewritten. This could over-rewrite, but I don't see a simpler
solution. Unlike cgo, it is easy to edit the output to fix any
issues. And fix is a much rarer operation than cgo.
This is a revert of portions of CL 87616.
Update #23091
Change-Id: I74ecd9fb25490a3d279b372e107248452bb62185
Reviewed-on: https://go-review.googlesource.com/88075
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
If a file uses cgo, incorporate the types generated by running cgo.
Update #23091
Change-Id: I10958fa7fd6027c2c96a9fd8a9658de35439719f
Reviewed-on: https://go-review.googlesource.com/87616
Reviewed-by: Robert Griesemer <gri@golang.org>
Cgo currently maps CFTypeRef and its subtypes to unsafe.Pointer
or a pointer to a named empty struct.
However, Darwin sometimes encodes some of CFTypeRef's subtypes as a
few int fields packed in a pointer wrapper. This hackery confuses the
Go runtime as the pointers can look like they point to things that
shouldn't be pointed at.
Switch CFTypeRef and its subtypes to map to uintptr.
Detecting the affected set of types is tricky, there are over 200 of
them, and the set isn't static across Darwin versions. Fortunately,
downcasting from CFTypeRef to a subtype requires calling CFGetTypeID,
getting a CFTypeID token, and comparing that with a known id from a
*GetTypeID() call. So we can find all the type names by detecting all
the *GetTypeID() prototypes and rewriting the corresponding *Ref types
to uintptr. This strategy covers all the cases I've checked and is
unlikely to have a false positive.
Update #23091.
Change-Id: I487eb4105c9b4785ba564de9c38d472c8c9a76ac
Reviewed-on: https://go-review.googlesource.com/87615
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We used a mix of both before.
I've never heard anybody say "an arr-double you mutex" when speaking.
Fixes#23457
Change-Id: I802b5eb2339f885ca9d24607eeda565763165298
Reviewed-on: https://go-review.googlesource.com/87896
Reviewed-by: Andrew Bonventre <andybons@golang.org>
CL 67332 created the fast no-syscall path for time.Now in High Sierra
but managed to break Sierra and older by forcing them into the slow
syscall path: the version check based on commpage version was wrong.
This CL uses the Darwin version number instead.
The assembly diff is noisy because many variables had to be
renamed, but the only actual change is the version check.
Fixes#23419.
Change-Id: Ie31ef5fb88f66d1517a8693942a7fb6100c213b0
Reviewed-on: https://go-review.googlesource.com/87655
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The tool was moved to tools/Testing/selftests within the Linux kernel
source tree. Adjust the URL in the comments of vdso_linux.go
Change-Id: I86b9cae4b898c4a45bc7c54891ce6ead91a22670
Reviewed-on: https://go-review.googlesource.com/87815
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The cgo checker was issuing an error with cgocheck=2 when a timer
bucket was stored in a pollDesc. The pollDesc values are allocated
using persistentalloc, so they are not in the Go heap. The code is OK
since timer bucket pointers point into a global array, and as such are
never garbage collected or moved.
Mark timersBucket notinheap to avoid the problem. timersBucket values
only occur in the global timers array.
Fixes#23435
Change-Id: I835f31caafd54cdacc692db5989de63bb49e7697
Reviewed-on: https://go-review.googlesource.com/87637
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I found the previous text choppy and hard to follow, and in putting
this CL together, based entirely on the existing text, I found
several details that seemed misleading to me.
This is my attempt to make the text simultaneously easier to
understand, more complete, and more precise. I may have failed in
all three, but I wanted to try.
Change-Id: I088cb457f6fcad8f2b40236949cc3ac43455e600
Reviewed-on: https://go-review.googlesource.com/87735
Reviewed-by: Russ Cox <rsc@golang.org>
After CL 69831, addTransitiveLinkDeps ensures that all dependencies of
a link appear in Deps. We no longer need to traverse through all
actions to find them. And the old scheme of looking through all the
actions and assuming we would see shared library actions before
libraries they depend on no longer works.
Now that we have complete deps, change to a simpler scheme in which we
find the shared libraries in the deps, and then use that to sort the
deps into archives and shared libraries.
Fixes#22224
Change-Id: I14fcc773ac59b6f5c2965cc04d4ed962442cc89e
Reviewed-on: https://go-review.googlesource.com/87497
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Previously we would only extract a single URL from a given CRLDP, but
https://tools.ietf.org/html/rfc5280#section-4.2.1.13 permits multiple
URLs for a single distribution point.
Fixes#23403
Change-Id: I2eaed1537df02d0627db1b86bcd9c94506236bea
Reviewed-on: https://go-review.googlesource.com/87299
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
and that they are covered by the CRYPTOGAMS license.
Fixes#22637
Change-Id: I75b8e08d3a8b569edf383c078bb11c796b766c81
Reviewed-on: https://go-review.googlesource.com/87315
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Handlers can be registered for specific hosts by specifying the host as
part of the mux pattern. If a trailing slash route is registered for
these host-based patterns, shouldRedirect should indicate that
a redirect is required.
This change modifies shouldRedirect to also take the host of the
request, and now considers host-based patterns while determining if
a request should be redirected.
Fixes#23183
Change-Id: If8753e130d5d877acdc55344833e3b289bbed2b4
Reviewed-on: https://go-review.googlesource.com/84695
Reviewed-by: Kunpei Sakai <namusyaka@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The net/http Transport was testing for a sentinel x/net/http2 error
value with ==, which meant it was only testing the bundled version. If
a user enabled http2 via golang.org/x/net/http2, the error value had a
different name.
This also updates the bundled x/net/http2 to git rev ab555f36 for:
http2: add internal function isNoCachedConnError to test for ErrNoCachedConn
https://golang.org/cl/87297Fixes#22091
Change-Id: I3fb85e2b7ba7d145dd66767e1795a56de633958c
Reviewed-on: https://go-review.googlesource.com/87298
Reviewed-by: Tom Bergan <tombergan@google.com>
GCC always recognizes the -fsplit-stack option, but then tests whether
it is supported by the selected target. If not, it reports
cc1: error: ‘-fsplit-stack’ is not supported by this compiler configuration
Check for that error message when deciding whether a compiler option works.
Change-Id: I2eef8d550bbecba3a087869df2c7351280c77290
Reviewed-on: https://go-review.googlesource.com/87136
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
We've had a series of problems with tests unexpectedly (and innocently)
looking at system files that appear to (but don't) change in meaningful ways,
like /dev/null on OS X having a modification time set to the current time.
Cut all these off by only applying file change detection to the local package
root: the GOROOT or specific sub-GOPATH in which the package being tested
is found.
(This means that if you test reads /tmp/x and you change /tmp/x, the cached
result will still be used. Don't do that, or else use -count=1.)
Fixes#23390.
Change-Id: I30b6dd194835deb645a040aea5e6e4f68af09edb
Reviewed-on: https://go-review.googlesource.com/87015
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These were the last two occurences of exec.Command("go", ...) in all of
std cmd. Checked with:
gogrep '$(f is(func))("go", $*_)' std cmd
Also changed lp_windows_test to use a test package name to avoid a
circular dependency, since internal/testenv imports os/exec.
Change-Id: I9a18948600dfecc8507ad76172e219e78b791ffd
Reviewed-on: https://go-review.googlesource.com/87200
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Updates #17245
Change-Id: I3d7ea362809040fbbba4b33efd57bf2d27d4c390
Reviewed-on: https://go-review.googlesource.com/87257
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Given an inlinable method M in package P:
func (r *MyStruct) M(...) {
When M is compiled within its home package, the source position that
the compiler records for 'r' (receiver parameter variable) is
accurate, whereas if M is built as part of the compilation of some
other package (body read from export data), the declaration line
assigned to 'r' will be the line number of the 'import' directive, not
the source line from M's source file.
This inconsistency can cause differences in the size of abstract
parameter DIEs (due to variable-length encoding), which can then in
turn result in bad abstract origin offsets, which in turn triggers
build failures on iOS (dsymutil crashes when it encounters an
incorrect abstract origin reference).
Work around the problem by removing the "declaration line number"
attribute within the abstract parameter abbreviation table entry. The
decl line attribute doesn't contribute a whole lot to the debugging
experience, and it gets rid of the inconsistencies that trigger the
dsymutil crashes.
Updates #23374.
Change-Id: I0fdc8e19a48db0ccd938ceadf85103936f89ce9f
Reviewed-on: https://go-review.googlesource.com/87055
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Ensure that we do not insert any escapers into pipelines that
already contain an equivalent escaper. This prevents overescaping
from occuring even when an aliased parse tree that has already
been escaped is escaped again.
Fixes#21844
Change-Id: Ic00d5e01c97ef09a4e49407009cf71b0d07f5c0e
Reviewed-on: https://go-review.googlesource.com/83920
Reviewed-by: Russ Cox <rsc@golang.org>
There should not be a comma before "and" in the original text,
because what follows is not a complete sentence. Rewrite.
Change-Id: Ie99f204cc87e911fb46149e2eb65e132fa1eb63a
Reviewed-on: https://go-review.googlesource.com/87020
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
All credit and blame goes to Ian for this suggestion, copied from the
runtime.
Fixes#23382
Updates #7921
Change-Id: I3d5a9ee4ab730c87e0f3feff3e7fceff9bcf9e18
Reviewed-on: https://go-review.googlesource.com/86976
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Type values being comparable implies that Type is a valid map key type.
As previously written, they sound unrelated.
Change-Id: I8e2235275d62898bfb47de850e8257b51ab5cbd6
Reviewed-on: https://go-review.googlesource.com/87021
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Suppose you build the Go toolchain in directory A,
move the whole thing to directory B, and then use
it from B to build a new program hello.exe, and then
run hello.exe, and hello.exe crashes with a stack
trace into the standard library.
Long ago, you'd have seen hello.exe print file names
in the A directory tree, even though the files had moved
to the B directory tree. About two years ago we changed
the compiler to write down these files with the name
"$GOROOT" (that literal string) instead of A, so that the
final link from B could replace "$GOROOT" with B,
so that hello.exe's crash would show the correct source
file paths in the stack trace. (golang.org/cl/18200)
Now suppose that you do the same thing but hello.exe
doesn't crash: it prints fmt.Println(runtime.GOROOT()).
And you run hello.exe after clearing $GOROOT from the
environment.
Long ago, you'd have seen hello.exe print A instead of B.
Before this CL, you'd still see hello.exe print A instead of B.
This case is the one instance where a moved toolchain
still divulges its origin. Not anymore. After this CL, hello.exe
will print B, because the linker sets runtime/internal/sys.DefaultGoroot
with the effective GOROOT from link time.
This makes the default result of runtime.GOROOT once again
match the file names recorded in the binary, after two years
of divergence.
With that cleared up, we can reintroduce GOROOT into the
link action ID and also reenable TestExecutableGOROOT/RelocatedExe.
When $GOROOT_FINAL is set during link, it is used
in preference to $GOROOT, as always, but it was easier
to explain the behavior above without introducing that
complication.
Fixes#22155.
Fixes#20284.
Fixes#22475.
Change-Id: Ifdaeb77fd4678fdb337cf59ee25b2cd873ec1016
Reviewed-on: https://go-review.googlesource.com/86835
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The linker has been applying -X options before loading symbols,
meaning that when it sees -X y=z it creates a symbol named y
and initializes its string data to z. The symbol named y is marked
"DUPOK" so that when the actual packages are loaded, no error is
emitted when the real y is seen. The predefined y's data is used
instead of whatever the real y says.
If we define -X y=z and we never load y, then the predefined symbol
is dropped during dead code elimination, but not in shared library
builds. Shared library builds must include all symbols, so we have to
be more careful about not defining symbols that wouldn't have
appeared anyway.
To be more careful, save the -X settings until after all the symbols
are loaded from the packages, and then apply the string changes
to whatever symbols are known (but ignore the ones that were not
loaded at all). This ends up being simpler anyway, since it doesn't
depend on DUPOK magic.
Makes CL 86835 safe.
Fixes#23273.
Change-Id: Ib4c9b2d5eafa97c5a8114401dbec0134c76be54f
Reviewed-on: https://go-review.googlesource.com/86915
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit cd0a5f0829, which
unnecessarily restricts the use of AddParseTree.
Change-Id: I1155214a20ba08981d604404e79fff54874fd8e4
Reviewed-on: https://go-review.googlesource.com/83919
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
When benchmarks run, they print lines like:
BenchmarkGenericNoMatch-8 3000000 385 ns/op
The first field, padded by spaces and followed by a tab,
is printed when the benchmark begins running.
The rest of the line is printed when the benchmark ends.
Tools and people can watch the timing of these prints
to see which benchmark is running.
To allow tools consuming json output to continue to be
able to see which benchmark is running, this CL adds a
special case to the usual "line at a time" behavior to flush
the benchmark name if it is observed separately from the
rest of the line.
Fixes#23352.
Change-Id: I7b6410698d78034eec18745d7f57b7d8e9575dbb
Reviewed-on: https://go-review.googlesource.com/86695
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Apple changed the format of its support page, so we need to
restructure the HTML parser. The HTML table is now parsed using
regular expressions, and certificates are then found in macOS
trust store by their fingerprint.
Fixes#22181
Change-Id: I29e7a40d37770bb005d728f1832299c528691f7e
Reviewed-on: https://go-review.googlesource.com/77252
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This just adds support on ELF systems, which is OK for now since that
is all that gccgo works on.
For the archive file generated by the compiler we add a new file
_buildid.o that has a section .go.buildid containing the build ID.
Using a new file lets us set the SHF_EXCLUDE bit in the section header,
so the linker will discard the section. It would be nicer to use
`objcopy --add-section`, but objcopy doesn't support setting the
SHF_EXCLUDE bit.
For an executable we just use an ordinary GNU build ID. Doing this
required modifying cmd/internal/buildid to look for a GNU build ID,
and use it if there is no other Go-specific note.
This CL fixes a minor bug in gccgoTOolchain.link: it was using .Target
instead of .built, so it failed for a cached file.
This CL fixes a bug reading note segments: the notes are aligned as
reported by the PT_NOTE's alignment field.
Updates #22472
Change-Id: I4d9e9978ef060bafc5b9574d9af16d97c13f3102
Reviewed-on: https://go-review.googlesource.com/85555
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
If a benchmark calls b.Log without failing (without b.Error/b.Fatal/b.FailNow)
then that turns into output very much like a test passing,
except it says BENCH instead of PASS.
Benchmarks failing say FAIL just like tests failing.
Fixes#23346.
Change-Id: Ib188e695952da78057ab4a13f90d49937aa3c232
Reviewed-on: https://go-review.googlesource.com/86396
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It is natural for tools to take a large string concatenation like
"1" + "1" + "1" + ... + "1"
and translate that into a sequence of go/constant calls:
x := constant.MakeString("1")
x = constant.BinaryOp(x, token.ADD, constant.MakeString("1"))
x = constant.BinaryOp(x, token.ADD, constant.MakeString("1"))
x = constant.BinaryOp(x, token.ADD, constant.MakeString("1"))
x = constant.BinaryOp(x, token.ADD, constant.MakeString("1"))
...
If the underlying representation of a string constant is a Go string,
then this leads to O(N²) memory for the concatenation of N strings,
allocating memory for "1", "11", "111", "1111", and so on.
This makes go/types and in particular cmd/vet run out of memory
(or at least use far too much) on machine-generated string concatenations,
such as those generated by go-bindata.
This CL allows code like the above to operate efficiently, by delaying
the evaluation of the actual string constant value until it is needed.
Now the representation of a string constant is either a string or an
explicit addition expression. The addition expression is turned into
a string the first time it is requested and then cached for future use.
This slows down the use of single strings, but analyses are likely not
dominated by that use anyway. It speeds up string concatenations,
especially large ones, significantly.
On my Mac running 32-bit code:
name old time/op new time/op delta
StringAdd/1-8 160ns ± 2% 183ns ± 1% +13.98% (p=0.000 n=10+10)
StringAdd/4-8 650ns ± 1% 927ns ± 4% +42.73% (p=0.000 n=10+10)
StringAdd/16-8 3.93µs ± 1% 2.78µs ± 2% -29.12% (p=0.000 n=8+9)
StringAdd/64-8 37.3µs ± 9% 10.1µs ± 5% -73.06% (p=0.000 n=10+10)
StringAdd/256-8 513µs ± 5% 38µs ± 1% -92.63% (p=0.000 n=10+10)
StringAdd/1024-8 5.67ms ± 4% 0.14ms ± 2% -97.45% (p=0.000 n=8+10)
StringAdd/4096-8 77.1ms ± 9% 0.7ms ± 2% -99.10% (p=0.000 n=10+9)
StringAdd/16384-8 1.33s ± 7% 0.00s ±10% -99.64% (p=0.000 n=10+10)
StringAdd/65536-8 21.5s ± 4% 0.0s ± 8% -99.89% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
StringAdd/1-8 232B ± 0% 256B ± 0% +10.34% (p=0.000 n=10+10)
StringAdd/4-8 1.20kB ± 0% 1.24kB ± 0% +3.33% (p=0.000 n=10+10)
StringAdd/16-8 14.7kB ± 0% 4.6kB ± 0% -68.87% (p=0.000 n=10+10)
StringAdd/64-8 223kB ± 0% 16kB ± 0% -92.66% (p=0.000 n=10+10)
StringAdd/256-8 3.48MB ± 0% 0.07MB ± 0% -98.07% (p=0.000 n=10+10)
StringAdd/1024-8 55.7MB ± 0% 0.3MB ± 0% -99.53% (p=0.000 n=10+10)
StringAdd/4096-8 855MB ± 0% 1MB ± 0% -99.88% (p=0.000 n=10+10)
StringAdd/16384-8 13.5GB ± 0% 0.0GB ± 0% -99.97% (p=0.000 n=9+10)
StringAdd/65536-8 215GB ± 0% 0GB ± 0% -99.99% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
StringAdd/1-8 3.00 ± 0% 3.00 ± 0% ~ (all equal)
StringAdd/4-8 9.00 ± 0% 11.00 ± 0% +22.22% (p=0.000 n=10+10)
StringAdd/16-8 33.0 ± 0% 25.0 ± 0% -24.24% (p=0.000 n=10+10)
StringAdd/64-8 129 ± 0% 75 ± 0% -41.86% (p=0.000 n=10+10)
StringAdd/256-8 513 ± 0% 269 ± 0% -47.56% (p=0.000 n=10+10)
StringAdd/1024-8 2.05k ± 0% 1.04k ± 0% -49.29% (p=0.000 n=10+10)
StringAdd/4096-8 8.19k ± 0% 4.12k ± 0% -49.77% (p=0.000 n=10+10)
StringAdd/16384-8 32.8k ± 0% 16.4k ± 0% -49.97% (p=0.000 n=9+10)
StringAdd/65536-8 131k ± 0% 66k ± 0% -50.11% (p=0.000 n=10+10)
https://perf.golang.org/search?q=upload:20180105.2Fixes#23348 (originally reported as cmd/vet failures in comments on #23222).
This makes constant.Values of Kind String no longer meaningful for ==, which
required fixes in go/types. While there, also fix go/types handling of constant.Values
of Kind Int (for uint64), Float, and Complex.
Change-Id: I80867bc9c4232c5c9b213443ff16645434a68b36
Reviewed-on: https://go-review.googlesource.com/86395
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
I marked every test that takes more than 0.5 seconds on my machine
as something to run only when not in -short mode, or in -short mode
on the beefy linux/amd64, windows/amd64, and darwin/amd64 builders.
I also shortened a few needlessly-expensive tests where possible.
Cuts the time for go test -short cmd/go from 45s to 15s on my machine.
Should help even more on some of our builders and slower user machines.
Fixes#23287.
Change-Id: I0e36003ef947b0ebe4224a1373731f9fa9216843
Reviewed-on: https://go-review.googlesource.com/86252
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also vendors x/net/http git rev 42fe2e1c for:
http2: don't check WriteHeader status if we've already sent the header
https://golang.org/cl/86255Fixes#23010
Change-Id: I4f3dd63acb52d5a34a0350aaf847a7a376d6968f
Reviewed-on: https://go-review.googlesource.com/86275
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The docs were too specific. Make it vaguer. There are conditions for
which the Transport will try to reuse a connection anyway, even if the
Response Body isn't read to EOF or closed, but we don't need to get
into all the details in the docs.
Fixes#22954
Change-Id: I3b8ae32aeb1a61b396d0026e129552afbfecceec
Reviewed-on: https://go-review.googlesource.com/86276
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 84735 strengthened the -x test to make sure commands succeed,
using set -e, but the gcc flag tests can fail. Change them to say || true.
Fixes#23337.
Change-Id: I01e4017cb36ceb147b56935c2636de52ce7bdfdb
Reviewed-on: https://go-review.googlesource.com/86239
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Follows the wording in RFC4366 more precisely which allows a server
to optionally return a "certificate_status" when responding to a
client hello containing "status_request" extension.
fixes#8549
Change-Id: Ib02dc9f972da185b25554568fe6f8bc411d9c0b7
Reviewed-on: https://go-review.googlesource.com/86115
Reviewed-by: Adam Langley <agl@golang.org>
In the comment of seedPost, the word: condiiton was changed to: condition
Change-Id: I8967cc0e9f5d37776bada96cc1443c8bf46e1117
Reviewed-on: https://go-review.googlesource.com/86156
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If test case framing appears in ordinary test output,
then test2json can get confused. If the fake framing is being
saved with t.Logf/t.Errorf/etc then we can already
distinguish it from real framing, and the code did.
It just forgot to write that framing as output (1-line fix).
If the fake framing is being generated by printing directly
to stdout/stderr, then test2json will simply get confused.
There's not a lot to do at that point (maybe it's even a feature).
Fixes#23036.
Change-Id: I29449c7ace304172b89d8babe23de507c0500455
Reviewed-on: https://go-review.googlesource.com/86238
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
go test -json was inadvertently disabling caching. Fix that.
Fixes#22984.
Change-Id: Ic933a8c8ac00ce8253e934766954b1ccc6ac0cec
Reviewed-on: https://go-review.googlesource.com/84075
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If you have a package p1 with an xtest (package p1_test)
that imports p2, where p2 itself imports p1, then when
trying to do coverage for p1 we need to make sure to
recompile p2. The problem was that the overall package
import graph looked like:
main -> p1_test -> p2 -> p1
Since we were recompiling p1 with coverage, we correctly
figured out that because p2 depends on a package being
recompiled due to coverage, p2 also needs to be split (forked) to
insert the dependency on the modified p1. But then we used
the same logic to split p1_test and main, with the effect that
the changes to p2 and p1_test and main were lost, since the
caller was still holding on to the original main, not the split version.
Change the code to treat main and p1_test as "already split"
and just update them in place.
Fixes#23314.
Change-Id: If7edeca6e39cdaeb5b9380d00b0c7d8c5891f086
Reviewed-on: https://go-review.googlesource.com/86237
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
findrunnable loops over allp to check run queues *after* it has
dropped its own P. This is unsafe because allp can change when nothing
is blocking safe-points. Hence, procresize could change allp
concurrently with findrunnable's loop. Beyond generally violating Go's
memory model, in the best case this could findrunnable to observe a
nil P pointer if allp has been grown but the new slots not yet
initialized. In the worst case, the reads of allp could tear, causing
findrunnable to read a word that isn't even a valid *P pointer.
Fix this by taking a snapshot of the allp slice header (but not the
backing store) before findrunnable drops its P and iterating over this
snapshot. The actual contents of allp are immutable up to len(allp),
so this fixes the race.
Updates #23098 (may fix).
Change-Id: I556ae2dbfffe9fe4a1bf43126e930b9e5c240ea8
Reviewed-on: https://go-review.googlesource.com/86215
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Update rewrite algorithm by coping code from
go/internal/work/buildid:updateBuildID.
Probably, this is not the best option. We could provide high-level API
in cmd/internal/buildid in the future.
Fixes#23181
Change-Id: I336a7c50426ab39bc9998b55c372af61a4fb21a7
Reviewed-on: https://go-review.googlesource.com/84735
Reviewed-by: Russ Cox <rsc@golang.org>
In func TestXxxx(*testing.T) the Xxxx can be anything that can appear
in an identifier, but can't start with a lowercase letter. Clarify the docs.
Fixes#23322
Change-Id: I5c297916981f7e3890ee955d12bc7422a75488e2
Reviewed-on: https://go-review.googlesource.com/86001
Reviewed-by: Rob Pike <r@golang.org>
CL 78735 description:
time: add space padding layout strings(using underscore) for not only day but others
As mentioned in #22802, only day component of layout string has space
padding(represented by one underscore before its placeholder). This
commit expands the rule for month, hour, minute and second.
Updates #22802 (maybe fixes it)
Revert this CL because it breaks currently working formats that happen
to use underscores.
Fixes#23259
Change-Id: I64acaaca9b5b74785ee0f0be7910574e87daa649
Reviewed-on: https://go-review.googlesource.com/85998
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
I don't know why these errors occur. Ignore them to avoid breaking the
build.
Updates #22019
Change-Id: Ia048e6d9b928e8e237b311ff3a364e7a23af4aa4
Reviewed-on: https://go-review.googlesource.com/86000
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We retrieve an error using getsockopt with SO_ERROR. We were reporting
the error as coming from "getsockopt", but really it is coming from
"connect". It is not getsockopt that failed.
Fixes#19302
Change-Id: I510ab76e4b04c70cd9dfdfc46d9a410bf653d017
Reviewed-on: https://go-review.googlesource.com/85997
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Commit c2c07c7989 (CL 49331) changed the linker and runtime to always
use 2MB stacks on 64-bit Windows. This is the corresponding change to
make 32-bit Windows always use large (1MB) stacks because it's
difficult to detect when Windows applications will call into arbitrary
C code that may expect a large stack.
This is done as a separate change because it's possible this will
cause too much address space pressure for a 32-bit address space. On
the other hand, cgo binaries on Windows already use 1MB stacks and
there haven't been complaints.
Updates #20975.
Change-Id: I8ce583f07cb52254fb4bd47250f1ef2b789bc490
Reviewed-on: https://go-review.googlesource.com/49610
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
During the refactor in 1126d1483f I
introduced a logical error within one withLock function that used
the result of the call before checking for the error. Change
the order so that the error is checked before the result is used.
None of the other withLock uses have similar issues.
Fixes#23208
Change-Id: I6c5dcf262e36bad4369c850f1e0131066360a82e
Reviewed-on: https://go-review.googlesource.com/85175
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A Select Op could produce a value with upper 32 bits NOT zeroed,
for example, Div32 is lowered to (Select0 (DIVL x y)).
In theory, we could look into the argument of a Select to decide
whether the upper bits are zeroed. As it is late in release cycle,
just disable this optimization for Select for now.
Fixes#23305.
Change-Id: Icf665a2af9ccb0a7ba0ae00c683c9e349638bf85
Reviewed-on: https://go-review.googlesource.com/85736
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Modify the regex in TestLinuxSendfile to not match the parameters of
the syscall, just its name and the opening parenthesis. This is enough
to recognize that the syscall was invoked.
This fixes the TestLinuxSendfile test when running in Clear Linux,
where strace always execute with -yy implied, having output with extra
information in the parameters:
[pid 5336] sendfile(6<TCP:[127.0.0.1:35007->127.0.0.1:55170]>, 8</home/c/src/go/src/net/http/testdata/index.html>, NULL, 22) = 22
Change-Id: If7639b785d5fdf65fae8e6149a97a57b06ea981c
Reviewed-on: https://go-review.googlesource.com/85657
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#23224
The previous Pow code had an optimization for
powers equal to ±0.5 that used Sqrt for
increased accuracy/speed. This caused special
cases involving powers of ±0.5 to disagree with
the Pow spec. This change places the Sqrt optimization
after all of the special case handling.
Change-Id: I6bf757f6248256b29cc21725a84e27705d855369
Reviewed-on: https://go-review.googlesource.com/85660
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
this solution as it stands doesn't work with
non-english device names (golang/go#23191 (which has a fix))
and names some devices differently (golang/go#23153) probably due to the
fact that this test previously only ran on Server 2008.
Re-opens golang/go#20073
Change-Id: I5c36774ddd85ac07620b4015372d564acbb169ad
Reviewed-on: https://go-review.googlesource.com/85315
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Refactoring to make it slightly easier to add tests,
easier to add variable-printing-support for Delve,
and made naming and tagging more consistent.
No changes to the content of the test itself or when it is
run.
Change-Id: I374815b65a203bd43b27edebd90b859466d1c33b
Reviewed-on: https://go-review.googlesource.com/84979
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
golang.org/cl/81315 attempted to distinguish system goroutines
by examining the function name in the goroutine stack. It assumes that
the information would be available when GoSysBlock or GoInSyscall
events are processed, but it turned out the stack information is
set too late (when the goroutine gets a chance to run).
This change initializes the goroutine information entry when
processing GoCreate event which should be one of the very first
events for the every goroutine in trace.
Fixes#22574
Change-Id: I1ed37087ce2e78ed27c9b419b7d942eb4140cc69
Reviewed-on: https://go-review.googlesource.com/83595
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
My previous fix for issue 23179 was incomplete; it turns out that if
an unnamed parameter is below a specific size threshold, it gets
register-promoted away by the compiler (hence not encountered during
some parts of DWARF inline info processing), but if it is sufficiently
large, it is allocated to the stack as a named variable and treated as
a regular parameter by DWARF generation. Interestingly, something in
the ppc64le build of k8s causes an unnamed parameter to be retained
(where on amd64 it is deleted), meaning that this wasn't caught in my
amd64 testing.
The fix is to insure that "_" params are treated in the same way that
"~r%d" return temps are when matching up post-optimization inlined
routine params with pre-inlining declarations. I've also updated the
test case to include a "_" parameter with a very large size, which
also triggers the bug on amd64.
Fixes#23179.
Change-Id: I961c84cc7a873ad3f8f91db098a5e13896c4856e
Reviewed-on: https://go-review.googlesource.com/84975
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The current implementation prints a log, "invalid program: unexpected
type for embedded field", when the form *package.ident is embedded in
a struct declaration.
Note that since valid qualified identifiers must be exported, the result
for a valid program does not change.
Change-Id: If8b9d7056c56b6a6c5482eb749168a63c65ef685
Reviewed-on: https://go-review.googlesource.com/84436
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The helper routine for returning pre-inlining parameter declarations
wasn't properly handling the case where you have more than one
parameter named "_" in a function signature; this triggered a map
collision later on when the function was inlined and DWARF was
generated for the inlined routine instance.
Fixes#23179.
Change-Id: I12e5d6556ec5ce08e982a6b53666a4dcc1d22201
Reviewed-on: https://go-review.googlesource.com/84755
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The driver.Value type may be more then the documented 6 types if the
database driver supports it. Document that fact.
Updates #23077
Change-Id: If7e2112fa61a8cc4e155bb31e94e89b20c607242
Reviewed-on: https://go-review.googlesource.com/84636
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Disable the three linker DWARF tests that invoke the compiler in
non-debug mode on Solaris, since this seems to trigger a split stack
overflow. These can be turned back on once the issue in question is
resolved.
Updates #23168.
Change-Id: I5be1b098e33e8bad3bc234a0964eab1dee7e7954
Reviewed-on: https://go-review.googlesource.com/84655
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Rename -list flag to -errlist to avoid confusion with the go
test flag -list (introduced later).
This flag is only needed to get an error list when running the
go/types test harness manually on select files, e.g., as in:
go test -run=Check -files=x.go -errlist
Change-Id: I67febcf968d2d8f4ff00c81eea7b2df723560eac
Reviewed-on: https://go-review.googlesource.com/84378
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Exercise of preparing a how-to document motivated me to
clean up some of the stupider wonkier bits. Since this
does not run for test -short, expect no change for trybots,
did pass testing with OSX gdb and a refreshed copy of Delve.
Change-Id: I58edd10599b172c4787ff5f110db078f6c2c81c5
Reviewed-on: https://go-review.googlesource.com/83957
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Location lists are only supported on x86 and amd64, so the
test expecting them failed everywhere else. Make that test
skip unless GOARCH is x86 or amd64.
Change-Id: Id86b34d30c6a0b97e6fa0cd5aca31f51ed84f556
Reviewed-on: https://go-review.googlesource.com/84395
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The default test timeout is 10 minutes if unspecified.
The misc/cgo/testshared test didn't use t.timeout(sec), which respects
GO_TEST_TIMEOUT_SCALE, so all builders got the default 10 minute
timeout. arm5 needs more, though, so specify 10 minutes explicitly,
which will then get scaled accordingly on slower builders.
Change-Id: I19ecfdcd9c865f2b69524484415b8fbd2852718e
Reviewed-on: https://go-review.googlesource.com/84315
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Change the compiler's DWARF inline info generation to be more careful
about producing consistent instances of abstract function DIEs. The
new strategy is to insure that the only params/variables created in an
abstract subprogram DIE are those corresponding to declarations in the
original pre-inlining version of the code. If a concrete subprogram
winds up with other vars as part of the compilation process (return
temps, for example, or scalars generated by splitting a structure into
pieces) these are emitted as regular param/variable DIEs instead of
concrete DIEs.
The linker dwarf test now has a couple of new testpoints that include
checks to make sure that all abstract DIE references are
sane/resolvable; this will help catch similar problems in the future.
Fixes#23046.
Change-Id: I9b0030da8673fbb80b7ad50461fcf8c6ac823a37
Reviewed-on: https://go-review.googlesource.com/83675
Run-TryBot: Than McIntosh <thanm@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This attempts to symbolize the PC of morestack's caller when there's a
stack split at a bad time. The stack trace starts at the *caller* of
the function that attempted to grow the stack, so this is useful if it
isn't obvious what's being called at that point, such as in #21431.
Change-Id: I5dee305d87c8069611de2d14e7a3083d76264f8f
Reviewed-on: https://go-review.googlesource.com/84115
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/go has grown slow, even in short mode, and it's now regularly
failing on a number of builders where it's taking over the previous 3
minute timeout. for now, give it more time.
Change-Id: If565baf71c2770880b2e2139b47e03433951331f
Reviewed-on: https://go-review.googlesource.com/84235
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing code simply dropped them on the floor. Don't do that.
Fixes#23130.
Change-Id: I10f20e41f2c466a76519983253f87af7cf6d5e70
Reviewed-on: https://go-review.googlesource.com/83918
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change dump file names to group them alphabetically in directory
listings, in pass run order.
Change-Id: I8070578a5b4a3a7983dcc527ea1cfdb10a6d7d24
Reviewed-on: https://go-review.googlesource.com/83958
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The name-based heuristics fail too often to be on during "go test",
but we really want the printf vet check in "go test", so change to
a list of exactly which standard library functions are print-like.
For a later release we'd like to bring back checking for user-defined
wrappers, but in a completely precise way. Not for Go 1.10, though.
The new, more precise list includes t.Skipf, which caught some
mistakes in standard library tests.
Fixes#22936.
Change-Id: I110448e3f6b75afd4327cf87b6abb4cc2021fd0d
Reviewed-on: https://go-review.googlesource.com/83838
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Two minor changes to allow fixes in cmd/vet's printf checking.
1. Pass package import path in vet config, so that vet knows
whether it is, for example, vetting "fmt".
2. Add new, but undocumented and for now unsupported
flag -vettool to control which vet binary is invoked during go vet.
This lets the cmd/vet tests build and test a throwaway vet.exe
using cmd/go to ensure type checking information, all without
installing a potentially buggy cmd/vet.
For #22936.
Change-Id: I18df7c796ebc711361c847c63eb3ee17fb041ff7
Reviewed-on: https://go-review.googlesource.com/83837
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Srcset is largely the same as a URL, but is escaped in URL contexts.
Inside a srcset attribute, URLs have their commas percent-escaped to
avoid having the URL be interpreted as multiple URLs. Srcset is placed
in a srcset attribute literally.
Fixes#17441
Change-Id: I676b544784c7e54954ddb91eeff242cab25d02c4
Reviewed-on: https://go-review.googlesource.com/38324
Reviewed-by: Kunpei Sakai <namusyaka@gmail.com>
Reviewed-by: Mike Samuel <mikesamuel@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(This only manifested in test vet failures for packages without tests,
or else we'd probably have seen this sooner.)
Fixes#23047.
Change-Id: I41d09a7780999bbe1951377ffcc811ba86ea5000
Reviewed-on: https://go-review.googlesource.com/83955
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tests exist that call m.Run in a loop‽
Now we have one too.
Fixes#23129.
Change-Id: I8cbecb724f239ae14ad45d75e67d12c80e41c994
Reviewed-on: https://go-review.googlesource.com/83956
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
os.NewFile doesn't put the fd into non-blocking mode.
In most cases, an *os.File returned by os.NewFile is in blocking mode.
Updates #7970
Updates #21856
Updates #23111
Change-Id: Iab08432e41f7ac1b5e25aaa8855d478adb7f98ed
Reviewed-on: https://go-review.googlesource.com/83995
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The export prologue goes into the _cgo_export.h file, where it may be
be #include'd by a .swig file. As SWIG defines its own type "intgo",
the definition of "intgo" in the export prologue could conflict.
Since we don't need to define "intgo" in the _cgo_export.h file, don't.
Defining "intgo" in _cgo_export.h was new for this release, so this
should not break any existing code.
No test case as I can't quite bring myself to write a test that
combines SWIG and cgo.
Change-Id: I8073e8300a1860cecd5994b9ad07dd35a4298c89
Reviewed-on: https://go-review.googlesource.com/83936
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
t.Run(f) does not wait for f after f calls t.Parallel.
Otherwise it would be impossible to create new
parallel sibling subtests for f.
Fixes#22993.
Change-Id: I27e1555ab1ff608eb8155db261d5e7ee8f486aef
Reviewed-on: https://go-review.googlesource.com/83880
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If package strings has a particular set of gcflags, then the strings_test
pseudo-package built as part of the test binary started inheriting the
same flags in CL 81496, to fix#22831.
Now the package main and final test binary link built as part of the
strings test binary also inherit the same flags, to fix#22994.
I am slightly uneasy about reusing package strings's flags for
package main, but the alternative would be to introduce some
kind of special case, which I'd be even more uneasy about.
This interpretation preserves the Go 1.9 behavior of existing
commands like:
go test -c -ldflags=-X=mypkg.debugString=foo mypkg
Fixes#22994.
Change-Id: I9ab83bf1a9a6adae530a7715b907e709fd6c1b5d
Reviewed-on: https://go-review.googlesource.com/83879
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The current implementation ignores certs wherein the
Subject does not match the Issuer. An example of where
this causes issue is an enterprise environment with
intermediate CAs. In this case, the issuer is separate
(and may be loaded) but the intermediate is ignored.
A TLS handshake that does not include the intermediate
cert would then fail with an untrusted error in Go.
On other platforms (darwin-nocgo included), all trusted
certs are loaded and accepted reguardless of
Subject/Issuer names.
This change removes the Subject/Issuer name-matching
restriction of certificates when trustAsRoot is set,
allowing all trusted certs to be loaded on darwin (cgo).
Refs #16532
Change-Id: I451e929588f8911892be6bdc2143d0799363c5f8
Reviewed-on: https://go-review.googlesource.com/36942
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
As mentioned in #22802, only day component of layout string has space
padding(represented by one underscore before its placeholder). This
commit expands the rule for month, hour, minute and second.
Updates #22802 (maybe fixes it)
Change-Id: I886998380489862ab9a324a6774f2e4cf7124122
Reviewed-on: https://go-review.googlesource.com/78735
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
An exec command is normally used on platforms were the test is run in
some unusual way, making it less likely that the testlog will be useful.
Updates #22593
Change-Id: I0768f6da89cb559d8d675fdf6d685db9ecedab9e
Reviewed-on: https://go-review.googlesource.com/83578
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We can't currently inline functions that contain closures anyway, so
just delete this budgeting code for now. Re-enable once we can (if
ever) inline functions with nested closures.
Updates #15561.
Fixes#23093.
Change-Id: Idc5f8e042ccfcc8921022e58d3843719d4ab821e
Reviewed-on: https://go-review.googlesource.com/83538
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When using -importcfg, the import paths recorded by the compiler in
the object file are simply the import paths. When not using -importcfg,
the import paths have a trailing ".a". Assume that if we are using
-importcfg with the compiler, we are using it with the linker,
and so if the linker sees an -importcfg option it should not
strip ".a" from the import path read from the object files.
This was mostly working because the linker only strips a trailing
".x" for a literal dot and any single character 'x'. Since few import
paths end with ".x", most programs worked fine.
Fixes#22986
Change-Id: I6c10a160b97dd63fff3931f27a1514c856e8cd52
Reviewed-on: https://go-review.googlesource.com/81878
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Otherwise, on systems for which syscall does not implement Getwd,
a lot of unnecessary files and directories get added to the testlog,
right up the root directory. This was causing tests on such systems
to fail to cache in practice.
Updates #22593
Change-Id: Ic8cb3450ea62aa0ca8eeb15754349f151cd76f85
Reviewed-on: https://go-review.googlesource.com/83455
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
All plugins issues I would call bugs now closed, so
(with some amount of optimism) update the plugin documentation.
Change-Id: Ia421c18a166d7cdf599ac86f2336541c1ef42a0d
Reviewed-on: https://go-review.googlesource.com/65670
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It causes every test to fail as the log file is on the local file system,
not the NaCl file system.
Updates #22593
Change-Id: Iee3d8307317bd792c9c701baa962ebbbfa34c147
Reviewed-on: https://go-review.googlesource.com/83256
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead of requiring that cmd/api/run.go be edited upon each
release to include the next Go version number, look in $GOROOT/api
for files with the prefix go1* and use those instead to perform
API checks.
Change-Id: I5d9407f2bd368ff5e62f487cccdd245641ca9c9b
Reviewed-on: https://go-review.googlesource.com/83355
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The Builder's ReadFrom method allows the underlying unsafe slice to
escape, and for callers to subsequently modify memory that had been
unsafely converted into an immutable string.
In the original proposal for Builder (#18990), I'd noted there should
be no Read methods:
> There would be no Reset or Bytes or Truncate or Read methods.
> Nothing that could mutate the []byte once it was unsafely converted
> to a string.
And in my prototype (https://golang.org/cl/37767), I handled ReadFrom
properly, but when https://golang.org/cl/74931 arrived, I missed that
it had a ReadFrom method and approved it.
Because we're so close to the Go 1.10 release, just remove the
ReadFrom method rather than think about possible fixes. It has
marginal utility in a Builder anyway.
Also, fix a separate bug that also allowed mutation of a slice's
backing array after it had been converted into a slice by disallowing
copies of the Builder by value.
Updates #18990Fixes#23083Fixes#23084
Change-Id: Id1f860f8a4f5f88b32213cf85108ebc609acb95f
Reviewed-on: https://go-review.googlesource.com/83255
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When we write a cached test result, we now also write a log of the
environment variables and files inspected by the test run,
along with a hash of their content. Before reusing a cached test result,
we recompute the hash of the content specified by the log, and only
use the result if that content has not changed.
This makes test caching behave correctly for tests that consult
environment variables or stat or read files or directories.
Fixes#22593.
Change-Id: I8608798e73c90e0c1911a38bf7e03e1232d784dc
Reviewed-on: https://go-review.googlesource.com/81895
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, wbBufFlush does nothing if the goroutine is dying on the
assumption that the system is crashing anyway and running the write
barrier may crash it even more. However, it fails to reset the
buffer's "next" pointer. As a result, if there are later write
barriers on the same P, the write barrier will overflow the write
barrier buffer and start corrupting other fields in the P or other
heap objects. Often, this corrupts fields in the next allocated P
since they tend to be together in the heap.
Fix this by always resetting the buffer's "next" pointer, even if
we're not doing anything with the pointers in the buffer.
Updates #22987 and #22988. (May fix; it's hard to say.)
Change-Id: I82c11ea2d399e1658531c3e8065445a66b7282b2
Reviewed-on: https://go-review.googlesource.com/83016
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
heapBits.bits is used during bulkBarrierPreWrite via
heapBits.isPointer, which means it must not be preempted. If it is
preempted, several bad things can happen:
1. This could allow a GC phase change, and the resulting shear between
the barriers and the memory writes could result in a lost pointer.
2. Since bulkBarrierPreWrite uses the P's local write barrier buffer,
if it also migrates to a different P, it could try to append to the
write barrier buffer concurrently with another write barrier. This can
result in the buffer's next pointer skipping over its end pointer,
which results in a buffer overflow that can corrupt arbitrary other
fields in the Ps (or anything in the heap, really, but it'll probably
crash from the corrupted P quickly).
Fix this by marking heapBits.bits go:nosplit. This would be the
perfect use for a recursive no-preempt annotation (#21314).
This doesn't actually affect any binaries because this function was
always inlined anyway. (I discovered it when I was modifying heapBits
and make h.bits() no longer inline, which led to rampant crashes from
problem 2 above.)
Updates #22987 and #22988 (but doesn't fix because it doesn't actually
change the generated code).
Change-Id: I60ebb928b1233b0613361ac3d0558d7b1cb65610
Reviewed-on: https://go-review.googlesource.com/83015
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Parts of TestUDPConnSpecificMethods and TestWriteToUDP fail, because
UDPConn.WriteMsgUDP is broken on Windows XP. UDPConn.WriteMsgUDP uses
Windows WSASendMsg API, but that call is not implemented on Windows XP (see
https://msdn.microsoft.com/en-us/library/windows/desktop/ms741692(v=vs.85).aspx
for details)
Update #23072
Change-Id: I4e8f149bc62bd87cd7c199e6832b9ce479af0a3e
Reviewed-on: https://go-review.googlesource.com/83077
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
On DragonFly mmap with MAP_STACK returns the top of the region, not
the bottom. Rather than try to cope, just don't use the flag anywhere.
Fixes#23061
Change-Id: Ib5df4dd7c934b3efecfc4bc87f8989b4c37555d7
Reviewed-on: https://go-review.googlesource.com/83035
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The package unsafe docs say it's safe to convert an unsafe.Pointer to
uintptr in the argument list to an assembly function, but it was
erroneously only detecting normal pointers converted to unsafe.Pointer
and then to intptr.
Fixes#23051.
Change-Id: Id1be19f6d8f26f2d17ba815191717d2f4f899732
Reviewed-on: https://go-review.googlesource.com/82817
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This essentially applies https://golang.org/cl/81636 to the net package.
The full truth seems too complicated to write in this method's doc, so
I'm going with a simple half truth.
The full truth is that File returns the descriptor in blocking mode,
because that is historically how it worked, and existing programs
would be surprised if the descriptor is suddenly non-blocking. On Unix
systems whether a socket is non-blocking or not is a property of the
underlying file description, not of a particular file descriptor, so
changing the returned descriptor to blocking mode also changes the
existing socket to blocking mode. Blocking mode works fine, althoug I/O
operations now take up a thread. SetDeadline and friends rely on the
runtime poller, and the runtime poller only works if the descriptor is
non-blocking. So it's correct that calling File disables SetDeadline.
The other half of the truth is that if the program is willing to work
with a non-blocking descriptor, it could call
syscall.SetNonblock(f.Fd(), true) to change the descriptor, and
the original socket, to non-blocking mode. At that point SetDeadline
would start working again. I tried to write that in a way that is
short and comprehensible but failed. Since we now have the RawConn
approach to frobbing the descriptor, and hopefully most people can use
that rather than calling File, I decided to punt.
Updates #22934Fixes#21862
Change-Id: If269da762f6f5a88c334e7b6d6f3998f7e10b11e
Reviewed-on: https://go-review.googlesource.com/82915
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This test has been getting occasional timeouts on the race builder.
The point of the test is whether a file descriptor leaks, not whether
the connection occurs in a certain amount of time. So use a very large
timeout. The connection is normally fast and the timeout doesn't matter.
Updates #13324
Change-Id: Ie1051c4a0be1fca4e63b1277101770be0cdae512
Reviewed-on: https://go-review.googlesource.com/82916
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The jobject type is declared as a pointer, but some JVMs
(Dalvik, ART) store non-pointer values in them. In Go, we must
use uintptr instead of a real pointer for these types.
This is similar to the CoreFoundation types on Darwin which
were "fixed" in CL 66332.
Update #22906
Update #21897
RELNOTE=yes
Change-Id: I0d4c664501d89a696c2fb037c995503caabf8911
Reviewed-on: https://go-review.googlesource.com/81876
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This error message should reference t2, not t1.
Change-Id: I2e42b8335ca9367a1fb7f76c38a1bcf8f32a2bf3
Reviewed-on: https://go-review.googlesource.com/82816
Reviewed-by: Gabriel Aszalos <gabriel.aszalos@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Gabriel Aszalos <gabriel.aszalos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This was originally done in https://golang.org/cl/31359 but partially
undone (apparently unintentionally) in https://golang.org/cl/34310
Fix it, and update tests to ensure the error is unrecoverable.
Fixes#23039
Change-Id: I923ebd613a05e67d8acce77f4a68c64c8574faa6
Reviewed-on: https://go-review.googlesource.com/82656
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
The DNS code can start goroutines and not wait for them to complete.
This does no harm, but in tests this can cause a race condition with
the test hooks that are installed and unintalled around the tests.
Add a WaitGroup that tests of DNS can use to avoid the race.
Fixes#21090
Change-Id: I6c1443a9c2378e8b89d0ab1d6390c0e3e726b0ce
Reviewed-on: https://go-review.googlesource.com/82795
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If (*Template).New replaces an existing template, reset the
existing template that is going to be replaced so that any
later attempt to execute this orphaned template will fail.
Fixes#22780
Change-Id: I0e058f42c1542c86d19dc5f6c4e1e859e670a4a2
Reviewed-on: https://go-review.googlesource.com/78542
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Pointer arithemetic is done mod 2^32 on 386, so we can just
drop the high bits of any large constant offsets.
The bounds check will make sure wraparounds are never observed.
Fixes#21655
Change-Id: I68ae5bbea9f02c73968ea2b21ca017e5ecb89223
Reviewed-on: https://go-review.googlesource.com/82675
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
I can reproduce with a very short timeout (fractions of a millisecond)
combined with -race.
But given that this is inherently sensitive to actual time, add a
testing mechanism to retry with increasingly large times to compensate
for busy buidlers. This also means the test is usually faster now,
too, since we can start with smaller durations.
Fixes#19608
Change-Id: I3a222464720195849da768e9801eb7b43baa4aeb
Reviewed-on: https://go-review.googlesource.com/82595
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The DWARF inline info generation code was using file/line/column (from
src.Pos) as a means of matching up pre- and post-optimization variable
nodes. This turns out to be problematic since it looks as though
distinct formals on the same line can be assigned the same column
number. Work around this issue by adding variable names to the
disambiguation code. Added a testpoint to the linker DWARF test that
checks to make sure each abstract origin offset of distinct within a
given DWARF DW_AT_inlined_routine body.
Fixes#23020.
Change-Id: Ie09bbe01dc60822d84d4085547b138e644036fb3
Reviewed-on: https://go-review.googlesource.com/82396
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Mark Rushakoff
Daniel Martí
Robert Griesemer
Austin Clements
Andrew Bonventre
Keith Randall
Russ Cox
Michael Hudson-Doyle
Ian Lance Taylor
Keith Randall
fanzha02
Mikio Hara
David du Colombier
Tobias Klauser
Mohit Agarwal
Rob Pike
Daniel Theophanes
Richard Miller
Kevin Burke
Hana Kim
Fazlul Shahriar
Martin Möhrmann
Elias Naur
Adam Langley
Caleb Spare
Brad Fitzpatrick
Giovanni Bajo
Kunpei Sakai
Terin Stock
Than McIntosh
Samuel Tan
Brad Burch
Paul PISCUC
Agniva De Sarker
Hiroshi Ioka
Cherry Zhang
Caio Marcelo de Oliveira Filho
Brian Kessler
Tim Cooper
Filippo Valsorda
Igor Vashyst
Jeff Johnson
David Chase
Ben Shi
elpinal
Matthijs Kooijman
Geoff Berry
Mike Samuel
Wèi Cōngruì
Nathaniel Caza
Hanjun Kim
Matthew Dempsky
David Crawshaw
Alex Brainman
Paul Boyd
Rhys Hiltner
Joe Kyo
Artyom Pervukhin