Partial undo, just of go:nosplit annotation. Somehow it
is breaking the windows builders.
TBR=bradfitz
««« original CL description
runtime: implement string ops in Go
Also implement go:nosplit annotation. Not really needed
for now, but we'll definitely need it for other conversions.
benchmark old ns/op new ns/op delta
BenchmarkRuneIterate 534 474 -11.24%
BenchmarkRuneIterate2 535 470 -12.15%
LGTM=bradfitz
R=golang-codereviews, dave, bradfitz, minux
CC=golang-codereviews
https://golang.org/cl/93380044
»»»
TBR=bradfitz
CC=golang-codereviews
https://golang.org/cl/105260044
Also implement go:nosplit annotation. Not really needed
for now, but we'll definitely need it for other conversions.
benchmark old ns/op new ns/op delta
BenchmarkRuneIterate 534 474 -11.24%
BenchmarkRuneIterate2 535 470 -12.15%
LGTM=bradfitz
R=golang-codereviews, dave, bradfitz, minux
CC=golang-codereviews
https://golang.org/cl/93380044
We don't need to shift array elements to shuffle them.
We just have to swap a selected element with 0th element.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/91750044
Printf("%x", "abc") was "0x610x620x63"; is now "0x616263", which
is surely better.
Printf("% #x", "abc") is still "0x61 0x62 0x63".
Fixes#8080.
LGTM=bradfitz, gri
R=golang-codereviews, bradfitz, gri
CC=golang-codereviews
https://golang.org/cl/106990043
Also added a test to verify os.Getppid() works across all platforms
LGTM=alex.brainman
R=golang-codereviews, alex.brainman, shreveal, iant
CC=golang-codereviews
https://golang.org/cl/102320044
Reportedly in the Linux 3.16 kernel the VDSO will not have
section headers or a normal symbol table.
Too late for 1.3 but perhaps for 1.3.1, if there is one.
Fixes#8197.
LGTM=rsc
R=golang-codereviews, mattn.jp, rsc
CC=golang-codereviews
https://golang.org/cl/101260044
bufio.Scanner.Scan returns whether the scan succeeded, not whether it
is done, so the test was mistakenly breaking early.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/93670045
makes windows-amd64-race benchmarks slower
««« original CL description
testing: make benchmarking faster
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
»»»
TBR=josharian
CC=golang-codereviews
https://golang.org/cl/105950044
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