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

Question

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?

Agraffe

Answer 1

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.

Anode

Thanks, that's spot on. The issue was related to the compiler's decision to use ldrd. When I cast data to a BBPointI32* it assumed that this meant that the pointer was properly aligned, but it wasn't. So, instead of: m_value = (BBPointI32)data; I needed to use: memcpy(&m_value, data, length); – Agraffe

@biorhythmist.. not just an hilarious twitterer :) – Phenformin

I'm experiencing something very similar. Happens on iPad 3, and not on iPad 2! – Anatto

Answer 2

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.

Alidia

Answer 3

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.

Preparative

I didn't notice the EXC_ARM_DA_ALIGN on my end, but yep, that's the change I made also. Thanks for your help. – Agraffe

Answer 4

I got to this thread, googling "EXC_ARM_DA_ALIGN" and "EXC_BAD_ACCESS". None of the other answers helped me, as this error cropped up because of something relatively simple. I had written:

theArray = [[NSArray alloc] initWithObjects:@"first", @"second", @"third",
                        @"fourth", @"fifth", "sixth", nil];

i.e. I had left off a @ in front of a string literal. Putting it back in solved the error.

Shippen

lol - I just spent 2 hours trying to track this one down. Thanks for posting the solution. – Spinthariscope