- 07 Jun, 2014 4 commits
-
-
Russ Cox authored
««« CL 102220043 / 00224712f89e runtime: fix panic stack during runtime.Goexit during panic 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 »»» LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, r https://golang.org/cl/108780044
-
Russ Cox authored
««« CL 108740047 / c8e9255aed3f cmd/6g: fix stack zeroing on native client 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 »»» LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, r https://golang.org/cl/107830044
-
Russ Cox authored
««« CL 101980047 / 12c9a9ff50d8 doc: fix happens-before rules for buffered channels The current wording is reversed in 2 places. Not sure how it got 4 LGTMs (mine was there as well). Update #6242. LGTM=dan.kortschak, r, rsc R=golang-codereviews, 0xjnml, dan.kortschak, r, rsc CC=golang-codereviews https://golang.org/cl/101980047 »»» LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, r https://golang.org/cl/106830047
-
Russ Cox authored
««« CL 103080043 / 5e058e21b67d cmd/cgo: for typedef of untagged struct, use typedef name in C code Fixes #8148. LGTM=cookieo9, rsc R=rsc, cookieo9 CC=golang-codereviews https://golang.org/cl/103080043 »»» LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, r https://golang.org/cl/103900046
-
- 05 Jun, 2014 1 commit
-
-
Shenghou Ma authored
[release-branch.go1.3] doc/install-source.html: document that GO386 will be auto-detected when building on both 386 and amd64. ««« CL 102150046 / ccf7893cc2f0 doc/install-source.html: document that GO386 will be auto-detected when building on both 386 and amd64. Fixes #8152. LGTM=iant R=iant CC=golang-codereviews https://golang.org/cl/102150046 »»» LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/105830044
-
- 04 Jun, 2014 2 commits
-
-
David Symonds authored
««« CL 96680045 / 5439c77d4acb debug/elf: support DWARF that needs relocs for 386 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 »»» LGTM=rsc R=rsc CC=golang-codereviews https://golang.org/cl/102140043
-
David Symonds authored
««« CL 103020044 / 318b56ffe04b compress/gzip: allow Reset on Reader without NewReader Fixes #8126. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/103020044 »»» TBR=rsc R=rsc CC=golang-codereviews https://golang.org/cl/105820043
-
- 03 Jun, 2014 8 commits
-
-
Russ Cox authored
««« CL 107740043 / d86ec79a5f30 crypto/tls: fix typo referencing the required Config field Thanks to Frithjof Schulze for noticing. LGTM=adg R=adg CC=agl, golang-codereviews, r https://golang.org/cl/107740043 »»» LGTM=r R=golang-codereviews, r CC=bradfitz, golang-codereviews, iant https://golang.org/cl/103020043
-
Russ Cox authored
««« CL 102040046 / a078b2056ebc cmd/gc: fix escape analysis of func returning indirect of parameter 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 »»» LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/103870043
-
David Symonds authored
««« CL 100930044 / fde405c62fca time: support version 3 zone records Fixes #8134 LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, r, rsc https://golang.org/cl/100930044 »»» LGTM=rsc R=adg, rsc CC=golang-codereviews https://golang.org/cl/96690043
-
David Symonds authored
««« CL 96670046 / 1bec455e95f1 cmd/gc: fix liveness for address-taken variables in inlined functions 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 »»» TBR=adg R=adg CC=golang-codereviews https://golang.org/cl/103810046
-
David Symonds authored
««« CL 100940043 / 93baf7bea171 runtime: fix 1-byte return during x.(T) for 0-byte T 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 »»» TBR=adg CC=golang-codereviews https://golang.org/cl/105760045
-
David Symonds authored
««« CL 104840043 / 876107512a67 cmd/gc: don't generate zillions of linehists for wrapper functions 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 »»» TBR=adg CC=golang-codereviews https://golang.org/cl/102070045
-
David Symonds authored
««« CL 102080043 / 256d975c53cb cmd/cgo: use same Go type for typedef to anonymous struct 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 »»» TBR=adg CC=golang-codereviews https://golang.org/cl/102100043
-
David Symonds authored
««« CL 103810044 / 603f6c3b152c doc: mention WriteHeapDump in 1.3 release notes LGTM=r R=khr, r CC=golang-codereviews https://golang.org/cl/103810044 »»» TBR=adg R=adg CC=golang-codereviews https://golang.org/cl/99700043
-
- 02 Jun, 2014 7 commits
-
-
Andrew Gerrand authored
LGTM=minux R=golang-codereviews, minux CC=golang-codereviews https://golang.org/cl/102920050
-
Andrew Gerrand authored
There's no cmd/link in this branch. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/102050043
-
-
-
Shenghou Ma authored
LGTM=adg R=golang-codereviews, r, adg CC=golang-codereviews https://golang.org/cl/102020045
-
Andrew Gerrand authored
LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/102040047
-
Brad Fitzpatrick authored
Update #8112 LGTM=adg R=adg CC=golang-codereviews https://golang.org/cl/104790045
-
- 01 Jun, 2014 7 commits
-
-
Shenghou Ma authored
Fix plan 9 build. TBR=rsc R=golang-codereviews CC=golang-codereviews https://golang.org/cl/100880047
-
Russ Cox authored
Fixes #7452. LGTM=minux, iant R=minux, iant CC=golang-codereviews https://golang.org/cl/104770046
-
Andrew Gerrand authored
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
-
Andrew Gerrand authored
TBR=rsc R=golang-codereviews CC=golang-codereviews https://golang.org/cl/106730043
-
Andrew Gerrand authored
This program has barely been touched since it was first committed, and in its current state it opens a code execution vector similar to the one that was recently fixed in go.tools/playground/socket. Rather than try to make it secure, remove it. LGTM=minux, rsc R=rsc, minux CC=golang-codereviews https://golang.org/cl/102030047
-
Russ Cox authored
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
-
Rob Pike authored
LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/102040045
-
- 31 May, 2014 8 commits
-
-
Keith Randall authored
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
-
Russ Cox authored
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
-
Russ Cox authored
Fixes #8102. LGTM=bradfitz, dvyukov R=golang-codereviews, bradfitz, dvyukov CC=golang-codereviews https://golang.org/cl/100870046
-
Russ Cox authored
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
-
Shenghou Ma authored
TestWriteHeap is useless on NaCl anyway. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/101980048
-
Shenghou Ma authored
LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://golang.org/cl/102920047
-
Shenghou Ma authored
Fixes #8119. LGTM=khr, rsc R=alex.brainman, khr, bradfitz, rsc CC=golang-codereviews https://golang.org/cl/93640044
-
Shenghou Ma authored
Fixes #7958. LGTM=rsc R=golang-codereviews, rsc, r, gobot CC=golang-codereviews https://golang.org/cl/91520043
-
- 30 May, 2014 2 commits
-
-
Russ Cox authored
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 65000655 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
-
Ian Lance Taylor authored
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
-
- 29 May, 2014 1 commit
-
-
Russ Cox authored
[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
-