Skip to content

Commit c2ef692

Browse files
johnezangjohnezang
committed
Fixes issues johnezang#9, johnezang#15, johnezang#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.
1 parent 660546f commit c2ef692

1 file changed

Lines changed: 51 additions & 14 deletions

File tree

‎JSONKit.m‎

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -692,14 +692,14 @@ + (id)allocWithZone:(NSZone *)zone
692692
static void _JKArrayInsertObjectAtIndex(JKArray *array, id newObject, NSUInteger objectIndex) {
693693
NSCParameterAssert((array != NULL) && (array->objects != NULL) && (array->count <= array->capacity) && (objectIndex <= array->count) && (newObject != NULL));
694694
if(!((array != NULL) && (array->objects != NULL) && (objectIndex <= array->count) && (newObject != NULL))) { [newObject autorelease]; return; }
695-
array->count++;
696-
if(array->count >= array->capacity) {
697-
array->capacity += 16UL;
695+
if((array->count + 1UL) >= array->capacity) {
698696
id *newObjects = NULL;
699-
if((newObjects = (id *)realloc(array->objects, sizeof(id) * array->capacity)) == NULL) { [NSException raise:NSMallocException format:@"Unable to resize objects array."]; }
697+
if((newObjects = (id *)realloc(array->objects, sizeof(id) * (array->capacity + 16UL))) == NULL) { [NSException raise:NSMallocException format:@"Unable to resize objects array."]; }
700698
array->objects = newObjects;
699+
array->capacity += 16UL;
701700
memset(&array->objects[array->count], 0, sizeof(id) * (array->capacity - array->count));
702701
}
702+
array->count++;
703703
if((objectIndex + 1UL) < array->count) { memmove(&array->objects[objectIndex + 1UL], &array->objects[objectIndex], sizeof(id) * ((array->count - 1UL) - objectIndex)); array->objects[objectIndex] = NULL; }
704704
array->objects[objectIndex] = newObject;
705705
}
@@ -714,11 +714,11 @@ static void _JKArrayReplaceObjectAtIndexWithObject(JKArray *array, NSUInteger ob
714714
}
715715

716716
static void _JKArrayRemoveObjectAtIndex(JKArray *array, NSUInteger objectIndex) {
717-
NSCParameterAssert((array != NULL) && (array->objects != NULL) && (array->count <= array->capacity) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL));
718-
if(!((array != NULL) && (array->objects != NULL) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL))) { return; }
717+
NSCParameterAssert((array != NULL) && (array->objects != NULL) && (array->count > 0UL) && (array->count <= array->capacity) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL));
718+
if(!((array != NULL) && (array->objects != NULL) && (array->count > 0UL) && (array->count <= array->capacity) && (objectIndex < array->count) && (array->objects[objectIndex] != NULL))) { return; }
719719
CFRelease(array->objects[objectIndex]);
720720
array->objects[objectIndex] = NULL;
721-
if((objectIndex + 1UL) < array->count) { memmove(&array->objects[objectIndex], &array->objects[objectIndex + 1UL], sizeof(id) * ((array->count - 1UL) - objectIndex)); array->objects[array->count] = NULL; }
721+
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; }
722722
array->count--;
723723
}
724724

