ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a subscriber: cfe-commits.

When compiling a constexpr NSString initialized with an objective-c string 
literal, CodeGen currently emits objc_storeStrong on an uninitialized alloca, 
which causes a crash:

  constexpr NSString *S = @"abc";



  %S = alloca %0*, align 8
  %0 = bitcast %0** %S to i8** ; this points to uninitialized memory
  call void @objc_storeStrong(i8** %0, i8* bitcast 
(%struct.__NSConstantString_tag* @_unnamed_cfstring_ to i8*)) #1

This patch fixes the crash by directly storing the pointer to the 
__NSConstantString_tag for the string into the allocated alloca. The rationale 
for this change is that retain or release on an Objective-c string literal (or 
anything used to initialize a constexpr variable) has no effect and therefore 
objc_storeStrong isn't needed in this case.

rdar://problem/28562009


https://reviews.llvm.org/D25547

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenObjCXX/arc-constexpr.mm

Index: test/CodeGenObjCXX/arc-constexpr.mm
===================================================================
--- /dev/null
+++ test/CodeGenObjCXX/arc-constexpr.mm
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s
+
+// CHECK: %[[TYPE:[a-z0-9]+]] = type opaque
+// CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag
+
+// CHECK: define void @_Z5test1v
+// CHECK:   %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]*
+// CHECK:   store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[ALLOCA]]
+// CHECK:   %[[V0:[a-z0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]] to i8**
+// CHECK:   call void @objc_storeStrong(i8** %[[V0]], i8* null)
+
+@class NSString;
+
+void test1() {
+  constexpr NSString *S = @"abc";
+}
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2709,7 +2709,8 @@
   /// EmitStoreThroughLValue - Store the specified rvalue into the specified
   /// lvalue, where both are guaranteed to the have the same type, and that type
   /// is 'Ty'.
