Merge branch 'dmaclach-block_fix'

erikdoe · erikdoe · commit 9b0e31451196 · 2020-05-24T17:43:59.000+02:00
diff --git a/Source/OCMock.xcodeproj/project.pbxproj b/Source/OCMock.xcodeproj/project.pbxproj
@@ -279,6 +279,8 @@
 		817EB15C1BD765130047E85A /* OCMBlockArgCaller.h in Headers */ = {isa = PBXBuildFile; fileRef = 2FA2891034E7B73AA3511D17 /* OCMBlockArgCaller.h */; };
 		817EB15D1BD765130047E85A /* OCMArgAction.h in Headers */ = {isa = PBXBuildFile; fileRef = 2FA2833B48908EAD36444671 /* OCMArgAction.h */; };
 		817EB1661BD7674D0047E85A /* OCMFunctionsPrivate.h in Headers */ = {isa = PBXBuildFile; fileRef = 03F370CA1BAA1DE800CAD3E8 /* OCMFunctionsPrivate.h */; };
+		8BF73E53246CA75E00B9A52C /* OCMNoEscapeBlockTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BF73E52246CA75E00B9A52C /* OCMNoEscapeBlockTests.m */; settings = {COMPILER_FLAGS = "-Xclang -fexperimental-optimized-noescape"; }; };
+		8BF73E54246CA75E00B9A52C /* OCMNoEscapeBlockTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BF73E52246CA75E00B9A52C /* OCMNoEscapeBlockTests.m */; settings = {COMPILER_FLAGS = "-Xclang -fexperimental-optimized-noescape"; }; };
 		8DE97C5522B43EE60098C63F /* OCMockObject.m in Sources */ = {isa = PBXBuildFile; fileRef = 03B3159E146333BF0052CD09 /* OCMockObject.m */; };
 		8DE97C5622B43EE60098C63F /* OCClassMockObject.m in Sources */ = {isa = PBXBuildFile; fileRef = 03B3158C146333BF0052CD09 /* OCClassMockObject.m */; };
 		8DE97C5722B43EE60098C63F /* OCPartialMockObject.m in Sources */ = {isa = PBXBuildFile; fileRef = 03B315AA146333BF0052CD09 /* OCPartialMockObject.m */; };
@@ -569,6 +571,7 @@
 		3CFBDD751BB3DB200050D9C5 /* TestClassWithCustomReferenceCounting.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TestClassWithCustomReferenceCounting.h; sourceTree = "<group>"; };
 		3CFBDD761BB3DB200050D9C5 /* TestClassWithCustomReferenceCounting.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = TestClassWithCustomReferenceCounting.m; sourceTree = "<group>"; };
 		817EB1621BD765130047E85A /* OCMock.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OCMock.framework; sourceTree = BUILT_PRODUCTS_DIR; };
+		8BF73E52246CA75E00B9A52C /* OCMNoEscapeBlockTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMNoEscapeBlockTests.m; sourceTree = "<group>"; };
 		8DE97CA022B43EE60098C63F /* OCMock.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OCMock.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		A02926811CA0725A00594AAF /* TestObjects.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = TestObjects.xcdatamodel; sourceTree = "<group>"; };
 		D31108AD1828DB8700737925 /* OCMockLibTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = OCMockLibTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
@@ -743,6 +746,7 @@
 				03B316231463350E0052CD09 /* OCMockObjectHamcrestTests.m */,
 				038599F623807B06002B3ABE /* OCMockObjectInternalTests.m */,
 				2FA2813F93050582D83E1499 /* OCMockObjectRuntimeTests.m */,
+				8BF73E52246CA75E00B9A52C /* OCMNoEscapeBlockTests.m */,
 				03B316271463350E0052CD09 /* OCMStubRecorderTests.m */,
 				037ECD5318FAD84100AF0E4C /* OCMInvocationMatcherTests.m */,
 				031E50571BB4A56300E257C3 /* OCMBoxedReturnValueProviderTests.m */,
