bcraig created this revision.
bcraig added reviewers: zaks.anna, jordan_rose, xazax.hun.
bcraig added a subscriber: cfe-commits.

The intent of this checker is to generate a report for any class / structure 
that could reduce its padding by reordering the fields.  This results in a very 
noisy checker.  To reduce the noise, this checker will currently only warn when 
the number of bytes over "optimal" is more than 8.  This value is configurable 
with -analyzer-config performance.Padding:AllowedPad=N.  Even with the default 
of 8, this checker is too noisy to justify turning on by default.  Clang+LLVM 
has hundreds of violations.  A large C codebase was capable of generating more 
than 600 GB of HTML reports with AllowedPad=0.

The checker searches for padding violations in two main ways.  First, it goes 
record by record.  A report is generated if the fields could be reordered in a 
way that reduces the padding by more than AllowedPad bytes.  Second, the 
checker will generate a report if an array will cause more than AllowedPad 
padding bytes to be generated.

The record checker currently skips many ABI specific cases.  Classes with base 
classes are skipped because base class tail padding is ABI specific.  Bitfields 
are just plain hard, and duplicating that code seems like a bad idea.  VLAs are 
both uncommon and non-trivial to fix.

The array checker isn't very thorough right now.  It only checks to see if the 
element type's fields could be reordered, and it doesn't recursively check to 
see if any of the fields' fields could be reordered.  At some point in the 
future, it would be nice if "arrays" could also look at array new usages and 
malloc patterns that appear to be creating arrays.


http://reviews.llvm.org/D14779

Files:
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/Analysis/padding_c.c
  test/Analysis/padding_cpp.cpp

