Refactored dtor sanitizing into EHScopeStack

[nmusgrave]

Summary

Dtor sanitization handled amidst other dtor cleanups,
between cleaning bases and fields. Sanitizer call pushed onto
stack of cleanup operations.

[nmusgrave]

• Refactoring dtor sanitizing emission order
• support for virtual functions & virtual bases WIP

[nmusgrave]

• Refactoring dtor sanitizing emission order
• support for virtual functions & virtual bases WIP
• Repress dtor aliasing when sanitizing in dtor

[eugenis]

This patch needs tests.

lib/CodeGen/CGCXX.cpp

43

We should still emit as alias if the derived class has no new fields. Effectively, you need to check the same conditions as in SanitizeDtor class, to avoid pointlessly suppressing alias generation in cases when we end up not poisoning any memory.

Also, this should take the msan function attribute into account.

Don't fold it into the same statement as CXXCtorDtorAliases, and add a comment explaining why this is done.

lib/CodeGen/CGClass.cpp

1539

This is no longer true.

1543

extra empty line

1588

Move this line above to group it with related code (Fn assignment).

[nmusgrave]

In D12022#226237, @eugenis wrote:

This patch needs tests.

The runtime test dtor-derived-class.cpp fails without this patch. I'll write a cfe test as well to verify the virtual function table is properly initialized

[eugenis]

In D12022#228180, @nmusgrave wrote:

In D12022#226237, @eugenis wrote:

This patch needs tests.

The runtime test dtor-derived-class.cpp fails without this patch. I'll write a cfe test as well to verify the virtual function table is properly initialized

A good test in cfe would involve a class with a base class, a virtual base class, and a non-trivial member, and would check the order of calls in the derived class destructor.

[eugenis]

+cfe-commits

[nmusgrave]

• CFE test for dtor aliasing, and repression of aliasing in dtor code generation.

[nmusgrave]

• More complex testing for destruction order. Tests class with base, virtual base, trivial, and nontrivial member to ensure destruction order is correct.

[eugenis]

The test for virtual base that you added effectively disables the aliasing test, because Derived now has non-trivial members.
Move it to a separate file.

test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

39

I don't think this is interesting.

48

That's a lot of checks.
How about:
CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
CHECK-NOT: ret void
CHECK: call void {{.*}}ZN7DerivedD2Ev
CHECK-NOT: ret void
CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
CHECK: ret void

And the same in ZN7DerivedD2Ev.
And remove all other destructors.

[nmusgrave]

• Poisoning on field-by-field basis, with collective poisoning of trivial members when possible.
• Cleaned up implementation of calculating region to poison in dtor.

[eugenis]

lib/CodeGen/CGCXX.cpp

49

I'm not sure this is correct. It says not to use an alias if D has trivial body. Should not it be the other way around?

[nmusgrave]

lib/CodeGen/CGCXX.cpp

49

It's counter-intuitive:
TryEmitBaseDestructorAsAlias returns false when TryEmitDefinitionAsAlias returns false.
TryEmitDefinitionAsAlias returns false when an alias is successfully created (line 216 in this CL)

[nmusgrave]

lib/CodeGen/CGCXX.cpp

49

For example: line 3711 of clang/lib/CodeGen/MicrosoftCXXABI.cpp
bool ProducedAlias = !CGM.TryEmitDefinitionAsAlias(

GlobalDecl(dtor, Dtor_Complete), GlobalDecl(dtor, Dtor_Base), true);

^uses the negation of the function's return value

[eugenis]

lib/CodeGen/CGClass.cpp

1576

Why do you need to special-case pointers?

[nmusgrave]

lib/CodeGen/CGClass.cpp

1576

FieldHasTrivialDestructorBody doesn't catch pointers- it identifies their base type as some class, and returns false

[nmusgrave]

• Refactoring dtor sanitizing emission order
• support for virtual functions & virtual bases WIP
• Repress dtor aliasing when sanitizing in dtor
• CFE test for dtor aliasing, and repression of aliasing in dtor code generation.
• More complex testing for destruction order.
• Poison trivial members one-by-one.
• Poisoning on field-by-field basis, with collective poisoning of trivial members when possible.
• Cleaned up implementation of calculating region to poison in dtor.
• Checking for existence of a single trivial field.

