One of the reasons why AnalyzerOptions is so chaotic is that options can be retrieved from the command line whenever and wherever. This allowed for some options to be forgotten for a looooooong time. Have you ever heard of "region-store-small-struct-limit"? In order to prevent this in the future, I'm proposing to restrict AnalyzerOptions' interface so that only checker options can be retrieved without special getters. I would like to make every option be accessible only through a getter, but checkers from plugins are a thing, so I'll have to figure something out for that. This also forces developers who'd like to add a new option to register it properly in the .def file. This is done by * making the third checker pointer parameter non-optional, and checked by an assert to be non-null. * I added new, but private non-checkers option initializers, meant only for internal use, * Renamed these methods accordingly (mind the consistent name for once with getBooleanOption!): - getOptionAsString -> getCheckerStringOption, - getOptionAsInteger -> getCheckerIntegerOption * The 3 functions meant for initializing data members (with the not very descriptive getBooleanOption, getOptionAsString and getOptionAsUInt names) were renamed to be overloads of the getAndInitOption function name. * All options were in some way retrieved via getCheckerOption. I removed it, and moved the logic to getStringOption and getCheckerStringOption. This did cause some code duplication, but that's the only way I could do it, now that checker and non-checker options are separated. Note that the non-checker version inserts the new option to the ConfigTable with the default value, but the checker version only attempts to find already existing entries. This is how it always worked, but this is clunky and I might end reworking that too, so we can eventually get a ConfigTable that contains the entire configuration of the analyzer. Differential Revision: https://reviews.llvm.org/D53483 llvm-svn: 346113
205 lines
8.1 KiB
C++
205 lines
8.1 KiB
C++
//===--- CloneChecker.cpp - Clone detection checker -------------*- C++ -*-===//
|
|
//
|
|
// The LLVM Compiler Infrastructure
|
|
//
|
|
// This file is distributed under the University of Illinois Open Source
|
|
// License. See LICENSE.TXT for details.
|
|
//
|
|
//===----------------------------------------------------------------------===//
|
|
///
|
|
/// \file
|
|
/// CloneChecker is a checker that reports clones in the current translation
|
|
/// unit.
|
|
///
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
#include "ClangSACheckers.h"
|
|
#include "clang/Analysis/CloneDetection.h"
|
|
#include "clang/Basic/Diagnostic.h"
|
|
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
|
|
#include "clang/StaticAnalyzer/Core/Checker.h"
|
|
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
|
|
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
|
|
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
|
|
|
|
using namespace clang;
|
|
using namespace ento;
|
|
|
|
namespace {
|
|
class CloneChecker
|
|
: public Checker<check::ASTCodeBody, check::EndOfTranslationUnit> {
|
|
mutable CloneDetector Detector;
|
|
mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
|
|
|
|
public:
|
|
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
|
|
BugReporter &BR) const;
|
|
|
|
void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
|
|
AnalysisManager &Mgr, BugReporter &BR) const;
|
|
|
|
/// Reports all clones to the user.
|
|
void reportClones(BugReporter &BR, AnalysisManager &Mgr,
|
|
std::vector<CloneDetector::CloneGroup> &CloneGroups) const;
|
|
|
|
/// Reports only suspicious clones to the user along with informaton
|
|
/// that explain why they are suspicious.
|
|
void reportSuspiciousClones(
|
|
BugReporter &BR, AnalysisManager &Mgr,
|
|
std::vector<CloneDetector::CloneGroup> &CloneGroups) const;
|
|
};
|
|
} // end anonymous namespace
|
|
|
|
void CloneChecker::checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
|
|
BugReporter &BR) const {
|
|
// Every statement that should be included in the search for clones needs to
|
|
// be passed to the CloneDetector.
|
|
Detector.analyzeCodeBody(D);
|
|
}
|
|
|
|
void CloneChecker::checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
|
|
AnalysisManager &Mgr,
|
|
BugReporter &BR) const {
|
|
// At this point, every statement in the translation unit has been analyzed by
|
|
// the CloneDetector. The only thing left to do is to report the found clones.
|
|
|
|
int MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
|
|
"MinimumCloneComplexity", 50, this);
|
|
assert(MinComplexity >= 0);
|
|
|
|
bool ReportSuspiciousClones = Mgr.getAnalyzerOptions()
|
|
.getCheckerBooleanOption("ReportSuspiciousClones", true, this);
|
|
|
|
bool ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
|
|
"ReportNormalClones", true, this);
|
|
|
|
StringRef IgnoredFilesPattern = Mgr.getAnalyzerOptions()
|
|
.getCheckerStringOption("IgnoredFilesPattern", "", this);
|
|
|
|
// Let the CloneDetector create a list of clones from all the analyzed
|
|
// statements. We don't filter for matching variable patterns at this point
|
|
// because reportSuspiciousClones() wants to search them for errors.
|
|
std::vector<CloneDetector::CloneGroup> AllCloneGroups;
|
|
|
|
Detector.findClones(
|
|
AllCloneGroups, FilenamePatternConstraint(IgnoredFilesPattern),
|
|
RecursiveCloneTypeIIHashConstraint(), MinGroupSizeConstraint(2),
|
|
MinComplexityConstraint(MinComplexity),
|
|
RecursiveCloneTypeIIVerifyConstraint(), OnlyLargestCloneConstraint());
|
|
|
|
if (ReportSuspiciousClones)
|
|
reportSuspiciousClones(BR, Mgr, AllCloneGroups);
|
|
|
|
// We are done for this translation unit unless we also need to report normal
|
|
// clones.
|
|
if (!ReportNormalClones)
|
|
return;
|
|
|
|
// Now that the suspicious clone detector has checked for pattern errors,
|
|
// we also filter all clones who don't have matching patterns
|
|
CloneDetector::constrainClones(AllCloneGroups,
|
|
MatchingVariablePatternConstraint(),
|
|
MinGroupSizeConstraint(2));
|
|
|
|
reportClones(BR, Mgr, AllCloneGroups);
|
|
}
|
|
|
|
static PathDiagnosticLocation makeLocation(const StmtSequence &S,
|
|
AnalysisManager &Mgr) {
|
|
ASTContext &ACtx = Mgr.getASTContext();
|
|
return PathDiagnosticLocation::createBegin(
|
|
S.front(), ACtx.getSourceManager(),
|
|
Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl()));
|
|
}
|
|
|
|
void CloneChecker::reportClones(
|
|
BugReporter &BR, AnalysisManager &Mgr,
|
|
std::vector<CloneDetector::CloneGroup> &CloneGroups) const {
|
|
|
|
if (!BT_Exact)
|
|
BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone"));
|
|
|
|
for (const CloneDetector::CloneGroup &Group : CloneGroups) {
|
|
// We group the clones by printing the first as a warning and all others
|
|
// as a note.
|
|
auto R = llvm::make_unique<BugReport>(*BT_Exact, "Duplicate code detected",
|
|
makeLocation(Group.front(), Mgr));
|
|
R->addRange(Group.front().getSourceRange());
|
|
|
|
for (unsigned i = 1; i < Group.size(); ++i)
|
|
R->addNote("Similar code here", makeLocation(Group[i], Mgr),
|
|
Group[i].getSourceRange());
|
|
BR.emitReport(std::move(R));
|
|
}
|
|
}
|
|
|
|
void CloneChecker::reportSuspiciousClones(
|
|
BugReporter &BR, AnalysisManager &Mgr,
|
|
std::vector<CloneDetector::CloneGroup> &CloneGroups) const {
|
|
std::vector<VariablePattern::SuspiciousClonePair> Pairs;
|
|
|
|
for (const CloneDetector::CloneGroup &Group : CloneGroups) {
|
|
for (unsigned i = 0; i < Group.size(); ++i) {
|
|
VariablePattern PatternA(Group[i]);
|
|
|
|
for (unsigned j = i + 1; j < Group.size(); ++j) {
|
|
VariablePattern PatternB(Group[j]);
|
|
|
|
VariablePattern::SuspiciousClonePair ClonePair;
|
|
// For now, we only report clones which break the variable pattern just
|
|
// once because multiple differences in a pattern are an indicator that
|
|
// those differences are maybe intended (e.g. because it's actually a
|
|
// different algorithm).
|
|
// FIXME: In very big clones even multiple variables can be unintended,
|
|
// so replacing this number with a percentage could better handle such
|
|
// cases. On the other hand it could increase the false-positive rate
|
|
// for all clones if the percentage is too high.
|
|
if (PatternA.countPatternDifferences(PatternB, &ClonePair) == 1) {
|
|
Pairs.push_back(ClonePair);
|
|
break;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
if (!BT_Suspicious)
|
|
BT_Suspicious.reset(
|
|
new BugType(this, "Suspicious code clone", "Code clone"));
|
|
|
|
ASTContext &ACtx = BR.getContext();
|
|
SourceManager &SM = ACtx.getSourceManager();
|
|
AnalysisDeclContext *ADC =
|
|
Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl());
|
|
|
|
for (VariablePattern::SuspiciousClonePair &Pair : Pairs) {
|
|
// FIXME: We are ignoring the suggestions currently, because they are
|
|
// only 50% accurate (even if the second suggestion is unavailable),
|
|
// which may confuse the user.
|
|
// Think how to perform more accurate suggestions?
|
|
|
|
auto R = llvm::make_unique<BugReport>(
|
|
*BT_Suspicious,
|
|
"Potential copy-paste error; did you really mean to use '" +
|
|
Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?",
|
|
PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM,
|
|
ADC));
|
|
R->addRange(Pair.FirstCloneInfo.Mention->getSourceRange());
|
|
|
|
R->addNote("Similar code using '" +
|
|
Pair.SecondCloneInfo.Variable->getNameAsString() + "' here",
|
|
PathDiagnosticLocation::createBegin(Pair.SecondCloneInfo.Mention,
|
|
SM, ADC),
|
|
Pair.SecondCloneInfo.Mention->getSourceRange());
|
|
|
|
BR.emitReport(std::move(R));
|
|
}
|
|
}
|
|
|
|
//===----------------------------------------------------------------------===//
|
|
// Register CloneChecker
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
void ento::registerCloneChecker(CheckerManager &Mgr) {
|
|
Mgr.registerChecker<CloneChecker>();
|
|
}
|