Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(682)

Unified Diff: chrome/browser/ui/cocoa/objc_zombie.mm

Issue 7084017: [Mac] Use object_destructInstance() if available. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Avi grammar nits. Created 9 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/chrome_browser_application_mac.mm ('k') | chrome/browser/ui/cocoa/objc_zombie_unittest.mm » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..d90be0702ca1ccdf8512b3a51fc9ea99f9c02a0a 100644
--- a/chrome/browser/ui/cocoa/objc_zombie.mm
+++ b/chrome/browser/ui/cocoa/objc_zombie.mm
@@ -11,7 +11,9 @@
#import <objc/objc-class.h>
#include "base/logging.h"
+#include "base/metrics/histogram.h"
#include "base/synchronization/lock.h"
+#include "base/sys_info.h"
#import "chrome/app/breakpad_mac.h"
#import "chrome/browser/ui/cocoa/objc_method_swizzle.h"
@@ -33,12 +35,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*| but 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,30 +75,69 @@ typedef struct {
ZombieRecord* g_zombies = NULL;
-// Lookup the private |object_cxxDestruct| function and return a
-// pointer to it. Returns |NULL| on failure.
-DestructFn* LookupObjectCxxDestruct() {
-#if ARCH_CPU_64_BITS
- // TODO(shess): Port to 64-bit. I believe using struct nlist_64
- // will suffice. http://crbug.com/44021 .
- NOTIMPLEMENTED();
+const char* LookupObjcRuntimePath() {
+ const void* addr = reinterpret_cast<void*>(&object_getClass);
+ Dl_info info;
+
+ // |dladdr()| doesn't document how long |info| will stay valid...
+ if (dladdr(addr, &info))
+ return info.dli_fname;
+
return NULL;
-#endif
+}
+
+// Lookup |objc_destructInstance()| dynamically because it isn't
+// available on 10.5, but we link with the 10.5 SDK.
+DestructFn* LookupObjectDestruct_10_6() {
+ const char* objc_runtime_path = LookupObjcRuntimePath();
+ if (!objc_runtime_path)
+ return NULL;
+
+ void* handle = dlopen(objc_runtime_path, RTLD_LAZY | RTLD_LOCAL);
+ if (!handle)
+ return NULL;
+
+ void* fn = dlsym(handle, "objc_destructInstance");
+
+ // |fn| would normally be expected to become invalid after this
+ // |dlclose()|, but since the |dlopen()| was on a library
+ // containing an already-mapped symbol, it will remain valid.
+ dlclose(handle);
+ return reinterpret_cast<DestructFn*>(fn);
+}
+
+// Under 10.5 |object_cxxDestruct()| is used but unfortunately it is
+// |__private_extern__| in the runtime, meaning |dlsym()| cannot reach it.
+DestructFn* LookupObjectDestruct_10_5() {
+ // |nlist()| is only present for 32-bit.
+#if ARCH_CPU_32_BITS
+ const char* objc_runtime_path = LookupObjcRuntimePath();
+ if (!objc_runtime_path)
+ return NULL;
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 ||
+ if (nlist(objc_runtime_path, nl) < 0 ||
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);
+ // Back out offset to |class_addIvar()| to get the baseline, then
+ // add back offset to |object_cxxDestruct()|.
+ uintptr_t reference_addr = reinterpret_cast<uintptr_t>(&class_addIvar);
+ reference_addr -= nl[1].n_value;
+ reference_addr += nl[0].n_value;
+
+ return reinterpret_cast<DestructFn*>(reference_addr);
+#endif
+
+ return NULL;
}
// Replacement |-dealloc| which turns objects into zombies and places
@@ -109,12 +153,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 +168,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
@@ -214,15 +259,59 @@ void ZombieObjectCrash(id object, SEL aSelector, SEL viaSelector) {
*zero = 0;
}
+// For monitoring failures in |ZombieInit()|.
+enum ZombieFailure {
+ FAILED_10_5,
+ FAILED_10_6,
+
+ // Add new versions before here.
+ FAILED_MAX,
+};
+
+void RecordZombieFailure(ZombieFailure failure) {
+ UMA_HISTOGRAM_ENUMERATION("OSX.ZombieInitFailure", failure, FAILED_MAX);
+}
+
// Initialize our globals, returning YES on success.
BOOL ZombieInit() {
static BOOL initialized = NO;
if (initialized)
return YES;
- Class rootClass = [NSObject class];
+ // Whitelist releases that are compatible with objc zombies.
+ int32 major_version = 0, minor_version = 0, bugfix_version = 0;
+ base::SysInfo::OperatingSystemVersionNumbers(
+ &major_version, &minor_version, &bugfix_version);
- g_object_cxxDestruct = LookupObjectCxxDestruct();
+ if (major_version < 10 || (major_version == 10 && minor_version < 5)) {
+ return NO;
+ } else if (major_version == 10 && minor_version == 5) {
+ g_objectDestruct = LookupObjectDestruct_10_5();
+ if (!g_objectDestruct) {
+ RecordZombieFailure(FAILED_10_5);
+ return NO;
+ }
+ } else if (major_version == 10 && minor_version == 6) {
+ g_objectDestruct = LookupObjectDestruct_10_6();
+ if (!g_objectDestruct) {
+ RecordZombieFailure(FAILED_10_6);
+ return NO;
+ }
+ } else {
+ // Assume the future looks like the present.
+ g_objectDestruct = LookupObjectDestruct_10_6();
+
+ // Put all future failures into the MAX bin. New OS releases come
+ // out infrequently enough that this should always correspond to
+ // "Next release", and once the next release happens that bin will
+ // get an official name.
+ if (!g_objectDestruct) {
+ RecordZombieFailure(FAILED_MAX);
+ return NO;
+ }
+ }
+
+ Class rootClass = [NSObject class];
g_originalDeallocIMP =
class_getMethodImplementation(rootClass, @selector(dealloc));
// objc_getClass() so CrZombie doesn't need +class.
@@ -230,7 +319,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;
« no previous file with comments | « chrome/browser/chrome_browser_application_mac.mm ('k') | chrome/browser/ui/cocoa/objc_zombie_unittest.mm » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698