It appears that something about Go on Windows
cannot handle the fault cause by a jump to address 0.
The way Go represents and calls functions, this
never happened at all, until CL 105140044.
This CL changes the code added in CL 105140044
to make jump to 0 impossible once again.
Fixes#8047. (again, on Windows)
TBR=bradfitz
R=golang-codereviews, dave
CC=adg, golang-codereviews, iant, r
https://golang.org/cl/105120044
jmpdefer modifies PC, SP, and LR, and not atomically,
so walking past jmpdefer will often end up in a state
where the three are not a consistent execution snapshot.
This was causing warning messages a few frames later
when the traceback realized it was confused, but given
the right memory it could easily crash instead.
Update #8153
LGTM=minux, iant
R=golang-codereviews, minux, iant
CC=golang-codereviews, r
https://golang.org/cl/107970043
The test requires that timerproc runs, but busy loops and starves
the scheduler so that, with high probability, timerproc doesn't run.
Avoid the issue by expecting the test to succeed; if not, a major
outside timeout will kill it and let us know.
As you can see from the diffs, there have been several attempts to
fix this with chicanery, but none has worked. Don't bother trying
any more.
Fixes#8136.
LGTM=rsc
R=rsc, josharian
CC=golang-codereviews
https://golang.org/cl/105140043
Allow the number of benchmark iterations to grow faster for fast benchmarks, and don't round up twice.
Using the default benchtime, this CL reduces wall clock time to run benchmarks:
net/http 49s -> 37s (-24%)
runtime 8m31s -> 5m55s (-30%)
bytes 2m37s -> 1m29s (-43%)
encoding/json 29s -> 21s (-27%)
strings 1m16s -> 53s (-30%)
LGTM=crawshaw
R=golang-codereviews, crawshaw
CC=golang-codereviews
https://golang.org/cl/101970047
Call copy with as large buffer as possible to reduce the
number of function calls.
benchmark old ns/op new ns/op delta
BenchmarkBytesRepeat 540 162 -70.00%
BenchmarkStringsRepeat 563 177 -68.56%
LGTM=josharian
R=golang-codereviews, josharian, dave, dvyukov
CC=golang-codereviews
https://golang.org/cl/90550043
Previously, an input string was stripped of newline
characters at the beginning of DecodeString and then passed
to Decode. Decode again tried to strip newline characters.
That's waste of time.
benchmark old MB/s new MB/s speedup
BenchmarkDecodeString 38.37 65.20 1.70x
LGTM=dave, bradfitz
R=golang-codereviews, dave, bradfitz
CC=golang-codereviews
https://golang.org/cl/91770051
There is a hierarchy of location defined by loop depth:
-1 = the heap
0 = function results
1 = local variables (and parameters)
2 = local variable declared inside a loop
3 = local variable declared inside a loop inside a loop
etc
In general if an address from loopdepth n is assigned to
something in loop depth m < n, that indicates an extended
lifetime of some form that requires a heap allocation.
Function results can be local variables too, though, and so
they don't actually fit into the hierarchy very well.
Treat the address of a function result as level 1 so that
if it is written back into a result, the address is treated
as escaping.
Fixes#8185.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/108870044
Provide Nextafter64 as alias to Nextafter.
For submission after the 1.3 release.
Fixes#8117.
LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/101750048
The analysis for &x was using the loop depth on x set
during x's declaration. A type switch creates a list of
implicit declarations that were not getting initialized
with loop depths.
Fixes#8176.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/108860043
The putpclcdelta function set the DWARF line number PC to
s->value + pcline->pc, which is correct, but the code then set
the local variable pc to epc, which can be a different value.
This caused the next delta in the DWARF table to be wrong.
Fixes#8098.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/104950045
A runtime.Goexit during a panic-invoked deferred call
left the panic stack intact even though all the stack frames
are gone when the goroutine is torn down.
The next goroutine to reuse that struct will have a
bogus panic stack and can cause the traceback routines
to walk into garbage.
Most likely to happen during tests, because t.Fatal might
be called during a deferred func and uses runtime.Goexit.
This "not enough cleared in Goexit" failure mode has
happened to us multiple times now. Clear all the pointers
that don't make sense to keep, not just gp->panic.
Fixes#8158.
LGTM=iant, dvyukov
R=iant, dvyukov
CC=golang-codereviews
https://golang.org/cl/102220043
I am not sure what the rounding here was
trying to do, but it was skipping the first
pointer on native client.
The code above the rounding already checks
that xoffset is widthptr-aligned, so the rnd
was a no-op everywhere but on Native Client.
And on Native Client it was wrong.
Perhaps it was supposed to be rounding down,
not up, but zerorange handles the extra 32 bits
correctly, so the rnd does not seem to be necessary
at all.
This wouldn't be worth doing for Go 1.3 except
that it can affect code on the playground.
Fixes#8155.
LGTM=r, iant
R=golang-codereviews, r, iant
CC=dvyukov, golang-codereviews, khr
https://golang.org/cl/108740047
It's not clear how widespread this issue is, but we do have a
test case generated by a development version of clang.
I don't know whether this should go into 1.3 or not; happy to
hear arguments either way.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/96680045
I introduced this bug when I changed the escape
analysis to run in phases based on call graph
dependency order, in order to be more precise about
inputs escaping back to outputs (functions returning
their arguments).
Given
func f(z **int) *int { return *z }
we were tagging the function as 'z does not escape
and is not returned', which is all true, but not
enough information.
If used as:
var x int
p := &x
q := &p
leak(f(q))
then the compiler might try to keep x, p, and q all
on the stack, since (according to the recorded
information) nothing interesting ends up being
passed to leak.
In fact since f returns *q = p, &x is passed to leak
and x needs to be heap allocated.
To trigger the bug, you need a chain that the
compiler wants to keep on the stack (like x, p, q
above), and you need a function that returns an
indirect of its argument, and you need to pass the
head of the chain to that function. This doesn't
come up very often: this bug has been present since
June 2012 (between Go 1 and Go 1.1) and we haven't
seen it until now. It helps that most functions that
return indirects are getters that are simple enough
to be inlined, avoiding the bug.
Earlier versions of Go also had the benefit that if
&x really wasn't used beyond x's lifetime, nothing
broke if you put &x in a heap-allocated structure
accidentally. With the new stack copying, though,
heap-allocated structures containing &x are not
updated when the stack is copied and x moves,
leading to crashes in Go 1.3 that were not crashes
in Go 1.2 or Go 1.1.
The fix is in two parts.
First, in the analysis of a function, recognize when
a value obtained via indirect of a parameter ends up
being returned. Mark those parameters as having
content escape back to the return results (but we
don't bother to write down which result).
Second, when using the analysis to analyze, say,
f(q), mark parameters with content escaping as
having any indirections escape to the heap. (We
don't bother trying to match the content to the
return value.)
The fix could be less precise (simpler).
In the first part we might mark all content-escaping
parameters as plain escaping, and then the second
part could be dropped. Or we might assume that when
calling f(q) all the things pointed at by q escape
always (for any f and q).
The fix could also be more precise (more complex).
We might record the specific mapping from parameter
to result along with the number of indirects from the
parameter to the thing being returned as the result,
and then at the call sites we could set up exactly the
right graph for the called function. That would make
notleaks(f(q)) be able to keep x on the stack, because
the reuslt of f(q) isn't passed to anything that leaks it.
The less precise the fix, the more stack allocations
become heap allocations.
This fix is exactly as precise as it needs to be so that
none of the current stack allocations in the standard
library turn into heap allocations.
Fixes#8120.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, khr, r
https://golang.org/cl/102040046
The 'address taken' bit in a function variable was not
propagating into the inlined copies, causing incorrect
liveness information.
LGTM=dsymonds, bradfitz
R=golang-codereviews, bradfitz
CC=dsymonds, golang-codereviews, iant, khr, r
https://golang.org/cl/96670046
The 1-byte write was silently clearing a byte on the stack.
If there was another function call with more arguments
in the same stack frame, no harm done.
Otherwise, if the variable at that location was already zero,
no harm done.
Otherwise, problems.
Fixes#8139.
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=golang-codereviews, iant, r
https://golang.org/cl/100940043
This is a workaround - the code should be better than this - but the
fix avoids generating large numbers of linehist entries for the wrapper
functions that enable interface conversions. There can be many of
them, they all happen at the end of compilation, and they can all
share a linehist entry.
Avoids bad n^2 behavior in liblink.
Test case in issue 8135 goes from 64 seconds to 2.5 seconds (still bad
but not intolerable).
Fixes#8135.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/104840043
If we see a typedef to an anonymous struct more than once,
presumably in two different Go files that import "C", use the
same Go type name.
Fixes#8133.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/102080043
Right now, any revision on the default branch after go1.3beta2 is
described by "go verson" as go1.3beta2 plus some revision.
That's OK for now, but once go1.3 is released, that will seem wrong.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/98650046
We were requiring that the defer stack and the panic stack
be completely processed, thinking that if any were left over
the stack scan and the defer stack/panic stack must be out
of sync. It turns out that the panic stack may well have
leftover entries in some situations, and that's okay.
Fixes#8132.
LGTM=minux, r
R=golang-codereviews, minux, r
CC=golang-codereviews, iant, khr
https://golang.org/cl/100900044
C globals are conservatively scanned. This helps
avoid false retention, especially for 32 bit.
LGTM=rsc
R=golang-codereviews, khr, rsc
CC=golang-codereviews
https://golang.org/cl/102040043
The 'continuation pc' is where the frame will continue
execution, if anywhere. For a frame that stopped execution
due to a CALL instruction, the continuation pc is immediately
after the CALL. But for a frame that stopped execution due to
a fault, the continuation pc is the pc after the most recent CALL
to deferproc in that frame, or else 0. That is where execution
will continue, if anywhere.
The liveness information is only recorded for CALL instructions.
This change makes sure that we never look for liveness information
except for CALL instructions.
Using a valid PC fixes crashes when a garbage collection or
stack copying tries to process a stack frame that has faulted.
Record continuation pc in heapdump (format change).
Fixes#8048.
LGTM=iant, khr
R=khr, iant, dvyukov
CC=golang-codereviews, r
https://golang.org/cl/100870044
Update #2675
The code here was using the error check for Linux/386,
not the one for FreeBSD/386. Most of the time it worked.
Thanks to Neel Natu (FreeBSD developer) for finding this.
The s/JCC/JAE/ a few lines later is a no-op but makes the
test match the rest of the file. Why we write JAE instead of JCC
I don't know, but the two are equivalent and the file might
as well be consistent.
LGTM=bradfitz, minux
R=golang-codereviews, bradfitz, minux
CC=golang-codereviews
https://golang.org/cl/99680044
This CL forces the optimizer to preserve some memory stores
that would be redundant except that a stack scan due to garbage
collection or stack copying might look at them during a function call.
As such, it forces additional memory writes and therefore slows
down the execution of some programs, especially garbage-heavy
programs that are already limited by memory bandwidth.
The slowdown can be as much as 7% for end-to-end benchmarks.
These numbers are from running go1.test -test.benchtime=5s three times,
taking the best (lowest) ns/op for each benchmark. I am excluding
benchmarks with time/op < 10us to focus on macro effects.
All benchmarks are on amd64.
Comparing tip (a27f34c771cb) against this CL on an Intel Core i5 MacBook Pro:
benchmark old ns/op new ns/op delta
BenchmarkBinaryTree17 3876500413 3856337341 -0.52%
BenchmarkFannkuch11 2965104777 2991182127 +0.88%
BenchmarkGobDecode 8563026 8788340 +2.63%
BenchmarkGobEncode 5050608 5267394 +4.29%
BenchmarkGzip 431191816 434168065 +0.69%
BenchmarkGunzip 107873523 110563792 +2.49%
BenchmarkHTTPClientServer 85036 86131 +1.29%
BenchmarkJSONEncode 22143764 22501647 +1.62%
BenchmarkJSONDecode 79646916 85658808 +7.55%
BenchmarkMandelbrot200 4720421 4700108 -0.43%
BenchmarkGoParse 4651575 4712247 +1.30%
BenchmarkRegexpMatchMedium_1K 71986 73490 +2.09%
BenchmarkRegexpMatchHard_1K 111018 117495 +5.83%
BenchmarkRevcomp 648798723 659352759 +1.63%
BenchmarkTemplate 112673009 112819078 +0.13%
Comparing tip (a27f34c771cb) against this CL on an Intel Xeon E5520:
BenchmarkBinaryTree17 5461110720 5393104469 -1.25%
BenchmarkFannkuch11 4314677151 4327177615 +0.29%
BenchmarkGobDecode 11065853 11235272 +1.53%
BenchmarkGobEncode 6500065 6959837 +7.07%
BenchmarkGzip 647478596 671769097 +3.75%
BenchmarkGunzip 139348579 141096376 +1.25%
BenchmarkHTTPClientServer 69376 73610 +6.10%
BenchmarkJSONEncode 30172320 31796106 +5.38%
BenchmarkJSONDecode 113704905 114239137 +0.47%
BenchmarkMandelbrot200 6032730 6003077 -0.49%
BenchmarkGoParse 6775251 6405995 -5.45%
BenchmarkRegexpMatchMedium_1K 111832 113895 +1.84%
BenchmarkRegexpMatchHard_1K 161112 168420 +4.54%
BenchmarkRevcomp 876363406 892319935 +1.82%
BenchmarkTemplate 146273096 148998339 +1.86%
Just to get a sense of where we are compared to the previous release,
here are the same benchmarks comparing Go 1.2 to this CL.
Comparing Go 1.2 against this CL on an Intel Core i5 MacBook Pro:
BenchmarkBinaryTree17 4370077662 3856337341 -11.76%
BenchmarkFannkuch11 3347052657 2991182127 -10.63%
BenchmarkGobDecode 8791384 8788340 -0.03%
BenchmarkGobEncode 4968759 5267394 +6.01%
BenchmarkGzip 437815669 434168065 -0.83%
BenchmarkGunzip 94604099 110563792 +16.87%
BenchmarkHTTPClientServer 87798 86131 -1.90%
BenchmarkJSONEncode 22818243 22501647 -1.39%
BenchmarkJSONDecode 97182444 85658808 -11.86%
BenchmarkMandelbrot200 4733516 4700108 -0.71%
BenchmarkGoParse 5054384 4712247 -6.77%
BenchmarkRegexpMatchMedium_1K 67612 73490 +8.69%
BenchmarkRegexpMatchHard_1K 107321 117495 +9.48%
BenchmarkRevcomp 733270055 659352759 -10.08%
BenchmarkTemplate 109304977 112819078 +3.21%
Comparing Go 1.2 against this CL on an Intel Xeon E5520:
BenchmarkBinaryTree17 5986953594 5393104469 -9.92%
BenchmarkFannkuch11 4861139174 4327177615 -10.98%
BenchmarkGobDecode 11830997 11235272 -5.04%
BenchmarkGobEncode 6608722 6959837 +5.31%
BenchmarkGzip 661875826 671769097 +1.49%
BenchmarkGunzip 138630019 141096376 +1.78%
BenchmarkHTTPClientServer 71534 73610 +2.90%
BenchmarkJSONEncode 30393609 31796106 +4.61%
BenchmarkJSONDecode 139645860 114239137 -18.19%
BenchmarkMandelbrot200 5988660 6003077 +0.24%
BenchmarkGoParse 6974092 6405995 -8.15%
BenchmarkRegexpMatchMedium_1K 111331 113895 +2.30%
BenchmarkRegexpMatchHard_1K 165961 168420 +1.48%
BenchmarkRevcomp 995049292 892319935 -10.32%
BenchmarkTemplate 145623363 148998339 +2.32%
Fixes#8036.
LGTM=khr
R=golang-codereviews, josharian, khr
CC=golang-codereviews, iant, r
https://golang.org/cl/99660044
The rtype struct is meant to be a copy of reflect.rtype. The
zero field was added to reflect.rtype in 18495:6e50725ac753.
LGTM=rsc
R=khr, rsc
CC=golang-codereviews
https://golang.org/cl/93660045
[Same as CL 102820043 except applied changes to 6g/gsubr.c
also to 5g/gsubr.c and 8g/gsubr.c. The problem I had last night
trying to do that was that 8g's copy of nodarg has different
(but equivalent) control flow and I was pasting the new code
into the wrong place.]
Description from CL 102820043:
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.
Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.
The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.
Fixes#8097.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/103750043
Breaks 386 and arm builds.
The obvious reason is that this CL only edited 6g/gsubr.c
and failed to edit 5g/gsubr.c and 8g/gsubr.c.
However, the obvious CL applying the same edit to those
files (CL 101900043) causes mysterious build failures
in various of the standard package tests, usually involving
reflect. Something deep and subtle is broken but only on
the 32-bit systems.
Undo this CL for now.
««« original CL description
cmd/gc: fix x=x crash
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.
Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.
The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.
Fixes#8097.
LGTM=r, khr
R=golang-codereviews, r, khr
CC=golang-codereviews, iant
https://golang.org/cl/102820043
»»»
TBR=r
CC=golang-codereviews, khr
https://golang.org/cl/95660043
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.
Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.
The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.
Fixes#8097.
LGTM=r, khr
R=golang-codereviews, r, khr
CC=golang-codereviews, iant
https://golang.org/cl/102820043
Update #8112
Hide one-pass regexp API.
This means moving the code from regexp/syntax to regexp,
but it avoids being locked into the specific API chosen for
the implementation.
It also removes a slice field from the syntax.Inst, which
should avoid bloating the memory footprint of a non-one-pass
regexp unnecessarily.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews, iant
https://golang.org/cl/98610046
For incomplete struct S, C.T and C.struct_S were interchangeable in Go 1.2
and earlier, because all incomplete types were interchangeable
(even C.struct_S1 and C.struct_S2).
CL 76450043, which fixed issue 7409, made different incomplete types
different from Go's point of view, so that they were no longer completely
interchangeable.
However, imprecision about C.T and C.struct_S - really the same
underlying C type - is the one behavior enabled by the bug that
is most likely to be depended on by existing cgo code.
Explicitly allow it, to keep that code working.
Fixes#7786.
LGTM=iant, r
R=golang-codereviews, iant, r
CC=golang-codereviews
https://golang.org/cl/98580046
Map iteration order issue. Go 1.2 and earlier had stable results
for small maps.
Fixes#8115
LGTM=r, rsc
R=golang-codereviews, r
CC=dsymonds, golang-codereviews, iant, rsc
https://golang.org/cl/98580047
CL 51010045 fixed the first one of these:
cmd/gc: return canonical Node* from temp
For historical reasons, temp was returning a copy
of the created Node*, not the original Node*.
This meant that if analysis recorded information in the
returned node (for example, n->addrtaken = 1), the
analysis would not show up on the original Node*, the
one kept in fn->dcl and consulted during liveness
bitmap creation.
Correct this, and watch for it when setting addrtaken.
Fixes#7083.
R=khr, dave, minux.ma
CC=golang-codereviews
https://golang.org/cl/51010045
CL 53200043 fixed the second:
cmd/gc: fix race build
Missed this case in CL 51010045.
TBR=khr
CC=golang-codereviews
https://golang.org/cl/53200043
This CL fixes the third. There are only three nod(OXXX, ...)
calls in sinit.c, so maybe we're done. Embarassing that it
took three CLs to find all three.
Fixes#8028.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant
https://golang.org/cl/100800046
In the first very rough draft of the reordering code
that was introduced in the Go 1.3 cycle, the pre-allocated
temporary for a ... argument was held in n->right.
It moved to n->alloc but the code avoiding n->right
was left behind in order.c. In copy(x, <-c), the receive
is in n->right and must be processed. Delete the special
case code, removing the bug.
Fixes#8039.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100820044
The code was assuming that pointer alignment is the
maximum alignment, but on NaCl uint64 alignment is
even more strict.
Brad checked in the test earlier today; this fixes the build.
Fixes#7863.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/98630046
The code cannot have worked before, because it was
trying to use the old value in a range check for the new
type, which might have a different representation
(hence the 'internal compiler error').
Fixes#8073.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/98630045
Common mistake (at least for me) because hg etc. require the prefix
while the go command forbids it.
Before:
% go get http://code.google.com/p/go.text/unicode/norm
package http:/code.google.com/p/go.text/unicode/norm: unrecognized import path "http:/code.google.com/p/go.text/unicode/norm"
After:
% go get http://code.google.com/p/go.text/unicode/norm
package http:/code.google.com/p/go.text/unicode/norm: "http://" not allowed in import path
LGTM=ruiu, rsc
R=rsc, ruiu
CC=golang-codereviews
https://golang.org/cl/97630046
This is a quick documentation change/clarification, as this
confused me before: in my own cgo-based projects, I currently have
identical #cgo directives in each relevant source file, and I notice
with go build -x that cgo is combining the directives, leading to
pkg-config invocations with the same package name (gtk+-3.0, in my
case) repeated several times, or on Mac OS X, LDFLAGS listing
-framework Foundation -framework AppKit multiple times. Since I am
about to add a CFLAGS as well, I checked the source to cmd/cgo and
go/build (where the work is actually done) to see if that still holds
true there. Hopefully other people who have made the same mistake I
have (I don't know if anyone has) can remove the excess declarations
now; this should make things slightly easier to manage as well.
LGTM=iant
R=golang-codereviews, gobot, iant
CC=golang-codereviews
https://golang.org/cl/91520046
Ignore symbols that aren't text, data, or bss since they cause
problems when dissassembling instructions with small immediate
values.
Before:
build.go:142 0x10ee 83ec50 SUBL $text/template/parse.autotmp_1293(SB), SP
After:
build.go:142 0x10ee 83ec50 SUBL $0x50, SP
Fixes#7947.
LGTM=rsc
R=rsc, 0intro
CC=golang-codereviews
https://golang.org/cl/93520045
Passes the expanded test in CL 100660044,
which gives me some confidence that it
might be right.
(The old code failed by not considering all the
low bits.)
LGTM=r
R=golang-codereviews, r, bradfitz
CC=golang-codereviews, iant, khr
https://golang.org/cl/99410051
Rewrite formatFloat to be much simpler and clearer and
avoid the tricky interaction with padding.
The issue refers to complex but the problem is just floating-point.
The new tests added were incorrectly formatted before this fix.
Fixes#8064.
LGTM=jscrockett01, rsc
R=rsc, jscrockett01
CC=golang-codereviews
https://golang.org/cl/99420048
This idea was rejected in CL 5731059. We should fix the
runtime docs instead.
««« original CL description
cmd/dist: reflect local changes to tree in goversion
runtime.Version() requires a trailing "+" when
tree had local modifications at time of build.
Fixes#7701
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/84040045
»»»
LGTM=rsc, mra
R=iant, rsc, mra
CC=golang-codereviews
https://golang.org/cl/100520043
Explain which files the go command looks at, and what they represent.
Fixes#6348.
LGTM=rsc
R=rsc, minux.ma
CC=golang-codereviews
https://golang.org/cl/96480043
Add nacl.bash, the NaCl version of all.bash.
It's a separate script because it builds a variant of package syscall
with a large zip file embedded in it, containing all the input files
needed for tests.
Disable various tests new since the last round, mostly the ones using os/exec.
Fixes#7945.
LGTM=dave
R=golang-codereviews, remyoudompheng, dave, bradfitz
CC=golang-codereviews
https://golang.org/cl/100590044
The USEFIELD instructions no longer make it to the linker,
so we have to do something else to pin the references
they were pinning. Emit a 0-length relocation of type R_USEFIELD.
Fixes#7486.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/95530043
The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot spots.
The move back to 4kB was predicated on copying stacks eliminating
the potential for hot spots.
Unfortunately, the fact that stacks do not copy 100% of the time means
that hot spots can still happen under the right conditions, and the slowdown
is worse now than it was in Go 1.2. There is a real program in issue 8030 that
sees about a 30x slowdown: it has a reflect call near the top of the stack
which inhibits any stack copying on that segment.
Go back to 8kB until stack copying can be used 100% of the time.
Fixes#8030.
LGTM=khr, dave, iant
R=iant, khr, r, bradfitz, dave
CC=golang-codereviews
https://golang.org/cl/92540043
The float32 const conversion used to round to float64
and then use the hardware to round to float32.
Even though there was a range check before this
conversion, the double rounding introduced inaccuracy:
the round to float64 might round the value further away
from the float32 range, reaching a float64 value that
could not actually be rounded to float32. The hardware
appears to give us 0 in that case, but it is probably undefined.
Double rounding also meant that the wrong value might
be used for certain border cases.
Do the rounding the float32 ourselves, just as we already
did the rounding to float64. This makes the conversion
precise and also makes the conversion match the range check.
Finally, add some code to print very large (bigger than float64)
floating point constants in decimal floating point notation instead
of falling back to the precise but human-unreadable binary floating
point notation.
Fixes#8015.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/100580044
Update #7980
This CL make the linker abort for the example program. For Go 1.4,
we need to find a general way to handle large memory model programs.
LGTM=dave, josharian, iant
R=iant, dave, josharian
CC=golang-codereviews
https://golang.org/cl/91500046
The temporary-introducing pass was not recursing
into the argumnt of a receive operation.
Fixes#8011.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews, iant, khr
https://golang.org/cl/91540043
This is the main point of confusion and the emphasis of
a recent Gophercon talk.
Fixes#5886. (mostly fixed in previous commits)
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/100560043
CL 22730043 fixed a bug in these functions: they could
return 1.0 despite documentation saying otherwise.
But the fix changed the values returned in the non-buggy case too,
which might invalidate programs depending on a particular
stream when using rand.Seed(0) or when passing their own
Source to rand.New.
The example test says:
// These tests serve as an example but also make sure we don't change
// the output of the random number generator when given a fixed seed.
so I think there is some justification for thinking we have
promised not to change the values. In any case, there's no point in
changing the values gratuitously: we can easily fix this bug without
changing the values, and so we should.
That CL just changed the test values too, which defeats the
stated purpose, but it was just a comment.
Add an explicit regression test, which might be
a clearer signal next time that we don't want to change
the values.
Fixes#6721. (again)
Fixes#8013.
LGTM=r
R=iant, r
CC=golang-codereviews
https://golang.org/cl/95460049
Currently freeOSMemory makes only marking phase of GC, but not sweeping phase.
So recently memory is not released after freeOSMemory.
Do both marking and sweeping during freeOSMemory.
Fixes#8019.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, rsc
https://golang.org/cl/97550043
Rename Seek to seek in asm file, was overlooked in CL 99320043.
LGTM=bradfitz, r
R=r, rsc, bradfitz
CC=golang-codereviews
https://golang.org/cl/99320044
These functions claimed to return error (an interface)
and be implemented entirely in assembly, but it's not
possible to create an interface from assembly
(at least not easily).
In reality the functions were written to return an errno uintptr
despite the Go prototype saying error.
When the errno was 0, they coincidentally filled out a nil error
by writing the 0 to the type word of the interface.
If the errno was ever non-zero, the functions would
create a non-nil error that would crash when trying to
call err.Error().
Luckily these functions (Seek, Time, Gettimeofday) pretty
much never fail, so it was all kind of working.
Found by go vet.
LGTM=bradfitz, r
R=golang-codereviews, bradfitz, r
CC=golang-codereviews
https://golang.org/cl/99320043
TestLargeDefs was issuing over one million small writes to
create a 7MB file (large.go). This is quite slow on Plan 9
since our disk file systems aren't very fast and they're
usually accessed over the network.
Buffering the writes makes the test about six times faster.
Even on Linux, it's about 1.5 times faster.
Here are the results on a slow Plan 9 machine:
Before:
% ./pack.test -test.v -test.run TestLargeDefs
=== RUN TestLargeDefs
--- PASS: TestLargeDefs (125.11 seconds)
PASS
After:
% ./pack.test -test.v -test.run TestLargeDefs
=== RUN TestLargeDefs
--- PASS: TestLargeDefs (20.835 seconds)
PASS
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/95040044
The introduction of temporaries in order.c was not
quite right for two corner cases:
1) The rewrite that pushed new variables on the lhs of
a receive into the body of the case was dropping the
declaration of the variables. If the variables escape,
the declaration is what allocates them.
Caught by escape analysis sanity check.
In fact the declarations should move into the body
always, so that we only allocate if the corresponding
case is selected. Do that. (This is an optimization that
was already present in Go 1.2. The new order code just
made it stop working.)
Fixes#7997.
2) The optimization to turn a single-recv select into
an ordinary receive assumed it could take the address
of the destination; not so if the destination is _.
Fixes#7998.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100480043
Calling tar.Reader.Read() used to work fine, but without this patch it panics.
Simply return EOF to indicate the tar.Reader.Next() needs to be called.
LGTM=iant, bradfitz
R=golang-codereviews, bradfitz, iant, mikioh.mikioh, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/94530043
This CL restores dropped constants not supported in OpenBSD 5.5
and tris to keep the promise of API compatibility.
Update #7049
LGTM=jsing, bradfitz, rsc
R=rsc, jsing, robert.hencke, minux.ma, bradfitz, iant
CC=golang-codereviews
https://golang.org/cl/96970043
- use Init to establish heap invariant on
a non-empty heap
- use Fix to update heap after an element's
properties have been changed
(The old code used Init where it wasn't needed,
and didn't use Fix because Fix was added after
the example was written.)
LGTM=bradfitz
R=adonovan, bradfitz
CC=golang-codereviews
https://golang.org/cl/94520043
for GOOS in darwin freebsd linux nacl netbsd openbsd plan9 solaris windows
do
for GOARCH in 386 amd64 amd64p32 arm
do
go vet
done
done
These are all real mistakes being corrected, but none
of them should be able to cause problems today
due to the NOSPLIT on the functions.
However, vet has also identified a few important problems.
I'm sending this CL to get rid of the trivial 'go vet' results
before attacking the real ones.
LGTM=r
R=golang-codereviews, r, bradfitz
CC=golang-codereviews
https://golang.org/cl/95460046
None of these are real bugs.
The variable name in the reference is not semantically meaningful,
except that 'go vet' will double check the offset against the name for you.
The stack sizes being corrected really are incorrect but they are also
in NOSPLIT functions so they typically don't matter.
Found by vet.
GOOS=linux GOARCH=amd64 go vet sync/atomic
GOOS=linux GOARCH=amd64p32 go vet sync/atomic
GOOS=linux GOARCH=386 go vet sync/atomic
GOOS=linux GOARCH=arm go vet sync/atomic
GOOS=freebsd GOARCH=arm go vet sync/atomic
GOOS=netbsd GOARCH=arm go vet sync/atomic
LGTM=r
R=r, bradfitz
CC=golang-codereviews
https://golang.org/cl/100500043
The GC program describing a data structure sometimes trusts the
pointer base type and other times does not (if not, the garbage collector
must fall back on per-allocation type information stored in the heap).
Make the scanning of a pointer in an interface do the same.
This fixes a crash in a particular use of reflect.SliceHeader.
Fixes#8004.
LGTM=khr
R=golang-codereviews, khr
CC=0xe2.0x9a.0x9b, golang-codereviews, iant, r
https://golang.org/cl/100470045
Globals, function arguments, and results are special cases in
registerization.
Globals must be flushed aggressively, because nearly any
operation can cause a panic, and the recovery code must see
the latest values. Globals also must be loaded aggressively,
because nearly any store through a pointer might be updating a
global: the compiler cannot see all the "address of"
operations on globals, especially exported globals. To
accomplish this, mark all globals as having their address
taken, which effectively disables registerization.
If a function contains a defer statement, the function results
must be flushed aggressively, because nearly any operation can
cause a panic, and the deferred code may call recover, causing
the original function to return the current values of its
function results. To accomplish this, mark all function
results as having their address taken if the function contains
any defer statements. This causes not just aggressive flushing
but also aggressive loading. The aggressive loading is
overkill but the best we can do in the current code.
Function arguments must be considered live at all safe points
in a function, because garbage collection always preserves
them: they must be up-to-date in order to be preserved
correctly. Accomplish this by marking them live at all call
sites. An earlier attempt at this marked function arguments as
having their address taken, which disabled registerization
completely, making programs slower. This CL's solution allows
registerization while preserving safety. The benchmark speedup
is caused by being able to registerize again (the earlier CL
lost the same amount).
benchmark old ns/op new ns/op delta
BenchmarkEqualPort32 61.4 56.0 -8.79%
benchmark old MB/s new MB/s speedup
BenchmarkEqualPort32 521.56 570.97 1.09x
Fixes#1304. (again)
Fixes#7944. (again)
Fixes#7984.
Fixes#7995.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, r
https://golang.org/cl/97500044
The function takes 32 bytes of arguments: 8 for the *block
and then 3*8 for the slice.
The 24 is not causing a bug (today at least) because the
final word is the cap of the slice, which the assembly
does not use.
Identified by 'go vet std'.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/96360043
Turns out elf.File.Sections is indexed by the actual
section number, not the number minus one.
I don't know why I thought the -1 was necessary.
Fixes objdump test (and therefore build) on ELF systems.
While we're here, fix bounds on gnuDump so that we
don't crash when asked to disassemble outside
the text segment. May fix Windows build or at least
make the failure more interesting.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/92390043
There is some duplication here with cmd/nm.
There is a TODO to address that after 1.3 is out.
Update #7452
x86 disassembly works and is tested.
The arm disassembler does not exist yet
and is therefore not yet hooked up.
LGTM=crawshaw, iant
R=crawshaw, iant
CC=golang-codereviews
https://golang.org/cl/91360046
Do not use ustar format if we need the GNU one.
Change \000 to \x00 for consistency
Check for "ustar\x00" instead of "ustar\x00\x00" for conistency with tar
and compatiblity with archive generated with older code (which was ustar\x00\x20\x00)
Add test for long name + big file.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/99050043
AddressSanitizer says:
AddressSanitizer: heap-buffer-overflow on address 0x60200001b6f3
READ of size 6 at 0x60200001b6f3 thread T0
#0 0x46741b in __interceptor_memcmp asan_interceptors.cc:337
#1 0x4b5794 in compile src/cmd/6g/../gc/pgen.c:177
#2 0x509b81 in funccompile src/cmd/gc/dcl.c:1457
#3 0x520fe2 in p9main src/cmd/gc/lex.c:489
#4 0x5e2e01 in main src/lib9/main.c:57
#5 0x7fab81f7976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
#6 0x4b16dc in _start (pkg/tool/linux_amd64/6g+0x4b16dc)
0x60200001b6f3 is located 0 bytes to the right of 3-byte region [0x60200001b6f0,0x60200001b6f3)
allocated by thread T0 here:
#0 0x493ec8 in __interceptor_malloc asan_malloc_linux.cc:75
#1 0x54d64e in mal src/cmd/gc/subr.c:459
#2 0x5260d5 in yylex src/cmd/gc/lex.c:1605
#3 0x52078f in p9main src/cmd/gc/lex.c:402
#4 0x5e2e01 in main src/lib9/main.c:57
If the memory block happens to be at the end of hunk and page bounadry,
this out-of-bounds can lead to a crash.
LGTM=dave, iant
R=golang-codereviews, dave, iant
CC=golang-codereviews
https://golang.org/cl/93370043
The code recurs very deeply in cases like (?:x{1,1000}){1,1000}
Since if much time is spent checking whether one pass is possible, it's not
worth doing at all, a simple fix is proposed: Stop if the check takes too long.
To do this, we simply avoid machines with >1000 instructions.
Benchmarks show a percent or less change either way, effectively zero.
Fixes#7608.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/92290043
If a map variable is created with reflect.New it has incorrect type (map[unsafe.Pointer]unsafe.Pointer).
If GC follows such pointer, it scans Hmap and buckets with incorrect type.
This can lead to overscan of up to 120 bytes for map[int8]struct{}.
Which in turn can lead to crash if the memory after a bucket object is unaddressable
or false retention (buckets are scanned as arrays of unsafe.Pointer).
I don't see how it can lead to heap corruptions, though.
LGTM=khr
R=rsc, khr
CC=golang-codereviews
https://golang.org/cl/96270044
mstats.last_gc is unix time now, it is compared with abstract monotonic time.
On my machine GC is forced every 5 mins regardless of last_gc.
LGTM=rsc
R=golang-codereviews
CC=golang-codereviews, iant, rsc
https://golang.org/cl/91350045
I have no test case for this at tip.
The original report included a program crashing at revision 88ac7297d2fa.
I tested this code at that revision and it does fix the crash.
However, at tip the reported code no longer crashes, presumably
because some allocation patterns have changed. I believe the
bug is still present at tip and that this code still fixes it.
Fixes#7143.
LGTM=alex.brainman
R=golang-codereviews, alex.brainman
CC=dvyukov, golang-codereviews
https://golang.org/cl/96300046
Originally it was an error, which made perfect sense, but in issue 2540
I got talked out of this sensible behavior. I'm not thrilled with the "new"
behavior but it's been there since Go 1.1 so we're stuck with it now.
Fixes#6724.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/100430043
This changes allows the first token encoded to be a xml declaration. A ProcInst with target of xml. Any other ProcInst after that with a target of xml will fail
Fixes#7380.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/72410043
The inputs to a function are marked live at all times in the
liveness bitmaps, so that the garbage collector will not free
the things they point at and reuse the pointers, so that the
pointers shown in stack traces are guaranteed not to have
been recycled.
Unfortunately, no one told the register optimizer that the
inputs need to be preserved at all call sites. If a function
is done with a particular input value, the optimizer will stop
preserving it across calls. For single-word values this just
means that the value recorded might be stale. For multi-word
values like slices, the value recorded could be only partially stale:
it can happen that, say, the cap was updated but not the len,
or that the len was updated but not the base pointer.
Either of these possibilities (and others) would make the
garbage collector misinterpret memory, leading to memory
corruption.
This came up in a real program, in which the garbage collector's
'slice len ≤ slice cap' check caught the inconsistency.
Fixes#7944.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, khr
https://golang.org/cl/100370045
This is joint work with Daniel Morsing.
In order for the register allocator to alias two variables, they must have the same width, stack offset, and etype. Code generation was altering a variable's etype in a few places. This prevented the variable from being moved to a register, which in turn prevented peephole optimization. This failure to alias was very common, with almost 23,000 instances just running make.bash.
This phenomenon was not visible in the register allocation debug output because the variables that failed to alias had the same name. The debugging-only change to bits.c fixes this by printing the variable number with its name.
This CL fixes the source of all etype mismatches for 6g, all but one case for 8g, and depressingly few cases for 5g. (I believe that extending CL 6819083 to 5g is a prerequisite.) Fixing the remaining cases in 8g and 5g is work for the future.
The etype mismatch fixes are:
* [gc] Slicing changed the type of the base pointer into a uintptr in order to perform arithmetic on it. Instead, support addition directly on pointers.
* [*g] OSPTR was giving type uintptr to slice base pointers; undo that. This arose, for example, while compiling copy(dst, src).
* [8g] 64 bit float conversion was assigning int64 type during codegen, overwriting the existing uint64 type.
Note that some etype mismatches are appropriate, such as a struct with a single field or an array with a single element.
With these fixes, the number of registerizations that occur while running make.bash for 6g increases ~10%. Hello world binary size shrinks ~1.5%. Running all benchmarks in the standard library show performance improvements ranging from nominal to substantive (>10%); a full comparison using 6g on my laptop is available at https://gist.github.com/josharian/8f9b5beb46667c272064. The microbenchmarks must be taken with a grain of salt; see issue 7920. The few benchmarks that show real regressions are likely due to issue 7920. I manually examined the generated code for the top few regressions and none had any assembly output changes. The few benchmarks that show extraordinary improvements are likely also due to issue 7920.
Performance results from 8g appear similar to 6g.
5g shows no performance improvements. This is not surprising, given the discussion above.
Update #7316
LGTM=rsc
R=rsc, daniel.morsing, bradfitz
CC=dave, golang-codereviews
https://golang.org/cl/91850043
The runtime was detecting the cycle already,
but we can give a better error without even
building the binary.
Fixes#7789.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/96290043
This change requires using SWIG version 3.0 or later. Earlier
versions of SWIG do not generate the pragmas required to use
the external linker.
Fixes#7155.
Fixes#7156.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/97120046
Before we used line 1 of the first source file.
This should be clearer.
Fixes#4388.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/92250044
<enter reason for undo>
««« original CL description
net: make use of SO_LINGER_SEC on darwin
Fixes#7971.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/92210044
»»»
TBR=iant
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/96220049
If it's not used (such as on other systems or if softfloat
is disabled) the linker will discard it.
The alternative is to teach cmd/go that every binary
depends on math implicitly on arm. I started down that
path but it's too scary. If we're going to get dependencies
right we should get dependencies right.
Fixes#6994.
LGTM=bradfitz, dave
R=golang-codereviews, bradfitz, dave
CC=golang-codereviews
https://golang.org/cl/95290043
This is a trivial change to make use of an existing `nl` byte slice
containing a single '\n' character. It's already declared and
used in another place in this file, so it might as well be used
in the other location instead of
a new slice literal. There should be no change in behavior,
aside from potentially less allocations.
This is my first CL, so I wanted to use a simple, hopefully non-controversial,
minor improvement to get more comfortable with golang contribution process.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/97280043
<enter reason for undo>
««« original CL description
runtime/race: fix the link for the race detector.
LGTM=bradfitz
R=golang-dev, bradfitz
CC=golang-codereviews
https://golang.org/cl/100330043
»»»
TBR=minux
R=minux.ma
CC=golang-codereviews
https://golang.org/cl/96200044
list has been adding them one at a time haphazardly
(race and tags were there and documented; compiler
was there and undocumented).
clean -i needs -compiler in order to clean the
installed targets for alternate compilers.
Fixes#7302.
While we're here, tweak the language in the 'go get' docs
about build flags.
Fixes#7807.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/99130043
If you write:
var x = 3
then the compiler arranges for x to be initialized in the linker
with an actual 3 from the data segment, rather than putting
x in the bss and emitting init-time "x = 3" assignment code.
If you write:
var y = x
var x = 3
then the compiler is clever and treats this the same as if
the code said 'y = 3': they both end up in the data segment
with no init-time assignments.
If you write
var y = x
var x int
then the compiler was treating this the same as if the
code said 'x = 0', making both x and y zero and avoiding
any init-time assignment.
This copying optimization to avoid init-time assignment of y
is incorrect if 'var x int' doesn't mean 'x = 0' but instead means
'x is initialized in C or assembly code'. The program ends up
with 'y = 0' instead of 'y = the value specified for x in that other code'.
Disable the propagation if there is no initializer for x.
This comes up in some uses of cgo, because cgo generates
Go globals that are initialized in accompanying C files.
Fixes#7665.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/93200044
If the ... element type contained no pointers,
then the escape analysis did not track the ... itself.
This manifested in an escaping ...byte being treated
as non-escaping.
Fixes#7934.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100310043
The register allocator decides which variables should be placed into registers by charging for each load/store and crediting for each use, and then selecting an allocation with minimal cost. NOPs will be eliminated, however, so using a variable in a NOP should not generate credit.
Issue 7867 arises from attempted registerization of multi-word variables because they are used in NOPs. By not crediting for that use, they will no longer be considered for registerization.
This fix could theoretically lead to better register allocation, but NOPs are rare relative to other instructions.
Fixes#7867.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/94810044
TestDialFailPDLeak was created for testing runtime-integrated netwrok
poller stuff and used during Go 1.2 development cycle. Unfortunately
it's still flakey because it depends on MemStats of runtime, not
pollcache directly, and MemStats accounts and revises its own stats
occasionally.
For now the codepaths related to runtime-intergrated network poller
are pretty stable, so removing this test case never suffers us.
Fixes#6553.
LGTM=josharian, iant
R=iant, josharian
CC=golang-codereviews
https://golang.org/cl/98080043
There was confusion in the behavior of json.Indent; This change
attempts to clarify the behavior by providing a bit more verbiage
to the documentation as well as provide an example function.
Fixes#7821.
LGTM=robert.hencke, adg
R=golang-codereviews, minux.ma, bradfitz, aram, robert.hencke, r, adg
CC=golang-codereviews
https://golang.org/cl/97840044
Existing test TestMaxOpenConns was failing occasionally, especially
with higher values of GOMAXPROCS.
Fixes#7532
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/95130043
Number of lost samples was overcounted (never reset).
Also remove unused variable (it's trivial to restore it for debugging if needed).
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, rsc
https://golang.org/cl/96060043
The previous syscall constants regeneration on openbsd was conducted
with OpenBSD current 3 months ago and it missed updating openbsd/386.
This CL adds TIOCGSID for fixing the inconsistency between opensbd/amd64
and openbsd/386.
Update #7049
LGTM=iant
R=jsing, rsc, iant
CC=golang-codereviews
https://golang.org/cl/96960044
Where the spelling changed from British to
US norm (e.g., optimise -> optimize) it follows
the style in that file.
LGTM=adonovan
R=golang-codereviews, adonovan
CC=golang-codereviews
https://golang.org/cl/96980043
Because gotraceback is called early and often, its cache commits to the value of getenv("GOTRACEBACK") before getenv is even ready. So now we reset its cache once getenv becomes ready. Panicking programs now dump core again.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/97800045
If slice append is the only place where a program allocates,
then it will consume all available memory w/o triggering GC.
This was demonstrated in the issue.
Fixes#7922.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews, iant, khr
https://golang.org/cl/91010048
The monotonic clock patch changed all runtime times
to abstract monotonic time. As the result user-visible
MemStats.LastGC become monotonic time as well.
Restore Unix time for LastGC.
This is the simplest way to expose time.now to runtime that I found.
Another option would be to change time.now to C called
int64 runtime.unixnanotime() and then express time.now in terms of it.
But this would require to introduce 2 64-bit divisions into time.now.
Another option would be to change time.now to C called
void runtime.unixnanotime1(struct {int64 sec, int32 nsec} *now)
and then express both time.now and runtime.unixnanotime in terms of it.
Fixes#7852.
LGTM=minux.ma, iant
R=minux.ma, rsc, iant
CC=golang-codereviews
https://golang.org/cl/93720045
If systems actually read that much, using 2GB-1 will
result in misaligned subsequent reads. Use 1GB instead,
which will certainly keep reads aligned and which is
plenty large enough.
Update #7812.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/94070044
This change allows us to give an hg tag such as "go1.3beta1" to
revisions in the main branch without breaking the build.
This is helpful for community members who want to build the beta
from source.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/90190044
The backing memory for >1 word interfaces was being scanned
conservatively.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/94000043
go test may call builder.init() multiple times which will create a new work directory. The cleanup needs to hoist the current work directory.
Fixes#7904.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/95900044
Not only ChST but also MeST (America/Metlakatla) is a zone
name containing a lower case letter.
LGTM=robert.hencke, r
R=golang-codereviews, robert.hencke, bradfitz, r
CC=golang-codereviews
https://golang.org/cl/99910043
Some system doesn't have libc.a available.
While we're at here, also export GOROOT in run.bash, so that
one doesn't need to set GOROOT to run run.bash.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/99870043
For gccgo we rename exported functions so that the compiler
will make them visible. This CL adds a #define so that C
functions that #include "cgo_export.h" can use the expected
names of the function.
The test for this is the existing issue6833 test in
misc/cgo/test. Without this CL it fails when using
-compiler=gccgo.
LGTM=minux.ma, rsc
R=golang-codereviews, gobot, rsc, minux.ma
CC=golang-codereviews
https://golang.org/cl/91830046
This should make Go build without setting CC and CXX on newer FreeBSDs.
LGTM=iant
R=golang-codereviews, dave, gobot, iant
CC=golang-codereviews
https://golang.org/cl/89230045
This CL tries to fill the gap between Linux and other Unix-like systems
in the same way UDPConn and UnixConn already did.
Fixes#7887.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/97810043
Use a real type for Gs instead of scanning them conservatively.
Zero the schedlink pointer when it is dead.
Update #7820
LGTM=rsc
R=rsc, dvyukov
CC=golang-codereviews
https://golang.org/cl/89360043
Previously we used strndup(3) to implement C.CString for gccgo. This
is wrong because strndup assumes the string to be null terminated,
and stops at the first null terminator. Instead, use malloc
and memmove to create a copy of the string, as we do in the
gc implementation.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/96790047
A very smart developer at Gophercon just asked me to help debug
a problem and I was horrified to learn that he was using httputil's
ClientConn. I forgot ClientConn and ServerConn were even included
in Go 1! They should've been deleted.
Scare people away from using them. The net/http package does
not use them and they're unused, unmaintained and untouched in
4+ years.
LGTM=r
R=r, adg
CC=golang-codereviews
https://golang.org/cl/92790043
Remove an RUnlock of syscall.ForkLock with no matching RLock.
Holding ForkLock in netFD.dup is unnecessary: dupCloseOnExecOld
locks and unlocks the lock on its own and dupCloseOnExec doesn't
need the ForkLock to be held.
LGTM=iant
R=golang-codereviews, bradfitz, iant
CC=golang-codereviews
https://golang.org/cl/99800044
Previously Read wouldn't return once its internal input buffer
is filled with non-data bytes.
Fixes#7875.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/90820043
I fixed this data race regression in two ways: in net/http itself, and also
partially reverting the change from https://golang.org/cl/77580046 .
Previously a Read from a strings.Reader or bytes.Reader returning 0 bytes
would not be a memory write. After 77580046 it was. This reverts that back
in case others depended on that. Also adds tests.
Fixes#7856
LGTM=ruiu, iant
R=iant, ruiu
CC=golang-codereviews, gri
https://golang.org/cl/94740044
Godoc makes it look better this way; before, it all ran together into nonsense.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/90400045
Previously it would panic because of out-of-bound access
if s1 is longer than s2.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/90110043
TestVariousDeadlines1Proc was flaky on my system,
failing on about 5% of runs.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/89830045
This should have been part of 36eb4a62fbb6,
but I later discovered that addresses are all wrong.
Appropriate test added now.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/89470043
`GOARCH=arm go tool 6c` used to give "<prog>: cannot use 6c with GOARCH=arm"
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/89330043
CL 89050043 only allows -ccflags for 'go test', this
CL really handles the flag like the other -??flags.
Many thanks to Dobrosław Żybort for pointing this out.
Fixes#7810 (again).
LGTM=iant, matrixik
R=golang-codereviews, iant, matrixik
CC=golang-codereviews
https://golang.org/cl/89230044
following on CL https://golang.org/cl/76810045 and
issue 7563, i now see there's another "remove(outfile)" a few
dozen lines down that also needs fixing.
LGTM=iant
R=golang-codereviews, iant
CC=0intro, golang-codereviews, r
https://golang.org/cl/89030043
changed (pwd string) to (dir string), as some think pwd means passwd.
Fixes#7811.
LGTM=iant
R=golang-codereviews, iant, bradfitz
CC=golang-codereviews
https://golang.org/cl/89100043
Patch from msolo. Just moving it to a CL.
The test fails before and passes with the fix.
Fixes#7098
LGTM=msolo, rsc
R=rsc, iant, msolo
CC=golang-codereviews
https://golang.org/cl/88900044
It is possible to use ./ imports on Windows but it
requires some extra command-line work
('go build' does this automatically, but we can't use 'go build' here).
Instead, use an ordinary import and -I/-L, which are easier to use.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/89040043
According to RFC 6265 a cookie value may contain neither
commas nor spaces but such values are very common in the
wild and browsers handle them very well so we'll allow
both commas and spaces.
Values starting or ending in a comma or a space are
sent in the quoted form to prevent missinterpetations.
RFC 6265 conforming values are handled as before and
semicolons, backslashes and double-quotes are still
disallowed.
Fixes#7243
LGTM=nigeltao
R=nigeltao
CC=bradfitz, golang-codereviews
https://golang.org/cl/86050045
Variables declared with 'var' have no sym->def.
Fixes#7794.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/88360043
Before the switch to liblink, the linkers accepted the -c flag
to print the call graph. This change restores the functionality.
This came in handy when I was trying to audit the use of SSE
instructions inside the Plan 9 note handler.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/73990043
https://golang.org/cl/60590044 edited
doc.go without editing the file it is generated from.
The edit was lost at the next mkdoc.sh.
Make the change in help.go and rerun mkdoc.sh.
Pointed out in the review of CL 68580043.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/88760043
This information is required by cmd/nm
to calculate absolute symbol addresses.
Update #6936
Update #7738
LGTM=rsc
R=golang-codereviews, bradfitz, gobot, rsc
CC=golang-codereviews
https://golang.org/cl/87500043
The new code is adapted from the Go 1.2 nosplit code,
but it does not have the bug reported in issue 7623:
g% go run nosplit.go
g% go1.2 run nosplit.go
BUG
rejected incorrectly:
main 0 call f; f 120
linker output:
# _/tmp/go-test-nosplit021064539
main.main: nosplit stack overflow
120 guaranteed after split check in main.main
112 on entry to main.f
-8 after main.f uses 120
g%
Fixes#6931.
Fixes#7623.
LGTM=iant
R=golang-codereviews, iant, ality
CC=golang-codereviews, r
https://golang.org/cl/88190043
The name linkwriteobj is misleading because it implies that
the function has something to do with the linker, which it
does not. The name is historical: the function performs an
operation that was previously performed by the linker, but no
longer is.
LGTM=rsc
R=rsc, minux.ma
CC=golang-codereviews
https://golang.org/cl/88210045
Without the leaf bit, the linker cannot record
the correct frame size in the symbol table, and
then stack traces get mangled. (Only for ARM.)
Fixes#7338.
Fixes#7347.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/88550043
A //line directive without a filename now denotes the empty
filename, not the current directory (the Go 1.2 behaviour) nor
the previous //line's filename (the behaviour since CL
86990044).
They should never appear (but they do, e.g. due to a bug in godoc).
Fixes#7765
LGTM=gri, rsc
R=rsc, gri
CC=golang-codereviews
https://golang.org/cl/88160050
A //line directive without a filename now denotes the same
filename as the previous line (as in C).
Previously it denoted the file's directory (!).
Fixes#7765
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/86990044
In large functions with many variables, the register optimizer
may give up and choose not to track certain variables at all.
In this case, the "nextinnode" information linking together
all the words from a given variable will be incomplete, and
the result may be that only some of a multiword value is
preserved across a call. That confuses the garbage collector,
so don't do that. Instead, mark those variables as having
their address taken, so that they will be preserved at all
calls. It's overkill, but correct.
Tested by hand using the 6g -S output to see that it does fix
the buggy generated code leading to the issue 7726 failure.
There is no automated test because I managed to break the
compiler while writing a test (see issue 7727). I will check
in a test along with the fix to issue 7727.
Fixes#7726.
LGTM=khr
R=khr, bradfitz, dave
CC=golang-codereviews
https://golang.org/cl/85200043
This has typically crashed in the past, although usually with
an 'all goroutines are asleep - deadlock!' message that shows
no goroutines (because there aren't any).
Previous discussion at:
https://groups.google.com/d/msg/golang-nuts/uCT_7WxxopQ/BoSBlLFzUTkJhttps://groups.google.com/d/msg/golang-dev/KUojayEr20I/u4fp_Ej5PdUJhttp://golang.org/issue/7711
There is general agreement that runtime.Goexit terminates the
main goroutine, so that main cannot return, so the program does
not exit.
The interpretation that all other goroutines exiting causes an
exit(0) is relatively new and was not part of those discussions.
That is what this CL changes.
Thankfully, even though the exit(0) has been there for a while,
some other accounting bugs made it very difficult to trigger,
so it is reasonable to replace. In particular, see golang.org/issue/7711#c10
for an examination of the behavior across past releases.
Fixes#7711.
LGTM=iant, r
R=golang-codereviews, iant, dvyukov, r
CC=golang-codereviews
https://golang.org/cl/88210044
linklookup uses hash(name, v) as the hash table index but then
only compares name to find a symbol to return.
If hash(name, v1) == hash(name, v2) for v1 != v2, the lookup
for v2 will return the symbol with v1.
The input routines assume that each symbol is found only once,
and then each symbol is added to a linked list, with the list header
in the symbol. Adding a symbol to such a list multiple times
short-circuits the list the second time it is added, causing symbols
to be dropped.
The liblink rewrite introduced an elegant, if inefficient, handling
of duplicated symbols by creating a dummy symbol to read the
duplicate into. The dummy symbols are named .dup with
sequential version numbers. With many .dup symbols, eventually
there will be a conflict, causing a duplicate list add, causing elided
symbols, causing a crash when calling one of the elided symbols.
The bug is old (2011) but could not have manifested until the
liblink rewrite introduced this heavily duplicated symbol .dup.
(See History section below.)
1. Correct the lookup function.
2. Since we want all the .dup symbols to be different, there's no
point in inserting them into the table. Call linknewsym directly,
avoiding the lookup function entirely.
3. Since nothing can refer to the .dup symbols, do not bother
adding them to the list of functions (textp) at all.
4. In lieu of a unit test, introduce additional consistency checks to
detect adding a symbol to a list multiple times. This would have
caught the short-circuit more directly, and it will detect a variety
of double-use bugs, including the one arising from the bad lookup.
Fixes#7749.
History
On April 9, 2011, I submitted CL 4383047, making ld 25% faster.
Much of the focus was on the hash table lookup function, and
one of the changes was to remove the s->version == v comparison [1].
I don't know if this was a simple editing error or if I reasoned that
same name but different v would yield a different hash slot and
so the name test alone sufficed. It is tempting to claim the former,
but it was probably the latter.
Because the hash is an iterated multiply+add, the version ends up
adding v*3ⁿ to the hash, where n is the length of the name.
A collision would need x*3ⁿ ≡ y*3ⁿ (mod 2²⁴ mod 100003),
or equivalently x*3ⁿ ≡ x*3ⁿ + (y-x)*3ⁿ (mod 2²⁴ mod 100003),
so collisions will actually be periodic: versions x and y collide
when d = y-x satisfies d*3ⁿ ≡ 0 (mod 2²⁴ mod 100003).
Since we allocate version numbers sequentially, this is actually
about the best case one could imagine: the collision rate is
much lower than if the hash were more random.
http://play.golang.org/p/TScD41c_hA computes the collision
period for various name lengths.
The most common symbol in the new linker is .dup, and for n=4
the period is maximized: the 100004th symbol is the first collision.
Unfortunately, there are programs with more duplicated symbols
than that.
In Go 1.2 and before, duplicate symbols were handled without
creating a dummy symbol, so this particular case for generating
many duplicate symbols could not happen. Go does not use
versioned symbols. Only C does; each input file gives a different
version to its static declarations. There just aren't enough C files
for this to come up in that context.
So the bug is old but the realization of the bug is new.
[1] https://golang.org/cl/4383047/diff/5001/src/cmd/ld/lib.c
LGTM=minux.ma, iant, dave
R=golang-codereviews, minux.ma, bradfitz, iant, dave
CC=golang-codereviews, r
https://golang.org/cl/87910047
When preparing a call with an interface method, the argument
frame holds the receiver "iword", but funcLayout was being
asked to write a descriptor as if the receiver were a complete
interface value. This was originally caught by running a large
program with Debug=3 in runtime/mgc0.c, but the new panic
in funcLayout suffices to catch the mistake with the existing
tests.
Fixes#7748.
LGTM=bradfitz, iant
R=golang-codereviews, bradfitz, iant
CC=golang-codereviews, khr
https://golang.org/cl/88100048
Having the pointers means you can grub around in the
binary finding out more about them.
This helped with issue 7748.
LGTM=minux.ma, bradfitz
R=golang-codereviews, minux.ma, bradfitz
CC=golang-codereviews
https://golang.org/cl/88090045
Make it clear that types that wrap another reader or writer delegate to the wrapped type.
Fixes#7667
LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/85720044
This code never got updated after the liblink shuffle.
Tested by hand that it works and respects GOROOT_FINAL.
The discussion in issue 6963 suggests that perhaps we should
just drop runtime-gdb.py entirely, but I am not convinced
that is true. It was in Go 1.2 and I don't see a reason not to
keep it in Go 1.3. The fact that binaries have not been emitting
the reference was just a missed detail in the liblink conversion,
not part of a grand plan.
Fixes#7506.
Fixes#6963.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/87870048
If we compile a generated file stored in a temporary
directory - let's say /tmp/12345/work/x.c - then by default
6c stores the full path and then the pcln table in the
final binary includes the full path. This makes repeated builds
(using different temporary directories) produce different
binaries, even if the inputs are the same.
In the old 'go tool pack', the P flag specified a prefix to remove
from all stored paths (if present), and cmd/go invoked
'go tool pack grcP $WORK' to remove references to the
temporary work directory.
We've changed the build to avoid pack as much as possible,
under the theory that instead of making pack convert from
.6 to .a, the tools should just write the .a directly and save a
round of I/O.
Instead of going back to invoking pack always, define a common
flag -trimpath in the assemblers, C compilers, and Go compilers,
implemented in liblink, and arrange for cmd/go to use the flag.
Then the object files being written out have the shortened paths
from the start.
While we are here, reimplement pcln support for GOROOT_FINAL.
A build in /tmp/go uses GOROOT=/tmp/go, but if GOROOT_FINAL=/usr/local/go
is set, then a source file named /tmp/go/x.go is recorded instead as
/usr/local/go/x.go. We use this so that we can prepare distributions
to be installed in /usr/local/go without actually working in that
directory. The conversion to liblink deleted all the old file name
handling code, including the GOROOT_FINAL translation.
Bring the GOROOT_FINAL translation back.
Before this CL, using GOROOT_FINAL=/goroot make.bash:
g% strings $(which go) | grep -c $TMPDIR
6
g% strings $(which go) | grep -c $GOROOT
793
g%
After this CL:
g% strings $(which go) | grep -c $TMPDIR
0
g% strings $(which go) | grep -c $GOROOT
0
g%
(The references to $TMPDIR tend to be cgo-generated source files.)
Adding the -trimpath flag to the assemblers required converting
them to the new Go-semantics flag parser. The text in go1.3.html
is copied and adjusted from go1.1.html, which is when we applied
that conversion to the compilers and linkers.
Fixes#6989.
LGTM=iant
R=r, iant
CC=golang-codereviews
https://golang.org/cl/88300045
Per golang-nuts question. Writing to p breaks
other writers (e.g. io.MultiWriter).
Make this explicit.
LGTM=gri, r, rsc
R=r, rsc, gri, joshlf13
CC=golang-codereviews
https://golang.org/cl/87780046
My cmd/go got in a weird state where it started invoking pack grcP.
Change pack to print a 1-line explanation of the usage problem
before the generic usage message.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/87770047
OpenBSD is excluded from all the usual thread-local storage
code, not just emitting the tbss section in the external link .o
but emitting a PT_TLS section in an internally-linked executable.
I assume it just has no proper TLS support. Exclude it here too.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/87900045
We get
/usr/lib/libc.a(stack_protector.o): In function `__stack_chk_fail_local':
stack_protector.c:(.text+0x158): multiple definition of `__stack_chk_fail_local'
/var/tmp/go-link-04838a/000001.o:/tmp/gobuilder/netbsd-386-minux-c7a9e9243878/go/src/pkg/runtime/cgo/gcc_386.S:41: first defined here
I am assuming this has never worked and possibly is not intended to work.
(Some systems are vehemently against static linking.)
TBR=iant
CC=golang-codereviews
https://golang.org/cl/88130046
When I did the original 386 ports on Linux and OS X, I chose to
define GS-relative expressions like 4(GS) as relative to the actual
thread-local storage base, which was usually GS but might not be
(it might be FS, or it might be a different constant offset from GS or FS).
The original scope was limited but since then the rewrites have
gotten out of control. Sometimes GS is rewritten, sometimes FS.
Some ports do other rewrites to enable shared libraries and
other linking. At no point in the code is it clear whether you are
looking at the real GS/FS or some synthesized thing that will be
rewritten. The code manipulating all these is duplicated in many
places.
The first step to fixing issue 7719 is to make the code intelligible
again.
This CL adds an explicit TLS pseudo-register to the 386 and amd64.
As a register, TLS refers to the thread-local storage base, and it
can only be loaded into another register:
MOVQ TLS, AX
An offset from the thread-local storage base is written off(reg)(TLS*1).
Semantically it is off(reg), but the (TLS*1) annotation marks this as
indexing from the loaded TLS base. This emits a relocation so that
if the linker needs to adjust the offset, it can. For example:
MOVQ TLS, AX
MOVQ 8(AX)(TLS*1), CX // load m into CX
On systems that support direct access to the TLS memory, this
pair of instructions can be reduced to a direct TLS memory reference:
MOVQ 8(TLS), CX // load m into CX
The 2-instruction and 1-instruction forms correspond roughly to
ELF TLS initial exec mode and ELF TLS local exec mode, respectively.
Liblink applies this rewrite on systems that support the 1-instruction form.
The decision is made using only the operating system (and probably
the -shared flag, eventually), not the link mode. If some link modes
on a particular operating system require the 2-instruction form,
then all builds for that operating system will use the 2-instruction
form, so that the link mode decision can be delayed to link time.
Obviously it is late to be making changes like this, but I despair
of correcting issue 7719 and issue 7164 without it. To make sure
I am not changing existing behavior, I built a "hello world" program
for every GOOS/GOARCH combination we have and then worked
to make sure that the rewrite generates exactly the same binaries,
byte for byte. There are a handful of TODOs in the code marking
kludges to get the byte-for-byte property, but at least now I can
explain exactly how each binary is handled.
The targets I tested this way are:
darwin-386
darwin-amd64
dragonfly-386
dragonfly-amd64
freebsd-386
freebsd-amd64
freebsd-arm
linux-386
linux-amd64
linux-arm
nacl-386
nacl-amd64p32
netbsd-386
netbsd-amd64
openbsd-386
openbsd-amd64
plan9-386
plan9-amd64
solaris-amd64
windows-386
windows-amd64
There were four exceptions to the byte-for-byte goal:
windows-386 and windows-amd64 have a time stamp
at bytes 137 and 138 of the header.
darwin-386 and plan9-386 have five or six modified
bytes in the middle of the Go symbol table, caused by
editing comments in runtime/sys_{darwin,plan9}_386.s.
Fixes#7164.
LGTM=iant
R=iant, aram, minux.ma, dave
CC=golang-codereviews
https://golang.org/cl/87920043
It was said already but apparently not enough times.
Fixes#6985.
LGTM=crawshaw
R=golang-codereviews, crawshaw
CC=golang-codereviews
https://golang.org/cl/86300043
Do not consider idle finalizer/bgsweep/timer goroutines as doing something useful.
We can't simply set isbackground for the whole lifetime of the goroutines,
because when finalizer goroutine calls user function, we do want to consider it
as doing something useful.
This is borken due to timers for quite some time.
With background sweep is become even more broken.
Fixes#7784.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/87960044
This breaks "go get -d repo/path/...".
««« original CL description
cmd/go: do not miss an error if import path contains "cmd/something"
Fixes#7638
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/87300043
»»»
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/87890043
Windows is building a chain to the AddTrust root which is different
from the native Go code and causing a build failure.
This change alters the test so that both should build to the AddTrust
root.
R=bradfitz
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/87570044
The relocation and automatic variable types were using
arch-specific numbers. Introduce portable enumerations
instead.
To the best of my knowledge, these are the only arch-specific
bits left in the new object file format.
Remove now, before Go 1.3, because file formats are forever.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/87670044
Comodo are now using a SHA-384 signed intermediate. The crypto/x509
package seeks to import hash functions needed for typical operation
without needing to import every hash function possible. Since a SHA-384
certificate is being used by Comodo, crypto/sha512 now appears to fall
into the scope of "typical operation".
R=bradfitz
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/87670045
Update #7264
Races:
http://build.golang.org/log/a2e401fdcd4903a61a3375bff5da702a20ddafadhttp://build.golang.org/log/ec4c69e92076a747ac6d5df7eb7b382b31ab3d43
I think this is the first time I've actually seen a manifestation
of Issue 7264, and one that I can reproduce.
I don't know why it triggers on this test and not any others
just like it, or why I can't reproduce Issue 7264
independently, even when Dmitry gives me minimal repros.
Work around it for now with some synchronization to make the
race detector happy.
The proper fix will probably be in net/http/httptest itself, not
in all hundred some tests.
LGTM=rsc
R=rsc
CC=dvyukov, golang-codereviews
https://golang.org/cl/87640043
There are changes we know we want to make, but not before Go 1.3
Add a version number so that we can make them more easily later.
LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/87670043
Currently Pool can cache up to 15 elements per P, and these elements are not accesible to other Ps.
If a Pool caches large objects, say 2MB, and GOMAXPROCS is set to a large value, say 32,
then the Pool can waste up to 960MB.
The new caching policy caches at most 1 per-P element, the rest is shared between Ps.
Get/Put performance is unchanged. Nested Get/Put performance is 57% worse.
However, overall scalability of nested Get/Put is significantly improved,
so the new policy starts winning under contention.
benchmark old ns/op new ns/op delta
BenchmarkPool 27.4 26.7 -2.55%
BenchmarkPool-4 6.63 6.59 -0.60%
BenchmarkPool-16 1.98 1.87 -5.56%
BenchmarkPool-64 1.93 1.86 -3.63%
BenchmarkPoolOverlflow 3970 6235 +57.05%
BenchmarkPoolOverlflow-4 10935 1668 -84.75%
BenchmarkPoolOverlflow-16 13419 520 -96.12%
BenchmarkPoolOverlflow-64 10295 380 -96.31%
LGTM=rsc
R=rsc
CC=golang-codereviews, khr
https://golang.org/cl/86020043
These are not ready and will not be in Go 1.3.
Fixes#6932.
LGTM=bradfitz
R=golang-codereviews, bradfitz, minux.ma
CC=golang-codereviews, iant, r
https://golang.org/cl/87630043
We have never released cmd/prof and don't plan to.
Now that nm, addr2line, and objdump have been rewritten in Go,
cmd/prof is the only thing keeping us from deleting libmach.
Delete cmd/prof, and then since nothing is using libmach, delete libmach.
13,000 lines of C deleted.
LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews, iant, r
https://golang.org/cl/87020044
Update cmd/dist not to build the C version.
Update cmd/go to install the Go version to the tool directory.
Update #7452
This is the basic logic needed for objdump, and it works well enough
to support the pprof list and weblist commands. A real disassembler
needs to be added in order to support the pprof disasm command
and the per-line assembly displays in weblist. That's still to come.
Probably objdump will move to go.tools when the disassembler
is added, but it can stay here for now.
LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews, iant, r
https://golang.org/cl/87580043
It looks like maybe on slower builders 4 seconds is not enough.
Trying to get rid of the flaky failures.
TBR=iant
CC=golang-codereviews
https://golang.org/cl/86870044
What was happening on Issue 7010 was handler intentionally took 30
milliseconds and the proxy's client timeout was 35 milliseconds. Then it
slammed the proxy with a bunch of requests.
Sometimes the server would be too slow to respond in its 5 millisecond
window and the client code would cancel the request, force-closing the
persistConn. If this came at the right time, the server's reply was
already in flight, and one of the goroutines would report:
Unsolicited response received on idle HTTP channel starting with "H"; err=<nil>
... rightfully scaring the user.
But the error was already handled and returned to the user, and this
connection knows it's been shut down. So look at the closed flag after
acquiring the same mutex guarding another field we were checking, and
don't complain if it's a known shutdown.
Also move closed down below the mutex which guards it.
Fixes#7010
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/86740044
The test fails now with -race, so it's disabled.
The intention is that the fix for issue 7264
will also modify this test the same way and enable it.
Reporduce with 'go test -race -issue7264 -cpu=4'.
Update #7264
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/86770043
Add test for multipart form requests with an invalid content-type to ensure
ErrNotMultipart is returned.
Change ParseMultipartForm to return ErrNotMultipart when it is returned by multipartReader.
Modify test for empty multipart request handling to use POST so that the body is checked.
Fixes#6334.
This is the first changeset working on multipart request handling. Further changesets
could add more tests and clean up the TODO.
LGTM=bradfitz
R=golang-codereviews, gobot, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/44040043
I was implementing rules from RFC 2616. The rules are apparently useless,
ambiguous, and too strict for common software on the Internet. (e.g. curl)
Add more tests, including a test of a chunked request.
Fixes#7625
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/84480045
Also: Simplify ReadSlice implementation and
ensure that it doesn't call fill() with a full
buffer (this caused a failure in net/textproto
TestLargeReadMIMEHeader because fill() wasn't able
to read more data).
Fixes#7745.
LGTM=bradfitz
R=r, bradfitz
CC=golang-codereviews
https://golang.org/cl/86590043
To create a valid JSON string, "%s" is not enough.
Fixes#7761.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/86730043
It runs too long in -short mode.
Disable the one in init, because it doesn't respect -short.
Make the part that claims to test execution in a finalizer
actually execute the test in the finalizer.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=aram.h, golang-codereviews, iant, khr
https://golang.org/cl/86550045
The Go HTTP server doesn't use Response.Write, but others do,
so make it correct. Add a bunch more tests.
This bug is almost a year old. :/
Fixes#5381
LGTM=adg
R=golang-codereviews, adg
CC=dsymonds, golang-codereviews, rsc
https://golang.org/cl/85740046
Go's had pretty decent HTTP Trailer support for a long time, but
the docs have been largely non-existent. Fix that.
In the process, re-learn the Trailer code, clean some stuff
up, add some error checks, remove some TODOs, fix a minor bug
or two, and add tests.
LGTM=adg
R=golang-codereviews, adg
CC=dsymonds, golang-codereviews, rsc
https://golang.org/cl/86660043
There is a race condition that causes spurious wakeup from Wait
in the following case:
G1: decrement wg.counter, observe the counter is now 0
(should unblock goroutines queued *at this moment*)
G2: increment wg.counter
G2: call Wait() to add itself to the wait queue
G1: acquire wg.m, unblock all waiting goroutines
In the last step G2 is spuriously woken up by G1.
Fixes#7734.
LGTM=rsc, dvyukov
R=dvyukov, 0xjnml, rsc
CC=golang-codereviews
https://golang.org/cl/85580043
In a typical HTTP request, the client writes the request, and
then the server replies. Go's HTTP client code (Transport) has
two goroutines per connection: one writing, and one reading. A
third goroutine (the one initiating the HTTP request)
coordinates with those two.
Because most HTTP requests are done when the server replies,
the Go code has always handled connection reuse purely in the
readLoop goroutine.
But if a client is writing a large request and the server
replies before it's consumed the entire request (e.g. it
replied with a 403 Forbidden and had no use for the body), it
was possible for Go to re-select that connection for a
subsequent request before we were done writing the first. That
wasn't actually a data race; the second HTTP request would
just get enqueued to write its request on the writeLoop. But
because the previous writeLoop didn't finish writing (and
might not ever), that connection is in a weird state. We
really just don't want to get into a state where we're
re-using a connection when the server spoke out of turn.
This CL changes the readLoop goroutine to verify that the
writeLoop finished before returning the connection.
In the process, it also fixes a potential goroutine leak where
a connection could close but the recycling logic could be
blocked forever waiting for the client to read to EOF or
error. Now it also selects on the persistConn's close channel,
and the closer of that is no longer the readLoop (which was
dead locking in some cases before). It's now closed at the
same place the underlying net.Conn is closed. This likely fixes
or helps Issue 7620.
Also addressed some small cosmetic things in the process.
Update #7620Fixes#7569
LGTM=adg
R=golang-codereviews, adg
CC=dsymonds, golang-codereviews, rsc
https://golang.org/cl/86290043
We originally decided to skip this test in short mode
to prevent the parallel runtime test to timeout on the
Plan 9 builder. This should no longer be required since
the issue was fixed in CL 86210043.
LGTM=dave, bradfitz
R=dvyukov, dave, bradfitz
CC=golang-codereviews, rsc
https://golang.org/cl/84790044
If you pass ns = 100,000 to this function, timediv will
return ms = 0. tsemacquire in /sys/src/9/port/sysproc.c
will return immediately when ms == 0 and the semaphore
cannot be acquired immediately - it doesn't sleep - so
notetsleep will spin, chewing cpu and repeatedly reading
the time, until the 100us have passed.
Thanks to the time reads it won't take too many iterations,
but whatever we are waiting for does not get a chance to
run. Eventually the notetsleep spin loop returns and we
end up in the stoptheworld spin loop - actually a sleep
loop but we're not doing a good job of sleeping.
After 100ms or so of this, the kernel says enough and
schedules a different thread. That thread manages to do
whatever we're waiting for, and the spinning in the other
thread stops. If tsemacquire had actually slept, this
would have happened much quicker.
Many thanks to Russ Cox for help debugging.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/86210043
even when there are no *_test.go files present.
rsc suggested this change
Fixes#7108
LGTM=r, adg
R=golang-codereviews, r, adg
CC=golang-codereviews
https://golang.org/cl/84300043
The buffer length should be the size in bytes
instead of the number of structs.
Fixes#6588.
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh, adg
CC=golang-codereviews
https://golang.org/cl/84830043
Explain what its purpose is and give examples of good and bad use.
Fixes#7167.
LGTM=dvyukov, rsc
R=golang-codereviews, dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/85880044
On amd64p32 pointers are 32-bit-aligned and cannot be assumed to
have an offset multiple of widthreg. Instead check that they are
withptr-aligned.
Also change the threshold for region merging to 2*widthreg
instead of 2*widthptr because performance on amd64 and amd64p32
is expected to be the same.
Fixes#7712.
LGTM=khr
R=rsc, dave, khr, brad, bradfitz
CC=golang-codereviews
https://golang.org/cl/84690044
Cuts the number of calls from 6 to 2 in the non-debug case.
LGTM=iant
R=golang-codereviews, iant
CC=0intro, aram, golang-codereviews, khr
https://golang.org/cl/86040043
1) The code to catch an exception marked the template as escaped
when it was not yet, which caused subsequent executions of the
template to not escape properly.
2) ensurePipelineContains needs to handled Field as well as
Identifier nodes.
Fixes#7379.
LGTM=mikesamuel
R=mikesamuel
CC=golang-codereviews
https://golang.org/cl/85240043
Getenv() should not call malloc when called from
gotraceback(). Instead, we return a static buffer
in this case, with enough room to hold the longest
value.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/85680043
On Plan 9 gotraceback calls getenv calls malloc, and we gotraceback
on every call to gentraceback, which happens during garbage collection.
Honestly I don't even know how this works on Plan 9.
I suspect it does not, and that we are getting by because
no one has tried to run with $GOTRACEBACK set at all.
This will speed up all the other systems by epsilon, since they
won't call getenv and atoi repeatedly.
LGTM=bradfitz
R=golang-codereviews, bradfitz, 0intro
CC=golang-codereviews
https://golang.org/cl/85430046
Add a $Context variable to the template so that the build.Context values
such as BuildTags can be accessed.
Fixes#6666.
LGTM=adg, rsc
R=golang-codereviews, gobot, adg, rsc
CC=golang-codereviews
https://golang.org/cl/72770043
Short circuit for calling values funcs by MakeFunc was placed
before variadic arg rearrangement code in reflect.call.
Fixes#7534.
LGTM=khr
R=golang-codereviews, bradfitz, khr, rsc
CC=golang-codereviews
https://golang.org/cl/75370043
Now that we have a constant-time P-256 implementation, it's worth
paying more attention elsewhere.
The inversion of k in (EC)DSA was using Euclid's algorithm which isn't
constant-time. This change switches to Fermat's algorithm, which is
much better. However, it's important to note that math/big itself isn't
constant time and is using a 4-bit window for exponentiation with
variable memory access patterns.
(Since math/big depends quite deeply on its values being in minimal (as
opposed to fixed-length) represetation, perhaps crypto/elliptic should
grow a constant-time implementation of exponentiation in the scalar
field.)
R=bradfitz
Fixes#7652.
LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/82740043
Given
type Outer struct {
*Inner
...
}
the compiler generates the implementation of (*Outer).M dispatching to
the embedded Inner. The implementation is logically:
func (p *Outer) M() {
(p.Inner).M()
}
but since the only change here is the replacement of one pointer
receiver with another, the actual generated code overwrites the
original receiver with the p.Inner pointer and then jumps to the M
method expecting the *Inner receiver.
During reflect.Value.Call, we create an argument frame and the
associated data structures to describe it to the garbage collector,
populate the frame, call reflect.call to run a function call using
that frame, and then copy the results back out of the frame. The
reflect.call function does a memmove of the frame structure onto the
stack (to set up the inputs), runs the call, and the memmoves the
stack back to the frame structure (to preserve the outputs).
Originally reflect.call did not distinguish inputs from outputs: both
memmoves were for the full stack frame. However, in the case where the
called function was one of these wrappers, the rewritten receiver is
almost certainly a different type than the original receiver. This is
not a problem on the stack, where we use the program counter to
determine the type information and understand that during (*Outer).M
the receiver is an *Outer while during (*Inner).M the receiver in the
same memory word is now an *Inner. But in the statically typed
argument frame created by reflect, the receiver is always an *Outer.
Copying the modified receiver pointer off the stack into the frame
will store an *Inner there, and then if a garbage collection happens
to scan that argument frame before it is discarded, it will scan the
*Inner memory as if it were an *Outer. If the two have different
memory layouts, the collection will intepret the memory incorrectly.
Fix by only copying back the results.
Fixes#7725.
LGTM=khr
R=khr
CC=dave, golang-codereviews
https://golang.org/cl/85180043
It turns out there is a relatively common pattern that relies on
inverted channel semaphore:
gate := make(chan bool, N)
for ... {
// limit concurrency
gate <- true
go func() {
foo(...)
<-gate
}()
}
// join all goroutines
for i := 0; i < N; i++ {
gate <- true
}
So handle synchronization on inverted semaphores with cap>1.
Fixes#7718.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/84880046
This code tests linkmode == LinkExternal but is only invoked
by the compiler/assembler, not the linker.
Update #7164
LGTM=rsc
R=rsc, dave
CC=golang-codereviews
https://golang.org/cl/85080043
Defers generated from cgo lie to us about their argument layout.
Mark those defers as not copyable.
CL 83820043 contains an additional test for this code and should be
checked in (and enabled) after this change is in.
Fixes bug 7695.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/84740043
Iterate the right number of times in arrays and channels.
Handle channels with zero-sized objects in them.
Output longer type names if we have them.
Compute argument offset correctly.
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/82980043
Also makes ErrWriteToConnected more appropriate; it's used
not only UDPConn operations but UnixConn operations.
Update #4856
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/84800044
The prefix was not uniformly applied and is probably better
left off for using with OpError.
Update #4856
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/84660046
Takes advantage of CL 83740044, to optimize map[string] lookup
from []byte key.
Deletes code.
No conditional check for gccgo, since Ian plans to add this
to gccgo before GCC 4.10 (Go 1.3).
benchmark old ns/op new ns/op delta
BenchmarkReadMIMEHeader 6066 5086 -16.16%
benchmark old allocs new allocs delta
BenchmarkReadMIMEHeader 12 12 +0.00%
benchmark old bytes new bytes delta
BenchmarkReadMIMEHeader 1317 1317 +0.00%
Update #3512
LGTM=rsc
R=rsc, dave
CC=golang-codereviews, iant
https://golang.org/cl/84230043
Superficial inconsistencies that trigger warnings in
Plan 9. Small enough to be considered trivial and
seemingly benign outside of the Plan 9 environment.
LGTM=iant
R=golang-codereviews, 0intro, iant
CC=golang-codereviews
https://golang.org/cl/73460043
If an error happens on a connection, server goroutine can call b.Logf
after benchmark finishes.
So join both client and server goroutines.
Update #7718
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/84750047
I have no idea what this code is for, but it pretty
clearly needs to be uint64, not uint32.
LGTM=aram
R=0intro, aram
CC=golang-codereviews
https://golang.org/cl/84410043
Native Client forbids jumps/calls to arbitrary locations and
enforces a particular alignement, which makes the Duff's device
ineffective.
LGTM=khr
R=rsc, dave, khr
CC=golang-codereviews
https://golang.org/cl/84400043
Trying to make GODEBUG=gcdead=1 work with liveness
and in particular ambiguously live variables.
1. In the liveness computation, mark all ambiguously live
variables as live for the entire function, except the entry.
They are zeroed directly after entry, and we need them not
to be poisoned thereafter.
2. In the liveness computation, compute liveness (and deadness)
for all parameters, not just pointer-containing parameters.
Otherwise gcdead poisons untracked scalar parameters and results.
3. Fix liveness debugging print for -live=2 to use correct bitmaps.
(Was not updated for compaction during compaction CL.)
4. Correct varkill during map literal initialization.
Was killing the map itself instead of the inserted value temp.
5. Disable aggressive varkill cleanup for call arguments if
the call appears in a defer or go statement.
6. In the garbage collector, avoid bug scanning empty
strings. An empty string is two zeros. The multiword
code only looked at the first zero and then interpreted
the next two bits in the bitmap as an ordinary word bitmap.
For a string the bits are 11 00, so if a live string was zero
length with a 0 base pointer, the poisoning code treated
the length as an ordinary word with code 00, meaning it
needed poisoning, turning the string into a poison-length
string with base pointer 0. By the same logic I believe that
a live nil slice (bits 11 01 00) will have its cap poisoned.
Always scan full multiword struct.
7. In the runtime, treat both poison words (PoisonGC and
PoisonStack) as invalid pointers that warrant crashes.
Manual testing as follows:
- Create a script called gcdead on your PATH containing:
#!/bin/bash
GODEBUG=gcdead=1 GOGC=10 GOTRACEBACK=2 exec "$@"
- Now you can build a test and then run 'gcdead ./foo.test'.
- More importantly, you can run 'go test -short -exec gcdead std'
to run all the tests.
Fixes#7676.
While here, enable the precise scanning of slices, since that was
disabled due to bugs like these. That now works, both with and
without gcdead.
Fixes#7549.
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83410044
runtime.Version() requires a trailing "+" when
tree had local modifications at time of build.
Fixes#7701
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/84040045
The garbage collector poison pointers
(0x6969696969696969 and 0x6868686868686868)
are malformed addresses on amd64.
That is, they are not 48-bit addresses sign extended
to 64 bits. This causes a different kind of hardware fault
than the usual 'unmapped page' when accessing such
an address, and OS X 10.9.2 sends the resulting SIGSEGV
incorrectly, making it look like it was user-generated
rather than kernel-generated and does not include the
faulting address. This means that in GODEBUG=gcdead=1
mode, if there is a bug and something tries to dereference
a poisoned pointer, the runtime delivers the SIGSEGV to
os/signal and returns to the faulting code, which faults
again, causing the process to hang instead of crashing.
Fix by rewriting "user-generated" SIGSEGV on OS X to
look like a kernel-generated SIGSEGV with fault address
0xb01dfacedebac1e.
I chose that address because (1) when printed in hex
during a crash, it is obviously spelling out English text,
(2) there are no current Google hits for that pointer,
which will make its origin easy to find once this CL
is indexed, and (3) it is not an altogether inaccurate
description of the situation.
Add a test. Maybe other systems will break too.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, ken
https://golang.org/cl/83270049
Delaying the runtime.throw until here will print more information.
In particular it will print the signal and code values, which means
it will show the fault address.
The canpanic checks were added recently, in CL 75320043.
They were just not added in exactly the right place.
LGTM=iant
R=dvyukov, iant
CC=golang-codereviews
https://golang.org/cl/83980043
Brad has been asking for this for a while.
I have resisted because I wanted to find a more general way to
do this, one that would keep the performance of code introducing
variables the same as the performance of code that did not.
(See golang.org/issue/3512#c20).
I have not found the more general way, and recent changes to
remove ambiguously live temporaries have blown away the
property I was trying to preserve, so that's no longer a reason
not to make the change.
Fixes#3512.
LGTM=iant
R=iant
CC=bradfitz, golang-codereviews, khr, r
https://golang.org/cl/83740044
Cuts hello world by 70kB, because we don't write
those names into the symbol table.
Update #6853
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/80370045
This is just testing the status quo, so that any future attempt
to change it will make the test break and redirect the person
making the change to look at issue 6027.
Fixes#6027.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/83930046
Supports all the current GNU tar sparse formats, including the
old GNU format and the GNU PAX format versions 0.0, 0.1, and 1.0.
Fixes#3864.
LGTM=rsc
R=golang-codereviews, dave, gobot, dsymonds, rsc
CC=golang-codereviews
https://golang.org/cl/64740043
The software floating point runs with m->locks++
to avoid being preempted; recognize this case in panic
and undo it so that m->locks is maintained correctly
when panicking.
Fixes#7553.
LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews
https://golang.org/cl/84030043
The old limit of 5 was chosen because we didn't actually know how
many bytes of arguments there were; 5 was a halfway point between
printing some useful information and looking ridiculous.
Now we know how many bytes of arguments there are, and we stop
the printing when we reach that point, so the "looking ridiculous" case
doesn't happen anymore: we only print actual argument words.
The cutoff now serves only to truncate very long (but real) argument lists.
In multiple debugging sessions recently (completely unrelated bugs)
I have been frustrated by not seeing more of the long argument lists:
5 words is only 2.5 interface values or strings, and not even 2 slices.
Double the max amount we'll show.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/83850043
The data field is the generic array that acts as a standin
for the keys and values arrays for the generic runtime code.
We want to substitute the keys and values arrays for the data
array, not just add keys and values in addition to it.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/81160044
Darwin 10.6 (gcc 4.2) and some older versions of gcc default to C90 mode, not C99 mode. Silence the warning.
LGTM=aram, iant
R=golang-codereviews, aram, iant
CC=golang-codereviews
https://golang.org/cl/83090050
There is no way to call them from outside the net package.
They are used to implement UCPConn.ReadMsgUDP and similar.
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/83730044
Reduce footprint of liveness bitmaps by about 5x.
1. Mark all liveness bitmap symbols as 4-byte aligned
(they were aligned to a larger size by default).
2. The bitmap data is a bitmap count n followed by n bitmaps.
Each bitmap begins with its own count m giving the number
of bits. All the m's are the same for the n bitmaps.
Emit this bitmap length once instead of n times.
3. Many bitmaps within a function have the same bit values,
but each call site was given a distinct bitmap. Merge duplicate
bitmaps so that no bitmap is written more than once.
4. Many functions end up with the same aggregate bitmap data.
We used to name the bitmap data funcname.gcargs and funcname.gclocals.
Instead, name it gclocals.<md5 of data> and mark it dupok so
that the linker coalesces duplicate sets. This cut the bitmap
data remaining after step 3 by 40%; I was not expecting it to
be quite so dramatic.
Applied to "go build -ldflags -w code.google.com/p/go.tools/cmd/godoc":
bitmaps pclntab binary on disk
before this CL 1326600 1985854 12738268
4-byte align 1154288 (0.87x) 1985854 (1.00x) 12566236 (0.99x)
one bitmap len 782528 (0.54x) 1985854 (1.00x) 12193500 (0.96x)
dedup bitmap 414748 (0.31x) 1948478 (0.98x) 11787996 (0.93x)
dedup bitmap set 245580 (0.19x) 1948478 (0.98x) 11620060 (0.91x)
While here, remove various dead blocks of code from plive.c.
Fixes#6929.
Fixes#7568.
LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83630044
warning: src/cmd/8g/ggen.c:35 non-interruptable temporary
warning: src/cmd/gc/walk.c:656 set and not used: l
warning: src/cmd/gc/walk.c:658 set and not used: l
LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews
https://golang.org/cl/83660043