Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixes issues #9, #15, #40, aka "crashes on 64-bit Lion / 10.7 ABI".

This commit implements a work around for a bug in 10.7 that was caused by
a 10.7 64-bit ABI breaking change.

Technically, this is not a bug in JSONKit, but with Mac OS X.

When making changes to the ABI, it is (at least de facto) required to bump
the "major version" of a shared library so that code designed around and
built against the "guarantees" provided by previous versions ABI / API
are not violated.

Not only was this not done in 10.7, the ABI breaking change isn't even
officially documented (to the best of my knowledge).  It's certainly not
mentioned in the 10.7 release notes.
  • Loading branch information...
commit c2ef69242b60dc2475328b08131f9aa3078a51fb 1 parent 660546f
@johnezang johnezang authored
Showing with 51 additions and 14 deletions.
  1. +51 −14 JSONKit.m
View
65 JSONKit.m
@@ -692,14 +692,14 @@ + (id)allocWithZone:(NSZone *)zone
static void _JKArrayInsertObjectAtIndex(JKArray *array, id newObject, NSUInteger objectIndex) {
NSCParameterAssert((array != NULL) && (array->objects != NULL) && (array->count <= array->capacity) && (objectIndex <= array->count) && (newObject != NULL));
if(!((array != NULL) && (array->objects != NULL) && (objectIndex <= array->count) && (newObject != NULL))) { [newObject autorelease]; return; }
- array->count++;
- if(array->count >= array->capacity) {
- array->capacity += 16UL;
+ if((array->count + 1UL) >= array->capacity) {
id *newObjects = NULL;
- if((newObjects = (id *)realloc(array->objects, sizeof(id) * array->capacity)) == NULL) { [NSException raise:NSMallocException format:@"Unable to resize objects array."]; }
+ if((newObjects = (id *)realloc(array->objects, sizeof(id) * (array->capacity + 16UL))) == NULL) { [NSException raise:NSMallocException format:@"Unable to resize objects array."]; }
array->objects = newObjects;
+ array->capacity += 16UL;
memset(&array->objects[array->count], 0, sizeof(id) * (array->capacity - array->count));
}
+ array->count++;
if((objectIndex + 1UL) < array->count) { memmove(&array->objects[objectIndex + 1UL], &array->objects[objectIndex], sizeof(id) * ((array->count - 1UL) - objectIndex)); array->objects[objectIndex] = NULL; }
array->objects[objectIndex] = newObject;
}
@@ -714,11 +714,11 @@ static void _JKArrayReplaceObjectAtIndexWithObject(JKArray *array, NSUInteger ob
}
static void _JKArrayRemoveObjectAtIndex(JKArray *array, NSUInteger objectIndex) {
- NSCParameterAssert((array != NULL) && (array->objects != NULL) && (array->count <= array->capacity) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL));
- if(!((array != NULL) && (array->objects != NULL) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL))) { return; }
+ NSCParameterAssert((array != NULL) && (array->objects != NULL) && (array->count > 0UL) && (array->count <= array->capacity) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL));
+ if(!((array != NULL) && (array->objects != NULL) && (array->count > 0UL) && (array->count <= array->capacity) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL))) { return; }
CFRelease(array->objects[objectIndex]);
array->objects[objectIndex] = NULL;
- if((objectIndex + 1UL) < array->count) { memmove(&array->objects[objectIndex], &array->objects[objectIndex + 1UL], sizeof(id) * ((array->count - 1UL) - objectIndex)); array->objects[array->count] = NULL; }
+ if((objectIndex + 1UL) < array->count) { memmove(&array->objects[objectIndex], &array->objects[objectIndex + 1UL], sizeof(id) * ((array->count - 1UL) - objectIndex)); array->objects[array->count - 1UL] = NULL; }
array->count--;
}
@@ -2556,20 +2556,57 @@ static int jk_encode_add_atom_to_buffer(JKEncodeState *encodeState, void *object
// When we encounter a class that we do not handle, and we have either a delegate or block that the user supplied to format unsupported classes,
// we "re-run" the object check. However, we re-run the object check exactly ONCE. If the user supplies an object that isn't one of the
- // supported classes, we fail the second type (i.e., double fault error).
+ // supported classes, we fail the second time (i.e., double fault error).
BOOL rerunningAfterClassFormatter = NO;
-rerunAfterClassFormatter:
+ rerunAfterClassFormatter:;
+
+ // XXX XXX XXX XXX
+ //
+ // We need to work around a bug in 10.7, which breaks ABI compatibility with Objective-C going back not just to 10.0, but OpenStep and even NextStep.
+ //
+ // It has long been documented that "the very first thing that a pointer to an Objective-C object "points to" is a pointer to that objects class".
+ //
+ // This is euphemistically called "tagged pointers". There are a number of highly technical problems with this, most involving long passages from
+ // the C standard(s). In short, one can make a strong case, couched from the perspective of the C standard(s), that that 10.7 "tagged pointers" are
+ // fundamentally Wrong and Broken, and should have never been implemented. Assuming those points are glossed over, because the change is very clearly
+ // breaking ABI compatibility, this should have resulted in a minimum of a "minimum version required" bump in various shared libraries to prevent
+ // causes code that used to work just fine to suddenly break without warning.
+ //
+ // In fact, the C standard says that the hack below is "undefined behavior"- there is no requirement that the 10.7 tagged pointer hack of setting the
+ // "lower, unused bits" must be preserved when casting the result to an integer type, but this "works" because for most architectures
+ // `sizeof(long) == sizeof(void *)` and the compiler uses the same representation for both. (note: this is informal, not meant to be
+ // normative or pedantically correct).
+ //
+ // In other words, while this "works" for now, technically the compiler is not obligated to do "what we want", and a later version of the compiler
+ // is not required in any way to produce the same results or behavior that earlier versions of the compiler did for the statement below.
+ //
+ // Fan-fucking-tastic.
+ //
+ // Why not just use `object_getClass()`? Because `object->isa` reduces to (typically) a *single* instruction. Calling `object_getClass()` requires
+ // that the compiler potentially spill registers, establish a function call frame / environment, and finally execute a "jump subroutine" instruction.
+ // Then, the called subroutine must spend half a dozen instructions in its prolog, however many instructions doing whatever it does, then half a dozen
+ // instructions in its prolog. One instruction compared to dozens, maybe a hundred instructions.
+ //
+ // Yes, that's one to two orders of magnitude difference. Which is compelling in its own right. When going for performance, you're often happy with
+ // gains in the two to three percent range.
+ //
+ // XXX XXX XXX XXX
+
+ BOOL workAroundMacOSXABIBreakingBug = NO;
+ if(JK_EXPECT_F(((NSUInteger)object) & 0x1)) { workAroundMacOSXABIBreakingBug = YES; goto slowClassLookup; }
+
if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.stringClass)) { isClass = JKClassString; }
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.numberClass)) { isClass = JKClassNumber; }
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.dictionaryClass)) { isClass = JKClassDictionary; }
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.arrayClass)) { isClass = JKClassArray; }
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.nullClass)) { isClass = JKClassNull; }
else {
- if(JK_EXPECT_T([object isKindOfClass:[NSString class]])) { encodeState->fastClassLookup.stringClass = object->isa; isClass = JKClassString; }
- else if(JK_EXPECT_T([object isKindOfClass:[NSNumber class]])) { encodeState->fastClassLookup.numberClass = object->isa; isClass = JKClassNumber; }
- else if(JK_EXPECT_T([object isKindOfClass:[NSDictionary class]])) { encodeState->fastClassLookup.dictionaryClass = object->isa; isClass = JKClassDictionary; }
- else if(JK_EXPECT_T([object isKindOfClass:[NSArray class]])) { encodeState->fastClassLookup.arrayClass = object->isa; isClass = JKClassArray; }
- else if(JK_EXPECT_T([object isKindOfClass:[NSNull class]])) { encodeState->fastClassLookup.nullClass = object->isa; isClass = JKClassNull; }
+ slowClassLookup:
+ if(JK_EXPECT_T([object isKindOfClass:[NSString class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.stringClass = object->isa; } isClass = JKClassString; }
+ else if(JK_EXPECT_T([object isKindOfClass:[NSNumber class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.numberClass = object->isa; } isClass = JKClassNumber; }
+ else if(JK_EXPECT_T([object isKindOfClass:[NSDictionary class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.dictionaryClass = object->isa; } isClass = JKClassDictionary; }
+ else if(JK_EXPECT_T([object isKindOfClass:[NSArray class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.arrayClass = object->isa; } isClass = JKClassArray; }
+ else if(JK_EXPECT_T([object isKindOfClass:[NSNull class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.nullClass = object->isa; } isClass = JKClassNull; }
else {
if((rerunningAfterClassFormatter == NO) && (
#ifdef __BLOCKS__
Please sign in to comment.
Something went wrong with that request. Please try again.