@@ -1501,6 +1505,7 @@
 				2FA28FA53C57236B6DD64E82 /* OCMockObjectRuntimeTests.m in Sources */,
 				2FA2839F33289795284C32FB /* OCMockObjectTests.m in Sources */,
 				038599F723807B06002B3ABE /* OCMockObjectInternalTests.m in Sources */,
+				8BF73E53246CA75E00B9A52C /* OCMNoEscapeBlockTests.m in Sources */,
 				2FA28AB33F01A7D980F2C705 /* OCMockObjectDynamicPropertyMockingTests.m in Sources */,
 				031E50581BB4A56300E257C3 /* OCMBoxedReturnValueProviderTests.m in Sources */,
 			);
@@ -1613,6 +1618,7 @@
 				A06930951CA1BFC900513023 /* TestObjects.xcdatamodeld in Sources */,
 				2FA28295E1F58F40A77D7448 /* OCMockObjectRuntimeTests.m in Sources */,
 				038599F823807B06002B3ABE /* OCMockObjectInternalTests.m in Sources */,
+				8BF73E54246CA75E00B9A52C /* OCMNoEscapeBlockTests.m in Sources */,
 				2FA28246CD449A01717B1CEC /* OCMockObjectTests.m in Sources */,
 				2FA28F12AAD384A8CB16094B /* OCMockObjectDynamicPropertyMockingTests.m in Sources */,
 			);
