Insert poisoned paddings between fields in C++ classes so that AddressSanitizer can find intra-object-overflow bugs

[kcc]

Summary

The general approach is to add extra paddings after every field
in AST/RecordLayoutBuilder.cpp, then add code to CTORs/DTORs that poisons the paddings
(CodeGen/CGClass.cpp).

Everything is done under the flag -fsanitize-address-field-padding.
The blacklist file (-fsanitize-blacklist) allows to avoid the transformation
for given classes or source files.

See also https://code.google.com/p/address-sanitizer/wiki/IntraObjectOverflow

[rsmith]

lib/AST/RecordLayoutBuilder.cpp

1343–1344

Typo. "Later in CodeGen, extra code will [...]"

1345

This does not depend on the value of FD; perhaps remove FD and just call it once per class?

1347–1348

This might be easier if you insert padding before fields rather than after them.

1350–1351

Maybe factor out adding the padding from adding the field? I don't think they need to be done together, do they?

Also, you shouldn't add padding at the end of the class if it ends in a flexible array member.

1359–1364

What's the reason behind picking these rules? It seems like we could be more aggressive here and still follow the C++ standard. Is this for C compatibility, or people who roll their own memcpy, or something else? I guess the ASan-instrumented memcpy can be taught that it's OK to copy intra-object redzone?

1361–1362

This 'simple destructor' check doesn't seem right. What properties do you intend to check for here?

1366–1376

Please fix =)

I think reusing the same -fsanitize-blacklist flag makes sense.

1378–1384

If you're going to keep this, it should be put behind a remark flag, and should use the normal diagnostics machinery.

lib/CodeGen/CGClass.cpp

705

"ctor" and "dtor" in lowercase please.

Stripping the poison in the dtor doesn't match C++ semantics: you aren't required to run the dtor to reuse the storage as something else, but conversely you can't reuse the storage as something else unless you use placement-new to construct a new object.

As a compromise to be nicer to existing code, maybe we should strip poison both in the dtor and in a new-expression?

713–714

This doesn't match the set of checks you did in MayInsertExtraPaddingAfterField; in particular, do you need to consider unions here?

718–719

Field names should be capitalized.

729

Please don't hard-code 8 here; use ASTContext::toCharUnitsFromBits instead.

747

This is IntPtrTy (a member of CodeGenFunction).

[majnemer]

lib/AST/RecordLayoutBuilder.cpp

1371–1372

This doesn't look windows friendly.

[kcc]

Addressed most of the comments in the review, added tests,
moved the check for validity of the instrumentation to RecordDecl::MayInsertExtraPadding,
added a remark flag -Rsanitize-address.

PTAL

This CL still has one FIXME pending Alexey's changes for -fsanitize-blacklist flag.

lib/AST/RecordLayoutBuilder.cpp

1343–1344

done

1345

Agree. Moved to RecordDecl::MayInsertExtraPadding
In future I may need a separate function to check a particular field (e.g. if we want paddings only after arrays).
But not in this patch

1347–1348

I've reverted the loop to its previous state because I actually decided to instrument the last field as well.
This handles cases where we have an array of objects and an overflow from the last field of one objects touches the other object.

1350–1351

Maybe factor out adding the padding from adding the field? I don't think they need to be done together, do they?

Err. Not sure here. It looks like it fits here naturally because we change the FieldSize which then affect the computation if the type's size, etc

Also, you shouldn't add padding at the end of the class if it ends in a flexible array member.

Added the check for !FieldSize.isZero()

1359–1364

It's quite hard to teach asan memcpy to avoid intra object redzones w/o extra false negatives and performance penalty.
But I do want to relax the requirement of non-standard layout, just not in this CL

1361–1362

I wanted to limit to classes that have a user defined DTOR, at least in one of the members, subclasses, etc.
As I say in the comment above, we'll try to relax some of these requirements.
This and other limitations are something that I was able to debug so far, I'll be removing/relaxing these requirements one by one later.

1366–1376

Alexey will commit a separate patch to move fsanitize=blacklist to langopts.
Clearly, I will wait for that and update this CL

1378–1384

done (-Rsanitize-address)

lib/CodeGen/CGClass.cpp

705