[nmusgrave]

• Refactored for simpler trivial field checking

[eugenis]

lib/CodeGen/CGCXX.cpp

45

This is unused.

48

This is unnecessary:
Layout.getFieldCount() > 0

126

This function is called from TryEmitBaseDestructorAsAlias.
This check is stronger than the one in the calling function, so that one has no effect.
Maybe move the check to this function?

lib/CodeGen/CGClass.cpp

1352

Remove context from arguments, use CGF.getContext()
Consider making this a method of CodegenFunction

1485

extra line

1496

unrelated changes

1706

1. Only do this if sanitizing.
2. I think this needs to be NormalAndEHCleanup

[nmusgrave]

• Simplify function invocations

[nmusgrave]

• Check flags before dtor sanitizing

[eugenis]

lib/CodeGen/CGCXX.cpp

31

I think this should be called "HasFieldWithTrivialDestructor".
Also, pass D->getParent() to this function instead of D.

[nmusgrave]

• Simplify parameters, rename function, for examining fields of class to destroy.

[eugenis]

Richard, would you mind taking a look a this change?

[nmusgrave]

• Checking for existence of fields to poison in alias emission.

[nmusgrave]

test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

31

Its checking that my aliasing is repressed when appropriate.Should I keep or get rid of this test case?

[eugenis]

test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

31

Hm, it looks like my comment was for some older version of this code.
Keep it.
You can make it a bit more resistant against unrelated changes. We only need to check that both D1 and D2 destructors appear in the vtable, and don't care about any of the i8* casts. Smth like this:
@_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev

[nmusgrave]

• Alias-repressing test case ignores casting of pointers.

[rsmith]

lib/CodeGen/CGCXX.cpp

42–44

I assume the rationale here is that the field and base class destructors will mark their storage as destroyed, so there's nothing else to mark? This may mean that vptrs and padding bytes don't get marked as destroyed, is that OK?

45–47

This check seems to be redundant, since TryEmitDefinitionAsAlias checks the same thing. Remove this copy?

128

This function isn't (or wasn't) specific to destructors; I think you should be checking for this as part of your if below rather than asserting it.

129–131

Please reorder this after the quicker, higher-level checks below. (Maybe move it to just before we create the mangled name?)

lib/CodeGen/CGClass.cpp

1334–1335

This appears redundant; we'd return true for pointers on line 1341 anyway. (getBaseElementType only strips off array types, not pointer types).

1546

Add full stop.

1551

You're only counting the direct fields here, not the inherited ones.

1552–1555

unsigned totalFields = std::distance(Dtor->getParent()->field_begin(),
                                     Dtor->getParent()->field_end());

1564

todo -> TODO (or FIXME)

1618

I think this is off by one. layoutEndOffset is the index of the first field that is not poisoned, so this will trigger if we're poisoning up to, but not including, the last field, as well as triggering if we're poisoning the last field.

1641–1642

Maybe do this once in Emit (if you poison anything) rather than doing it every time you emit a poison call?

1706

Please add a comment here. "Mark the lifetime of fields as ending after field destructors run and before we destroy base classes." or similar.

[eugenis]

lib/CodeGen/CGCXX.cpp

42–44

Yes, each (sub-)object poisons its storage in the base destructor, and only does it for fields that don't have their own destructors to avoid double-poisoning.

Poisoning vptr would be interesting. Yes, current implementation fails to do this. Padding - less so, but it would make the tool a little bit more predictable.

[nmusgrave]

• Simplified implementation of class field poisoning, to avoid redundant counting and conditional checks. Expensive checks delayed.

[nmusgrave]

lib/CodeGen/CGCXX.cpp

42–44

vptr poisoning will be implemented in another CL after this is approved

[eugenis]

lib/CodeGen/CGCXX.cpp

45

This simply suppresses all dtor alias under UseAfterDtor, effectively disabling the second check below.
Remove it.

124

formatting

141

... with trivial destructors

lib/CodeGen/CGClass.cpp

1547

Probably better check Dtor->getParent()->field_empty() for consistency.

1551

disable tail calls only when !field_empty()

[nmusgrave]

