TCMalloc_PageMap2 attempts to access null--crashes

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Rare condition where memory was already allocated by the application 
but is incorrectly construed as belonging to TCMalloc.
2. The call to void *get(Number k) const (line 135) returns root_[i1]-
>values[i2] without ensuring root_[i1]->values is not null.

What is the expected output? What do you see instead?
Expected that the function return null if root_[i1]->values is null.  
Instead it dereferences null. 
Changed the line to:
return root_[i1]->values ? root_[i1]->values[i2] : NULL; 

What version of the product are you using? On what operating system?
perftools 1.3

Please provide any additional information below.

Original issue reported on code.google.com by escri...@hotmail.com on 21 Jul 2009 at 2:37

[GoogleCodeExporter]

Can you give a backtrace of where this is happening?  Is it in a free() call?

One issue is that this get() call is in the fast path to most memory 
operations, and
I'm not sure how much it will slow things down (we not only have to check for 
NULL,
we have to check if i1 is valid).  So if there is a more targetted fix, it 
might be
better.  I'll ask around here to see if folks have any ideas.

Original comment by csilv...@gmail.com on 22 Jul 2009 at 7:21

• Added labels: Priority-Medium, Type-Defect

[GoogleCodeExporter]

Yes, it was definitely in a free call.  It looked like the fallback was taken 
fine 
when NULL was returned.

Original comment by escri...@hotmail.com on 23 Jul 2009 at 6:08

[GoogleCodeExporter]

OK, I have a fix that should help in this case, and will be out in the next 
release.

Original comment by csilv...@gmail.com on 10 Aug 2009 at 6:36

• Changed state: Started

[GoogleCodeExporter]

Hi,
I am having similar problem when using tcmalloc as a static lib (linked with 
static
crt) - on win32.
in my case root_ itself is NULL, so the solution above is not enough.
Note that this crash happens only when I am using tcmalloc as a lib and not 
when I am
using it as a dll (with dynamic link to the crt)

Is it possible to post the patch here so I can test it and see if it fix the 
crash?
Thanks.

Original comment by jontra@gmail.com on 31 Aug 2009 at 1:58

[GoogleCodeExporter]

Hmm, I don't think my changes will help if root_ is NULL -- they basically just 
check
to see if i is out of bounds before calling root_[i].  But they don't check 
root_ itself.

I'm looking at the code trying to figure out how root_ can be NULL.  On a 32 bit
system, it seems like it should be impossible.  If you can get this under a 
debugger,
can you see what the type of root_ is?  And what class it belongs to:
TCMalloc_PageMap2 or TCMalloc_PageMap3?

Original comment by csilv...@gmail.com on 31 Aug 2009 at 5:41

[GoogleCodeExporter]

My bad.
root_ is not NULL, but root_[i1] when i1==3 is NULL;
i changed the line to:
   return root_[i1] ? root_[i1]->values[i2] : NULL;
Maybe we should check root_[i1] AND values for NULL, so the line should be:
   return !root_[i1] || !root_[i1]->values ? NULL : root_[i1]->values[i2];

Both versions work for me perfectly.

This is the call stack:
test.exe!TCMalloc_PageMap2<20>::get(unsigned int k=101728)  Line 136 + 0xc bytes
test.exe!tcmalloc::PageHeap::GetDescriptor(unsigned int p=101728)  Line 116
test.exe!`anonymous namespace'::do_free_with_callback(void * ptr=0x18d60068, 
void
(void *)* invalid_free_fn=0x00b303e0)  Line 828 + 0x10 bytes
test.exe!`anonymous
namespace'::LibcInfoWithPatchFunctions<0>::Perftools_deletearray_nothrow(void *
p=0x18d60068, const std::nothrow_t & __formal={...})  Line 725 + 0x10 bytes 
test.exe!str_free(char * * l=0x00b31108)  Line 253 + 0xb bytes  C

Original comment by jontra@gmail.com on 1 Sep 2009 at 6:21

[GoogleCodeExporter]

OK, great, that's similar to the patch I have, so looks like this should work 
well
for you.  Glad you got it fixed up.

Original comment by csilv...@gmail.com on 1 Sep 2009 at 3:21

[GoogleCodeExporter]

This should be fixed in perftools 1.4, just released.

Original comment by csilv...@gmail.com on 11 Sep 2009 at 6:57

• Changed state: Fixed