diff --git a/Source/OCMock/NSInvocation+OCMAdditions.m b/Source/OCMock/NSInvocation+OCMAdditions.m
@@ -88,10 +88,18 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
             {
                 if(OCMIsBlockType(argumentType))
                 {
-                    // block types need to be copied in case they're stack blocks
-                    id blockArgument = [argument copy];
-                    [retainedArguments addObject:blockArgument];
-                    [blockArgument release];
+                    // 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)
+                    {
+                        id blockArgument = [argument copy];
+                        [retainedArguments addObject:blockArgument];
+                        [blockArgument release];
+                    }
                 }
                 else if(OCMIsClassType(argumentType) && object_isClass(argument))
                 {
@@ -116,9 +124,13 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
         {
             if(OCMIsBlockType(returnType))
             {
-                id blockReturnValue = [returnValue copy];
-                [retainedArguments addObject:blockReturnValue];
-                [blockReturnValue release];
+                // See above for an explanation
+                if(OCMIsNonEscapingBlock(returnValue) == NO)
+                {
+                    id blockReturnValue = [returnValue copy];
+                    [retainedArguments addObject:blockReturnValue];
+                    [blockReturnValue release];
+                }
             }
             else
             {
diff --git a/Source/OCMock/NSMethodSignature+OCMAdditions.m b/Source/OCMock/NSMethodSignature+OCMAdditions.m
@@ -111,38 +111,14 @@ + (objc_property_t)propertyMatchingSelector:(SEL)selector inClass:(Class)aClass
 
 #pragma mark    Signatures for blocks
 
-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
-{
-    OCMBlockDescriptionFlagsHasCopyDispose = (1 << 25),
-    OCMBlockDescriptionFlagsHasSignature   = (1 << 30)
-};
-
-
 + (NSMethodSignature *)signatureForBlock:(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;
+    struct OCMBlockDef *blockRef = (__bridge struct OCMBlockDef *) block;
 
     if(!(blockRef->flags & OCMBlockDescriptionFlagsHasSignature))
         return nil;
@@ -152,11 +128,11 @@ + (NSMethodSignature *)signatureForBlock:(id)block
     signatureLocation += sizeof(unsigned long int);
     if(blockRef->flags & OCMBlockDescriptionFlagsHasCopyDispose)
     {
-        signatureLocation += sizeof(void(*)(void *dst, void *src));
+        signatureLocation += sizeof(void (*)(void *dst, void *src));
         signatureLocation += sizeof(void (*)(void *src));
     }
 
-    const char *signature = (*(const char **)signatureLocation);
+    const char *signature = (*(const char **) signatureLocation);
     return [NSMethodSignature signatureWithObjCTypes:signature];
 }
 
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)
diff --git a/Source/OCMock/OCMFunctionsPrivate.h b/Source/OCMock/OCMFunctionsPrivate.h
@@ -45,3 +45,31 @@ OCPartialMockObject *OCMGetAssociatedMockForObject(id anObject);
 
 void OCMReportFailure(OCMLocation *loc, NSString *description);
 
+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
@@ -0,0 +1,97 @@
+/*
+ *  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
+ *  a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ *  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ *  License for the specific language governing permissions and limitations
+ *  under the License.
+ */
+
+#import <XCTest/XCTest.h>
+#import <OCMock/OCMock.h>
+#import "OCMFunctionsPrivate.h"
+
+@interface NSString(NoEscapeBlock)
+@end
+
+@implementation NSString(NoEscapeBlock)
+
+- (void)methodWithNoEscapeBlock:(void(NS_NOESCAPE ^)(void))block
+{
+}
+
+@end
+
+// Verifies that the block being passed in is a noescape block.
+@interface BlockCapturer : NSProxy
+@end
+
+@implementation BlockCapturer
+{
+    XCTestExpectation *expectation;
+}
+
+- (instancetype)initWithExpectation:(XCTestExpectation *)anExpectation
+{
+    expectation = anExpectation;
+    return self;
+}
+
+- (NSMethodSignature *)methodSignatureForSelector:(SEL)selector
+{
+    return [NSString instanceMethodSignatureForSelector:selector];
+}
+
+- (void)forwardInvocation:(NSInvocation *)invocation
+{
+    __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];
+}
+
+- (void)testNoEscapeBlocksAreNotRetained
+{
+    // 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

Original file line number	Diff line number	Diff line change
`@@ -88,10 +88,18 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude`
`88`	`88`	`{`
`89`	`89`	`if(OCMIsBlockType(argumentType))`
`90`	`90`	`{`
`91`		`- // block types need to be copied in case they're stack blocks`
`92`		`- id blockArgument = [argument copy];`
`93`		`- [retainedArguments addObject:blockArgument];`
`94`		`- [blockArgument release];`
	`91`	`+ // Block types need to be copied because they could be stack blocks.`
	`92`	`+ // However, non-escaping blocks have a lifetime that is stack-based and they`
	`93`	`+ // treat copy/release as a no-op. For details see:`
	`94`	`+ // https://reviews.llvm.org/rGdbfa453e4138bb977644929c69d1c71e5e8b4bee`
	`95`	`+ // If we keep a reference to a non-escaping block in retainedArguments, it`
	`96`	`+ // will end up as dangling pointer, resulting in a crash later.`
	`97`	`+ if(OCMIsNonEscapingBlock(argument) == NO)`
	`98`	`+ {`
	`99`	`+ id blockArgument = [argument copy];`
	`100`	`+ [retainedArguments addObject:blockArgument];`
	`101`	`+ [blockArgument release];`
	`102`	`+ }`
`95`	`103`	`}`
`96`	`104`	`else if(OCMIsClassType(argumentType) && object_isClass(argument))`
`97`	`105`	`{`
`@@ -116,9 +124,13 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude`
`116`	`124`	`{`
`117`	`125`	`if(OCMIsBlockType(returnType))`
`118`	`126`	`{`
`119`		`- id blockReturnValue = [returnValue copy];`
`120`		`- [retainedArguments addObject:blockReturnValue];`
`121`		`- [blockReturnValue release];`
	`127`	`+ // See above for an explanation`
	`128`	`+ if(OCMIsNonEscapingBlock(returnValue) == NO)`
	`129`	`+ {`
	`130`	`+ id blockReturnValue = [returnValue copy];`
	`131`	`+ [retainedArguments addObject:blockReturnValue];`
	`132`	`+ [blockReturnValue release];`
	`133`	`+ }`
`122`	`134`	`}`
`123`	`135`	`else`
`124`	`136`	`{`