Hi samsonov, rnk, rsmith,
In the current state the change is not for commit
(it has several FIXMEs that need to be resolved,
then the tests will need to be added).
A this point I only want to ask you if the approach makes sence
and I've chosen the correct places to implement it.
Please also comment the FIXMEs if you have ideas.
The general approach is to add extra paddings after every field
(except for the last one) in AST/RecordLayoutBuilder.cpp,
then add code to CTORs/DTORs that poisons the paddings
(CodeGen/CGClass.cpp).
This uses a flag -fsanitize-address-field-padding that is under
review separately in http://reviews.llvm.org/D5676
See also https://code.google.com/p/address-sanitizer/wiki/IntraObjectOverflow
http://reviews.llvm.org/D5687
Files:
lib/AST/RecordLayoutBuilder.cpp
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -21,6 +21,7 @@
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/SpecialCaseList.h"
using namespace clang;
@@ -651,8 +652,10 @@
void Layout(const CXXRecordDecl *D);
void Layout(const ObjCInterfaceDecl *D);
+ bool MayInsertExtraPaddingAfterField(const RecordDecl *RD,
+ const FieldDecl *FD);
void LayoutFields(const RecordDecl *D);
- void LayoutField(const FieldDecl *D);
+ void LayoutField(const FieldDecl *D, bool InsertPoisonedRedzone);
void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
bool FieldPacked, const FieldDecl *D);
void LayoutBitField(const FieldDecl *D);
@@ -1331,18 +1334,68 @@
// 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);
}
+// Returns true if we want to insert an extra padding in RD after FD.
+// These padding are added to help AddressSanitizer detect intra-object-overflow
+// bugs. Later in CogeGen an extra code will be emitted to poison these
+// extra paddings.
+bool RecordLayoutBuilder::MayInsertExtraPaddingAfterField(const RecordDecl *RD,
+ const FieldDecl *FD) {
+ if (!Context.getLangOpts().Sanitize.Address ||
+ !Context.getLangOpts().Sanitize.AddressFieldPadding)
+ return false;
+ const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD);
+ // We may be able to relax some of these requirements.
+ const char *ReasonToReject = 0;
+ if (!CXXRD)
+ ReasonToReject = "not C++";
+ else if (CXXRD->hasAttr<PackedAttr>())
+ ReasonToReject = "packed";
+ else if (CXXRD->isUnion())
+ ReasonToReject = "union";
+ else if (CXXRD->isTriviallyCopyable())
+ ReasonToReject = "trivially copyable";
+ else if (CXXRD->hasSimpleDestructor())
+ ReasonToReject = "has simple DTOR";
+ else if (CXXRD->isStandardLayout())
+ ReasonToReject = "standard layout";
+
+ // FIXME (before commit): where should this SpecialCaseList reside?
+ // In RecordLayoutBuilder? In global scope?
+ // Also, can we reuse the existing -fsanitize-blacklist flag to pass
+ // the file path (i.e. the same blacklist file will be used in AST and
+ // CogeGen), or we should have another flag?
+ auto SCL = llvm::SpecialCaseList::createOrDie(
+ "/tmp/blacklist.txt");
+ StringRef Filename =
+ Context.getSourceManager().getFilename(RD->getLocation());
+ if (SCL->inSection("src", Filename))
+ ReasonToReject = "blacklisted by src";
+
+ // FIXME (before commit): what flag to use for debug output?
+ if (0) {
+ const char *ToPrint = ReasonToReject ? ReasonToReject : "ACCEPTED";
+ llvm::errs() << "ASAN_FIELD_PADDING: " << Filename << " " << *RD
+ << "::" << *FD << " : " << ToPrint << "\n";
+ }
+ return ReasonToReject == 0;
+}
+
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.
- for (const auto *Field : D->fields())
- LayoutField(Field);
+ SmallVector<FieldDecl *, 16> Fields(D->field_begin(), D->field_end());
+ // FIXME (before commit): is there a way to know that the field is last
+ // w/o this temporary vector?
+ for (size_t i = 0, e = Fields.size(); i < e; i++)
+ LayoutField(Fields[i],
+ i + 1 != e && MayInsertExtraPaddingAfterField(D, Fields[i]));
}
void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
@@ -1645,7 +1698,8 @@
Context.toCharUnitsFromBits(UnpackedFieldAlign));
}
-void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
+void RecordLayoutBuilder::LayoutField(const FieldDecl *D,
+ bool InsertPoisonedRedzone) {
if (D->isBitField()) {
LayoutBitField(D);
return;
@@ -1749,6 +1803,15 @@
Context.toBits(UnpackedFieldOffset),
Context.toBits(UnpackedFieldAlign), FieldPacked, D);
+ if (InsertPoisonedRedzone) {
+ 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)
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -692,8 +692,74 @@
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) {
+ if (!getLangOpts().Sanitize.AddressFieldPadding) return;
+ const CXXRecordDecl *ClassDecl =
+ Prologue ? cast<CXXConstructorDecl>(CurGD.getDecl())->getParent()
+ : cast<CXXDestructorDecl>(CurGD.getDecl())->getParent();
+ if (ClassDecl->hasAttr<PackedAttr>() || ClassDecl->isStandardLayout() ||
+ ClassDecl->isTriviallyCopyable())
+ return;
+
+ struct SizeAndOffset {
+ uint64_t size;
+ uint64_t offset;
+ };
+
+ unsigned PtrSize = CGM.getDataLayout().getPointerSizeInBits();
+ const ASTRecordLayout &Info = getContext().getASTRecordLayout(ClassDecl);
+
+ // Populate sizes and offsets of fields.
+ // FIXME: can we get those from a single source?
+ SmallVector<SizeAndOffset, 16> SSV(Info.getFieldCount());
+ for (unsigned i = 0, e = Info.getFieldCount(); i != e; ++i)
+ SSV[i].offset = Info.getFieldOffset(i) / 8;
+
+
+ size_t n_fields = 0;
+ for (const auto *Field : ClassDecl->fields()) {
+ const FieldDecl *D = Field;
+ std::pair<CharUnits, CharUnits> FieldInfo =
+ getContext().getTypeInfoInChars(D->getType());
+ CharUnits FieldSize = FieldInfo.first;
+ assert(n_fields < SSV.size());
+ SSV[n_fields].size = D->isBitField() ? 0 : FieldSize.getQuantity();
+ n_fields++;
+ }
+ assert(n_fields == 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 *PtrIntTy = Builder.getIntNTy(PtrSize);
+ llvm::Type *Args[2] = {PtrIntTy, PtrIntTy};
+ 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, PtrIntTy);
+
+ // For each field (but the last) check if it has sufficient padding,
+ // if so (un)poison it with a call.
+ for (size_t i = 0; i < SSV.size() - 1; i++) {
+ uint64_t PoisonSize = SSV[i+1].offset - SSV[i].offset - SSV[i].size;
+ uint64_t EndOffset = SSV[i].offset + SSV[i].size;
+ if (PoisonSize < 8 || !SSV[i].size || (SSV[i+1].offset % 8) != 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();
@@ -782,7 +848,10 @@
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.AddressFieldPadding)
+ return false;
Qualifiers Qual = F->getType().getQualifiers();
if (Qual.hasVolatile() || Qual.hasObjCLifetime())
return false;
@@ -1282,6 +1351,7 @@
bool isTryBody = (Body && isa<CXXTryStmt>(Body));
if (isTryBody)
EnterCXXTryStmt(*cast<CXXTryStmt>(Body), true);
+ EmitAsanPrologueOrEpilogue(false);
// Enter the epilogue cleanups.
RunCleanupsScope DtorEpilogue(*this);
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1268,6 +1268,7 @@
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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