• Update comments, consistent style for attribute checking.

[eugenis]

LGTM but please wait for Richard's answer

lib/CodeGen/CGClass.cpp

1551

oh, right, that's already the case.

[rsmith]

I'd like to see some tests for poisoning bitfields, and particularly anonymous bitfields (I think the code works fine in those cases, but they're a bit special so explicitly testing them is useful).

lib/CodeGen/CGCXX.cpp

145

&& goes on previous line.

lib/CodeGen/CGClass.cpp

1547

This check is equivalent and more efficient.

1559

Can you use a range for?

1587

Doxygenize this comment (use ///, \param).

[nmusgrave]

• Testing sanitizing bit fields.

[nmusgrave]

• Refined testing for bit fields.

[rsmith]

lib/CodeGen/CGCXX.cpp

140–147

Having checked a bit more, I think this test should go in TryEmitBaseDestructorAsAlias: it's OK to emit the complete object destructor for a class as an alias for the base subobject constructor for the same class even if this sanitizer is enabled, but it's not OK to emit the destructor for the class as an alias for a base class's destructor (which is what TryEmitBaseDestructorAsAlias is checking).

The check then becomes a lot simpler: with the sanitizer enabled, for each field we will do exactly one of (a) invoke a destructor (which will poison the memory) or (b) poison the memory ourselves. So if the sanitizer is enabled, we can emit an alias only if the derived class has no fields.

lib/CodeGen/CGClass.cpp

1559–1560

Separately declaring F and Field seems unnecessary; how about:

for (const FieldDecl *Field : Dtor->getParent()->fields()) {

1592–1593

Initialize these as you declare them, rather than separately:

llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
  // ...
llvm::Value *OffsetPtr = // ...

test/CodeGenCXX/sanitize-dtor-bit-field.cpp

2

It'd also be good to test for the case where a bit-field is adjacent to non-sanitized fields:

struct A { char c; ~A(); };
struct B {
  A x;
  int n : 1;
  A y;
};

[nmusgrave]

• Simplified fields and checks for aliasing.

[rsmith]

LGTM with a couple of tweaks.

lib/CodeGen/CGCXX.cpp

52–53

You can just use !D->getParent()->field_empty()

lib/CodeGen/CGClass.cpp

1289

I think you no longer need to make these functions members.

[nmusgrave]

• Clean method headers, style.

lib/CodeGen/CGCXX.cpp

 #include "clang/AST/Mangle.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/StringExtras.h"
 using namespace clang;
 using namespace CodeGen;
 
+static bool HasFieldWithTrivialDestructor(CodeGenModule &CGM,
+                                          const CXXRecordDecl *D) {
+  for (const auto *Field : D->fields())
+    if (CGM.FieldHasTrivialDestructorBody(CGM.getContext(), Field))
+      return true;
+  return false;
+}
+
 /// Try to emit a base destructor as an alias to its primary
 /// base-class destructor.
 bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
   if (!getCodeGenOpts().CXXCtorDtorAliases)
     return true;
 
   // Producing an alias to a base class ctor/dtor can degrade debug quality
   // as the debugger cannot tell them apart.
   if (getCodeGenOpts().OptimizationLevel == 0)
     return true;
 
   // If the destructor doesn't have a trivial body, we have to emit it
   // separately.
   if (!D->hasTrivialBody())
     return true;
 
   const CXXRecordDecl *Class = D->getParent();
 
   // We are going to instrument this destructor, so give up even if it is
   // currently empty.
   if (Class->mayInsertExtraPadding())
     return true;
 

 }
 
 /// Try to emit a definition as a global alias for another definition.
 /// If \p InEveryTU is true, we know that an equivalent alias can be produced
 /// in every translation unit.
 bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
                                              GlobalDecl TargetDecl,
                                              bool InEveryTU) {
   if (!getCodeGenOpts().CXXCtorDtorAliases)
     return true;
 
   // The alias will use the linkage of the referent.  If we can't
   // support aliases with that linkage, fail.
   llvm::GlobalValue::LinkageTypes Linkage = getFunctionLinkage(AliasDecl);
 
   // We can't use an alias if the linkage is not valid for one.
   if (!llvm::GlobalAlias::isValidLinkage(Linkage))
     return true;
 
   // Don't create a weak alias for a dllexport'd symbol.
   if (AliasDecl.getDecl()->hasAttr<DLLExportAttr>() &&
       llvm::GlobalValue::isWeakForLinker(Linkage))
     return true;
 
+  // If sanitizing memory to check for use-after-dtor, do not emit as
+  //  an alias, unless this class owns no members with trivial destructors.
+  const CXXMethodDecl *MD =
+      cast<CXXMethodDecl>(AliasDecl.getDecl())->getCanonicalDecl();
+  if (isa<CXXDestructorDecl>(MD) &&
+      getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+      HasFieldWithTrivialDestructor(*this, MD->getParent()))
+    return true;
+
   llvm::GlobalValue::LinkageTypes TargetLinkage =
       getFunctionLinkage(TargetDecl);
 
   // Check if we have it already.
   StringRef MangledName = getMangledName(AliasDecl);
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
   if (Entry && !Entry->isDeclaration())
     return false;

lib/CodeGen/CGClass.cpp

for (; B != E; B++) {
     assert(!Member->isBaseInitializer());
     assert(Member->isAnyMemberInitializer() &&
            "Delegating initializer on non-delegating constructor");
     CM.addMemberInitializer(Member);
   }
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext &Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext &Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext &Context,
                          const CXXRecordDecl *BaseClassDecl,
                          const CXXRecordDecl *MostDerivedClassDecl)
 {
   // If the destructor is trivial we don't have to check anything else.
   if (BaseClassDecl->hasTrivialDestructor())
     return true;
 
   if (!BaseClassDecl->getDestructor()->hasTrivialBody())
for (const auto &I : BaseClassDecl->vbases()) {
                                     MostDerivedClassDecl))
         return false;
     }
   }
 
   return true;
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext &Context,
+bool CodeGenModule::FieldHasTrivialDestructorBody(ASTContext &Context,
                                                   const FieldDecl *Field)
 {
   QualType FieldBaseElementType = Context.getBaseElementType(Field->getType());
 
   const RecordType *RT = FieldBaseElementType->getAs<RecordType>();
   if (!RT)
     return true;
 
   CXXRecordDecl *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
 
   // The destructor for an implicit anonymous union member is never invoked.
   if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion())
     return false;
 
   return HasTrivialDestructorBody(Context, FieldClassDecl, FieldClassDecl);
 }
 
 /// CanSkipVTablePointerInitialization - Check whether we need to initialize
 /// any vtable pointers before calling this destructor.
