koolhazz / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

On OS X, during process exit, the static DebugMallocImplementation object may be used after destruction by mz_size, etc. #581

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Build gperftools with debugallocation.o in play on OS X.

2. Have some file scoped static (lets call it 'owner'), initialized before 
'debug_malloc_implementation' in static construction order, which at some point 
after main starts takes ownership of memory allocated by tcmalloc.

3. At process teardown, static destructors are run in reverse order, so 
debug_malloc_implementation is destroyed first, then the 'owner' object is 
destroyed, and calls 'free' or operator delete on the memory it owns (which was 
allocated by tcmalloc).

4. The OS X zone alloc system calls into mz_size in libc_override_osx.h, which 
makes use of MallocExtension::instance. However the object pointed by the 
result of MallocExtension::instance (the static debug_malloc_implementation 
object) has already been destroyed.

5. mz_size returns zero, apparently, so the deallocation is routed incorrectly, 
leading to a termination with a message like: 'malloc: *** error for object 
0x107a9c6e0: pointer being freed was not allocated'

What is the expected output? What do you see instead?
I would expect the debug allocation subsystem to continue to properly route 
deallocations to the correct zone during process termination.

What version of the product are you using? On what operating system?
gperftools-2.0 on OS X x86_64, statically linked, using the XCode-4.6.3 command 
line tools GCC. Interestingly, the problem does not reproduce when using the 
more modern toolchains on OS X, like the XCode 5 clang. However, this could be 
due to static destruction ordering differences since the project in question 
statically links gperftools.

Please provide any additional information below.
When not using the debugallocator, the MallocExtension subclass is dynamically 
allocated in TCMallocGuard::TCMallocGuard(). However, when using 
debugallocation.cc, the lifetime of the MallocExtension subclass is file 
scoped. As an experiment, I changed debugallocation.cc to also dynamically 
allocate the DebugMallocExtension:

REGISTER_MODULE_INITIALIZER(debugallocation, {
  // Either we or valgrind will control memory management.  We
  // register our extension if we're the winner. Otherwise let
  // Valgrind use its own malloc (so don't register our extension).
  if (!RunningOnValgrind()) {
    MallocExtension::Register(new DebugMallocImplementation);
  }
});

With this change in place, file scoped statics owning memory allocated by 
tcmalloc do not cause a crash on termination. I'm not suggesting that this is 
the proper fix, but it does seem to demonstrate that the premature destruction 
of the DebugMallocImplementation instance is the root cause.

Original issue reported on code.google.com by andrew.m...@10gen.com on 16 Oct 2013 at 8:13

GoogleCodeExporter commented 9 years ago
I should clarify in step 5:

mz_size asks the MallocExtension instance if the provided pointer is owned by 
the MallocExtension. It appears that the destroyed MallocExtension does not 
return kOwned, so mz_size returns zero to its caller, ultimately leading to the 
"pointer being freed was not allocated" message.

Original comment by andrew.m...@10gen.com on 16 Oct 2013 at 8:37

GoogleCodeExporter commented 9 years ago
Attached is a repro against TOT gperftools built on OS X Mountain Lion, using 
XCode 5.

> /usr/bin/g++ -g ./gperftools-581.cpp -I/Users/andrew/dev/opt/include 
-L/Users/andrew/dev/opt/lib -Wl,-all_load 
/Users/andrew/dev/opt/lib/libtcmalloc_minimal_debug.a

There may be a more graceful way to get the static linking done, but it does 
require that the debug library by statically linked, and the -all_load is 
needed as well, since otherwise there are no unresolved symbols to be satisfied.

Running gives:
> ./a.out
Logger CTOR
Logger DTOR
a.out(35969) malloc: *** error for object 0x109bac0c0: pointer being freed was 
not allocated
*** set a breakpoint in malloc_error_break to debug

Running under the debugger:

(gdb) r
Starting program: /Users/andrew/dev/src/experiments/tcmalloc-osx-crash/a.out
Reading symbols for shared libraries ++.............................. done
Logger CTOR
Logger DTOR
a.out(35988) malloc: *** error for object 0x10062c0c0: pointer being freed was 
not allocated
*** set a breakpoint in malloc_error_break to debug

Program received signal SIGABRT, Aborted.
0x00007fff82707d46 in __kill ()
(gdb) bt
#0  0x00007fff82707d46 in __kill ()
#1  0x00007fff8c196f83 in abort ()
#2  0x00007fff8c16a989 in free ()
#3  0x0000000100001bc2 in (anonymous namespace)::Logger::~Logger 
(this=0x1000276c0) at gperftools-581.cpp:19
#4  0x0000000100001b15 in (anonymous namespace)::Logger::~Logger 
(this=0x1000276c0) at gperftools-581.cpp:17
#5  0x00007fff8c198521 in __cxa_finalize ()
#6  0x00007fff8c19a68b in exit ()
#7  0x00007fff8d8e07e8 in start ()

