[cfe-commits] r171555 - in /cfe/trunk: lib/CodeGen/CGBlocks.cpp test/CodeGenObjC/arc-unoptimized-byref-var.m

[John McCall]

On Jan 4, 2013, at 4:34 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> On Jan 4, 2013, at 3:56 PM, John McCall wrote:
>> On Jan 4, 2013, at 3:32 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>>> Author: fjahanian
>>> Date: Fri Jan  4 17:32:24 2013
>>> New Revision: 171555
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=171555&view=rev
>>> Log:
>>> objective-C arc: in copy helper function for 
>>> __strong __block variables, perform objc_storeStrong on
>>> source and destination instead of direct move. This 
>>> is done with -O0 and to improve some analysis.
>>> // rdar://12530881
>>> 
>>> Added:
>>>  cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m
>>> Modified:
>>>  cfe/trunk/lib/CodeGen/CGBlocks.cpp
>>> 
>>> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=171555&r1=171554&r2=171555&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Fri Jan  4 17:32:24 2013
>>> @@ -1565,6 +1565,11 @@
>>>   llvm::Value *null =
>>>     llvm::ConstantPointerNull::get(cast<llvm::PointerType>(value->getType()));
>>> 
>>> +    if (CGF.CGM.getCodeGenOpts().OptimizationLevel == 0) {
>>> +      CGF.EmitARCStoreStrongCall(destField, value, /*ignored*/ true);
>>> +      CGF.EmitARCStoreStrongCall(srcField, null, /*ignored*/ true);
>>> +      return;
>>> +    }
>>>   llvm::StoreInst *store = CGF.Builder.CreateStore(value, destField);
>>>   store->setAlignment(Alignment.getQuantity());
>>> 
>>> 
>>> Added: cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m?rev=171555&view=auto
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m (added)
>>> +++ cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m Fri Jan  4 17:32:24 2013
>>> @@ -0,0 +1,14 @@
>>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -o - %s | FileCheck -check-prefix=CHECK-UNOPT %s
>>> +// rdar://12530881
>>> +
>>> +void test19() {
>>> +   __block id x;
>>> +// CHECK-UNOPT: define internal void @__Block_byref_object_copy
>>> +// CHECK-UNOPT: [[X:%.*]] = getelementptr inbounds [[BYREF_T:%.*]]* [[VAR:%.*]], i32 0, i32 6
>>> +// CHECK-UNOPT: [[X2:%.*]] = getelementptr inbounds [[BYREF_T:%.*]]* [[VAR1:%.*]], i32 0, i32 6
>>> +// CHECK-UNOPT-NEXT: [[SIX:%.*]] = load i8** [[X2]], align 8
>>> +// CHECK-UNOPT-NEXT: call void @objc_storeStrong(i8** [[X]], i8* [[SIX]]) nounwind
>>> +// CHECK-UNOPT-NEXT: call void @objc_storeStrong(i8** [[X2]], i8* null) nounwind
>>> +// CHECK-UNOPT-NEXT: ret void
>> 
>> You need to initialize the destination to null (with a simple store) before doing a storeStrong to it.
> In r171572. This is I assume to prevent storeStrong from releasing the old value. I guess this should not happen
> in this context,

Right.  objc_storeStrong is an assignment, not an initialization;  it reads the old value out of the destination and releases it.  When moving a __block variable to the heap, libclosure basically allocates some memory, sets up the byref header, and then checks whether there's a copy helper;  if there is, it invokes it immediately.  Therefore, when we're invoked, the destination address is freshly-allocated memory, and we cannot rely on it having been zero-initialized.

John.