We may be able to relax the requirement of unpoisoning in dtor, but not in this CL.
I've seen at least few places (on in LLVM itself) where we should not instrument a class w/o dtor.

striping poisin in new-expression is not a solution as it may lead to false negatives (e.g. when a new-expression is called on an illegal memory)

713–714

Agree. I've moved all this logic into RecordDecl::MayInsertExtraPadding and call it twice, once from RecordLayoutBuilder::LayoutFields and once from CodeGenFunction::EmitAsanPrologueOrEpilogue

718–719

done

729

done

747

done

[samsonov]

lib/AST/RecordLayoutBuilder.cpp

1366–1376

r219842 moves `-fsanitize-blacklist` is in LangOpts. You've probably thought about it, but I will warn you anyway: recompiling the library with the same compiler and compile flags, but different blacklist file will make it ABI-incompatible. So, we should be really careful in using it.

[kcc]

Use proper blacklist machinery.
PTAL

[samsonov]

lib/AST/Decl.cpp

3642

This looks weird: II->getName() is neither mangled nor qualified (what would it return for the record declared inside the namespaces)? Consider testing smth. like printQualifiedName().

[rsmith]

include/clang/AST/Decl.h

3266

In new code, please use \brief instead of repeating the function name.

3269

Please use a leading lowercase letter for this function name for consistency with the rest of the class.

include/clang/Basic/DiagnosticFrontendKinds.td

49–51

Remarks are end-user-visible, so should be a bit more friendly than this. Also, this diagnostic should use "%select{list|of|strings}1" rather than putting user-visible strings in the code in Decl.cpp. Something like:

"-fsanitize-address-field-padding ignored for %0 because it %select{is packed|is a union|is trivially copyable|has a trivial destructor|is standard layout|is in a blacklisted file|is blacklisted}1"

