search

mattgallagher / CwlPreconditionTesting

A Mach exception handler that allows Swift precondition failures to be caught and tested.
ISC License
175 stars 46 forks source link

Define BadInstructionException for non-x86_64 arch #6

Closed jeffh closed 7 years ago

jeffh commented 7 years ago

Carthage, which sets ONLY_ACTIVE_ARCH = NO, will cause the swift compiler to generate a bridging header with a random architecture set. This means there is non-deterministic behavior for when BadInstructionException is defined in the bridging header. But CwlCatchBadInstruction.m needs BadInstructionException to be defined.

BadInstructionException implementation will fatally error if used on non-x86_64 architectures.

Fixes #5. From https://github.com/Quick/Nimble/pull/350.

jeffh commented 7 years ago

For reproducing the issue, setting ONLY_ACTIVE_ARCH = NO for iOS while Cleaning + Building several times causes this issue we've been seeing in #5.

mattgallagher commented 7 years ago

I'm still not able to reproduce this problem (I wonder if there's an additional environmental factor that I'm missing) but I can certainly understand where Xcode is likely screwing it up and your solution (make sure the definition remains in the interface for all architectures) seems like the right approach.

I've taken the spirit of this pull request (that BadInstructionException and the static catch_mach_exception_raise_state should be declared for all architectures) but I've implemented it in a slightly different way.

Instead of an implementation in CwlBadInstructionException.swift that is only active when arch(x86_64) and an implemention in CwlCatchBadInstruction.swift that is only active when NOT arch(x86_64), I've got a single implementation in CwlBadInstructionException.swift that is always active but the contents of the catch_mach_exception_raise_state function are wrapped in #if arch(x86_64)/#else/#endif, using the fatalError("Unavailable for this CPU architecture") from this pull request outside of arch(x86_64).

The net effect should be identical to this pull request while avoiding the duplication of the BadInstructionException interface.

Again though, since I'm unable to reproduce the problem, let me know if I've overlooked something.