Catch some "easy to spot" double frees in TCMalloc

Catch some "easy to spot" double frees in TCMalloc

This will only work when the double frees happen to be
close enough in time, and both happen on the same thread...
...but it should have almost zero cost to do the check.


r=jschuh
BUG=75921
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104188

[jar (doing other things), 2011-09-22 17:45:28]

I haven't tested it much yet.... but this seems like an easy way to get pretty
good coverage (especially in the doubly linked list case).

[jschuh, 2011-09-22 17:55:30]

On 2011/09/22 17:45:28, jar wrote:
> I haven't tested it much yet.... but this seems like an easy way to get pretty
> good coverage (especially in the doubly linked list case).

Enthusiastic LGTM, but what's the logic for not using a CHECK here? Because,
this seems like something we'd definitely want to terminate on in release
builds.

[jar (doing other things), 2011-09-22 19:04:44]

On 2011/09/22 17:55:30, Justin Schuh wrote:
> On 2011/09/22 17:45:28, jar wrote:
> > I haven't tested it much yet.... but this seems like an easy way to get
pretty
> > good coverage (especially in the doubly linked list case).
> 
> Enthusiastic LGTM, but what's the logic for not using a CHECK here? Because,
> this seems like something we'd definitely want to terminate on in release
> builds.

<doh>.... I was rushing with other stuff... and recalled I was supposed to use
something "different" here.  I need to get the Production version to compile, to
ensure it is OK, but the doubly-linked set appears to be compiling nicely.

Please comment on the placement of the checks, and I'll refine this CL shortly.

thanks.

[jschuh, 2011-09-28 23:13:11]

Sorry, fell off the end of my queue. To the extent that I understand the code
(which is only partially), it definitely seems like the correct location to
check.

http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromiu...
File third_party/tcmalloc/chromium/src/free_list.cc (right):

http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromiu...
third_party/tcmalloc/chromium/src/free_list.cc:72: void EnusreNonLoop(void*
node, void* next) {
I think you meant EnsureNonLoop.

[jar (doing other things), 2011-09-29 22:30:56]

http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromiu...
File third_party/tcmalloc/chromium/src/free_list.cc (right):

http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromiu...
third_party/tcmalloc/chromium/src/free_list.cc:72: void EnusreNonLoop(void*
node, void* next) {
On 2011/09/28 23:13:11, Justin Schuh wrote:
> I think you meant EnsureNonLoop.

Done.

[jar (doing other things), 2011-10-04 01:46:48]

ping

On Thu, Sep 29, 2011 at 3:30 PM, <jar@chromium.org> wrote:

>
> http://codereview.chromium.**org/8000001/diff/9001/third_**
>
party/tcmalloc/chromium/src/**free_list.cc<http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromium/src/free_list.cc>
> File third_party/tcmalloc/chromium/**src/free_list.cc (right):
>
> http://codereview.chromium.**org/8000001/diff/9001/third_**
>
party/tcmalloc/chromium/src/**free_list.cc#newcode72<http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromium/src/free_list.cc#newcode72>
> third_party/tcmalloc/chromium/**src/free_list.cc:72: void
> EnusreNonLoop(void* node, void* next) {
> On 2011/09/28 23:13:11, Justin Schuh wrote:
>
>> I think you meant EnsureNonLoop.
>>
>
> Done.
>
>
http://codereview.chromium.**org/8000001/<http://codereview.chromium.org/8000...
>

[jschuh, 2011-10-04 01:51:46]

Sorry, must not have clicked submit last time. lgtm.

On 2011/10/04 01:46:48, jar wrote:
> ping
> 
> On Thu, Sep 29, 2011 at 3:30 PM, <mailto:jar@chromium.org> wrote:
> 
> >
> > http://codereview.chromium.**org/8000001/diff/9001/third_**
> >
>
party/tcmalloc/chromium/src/**free_list.cc<http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromium/src/free_list.cc>
> > File third_party/tcmalloc/chromium/**src/free_list.cc (right):
> >
> > http://codereview.chromium.**org/8000001/diff/9001/third_**
> >
>
party/tcmalloc/chromium/src/**free_list.cc#newcode72<http://codereview.chromium.org/8000001/diff/9001/third_party/tcmalloc/chromium/src/free_list.cc#newcode72>
> > third_party/tcmalloc/chromium/**src/free_list.cc:72: void
> > EnusreNonLoop(void* node, void* next) {
> > On 2011/09/28 23:13:11, Justin Schuh wrote:
> >
> >> I think you meant EnsureNonLoop.
> >>
> >
> > Done.
> >
> >
>
http://codereview.chromium.**org/8000001/%3Chttp://codereview.chromium.org/80...>
> >