... would seem reasonable. (I don't think it's worth remarking once per class if the reason we're not sanitizing is because the current language is C; in that case, we should warn once for the entire compilation.)

lib/AST/Decl.cpp

3632

Please use nullptr rather than 0.

3641

hasSimpleDestructor still does not seem right; it's a notion internal to how we manage implicit declaration and definition for destructors and shouldn't be used for anything else. Does hasTrivialDestructor work for you?

3647

Blacklisting by identifier doesn't seem ideal; there should probably be some way of specifying a namespace in the blacklist file.

[kcc]

Addressed all comments except for one

include/clang/AST/Decl.h

3266

done

3269

done

include/clang/Basic/DiagnosticFrontendKinds.td

49–51

Done except for "we should warn once for the entire compilation for C"
What is the kosher way to achieve that in clang?
(I assume that a function-scope static boolean is not kosher)

lib/AST/Decl.cpp

3632

Var changed to int.

3641

Yes, this seems to work,

3647

done (using getQualifiedNameAsString())

[kcc]

Addressed all comments (but one), extended the test to check the blacklist functionality and the remarks

PTAL

[rsmith]

LGTM

include/clang/Basic/DiagnosticFrontendKinds.td

50–56

I don't think these should be BackendInfo, because they're reflecting frontend semantics changes. They should probably be just ShowInSystemHeader.

lib/AST/Decl.cpp

3629–3630

It would seem prudent to also test whether the type is in an extern "C" linkage specification in C++ here:

if (!CXXRD || CXXRD->isExternCContext())

[kcc]

Addressed last two Richard's comments.
Added a test for an extern "C" class.

include/clang/AST/Decl.h

static bool classofKind(Kind K) {
     return K >= firstRecord && K <= lastRecord;
   }
 
   /// isMsStrust - Get whether or not this is an ms_struct which can
   /// be turned on with an attribute, pragma, or -mms-bitfields
   /// commandline option.
   bool isMsStruct(const ASTContext &C) const;
 
+  /// \brief Whether we are allowed to insert extra padding between fields.
+  /// These padding are added to help AddressSanitizer detect
+  /// intra-object-overflow bugs.
+  bool mayInsertExtraPadding(bool EmitRemark = false) const;
+
 private:
   /// \brief Deserialize just the fields.
   void LoadFieldsFromExternalStorage() const;
 };
 
 class FileScopeAsmDecl : public Decl {
   virtual void anchor();
   StringLiteral *AsmString;

include/clang/Basic/DiagnosticFrontendKinds.td

def remark_fe_backend_optimization_remark_missed : Remark<"%0">, BackendInfo,
     InGroup<BackendOptimizationRemarkMissed>;
 def remark_fe_backend_optimization_remark_analysis : Remark<"%0">, BackendInfo,
     InGroup<BackendOptimizationRemarkAnalysis>;
 def warn_fe_backend_optimization_failure : Warning<"%0">, BackendInfo,
     InGroup<BackendOptimizationFailure>, DefaultWarn;
 def note_fe_backend_optimization_remark_invalid_loc : Note<"could "
   "not determine the original source location for %0:%1:%2">;
 
+def remark_sanitize_address_insert_extra_padding_accepted : Remark<
+    "-fsanitize-address-field-padding applied to %0">, ShowInSystemHeader,
+    InGroup<SanitizeAddressRemarks>;
+def remark_sanitize_address_insert_extra_padding_rejected : Remark<
+    "-fsanitize-address-field-padding ignored for %0 because it "
+    "%select{is not C++|is packed|is a union|is trivially copyable|"
+    "has trivial destructor|is standard layout|is in a blacklisted file|"
+    "is blacklisted}1">, ShowInSystemHeader,
+    InGroup<SanitizeAddressRemarks>;
+
 def err_fe_invalid_code_complete_file : Error<
     "cannot locate code-completion file %0">, DefaultFatal;
 def err_fe_stdout_binary : Error<"unable to change standard output to binary">,
   DefaultFatal;
 def err_fe_dependency_file_requires_MT : Error<
     "-dependency-file requires at least one -MT or -MQ option">;
 def err_fe_invalid_plugin_name : Error<
     "unable to find plugin '%0'">;

include/clang/Basic/DiagnosticGroups.td

 def BackendOptimizationRemarkMissed : DiagGroup<"pass-missed">;
 def BackendOptimizationRemarkAnalysis : DiagGroup<"pass-analysis">;
 def BackendOptimizationFailure : DiagGroup<"pass-failed">;
 
 // Instrumentation based profiling warnings.
 def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
 def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
 
+// AddressSanitizer frontent instrumentation remarks.
+def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;
+
 // A warning group for warnings about code that clang accepts when
 // compiling CUDA C/C++ but which is not compatible with the CUDA spec.
 def CudaCompat : DiagGroup<"cuda-compat">;

include/clang/Basic/SanitizerBlacklist.h

 class SanitizerBlacklist {
   std::unique_ptr<llvm::SpecialCaseList> SCL;
 
 public:
   SanitizerBlacklist(const std::string &BlacklistPath);
   bool isIn(const llvm::Function &F) const;
   bool isIn(const llvm::GlobalVariable &G,
             StringRef Category = StringRef()) const;
-  bool isBlacklistedType(StringRef MangledTypeName) const;
+  bool isBlacklistedType(StringRef MangledTypeName,
+                         StringRef Category = StringRef()) const;
   bool isBlacklistedFunction(StringRef FunctionName) const;
   bool isBlacklistedFile(StringRef FileName,
                          StringRef Category = StringRef()) const;
 };
 
 }  // end namespace clang
 
 #endif

lib/AST/Decl.cpp

 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 
 using namespace clang;
 
 Decl *clang::getPrimaryMergedDecl(Decl *D) {
   return D->getASTContext().getPrimaryMergedDecl(D);
 }
#endif
 
   if (Decls.empty())
     return;
 
   std::tie(FirstDecl, LastDecl) = BuildDeclChain(Decls,
                                                  /*FieldsAlreadyLoaded=*/false);
 }
 
