1 files changed, 36 insertions, 22 deletions

diff --git a/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp b/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
index 0640d2f49f43a..a51dda6fe858c 100644
--- a/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -82,7 +82,11 @@ public:
     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) {
@@ -90,7 +94,7 @@ public:
       // 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 @@ public:
   /// 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 @@ public:
     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 @@ public:
       // 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 @@ public:
         // 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 @@ public:
     // 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 @@ public:
     }
 
     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));
   }
 };