alexshap updated this revision to Diff 67797.
alexshap added a comment.
Update the tests. Update the format again (do not include the type name because
the diagnostics becomes too verbose and unreadable (i ran it against LLVM and
clang - with type names the diagnostic messages are getting very long, without
them they look normal (IMO)),
Test plan: make -j8 check-clang-analysis (passed).
https://reviews.llvm.org/D23387
Files:
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
test/Analysis/padding_message.cpp
Index: test/Analysis/padding_message.cpp
===================================================================
--- test/Analysis/padding_message.cpp
+++ test/Analysis/padding_message.cpp
@@ -1,13 +1,25 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux -std=c++14 -analyze -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
-// expected-warning@+1{{Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
struct IntSandwich {
char c1;
int i;
char c2;
};
-// expected-warning@+1{{Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
struct TurDuckHen {
char c1;
struct IntSandwich i;
@@ -16,7 +28,17 @@
#pragma pack(push)
#pragma pack(2)
-// expected-warning@+1{{Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal)}}
+// expected-warning@+11{{\
+Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal). \
+Optimal fields order: \
+i1, \
+i2, \
+i3, \
+c1, \
+c2, \
+c3, \
+c4, \
+}}
struct SmallIntSandwich {
char c1;
int i1;
@@ -34,7 +56,13 @@
int i;
};
-// expected-warning@+1{{Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+u, \
+c1, \
+c2, \
+}}
struct HoldsAUnion {
char c1;
union SomeUnion u;
@@ -49,28 +77,53 @@
int i[5];
};
-// expected-warning@+1{{Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+m, \
+s, \
+s2, \
+}}
struct StructSandwich {
struct SmallCharArray s;
struct MediumIntArray m;
struct SmallCharArray s2;
};
-// expected-warning@+1{{Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
typedef struct {
char c1;
int i;
char c2;
} TypedefSandwich;
-// expected-warning@+1{{Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
struct StructAttrAlign {
char c1;
int i;
char c2;
} __attribute__((aligned(8)));
-// expected-warning@+1{{Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal)}}
+// expected-warning@+8{{\
+Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal). \
+Optimal fields order: \
+c, \
+c1, \
+c2, \
+x, \
+}}
struct OverlyAlignedChar {
char c1;
int x;
@@ -78,7 +131,13 @@
char c __attribute__((aligned(4096)));
};
-// expected-warning@+1{{Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal). \
+Optimal fields order: \
+o, \
+c1, \
+c2, \
+}}
struct HoldsOverlyAlignedChar {
char c1;
struct OverlyAlignedChar o;
@@ -86,7 +145,13 @@
};
void internalStructFunc() {
- // expected-warning@+1{{Excessive padding in 'struct X' (6 padding bytes, where 2 is optimal)}}
+ // expected-warning@+7{{\
+Excessive padding in 'struct X' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+t, \
+c1, \
+c2, \
+}}
struct X {
char c1;
int t;
@@ -96,7 +161,13 @@
}
void typedefStructFunc() {
- // expected-warning@+1{{Excessive padding in 'S' (6 padding bytes, where 2 is optimal)}}
+ // expected-warning@+7{{\
+Excessive padding in 'S' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+t, \
+c1, \
+c2, \
+}}
typedef struct {
char c1;
int t;
@@ -105,21 +176,39 @@
S obj;
}
-// expected-warning@+1{{Excessive padding in 'struct DefaultAttrAlign' (22 padding bytes, where 6 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct DefaultAttrAlign' (22 padding bytes, where 6 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
struct DefaultAttrAlign {
char c1;
long long i;
char c2;
} __attribute__((aligned));
-// expected-warning@+1{{Excessive padding in 'struct SmallArrayShortSandwich' (2 padding bytes, where 0 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct SmallArrayShortSandwich' (2 padding bytes, where 0 is optimal). \
+Optimal fields order: \
+s, \
+c1, \
+c2, \
+}}
struct SmallArrayShortSandwich {
char c1;
short s;
char c2;
} ShortArray[20];
-// expected-warning@+1{{Excessive padding in 'struct SmallArrayInFunc' (2 padding bytes, where 0 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct SmallArrayInFunc' (2 padding bytes, where 0 is optimal). \
+Optimal fields order: \
+s, \
+c1, \
+c2, \
+}}
struct SmallArrayInFunc {
char c1;
short s;
@@ -130,7 +219,13 @@
struct SmallArrayInFunc Arr[15];
}
-// expected-warning@+1{{Excessive padding in 'class VirtualIntSandwich' (10 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'class VirtualIntSandwich' (10 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
class VirtualIntSandwich {
virtual void foo() {}
char c1;
@@ -139,7 +234,14 @@
};
// constructed so as not to have tail padding
-// expected-warning@+1{{Excessive padding in 'class InnerPaddedB' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+8{{\
+Excessive padding in 'class InnerPaddedB' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i1, \
+i2, \
+c1, \
+c2, \
+}}
class InnerPaddedB {
char c1;
int i1;
@@ -149,17 +251,35 @@
class Empty {}; // no-warning
-// expected-warning@+1{{Excessive padding in 'class LotsOfSpace' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'class LotsOfSpace' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+e1, \
+e2, \
+}}
class LotsOfSpace {
Empty e1;
int i;
Empty e2;
};
-// expected-warning@+1{{Excessive padding in 'TypedefSandwich2' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'TypedefSandwich2' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+t, \
+c1, \
+c2, \
+}}
typedef struct {
char c1;
- // expected-warning@+1{{Excessive padding in 'TypedefSandwich2::NestedTypedef' (6 padding bytes, where 2 is optimal)}}
+ // expected-warning@+7{{\
+Excessive padding in 'TypedefSandwich2::NestedTypedef' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+i, \
+c1, \
+c2, \
+}}
typedef struct {
char c1;
int i;
@@ -171,7 +291,13 @@
template <typename T>
struct Foo {
- // expected-warning@+1{{Excessive padding in 'struct Foo<int>::Nested' (6 padding bytes, where 2 is optimal)}}
+ // expected-warning@+7{{\
+Excessive padding in 'struct Foo<int>::Nested' (6 padding bytes, where 2 is optimal). \
+Optimal fields order: \
+t, \
+c1, \
+c2, \
+}}
struct Nested {
char c1;
T t;
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -82,15 +82,19 @@
CharUnits BaselinePad = calculateBaselinePad(RD, ASTContext, RL);
if (BaselinePad.isZero())
return;
- CharUnits OptimalPad = calculateOptimalPad(RD, ASTContext, RL);
+ CharUnits OptimalPad;
+ SmallVector<const FieldDecl *, 20> OptimalFieldsOrder;
+ std::tie(OptimalPad, OptimalFieldsOrder) =
+ calculateOptimalPad(RD, ASTContext, RL);
+
CharUnits DiffPad = PadMultiplier * (BaselinePad - OptimalPad);
if (DiffPad.getQuantity() <= AllowedPad) {
assert(!DiffPad.isNegative() && "DiffPad should not be negative");
// There is not enough excess padding to trigger a warning.
return;
}
- reportRecord(RD, BaselinePad, OptimalPad);
+ reportRecord(RD, BaselinePad, OptimalPad, OptimalFieldsOrder);
}
/// \brief Look for arrays of overly padded types. If the padding of the
@@ -199,22 +203,30 @@
/// 7. Add tail padding by rounding the current offset up to the structure
/// alignment. Track the amount of padding added.
- static CharUnits calculateOptimalPad(const RecordDecl *RD,
- const ASTContext &ASTContext,
- const ASTRecordLayout &RL) {
- struct CharUnitPair {
+ static std::pair<CharUnits, SmallVector<const FieldDecl *, 20>>
+ calculateOptimalPad(const RecordDecl *RD, const ASTContext &ASTContext,
+ const ASTRecordLayout &RL) {
+ struct FieldInfo {
CharUnits Align;
CharUnits Size;
- bool operator<(const CharUnitPair &RHS) const {
+ const FieldDecl *Field;
+ bool operator<(const FieldInfo &RHS) const {
// Order from small alignments to large alignments,
// then large sizes to small sizes.
- return std::make_pair(Align, -Size) <
- std::make_pair(RHS.Align, -RHS.Size);
+ // then large field indices to small field indices
+ return std::make_tuple(Align, -Size,
+ Field ? -static_cast<int>(Field->getFieldIndex())
+ : 0) <
+ std::make_tuple(
+ RHS.Align, -RHS.Size,
+ RHS.Field ? -static_cast<int>(RHS.Field->getFieldIndex())
+ : 0);
}
};
- SmallVector<CharUnitPair, 20> Fields;
+ SmallVector<FieldInfo, 20> Fields;
auto GatherSizesAndAlignments = [](const FieldDecl *FD) {
- CharUnitPair RetVal;
+ FieldInfo RetVal;
+ RetVal.Field = FD;
auto &Ctx = FD->getASTContext();
std::tie(RetVal.Size, RetVal.Align) =
Ctx.getTypeInfoInChars(FD->getType());
@@ -226,14 +238,13 @@
std::transform(RD->field_begin(), RD->field_end(),
std::back_inserter(Fields), GatherSizesAndAlignments);
std::sort(Fields.begin(), Fields.end());
-
// This lets us skip over vptrs and non-virtual bases,
// so that we can just worry about the fields in our object.
// Note that this does cause us to miss some cases where we
// could pack more bytes in to a base class's tail padding.
CharUnits NewOffset = ASTContext.toCharUnitsFromBits(RL.getFieldOffset(0));
CharUnits NewPad;
-
+ SmallVector<const FieldDecl *, 20> OptimalFieldsOrder;
while (!Fields.empty()) {
unsigned TrailingZeros =
llvm::countTrailingZeros((unsigned long long)NewOffset.getQuantity());
@@ -242,7 +253,7 @@
// our long long (and CharUnits internal type) negative. So shift 62.
long long CurAlignmentBits = 1ull << (std::min)(TrailingZeros, 62u);
CharUnits CurAlignment = CharUnits::fromQuantity(CurAlignmentBits);
- CharUnitPair InsertPoint = {CurAlignment, CharUnits::Zero()};
+ FieldInfo InsertPoint = {CurAlignment, CharUnits::Zero(), nullptr};
auto CurBegin = Fields.begin();
auto CurEnd = Fields.end();
@@ -255,6 +266,7 @@
// We found a field that we can layout with the current alignment.
--Iter;
NewOffset += Iter->Size;
+ OptimalFieldsOrder.push_back(Iter->Field);
Fields.erase(Iter);
} else {
// We are poorly aligned, and we need to pad in order to layout another
@@ -268,18 +280,18 @@
// Calculate tail padding.
CharUnits NewSize = NewOffset.alignTo(RL.getAlignment());
NewPad += NewSize - NewOffset;
- return NewPad;
+ return {NewPad, std::move(OptimalFieldsOrder)};
}
- void reportRecord(const RecordDecl *RD, CharUnits BaselinePad,
- CharUnits TargetPad) const {
+ void reportRecord(
+ const RecordDecl *RD, CharUnits BaselinePad, CharUnits OptimalPad,
+ const SmallVector<const FieldDecl *, 20> &OptimalFieldsOrder) const {
if (!PaddingBug)
PaddingBug =
llvm::make_unique<BugType>(this, "Excessive Padding", "Performance");
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
-
Os << "Excessive padding in '";
Os << QualType::getAsString(RD->getTypeForDecl(), Qualifiers()) << "'";
@@ -294,16 +306,18 @@
}
Os << " (" << BaselinePad.getQuantity() << " padding bytes, where "
- << TargetPad.getQuantity() << " is optimal). Consider reordering "
- << "the fields or adding explicit padding members.";
+ << OptimalPad.getQuantity() << " is optimal). \n"
+ << "Optimal fields order: \n";
+ for (const auto *FD : OptimalFieldsOrder)
+ Os << FD->getName() << ", \n";
+ Os << "consider reordering the fields or adding explicit padding "
+ "members.";
PathDiagnosticLocation CELoc =
PathDiagnosticLocation::create(RD, BR->getSourceManager());
-
auto Report = llvm::make_unique<BugReport>(*PaddingBug, Os.str(), CELoc);
Report->setDeclWithIssue(RD);
Report->addRange(RD->getSourceRange());
-
BR->emitReport(std::move(Report));
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
