Magic-Mirror/llvm-capstone
1
0
Fork 0
mirror of https://github.com/capstone-engine/llvm-capstone.git synced 2024-12-15 04:00:56 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

[analyzer] Allow padding checker to traverse simple class hierarchies

Browse Source 
The existing padding checker skips classes that have any base classes. 
This patch allows the checker to traverse very simple cases: 
classes that have no fields and have exactly one base class. 
This is important mostly in the case of array declarations.

Patch by Max Bernstein!

Test plan: make check-all

Differential revision: https://reviews.llvm.org/D53206

llvm-svn: 345558
This commit is contained in:
Alexander Shaposhnikov 2018-10-30 01:20:37 +00:00
parent 7c180fa915
commit e2f073463e
2 changed files with 58 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
View File

@ -75,6 +75,20 @@ public:
if (shouldSkipDecl(RD))
return;
// TODO: Figure out why we are going through declarations and not only
// definitions.
if (!(RD = RD->getDefinition()))
return;
// This is the simplest correct case: a class with no fields and one base
// class. Other cases are more complicated because of how the base classes
// & fields might interact, so we don't bother dealing with them.
// TODO: Support other combinations of base classes and fields.
if (auto *CXXRD = dyn_cast<CXXRecordDecl>(RD))
if (CXXRD->field_empty() && CXXRD->getNumBases() == 1)
return visitRecord(CXXRD->bases().begin()->getType()->getAsRecordDecl(),
PadMultiplier);
auto &ASTContext = RD->getASTContext();
const ASTRecordLayout &RL = ASTContext.getASTRecordLayout(RD);
assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
@ -112,12 +126,15 @@ public:
if (RT == nullptr)
return;
// TODO: Recurse into the fields and base classes to see if any
// of those have excess padding.
// TODO: Recurse into the fields to see if they have excess padding.
visitRecord(RT->getDecl(), Elts);
}
bool shouldSkipDecl(const RecordDecl *RD) const {
// TODO: Figure out why we are going through declarations and not only
// definitions.
if (!(RD = RD->getDefinition()))
return true;
auto Location = RD->getLocation();
// If the construct doesn't have a source file, then it's not something
// we want to diagnose.
@ -132,13 +149,14 @@ public:
// 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)
// We will skip objects with base classes for now, unless they do not
// have fields.
// TODO: Handle more base class scenarios.
if (!CXXRD->field_empty() && CXXRD->getNumBases() != 0)
return true;
if (CXXRD->field_empty() && CXXRD->getNumBases() != 1)
return true;
// Virtual bases are complicated, skipping those for now.
if (CXXRD->getNumVBases() != 0)
@ -150,6 +168,10 @@ public:
if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
return true;
}
// How do you reorder fields if you haven't got any?
else if (RD->field_empty())
return true;
auto IsTrickyField = [](const FieldDecl *FD) -> bool {
// Bitfield layout is hard.
if (FD->isBitField())
@ -323,7 +345,7 @@ public:
BR->emitReport(std::move(Report));
}
};
}
} // namespace
void ento::registerPaddingChecker(CheckerManager &Mgr) {
Mgr.registerChecker<PaddingChecker>();

28
clang/test/Analysis/padding_inherit.cpp Normal file
View File

@ -0,0 +1,28 @@
// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=20 -verify %s
// A class that has no fields and one base class should visit that base class
// instead. Note that despite having excess padding of 2, this is flagged
// because of its usage in an array of 100 elements below (`ais').
// TODO: Add a note to the bug report with BugReport::addNote() to mention the
// variable using the class and also mention what class is inherting from what.
// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
struct FakeIntSandwich {
char c1;
int i;
char c2;
};
struct AnotherIntSandwich : FakeIntSandwich { // no-warning
};
// But we don't yet support multiple base classes.
struct IntSandwich {};
struct TooManyBaseClasses : FakeIntSandwich, IntSandwich { // no-warning
};
AnotherIntSandwich ais[100];
struct Empty {};
struct DoubleEmpty : Empty { // no-warning
Empty e;
};