Index: test/Analysis/padding_cpp.cpp
===================================================================
--- /dev/null
+++ test/Analysis/padding_cpp.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -std=c++14 -analyze -analyzer-checker=performance -analyzer-config performance.Padding:AllowedPad=2 -verify %s
+
+// Make sure that the C cases still work fine, even when compiled as C++.
+#include "padding_c.c"
+
+struct BigCharArray2 { // no-warning
+  char c[129];
+};
+
+// xxxexpected-warning@+1{{Excessive padding in 'struct LowAlignmentBase'}}
+struct LowAlignmentBase : public BigCharArray2 {
+  int i;
+  char c;
+};
+
+struct CorrectLowAlignmentBase : public BigCharArray2 { // no-warning
+  char c;
+  int i;
+};
+
+// xxxexpected-warning@+1{{Excessive padding in 'struct LowAlignmentBase2'}}
+struct LowAlignmentBase2 : public BigCharArray2 {
+  char c1;
+  int i;
+  char c2;
+};
+
+class PaddedA { // expected-warning{{Excessive padding in 'class PaddedA'}}
+  char c1;
+  int i;
+  char c2;
+};
+
+class VirtualPaddedA : public PaddedA { // no-warning
+  virtual void foo() {}
+};
+
+class VirtualIntSandwich { // expected-warning{{Excessive padding in 'class VirtualIntSandwich'}}
+  virtual void foo() {}
+  char c1;
+  int i;
+  char c2;
+};
+
+// constructed so as not to have tail padding
+class InnerPaddedB { // expected-warning{{Excessive padding in 'class InnerPaddedB'}}
+  char c1;
+  int i1;
+  char c2;
+  int i2;
+};
+
+class TailPaddedB { // expected-warning{{Excessive padding in 'class TailPaddedB'}}
+  char c1;
+  int i1;
+  char c2;
+};
+
+class SI : public PaddedA { // no-warning
+  char c;
+};
+
+class SI2 : public PaddedA { // xxxexpected-warning{{Excessive padding in 'class SI2'}}
+  char c10;
+  int i10;
+  char c11;
+};
+
+class VirtualSI : virtual public PaddedA { // no-warning
+  char c;
+};
+
+// currently not checked for
+class VirtualSI2 : virtual public PaddedA { // no-warning
+  char c10;
+  int i10;
+  char c11;
+};
+
+class VtblSI : public PaddedA { // no-warning
+  virtual void foo() {}
+  char c;
+};
+
+class VtblSI2 : public PaddedA { // xxxexpected-warning{{Excessive padding in 'class VtblSI2'}}
+  virtual void foo() {}
+  char c10;
+  int i10;
+  char c11;
+};
+
+class VtblSI3 : public VirtualPaddedA { // xxxexpected-warning{{Excessive padding in 'class VtblSI3'}}
+  char c10;
+  int i10;
+  char c11;
+};
+
+class MI : public PaddedA, public InnerPaddedB { // no-warning
+  char c;
+};
+
+class MI2 : public PaddedA, public InnerPaddedB { // xxxexpected-warning{{Excessive padding in 'class MI2'}}
+  char c10;
+  int i10;
+  char c11;
+};
+
+class VtblMI : public PaddedA, public InnerPaddedB { // xxxexpected-warning{{Excessive padding in 'class VtblMI'}}
+  virtual void foo() {}
+  char c10;
+  int i10;
+  char c11;
+};
+
+class VtblMI2 : public VirtualPaddedA, public InnerPaddedB { // xxxexpected-warning{{Excessive padding in 'class VtblMI2'}}
+  char c10;
+  int i10;
+  char c11;
+};
+
+class Empty {}; // no-warning
+
+class LotsOfSpace { // expected-warning{{Excessive padding in 'class LotsOfSpace'}}
+  Empty e1;
+  int i;
+  Empty e2;
+};
+
+class EBO1 : public Empty { // xxxexpected-warning{{Excessive padding in 'class EBO1'}}
+  char c1;
+  int i;
+  char c2;
+};
+
+class EBO2 : public Empty { // xxxexpected-warning{{Excessive padding in 'class EBO2'}}
+  Empty c1;
+  int i;
+  Empty c2;
+};
+
+template <typename T>
+class TemplateSandwich { // expected-warning{{Excessive padding in 'class TemplateSandwich<int>' instantiated here}}
+  char c1;
+  T t;
+  char c2;
+};
+
+template <typename T>
+class TemplateSandwich<T *> { // expected-warning{{Excessive padding in 'class TemplateSandwich<void *>' instantiated here}}
+  char c1;
+  T *t;
+  char c2;
+};
+
+template <>
+class TemplateSandwich<long long> { // expected-warning{{Excessive padding in 'class TemplateSandwich<long long>' (}}
+  char c1;
+  long long t;
+  char c2;
+};
+
+class Holder1 { // no-warning
+  TemplateSandwich<int> t1;
+  TemplateSandwich<char> t2;
+  TemplateSandwich<void *> t3;
+};
+
+typedef struct { // expected-warning{{Excessive padding in 'TypedefSandwich2'}}
+  char c1;
+  typedef struct { // expected-warning{{Excessive padding in 'TypedefSandwich2::NestedTypedef'}}
+    char c1;
+    int i;
+    char c2;
+  } NestedTypedef;
+  NestedTypedef t;
+  char c2;
+} TypedefSandwich2;
+
+template <typename T>
+struct Foo {
+  // expected-warning@+1{{Excessive padding in 'struct Foo<int>::Nested'}}
+  struct Nested {
+    char c1;
+    T t;
+    char c2;
+  };
+};
+
+struct Holder { // no-warning
+  Foo<int>::Nested t1;
+  Foo<char>::Nested t2;
+};
+
+struct GlobalsForLambda { // no-warning
+  int i;
+  char c1;
+  char c2;
+} G;
+
+// expected-warning@+1{{Excessive padding in 'class (lambda}}
+auto lambda1 = [ c1 = G.c1, i = G.i, c2 = G.c2 ]{};
+auto lambda2 = [ i = G.i, c1 = G.c1, c2 = G.c2 ]{}; // no-warning
Index: test/Analysis/padding_c.c
===================================================================
--- /dev/null
+++ test/Analysis/padding_c.c
@@ -0,0 +1,243 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=performance -analyzer-config performance.Padding:AllowedPad=2 -verify %s
+
+#if __has_include(<stdalign.h>)
+#include <stdalign.h>
+#endif
+
+#if __has_include(<stdalign.h>) || defined(__cplusplus)
+// expected-warning@+1{{Excessive padding in 'struct FieldAttrAlign' (6 padding}}
+struct FieldAttrAlign {
+  char c1;
+  alignas(4) int i;
+  char c2;
+};
+
+// expected-warning@+1{{Excessive padding in 'struct FieldAttrOverAlign' (10 padding}}
+struct FieldAttrOverAlign {
+  char c1;
+  alignas(8) int i;
+  char c2;
+};
+
+#endif // __has_include(<stdalign.h>) || defined(__cplusplus)
+
+// Re-ordering members of these structs won't reduce padding, so don't warn
+struct LeadingChar { // no-warning
+  char c;
+  int i;
+};
+
+struct TrailingChar { // no-warning
+  int i;
+  char c;
+};
+
+struct Helpless { // no-warning
+  struct TrailingChar i1;
+  struct LeadingChar i2;
+  char c;
+};
+
+#pragma pack(push)
+#pragma pack(1)
+struct SquishedIntSandwich { // no-warning
+  char c1;
+  int i;
+  char c2;
+};
+#pragma pack(pop)
+
+// Re-ordering members of these structs will reduce padding, so warn
+struct IntSandwich { // expected-warning{{Excessive padding in 'struct IntSandwich'}}
+  char c1;
+  int i;
+  char c2;
+};
+
+struct TurDuckHen { // expected-warning{{Excessive padding in 'struct TurDuckHen'}}
+  char c1;
+  struct IntSandwich i;
+  char c2;
+};
+
+#pragma pack(push)
+#pragma pack(2)
+struct SmallIntSandwich { // expected-warning{{Excessive padding in 'struct SmallIntSandwich'}}
+  char c1;
+  int i1;
+  char c2;
+  int i2;
+  char c3;
+  int i3;
+  char c4;
+};
+#pragma pack(pop)
+
+union SomeUnion { // no-warning
+  char c;
+  short s;
+  int i;
+};
+
+struct HoldsAUnion { // expected-warning{{Excessive padding in 'struct HoldsAUnion'}}
+  char c1;
+  union SomeUnion u;
+  char c2;
+};
+
+struct BigCharArray { // no-warning
+  char c[129];
+};
+
+struct SmallCharArray { // no-warning
+  char c[5];
+};
+
+struct MediumIntArray { // no-warning
+  int i[5];
+};
+
+struct LargeSizeToSmallSize { // expected-warning{{Excessive padding in 'struct LargeSizeToSmallSize'}}
+  struct BigCharArray b;
+  struct MediumIntArray m;
+  struct SmallCharArray s;
+};
+
+struct LargeAlignToSmallAlign { // no-warning
+  struct MediumIntArray m;
+  struct BigCharArray b;
+  struct SmallCharArray s;
+};
+
+// Currently ignoring VLA padding problems.  Still need to make sure we don't
+// choke on VLAs though
+struct HoldsVLA { // no-warning
+  char c1;
+  int x;
+  char c2;
+  int vla[];
+};
+
+// Currently ignoring bitfield padding problems.  Still need to make sure we
+// don't choke on bitfields though
+struct HoldsBitfield { // no-warning
+  char c1;
+  int x;
+  char c2;
+  unsigned char b1 : 3;
+  unsigned char b2 : 3;
+  unsigned char b3 : 2;
+};
+
+typedef struct { // expected-warning{{Excessive padding in 'TypedefSandwich'}}
+  char c1;
+  int i;
+  char c2;
+} TypedefSandwich;
+
+// expected-warning@+1{{Excessive padding in 'struct StructAttrAlign' (10 padding}}
+struct StructAttrAlign {
+  char c1;
+  int i;
+  char c2;
+} __attribute__((aligned(8)));
+
+struct CorrectOverlyAlignedChar { // no-warning
+  char c __attribute__((aligned(4096)));
+  char c1;
+  int x1;
+  char c2;
+  int x2;
+  char c3;
+};
+
+struct OverlyAlignedChar { // expected-warning{{Excessive padding in 'struct OverlyAlignedChar'}}
+  char c1;
+  int x;
+  char c2;
+  char c __attribute__((aligned(4096)));
+};
+
+struct HoldsOverlyAlignedChar { // expected-warning{{Excessive padding in 'struct HoldsOverlyAlignedChar'}}
+  char c1;
+  struct OverlyAlignedChar o;
+  char c2;
+};
+
+void internalStructFunc() {
+  struct X { // expected-warning{{Excessive padding in 'struct X'}}
+    char c1;
+    int t;
+    char c2;
+  };
+  struct X obj;
+}
+
+void typedefStructFunc() {
+  typedef struct { // expected-warning{{Excessive padding in 'S'}}
+    char c1;
+    int t;
+    char c2;
+  } S;
+  S obj;
+}
+
+void anonStructFunc() {
+  struct { // expected-warning{{Excessive padding in 'struct (anonymous}}
+    char c1;
+    int t;
+    char c2;
+  } obj;
+}
+
+// expected-warning@+1{{Excessive padding in 'struct DefaultAttrAlign'}}
+struct DefaultAttrAlign {
+  char c1;
+  long long i;
+  char c2;
+} __attribute__((aligned));
+
+struct CorrectDefaultAttrAlign { // no-warning
+  long long i;
+  char c1;
+  char c2;
+} __attribute__((aligned));
+
+struct TooSmallShortSandwich { // no-warning
+  char c1;
+  short s;
+  char c2;
+};
+
+// expected-warning@+1{{Excessive padding in 'struct SmallArrayShortSandwich'}}
+struct SmallArrayShortSandwich {
+  char c1;
+  short s;
+  char c2;
+} ShortArray[20];
+
+// expected-warning@+1{{Excessive padding in 'struct SmallArrayInFunc'}}
+struct SmallArrayInFunc {
+  char c1;
+  short s;
+  char c2;
+};
+
+void arrayHolder() {
+  struct SmallArrayInFunc Arr[15];
+}
+
+// xxxexpected-warning@+1{{Excessive padding in 'struct SmallArrayInStruct'}}
+struct SmallArrayInStruct {
+  char c1;
+  short s;
+  char c2;
+};
+
+struct HoldsSmallArray {
+  struct SmallArrayInStruct Field[20];
+} HoldsSmallArrayElt;
+
+void nestedPadding() {
+  struct HoldsSmallArray Arr[15];
+}
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -0,0 +1,342 @@
+//=======- PaddingChecker.cpp ------------------------------------*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines a checker that checks for padding that could be
+//  removed by re-ordering members
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/CharUnits.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/raw_ostream.h"
+#include <numeric>
+
+using namespace clang;
+using namespace ento;
+
+//===----------------------------------------------------------------------===//
+// PaddingChecker
+//===----------------------------------------------------------------------===//
+namespace {
+class PaddingChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+private:
+  mutable std::unique_ptr<BugType> PaddingBug;
+
+public:
+  void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
+                    BugReporter &BR) const {
+    int AllowedPad =
+        MGR.getAnalyzerOptions().getOptionAsInteger("AllowedPad", 8, this);
+    assert(AllowedPad >= 0 && "AllowedPad option should be non-negative");
+
+    struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
+      const PaddingChecker *Self;
+      BugReporter &BR;
+      int64_t AllowedPad;
+      bool shouldVisitTemplateInstantiations() const { return true; }
+      bool shouldVisitImplicitCode() const { return true; }
+      LocalVisitor(const PaddingChecker *Self, BugReporter &BR,
+                   uint64_t AllowedPad)
+          : Self(Self), BR(BR), AllowedPad(AllowedPad) {}
+      bool VisitRecordDecl(const RecordDecl *RD) {
+        Self->visitRecord(RD, BR, AllowedPad);
+        return true;
+      }
+      bool VisitVarDecl(const VarDecl *VD) {
+        Self->visitVariable(VD, BR, AllowedPad);
+        return true;
+      }
+      // TODO: Visit array new and mallocs for arrays.
+    };
+
+    LocalVisitor visitor(this, BR, AllowedPad);
+    visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+  }
+
+  void visitRecord(const RecordDecl *RD, BugReporter &BR,
+                   int64_t AllowedPad) const {
+    if (shouldSkipDecl(RD, BR))
+      return;
+
+    auto &ASTContext = RD->getASTContext();
+    const ASTRecordLayout &RL = ASTContext.getASTRecordLayout(RD);
+    assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
+
+    CharUnits BaselinePad = calculateBaselinePad(RD, ASTContext, RL);
+    if (BaselinePad.isZero())
+      return;
+    CharUnits OptimalPad = calculateOptimalPad(RD, ASTContext, RL);
+
+    CharUnits DiffPad = BaselinePad - OptimalPad;
+    if (DiffPad.getQuantity() <= AllowedPad) {
+      if (DiffPad.isNegative()) {
+        llvm_unreachable("OptimalPad was somehow larger than the BaselinePad");
+      }
+      // There is not enough excess padding to trigger a warning.
+      return;
+    }
+    reportRecord(RD, BaselinePad, OptimalPad, BR);
+  }
+
+  // Look for arrays of overly padded types.  If the padding of the
+  // array type exceeds AllowedPad, then generate a report.
+  void visitVariable(const VarDecl *VD, BugReporter &BR,
+                     int64_t AllowedPad) const {
+    const ArrayType *ArrTy = VD->getType()->getAsArrayTypeUnsafe();
+    if (ArrTy == nullptr)
+      return;
+    const RecordType *RT = ArrTy->getElementType()->getAs<RecordType>();
+    if (RT == nullptr)
+      return;
+    const RecordDecl *RD = RT->getDecl();
+    if (shouldSkipDecl(RD, BR))
+      return;
+    uint64_t Elts = 0;
+    if (const ConstantArrayType *CArrTy = dyn_cast<ConstantArrayType>(ArrTy))
+      Elts = CArrTy->getSize().getZExtValue();
+    if (Elts == 0)
+      return;
+    auto &ASTContext = RD->getASTContext();
+    const ASTRecordLayout &RL = ASTContext.getASTRecordLayout(RD);
+    assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
+
+    // TODO: Recurse into the fields and base classes to see if any
+    // of those have excess padding.
+    CharUnits BaselinePad = calculateBaselinePad(RD, ASTContext, RL);
+    if (BaselinePad.isZero())
+      return;
+    CharUnits OptimalPad = calculateOptimalPad(RD, ASTContext, RL);
+
+    CharUnits DiffPad = Elts * (BaselinePad - OptimalPad);
+    if (DiffPad.getQuantity() <= AllowedPad) {
+      if (DiffPad.isNegative()) {
+        llvm_unreachable("OptimalPad was somehow larger than the BaselinePad");
+      }
+      // There is not enough excess padding to trigger a warning.
+      return;
+    }
+    reportRecord(RD, BaselinePad, OptimalPad, BR);
+  }
+
+  static bool shouldSkipDecl(const RecordDecl *RD, BugReporter &BR) {
+    auto Location = RD->getLocation();
+    // If the construct doesn't have a source file, then it's not something
+    // we want to diagnose.
+    if (!Location.isValid())
+      return true;
+    SrcMgr::CharacteristicKind Kind =
+        BR.getSourceManager().getFileCharacteristic(Location);
+    // Throw out all records that come from system headers.
+    if (Kind != SrcMgr::C_User)
+      return true;
+
+    // Not going to attempt to optimize unions.
+    if (RD->isUnion())
+      return true;
+    // How do you reorder fields if you haven't got any?
+    if (RD->field_empty())
+      return true;
+    if (auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+      // Tail padding with base classes ends up being very complicated.
+      // We will skip objects with base classes for now.
+      if (CXXRD->getNumBases() != 0)
+        return true;
+      // Virtual bases are complicated, skipping those for now.
+      if (CXXRD->getNumVBases() != 0)
+        return true;
+      // Can't layout a template, so skip it.  We do still layout the
+      // instantiations though.
+      if (CXXRD->getTypeForDecl()->isDependentType())
+        return true;
+      if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
+        return true;
+    }
+    auto IsTrickyField = [](const FieldDecl *FD) -> bool {
+      // Bitfield layout is hard.
+      if (FD->isBitField())
+        return true;
+
+      // Variable length arrays are tricky too.
+      QualType Ty = FD->getType();
+      if (Ty->isIncompleteArrayType())
+        return true;
+      return false;
+    };
+
+    if (std::any_of(RD->field_begin(), RD->field_end(), IsTrickyField))
+      return true;
+    return false;
+  }
+
+  template <typename MappedT, typename RangeT, typename MapFunc,
+            typename LeftFoldFunc>
+  static MappedT mapLeftFold(RangeT R, MappedT M, MapFunc MapF,
+                             LeftFoldFunc LeftFoldF) {
+    for (const auto &Elt : R) {
+      M = LeftFoldF(M, MapF(Elt));
+    }
+    return M;
+  }
+
+  static CharUnits calculateBaselinePad(const RecordDecl *RD,
+                                        const ASTContext &ASTContext,
+                                        const ASTRecordLayout &RL) {
+    struct CharUnitInfo {
+      CharUnitInfo() = default;
+      explicit CharUnitInfo(CharUnits Off) : Offset(Off) {}
+      CharUnits Size;
+      CharUnits Padding;
+      CharUnits Offset;
+    };
+    auto MapFunc = [&ASTContext, &RL](FieldDecl *FD) {
+      CharUnitInfo Info;
+      std::tie(Info.Size, std::ignore) =
+          ASTContext.getTypeInfoInChars(FD->getType());
+
+      auto FieldOffsetBits = RL.getFieldOffset(FD->getFieldIndex());
+      Info.Offset = ASTContext.toCharUnitsFromBits(FieldOffsetBits);
+      return Info;
+    };
+    auto LeftFoldFunc =
+        [&ASTContext, &RL](const CharUnitInfo &Old, const CharUnitInfo &New) {
+          CharUnitInfo Retval;
+          Retval.Padding = Old.Padding + (New.Offset - Old.Offset);
+          Retval.Offset = New.Offset + New.Size;
+          return Retval;
+        };
+
+    CharUnits OptimalOffset =
+        ASTContext.toCharUnitsFromBits(RL.getFieldOffset(0));
+
+    CharUnitInfo Results = mapLeftFold(
+        RD->fields(), CharUnitInfo(OptimalOffset), MapFunc, LeftFoldFunc);
+
+    Results.Padding += RL.getSize() - Results.Offset; // Measure tail padding.
+    return Results.Padding;
+  }
+
+  static CharUnits calculateOptimalPad(const RecordDecl *RD,
+                                       const ASTContext &ASTContext,
+                                       const ASTRecordLayout &RL) {
+    struct CharUnitPair {
+      CharUnits Align;
+      CharUnits Size;
+      bool operator<(const CharUnitPair &RHS) const {
+        // Order from small alignments to large alignments,
+        // then large sizes to small sizes.
+        // In the typical case, this will result in us popping
+        // the back element of the vector, instead of shifting lots
+        // of elements.
+        return std::make_pair(Align, -Size) <
+               std::make_pair(RHS.Align, -RHS.Size);
+      }
+    };
+    SmallVector<CharUnitPair, 20> Fields;
+    auto Transformation = [](const FieldDecl *FD) {
+      CharUnitPair RetVal;
+      auto &Ctx = FD->getASTContext();
+      std::tie(RetVal.Size, RetVal.Align) =
+          Ctx.getTypeInfoInChars(FD->getType());
+      assert(llvm::isPowerOf2_64(RetVal.Align.getQuantity()));
+      if (auto Max = FD->getMaxAlignment())
+        RetVal.Align = std::max(Ctx.toCharUnitsFromBits(Max), RetVal.Align);
+      return RetVal;
+    };
+    std::transform(RD->field_begin(), RD->field_end(),
+                   std::back_inserter(Fields), Transformation);
+    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;
+
+    while (!Fields.empty()) {
+      unsigned TrailingZeros =
+          llvm::countTrailingZeros((unsigned long long)NewOffset.getQuantity());
+      // If NewOffset is zero, then countTrailingZeros will be 64.  Shifting
+      // 64 will overflow our unsigned long long.  Shifting 63 will turn
+      // 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()};
+      auto CurBegin = Fields.begin();
+      auto CurEnd = Fields.end();
+      auto Iter = std::upper_bound(CurBegin, CurEnd, InsertPoint);
+      if (Iter != CurBegin) {
+        // We found a field that we can layout with the current alignment.
+        --Iter;
+        NewOffset += Iter->Size;
+        Fields.erase(Iter);
+      } else {
+        // We are poorly aligned, and we need to pad in order to layout another
+        // field.  Round up to at least the smallest field aligment that we
+        // currently have.
+        CharUnits NextOffset = NewOffset.RoundUpToAlignment(Fields[0].Align);
+        NewPad += NextOffset - NewOffset;
+        NewOffset = NextOffset;
+      }
+    }
+    // Calculate tail padding.
+    CharUnits NewSize = NewOffset.RoundUpToAlignment(RL.getAlignment());
+    NewPad += NewSize - NewOffset;
+    return NewPad;
+  }
+
+  void reportRecord(const RecordDecl *RD, CharUnits BaselinePad,
+                    CharUnits TargetPad, BugReporter &BR) 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()) << "'";
+
+    if (auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+      // TODO: make this show up better in the console output and in
+      // the HTML.  Maybe just make it show up in HTML like the path
+      // diagnostics show.
+      SourceLocation ILoc = TSD->getPointOfInstantiation();
+      if (ILoc.isValid())
+        Os << " instantiated here: "
+           << ILoc.printToString(BR.getSourceManager());
+    }
+
+    Os << " (" << BaselinePad.getQuantity() << " padding bytes, where "
+       << TargetPad.getQuantity() << " is optimal)";
+
+    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));
+  }
+};
+}
+
+void ento::registerPaddingChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<PaddingChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -46,6 +46,8 @@
 def DeadCode : Package<"deadcode">;
 def DeadCodeAlpha : Package<"deadcode">, InPackage<Alpha>, Hidden;
 
+def Performance : Package<"performance">;
+
 def Security : Package <"security">;
 def InsecureAPI : Package<"insecureAPI">, InPackage<Security>;
 def SecurityAlpha : Package<"security">, InPackage<Alpha>, Hidden;
@@ -273,6 +275,18 @@
 } // end "alpha.deadcode"
 
 //===----------------------------------------------------------------------===//
+// Performance checkers.
+//===----------------------------------------------------------------------===//
+
+let ParentPackage = Performance in {
+
+def PaddingChecker : Checker<"Padding">,
+  HelpText<"Check for excessively padded structs.">,
+  DescFile<"PaddingChecker.cpp">;
+
+} // end: "padding"
+
+//===----------------------------------------------------------------------===//
 // Security checkers.
 //===----------------------------------------------------------------------===//
 
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -57,6 +57,7 @@
   ObjCMissingSuperCallChecker.cpp
   ObjCSelfInitChecker.cpp
   ObjCUnusedIVarsChecker.cpp
+  PaddingChecker.cpp
   PointerArithChecker.cpp
   PointerSubChecker.cpp
   PthreadLockChecker.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to