If we set a breakpoint in DebugMallocImplementation::~DebugMallocImplementation 
and then stop in all subsequent calls to mz_size, we can see that the 
implementation of mz_size is obtaining a pointer to the already destroyed 
DebugMallocImplementation:

(gdb) break DebugMallocImplementation::~DebugMallocImplementatino
name of destructor must equal name of class
(gdb) break DebugMallocImplementation::~DebugMallocImplementation
[0] cancel
[1] all
[2] DebugMallocImplementation::~DebugMallocImplementation() at tcmalloc.cc:562
[3] DebugMallocImplementation::~DebugMallocImplementation() at 
src/debugallocation.cc:1005
> 1
Breakpoint 1 at 0x100007903: file tcmalloc.cc, line 562.
Breakpoint 2 at 0x1000078db: file src/debugallocation.cc, line 1005.
warning: Multiple breakpoints were set.
Use the "delete" command to delete unwanted breakpoints.
(gdb) r
Starting program: /Users/andrew/dev/src/experiments/tcmalloc-osx-crash/a.out
Logger CTOR

Breakpoint 2, DebugMallocImplementation::~DebugMallocImplementation 
(this=0x100044ee0) at src/debugallocation.cc:1005
1005    class DebugMallocImplementation : public TCMallocImplementation {
(gdb) print this
$1 = (DebugMallocImplementation * const) 0x100044ee0
(gdb) break mz_size
Breakpoint 3 at 0x1000024dc: file libc_override_osx.h, line 104.
(gdb) c
Continuing.
Logger DTOR

Breakpoint 3, mz_size (zone=0x1000450c0, ptr=0x10062c0c0) at 
libc_override_osx.h:104
104   if (MallocExtension::instance()->GetOwnership(ptr) != 
MallocExtension::kOwned)
(gdb) print MallocExtension::instance()
$2 = (MallocExtension *) 0x100044ee0
(gdb) c
Continuing.
a.out(35991) malloc: *** error for object 0x10062c0c0: pointer being freed was 
not allocated
*** set a breakpoint in malloc_error_break to debug

Program received signal SIGABRT, Aborted.
0x00007fff82707d46 in __kill ()

Original comment by andrew.m...@10gen.com on 17 Oct 2013 at 5:33

Attachments:

GoogleCodeExporter commented 9 years ago
Is there some additional information that I need to provide to get this issue 
accepted (or, I suppose, rejected if I got something above wrong).

Thanks,
Andrew

Original comment by andrew.c.morrow@gmail.com on 29 Oct 2013 at 7:11

GoogleCodeExporter commented 9 years ago
I don't think any more info is needed. More OSX expertise and just more human 
time is needed.

I will eventually get to this ticket, but cannot promise you anything specific.

Original comment by alkondratenko on 29 Oct 2013 at 7:36

GoogleCodeExporter commented 9 years ago
Any hope of seeing a fix for this before the next gperftools release happens 
(whenever that might be)? The change mentioned above of dynamically allocating 
the DebugMallocImplementation does seem to work.

Original comment by andrew.c.morrow@gmail.com on 6 Jan 2014 at 4:13

GoogleCodeExporter commented 9 years ago
I think fix here 
(https://github.com/alk/gperftools/commit/702bb8e2e2c70d4c71fb80bf430cafd5d47c3a
aa) should work. It basically does what you've asked for, except it avoids 
dynamically allocating malloc extension itself.

Please test it on your platform and report if it works. I'm not merging it yet 
and waiting for your feedback.

Original comment by alkondratenko on 11 Jan 2014 at 8:51

GoogleCodeExporter commented 9 years ago
Thank you for the fix. I'm working on trying to repro the original issue so I 
can verify your fix, but having a bit of difficulty due to having upgraded to 
Mavericks / XCode 5 since filing this ticket, and the issue does not appear to 
manifest with clang.

Once I have a repro again I'll try your fix and let you know.

Original comment by andrew.c.morrow@gmail.com on 13 Jan 2014 at 5:05

GoogleCodeExporter commented 9 years ago
OK got my environment issues resolved, and I was able to repro the failure, and 
confirm that your patch would resolve it.

I do have a worry about the patch. I don't see anything that ensures that 
debug_malloc_implementation_space is suitably aligned to hold an instance of 
DebugMallocImplementation.

Original comment by andrew.c.morrow@gmail.com on 13 Jan 2014 at 6:29

GoogleCodeExporter commented 9 years ago
Good point. Thanks for raising it. I'll deal with it as well.

Original comment by alkondratenko on 13 Jan 2014 at 6:31

GoogleCodeExporter commented 9 years ago
fix merged. Thanks.

Original comment by alkondratenko on 18 Jan 2014 at 10:31