-static bool CanSkipVTablePointerInitialization(ASTContext &Context,
+static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
                                                const CXXDestructorDecl *Dtor) {
   if (!Dtor->hasTrivialBody())
     return false;
 
   // Check the fields.
   const CXXRecordDecl *ClassDecl = Dtor->getParent();
   for (const auto *Field : ClassDecl->fields())
-    if (!FieldHasTrivialDestructorBody(Context, Field))
+    if (!CGF.CGM.FieldHasTrivialDestructorBody(CGF.getContext(), Field))
       return false;
 
   return true;
 }
 
-// Generates function call for handling object poisoning, passing in
-// references to 'this' and its size as arguments.
-// Disables tail call elimination, to prevent the current stack frame from
-// disappearing from the stack trace.
-static void EmitDtorSanitizerCallback(CodeGenFunction &CGF,
-                                      const CXXDestructorDecl *Dtor) {
-  const ASTRecordLayout &Layout =
-      CGF.getContext().getASTRecordLayout(Dtor->getParent());
-
-  // Nothing to poison
-  if(Layout.getFieldCount() == 0)
-    return;
-
-  // Construct pointer to region to begin poisoning, and calculate poison
-  // size, so that only members declared in this class are poisoned.
-  llvm::Value *OffsetPtr;
-  CharUnits::QuantityType PoisonSize;
-  ASTContext &Context = CGF.getContext();
-
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-      CGF.SizeTy, Context.toCharUnitsFromBits(Layout.getFieldOffset(0)).
-      getQuantity());
-
-  OffsetPtr = CGF.Builder.CreateGEP(CGF.Builder.CreateBitCast(
-      CGF.LoadCXXThis(), CGF.Int8PtrTy), OffsetSizePtr);
-
-  PoisonSize = Layout.getSize().getQuantity() -
-      Context.toCharUnitsFromBits(Layout.getFieldOffset(0)).getQuantity();
-
-  llvm::Value *Args[] = {
-    CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
-    llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
-
-  llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
-
-  llvm::FunctionType *FnType =
-      llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
-  llvm::Value *Fn =
-      CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
-
-  // Disables tail call elimination, to prevent the current stack frame from
-  // disappearing from the stack trace.
-  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
-  CGF.EmitNounwindRuntimeCall(Fn, Args);
-}
-
 /// EmitDestructorBody - Emits the body of the current destructor.
 void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CurGD.getDecl());
   CXXDtorType DtorType = CurGD.getDtorType();
 
   Stmt *Body = Dtor->getBody();
   if (Body)
     incrementProfileCounter(Body);