+bool RecordDecl::mayInsertExtraPadding(bool EmitRemark) const {
+  ASTContext &Context = getASTContext();
+  if (!Context.getLangOpts().Sanitize.Address ||
+      !Context.getLangOpts().Sanitize.SanitizeAddressFieldPadding)
+    return false;
+  auto &Blacklist = Context.getSanitizerBlacklist();
+  const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(this);
+  StringRef Filename = Context.getSourceManager().getFilename(getLocation());
+  // We may be able to relax some of these requirements.
+  int ReasonToReject = -1;
+  if (!CXXRD || CXXRD->isExternCContext())
+    ReasonToReject = 0;  // is not C++.
+  else if (CXXRD->hasAttr<PackedAttr>())
+    ReasonToReject = 1;  // is packed.
+  else if (CXXRD->isUnion())
+    ReasonToReject = 2;  // is a union.
+  else if (CXXRD->isTriviallyCopyable())
+    ReasonToReject = 3;  // is trivially copyable.
+  else if (CXXRD->hasTrivialDestructor())
+    ReasonToReject = 4;  // has trivial destructor.
+  else if (CXXRD->isStandardLayout())
+    ReasonToReject = 5;  // is standard layout.
+  else if (Blacklist.isBlacklistedFile(Filename, "field-padding"))
+    ReasonToReject = 6;  // is in a blacklisted file.
+  else if (Blacklist.isBlacklistedType(getQualifiedNameAsString(),
+                                       "field-padding"))
+    ReasonToReject = 7;  // is blacklisted.
+
+  if (EmitRemark) {
+    if (ReasonToReject >= 0)
+      Context.getDiagnostics().Report(
+          getLocation(),
+          diag::remark_sanitize_address_insert_extra_padding_rejected)
+          << getQualifiedNameAsString() << ReasonToReject;
+    else
+      Context.getDiagnostics().Report(
+          getLocation(),
+          diag::remark_sanitize_address_insert_extra_padding_accepted)
+          << getQualifiedNameAsString();
+  }
+  return ReasonToReject < 0;
+}
+
 //===----------------------------------------------------------------------===//
 // BlockDecl Implementation
 //===----------------------------------------------------------------------===//
 
 void BlockDecl::setParams(ArrayRef<ParmVarDecl *> NewParamInfo) {
   assert(!ParamInfo && "Already has param info!");
 
   // Zero params -> null pointer.

lib/AST/RecordLayoutBuilder.cpp

void resetWithTargetAlignment(CharUnits TargetAlignment) {
     Alignment = UnpackedAlignment = TargetAlignment;
   }
 
   void Layout(const RecordDecl *D);
   void Layout(const CXXRecordDecl *D);
   void Layout(const ObjCInterfaceDecl *D);
 
   void LayoutFields(const RecordDecl *D);
-  void LayoutField(const FieldDecl *D);
+  void LayoutField(const FieldDecl *D, bool InsertExtraPadding);
   void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
                           bool FieldPacked, const FieldDecl *D);
   void LayoutBitField(const FieldDecl *D);
 
   TargetCXXABI getCXXABI() const {
     return Context.getTargetInfo().getCXXABI();
   }
 
if (ObjCInterfaceDecl *SD = D->getSuperClass()) {
     setSize(SL.getDataSize());
     setDataSize(getSize());
   }
 
   InitializeLayout(D);
   // Layout each ivar sequentially.
   for (const ObjCIvarDecl *IVD = D->all_declared_ivar_begin(); IVD;
        IVD = IVD->getNextIvar())
-    LayoutField(IVD);
+    LayoutField(IVD, false);
 
   // Finally, round the size of the total struct up to the alignment of the
   // struct itself.
   FinishLayout(D);
 }
 
 void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
   // Layout each field, for now, just sequentially, respecting alignment.  In
   // the future, this will need to be tweakable by targets.
+  bool InsertExtraPadding = D->mayInsertExtraPadding(/*EmitRemark=*/true);
   for (const auto *Field : D->fields())