@@ -2556,20 +2556,57 @@ static int jk_encode_add_atom_to_buffer(JKEncodeState *encodeState, void *object
25562556

25572557
// 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,
25582558
// 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
2559-
// supported classes, we fail the second type (i.e., double fault error).
2559+
// supported classes, we fail the second time (i.e., double fault error).
25602560
BOOL rerunningAfterClassFormatter = NO;
2561-
rerunAfterClassFormatter:
2561+
rerunAfterClassFormatter:;
2562+
2563+
// XXX XXX XXX XXX
2564+
//
2565+
// 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.
2566+
//
2567+
// 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".
2568+
//
2569+
// This is euphemistically called "tagged pointers". There are a number of highly technical problems with this, most involving long passages from
2570+
// 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
2571+
// fundamentally Wrong and Broken, and should have never been implemented. Assuming those points are glossed over, because the change is very clearly
2572+
// breaking ABI compatibility, this should have resulted in a minimum of a "minimum version required" bump in various shared libraries to prevent
2573+
// causes code that used to work just fine to suddenly break without warning.
2574+
//
2575+
// 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
2576+
// "lower, unused bits" must be preserved when casting the result to an integer type, but this "works" because for most architectures
2577+
// `sizeof(long) == sizeof(void *)` and the compiler uses the same representation for both. (note: this is informal, not meant to be
2578+
// normative or pedantically correct).
2579+
//
2580+
// 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
2581+
// is not required in any way to produce the same results or behavior that earlier versions of the compiler did for the statement below.
2582+
//
2583+
// Fan-fucking-tastic.
2584+
//
2585+
// Why not just use `object_getClass()`? Because `object->isa` reduces to (typically) a *single* instruction. Calling `object_getClass()` requires
2586+
// that the compiler potentially spill registers, establish a function call frame / environment, and finally execute a "jump subroutine" instruction.
2587+
// Then, the called subroutine must spend half a dozen instructions in its prolog, however many instructions doing whatever it does, then half a dozen
2588+
// instructions in its prolog. One instruction compared to dozens, maybe a hundred instructions.
2589+
//
2590+
// 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
2591+
// gains in the two to three percent range.
2592+
//
2593+
// XXX XXX XXX XXX
2594+
2595+
BOOL workAroundMacOSXABIBreakingBug = NO;
2596+
if(JK_EXPECT_F(((NSUInteger)object) & 0x1)) { workAroundMacOSXABIBreakingBug = YES; goto slowClassLookup; }
2597+
25622598
if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.stringClass)) { isClass = JKClassString; }
25632599
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.numberClass)) { isClass = JKClassNumber; }
25642600
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.dictionaryClass)) { isClass = JKClassDictionary; }
25652601
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.arrayClass)) { isClass = JKClassArray; }
25662602
else if(JK_EXPECT_T(object->isa == encodeState->fastClassLookup.nullClass)) { isClass = JKClassNull; }
25672603
else {
2568-
if(JK_EXPECT_T([object isKindOfClass:[NSString class]])) { encodeState->fastClassLookup.stringClass = object->isa; isClass = JKClassString; }
2569-
else if(JK_EXPECT_T([object isKindOfClass:[NSNumber class]])) { encodeState->fastClassLookup.numberClass = object->isa; isClass = JKClassNumber; }
2570-
else if(JK_EXPECT_T([object isKindOfClass:[NSDictionary class]])) { encodeState->fastClassLookup.dictionaryClass = object->isa; isClass = JKClassDictionary; }
2571-
else if(JK_EXPECT_T([object isKindOfClass:[NSArray class]])) { encodeState->fastClassLookup.arrayClass = object->isa; isClass = JKClassArray; }
2572-
else if(JK_EXPECT_T([object isKindOfClass:[NSNull class]])) { encodeState->fastClassLookup.nullClass = object->isa; isClass = JKClassNull; }
2604+
slowClassLookup:
2605+
if(JK_EXPECT_T([object isKindOfClass:[NSString class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.stringClass = object->isa; } isClass = JKClassString; }
2606+
else if(JK_EXPECT_T([object isKindOfClass:[NSNumber class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.numberClass = object->isa; } isClass = JKClassNumber; }
2607+
else if(JK_EXPECT_T([object isKindOfClass:[NSDictionary class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.dictionaryClass = object->isa; } isClass = JKClassDictionary; }
2608+
else if(JK_EXPECT_T([object isKindOfClass:[NSArray class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.arrayClass = object->isa; } isClass = JKClassArray; }
2609+
else if(JK_EXPECT_T([object isKindOfClass:[NSNull class]])) { if(workAroundMacOSXABIBreakingBug == NO) { encodeState->fastClassLookup.nullClass = object->isa; } isClass = JKClassNull; }
25732610
else {
25742611
if((rerunningAfterClassFormatter == NO) && (
25752612
#ifdef __BLOCKS__

0 commit comments

Comments
 (0)