void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
 
   case Dtor_Base:
     assert(Body);
 
     // Enter the cleanup scopes for fields and non-virtual bases.
     EnterDtorCleanups(Dtor, Dtor_Base);
 
     // Initialize the vtable pointers before entering the body.
-    if (!CanSkipVTablePointerInitialization(getContext(), Dtor))
+    if (!CanSkipVTablePointerInitialization(*this, Dtor))
         InitializeVTablePointers(Dtor->getParent());
 
     if (isTryBody)
       EmitStmt(cast<CXXTryStmt>(Body)->getTryBlock());
     else if (Body)
       EmitStmt(Body);
     else {
       assert(Dtor->isImplicit() && "bodyless dtor not implicit");
       // nothing to do besides what's in the epilogue
     }
     // -fapple-kext must inline any call to this dtor into
     // the caller's body.
     if (getLangOpts().AppleKext)
       CurFn->addFnAttr(llvm::Attribute::AlwaysInline);
 
-    // Insert memory-poisoning instrumentation, before final clean ups,
-    // to ensure this class's members are protected from invalid access.
-    if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor
-        && SanOpts.has(SanitizerKind::Memory))
-      EmitDtorSanitizerCallback(*this, Dtor);
-
     break;
   }
 
   // Jump out through the epilogue cleanups.
   DtorEpilogue.ForceCleanup();
 
   // Exit the try if applicable.
   if (isTryBody)
void Emit(CodeGenFunction &CGF, Flags flags) override {
       const CXXRecordDecl *ClassDecl = Dtor->getParent();
       CGF.EmitDeleteCall(Dtor->getOperatorDelete(), CGF.LoadCXXThis(),
                          CGF.getContext().getTagDeclType(ClassDecl));
     }
   };
 
   struct CallDtorDeleteConditional final : EHScopeStack::Cleanup {
     llvm::Value *ShouldDeleteCondition;
   public:
     CallDtorDeleteConditional(llvm::Value *ShouldDeleteCondition)
         : ShouldDeleteCondition(ShouldDeleteCondition) {
       assert(ShouldDeleteCondition != nullptr);
     }
 
     void Emit(CodeGenFunction &CGF, Flags flags) override {
       llvm::BasicBlock *callDeleteBB = CGF.createBasicBlock("dtor.call_delete");
       llvm::BasicBlock *continueBB = CGF.createBasicBlock("dtor.continue");
       llvm::Value *ShouldCallDelete
         = CGF.Builder.CreateIsNull(ShouldDeleteCondition);
       CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);
 
       CGF.EmitBlock(callDeleteBB);
       const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CGF.CurCodeDecl);
       const CXXRecordDecl *ClassDecl = Dtor->getParent();
       CGF.EmitDeleteCall(Dtor->getOperatorDelete(), CGF.LoadCXXThis(),
                          CGF.getContext().getTagDeclType(ClassDecl));
       CGF.Builder.CreateBr(continueBB);
 
       CGF.EmitBlock(continueBB);
     }
   };
 
   class DestroyField  final : public EHScopeStack::Cleanup {
     const FieldDecl *field;
     CodeGenFunction::Destroyer *destroyer;
     bool useEHCleanupForArray;
 
   public:
     DestroyField(const FieldDecl *field, CodeGenFunction::Destroyer *destroyer,
                  bool useEHCleanupForArray)
         : field(field), destroyer(destroyer),
           useEHCleanupForArray(useEHCleanupForArray) {}
 
     void Emit(CodeGenFunction &CGF, Flags flags) override {
       // Find the address of the field.
       llvm::Value *thisValue = CGF.LoadCXXThis();
       QualType RecordTy = CGF.getContext().getTagDeclType(field->getParent());
       LValue ThisLV = CGF.MakeAddrLValue(thisValue, RecordTy);
       LValue LV = CGF.EmitLValueForField(ThisLV, field);
       assert(LV.isSimple());
 
       CGF.emitDestroy(LV.getAddress(), field->getType(), destroyer,
                       flags.isForNormalCleanup() && useEHCleanupForArray);
     }
   };
