[Mac] Use object_destructInstance() if available.

chrome/browser/ui/cocoa/objc_zombie.mm

Index: chrome/browser/ui/cocoa/objc_zombie.mm
diff --git a/chrome/browser/ui/cocoa/objc_zombie.mm b/chrome/browser/ui/cocoa/objc_zombie.mm
index 7dcbfc6b6b6bb79d6ebd63b65b782fe3bf860507..1e82f31bbc3ca8a1d89dd4b8c0d9f411eaa10bd2 100644
--- a/chrome/browser/ui/cocoa/objc_zombie.mm
+++ b/chrome/browser/ui/cocoa/objc_zombie.mm
@@ -33,12 +33,15 @@
 
 namespace {
 
-// |object_cxxDestruct()| is an Objective-C runtime function which
-// traverses the object's class tree for ".cxxdestruct" methods which
-// are run to call C++ destructors as part of |-dealloc|.  The
-// function is not public, so must be looked up using nlist.
+// Function which destroys the contents of an object without freeing
+// the object.  On 10.5 this is |object_cxxDestruct()|, which
+// traverses the class hierarchy to run the C++ destructors.  On 10.6
+// this is |objc_destructInstance()| which calls
+// |object_cxxDestruct()| and removes associative references.
+// |objc_destructInstance()| returns |void*|, pretending it has no
+// return value makes the code simpler.
 typedef void DestructFn(id obj);
-DestructFn* g_object_cxxDestruct = NULL;
+DestructFn* g_objectDestruct = NULL;
 
 // The original implementation for |-[NSObject dealloc]|.
 IMP g_originalDeallocIMP = NULL;
@@ -70,9 +73,9 @@ typedef struct {
 
 ZombieRecord* g_zombies = NULL;
 
-// Lookup the private |object_cxxDestruct| function and return a
-// pointer to it.  Returns |NULL| on failure.
-DestructFn* LookupObjectCxxDestruct() {
+// Lookup the private object destruction function and return a pointer
+// to it.  Returns |NULL| on failure.
+DestructFn* LookupObjectDestruct() {
 #if ARCH_CPU_64_BITS
   // TODO(shess): Port to 64-bit.  I believe using struct nlist_64
   // will suffice.  http://crbug.com/44021 .
@@ -80,20 +83,37 @@ DestructFn* LookupObjectCxxDestruct() {
   return NULL;
 #endif
 
+  // Lookup |objc_destructInstance()| dynamically because it isn't
+  // available on 10.5.
+  const void* addr = reinterpret_cast<void*>(&object_getClass);
+  Dl_info info;
+  if (dladdr(addr, &info)) {
+    void* handle = dlopen(info.dli_fname, RTLD_LAZY|RTLD_LOCAL|RTLD_FIRST);

[Mark Mentovai, 2011/05/31 21:00:57]

Not RTLD_FIRST. RTLD_LAZY | RTLD_LOCAL is fine. Note the space around the
bitwise-or operator.

[Scott Hess - ex-Googler, 2011/06/07 21:24:51]

Done.

+    if (handle) {
+      void* fn = dlsym(handle, "objc_destructInstance");
+      dlclose(handle);

[Mark Mentovai, 2011/05/31 21:00:57]

This is normally dangerous. If you get a pointer from dlsym, it’s only
guaranteed to be valid as long as the handle that you dlopened is. However, the
use here is safe, because your dlopen is just incrementing the reference count
of something that you already know is open, and isn’t going to be unloaded
because it was loaded by dyld in response to a load command.

A brief comment explaining that would be helpful.

[Scott Hess - ex-Googler, 2011/06/07 21:24:51]

Done.

You're making me think fondly about dlsym(RTLD_DEFAULT, "..."), though :-).

+      if (fn)
+        return reinterpret_cast<DestructFn*>(fn);

[Mark Mentovai, 2011/05/31 21:00:57]

This cast is fine.

[Scott Hess - ex-Googler, 2011/06/07 21:24:51]

OK.

+    }
+  }
+
+  // |object_cxxDestruct()| is |__private_extern__| in the runtime, so
+  // there isn't a straight-forward way to find it.
   struct nlist nl[3];
   bzero(&nl, sizeof(nl));
 
-  nl[0].n_un.n_name = (char*)"_object_cxxDestruct";
+  nl[0].n_un.n_name = const_cast<char*>("_object_cxxDestruct");
 
   // My ability to calculate the base for offsets is apparently poor.
   // Use |class_addIvar| as a known reference point.
-  nl[1].n_un.n_name = (char*)"_class_addIvar";
+  nl[1].n_un.n_name = const_cast<char*>("_class_addIvar");
 
   if (nlist("/usr/lib/libobjc.dylib", nl) < 0 ||

[Mark Mentovai, 2011/05/31 21:00:57]

You could maybe use info.dli_fname here too, since you already have it? Or maybe
you won’t already have it if this becomes the 10.5-only codepath and the other
then 10.6-and-up codepath.

[Scott Hess - ex-Googler, 2011/06/07 21:24:51]

I did break the function across the two code paths, since they should never be
shared in real life, but I can abstract that bit out.

       nl[0].n_type == N_UNDF || nl[1].n_type == N_UNDF)
     return NULL;
 
-  return (DestructFn*)((char*)&class_addIvar - nl[1].n_value + nl[0].n_value);
+  return reinterpret_cast<DestructFn*>(
+      reinterpret_cast<char*>(&class_addIvar) - nl[1].n_value + nl[0].n_value);

[Mark Mentovai, 2011/05/31 21:05:05]

Maybe intptr_t is a little more meaningful than char*.

[Scott Hess - ex-Googler, 2011/06/07 21:24:51]

Done.

 }
 
 // Replacement |-dealloc| which turns objects into zombies and places
@@ -109,12 +129,12 @@ void ZombieDealloc(id self, SEL _cmd) {
     return;
   }
 
-  // Use the original |-dealloc| if |object_cxxDestruct| was never
+  // Use the original |-dealloc| if |g_objectDestruct| was never
   // initialized, because otherwise C++ destructors won't be called.
   // This case should be impossible, but doing it wrong would cause
   // terrible problems.
-  DCHECK(g_object_cxxDestruct);
-  if (!g_object_cxxDestruct) {
+  DCHECK(g_objectDestruct);
+  if (!g_objectDestruct) {
     g_originalDeallocIMP(self, _cmd);
     return;
   }
@@ -124,10 +144,11 @@ void ZombieDealloc(id self, SEL _cmd) {
 
   // Destroy the instance by calling C++ destructors and clearing it
   // to something unlikely to work well if someone references it.
-  // NOTE(shess): |object_dispose()| will call |object_cxxDestruct()|
-  // again when the zombie falls off the treadmill!  But by then |isa|
-  // will be something without destructors, so it won't hurt anything.
-  (*g_object_cxxDestruct)(self);
+  // NOTE(shess): |object_dispose()| will call this again when the
+  // zombie falls off the treadmill!  But by then |isa| will be a
+  // class without C++ destructors or associative references, so it
+  // won't hurt anything.
+  (*g_objectDestruct)(self);
   memset(self, '!', size);
 
   // If the instance is big enough, make it into a fat zombie and have
@@ -222,7 +243,7 @@ BOOL ZombieInit() {
 
   Class rootClass = [NSObject class];
 
-  g_object_cxxDestruct = LookupObjectCxxDestruct();
+  g_objectDestruct = LookupObjectDestruct();
   g_originalDeallocIMP =
       class_getMethodImplementation(rootClass, @selector(dealloc));
   // objc_getClass() so CrZombie doesn't need +class.
@@ -230,7 +251,7 @@ BOOL ZombieInit() {
   g_fatZombieClass = objc_getClass("CrFatZombie");
   g_fatZombieSize = class_getInstanceSize(g_fatZombieClass);
 
-  if (!g_object_cxxDestruct || !g_originalDeallocIMP ||
+  if (!g_objectDestruct || !g_originalDeallocIMP ||
       !g_zombieClass || !g_fatZombieClass)
     return NO;