Why does this code cause "EXC_BAD_INSTRUCTION"?

Question

dispatch_semaphore_t aSemaphore = dispatch_semaphore_create(1);        
dispatch_semaphore_wait(aSemaphore, DISPATCH_TIME_FOREVER);
dispatch_release(aSemaphore);

When the program runs to dispatch_release(aSemaphore), it will cause "EXC_BAD_INSTRUCTION", and then crash. Why?

Answer 1

I tried this code and it does indeed die with illegal instruction. So I did some digging and found that it's dying in _dispatch_semaphore_dispose. So let's look at what that is (ARMv7 here, because it's easy to understand!):

__dispatch_semaphore_dispose:
000040a0            b590        push    {r4, r7, lr}
000040a2            4604        mov     r4, r0
000040a4            af01        add     r7, sp, #4
000040a6        e9d40108        ldrd    r0, r1, [r4, #32]
000040aa            4288        cmp     r0, r1
000040ac            da00        bge.n   0x40b0
000040ae            defe        trap
...

It dies at 0x40ae, which is a duff instruction put there so that it crashes if the bge.n doesn't make us branch to jump over it.

The reason it's failing is because r0 must be less than r1. r0 and r1 are loaded from the memory at r4 + 32 which having gone back up the stack to figure it out I think r4 is aSemaphore in the example code, i.e. the thing passed into dispatch_semaphore_release. The + 32 signifies it is reading 32 bytes into the struct that aSemaphore is pointing to (it's a pointer to a dispatch_semaphore_s struct). So overall what it's doing it reading 4 bytes from aSemaphore + 32 and putting them into r0 and reading 4 bytes from aSemaphore + 36 and putting them into r1.

The compare is then effectively comparing the value of aSemaphore + 32 and aSemaphore + 36. Reading what dispatch_semaphore_create does I can see that it stores the value passed in to both aSemaphore + 32 and aSemaphore + 36. I also found that dispatch_semaphore_wait and dispatch_semaphore_signal touch the value at aSemaphore + 32, to increment and decrement it. This means that the reason it's breaking is because the current value of the semaphore is less than the value passed into dispatch_semaphore_create. So you can't dispose of a semaphore when the current value is less than the value it was created with.

If you've read to here and understood my ramblings then well done! Hope it helps!

UPDATE:

It's probably better to look at the source (pointed out by JustSid) here - http://opensource.apple.com/source/libdispatch/libdispatch-187.7/src/semaphore.c - looking at the _dispatch_semaphore_dispose function we see:

if (dsema->dsema_value < dsema->dsema_orig) {
    DISPATCH_CLIENT_CRASH("Semaphore/group object deallocated while in use");
}

So, yes, there you go, that's why it crashes!

Answer 2

Somewhat more succinct answer: You are creating the semaphore with the wrong value, it should be zero. Creating it with a value of 1 means you are later releasing a semaphore that's still "in use" and GCD is deliberately generating an illegal instruction in order to help you debug the fact that you have a semaphore with more waiters on it.

Answer 3

You can create a semaphore with zero value, but I believe it will be just useless. I had a semaphore field in a class which caused it to crash at deinitialisation. This is how I fixed it (Swift code):

deinit {
  while (dispatch_semaphore_signal(semaphore) != 0) {}
}

A rather awkward patch, but it works!