+
+  class SanitizeDtor final : public EHScopeStack::Cleanup {
+    const CXXDestructorDecl *Dtor;
+
+  public:
+    SanitizeDtor(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+
+    // Generate function call for handling object poisoning.
+    // Disables tail call elimination, to prevent the current stack frame
+    // from disappearing from the stack trace.
+    void Emit(CodeGenFunction &CGF, Flags flags) override {
+      const ASTRecordLayout &Layout =
+          CGF.getContext().getASTRecordLayout(Dtor->getParent());
+
+      // Nothing to poison.
+      if (Layout.getFieldCount() == 0)
+        return;
+
+      // Prevent the current stack frame from disappearing from the stack trace.
+      CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+
+      // Construct pointer to region to begin poisoning, and calculate poison
+      // size, so that only members declared in this class are poisoned.
+      ASTContext &Context = CGF.getContext();
+      unsigned fieldIndex = 0;
+      int startIndex = -1;
+      // RecordDecl::field_iterator Field;
+      for (const auto F : Dtor->getParent()->fields()) {
+        FieldDecl const *Field = F;
+        // Poison field if it is trivial
+        if (CGF.CGM.FieldHasTrivialDestructorBody(Context, Field)) {
+          // Start sanitizing at this field
+          if (startIndex < 0)
+            startIndex = fieldIndex;
+
+          // Currently on the last field, and it must be poisoned with the
+          // current block.
+          if (fieldIndex == Layout.getFieldCount() - 1) {
+            PoisonBlock(CGF, startIndex, Layout.getFieldCount());
+          }
+        } else if (startIndex >= 0) {
+          // No longer within a block of memory to poison, so poison the block
+          PoisonBlock(CGF, startIndex, fieldIndex);
+          // Re-set the start index
+          startIndex = -1;
+        }
+        fieldIndex += 1;
+      }
+    }
+
+  private:
+    /// \param layoutStartOffset: index of the ASTRecordLayout field to
+    ///     start poisoning (inclusive)
+    /// \param layoutEndOffset: index of the ASTRecordLayout field to
+    ///     end poisoning (exclusive)
+    void PoisonBlock(CodeGenFunction &CGF, unsigned layoutStartOffset,
+                     unsigned layoutEndOffset) {
+      ASTContext &Context = CGF.getContext();
+      const ASTRecordLayout &Layout =
+          Context.getASTRecordLayout(Dtor->getParent());
+      llvm::ConstantInt *OffsetSizePtr;
+      llvm::Value *OffsetPtr;
+      CharUnits::QuantityType PoisonSize;
+
+      OffsetSizePtr = llvm::ConstantInt::get(
+          CGF.SizeTy,
+          Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
+              .getQuantity());
+
+      OffsetPtr = CGF.Builder.CreateGEP(
+          CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
+          OffsetSizePtr);
+
+      if (layoutEndOffset >= Layout.getFieldCount()) {
+        PoisonSize = Layout.getNonVirtualSize().getQuantity() -
+                     Context.toCharUnitsFromBits(
+                                Layout.getFieldOffset(layoutStartOffset))
+                         .getQuantity();
+      } else {
+        PoisonSize = Context.toCharUnitsFromBits(
+                                Layout.getFieldOffset(layoutEndOffset) -
+                                Layout.getFieldOffset(layoutStartOffset))
+                         .getQuantity();
+      }
+
+      if (PoisonSize == 0)
+        return;
+
+      // Pass in void pointer and size of region as arguments to runtime
+      // function
+      llvm::Value *Args[] = {CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
+                             llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
+
+      llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
+
+      llvm::FunctionType *FnType =
+          llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
+      llvm::Value *Fn =
+          CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
+      CGF.EmitNounwindRuntimeCall(Fn, Args);
+    }
+  };
 }
 
 /// \brief Emit all code that comes at the end of class's
 /// destructor. This is to call destructors on members and base classes
 /// in reverse order of their construction.
 void CodeGenFunction::EnterDtorCleanups(const CXXDestructorDecl *DD,
                                         CXXDtorType DtorType) {
   assert((!DD->isTrivial() || DD->hasAttr<DLLExportAttr>()) &&
          "Should not emit dtor epilogue for non-exported trivial dtor!");
 
   // The deleting-destructor phase just needs to call the appropriate
   // operator delete that Sema picked up.
   if (DtorType == Dtor_Deleting) {
     assert(DD->getOperatorDelete() &&
            "operator delete missing - EnterDtorCleanups");
     if (CXXStructorImplicitParamValue) {
       // If there is an implicit param to the deleting dtor, it's a boolean
for (const auto &Base : ClassDecl->bases()) {
     if (BaseClassDecl->hasTrivialDestructor())
       continue;
 
     EHStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup,
                                       BaseClassDecl,
                                       /*BaseIsVirtual*/ false);
   }
 
+  // Poison fields such that access after their destructors are
+  // invoked, and before the base class destructor runs, is invalid.
+  if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+      SanOpts.has(SanitizerKind::Memory))
+    EHStack.pushCleanup<SanitizeDtor>(NormalAndEHCleanup, DD);
+
   // Destroy direct fields.
   for (const auto *Field : ClassDecl->fields()) {
     QualType type = Field->getType();
     QualType::DestructionKind dtorKind = type.isDestructedType();
     if (!dtorKind) continue;
 
     // Anonymous union members do not have their destructors called.
     const RecordType *RT = type->getAsUnionType();

lib/CodeGen/CodeGenModule.h

public:
   void addDeferredVTable(const CXXRecordDecl *RD) {
     DeferredVTables.push_back(RD);
   }
 
   /// Emit code for a singal global function or var decl. Forward declarations
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext &Context,
+                           const CXXRecordDecl *BaseClassDecl,
+                           const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext &Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
                                 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
 
   /// Set attributes for a global definition.
   void setFunctionDefinitionAttributes(const FunctionDecl *D,
                                        llvm::Function *F);
 

test/CodeGenCXX/sanitize-dtor-bit-field.cpp

+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+// 24 bytes total
+struct Packed {
+  // Packed into 4 bytes
+  unsigned int a : 1;
+  unsigned int b : 1;
+  //unsigned int c : 1;
+  // Force alignment to next 4 bytes
+  unsigned int   : 0;
+  unsigned int d : 1;
+  // Force alignment, 8 more bytes
+  double e = 5.0;
+  // 4 bytes
+  unsigned int f : 1;
+};
+Packed p;
+
+
+// 1 byte total
+struct Empty {
+  unsigned int : 0;
+};
+Empty e;
+
+
+// 4 byte total
+struct Simple {
+  unsigned int a : 1;
+};
+Simple s;
+
+
+// 8 bytes total
+struct Anon {
+  // 1 byte
+  unsigned int a : 1;
+  unsigned int b : 2;
+  // Force alignment to next byte
+  unsigned int   : 0;
+  unsigned int c : 1;
+};
+Anon a;
+
+struct Foo {
+  int f;
+};
+Foo f;
+
+// CHECK-LABEL: define {{.*}}PackedD2Ev
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 17
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}EmptyD2Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback{{.*}}i64 0
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 1
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}AnonD2Ev
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 5
+// CHECK: ret void

test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp

+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template <class T>
+class Vector {
+public:
+  int size;
+  ~Vector() {
+    size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+    b1 = 5;
+    b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+    vb1 = 10;
+    vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vector<int> v;
+  int d2;
+  Derived() {
+    d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void

test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template <class T>
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vector<int> v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev