Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(214)

Issue 71030044: code review 71030044: cmd/ld: clear unused ctxt before morestack (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by rsc
Modified:
11 years ago
Reviewers:
gobot, dvyukov
CC:
khr, dvyukov, golang-codereviews
Visibility:
Public.

Description

cmd/ld: clear unused ctxt before morestack For non-closure functions, the context register is uninitialized on entry and will not be used, but morestack saves it and then the garbage collector treats it as live. This can be a source of memory leaks if the context register points at otherwise dead memory. Avoid this by introducing a parallel set of morestack functions that clear the context register, and use those for the non-closure functions. I hope this will help with some of the finalizer flakiness, but it probably won't. Fixes issue 7244.

Patch Set 1 #

Patch Set 2 : diff -r d187b31342b5 https://code.google.com/p/go/ #

Patch Set 3 : diff -r d187b31342b5 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 234b65f69439 https://code.google.com/p/go/ #

Patch Set 5 : diff -r 363a4ef7e011 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -20 lines) Patch
M include/link.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/5g/gsubr.c View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/6g/gsubr.c View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/8g/gsubr.c View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/gc/closure.c View 1 2 chunks +2 lines, -0 lines 0 comments Download
M src/cmd/gc/go.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/cmd/gc/pgen.c View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/ld/textflag.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/liblink/obj5.c View 1 5 chunks +8 lines, -6 lines 0 comments Download
M src/liblink/obj6.c View 1 7 chunks +18 lines, -8 lines 0 comments Download
M src/liblink/obj8.c View 1 5 chunks +7 lines, -5 lines 0 comments Download
M src/pkg/runtime/asm_386.s View 1 1 chunk +4 lines, -0 lines 0 comments Download
M src/pkg/runtime/asm_amd64.s View 1 1 chunk +40 lines, -0 lines 0 comments Download
M src/pkg/runtime/asm_amd64p32.s View 1 1 chunk +40 lines, -0 lines 0 comments Download
M src/pkg/runtime/asm_arm.s View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 8
rsc
Hello khr (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
11 years ago (2014-03-04 04:57:55 UTC) #1
rsc
As expected, this does not fix the amd64 finalizer flakiness. Still worth doing.
11 years ago (2014-03-04 05:27:48 UTC) #2
dvyukov
LGTM please also test something with -cpuprofile
11 years ago (2014-03-04 05:59:06 UTC) #3
rsc
On Tue, Mar 4, 2014 at 12:59 AM, <dvyukov@google.com> wrote: > LGTM > > please ...
11 years ago (2014-03-04 06:00:52 UTC) #4
dvyukov
On Tue, Mar 4, 2014 at 10:00 AM, Russ Cox <rsc@golang.org> wrote: > On Tue, ...
11 years ago (2014-03-04 06:29:18 UTC) #5
rsc
I take it back. This change was not sufficient to remove the finalizer flakiness but ...
11 years ago (2014-03-04 18:51:45 UTC) #6
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=b57ca889129c *** cmd/ld: clear unused ctxt before morestack For non-closure functions, the ...
11 years ago (2014-03-04 18:53:11 UTC) #7
gobot
11 years ago (2014-03-04 19:01:04 UTC) #8
Message was sent while issue was closed.
This CL appears to have broken the linux-arm-panda builder.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b