Update #8798
This is a new implementation of pprof,
written in Go instead of in Perl.
It was written primarily by Raul Silvera and
is in use for profiling programs of all languages
inside Google.
The internal structure is a bit package-heavy,
but it matches the copy used inside Google, and
since it is in an internal directory, we can make
changes to it later if we need to.
The only "new" file here is src/cmd/pprof/pprof.go,
which stitches together the Google pprof and the
Go command libraries for object file access.
I am explicitly NOT interested in style or review
comments on the rest of the files
(that is, src/cmd/pprof/internal/...).
Those are intended to stay as close to the Google
copies as possible, like we did with the pprof Perl script.
Still to do:
- Basic tests.
- Real command documentation.
- Hook up disassemblers.
LGTM=r
R=r, bradfitz, alex.brainman, dave
CC=golang-codereviews
https://golang.org/cl/153750043
For Go 1.3 these external packages were collapsed into
large single-file implementations stored in the cmd/objdump
directory.
For Go 1.4 we want pprof to be able to link against them too,
so move them into cmd/internal, where they can be shared.
The new files are copied from the repo in the file path (rsc.io/...).
Those repos were code reviewed during development
(mainly by crawshaw and minux), because we knew the
main repo would use them.
Update #8798
LGTM=bradfitz
R=crawshaw, bradfitz
CC=golang-codereviews
https://golang.org/cl/153750044
GC types were not being generated for the garbage collector
work buffer. The markfor object was being collected as a result.
This broke amd64p32 and maybe plan9 builds. Why it didn't break
every build I'm not sure...
Fixes#8812
LGTM=0intro, rsc
R=golang-codereviews, dave, khr, 0intro, rsc
CC=golang-codereviews
https://golang.org/cl/149260043
Our traceback code needs to know the PC of several special
functions, including goexit, mcall, etc. Make sure that
these PCs are initialized before any traceback occurs.
Fixes#8766
LGTM=rsc
R=golang-codereviews, rsc, khr, bradfitz
CC=golang-codereviews
https://golang.org/cl/145570043
See comment 4 of https://code.google.com/p/go/issues/detail?id=8483#c4:
"So if a user creates a http.Client, issues a bunch of
requests and then wants to shutdown it and all opened connections;
what is she intended to do? The report suggests that just waiting for
all pending requests and calling CloseIdleConnections won't do, as
there can be new racing connections. Obviously she can't do what
you've done in the test, as it uses the unexported function. If this
happens periodically, it can lead to serious resource leaks (the
transport is also preserved alive). Am I missing something?"
This CL tracks the user's intention to close all idle
connections (CloseIdleConnections sets it true; and making a
new request sets it false). If a pending dial finishes and
nobody wants it, before it's retained for a future caller, the
"wantIdle" bool is checked and it's closed if the user has
called CloseIdleConnections without a later call to make a new
request.
Fixes#8483
LGTM=adg
R=golang-codereviews, dvyukov, adg
CC=golang-codereviews, rsc
https://golang.org/cl/148970043
Fixes#5551.
Fixes#4449.
Adds tests for both issues.
Note that the two issues occur only when formatting partial Go code
with indent.
The best way to understand the change is as follows: I took the code
of cmd/gofmt and go/format, combined it into one unified code that
does not suffer from either 4449 nor 5551, and then applied that code
to both cmd/gofmt and go/format.
As a result, there is now much more identical code between the two
packages, making future code deduplication easier (it was not possible
to do that now without adding public APIs, which I was advised not to
do at this time).
More specifically, I took the parse() of cmd/gofmt which correctly
preserves comments (issue 5551) and modified it to fix issue where
it would sometimes modify literal values (issue 4449).
I ended up removing the matchSpace() function because it no longer
needed to do some of its work (insert indent), and a part of its work
had to be done in advance (determining the indentation of first code
line), because that calculation is required for cfg.Fprint() to run.
adjustIndent is used to adjust the indent of cfg.Fprint() to compensate
for the body of wrapper func being indented by one level. This allows
to get rid of the bytes.Replace text manipulation of inner content,
which was problematic and sometimes altered raw string literals (issue
4449). This means that sometimes the value of cfg.Indent is negative,
but that works as expected.
So now the algorithm for formatting partial Go code is:
1. Determine and prepend leading space of original source.
2. Determine and prepend indentation of first code line.
3. Format and write partial Go code (with all of its leading &
trailing space trimmed).
4. Determine and append trailing space of original source.
LGTM=gri
R=golang-codereviews, bradfitz, gri
CC=golang-codereviews
https://golang.org/cl/142360043
Fixes#8690.
This CL moves the save of LR around BL runtime.read_tls_fallback to liblink as it is not needed when MRC is not replaced.
LGTM=rsc, minux
R=rsc, khr, minux
CC=golang-codereviews
https://golang.org/cl/147310043
On android, root certificates appear to be stored in the folder
/system/etc/security/cacerts, which has many certs in several
different files. This change adds a new array of directories in
which certs can be found.
To test this, I simply tried making a request with the http
library to an HTTPS URL on an android emulator and manually
verified that it worked.
LGTM=crawshaw
R=golang-codereviews, gobot, crawshaw
CC=golang-codereviews
https://golang.org/cl/151800043
We're not comparing with code addresses any more. Instead,
we use nil algorithm functions to mark uncomparable types.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/151040044
CL 149110043 changed yacc to no longer keep a leading space
for quoted tokens. That is OK by itself but unfortunately
yacc was relying on that leading space to notice which tokens
it should not output as const declarations.
Add a few such tokens to expr.y, although it won't make any
immediate difference as we seem to have no tests for yacc.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/152720043
Unnecessary; covered by https://golang.org/cl/141690043
Verified by jonathan@titanous.com on golang-dev.
««« original CL description
cmd/ld: close outfile before cleanup
This prevents the temporary directory from being leaked when
the linker is run on a FUSE filesystem.
Fixes#8684.
LGTM=bradfitz
R=golang-codereviews, rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/141840043
»»»
LGTM=jonathan, iant
R=iant, jonathan
CC=golang-codereviews
https://golang.org/cl/150250045
The existing spec rules on package initialization were
contradictory: They specified that 1) dependent variables
are initialized in dependency order, and 2) independent
variables are initialized in declaration order. This 2nd
rule cannot be satisfied in general. For instance, for
var (
c = b + 2
a = 0
b = 1
)
because of its dependency on b, c must be initialized after b,
leading to the partial order b, c. Because a is independent of
b but is declared before b, we end up with the order: a, b, c.
But a is also independent of c and is declared after c, so the
order b, c, a should also be valid in contradiction to a, b, c.
The new rules are given in form of an algorithm which outlines
initialization order explicitly.
gccgo and go/types already follow these rules.
Fixes#8485.
LGTM=iant, r, rsc
R=r, rsc, iant, ken, gordon.klaus, adonovan
CC=golang-codereviews
https://golang.org/cl/142880043
This prevents the temporary directory from being leaked when
the linker is run on a FUSE filesystem.
Fixes#8684.
LGTM=bradfitz
R=golang-codereviews, rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/141840043
This fixes the bug in which the linker reports "missing Go
type information" when a -X option refers to a symbol that is
not used.
Fixes#8821.
LGTM=rsc
R=rsc, r
CC=golang-codereviews
https://golang.org/cl/151000043
The extra-clever code in Sincos is trying to do
if v&2 == 0 {
mask = 0xffffffffffffffff
} else {
mask = 0
}
It does this by turning v&2 into a float64 X0 and then using
MOVSD $0.0, X3
CMPSD X0, X3, 0
That CMPSD is defined to behave like:
if X0 == X3 {
X3 = 0xffffffffffffffff
} else {
X3 = 0
}
which gives the desired mask in X3. The goal in using the
CMPSD was to avoid a conditional branch.
This code fails when called from a PortAudio callback.
In particular, the failure behavior is exactly as if the
CMPSD always chose the 'true' execution.
Notice that the comparison X0 == X3 is comparing as
floating point values the 64-bit pattern v&2 and the actual
floating point value zero. The only possible values for v&2
are 0x0000000000000000 (floating point zero)
and 0x0000000000000002 (floating point 1e-323, a denormal).
If they are both comparing equal to zero, I conclude that
in a PortAudio callback (whatever that means), the processor
is running in "denormals are zero" mode.
I confirmed this by placing the processor into that mode
and running the test case in the bug; it produces the
incorrect output reported in the bug.
In general, if a Go program changes the floating point math
modes to something other than what Go expects, the math
library is not going to work exactly as intended, so we might
be justified in not fixing this at all.
However, it seems reasonable that the client code might
have expected "denormals are zero" mode to only affect
actual processing of denormals. This code has produced
what is in effect a gratuitous denormal by being extra clever.
There is nothing about the computation being requested
that fundamentally requires a denormal.
It is also easy to do this computation in integer math instead:
mask = ((v&2)>>1)-1
Do that.
For the record, the other math tests that fail if you put the
processor in "denormals are zero" mode are the tests for
Frexp, Ilogb, Ldexp, Logb, Log2, and FloatMinMax, but all
fail processing denormal inputs. Sincos was the only function
for which that mode causes incorrect behavior on non-denormal inputs.
The existing tests check that the new assembly is correct.
There is no test for behavior in "denormals are zero" mode,
because I don't want to add assembly to change that.
Fixes#8623.
LGTM=josharian
R=golang-codereviews, josharian
CC=golang-codereviews, iant, r
https://golang.org/cl/151750043
go test's handling of _test.go files when the entire
package's set of files has no Test functions has varied
over the past few releases. There are a few interesting
cases (all contain no Test functions):
(1) x_test.go has syntax errors
(2) x_test.go has type errors
(3) x_test.go has runtime errors (say, a func init that panics)
In Go 1.1, tests with (1) or (2) failed; (3) passed.
In Go 1.2, tests with (1) or (2) failed; (3) passed.
In Go 1.3, tests with (1) failed; (2) or (3) passed.
After this CL, tests with (1), (2), or (3) all fail.
This is clearly a corner case, but it seems to me that
the behavior of the test should not change if you
add or remove a line like
func TestAlwaysPasses(t *testing.T) {}
That implies that the _test.go files must always
be built and always be imported into the test binary.
Doing so means that (1), (2), and (3) must all fail.
Fixes#8337.
LGTM=iant
R=golang-codereviews, iant
CC=adg, golang-codereviews, r
https://golang.org/cl/150980043
From issue 7967 I learned:
1) yacc accepts either 'x' or "x" to mean token value 0x78
2) yacc also accepts 'xyz' and "XYZ" to mean token value 0x78
Use strconv.Unquote to simplify the handling of quoted
strings and check that each has only one rune.
Although this does clean things up, it makes 'x' and "x"
treated as different internally (now they are stored as
`'x'` and `"x"`; before they were both ` x`). Grammars that
use both interchangeably will now die with an error
similar to the one from issue 7967:
yacc bug -- cannot have 2 different Ts with same value
"+" and '+'
The echoing of the quotes should make clear what is going on.
The other semantic change caused by using strconv.Unquote
is that '\"' and "\'" are no longer valid. Like in Go, they must be
spelled without the backslash: '"' and "'".
On the other hand, now yacc and Go agree about what character
and string literals mean.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/149110043
The one line that you can't test easily was broken.
This manifested as a failure of a pre-existing test
in test.bash but I didn't notice it (there are a few other
long-standing failures that need to be fixed).
TBR=r
CC=golang-codereviews
https://golang.org/cl/146340044
Today, 'go build -a my/pkg' and 'go install -a my/pkg'
recompile not just my/pkg and all its dependencies that
you wrote but also the standard library packages.
Recompiling the standard library is problematic on
some systems because the installed copy is not writable.
The -a behavior means that you can't use 'go install -a all'
or 'go install -a my/...' to rebuild everything after a Go
release - the rebuild stops early when it cannot overwrite
the installed standard library.
During development work, however, you do want install -a
to rebuild everything, because anything might have changed.
Resolve the conflict by making the behavior of -a depend
on whether we are using a released copy of Go or a devel copy.
In the release copies, -a no longer applies to the standard library.
In the devel copies, it still does.
This is the latest in a long line of refinements to the
"do I build this or not" logic. It is surely not the last.
Fixes#8290.
LGTM=r
R=golang-codereviews, r, tracey.brendan
CC=adg, golang-codereviews, iant
https://golang.org/cl/151730045
This fixes the test/linkx.go test, which does not run by default.
(Issue 4139 is about fixing that.)
Fixes#8806.
LGTM=r
R=golang-codereviews, r
CC=bradfitz, golang-codereviews, iant
https://golang.org/cl/145420043
Also rebuild doc.go; was stale, so contains extra changes.
Fixes#8677.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews, iant
https://golang.org/cl/148170043
While we are here, remove undocumented, meaningless test -file flag.
Fixes#7724.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews, iant
https://golang.org/cl/149070043
Fix by atom (from CL 89190044), comment and test by me.
Fixes#6823.
LGTM=crawshaw
R=golang-codereviews, crawshaw
CC=0xe2.0x9a.0x9b, adg, golang-codereviews, iant, r
https://golang.org/cl/148180043
If you say 'go get -v' you get extra information when import
paths are not of the expected form.
If you say 'go get -v src/rsc.io/pdf' the message says that
src/rsc.io/pdf does not contain a hostname, which is incorrect.
The problem is that it does not begin with a hostname.
Fixes#7432.
LGTM=r
R=golang-codereviews, r
CC=bradfitz, golang-codereviews, iant
https://golang.org/cl/144650043
If you do 'go get -u rsc.io/pdf' and then rsc.io/pdf's redirect
changes to point somewhere else, after this CL a later
'go get -u rsc.io/pdf' will tell you that.
Fixes#8548.
LGTM=iant
R=golang-codereviews, iant
CC=adg, golang-codereviews, n13m3y3r, r
https://golang.org/cl/147170043
The pattern was only working if the checkout had
already been done, but the code was trying to make
it work even the first time. Test and fix.
Fixes#8335.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews, iant
https://golang.org/cl/146310043
