[CodeGen] Fix assignments of inline layouts into the byref structure

[vsk]

Summary

[CodeGen] Fix assignments of inline layouts into the byref structure

When using blocks, a byref structure is created to represent the
closure. The "byref.layout" field of this structure is an i8*. However,
some 'inline' layouts are represented as i64's, not i8*'s.

Prior to r246985 we cast the i64 'inline' layout to an i8* before
assigning it into the byref structure. This patch brings the cast back
and adds a regression test.

rdar://23713871

[rjmccall]

You don't need a CodeGenFunction for this; just use llvm::ConstantExpr::getBitCast. Please document the requirement that this returns an i8* in CGObjCRuntime.h.

[vsk]

• Update patch according to John's comments.

[rjmccall]

Thank you, that's great. LGTM.

[vsk]

Thanks for the review. Committed r256185

[vsk]

Closing old review.

lib/CodeGen/CGBlocks.cpp

   if (helpers) {
     storeHeaderField(helpers->CopyHelper, getPointerSize(),
                      "byref.copyHelper");
     storeHeaderField(helpers->DisposeHelper, getPointerSize(),
                      "byref.disposeHelper");
   }
 
   if (ByRefHasLifetime && HasByrefExtendedLayout) {
-    auto layoutInfo = CGM.getObjCRuntime().BuildByrefLayout(CGM, type);
+    auto layoutInfo = CGM.getObjCRuntime().BuildByrefLayout(CGM, *this, type);
     storeHeaderField(layoutInfo, getPointerSize(), "byref.layout");
   }
 }
 
 void CodeGenFunction::BuildBlockRelease(llvm::Value *V, BlockFieldFlags flags) {
   llvm::Value *F = CGM.getBlockObjectDispose();
   llvm::Value *args[] = {
     Builder.CreateBitCast(V, Int8PtrTy),

lib/CodeGen/CGObjCGNU.cpp

                                      const CGBlockInfo &blockInfo) override {
     return NULLPtr;
   }
   llvm::Constant *BuildRCBlockLayout(CodeGenModule &CGM,
                                      const CGBlockInfo &blockInfo) override {
     return NULLPtr;
   }
 
-  llvm::Constant *BuildByrefLayout(CodeGenModule &CGM, QualType T) override {
+  llvm::Value *BuildByrefLayout(CodeGenModule &CGM, CodeGenFunction &CGF,
+                                QualType T) override {
     return NULLPtr;
   }
 
   llvm::GlobalVariable *GetClassGlobal(const std::string &Name,
                                        bool Weak = false) override {
     return nullptr;
   }
 };

lib/CodeGen/CGObjCMac.cpp

   /// forward references will be filled in with empty bodies if no
   /// definition is seen. The return value has type ProtocolPtrTy.
   virtual llvm::Constant *GetOrEmitProtocolRef(const ObjCProtocolDecl *PD)=0;
   llvm::Constant *BuildGCBlockLayout(CodeGen::CodeGenModule &CGM,
                                      const CGBlockInfo &blockInfo) override;
   llvm::Constant *BuildRCBlockLayout(CodeGen::CodeGenModule &CGM,
                                      const CGBlockInfo &blockInfo) override;
 
-  llvm::Constant *BuildByrefLayout(CodeGen::CodeGenModule &CGM,
-                                   QualType T) override;
+  llvm::Value *BuildByrefLayout(CodeGen::CodeGenModule &CGM,
+                                CodeGen::CodeGenFunction &CGF,
+                                QualType T) override;
 };
 
 class CGObjCMac : public CGObjCCommonMac {
 private:
   ObjCTypesHelper ObjCTypes;
 
   /// EmitModuleInfo - Another marker encoding module level
   /// information.

       fieldSize = CGM.getContext().getTypeSizeInChars(type);
     UpdateRunSkipBlockVars(CI.isByRef(), getBlockCaptureLifetime(type, false),
                            fieldOffset, fieldSize);
   }
   return getBitmapBlockLayout(false);
 }
 
 
-llvm::Constant *CGObjCCommonMac::BuildByrefLayout(CodeGen::CodeGenModule &CGM,
-                                                  QualType T) {
+llvm::Value *CGObjCCommonMac::BuildByrefLayout(CodeGen::CodeGenModule &CGM,
+                                               CodeGen::CodeGenFunction &CGF,
+                                               QualType T) {
   assert(CGM.getLangOpts().getGC() == LangOptions::NonGC);
   assert(!T->isArrayType() && "__block array variable should not be caught");
   CharUnits fieldOffset;
   RunSkipBlockVars.clear();
   bool hasUnion = false;
   if (const RecordType *record = T->getAs<RecordType>()) {
     BuildRCBlockVarRecordLayout(record, fieldOffset, hasUnion, true /*ByrefLayout */);
     llvm::Constant *Result = getBitmapBlockLayout(true);
-    return Result;
+    if (isa<llvm::ConstantInt>(Result))
+      return CGF.Builder.CreateIntToPtr(Result, CGM.Int8PtrTy,
+                                        "byref.layout_cast");
+    else
+      return Result;
   }
   llvm::Constant *nullPtr = llvm::Constant::getNullValue(CGM.Int8PtrTy);
   return nullPtr;
 }
 
 llvm::Value *CGObjCMac::GenerateProtocolRef(CodeGenFunction &CGF,
                                             const ObjCProtocolDecl *PD) {
   // FIXME: I don't understand why gcc generates this, or where it is

lib/CodeGen/CGObjCRuntime.h

   virtual void EmitGCMemmoveCollectable(CodeGen::CodeGenFunction &CGF,
                                         Address DestPtr,
                                         Address SrcPtr,
                                         llvm::Value *Size) = 0;
   virtual llvm::Constant *BuildGCBlockLayout(CodeGen::CodeGenModule &CGM,
                                   const CodeGen::CGBlockInfo &blockInfo) = 0;
   virtual llvm::Constant *BuildRCBlockLayout(CodeGen::CodeGenModule &CGM,
                                   const CodeGen::CGBlockInfo &blockInfo) = 0;
-  virtual llvm::Constant *BuildByrefLayout(CodeGen::CodeGenModule &CGM,
-                                           QualType T) = 0;
+  virtual llvm::Value *BuildByrefLayout(CodeGen::CodeGenModule &CGM,
+                                        CodeGen::CodeGenFunction &CGF,
+                                        QualType T) = 0;
   virtual llvm::GlobalVariable *GetClassGlobal(const std::string &Name,
                                                bool Weak = false) = 0;
 
   struct MessageSendInfo {
     const CGFunctionInfo &CallInfo;
     llvm::PointerType *MessengerType;
 
     MessageSendInfo(const CGFunctionInfo &callInfo,

test/CodeGenObjCXX/blocks.mm

 }
 
 // PR24780
 class CaptureThisAndAnotherPointer {
   void test(void *ptr) {
     takeBlock(^{ useValues(ptr, this); });
   }
 };
+
+// rdar://problem/23713871
+// Check that we don't crash when using BLOCK_LAYOUT_STRONG.
+#pragma clang assume_nonnull begin
+@interface NSUUID @end
+#pragma clang assume_nonnull end
+
+struct Wrapper1 { NSUUID *Ref; };
+struct Wrapper2 { Wrapper1 W1; };
+
+@implementation B
+- (void) captureStrongRef {
+  __block Wrapper2 W2;
+}
+@end