r242127 - Fix for clang memcpyizer bugs 23911 and 23924 (patch by Denis Zobnin)

Alexey Bataev a.bataev at hotmail.com
Tue Jul 14 00:55:49 PDT 2015


Author: abataev
Date: Tue Jul 14 02:55:48 2015
New Revision: 242127

URL: http://llvm.org/viewvc/llvm-project?rev=242127&view=rev
Log:
Fix for clang memcpyizer bugs 23911 and 23924 (patch by Denis Zobnin)

The fix is to remove duplicate copy-initialization of the only memcpy-able struct member and to correct the address of aggregately initialized members in destructors' calls during stack unwinding (in order to obtain address of struct member by using GEP instead of 'bitcast').
Differential Revision: http://reviews.llvm.org/D10990

Added:
    cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp   (with props)
    cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp   (with props)
Modified:
    cfe/trunk/lib/CodeGen/CGClass.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=242127&r1=242126&r2=242127&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Tue Jul 14 02:55:48 2015
@@ -554,6 +554,20 @@ static bool isMemcpyEquivalentSpecialMem
   return false;
 }
 
+static void EmitLValueForAnyFieldInitialization(CodeGenFunction &CGF,
+                                                CXXCtorInitializer *MemberInit,
+                                                LValue &LHS) {
+  FieldDecl *Field = MemberInit->getAnyMember();
+  if (MemberInit->isIndirectMemberInitializer()) {
+    // If we are initializing an anonymous union field, drill down to the field.
+    IndirectFieldDecl *IndirectField = MemberInit->getIndirectMember();
+    for (const auto *I : IndirectField->chain())
+      LHS = CGF.EmitLValueForFieldInitialization(LHS, cast<FieldDecl>(I));
+  } else {
+    LHS = CGF.EmitLValueForFieldInitialization(LHS, Field);
+  }
+}
+
 static void EmitMemberInitializer(CodeGenFunction &CGF,
                                   const CXXRecordDecl *ClassDecl,
                                   CXXCtorInitializer *MemberInit,
@@ -572,16 +586,7 @@ static void EmitMemberInitializer(CodeGe
   QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl);
   LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
 
-  if (MemberInit->isIndirectMemberInitializer()) {
-    // If we are initializing an anonymous union field, drill down to
-    // the field.
-    IndirectFieldDecl *IndirectField = MemberInit->getIndirectMember();
-    for (const auto *I : IndirectField->chain())
-      LHS = CGF.EmitLValueForFieldInitialization(LHS, cast<FieldDecl>(I));
-    FieldType = MemberInit->getIndirectMember()->getAnonField()->getType();
-  } else {
-    LHS = CGF.EmitLValueForFieldInitialization(LHS, Field);
-  }
+  EmitLValueForAnyFieldInitialization(CGF, MemberInit, LHS);
 
   // Special case: if we are in a copy or move constructor, and we are copying
   // an array of PODs or classes with trivial copy constructors, ignore the
@@ -1072,6 +1077,7 @@ namespace {
           CopyingValueRepresentation CVR(CGF);
           EmitMemberInitializer(CGF, ConstructorDecl->getParent(),
                                 AggregatedInits[0], ConstructorDecl, Args);
+          AggregatedInits.clear();
         }
         reset();
         return;
@@ -1088,10 +1094,14 @@ namespace {
       LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
 
       for (unsigned i = 0; i < AggregatedInits.size(); ++i) {
-        QualType FieldType = AggregatedInits[i]->getMember()->getType();
+        CXXCtorInitializer *MemberInit = AggregatedInits[i];
+        QualType FieldType = MemberInit->getAnyMember()->getType();
         QualType::DestructionKind dtorKind = FieldType.isDestructedType();
-        if (CGF.needsEHCleanup(dtorKind))
-          CGF.pushEHDestroy(dtorKind, LHS.getAddress(), FieldType);
+        if (!CGF.needsEHCleanup(dtorKind))
+          continue;
+        LValue FieldLHS = LHS;
+        EmitLValueForAnyFieldInitialization(CGF, MemberInit, FieldLHS);
+        CGF.pushEHDestroy(dtorKind, FieldLHS.getAddress(), FieldType);
       }
     }
 

Added: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp?rev=242127&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp Tue Jul 14 02:55:48 2015
@@ -0,0 +1,47 @@
+// Check that destructors of memcpy-able struct members are called properly
+// during stack unwinding after an exception.
+//
+// Check that destructor's argument (address of member to be destroyed) is
+// obtained by taking offset from struct, not by bitcasting pointers.
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -O0 -fno-elide-constructors -emit-llvm %s -o - | FileCheck %s
+
+struct ImplicitCopy {
+  int id;
+  ImplicitCopy() { id = 10; }
+  ~ImplicitCopy() { id = 20; }
+};
+
+struct ThrowCopy {
+  int id;
+  ThrowCopy() { id = 15; }
+  ThrowCopy(const ThrowCopy &x) {
+    id = 25;
+    throw 1;
+  }
+  ~ThrowCopy() { id = 35; }
+};
+
+struct Container {
+  int id;
+  ImplicitCopy o1;
+  ThrowCopy o2;
+
+  Container() { id = 1000; }
+  ~Container() { id = 2000; }
+};
+
+int main() {
+  try {
+    Container c1;
+    // CHECK-LABEL: main
+    // CHECK: %{{.+}} = getelementptr inbounds %struct.Container, %struct.Container* %{{.+}}, i32 0, i32 1
+    // CHECK-NOT: %{{.+}} = bitcast %struct.Container* %{{.+}} to %struct.ImplicitCopy*
+    Container c2(c1);
+
+    return 2;
+  } catch (...) {
+    return 1;
+  }
+  return 0;
+}

Propchange: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Rev URL

Propchange: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits-unwind.cpp
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp?rev=242127&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp Tue Jul 14 02:55:48 2015
@@ -0,0 +1,46 @@
+// Check that initialization of the only one memcpy-able struct member will not
+// be performed twice after successful non-trivial initializtion of the second
+// member.
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -O0 -fno-elide-constructors -emit-llvm %s -o - | FileCheck %s
+
+int globId = 0;
+
+struct ImplicitCopy {
+  int id;
+
+  ImplicitCopy() { id = 10; }
+  ~ImplicitCopy() { id = 20; }
+};
+
+struct ExplicitCopy {
+  int id;
+
+  ExplicitCopy() { id = 15; }
+  ExplicitCopy(const ExplicitCopy &x) { id = 25; }
+  ~ExplicitCopy() { id = 35; }
+};
+
+struct Container {
+  ImplicitCopy o1; // memcpy-able member.
+  ExplicitCopy o2; // non-trivial initialization.
+
+  Container() { globId = 1000; }
+  ~Container() { globId = 2000; }
+};
+
+int main() {
+  try {
+    Container c1;
+    // CHECK-DAG: call void @llvm.memcpy
+    // CHECK-DAG: declare void @llvm.memcpy
+    // CHECK-NOT: @llvm.memcpy
+    Container c2(c1);
+
+    return 2;
+  }
+  catch (...) {
+    return 1;
+  }
+  return 0;
+}

Propchange: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Rev URL

Propchange: cfe/trunk/test/CodeGenCXX/eh-aggregated-inits.cpp
------------------------------------------------------------------------------
    svn:mime-type = text/plain





More information about the cfe-commits mailing list