-    LayoutField(Field);
+    LayoutField(Field, InsertExtraPadding);
 }
 
 void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
                                              uint64_t TypeSize,
                                              bool FieldPacked,
                                              const FieldDecl *D) {
   assert(Context.getLangOpts().CPlusPlus &&
          "Can only have wide bit-fields in C++!");
 
   // Itanium C++ ABI 2.4:
   //   If sizeof(T)*8 < n, let T' be the largest integral POD type with
   //   sizeof(T')*8 <= n.
 
   QualType IntegralPODTypes[] = {
     Context.UnsignedCharTy, Context.UnsignedShortTy, Context.UnsignedIntTy,
     Context.UnsignedLongTy, Context.UnsignedLongLongTy
   };
 
   QualType Type;
   for (const QualType &QT : IntegralPODTypes) {
     uint64_t Size = Context.getTypeSize(QT);
 
     if (Size > FieldSize)
       break;
 
     Type = QT;
   }
   assert(!Type.isNull() && "Did not find a type!");
 
   CharUnits TypeAlign = Context.getTypeAlignInChars(Type);
 
   // We're not going to use any of the unfilled bits in the last byte.
   UnfilledBitsInLastUnit = 0;
   LastBitfieldTypeSize = 0;
 
   uint64_t FieldOffset;
   uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
 
   if (IsUnion) {
     setDataSize(std::max(getDataSizeInBits(), FieldSize));
     FieldOffset = 0;
   } else {
     // The bitfield is allocated starting at the next offset aligned 
     // appropriately for T', with length n bits.
     FieldOffset = llvm::RoundUpToAlignment(getDataSizeInBits(), 
                                            Context.toBits(TypeAlign));
void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
   // Update the size.
   setSize(std::max(getSizeInBits(), getDataSizeInBits()));
 
   // Remember max struct/class alignment.
   UpdateAlignment(Context.toCharUnitsFromBits(FieldAlign), 
                   Context.toCharUnitsFromBits(UnpackedFieldAlign));
 }
 
-void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {  
+void RecordLayoutBuilder::LayoutField(const FieldDecl *D,
+                                      bool InsertExtraPadding) {
   if (D->isBitField()) {
     LayoutBitField(D);
     return;
   }
 
   uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
 
   // Reset the unfilled bits.
void RecordLayoutBuilder::LayoutField(const FieldDecl *D,
   // Place this field at the current location.
   FieldOffsets.push_back(Context.toBits(FieldOffset));
 
   if (!ExternalLayout)
     CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFieldOffset, 
                       Context.toBits(UnpackedFieldOffset),
                       Context.toBits(UnpackedFieldAlign), FieldPacked, D);
 
+  if (InsertExtraPadding && !FieldSize.isZero()) {
+    CharUnits ASanAlignment = CharUnits::fromQuantity(8);
+    CharUnits ExtraSizeForAsan = ASanAlignment;
+    if (FieldSize % ASanAlignment)
+      ExtraSizeForAsan +=
+          ASanAlignment - CharUnits::fromQuantity(FieldSize % ASanAlignment);
+    FieldSize += ExtraSizeForAsan;
+  }
+
   // Reserve space for this field.
   uint64_t FieldSizeInBits = Context.toBits(FieldSize);
   if (IsUnion)
     setDataSize(std::max(getDataSizeInBits(), FieldSizeInBits));
   else
     setDataSize(FieldOffset + FieldSize);
 
   // Update the size.

lib/Basic/SanitizerBlacklist.cpp

 
 bool SanitizerBlacklist::isIn(const llvm::GlobalVariable &G,
                               StringRef Category) const {
   return isBlacklistedFile(G.getParent()->getModuleIdentifier(), Category) ||
          SCL->inSection("global", G.getName(), Category) ||
          SCL->inSection("type", GetGlobalTypeString(G), Category);
 }
 
-bool SanitizerBlacklist::isBlacklistedType(StringRef MangledTypeName) const {
-  return SCL->inSection("type", MangledTypeName);
+bool SanitizerBlacklist::isBlacklistedType(StringRef MangledTypeName,
+                                           StringRef Category) const {
+  return SCL->inSection("type", MangledTypeName, Category);
 }
 
 bool SanitizerBlacklist::isBlacklistedFunction(StringRef FunctionName) const {
   return SCL->inSection("fun", FunctionName);
 }
 
-bool SanitizerBlacklist::isBlacklistedFile(StringRef FileName,
+bool SanitizerBlacklist::isBlacklistedFile(StringRef Filename,
                                            StringRef Category) const {
-  return SCL->inSection("src", FileName, Category);
+  return SCL->inSection("src", Filename, Category);
 }
+
+

lib/CodeGen/CGClass.cpp

static bool IsConstructorDelegationValid(const CXXConstructorDecl *Ctor) {
 
   // FIXME: Decide if we can do a delegation of a delegating constructor.
   if (Ctor->isDelegatingConstructor())
     return false;
 
   return true;
 }
 
+// Emit code in ctor (Prologue==true) or dtor (Prologue==false)
+// to poison the extra field paddings inserted under
+// -fsanitize-address-field-padding=1|2.
+void CodeGenFunction::EmitAsanPrologueOrEpilogue(bool Prologue) {
+  ASTContext &Context = getContext();
+  const CXXRecordDecl *ClassDecl =
+      Prologue ? cast<CXXConstructorDecl>(CurGD.getDecl())->getParent()
+               : cast<CXXDestructorDecl>(CurGD.getDecl())->getParent();
+  if (!ClassDecl->mayInsertExtraPadding()) return;
+
+  struct SizeAndOffset {
+    uint64_t Size;
+    uint64_t Offset;
+  };
+
+  unsigned PtrSize = CGM.getDataLayout().getPointerSizeInBits();
+  const ASTRecordLayout &Info = Context.getASTRecordLayout(ClassDecl);
+
+  // Populate sizes and offsets of fields.
+  SmallVector<SizeAndOffset, 16> SSV(Info.getFieldCount());
+  for (unsigned i = 0, e = Info.getFieldCount(); i != e; ++i)
+    SSV[i].Offset =
+        Context.toCharUnitsFromBits(Info.getFieldOffset(i)).getQuantity();
+
+  size_t NumFields = 0;
+  for (const auto *Field : ClassDecl->fields()) {
+    const FieldDecl *D = Field;
+    std::pair<CharUnits, CharUnits> FieldInfo =
+        Context.getTypeInfoInChars(D->getType());
+    CharUnits FieldSize = FieldInfo.first;
+    assert(NumFields < SSV.size());
+    SSV[NumFields].Size = D->isBitField() ? 0 : FieldSize.getQuantity();
+    NumFields++;
+  }
+  assert(NumFields == SSV.size());
+  if (SSV.size() <= 1) return;
+
+  // We will insert calls to __asan_* run-time functions.
+  // LLVM AddressSanitizer pass may decide to inline them later.
+  llvm::Type *Args[2] = {IntPtrTy, IntPtrTy};
+  llvm::FunctionType *FTy =
+      llvm::FunctionType::get(CGM.VoidTy, Args, false);
+  llvm::Constant *F = CGM.CreateRuntimeFunction(
+      FTy, Prologue ? "__asan_poison_intra_object_redzone"
+                    : "__asan_unpoison_intra_object_redzone");
+
+  llvm::Value *ThisPtr = LoadCXXThis();
+  ThisPtr = Builder.CreatePtrToInt(ThisPtr, IntPtrTy);
+  QualType RecordTy = Context.getTypeDeclType(ClassDecl);
+  uint64_t TypeSize = Context.getTypeSizeInChars(RecordTy).getQuantity();
+
+  // For each field check if it has sufficient padding,
+  // if so (un)poison it with a call.
+  for (size_t i = 0; i < SSV.size(); i++) {
+    uint64_t AsanAlignment = 8;
+    uint64_t NextField = i == SSV.size() - 1 ? TypeSize : SSV[i + 1].Offset;
+    uint64_t PoisonSize = NextField - SSV[i].Offset - SSV[i].Size;
+    uint64_t EndOffset = SSV[i].Offset + SSV[i].Size;
+    if (PoisonSize < AsanAlignment || !SSV[i].Size ||
+        (NextField % AsanAlignment) != 0)
+      continue;
+    Builder.CreateCall2(
+        F, Builder.CreateAdd(ThisPtr, Builder.getIntN(PtrSize, EndOffset)),
+        Builder.getIntN(PtrSize, PoisonSize));
+  }
+}
+
 /// EmitConstructorBody - Emits the body of the current constructor.
 void CodeGenFunction::EmitConstructorBody(FunctionArgList &Args) {
+  EmitAsanPrologueOrEpilogue(true);
   const CXXConstructorDecl *Ctor = cast<CXXConstructorDecl>(CurGD.getDecl());
   CXXCtorType CtorType = CurGD.getCtorType();
 
   assert((CGM.getTarget().getCXXABI().hasConstructorVariants() ||
           CtorType == Ctor_Complete) &&
          "can only generate complete ctor for this ABI");
 
   // Before we go any further, try the complete->base constructor
namespace {
   public:
     FieldMemcpyizer(CodeGenFunction &CGF, const CXXRecordDecl *ClassDecl,
                     const VarDecl *SrcRec)
       : CGF(CGF), ClassDecl(ClassDecl), SrcRec(SrcRec), 
         RecLayout(CGF.getContext().getASTRecordLayout(ClassDecl)),
         FirstField(nullptr), LastField(nullptr), FirstFieldOffset(0),
         LastFieldOffset(0), LastAddedFieldIndex(0) {}
 
-    static bool isMemcpyableField(FieldDecl *F) {
+    bool isMemcpyableField(FieldDecl *F) const {
+      // Never memcpy fields when we are adding poisoned paddings.
+      if (CGF.getContext().getLangOpts().Sanitize.SanitizeAddressFieldPadding)
+        return false;
       Qualifiers Qual = F->getType().getQualifiers();
       if (Qual.hasVolatile() || Qual.hasObjCLifetime())
         return false;
       return true;
     }
 
     void addMemcpyableField(FieldDecl *F) {
       if (!FirstField)
void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
 
   Stmt *Body = Dtor->getBody();
 
   // If the body is a function-try-block, enter the try before
   // anything else.
   bool isTryBody = (Body && isa<CXXTryStmt>(Body));
   if (isTryBody)
     EnterCXXTryStmt(*cast<CXXTryStmt>(Body), true);
+  EmitAsanPrologueOrEpilogue(false);
 
   // Enter the epilogue cleanups.
   RunCleanupsScope DtorEpilogue(*this);
   
   // If this is the complete variant, just invoke the base variant;
   // the epilogue will destruct the virtual bases.  But we can't do
   // this optimization if the body is a function-try-block, because
   // we'd introduce *two* handler blocks.  In the Microsoft ABI, we 

lib/CodeGen/CodeGenFunction.h

public:
   void EmitBlockWithFallThrough(llvm::BasicBlock *BB, RegionCounter &Cnt);
 
   void EmitForwardingCallToLambda(const CXXMethodDecl *LambdaCallOperator,
                                   CallArgList &CallArgs);
   void EmitLambdaToBlockPointerBody(FunctionArgList &Args);
   void EmitLambdaBlockInvokeBody();
   void EmitLambdaDelegatingInvokeBody(const CXXMethodDecl *MD);
   void EmitLambdaStaticInvokeFunction(const CXXMethodDecl *MD);
+  void EmitAsanPrologueOrEpilogue(bool Prologue);
 
   /// EmitReturnBlock - Emit the unified return block, trying to avoid its
   /// emission when possible.
   void EmitReturnBlock();
 
   /// FinishFunction - Complete IR generation of the current function. It is
   /// legal to call this function even if there is no current insertion point.
   void FinishFunction(SourceLocation EndLoc=SourceLocation());

test/CodeGen/sanitize-address-field-padding.cpp

+// Test -fsanitize-address-field-padding
+// RUN: echo 'type:SomeNamespace::BlacklistedByName=field-padding' > %t.type.blacklist
+// RUN: echo 'src:*sanitize-address-field-padding.cpp=field-padding' > %t.file.blacklist
+// RUN: %clang_cc1 -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.type.blacklist -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.file.blacklist -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=FILE_BLACKLIST
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=NO_PADDING
+// REQUIRES: shell
+//
+
+// The reasons to ignore a particular class are not set in stone and will change.
+//
+// CHECK: -fsanitize-address-field-padding applied to Positive1
+// CHECK: -fsanitize-address-field-padding ignored for Negative1 because it is trivially copyable
+// CHECK: -fsanitize-address-field-padding ignored for Negative2 because it is trivially copyable
+// CHECK: -fsanitize-address-field-padding ignored for Negative3 because it is a union
+// CHECK: -fsanitize-address-field-padding ignored for Negative4 because it is trivially copyable
+// CHECK: -fsanitize-address-field-padding ignored for Negative5 because it is packed
+// CHECK: -fsanitize-address-field-padding ignored for SomeNamespace::BlacklistedByName because it is blacklisted
+// CHECK:  -fsanitize-address-field-padding ignored for ExternCStruct because it is not C++
+//
+// FILE_BLACKLIST: -fsanitize-address-field-padding ignored for Positive1 because it is in a blacklisted file
+// FILE_BLACKLIST-NOT: __asan_poison_intra_object_redzone
+// NO_PADDING-NOT: __asan_poison_intra_object_redzone
+
+
+class Positive1 {
+ public:
+  Positive1() {}
+  ~Positive1() {}
+  int make_it_non_standard_layout;
+ private:
+  char private1;
+  int private2;
+  short private_array[6];
+  long long private3;
+};
+
+Positive1 positive1;
+// Positive1 with extra paddings
+// CHECK: type { i32, [12 x i8], i8, [15 x i8], i32, [12 x i8], [6 x i16], [12 x i8], i64, [8 x i8] }
+
+class Negative1 {
+ public:
+  Negative1() {}
+  int public1, public2;
+};
+Negative1 negative1;
+// CHECK: type { i32, i32 }
+
+class Negative2 {
+ public:
+  Negative2() {}
+ private:
+  int private1, private2;
+};
+Negative2 negative2;
+// CHECK: type { i32, i32 }
+
+union Negative3 {
+  char m1[8];
+  long long m2;
+};
+
+Negative3 negative3;
+// CHECK: type { i64 }
+
+class Negative4 {
+ public:
+  Negative4() {}
+  // No DTOR
+  int make_it_non_standard_layout;
+ private:
+  char private1;
+  int private2;
+};
+
+Negative4 negative4;
+// CHECK: type { i32, i8, i32 }
+
+class __attribute__((packed)) Negative5 {
+ public:
+  Negative5() {}
+  ~Negative5() {}
+  int make_it_non_standard_layout;
+ private:
+  char private1;
+  int private2;
+};
+
+Negative5 negative5;
+// CHECK: type <{ i32, i8, i32 }>
+
+
+namespace SomeNamespace {
+class BlacklistedByName {
+ public:
+  BlacklistedByName() {}
+  ~BlacklistedByName() {}
+  int make_it_non_standard_layout;
+ private:
+  char private1;
+  int private2;
+};
+}  // SomeNamespace
+
+SomeNamespace::BlacklistedByName blacklisted_by_name;
+
+extern "C" {
+class ExternCStruct {
+ public:
+  ExternCStruct() {}
+  ~ExternCStruct() {}
+  int make_it_non_standard_layout;
+ private:
+  char private1;
+  int private2;
+};
+}  // extern "C"
+
+ExternCStruct extern_C_struct;
+
+// CTOR
+// CHECK-LABEL: define linkonce_odr void @_ZN9Positive1C1Ev
+// CHECK: call void @__asan_poison_intra_object_redzone({{.*}}, i64 12)
+// CHECK: call void @__asan_poison_intra_object_redzone({{.*}}, i64 15)
+// CHECK: call void @__asan_poison_intra_object_redzone({{.*}}, i64 12)
+// CHECK: call void @__asan_poison_intra_object_redzone({{.*}}, i64 12)
+// CHECK: call void @__asan_poison_intra_object_redzone({{.*}}, i64 8)
+// CHECK-NOT: __asan_poison_intra_object_redzone
+// CHECK: ret void
+// DTOR
+// CHECK-LABEL: define linkonce_odr void @_ZN9Positive1D1Ev
+// CHECK: call void @__asan_unpoison_intra_object_redzone({{.*}}, i64 12)
+// CHECK: call void @__asan_unpoison_intra_object_redzone({{.*}}, i64 15)
+// CHECK: call void @__asan_unpoison_intra_object_redzone({{.*}}, i64 12)
+// CHECK: call void @__asan_unpoison_intra_object_redzone({{.*}}, i64 12)
+// CHECK: call void @__asan_unpoison_intra_object_redzone({{.*}}, i64 8)
+// CHECK-NOT: __asan_unpoison_intra_object_redzone
+// CHECK: ret void
+//
+// CHECK-LABEL: define linkonce_odr void @_ZN9Negative1C1Ev
+// CHECK-NOT: call void @__asan_poison_intra_object_redzone
+// CHECK: ret void
+//
+// CHECK-LABEL: define linkonce_odr void @_ZN9Negative2C1Ev
+// CHECK-NOT: call void @__asan_poison_intra_object_redzone
+// CHECK: ret void
+//
+// CHECK-LABEL: define linkonce_odr void @_ZN9Negative4C1Ev
+// CHECK-NOT: call void @__asan_poison_intra_object_redzone
+// CHECK: ret void
+