-  void EmitStoreThroughLValue(RValue Src, LValue Dst, bool isInit = false);
+  void EmitStoreThroughLValue(RValue Src, LValue Dst, bool isInit = false,
+                              bool IsConstExpr = false);
   void EmitStoreThroughExtVectorComponentLValue(RValue Src, LValue Dst);
   void EmitStoreThroughGlobalRegLValue(RValue Src, LValue Dst);
 
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1593,7 +1593,7 @@
 /// lvalue, where both are guaranteed to the have the same type, and that type
 /// is 'Ty'.
 void CodeGenFunction::EmitStoreThroughLValue(RValue Src, LValue Dst,
-                                             bool isInit) {
+                                             bool isInit, bool IsConstExpr) {
   if (!Dst.isSimple()) {
     if (Dst.isVectorElt()) {
       // Read/modify/write the vector, inserting the new element.
@@ -1619,62 +1619,63 @@
   }
 
   // There's special magic for assigning into an ARC-qualified l-value.
-  if (Qualifiers::ObjCLifetime Lifetime = Dst.getQuals().getObjCLifetime()) {
-    switch (Lifetime) {
-    case Qualifiers::OCL_None:
-      llvm_unreachable("present but none");
+  if (!IsConstExpr) {
+    if (Qualifiers::ObjCLifetime Lifetime = Dst.getQuals().getObjCLifetime()) {
+      switch (Lifetime) {
+      case Qualifiers::OCL_None:
+        llvm_unreachable("present but none");
 
-    case Qualifiers::OCL_ExplicitNone:
-      // nothing special
-      break;
+      case Qualifiers::OCL_ExplicitNone:
+        // nothing special
+        break;
 
-    case Qualifiers::OCL_Strong:
-      EmitARCStoreStrong(Dst, Src.getScalarVal(), /*ignore*/ true);
-      return;
+      case Qualifiers::OCL_Strong:
+        EmitARCStoreStrong(Dst, Src.getScalarVal(), /*ignore*/ true);
+        return;
 
-    case Qualifiers::OCL_Weak:
-      EmitARCStoreWeak(Dst.getAddress(), Src.getScalarVal(), /*ignore*/ true);
-      return;
+      case Qualifiers::OCL_Weak:
+        EmitARCStoreWeak(Dst.getAddress(), Src.getScalarVal(), /*ignore*/ true);
+        return;
 
-    case Qualifiers::OCL_Autoreleasing:
-      Src = RValue::get(EmitObjCExtendObjectLifetime(Dst.getType(),
-                                                     Src.getScalarVal()));
-      // fall into the normal path
-      break;
+      case Qualifiers::OCL_Autoreleasing:
+        Src = RValue::get(
+            EmitObjCExtendObjectLifetime(Dst.getType(), Src.getScalarVal()));
+        // fall into the normal path
+        break;
+      }
     }
-  }
 
-  if (Dst.isObjCWeak() && !Dst.isNonGC()) {
-    // load of a __weak object.
-    Address LvalueDst = Dst.getAddress();
-    llvm::Value *src = Src.getScalarVal();
-     CGM.getObjCRuntime().EmitObjCWeakAssign(*this, src, LvalueDst);
-    return;
-  }
+    if (Dst.isObjCWeak() && !Dst.isNonGC()) {
+      // load of a __weak object.
+      Address LvalueDst = Dst.getAddress();
+      llvm::Value *src = Src.getScalarVal();
+      CGM.getObjCRuntime().EmitObjCWeakAssign(*this, src, LvalueDst);
+      return;
+    }
 
-  if (Dst.isObjCStrong() && !Dst.isNonGC()) {
-    // load of a __strong object.
-    Address LvalueDst = Dst.getAddress();
-    llvm::Value *src = Src.getScalarVal();
-    if (Dst.isObjCIvar()) {
-      assert(Dst.getBaseIvarExp() && "BaseIvarExp is NULL");
-      llvm::Type *ResultType = IntPtrTy;
-      Address dst = EmitPointerWithAlignment(Dst.getBaseIvarExp());
-      llvm::Value *RHS = dst.getPointer();
-      RHS = Builder.CreatePtrToInt(RHS, ResultType, "sub.ptr.rhs.cast");
-      llvm::Value *LHS =
-        Builder.CreatePtrToInt(LvalueDst.getPointer(), ResultType,
-                               "sub.ptr.lhs.cast");
-      llvm::Value *BytesBetween = Builder.CreateSub(LHS, RHS, "ivar.offset");
-      CGM.getObjCRuntime().EmitObjCIvarAssign(*this, src, dst,
-                                              BytesBetween);
-    } else if (Dst.isGlobalObjCRef()) {
-      CGM.getObjCRuntime().EmitObjCGlobalAssign(*this, src, LvalueDst,
-                                                Dst.isThreadLocalRef());
+    if (Dst.isObjCStrong() && !Dst.isNonGC()) {
+      // load of a __strong object.
+      Address LvalueDst = Dst.getAddress();
+      llvm::Value *src = Src.getScalarVal();
+      if (Dst.isObjCIvar()) {
+        assert(Dst.getBaseIvarExp() && "BaseIvarExp is NULL");
+        llvm::Type *ResultType = IntPtrTy;
+        Address dst = EmitPointerWithAlignment(Dst.getBaseIvarExp());
+        llvm::Value *RHS = dst.getPointer();
+        RHS = Builder.CreatePtrToInt(RHS, ResultType, "sub.ptr.rhs.cast");
+        llvm::Value *LHS =
+            Builder.CreatePtrToInt(LvalueDst.getPointer(), ResultType,
+                                   "sub.ptr.lhs.cast");
+        llvm::Value *BytesBetween = Builder.CreateSub(LHS, RHS, "ivar.offset");
+        CGM.getObjCRuntime().EmitObjCIvarAssign(*this, src, dst,
+                                                BytesBetween);
+      } else if (Dst.isGlobalObjCRef()) {
+        CGM.getObjCRuntime().EmitObjCGlobalAssign(*this, src, LvalueDst,
+                                                  Dst.isThreadLocalRef());
+      } else
+        CGM.getObjCRuntime().EmitObjCStrongCastAssign(*this, src, LvalueDst);
+      return;
     }
-    else
-      CGM.getObjCRuntime().EmitObjCStrongCastAssign(*this, src, LvalueDst);
-    return;
   }
 
   assert(Src.isScalar() && "Can't emit an agg store with this method");
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1235,7 +1235,7 @@
     // For simple scalar/complex initialization, store the value directly.
     LValue lv = MakeAddrLValue(Loc, type);
     lv.setNonGC(true);
-    return EmitStoreThroughLValue(RValue::get(constant), lv, true);
+    return EmitStoreThroughLValue(RValue::get(constant), lv, true, true);
   }
 
   // If this is a simple aggregate initialization, we can optimize it
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to