Issues Identified by TSan and UBSan #362
Description
Hi Erik, I'm working on a branch of OCMock to add additional quality checks to the project (ASan, TSan, & UBSan). While working on implementing these additional checks when running the unit tests, they uncovered some issues.
TSan Issues
The thread sanitizer found 5 tests involving notification mock objects that all failed with the same error: when verify is called on the mock, you end up with a Thread 1: EXC_BAD_ACCESS (code=EXC_I386_GPFLT). TSan is indicating that there is a memory read error, so I'm wondering if the notification mock is either in the process of being released or is already released since OCObserverMockObject has the autoRemoveFromCenter: concept.
Test Case '-[OCMockObjectMacroTests testSetsUpNotificationPostingAndNotificationObserving]' started.
ThreadSanitizer:DEADLYSIGNAL
==33509==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000109c0efa7 bp 0x7ffee7d2d750 sp 0x7ffee7d2d488 T121810)
==33509==The signal is caused by a READ memory access.
==33509==Hint: address points to the zero page.
#0 objc_retain <null>:525056192 (libobjc.A.dylib:x86_64+0x16fa6)
==33509==Register values:
rax = 0x0000000000000000 rbx = 0x00007b540005dfc0 rcx = 0x000000000000010c rdx = 0x000000010cbaa380
rdi = 0x4000000000000000 rsi = 0x0000000000000000 rbp = 0x00007ffee7d2d750 rsp = 0x00007ffee7d2d488
r8 = 0x0000000000000001 r9 = 0x0000000000000001 r10 = 0x0000000000000000 r11 = 0x0000000000000000
r12 = 0x00007b10001cb740 r13 = 0x00007b540005de80 r14 = 0x00007b10001cb748 r15 = 0x00007b080001fac0
ThreadSanitizer can not provide additional info.
SUMMARY: ThreadSanitizer: SEGV (libobjc.A.dylib:x86_64+0x16fa6) in objc_retain
==33509==ABORTING
Test Case '-[OCMockObjectMacroTests testNotificationObservingWithUserInfo]' started.
ThreadSanitizer:DEADLYSIGNAL
==33672==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000106896fa7 bp 0x7ffeeb0a5750 sp 0x7ffeeb0a5568 T122943)
==33672==The signal is caused by a READ memory access.
==33672==Hint: address points to the zero page.
#0 objc_retain <null>:475863232 (libobjc.A.dylib:x86_64+0x16fa6)
==33672==Register values:
rax = 0x0000000000000000 rbx = 0x00007b540005bf40 rcx = 0x000000000000010c rdx = 0x0000000109832380
rdi = 0x4000000000000000 rsi = 0x0000000000000000 rbp = 0x00007ffeeb0a5750 rsp = 0x00007ffeeb0a5568
r8 = 0x0000000000000001 r9 = 0x0000000000000001 r10 = 0x0000000000000000 r11 = 0x0000000000000000
r12 = 0x00007b10001cd400 r13 = 0x00007b540005be00 r14 = 0x00007b10001cd408 r15 = 0x00007b080001eaa0
ThreadSanitizer can not provide additional info.
SUMMARY: ThreadSanitizer: SEGV (libobjc.A.dylib:x86_64+0x16fa6) in objc_retain
==33672==ABORTING
Test Case '-[OCObserverMockObjectTest testAcceptsExpectedNotification]' started.
ThreadSanitizer:DEADLYSIGNAL
==33854==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000109953fa7 bp 0x7ffee7fe8750 sp 0x7ffee7fe8628 T124339)
==33854==The signal is caused by a READ memory access.
==33854==Hint: address points to the zero page.
#0 objc_retain <null>:527243456 (libobjc.A.dylib:x86_64+0x16fa6)
==33854==Register values:
rax = 0x0000000000000000 rbx = 0x00007b5400086240 rcx = 0x000000000000010c rdx = 0x000000010c8ef380
rdi = 0x4000000000000000 rsi = 0x0000000000000000 rbp = 0x00007ffee7fe8750 rsp = 0x00007ffee7fe8628
r8 = 0x0000000000000001 r9 = 0x0000000000000001 r10 = 0x0000000000000000 r11 = 0x0000000000000000
r12 = 0x00007b10001d0900 r13 = 0x00007b5400086100 r14 = 0x00007b10001d0908 r15 = 0x00007b080001fa40
ThreadSanitizer can not provide additional info.
SUMMARY: ThreadSanitizer: SEGV (libobjc.A.dylib:x86_64+0x16fa6) in objc_retain
==33854==ABORTING
Test Case '-[OCObserverMockObjectTest testAcceptsExpectedNotificationWithSpecifiedObjectAndUserInfo]' started.
ThreadSanitizer:DEADLYSIGNAL
==34033==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000101f27fa7 bp 0x7ffeefa14750 sp 0x7ffeefa145f8 T125537)
==34033==The signal is caused by a READ memory access.
==34033==Hint: address points to the zero page.
#0 objc_retain <null>:394086592 (libobjc.A.dylib:x86_64+0x16fa6)
==34033==Register values:
rax = 0x0000000000000000 rbx = 0x00007b54000765c0 rcx = 0x000000000000010c rdx = 0x0000000104f63380
rdi = 0x4000000000000000 rsi = 0x0000000000000000 rbp = 0x00007ffeefa14750 rsp = 0x00007ffeefa145f8
r8 = 0x0000000000000001 r9 = 0x0000000000000001 r10 = 0x0000000000000000 r11 = 0x0000000000000000
r12 = 0x00007b10001d0a40 r13 = 0x00007b5400076480 r14 = 0x00007b10001d0a48 r15 = 0x00007b080001ea80
ThreadSanitizer can not provide additional info.
SUMMARY: ThreadSanitizer: SEGV (libobjc.A.dylib:x86_64+0x16fa6) in objc_retain
==34033==ABORTING
Test Case '-[OCObserverMockObjectTest testChecksNotificationNamesCorrectly]' started.
ThreadSanitizer:DEADLYSIGNAL
==34211==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0001094acfa7 bp 0x7ffee848f750 sp 0x7ffee848f608 T126770)
==34211==The signal is caused by a READ memory access.
==34211==Hint: address points to the zero page.
#0 objc_retain <null>:522000576 (libobjc.A.dylib:x86_64+0x16fa6)
==34211==Register values:
rax = 0x0000000000000000 rbx = 0x00007b5400056a40 rcx = 0x000000000000010c rdx = 0x000000010c448380
rdi = 0x4000000000000000 rsi = 0x0000000000000000 rbp = 0x00007ffee848f750 rsp = 0x00007ffee848f608
r8 = 0x0000000000000001 r9 = 0x0000000000000001 r10 = 0x0000000000000000 r11 = 0x0000000000000000
r12 = 0x00007b10001d09c0 r13 = 0x00007b5400056900 r14 = 0x00007b10001d09c8 r15 = 0x00007b080001fb20
ThreadSanitizer can not provide additional info.
SUMMARY: ThreadSanitizer: SEGV (libobjc.A.dylib:x86_64+0x16fa6) in objc_retain
==34211==ABORTING
UBSan Issue
The undefined behavior sanitizer found 1 test that exposed a nullability issue in setupForwarderForSelector: in OCPartialMockObject. The testStubsMethodImplementation test in OCMockForwardingTargetTests indicates that when the forwarder is being setup, the originalMethod is nil, so when you pass that to method_getImplementation, that is not allowed since the method parameter is marked explicitly as nonnull. I'm wondering if you need to return early there or do something similar to a few lines below where you are checking if the type encoding is null.
I'd be happy to fix these on my branch where I'm adding these checks in, but I'm unsure of the correct solution.