Skip to content

Commit

Permalink
Fix #12266 (Performance: valueFlowCondition slowdown) (#5837)
Browse files Browse the repository at this point in the history
  • Loading branch information
@danmar
danmar authored Jan 29, 2024
1 parent f3bcfd5 commit 485df98
Show file tree
Hide file tree
Showing 16 changed files with 161 additions and 96 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ $(libcppdir)/errorlogger.o: lib/errorlogger.cpp externals/tinyxml2/tinyxml2.h li
$(libcppdir)/errortypes.o: lib/errortypes.cpp lib/config.h lib/errortypes.h lib/utils.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/errortypes.cpp

$(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueptr.h lib/vfvalue.h
$(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueptr.h lib/vfvalue.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/forwardanalyzer.cpp

$(libcppdir)/fwdanalysis.o: lib/fwdanalysis.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/fwdanalysis.h lib/library.h lib/mathlib.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
Expand Down
30 changes: 24 additions & 6 deletions lib/forwardanalyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
#include "analyzer.h"
#include "astutils.h"
#include "config.h"
#include "errorlogger.h"
#include "errortypes.h"
#include "mathlib.h"
#include "settings.h"
#include "symboldatabase.h"
#include "token.h"
#include "tokenlist.h"
#include "utils.h"
#include "valueptr.h"
#include "vfvalue.h"
Expand All @@ -45,16 +47,19 @@ namespace {
struct ForwardTraversal {
enum class Progress { Continue, Break, Skip };
enum class Terminate { None, Bail, Inconclusive };
ForwardTraversal(const ValuePtr<Analyzer>& analyzer, const Settings& settings)
: analyzer(analyzer), settings(settings)
ForwardTraversal(const ValuePtr<Analyzer>& analyzer, const TokenList& tokenList, ErrorLogger* const errorLogger, const Settings& settings)
: analyzer(analyzer), tokenList(tokenList), errorLogger(errorLogger), settings(settings)
{}
ValuePtr<Analyzer> analyzer;
const TokenList& tokenList;
ErrorLogger* const errorLogger;
const Settings& settings;
Analyzer::Action actions;
bool analyzeOnly{};
bool analyzeTerminate{};
Analyzer::Terminate terminate = Analyzer::Terminate::None;
std::vector<Token*> loopEnds;
int branchCount = 0;

Progress Break(Analyzer::Terminate t = Analyzer::Terminate::None) {
if ((!analyzeOnly || analyzeTerminate) && t != Analyzer::Terminate::None)
Expand Down Expand Up @@ -648,6 +653,10 @@ namespace {
}
} else if (tok->isControlFlowKeyword() && Token::Match(tok, "if|while|for (") &&
Token::simpleMatch(tok->next()->link(), ") {")) {
if (settings.checkLevel == Settings::CheckLevel::normal && ++branchCount > 4) {
reportError(Severity::information, "normalCheckLevelMaxBranches", "Limit analysis of branches. Use --check-level=exhausive to analyze all branches.");
return Break(Analyzer::Terminate::Bail);
}
Token* endCond = tok->next()->link();
Token* endBlock = endCond->next()->link();
Token* condTok = getCondTok(tok);
Expand Down Expand Up @@ -828,6 +837,15 @@ namespace {
return Progress::Continue;
}

void reportError(Severity severity, const std::string& id, const std::string& msg) {
if (errorLogger) {
const ErrorMessage::FileLocation loc(tokenList.getSourceFilePath(), 1, 1);
const std::list<ErrorMessage::FileLocation> callstack{loc};
const ErrorMessage errmsg(callstack, tokenList.getSourceFilePath(), severity, msg, id, Certainty::normal);
errorLogger->reportErr(errmsg);
}
}

static bool isFunctionCall(const Token* tok)
{
if (!Token::simpleMatch(tok, "("))
Expand Down Expand Up @@ -886,24 +904,24 @@ namespace {
};
}

Analyzer::Result valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const Settings& settings)
Analyzer::Result valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const TokenList& tokenList, ErrorLogger* const errorLogger, const Settings& settings)
{
if (a->invalid())
return Analyzer::Result{Analyzer::Action::None, Analyzer::Terminate::Bail};
ForwardTraversal ft{a, settings};
ForwardTraversal ft{a, tokenList, errorLogger, settings};
if (start)
ft.analyzer->updateState(start);
ft.updateRange(start, end);
return Analyzer::Result{ ft.actions, ft.terminate };
}

Analyzer::Result valueFlowGenericForward(Token* start, const ValuePtr<Analyzer>& a, const Settings& settings)
Analyzer::Result valueFlowGenericForward(Token* start, const ValuePtr<Analyzer>& a, const TokenList& tokenList, ErrorLogger* const errorLogger, const Settings& settings)
{
if (Settings::terminated())
throw TerminateException();
if (a->invalid())
return Analyzer::Result{Analyzer::Action::None, Analyzer::Terminate::Bail};
ForwardTraversal ft{a, settings};
ForwardTraversal ft{a, tokenList, errorLogger, settings};
ft.updateRecursive(start);
return Analyzer::Result{ ft.actions, ft.terminate };
}
6 changes: 5 additions & 1 deletion lib/forwardanalyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,19 @@

#include "analyzer.h"

class ErrorLogger;
class Settings;
class Token;
class TokenList;
template<class T> class ValuePtr;

Analyzer::Result valueFlowGenericForward(Token* start,
const Token* end,
const ValuePtr<Analyzer>& a,
const TokenList& tokenList,
ErrorLogger* const errorLogger,
const Settings& settings);

Analyzer::Result valueFlowGenericForward(Token* start, const ValuePtr<Analyzer>& a, const Settings& settings);
Analyzer::Result valueFlowGenericForward(Token* start, const ValuePtr<Analyzer>& a, const TokenList& tokenList, ErrorLogger* const errorLogger, const Settings& settings);

#endif
22 changes: 14 additions & 8 deletions lib/reverseanalyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@

namespace {
struct ReverseTraversal {
ReverseTraversal(const ValuePtr<Analyzer>& analyzer, const Settings& settings)
: analyzer(analyzer), settings(settings)
ReverseTraversal(const ValuePtr<Analyzer>& analyzer, const TokenList& tokenlist, ErrorLogger* const errorLogger, const Settings& settings)
: analyzer(analyzer), tokenlist(tokenlist), errorLogger(errorLogger), settings(settings)
{}
ValuePtr<Analyzer> analyzer;
const TokenList& tokenlist;
ErrorLogger* const errorLogger;
const Settings& settings;

std::pair<bool, bool> evalCond(const Token* tok) const {
Expand Down Expand Up @@ -239,6 +241,8 @@ namespace {
valueFlowGenericForward(nextAfterAstRightmostLeaf(assignTok->astOperand2()),
assignTok->astOperand2()->scope()->bodyEnd,
a,
tokenlist,
errorLogger,
settings);
}
// Assignment to
Expand All @@ -251,8 +255,10 @@ namespace {
valueFlowGenericForward(nextAfterAstRightmostLeaf(assignTok->astOperand2()),
assignTok->astOperand2()->scope()->bodyEnd,
a,
tokenlist,
errorLogger,
settings);
valueFlowGenericReverse(assignTok->astOperand1()->previous(), end, a, settings);
valueFlowGenericReverse(assignTok->astOperand1()->previous(), end, a, tokenlist, errorLogger, settings);
}
}
}
Expand Down Expand Up @@ -285,7 +291,7 @@ namespace {
break;
if (condAction.isModified())
break;
valueFlowGenericForward(condTok, analyzer, settings);
valueFlowGenericForward(condTok, analyzer, tokenlist, errorLogger, settings);
}
Token* thenEnd;
const bool hasElse = Token::simpleMatch(tok->link()->tokAt(-2), "} else {");
Expand Down Expand Up @@ -314,7 +320,7 @@ namespace {
break;

if (!thenAction.isModified() && !elseAction.isModified())
valueFlowGenericForward(condTok, analyzer, settings);
valueFlowGenericForward(condTok, analyzer, tokenlist, errorLogger, settings);
else if (condAction.isRead())
break;
// If the condition modifies the variable then bail
Expand All @@ -333,7 +339,7 @@ namespace {
}
Token* condTok = getCondTokFromEnd(tok->link());
if (condTok) {
Analyzer::Result r = valueFlowGenericForward(condTok, analyzer, settings);
Analyzer::Result r = valueFlowGenericForward(condTok, analyzer, tokenlist, errorLogger, settings);
if (r.action.isModified())
break;
}
Expand Down Expand Up @@ -389,10 +395,10 @@ namespace {
};
}

void valueFlowGenericReverse(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const Settings& settings)
void valueFlowGenericReverse(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const TokenList& tokenlist, ErrorLogger* const errorLogger, const Settings& settings)
{
if (a->invalid())
return;
ReverseTraversal rt{a, settings};
ReverseTraversal rt{a, tokenlist, errorLogger, settings};
rt.traverse(start, end);
}
4 changes: 3 additions & 1 deletion lib/reverseanalyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
#define reverseanalyzerH

struct Analyzer;
class ErrorLogger;
class Settings;
class Token;
class TokenList;
template<class T>
class ValuePtr;

void valueFlowGenericReverse(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const Settings& settings);
void valueFlowGenericReverse(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const TokenList& tokenlist, ErrorLogger* const errorLogger, const Settings& settings);

#endif
2 changes: 2 additions & 0 deletions lib/settings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,15 @@ void Settings::loadSummaries()
void Settings::setCheckLevelExhaustive()
{
// Checking can take a little while. ~ 10 times slower than normal analysis is OK.
checkLevel = CheckLevel::exhaustive;
performanceValueFlowMaxIfCount = -1;
performanceValueFlowMaxSubFunctionArgs = 256;
}

void Settings::setCheckLevelNormal()
{
// Checking should finish in reasonable time.
checkLevel = CheckLevel::normal;
performanceValueFlowMaxSubFunctionArgs = 8;
performanceValueFlowMaxIfCount = 100;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/settings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,12 @@ class CPPCHECKLIB WARN_UNUSED Settings {
void setCheckLevelExhaustive();
void setCheckLevelNormal();

enum class CheckLevel {
exhaustive,
normal
};
CheckLevel checkLevel = CheckLevel::normal;

private:
static std::string parseEnabled(const std::string &str, std::tuple<SimpleEnableGroup<Severity>, SimpleEnableGroup<Checks>> &groups);
std::string applyEnabled(const std::string &str, bool enable);
Expand Down
Loading

0 comments on commit 485df98

Please sign in to comment.