Made some refactorings.

erikdoe · May 24, 2020 · 30e6dc9 · 30e6dc9
1 parent 892373f
commit 30e6dc9
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 104 deletions.
diff --git a/Source/OCMock/NSInvocation+OCMAdditions.m b/Source/OCMock/NSInvocation+OCMAdditions.m
@@ -88,17 +88,14 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
             {
                 if(OCMIsBlockType(argumentType))
                 {
-                    if (!OCMIsBlockNoEscape(argument))
+                    // Block types need to be copied because they could be stack blocks.
+                    // However, non-escaping blocks have a lifetime that is stack-based and they
+                    // treat copy/release as a no-op. For details see:
+                    // https://reviews.llvm.org/rGdbfa453e4138bb977644929c69d1c71e5e8b4bee
+                    // If we keep a reference to a non-escaping block in retainedArguments, it
+                    // will end up as dangling pointer, resulting in a crash later.
+                    if(OCMIsNonEscapingBlock(argument) == NO)
                     {
-                        // Normal block types need to be copied in case they're stack blocks.
-                        // No escape blocks are special though and should not be retained because
-                        // their life time is stack based and copying/retaining a non-escaping
-                        // block is a no-op. For details see:
-                        // https://reviews.llvm.org/rGdbfa453e4138bb977644929c69d1c71e5e8b4bee
-                        // This means that if we keep a reference to a no escape block in
-                        // retainedArguments, it will end up as dangling pointer when we exit the
-                        // stack scope the block is declared in, and we will crash when we attempt
-                        // to use it.
                         id blockArgument = [argument copy];
                         [retainedArguments addObject:blockArgument];
                         [blockArgument release];
@@ -127,7 +124,8 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
         {
             if(OCMIsBlockType(returnType))
             {
-                if (!OCMIsBlockNoEscape(returnValue))
+                // See above for an explanation
+                if(OCMIsNonEscapingBlock(returnValue) == NO)
                 {
                     id blockReturnValue = [returnValue copy];
                     [retainedArguments addObject:blockReturnValue];

diff --git a/Source/OCMock/NSMethodSignature+OCMAdditions.m b/Source/OCMock/NSMethodSignature+OCMAdditions.m
@@ -113,9 +113,30 @@ + (objc_property_t)propertyMatchingSelector:(SEL)selector inClass:(Class)aClass
 
 + (NSMethodSignature *)signatureForBlock:(id)block
 {
-    return OCMSignatureForBlock(block);
+    /* For a more complete implementation of parsing the block data structure see:
+     *
+     * https://github.com/ebf/CTObjectiveCRuntimeAdditions/tree/master/CTObjectiveCRuntimeAdditions/CTObjectiveCRuntimeAdditions
+     */
+
+    struct OCMBlockDef *blockRef = (__bridge struct OCMBlockDef *) block;
+
+    if(!(blockRef->flags & OCMBlockDescriptionFlagsHasSignature))
+        return nil;
+
+    void *signatureLocation = blockRef->descriptor;
+    signatureLocation += sizeof(unsigned long int);
+    signatureLocation += sizeof(unsigned long int);
+    if(blockRef->flags & OCMBlockDescriptionFlagsHasCopyDispose)
+    {
+        signatureLocation += sizeof(void (*)(void *dst, void *src));
+        signatureLocation += sizeof(void (*)(void *src));
+    }
+
+    const char *signature = (*(const char **) signatureLocation);
+    return [NSMethodSignature signatureWithObjCTypes:signature];
 }
 
+
 #pragma mark    Extended attributes
 
 - (BOOL)usesSpecialStructureReturn

diff --git a/Source/OCMock/OCMFunctions.m b/Source/OCMock/OCMFunctions.m
@@ -310,6 +310,14 @@ BOOL OCMIsApplePrivateMethod(Class cls, SEL sel)
             ([selName hasPrefix:@"_"] || [selName hasSuffix:@"_"]);
 }
 
+
+BOOL OCMIsNonEscapingBlock(id block)
+{
+    struct OCMBlockDef *blockRef = (__bridge struct OCMBlockDef *)block;
+    return (blockRef->flags & OCMBlockIsNoEscape) != 0;
+}
+
+
 #pragma mark  Creating classes
 
 Class OCMCreateSubclass(Class class, void *ref)
@@ -424,61 +432,3 @@ void OCMReportFailure(OCMLocation *loc, NSString *description)
     }
 
 }
-
-#pragma mark  Block Support
-
-struct OCMBlockDef
-{
-    void *isa; // initialized to &_NSConcreteStackBlock or &_NSConcreteGlobalBlock
-    int flags;
-    int reserved;
-    void (*invoke)(void *, ...);
-    struct block_descriptor {
-        unsigned long int reserved;                 // NULL
-        unsigned long int size;                     // sizeof(struct Block_literal_1)
-        // optional helper functions
-        void (*copy_helper)(void *dst, void *src);  // IFF (1<<25)
-        void (*dispose_helper)(void *src);          // IFF (1<<25)
-        // required ABI.2010.3.16
-        const char *signature;                      // IFF (1<<30)
-    } *descriptor;
-};
-
-enum
-{
-    OCMBlockIsNoEscape                     = (1 << 23),
-    OCMBlockDescriptionFlagsHasCopyDispose = (1 << 25),
-    OCMBlockDescriptionFlagsHasSignature   = (1 << 30)
-};
-
-
-NSMethodSignature *OCMSignatureForBlock(id block)
-{
-    /* For a more complete implementation of parsing the block data structure see:
-     *
-     * https://github.com/ebf/CTObjectiveCRuntimeAdditions/tree/master/CTObjectiveCRuntimeAdditions/CTObjectiveCRuntimeAdditions
-     */
-
-    struct OCMBlockDef *blockRef = (__bridge struct OCMBlockDef *)block;
-
-    if(!(blockRef->flags & OCMBlockDescriptionFlagsHasSignature))
-        return nil;
-
-    void *signatureLocation = blockRef->descriptor;
-    signatureLocation += sizeof(unsigned long int);
-    signatureLocation += sizeof(unsigned long int);
-    if(blockRef->flags & OCMBlockDescriptionFlagsHasCopyDispose)
-    {
-        signatureLocation += sizeof(void(*)(void *dst, void *src));
-        signatureLocation += sizeof(void (*)(void *src));
-    }
-
-    const char *signature = (*(const char **)signatureLocation);
-    return [NSMethodSignature signatureWithObjCTypes:signature];
-}
-
-BOOL OCMIsBlockNoEscape(id block)
-{
-    struct OCMBlockDef *blockRef = (__bridge struct OCMBlockDef *)block;
-    return blockRef->flags & OCMBlockIsNoEscape;
-}
diff --git a/Source/OCMock/OCMFunctionsPrivate.h b/Source/OCMock/OCMFunctionsPrivate.h
@@ -45,5 +45,31 @@ OCPartialMockObject *OCMGetAssociatedMockForObject(id anObject);
 
 void OCMReportFailure(OCMLocation *loc, NSString *description);
 
-NSMethodSignature *OCMSignatureForBlock(id block);
-BOOL OCMIsBlockNoEscape(id block);
+BOOL OCMIsNonEscapingBlock(id block);
+
+
+
+struct OCMBlockDef
+{
+    void *isa; // initialized to &_NSConcreteStackBlock or &_NSConcreteGlobalBlock
+    int flags;
+    int reserved;
+    void (*invoke)(void *, ...);
+    struct block_descriptor {
+        unsigned long int reserved;                 // NULL
+        unsigned long int size;                     // sizeof(struct Block_literal_1)
+        // optional helper functions
+        void (*copy_helper)(void *dst, void *src);  // IFF (1<<25)
+        void (*dispose_helper)(void *src);          // IFF (1<<25)
+        // required ABI.2010.3.16
+        const char *signature;                      // IFF (1<<30)
+    } *descriptor;
+};
+
+enum
+{
+    OCMBlockIsNoEscape                     = (1 << 23),
+    OCMBlockDescriptionFlagsHasCopyDispose = (1 << 25),
+    OCMBlockDescriptionFlagsHasSignature   = (1 << 30)
+};
+
diff --git a/Source/OCMockTests/OCMNoEscapeBlockTests.m b/Source/OCMockTests/OCMNoEscapeBlockTests.m
@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2004-2020 Erik Doernenburg and contributors
+ *  Copyright (c) 2020 Erik Doernenburg and contributors
  *
  *  Licensed under the Apache License, Version 2.0 (the "License"); you may
  *  not use these files except in compliance with the License. You may obtain
@@ -18,10 +18,10 @@
 #import <OCMock/OCMock.h>
 #import "OCMFunctionsPrivate.h"
 
-@interface NSString (NoEscapeBlock)
+@interface NSString(NoEscapeBlock)
 @end
 
-@implementation NSString (NoEscapeBlock)
+@implementation NSString(NoEscapeBlock)
 
 - (void)methodWithNoEscapeBlock:(void(NS_NOESCAPE ^)(void))block
 {
@@ -35,62 +35,63 @@ @interface BlockCapturer : NSProxy
 
 @implementation BlockCapturer
 {
-  XCTestExpectation *expectation;
+    XCTestExpectation *expectation;
 }
 
 - (instancetype)initWithExpectation:(XCTestExpectation *)anExpectation
 {
-  expectation = anExpectation;
-  return self;
+    expectation = anExpectation;
+    return self;
 }
 
 - (NSMethodSignature *)methodSignatureForSelector:(SEL)selector
 {
-  return [NSString instanceMethodSignatureForSelector:selector];
+    return [NSString instanceMethodSignatureForSelector:selector];
 }
 
 - (void)forwardInvocation:(NSInvocation *)invocation
 {
-  __unsafe_unretained id block;
-  [invocation getArgument:&block atIndex:2];
-  if (OCMIsBlockNoEscape(block))
-  {
-    [expectation fulfill];
-  }
+    __unsafe_unretained id block;
+    [invocation getArgument:&block atIndex:2];
+    if(OCMIsNonEscapingBlock(block))
+    {
+        [expectation fulfill];
+    }
 }
 
 @end
 
+
 @interface OCMNoEscapeBlockTests : XCTestCase
 @end
 
 @implementation OCMNoEscapeBlockTests
 
 - (void)testThatBlocksAreNoEscape
 {
-  // This tests that this file is compiled with
-  // `-Xclang -fexperimental-optimized-noescape` or equivalent.
-  XCTestExpectation *expectation = [self expectationWithDescription:@"Block should be noescape"];
-  id blockCapturer = [[BlockCapturer alloc] initWithExpectation:expectation];
-  int i = 0;
-  [blockCapturer methodWithNoEscapeBlock:^{
-      // Force i to be pulled into the closure.
-      (void)i;
-  }];
-  [self waitForExpectationsWithTimeout:0 handler:nil];
+    // This tests that this file is compiled with
+    // `-Xclang -fexperimental-optimized-noescape` or equivalent.
+    XCTestExpectation *expectation = [self expectationWithDescription:@"Block should be noescape"];
+    id blockCapturer = [[BlockCapturer alloc] initWithExpectation:expectation];
+    int i = 0;
+    [blockCapturer methodWithNoEscapeBlock:^{
+        // Force i to be pulled into the closure.
+        (void)i;
+    }];
+    [self waitForExpectationsWithTimeout:0 handler:nil];
 }
 
-// This test will crash if it fails.
 - (void)testNoEscapeBlocksAreNotRetained
 {
-  // This tests that OCMock can handle noescape blocks.
-  id mock = [OCMockObject mockForClass:[NSString class]];
-  [[mock stub] methodWithNoEscapeBlock:[OCMArg invokeBlock]];
-  int i = 0;
-  [mock methodWithNoEscapeBlock:^{
-      // Force i to be pulled into the closure.
-      (void)i;
-  }];
+    // This tests that OCMock can handle noescape blocks.
+    // It crashes if it fails
+    id mock = [OCMockObject mockForClass:[NSString class]];
+    [[mock stub] methodWithNoEscapeBlock:[OCMArg invokeBlock]];
+    int i = 0;
+    [mock methodWithNoEscapeBlock:^{
+        // Force i to be pulled into the closure.
+        (void)i;
+    }];
 }
 
 @end