Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Crash on iOS device when dereferencing a pointer returned by NSCoder's decodeBytesForKey

I've found an unusual crasher with NSCoder when using the Apple LLVM Compiler 3.0 and compiled with -O3. It only crashes on devices. I've tested an iPhone 4 running iOS 5, an iPad 2 running iOS 5 and an iPad 1 running iOS 4. All crash identically. Here's the relevant section of code:

-(id)initWithCoder:(NSCoder*)decoder
{
    if (![super init])
    {
        return nil;
    }

    NSUInteger length = 0;

    uint8_t* data = (uint8_t*)[decoder decodeBytesForKey:BBKey returnedLength:&length];

    m_value = *(BBPointI32*)data;

    return self;
}

And here's what a BBPointI32 is:

typedef struct
{
    NSInteger x;
    NSInteger y;
}
BBPointI32;

The EXC_BAD_ACCESS happens when data is dereferenced. This is not a null pointer issue. If I attach GDB, I can see that length is 8, sizeof(BBPointI) is also 8 and the data is correct.

If I look at the disassembly, the crash is happening on:

ldrd    r2, r3, [r0]

Which looks fine. r0 contains 0xb546e, which is the address of data. When I inspect that memory, I can see that it contains the data I expect. For anyone who's interested, r2 contains 72 (not sure what that is) and r3 contains 8 (most probably the value of length).

Can anyone shed some light on this issue?

like image 589
Matt Comi Avatar asked Oct 11 '11 16:10

Matt Comi


3 Answers

ldrd needs the address to be 8-byte aligned. The *(BBPointI32 *)data idiom is not safe since data is not 8-byte aligned. Use memcpy to get the bytes into the struct instead.

like image 139
biorhythmist Avatar answered Sep 22 '22 06:09

biorhythmist


Are you using ARC? If so, I believe the issue is that the compiler is free to release decoder after the decodeBytesForKey: call (hence releasing the buffer to which the return value pointed).

It's the same interior pointer issue garbage collection has. You can CFRetain/CFRelease your decoder to extend its lifetime, or else just add a [decoder self] later in the method to keep it alive until that point.

I suspect you might also be able to solve this problem by annotating decoder with __attribute__((objc_precise_lifetime)), but my understanding of that attribute is somewhat hazy.

like image 31
Andy Matuschak Avatar answered Sep 26 '22 06:09

Andy Matuschak


Your example leaves a lot of variables to be questioned by any potential helper. For example: what if there's something funky with this unarchiver? Is memory being managed correctly?

I was able to reproduce the crash you're seeing, and can confirm it only occurs when -O3 is enabled, and not when e.g. None is selected for optimization. Here is a reduction of the crashing code that eliminates outside variables such as memory management of the coders, etc. Note the code below purposefully retains all objects to eliminate the possibility that the crash is related to an accidental over-release, or a side-effect of using ARC, as suggested by Andy in another answer:

typedef struct
{
    NSInteger x;
    NSInteger y;
}
BBPointI32;

- (void) testDecoding
{
    NSString* myKey = @"Testing";

    // First get an coder with bytes in it
    NSMutableData* myData = [[NSMutableData data] retain];
    NSKeyedArchiver* myCoder = [[NSKeyedArchiver alloc] initForWritingWithMutableData:myData];

    BBPointI32 encodedStruct = {1234, 5678};
    [myCoder encodeBytes:(const uint8_t *)&encodedStruct length:sizeof(encodedStruct) forKey:myKey];
    [myCoder finishEncoding];

    // Now decode it
    BBPointI32 decodedStruct;
    NSUInteger decodedLength = 0;
    NSKeyedUnarchiver* myDecoder = [[NSKeyedUnarchiver alloc] initForReadingWithData:myData];
    uint8_t* data = (uint8_t*)[myDecoder decodeBytesForKey:myKey returnedLength:&decodedLength];
    decodedStruct = *(BBPointI32*)data;
    NSLog(@"Got decoded struct with x = %ld, y = %ld, length = %lu", decodedStruct.x, decodedStruct.y, decodedLength);
}

- (void)applicationDidFinishLaunching:(UIApplication *)application {    
    NSLog(@"Testing decoding");
    [self testDecoding];
}

I think this gives a more succinct description of the problem that anybody who wants to help can use as a basis for diving in. My hunch thus far is this is an optimization bug in LLVM 3.0, but maybe somebody else will have a better theory about what is going on.

A point that you didn't mention in your question, but which I notice in the crash on my device, is the failure is accompanied by mention of a EXC_ARM_DA_ALIGN error as the reason for the bad access exception. I Googled a blog post that seems to allude to the same symptoms and probably cause for crashing as you are seeing here:

http://www.galloway.me.uk/2010/10/arm-hacking-exc_arm_da_align-exception/

Indeed, by changing the line above

decodedStruct = *(BBPointI32*)data;

to

memcpy(&decodedStruct, data, sizeof(decodedStruct));

The crashing behavior seems to be alleviated and the code behaves as expected.

like image 37
danielpunkass Avatar answered Sep 25 '22 06:09

danielpunkass