summaryrefslogtreecommitdiff
path: root/test
Commit message (Collapse)AuthorAgeFilesLines
* liblink: make GO_ARGS the default for functions beginning with ?Russ Cox2014-09-161-2/+11
| | | | | | | | | | | | | | | | | If there is a leading ?, assume there is a Go prototype and attach the Go prototype information to the function. If the function is not called from Go and does not need a Go prototype, it can be made file-local instead (using name<>(SB)). This fixes the current BSD build failures, by giving functions like sync/atomic.StoreUint32 argument stack map information. Fixes issue 8753. LGTM=khr, iant R=golang-codereviews, iant, khr, bradfitz CC=golang-codereviews, r, rlh https://codereview.appspot.com/142150043
* cmd/gc: say 'non-constant array bound' instead of 'invalid array bound'Russ Cox2014-09-161-7/+12
| | | | | | | | | Fixes issue 8196. LGTM=adonovan R=adonovan CC=golang-codereviews https://codereview.appspot.com/141510044
* cmd/ld: document that -X overwrites initialized variablesJosh Bleecher Snyder2014-09-151-2/+6
| | | | | | | | | Fixes issue 7626. LGTM=iant R=rsc, iant CC=golang-codereviews https://codereview.appspot.com/144870045
* test: make maplinear iterdelete test less flakyJosh Bleecher Snyder2014-09-151-9/+13
| | | | | | | | | | | | | iterdelete's run time varies; occasionally we get unlucky. To reduce spurious failures, average away some of the variation. On my machine, 8 of 5000 runs (0.15%) failed before this CL. After this CL, there were no failures after 35,000 runs. I confirmed that this adjusted test still fails before CL 141270043. LGTM=khr R=khr CC=bradfitz, golang-codereviews https://codereview.appspot.com/140610043
* cmd/gc: don't walk static nodes generated by anylit.R?my Oudompheng2014-09-151-0/+26
| | | | | | | | | | | | | | | | | | | During anylit run, nodes such as SLICEARR(statictmp, [:]) may be generated and are expected to be found unchanged by gen_as_init. In some walks (in particular walkselect), the statement may be walked again and lowered to its usual form, leading to a crash. Fixes issue 8017. Fixes issue 8024. Fixes issue 8058. LGTM=rsc R=golang-codereviews, dvyukov, gobot, rsc CC=golang-codereviews https://codereview.appspot.com/112080043
* cmd/gc: generate type alg after calling dowidth.R?my Oudompheng2014-09-153-0/+29
| | | | | | | | | | | | Previously it might happen before calling dowidth and result in a compiler crash. Fixes issue 8060. LGTM=dvyukov, rsc R=golang-codereviews, dvyukov, gobot, rsc CC=golang-codereviews https://codereview.appspot.com/110980044
* test: return errors earlier in run.goJosh Bleecher Snyder2014-09-111-0/+4
| | | | | | | | | Fixes issue 8184. LGTM=bradfitz R=bradfitz CC=golang-codereviews https://codereview.appspot.com/137510043
* cmd/gc: emit write barriersRuss Cox2014-09-112-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A write *p = x that needs a write barrier (not all do) now turns into runtime.writebarrierptr(p, x) or one of the other variants. The write barrier implementations are trivial. The goal here is to emit the calls in the correct places and to incur the cost of those function calls in the Go 1.4 cycle. Performance on the Go 1 benchmark suite below. Remember, the goal is to slow things down (and be correct). We will look into optimizations in separate CLs, as part of the process of comparing Go 1.3 against tip in order to make sure Go 1.4 runs at least as fast as Go 1.3. benchmark old ns/op new ns/op delta BenchmarkBinaryTree17 3118336716 3452876110 +10.73% BenchmarkFannkuch11 3184497677 3211552284 +0.85% BenchmarkFmtFprintfEmpty 89.9 107 +19.02% BenchmarkFmtFprintfString 236 287 +21.61% BenchmarkFmtFprintfInt 246 278 +13.01% BenchmarkFmtFprintfIntInt 395 458 +15.95% BenchmarkFmtFprintfPrefixedInt 343 378 +10.20% BenchmarkFmtFprintfFloat 477 525 +10.06% BenchmarkFmtManyArgs 1446 1707 +18.05% BenchmarkGobDecode 14398047 14685958 +2.00% BenchmarkGobEncode 12557718 12947104 +3.10% BenchmarkGzip 453462345 472413285 +4.18% BenchmarkGunzip 114226016 115127398 +0.79% BenchmarkHTTPClientServer 114689 112122 -2.24% BenchmarkJSONEncode 24914536 26135942 +4.90% BenchmarkJSONDecode 86832877 103620289 +19.33% BenchmarkMandelbrot200 4833452 4898780 +1.35% BenchmarkGoParse 4317976 4835474 +11.98% BenchmarkRegexpMatchEasy0_32 150 166 +10.67% BenchmarkRegexpMatchEasy0_1K 393 402 +2.29% BenchmarkRegexpMatchEasy1_32 125 142 +13.60% BenchmarkRegexpMatchEasy1_1K 1010 1236 +22.38% BenchmarkRegexpMatchMedium_32 232 301 +29.74% BenchmarkRegexpMatchMedium_1K 76963 102721 +33.47% BenchmarkRegexpMatchHard_32 3833 5463 +42.53% BenchmarkRegexpMatchHard_1K 119668 161614 +35.05% BenchmarkRevcomp 763449047 706768534 -7.42% BenchmarkTemplate 124954724 134834549 +7.91% BenchmarkTimeParse 517 511 -1.16% BenchmarkTimeFormat 501 514 +2.59% benchmark old MB/s new MB/s speedup BenchmarkGobDecode 53.31 52.26 0.98x BenchmarkGobEncode 61.12 59.28 0.97x BenchmarkGzip 42.79 41.08 0.96x BenchmarkGunzip 169.88 168.55 0.99x BenchmarkJSONEncode 77.89 74.25 0.95x BenchmarkJSONDecode 22.35 18.73 0.84x BenchmarkGoParse 13.41 11.98 0.89x BenchmarkRegexpMatchEasy0_32 213.30 191.72 0.90x BenchmarkRegexpMatchEasy0_1K 2603.92 2542.74 0.98x BenchmarkRegexpMatchEasy1_32 254.00 224.93 0.89x BenchmarkRegexpMatchEasy1_1K 1013.53 827.98 0.82x BenchmarkRegexpMatchMedium_32 4.30 3.31 0.77x BenchmarkRegexpMatchMedium_1K 13.30 9.97 0.75x BenchmarkRegexpMatchHard_32 8.35 5.86 0.70x BenchmarkRegexpMatchHard_1K 8.56 6.34 0.74x BenchmarkRevcomp 332.92 359.62 1.08x BenchmarkTemplate 15.53 14.39 0.93x LGTM=rlh R=rlh CC=dvyukov, golang-codereviews, iant, khr, r https://codereview.appspot.com/136380043
* runtime: add timing test for iterate/delete map idiom.Keith Randall2014-09-101-0/+18
| | | | | | | LGTM=bradfitz, iant R=iant, bradfitz CC=golang-codereviews https://codereview.appspot.com/140510043
* build: adjustments for move from src/pkg to srcRuss Cox2014-09-082-2/+2
| | | | | | | | | | | | | | | | | | | This CL adjusts code referring to src/pkg to refer to src. Immediately after submitting this CL, I will submit a change doing 'hg mv src/pkg/* src'. That change will be too large to review with Rietveld but will contain only the 'hg mv'. This CL will break the build. The followup 'hg mv' will fix it. For more about the move, see golang.org/s/go14nopkg. LGTM=r R=r CC=golang-codereviews https://codereview.appspot.com/134570043
* runtime: increase stack split limit againRuss Cox2014-09-061-2/+2
| | | | | | | | | | | | | | | | | | | | | Increase NOSPLIT reservation from 192 to 384 bytes. The problem is that the non-Unix systems (Solaris and Windows) just can't make system calls in a small amount of space, and then worse they do things that are complex enough to warrant calling runtime.throw on failure. We don't have time to rewrite the code to use less stack. I'm not happy about this, but it's still a small amount. The good news is that we're doing this to get to only using copying stacks for stack growth. Once that is true, we can drop the default stack size from 8k to 4k, which should more than make up for the bytes we're losing here. LGTM=r R=iant, r, bradfitz, aram.h CC=golang-codereviews https://codereview.appspot.com/140350043
* runtime: fix panic/wrapper/recover mathRuss Cox2014-09-061-2/+46
| | | | | | | | | | | | | | | | | | The gp->panicwrap adjustment is just fatally flawed. Now that there is a Panic.argp field, update that instead. That can be done on entry only, so that unwinding doesn't need to worry about undoing anything. The wrappers emit a few more instructions in the prologue but everything else in the system gets much simpler. It also fixes (without trying) a broken test I never checked in. Fixes issue 7491. LGTM=khr R=khr CC=dvyukov, golang-codereviews, iant, r https://codereview.appspot.com/135490044
* runtime: use reflect.call during panic instead of newstackcallRuss Cox2014-09-052-4/+4
| | | | | | | | | | newstackcall creates a new stack segment, and we want to be able to throw away all that code. LGTM=khr R=khr, iant CC=dvyukov, golang-codereviews, r https://codereview.appspot.com/139270043
* runtime: convert panic/recover to GoKeith Randall2014-09-051-1/+1
| | | | | | | | | | | | | created panic1.go just so diffs were available. After this CL is in, I'd like to move panic.go -> defer.go and panic1.go -> panic.go. LGTM=rsc R=rsc, khr CC=golang-codereviews https://codereview.appspot.com/133530045 Committer: Russ Cox <rsc@golang.org>
* runtime: increase nosplit area to 192Russ Cox2014-08-301-2/+2
| | | | | | | | | | | | | | | | | | | | In CL 131450043, which raised it to 160, I'd raise it to 192 if necessary. Apparently it is necessary on windows/amd64. One note for those concerned about the growth: in the old segmented stack world, we wasted this much space at the bottom of every stack segment. In the new contiguous stack world, each goroutine has only one stack segment, so we only waste this much space once per goroutine. So even raising the limit further might still be a net savings. Fixes windows/amd64 build. TBR=r CC=golang-codereviews https://codereview.appspot.com/132480043
* test: add test that caused gccgo to crash on valid codeIan Lance Taylor2014-08-281-0/+34
| | | | | | | | | Update issue 8612 LGTM=minux R=golang-codereviews, minux CC=golang-codereviews https://codereview.appspot.com/135170043
* runtime: give nosplit functions 32 more bytes of headroomRuss Cox2014-08-271-1/+9
| | | | | | | | | | | | | | | | | | | | | The Go calling convention uses more stack space than C. On 64-bit systems we've been right up against the limit (128 bytes, so only 16 words) and doing awful things to our source code to work around it. Instead of continuing to do awful things, raise the limit to 160 bytes. I am prepared to raise the limit to 192 bytes if necessary, but I think this will be enough. Should fix current link-time stack overflow errors on - nacl/arm - netbsd/amd64 - openbsd/amd64 - solaris/amd64 - windows/amd64 TBR=r CC=golang-codereviews, iant https://codereview.appspot.com/131450043
* cmd/gc, runtime: treat slices and strings like pointers in garbage collectionRuss Cox2014-08-252-7/+101
| | | | | | | | | | | | | | | | | | | Before, a slice with cap=0 or a string with len=0 might have its base pointer pointing beyond the actual slice/string data into the next block. The collector had to ignore slices and strings with cap=0 in order to avoid misinterpreting the base pointer. Now, a slice with cap=0 or a string with len=0 still has a base pointer pointing into the actual slice/string data, no matter what. The collector can now always scan the pointer, which means strings and slices are no longer special. Fixes issue 8404. LGTM=khr, josharian R=josharian, khr, dvyukov CC=golang-codereviews https://codereview.appspot.com/112570044
* cmd/gc: fix order of channel evaluation of receive channelsRuss Cox2014-08-251-0/+29
| | | | | | | | | | | | | | | | | | | Normally, an expression of the form x.f or *y can be reordered with function calls and communications. Select is stricter than normal: each channel expression is evaluated in source order. If you have case <-x.f and case <-foo(), then if the evaluation of x.f causes a panic, foo must not have been called. (This is in contrast to an expression like x.f + foo().) Enforce this stricter ordering. Fixes issue 8336. LGTM=dvyukov R=golang-codereviews, dvyukov CC=golang-codereviews, r https://codereview.appspot.com/126570043
* cmd/gc, runtime: refactor interface inlining decision into compilerRuss Cox2014-08-181-24/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | We need to change the interface value representation for concurrent garbage collection, so that there is no ambiguity about whether the data word holds a pointer or scalar. This CL does NOT make any representation changes. Instead, it removes representation assumptions from various pieces of code throughout the tree. The isdirectiface function in cmd/gc/subr.c is now the only place that decides that policy. The policy propagates out from there in the reflect metadata, as a new flag in the internal kind value. A follow-up CL will change the representation by changing the isdirectiface function. If that CL causes problems, it will be easy to roll back. Update issue 8405. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, r https://codereview.appspot.com/129090043
* cmd/gc: disallow pointer constantsMatthew Dempsky2014-08-151-0/+25
| | | | | | | | | | | Fixes issue 7760. LGTM=iant R=iant, remyoudompheng CC=golang-codereviews https://codereview.appspot.com/130720043 Committer: Ian Lance Taylor <iant@golang.org>
* cmd/gc: comma-ok assignments produce untyped bool as 2nd resultChris Manghane2014-08-112-4/+29
| | | | | | | LGTM=rsc R=gri, rsc CC=golang-codereviews https://codereview.appspot.com/127950043
* cmd/6g, cmd/8g: fix, test byte-sized magic multiplyRuss Cox2014-08-111-0/+31
| | | | | | | | | | | Credit to R?my for finding and writing test case. Fixes issue 8325. LGTM=r R=golang-codereviews, r CC=dave, golang-codereviews, iant, remyoudompheng https://codereview.appspot.com/124950043
* test: add another test case that gccgo crashed onIan Lance Taylor2014-08-081-0/+16
| | | | | | | LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://codereview.appspot.com/124020044
* test/mapnan.go: add regression test for non-empty interfaces.Alan Donovan2014-08-063-57/+144
| | | | | | | LGTM=rsc, khr R=rsc, khr, bradfitz CC=golang-codereviews https://codereview.appspot.com/126720043
* test: add test for function type in function literalIan Lance Taylor2014-08-041-0/+22
| | | | | | | | | | | The gccgo compiler used to fail this test. This was the root cause of http://gcc.gnu.org/PR61308 . The fix for the gccgo compiler is http://codereview.appspot.com/122020043 . LGTM=dave, bradfitz R=golang-codereviews, dave, bradfitz CC=golang-codereviews https://codereview.appspot.com/121200043
* test/run: go fmtDavid du Colombier2014-08-011-15/+15
| | | | | | | LGTM=josharian, r R=golang-codereviews, josharian, r CC=golang-codereviews https://codereview.appspot.com/120160043
* runtime: move built-in print routines to go.Keith Randall2014-07-312-0/+66
| | | | | | | | | Fixes issue 8297 LGTM=bradfitz R=golang-codereviews, bradfitz, khr, dave, dvyukov CC=golang-codereviews https://codereview.appspot.com/119240043
* runtime: rewrite malloc in Go.Keith Randall2014-07-301-5/+5
| | | | | | | | | | | | | This change introduces gomallocgc, a Go clone of mallocgc. Only a few uses have been moved over, so there are still lots of uses from C. Many of these C uses will be moved over to Go (e.g. in slice.goc), but probably not all. What should remain of C's mallocgc is an open question. LGTM=rsc, dvyukov R=rsc, khr, dave, bradfitz, dvyukov CC=golang-codereviews https://codereview.appspot.com/108840046
* test/run: always set goos and goarchDavid du Colombier2014-07-241-2/+11
| | | | | | | | | | | | | | | | | | Following CL 68150047, the goos and goarch variables are not currently set when the GOOS and GOARCH environment variables are not set. This made the content of the build tag to be ignored in this case. This CL sets goos and goarch to runtime.GOOS and runtime.GOARCH when the GOOS and GOARCH environments variables are not set. LGTM=aram, bradfitz R=golang-codereviews, aram, gobot, rsc, dave, bradfitz CC=golang-codereviews, rsc https://codereview.appspot.com/112490043
* test: avoid "declared but not used" errors in shift1.goIan Lance Taylor2014-07-201-0/+2
| | | | | | | | | | | | | | | I'm improving gccgo's detection of variables that are only set but not used, and it triggers additional errors on this code. The new gccgo errors are correct; gc seems to suppress them due to the other, expected, errors. This change uses the variables so that no compiler will complain. gccgo change is http://codereview.appspot.com/119920043 . LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://codereview.appspot.com/116050043
* test: add test for confusion with dot importsIan Lance Taylor2014-07-203-0/+28
| | | | | | | | | | The gccgo compiler would fail this test. The fix for gccgo is http://codereview.appspot.com/116960043 . LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://codereview.appspot.com/118000043
* cmd/gc: allocate select descriptor on stackDmitriy Vyukov2014-07-201-3/+3
| | | | | | | | | | | | | | | | | benchmark old ns/op new ns/op delta BenchmarkSelectUncontended 220 165 -25.00% BenchmarkSelectContended 209 161 -22.97% BenchmarkSelectProdCons 1042 904 -13.24% But more importantly this change will allow to get rid of free function in runtime. Fixes issue 6494. LGTM=rsc, khr R=golang-codereviews, rsc, dominik.honnef, khr CC=golang-codereviews, remyoudompheng https://codereview.appspot.com/107670043
* test: add some tests for mismatches between call results and usesIan Lance Taylor2014-07-191-0/+24
| | | | | | | LGTM=dvyukov R=golang-codereviews, dvyukov CC=golang-codereviews https://codereview.appspot.com/111360045
* test: add test for issue8347Shenghou Ma2014-07-181-0/+27
| | | | | | | | | Fixes issue 8347. LGTM=dave R=golang-codereviews, dave CC=golang-codereviews https://codereview.appspot.com/109600044
* cmd/gc: implement 'for range x {'Russ Cox2014-07-166-1/+102
| | | | | | | | | Fixes issue 6102. LGTM=gri R=ken, r, gri CC=golang-codereviews https://codereview.appspot.com/113120043
* test: add test for gccgo comment lexing failureIan Lance Taylor2014-07-081-0/+14
| | | | | | | | | | | http://gcc.gnu.org/PR61746 http://code.google.com/p/gofrontend/issues/detail?id=35 LGTM=crawshaw R=golang-codereviews, crawshaw CC=golang-codereviews https://codereview.appspot.com/111980043
* cmd/8g: don't allocate a register early for cap(CHAN).R?my Oudompheng2014-07-011-0/+7
| | | | | | | | | | | | There is no reason to generate different code for cap and len. Fixes issue 8025. Fixes issue 8026. LGTM=rsc R=rsc, iant, khr CC=golang-codereviews https://codereview.appspot.com/93570044
* test/fixedbugs: fix typo in commentDave Cheney2014-06-291-1/+1
| | | | | | | | | Fix copy paste error pointed out by rsc, https://codereview.appspot.com/107290043/diff/60001/test/fixedbugs/issue8074.go#newcode7 LGTM=ruiu, r R=golang-codereviews, ruiu, r CC=golang-codereviews https://codereview.appspot.com/106210047
* cmd/gc: drop parenthesization restriction for receiver typesRuss Cox2014-06-251-4/+6
| | | | | | | | | Matches CL 101500044. LGTM=gri R=gri CC=golang-codereviews https://codereview.appspot.com/110160044
* test: add test case for issue 8074.Dave Cheney2014-06-221-0/+16
| | | | | | | | | | | | | | | Fixes issue 8074. The issue was not reproduceable by revision go version devel +e0ad7e329637 Thu Jun 19 22:19:56 2014 -0700 linux/arm But include the original test case in case the issue reopens itself. LGTM=dvyukov R=golang-codereviews, dvyukov CC=golang-codereviews https://codereview.appspot.com/107290043
* test: speed up chan/select5Josh Bleecher Snyder2014-06-171-3/+3
| | | | | | | | | | | | | No functional changes. Generating shorter functions improves compilation time. On my laptop, this test's running time goes from 5.5s to 1.5s; the wall clock time to run all tests goes down 1s. On Raspberry Pi, this CL cuts 50s off the wall clock time to run all tests. Fixes issue 7503. LGTM=bradfitz R=golang-codereviews, bradfitz CC=golang-codereviews https://codereview.appspot.com/72590045
* runtime: fix defer of nil funcRuss Cox2014-06-121-0/+22
| | | | | | | | | Fixes issue 8047. LGTM=r, iant R=golang-codereviews, r, iant CC=dvyukov, golang-codereviews, khr https://codereview.appspot.com/105140044
* runtime: add test for issue 8047.Keith Randall2014-06-111-0/+29
| | | | | | | | | | | | | | | Make sure stack copier doesn't barf on a nil defer. Bug was fixed in https://codereview.appspot.com/101800043 This change just adds a test. Fixes issue 8047 LGTM=dvyukov, rsc R=dvyukov, rsc CC=golang-codereviews https://codereview.appspot.com/108840043 Committer: Russ Cox <rsc@golang.org>
* cmd/gc: fix &result escaping into resultRuss Cox2014-06-111-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 issue 8185. LGTM=iant R=iant CC=golang-codereviews https://codereview.appspot.com/108870044
* cmd/gc: fix escape analysis for &x inside switch x := v.(type)Russ Cox2014-06-111-0/+10
| | | | | | | | | | | | | | 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 issue 8176. LGTM=iant R=iant CC=golang-codereviews https://codereview.appspot.com/108860043
* runtime: fix panic stack during runtime.Goexit during panicRuss Cox2014-06-061-0/+41
| | | | | | | | | | | | | | | | | | | | | | | 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 issue 8158. LGTM=iant, dvyukov R=iant, dvyukov CC=golang-codereviews https://codereview.appspot.com/102220043
* cmd/6g: fix stack zeroing on native clientRuss Cox2014-06-051-0/+48
| | | | | | | | | | | | | | | | | | | | | | | | | | 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 issue 8155. LGTM=r, iant R=golang-codereviews, r, iant CC=dvyukov, golang-codereviews, khr https://codereview.appspot.com/108740047
* cmd/gc: fix escape analysis of func returning indirect of parameterRuss Cox2014-06-031-3/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 issue 8120. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, khr, r https://codereview.appspot.com/102040046
* cmd/gc: fix liveness for address-taken variables in inlined functionsRuss Cox2014-06-022-0/+71
| | | | | | | | | | | 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://codereview.appspot.com/96670046
View Lorry Controller Status