diff --git a/Makefile b/Makefile
index 5d53dfe38..ca87d358d 100644
--- a/Makefile
+++ b/Makefile
@@ -201,6 +201,7 @@ LIBOBJ = $(libcppdir)/analyzerinfo.o \
$(libcppdir)/platform.o \
$(libcppdir)/preprocessor.o \
$(libcppdir)/programmemory.o \
+ $(libcppdir)/reverseanalyzer.o \
$(libcppdir)/settings.o \
$(libcppdir)/suppressions.o \
$(libcppdir)/symboldatabase.o \
@@ -508,7 +509,7 @@ $(libcppdir)/errortypes.o: lib/errortypes.cpp lib/config.h lib/errortypes.h
$(libcppdir)/exprengine.o: lib/exprengine.cpp lib/astutils.h lib/bughuntingchecks.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/exprengine.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/exprengine.o $(libcppdir)/exprengine.cpp
-$(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h
+$(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/forwardanalyzer.o $(libcppdir)/forwardanalyzer.cpp
$(libcppdir)/importproject.o: lib/importproject.cpp externals/picojson.h externals/tinyxml/tinyxml2.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h
@@ -538,6 +539,9 @@ $(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.
$(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/programmemory.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/programmemory.o $(libcppdir)/programmemory.cpp
+$(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h
+ $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/reverseanalyzer.o $(libcppdir)/reverseanalyzer.cpp
+
$(libcppdir)/settings.o: lib/settings.cpp lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h lib/valueflow.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/settings.o $(libcppdir)/settings.cpp
@@ -565,7 +569,7 @@ $(libcppdir)/tokenlist.o: lib/tokenlist.cpp externals/simplecpp/simplecpp.h lib/
$(libcppdir)/utils.o: lib/utils.cpp lib/config.h lib/utils.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/utils.o $(libcppdir)/utils.cpp
-$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/astutils.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/programmemory.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h
+$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/programmemory.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/valueflow.o $(libcppdir)/valueflow.cpp
cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlineparser.h cli/cppcheckexecutor.h cli/filelister.h cli/threadexecutor.h externals/tinyxml/tinyxml2.h lib/check.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
diff --git a/lib/analyzer.h b/lib/analyzer.h
new file mode 100644
index 000000000..60f8bd359
--- /dev/null
+++ b/lib/analyzer.h
@@ -0,0 +1,110 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2020 Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#ifndef analyzerH
+#define analyzerH
+
+#include
+#include
+
+class Token;
+template
+class ValuePtr;
+
+struct Analyzer {
+ struct Action {
+
+ Action() : mFlag(0) {}
+
+ // cppcheck-suppress noExplicitConstructor
+ Action(unsigned int f) : mFlag(f) {}
+
+ enum {
+ None = 0,
+ Read = (1 << 0),
+ Write = (1 << 1),
+ Invalid = (1 << 2),
+ Inconclusive = (1 << 3),
+ Match = (1 << 4),
+ Idempotent = (1 << 5),
+ };
+
+ void set(unsigned int f, bool state = true) { mFlag = state ? mFlag | f : mFlag & ~f; }
+
+ bool get(unsigned int f) const { return ((mFlag & f) != 0); }
+
+ bool isRead() const { return get(Read); }
+
+ bool isWrite() const { return get(Write); }
+
+ bool isInvalid() const { return get(Invalid); }
+
+ bool isInconclusive() const { return get(Inconclusive); }
+
+ bool isNone() const { return mFlag == None; }
+
+ bool isModified() const { return isWrite() || isInvalid(); }
+
+ bool isIdempotent() const { return get(Idempotent); }
+
+ bool matches() const { return get(Match); }
+
+ Action& operator|=(Action a)
+ {
+ set(a.mFlag);
+ return *this;
+ }
+
+ friend Action operator|(Action a, Action b)
+ {
+ a |= b;
+ return a;
+ }
+
+ friend bool operator==(Action a, Action b) { return a.mFlag == b.mFlag; }
+
+ friend bool operator!=(Action a, Action b) { return a.mFlag != b.mFlag; }
+
+ private:
+ unsigned int mFlag;
+ };
+
+ enum class Direction { Forward, Reverse };
+
+ /// Analyze a token
+ virtual Action analyze(const Token* tok, Direction d) const = 0;
+ /// Update the state of the value
+ virtual void update(Token* tok, Action a, Direction d) = 0;
+ /// Try to evaluate the value of a token(most likely a condition)
+ virtual std::vector evaluate(const Token* tok) const = 0;
+ /// Lower any values to possible
+ virtual bool lowerToPossible() = 0;
+ /// Lower any values to inconclusive
+ virtual bool lowerToInconclusive() = 0;
+ /// If the analysis is unsure whether to update a scope, this will return true if the analysis should bifurcate the scope
+ virtual bool updateScope(const Token* endBlock, bool modified) const = 0;
+ /// If the value is conditional
+ virtual bool isConditional() const = 0;
+ /// The condition that will be assumed during analysis
+ virtual void assume(const Token* tok, bool state, const Token* at = nullptr) = 0;
+ /// Return analyzer for expression at token
+ virtual ValuePtr reanalyze(Token* tok, const std::string& msg = "") const = 0;
+ virtual ~Analyzer() {}
+};
+
+#endif
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index 4c3bbc648..9f12df3d0 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -306,6 +306,18 @@ static bool hasToken(const Token * startTok, const Token * stopTok, const Token
return false;
}
+template )>
+static T* previousBeforeAstLeftmostLeafGeneric(T* tok)
+{
+ T* leftmostLeaf = tok;
+ while (leftmostLeaf && leftmostLeaf->astOperand1())
+ leftmostLeaf = leftmostLeaf->astOperand1();
+ return leftmostLeaf->previous();
+}
+
+const Token* previousBeforeAstLeftmostLeaf(const Token* tok) { return previousBeforeAstLeftmostLeafGeneric(tok); }
+Token* previousBeforeAstLeftmostLeaf(Token* tok) { return previousBeforeAstLeftmostLeafGeneric(tok); }
+
template )>
static T* nextAfterAstRightmostLeafGeneric(T* tok)
{
diff --git a/lib/astutils.h b/lib/astutils.h
index 3171d65fd..ce15d33c3 100644
--- a/lib/astutils.h
+++ b/lib/astutils.h
@@ -90,6 +90,9 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
bool isTemporary(bool cpp, const Token* tok, const Library* library, bool unknown = false);
+const Token* previousBeforeAstLeftmostLeaf(const Token* tok);
+Token* previousBeforeAstLeftmostLeaf(Token* tok);
+
const Token * nextAfterAstRightmostLeaf(const Token * tok);
Token* nextAfterAstRightmostLeaf(Token* tok);
diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
index 11cebd655..9da2180f9 100644
--- a/lib/checknullpointer.cpp
+++ b/lib/checknullpointer.cpp
@@ -205,7 +205,9 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Set
return false;
}
- if (Token::Match(tok, "%name% ("))
+ // If its a function pointer then check if its called
+ if (tok->variable() && tok->variable()->isPointer() && Token::Match(tok->variable()->nameToken(), "%name% ) (") &&
+ Token::Match(tok, "%name% ("))
return true;
if (Token::Match(tok, "%var% = %var% .") &&
diff --git a/lib/checksizeof.cpp b/lib/checksizeof.cpp
index bfa401161..411c22205 100644
--- a/lib/checksizeof.cpp
+++ b/lib/checksizeof.cpp
@@ -177,6 +177,9 @@ void CheckSizeof::checkSizeofForPointerSize()
while (Token::Match(variable2, "%var% ::|."))
variable2 = variable2->tokAt(2);
+ if (!variable)
+ continue;
+
// Ensure the variables are in the symbol database
// Also ensure the variables are pointers
// Only keep variables which are pointers
diff --git a/lib/cppcheck.vcxproj b/lib/cppcheck.vcxproj
index 2d7fa3217..7a8fd9464 100644
--- a/lib/cppcheck.vcxproj
+++ b/lib/cppcheck.vcxproj
@@ -101,6 +101,7 @@
+
diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
index 318c21b43..a4e9a26f0 100644
--- a/lib/forwardanalyzer.cpp
+++ b/lib/forwardanalyzer.cpp
@@ -10,12 +10,12 @@
struct ForwardTraversal {
enum class Progress { Continue, Break, Skip };
- ForwardTraversal(const ValuePtr& analyzer, const Settings* settings)
- : analyzer(analyzer), settings(settings), actions(ForwardAnalyzer::Action::None), analyzeOnly(false)
+ ForwardTraversal(const ValuePtr& analyzer, const Settings* settings)
+ : analyzer(analyzer), settings(settings), actions(Analyzer::Action::None), analyzeOnly(false)
{}
- ValuePtr analyzer;
+ ValuePtr analyzer;
const Settings* settings;
- ForwardAnalyzer::Action actions;
+ Analyzer::Action actions;
bool analyzeOnly;
bool stopUpdates() {
@@ -25,12 +25,9 @@ struct ForwardTraversal {
std::pair evalCond(const Token* tok) {
std::vector result = analyzer->evaluate(tok);
- bool checkThen = std::any_of(result.begin(), result.end(), [](int x) {
- return x;
- });
- bool checkElse = std::any_of(result.begin(), result.end(), [](int x) {
- return !x;
- });
+ // TODO: We should convert to bool
+ bool checkThen = std::any_of(result.begin(), result.end(), [](int x) { return x == 1; });
+ bool checkElse = std::any_of(result.begin(), result.end(), [](int x) { return x == 0; });
return std::make_pair(checkThen, checkElse);
}
@@ -124,10 +121,10 @@ struct ForwardTraversal {
}
Progress update(Token* tok) {
- ForwardAnalyzer::Action action = analyzer->analyze(tok);
+ Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Forward);
actions |= action;
if (!action.isNone() && !analyzeOnly)
- analyzer->update(tok, action);
+ analyzer->update(tok, action, Analyzer::Direction::Forward);
if (action.isInconclusive() && !analyzer->lowerToInconclusive())
return Progress::Break;
if (action.isInvalid())
@@ -153,19 +150,21 @@ struct ForwardTraversal {
}
template
- T* findRange(T* start, const Token* end, std::function pred) {
+ T* findRange(T* start, const Token* end, std::function pred)
+ {
for (T* tok = start; tok && tok != end; tok = tok->next()) {
- ForwardAnalyzer::Action action = analyzer->analyze(tok);
+ Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Forward);
if (pred(action))
return tok;
}
return nullptr;
}
- ForwardAnalyzer::Action analyzeRecursive(const Token* start) {
- ForwardAnalyzer::Action result = ForwardAnalyzer::Action::None;
- std::function f = [&](const Token* tok) {
- result = analyzer->analyze(tok);
+ Analyzer::Action analyzeRecursive(const Token* start)
+ {
+ Analyzer::Action result = Analyzer::Action::None;
+ std::function f = [&](const Token* tok) {
+ result = analyzer->analyze(tok, Analyzer::Direction::Forward);
if (result.isModified() || result.isInconclusive())
return Progress::Break;
return Progress::Continue;
@@ -174,10 +173,11 @@ struct ForwardTraversal {
return result;
}
- ForwardAnalyzer::Action analyzeRange(const Token* start, const Token* end) {
- ForwardAnalyzer::Action result = ForwardAnalyzer::Action::None;
+ Analyzer::Action analyzeRange(const Token* start, const Token* end)
+ {
+ Analyzer::Action result = Analyzer::Action::None;
for (const Token* tok = start; tok && tok != end; tok = tok->next()) {
- ForwardAnalyzer::Action action = analyzer->analyze(tok);
+ Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Forward);
if (action.isModified() || action.isInconclusive())
return action;
result = action;
@@ -211,25 +211,25 @@ struct ForwardTraversal {
Inconclusive,
};
- ForwardAnalyzer::Action analyzeScope(const Token* endBlock) {
- return analyzeRange(endBlock->link(), endBlock);
- }
+ Analyzer::Action analyzeScope(const Token* endBlock) { return analyzeRange(endBlock->link(), endBlock); }
- ForwardAnalyzer::Action checkScope(Token* endBlock) {
- ForwardAnalyzer::Action a = analyzeScope(endBlock);
+ Analyzer::Action checkScope(Token* endBlock)
+ {
+ Analyzer::Action a = analyzeScope(endBlock);
forkScope(endBlock, a.isModified());
return a;
}
- ForwardAnalyzer::Action checkScope(const Token* endBlock) {
- ForwardAnalyzer::Action a = analyzeScope(endBlock);
+ Analyzer::Action checkScope(const Token* endBlock)
+ {
+ Analyzer::Action a = analyzeScope(endBlock);
return a;
}
Progress updateLoop(Token* endBlock, Token* condTok, Token* initTok = nullptr, Token* stepTok = nullptr) {
const bool isDoWhile = precedes(endBlock, condTok);
- ForwardAnalyzer::Action bodyAnalysis = analyzeScope(endBlock);
- ForwardAnalyzer::Action allAnalysis = bodyAnalysis;
+ Analyzer::Action bodyAnalysis = analyzeScope(endBlock);
+ Analyzer::Action allAnalysis = bodyAnalysis;
if (condTok)
allAnalysis |= analyzeRecursive(condTok);
if (initTok)
@@ -258,7 +258,7 @@ struct ForwardTraversal {
forkScope(endBlock, allAnalysis.isModified());
if (bodyAnalysis.isModified()) {
- Token* writeTok = findRange(endBlock->link(), endBlock, std::mem_fn(&ForwardAnalyzer::Action::isModified));
+ Token* writeTok = findRange(endBlock->link(), endBlock, std::mem_fn(&Analyzer::Action::isModified));
const Token* nextStatement = Token::findmatch(writeTok, ";|}", endBlock);
if (!Token::Match(nextStatement, ";|} break ;"))
return Progress::Break;
@@ -381,8 +381,8 @@ struct ForwardTraversal {
// Check if condition is true or false
bool checkThen, checkElse;
std::tie(checkThen, checkElse) = evalCond(condTok);
- ForwardAnalyzer::Action thenAction = ForwardAnalyzer::Action::None;
- ForwardAnalyzer::Action elseAction = ForwardAnalyzer::Action::None;
+ Analyzer::Action thenAction = Analyzer::Action::None;
+ Analyzer::Action elseAction = Analyzer::Action::None;
bool hasElse = Token::simpleMatch(endBlock, "} else {");
bool bail = false;
@@ -445,7 +445,7 @@ struct ForwardTraversal {
}
} else if (Token::simpleMatch(tok, "try {")) {
Token* endBlock = tok->next()->link();
- ForwardAnalyzer::Action a = analyzeScope(endBlock);
+ Analyzer::Action a = analyzeScope(endBlock);
if (updateRange(tok->next(), endBlock) == Progress::Break)
return Progress::Break;
if (a.isModified())
@@ -567,12 +567,19 @@ struct ForwardTraversal {
};
-ForwardAnalyzer::Action valueFlowGenericForward(Token* start,
- const Token* end,
- const ValuePtr& fa,
- const Settings* settings)
+Analyzer::Action valueFlowGenericForward(Token* start,
+ const Token* end,
+ const ValuePtr& a,
+ const Settings* settings)
{
- ForwardTraversal ft{fa, settings};
+ ForwardTraversal ft{a, settings};
ft.updateRange(start, end);
return ft.actions;
}
+
+Analyzer::Action valueFlowGenericForward(Token* start, const ValuePtr& a, const Settings* settings)
+{
+ ForwardTraversal ft{a, settings};
+ ft.updateRecursive(start);
+ return ft.actions;
+}
diff --git a/lib/forwardanalyzer.h b/lib/forwardanalyzer.h
index de9cfad6c..f4388bee8 100644
--- a/lib/forwardanalyzer.h
+++ b/lib/forwardanalyzer.h
@@ -19,103 +19,18 @@
#ifndef forwardanalyzerH
#define forwardanalyzerH
+#include "analyzer.h"
#include
class Settings;
class Token;
template class ValuePtr;
-struct ForwardAnalyzer {
- struct Action {
+Analyzer::Action valueFlowGenericForward(Token* start,
+ const Token* end,
+ const ValuePtr& a,
+ const Settings* settings);
- Action() : mFlag(0) {}
-
- // cppcheck-suppress noExplicitConstructor
- Action(unsigned int f) : mFlag(f) {}
-
- enum {
- None = 0,
- Read = (1 << 0),
- Write = (1 << 1),
- Invalid = (1 << 2),
- Inconclusive = (1 << 3),
- };
-
- void set(unsigned int f, bool state = true) {
- mFlag = state ? mFlag | f : mFlag & ~f;
- }
-
- bool get(unsigned int f) const {
- return ((mFlag & f) != 0);
- }
-
- bool isRead() const {
- return get(Read);
- }
-
- bool isWrite() const {
- return get(Write);
- }
-
- bool isInvalid() const {
- return get(Invalid);
- }
-
- bool isInconclusive() const {
- return get(Inconclusive);
- }
-
- bool isNone() const {
- return mFlag == None;
- }
-
- bool isModified() const {
- return isWrite() || isInvalid();
- }
-
- Action& operator|=(Action a) {
- set(a.mFlag);
- return *this;
- }
-
- friend Action operator|(Action a, Action b) {
- a |= b;
- return a;
- }
-
- friend bool operator==(Action a, Action b) {
- return a.mFlag == b.mFlag;
- }
-
- friend bool operator!=(Action a, Action b) {
- return a.mFlag != b.mFlag;
- }
-
- private:
- unsigned int mFlag;
- };
- /// Analyze a token
- virtual Action analyze(const Token* tok) const = 0;
- /// Update the state of the value
- virtual void update(Token* tok, Action a) = 0;
- /// Try to evaluate the value of a token(most likely a condition)
- virtual std::vector evaluate(const Token* tok) const = 0;
- /// Lower any values to possible
- virtual bool lowerToPossible() = 0;
- /// Lower any values to inconclusive
- virtual bool lowerToInconclusive() = 0;
- /// If the analysis is unsure whether to update a scope, this will return true if the analysis should bifurcate the scope
- virtual bool updateScope(const Token* endBlock, bool modified) const = 0;
- /// If the value is conditional
- virtual bool isConditional() const = 0;
- /// The condition that will be assumed during analysis
- virtual void assume(const Token* tok, bool state, const Token* at = nullptr) = 0;
- virtual ~ForwardAnalyzer() {}
-};
-
-ForwardAnalyzer::Action valueFlowGenericForward(Token* start,
- const Token* end,
- const ValuePtr& fa,
- const Settings* settings);
+Analyzer::Action valueFlowGenericForward(Token* start, const ValuePtr& a, const Settings* settings);
#endif
diff --git a/lib/lib.pri b/lib/lib.pri
index 98558776d..16d0f2df0 100644
--- a/lib/lib.pri
+++ b/lib/lib.pri
@@ -48,6 +48,7 @@ HEADERS += $${PWD}/analyzerinfo.h \
$${PWD}/platform.h \
$${PWD}/preprocessor.h \
$${PWD}/programmemory.h \
+ $${PWD}/reverseanalyzer.h \
$${PWD}/settings.h \
$${PWD}/suppressions.h \
$${PWD}/symboldatabase.h \
@@ -104,6 +105,7 @@ SOURCES += $${PWD}/analyzerinfo.cpp \
$${PWD}/platform.cpp \
$${PWD}/preprocessor.cpp \
$${PWD}/programmemory.cpp \
+ $${PWD}/reverseanalyzer.cpp \
$${PWD}/settings.cpp \
$${PWD}/suppressions.cpp \
$${PWD}/symboldatabase.cpp \
diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp
index 6b0e2d8ed..753a5c846 100644
--- a/lib/programmemory.cpp
+++ b/lib/programmemory.cpp
@@ -204,6 +204,8 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok
if (p.first != tok2->next()->varId())
continue;
const Token *vartok = tok2->tokAt(3);
+ if (vartok == tok)
+ continue;
pm.setValue(vartok->varId(), p.second);
setvar = true;
}
diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp
new file mode 100644
index 000000000..dfe4fe290
--- /dev/null
+++ b/lib/reverseanalyzer.cpp
@@ -0,0 +1,286 @@
+#include "reverseanalyzer.h"
+#include "analyzer.h"
+#include "astutils.h"
+#include "forwardanalyzer.h"
+#include "settings.h"
+#include "symboldatabase.h"
+#include "token.h"
+#include "valueptr.h"
+
+#include
+#include
+
+struct ReverseTraversal {
+ ReverseTraversal(const ValuePtr& analyzer, const Settings* settings)
+ : analyzer(analyzer), settings(settings)
+ {}
+ ValuePtr analyzer;
+ const Settings* settings;
+
+ std::pair evalCond(const Token* tok)
+ {
+ std::vector result = analyzer->evaluate(tok);
+ // TODO: We should convert to bool
+ bool checkThen = std::any_of(result.begin(), result.end(), [](int x) { return x == 1; });
+ bool checkElse = std::any_of(result.begin(), result.end(), [](int x) { return x == 0; });
+ return std::make_pair(checkThen, checkElse);
+ }
+
+ bool update(Token* tok)
+ {
+ Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Reverse);
+ if (!action.isNone())
+ analyzer->update(tok, action, Analyzer::Direction::Reverse);
+ if (action.isInconclusive() && !analyzer->lowerToInconclusive())
+ return false;
+ if (action.isInvalid())
+ return false;
+ return true;
+ }
+
+ bool updateRecursive(Token* start)
+ {
+ bool continueB = true;
+ visitAstNodes(start, [&](Token* tok) {
+ continueB &= update(tok);
+ if (continueB)
+ return ChildrenToVisit::op1_and_op2;
+ else
+ return ChildrenToVisit::done;
+ });
+ return continueB;
+ }
+
+ Analyzer::Action analyzeRecursive(const Token* start)
+ {
+ Analyzer::Action result = Analyzer::Action::None;
+ visitAstNodes(start, [&](const Token* tok) {
+ result |= analyzer->analyze(tok, Analyzer::Direction::Reverse);
+ if (result.isModified())
+ return ChildrenToVisit::done;
+ return ChildrenToVisit::op1_and_op2;
+ });
+ return result;
+ }
+
+ Analyzer::Action analyzeRange(const Token* start, const Token* end)
+ {
+ Analyzer::Action result = Analyzer::Action::None;
+ for (const Token* tok = start; tok && tok != end; tok = tok->next()) {
+ Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Reverse);
+ if (action.isModified())
+ return action;
+ result |= action;
+ }
+ return result;
+ }
+
+ Token* isDeadCode(Token* tok)
+ {
+ int opSide = 0;
+ for (; tok && tok->astParent(); tok = tok->astParent()) {
+ Token* parent = tok->astParent();
+ if (tok != parent->astOperand2())
+ continue;
+ if (Token::simpleMatch(parent, ":")) {
+ if (astIsLHS(tok))
+ opSide = 1;
+ else if (astIsRHS(tok))
+ opSide = 2;
+ else
+ opSide = 0;
+ }
+ if (!Token::Match(parent, "%oror%|&&|?"))
+ continue;
+ Token* condTok = parent->astOperand1();
+ if (!condTok)
+ continue;
+ bool checkThen, checkElse;
+ std::tie(checkThen, checkElse) = evalCond(condTok);
+
+ if (!checkThen && !checkElse) {
+ Analyzer::Action action = analyzeRecursive(condTok);
+ if (action.isRead() || action.isModified())
+ return parent;
+ }
+
+ if (parent->str() == "?") {
+ if (!checkElse && opSide == 1)
+ return parent;
+ if (!checkThen && opSide == 2)
+ return parent;
+ }
+ if (!checkThen && parent->str() == "&&")
+ return parent;
+ if (!checkElse && parent->str() == "||")
+ return parent;
+ }
+ return nullptr;
+ }
+
+ void traverse(Token* start)
+ {
+ for (Token* tok = start->previous(); tok; tok = tok->previous()) {
+ if (tok == start || (tok->str() == "{" && (tok->scope()->type == Scope::ScopeType::eFunction ||
+ tok->scope()->type == Scope::ScopeType::eLambda))) {
+ break;
+ }
+ if (Token::Match(tok, "return|break|continue"))
+ break;
+ // Evaluate LHS of assignment before RHS
+ if (Token* assignTok = assignExpr(tok)) {
+ Token* assignTop = assignTok;
+ bool continueB = true;
+ while (assignTop->isAssignmentOp()) {
+ if (!Token::Match(assignTop->astOperand1(), "%assign%")) {
+ continueB &= updateRecursive(assignTop->astOperand1());
+ }
+ if (!assignTop->astParent())
+ break;
+ assignTop = assignTop->astParent();
+ }
+ // Is assignment in dead code
+ if (Token* parent = isDeadCode(assignTok)) {
+ tok = parent;
+ continue;
+ }
+ // Simple assign
+ if (assignTok->astParent() == assignTop || assignTok == assignTop) {
+ Analyzer::Action rhsAction =
+ analyzer->analyze(assignTok->astOperand2(), Analyzer::Direction::Reverse);
+ Analyzer::Action lhsAction =
+ analyzer->analyze(assignTok->astOperand1(), Analyzer::Direction::Reverse);
+ // Assignment from
+ if (rhsAction.isRead()) {
+ const std::string info = "Assignment from '" + assignTok->expressionString() + "'";
+ ValuePtr a = analyzer->reanalyze(assignTok->astOperand1(), info);
+ if (a) {
+ valueFlowGenericForward(nextAfterAstRightmostLeaf(assignTok->astOperand2()),
+ assignTok->astOperand2()->scope()->bodyEnd,
+ a,
+ settings);
+ }
+ // Assignment to
+ } else if (lhsAction.matches()) {
+ const std::string info = "Assignment to '" + assignTok->expressionString() + "'";
+ ValuePtr a = analyzer->reanalyze(assignTok->astOperand2(), info);
+ if (a) {
+ valueFlowGenericForward(nextAfterAstRightmostLeaf(assignTok->astOperand2()),
+ assignTok->astOperand2()->scope()->bodyEnd,
+ a,
+ settings);
+ valueFlowGenericReverse(assignTok->astOperand1()->previous(), a, settings);
+ }
+ }
+ }
+ if (!continueB)
+ break;
+ valueFlowGenericForward(assignTop->astOperand2(), analyzer, settings);
+ tok = previousBeforeAstLeftmostLeaf(assignTop);
+ continue;
+ }
+ if (tok->str() == "}") {
+ Token* condTok = getCondTokFromEnd(tok);
+ if (!condTok)
+ break;
+ Analyzer::Action condAction = analyzeRecursive(condTok);
+ const bool inLoop = condTok->astTop() && Token::Match(condTok->astTop()->previous(), "for|while (");
+ // Evaluate condition of for and while loops first
+ if (inLoop) {
+ if (condAction.isModified())
+ break;
+ valueFlowGenericForward(condTok, analyzer, settings);
+ }
+ Token* thenEnd = nullptr;
+ Token* elseEnd = nullptr;
+ const bool hasElse = Token::simpleMatch(tok->link()->tokAt(-2), "} else {");
+ if (hasElse) {
+ elseEnd = tok;
+ thenEnd = tok->link()->tokAt(-2);
+ } else {
+ thenEnd = tok;
+ }
+
+ Analyzer::Action thenAction = analyzeRange(thenEnd->link(), thenEnd);
+ Analyzer::Action elseAction = Analyzer::Action::None;
+ if (hasElse) {
+ elseAction = analyzeRange(tok->link(), tok);
+ }
+ if (thenAction.isModified() && inLoop)
+ break;
+ else if (thenAction.isModified() && !elseAction.isModified())
+ analyzer->assume(condTok, hasElse, condTok);
+ else if (elseAction.isModified() && !thenAction.isModified())
+ analyzer->assume(condTok, !hasElse, condTok);
+ // Bail if one of the branches are read to avoid FPs due to over constraints
+ else if (thenAction.isIdempotent() || elseAction.isIdempotent() || thenAction.isRead() ||
+ elseAction.isRead())
+ break;
+ if (thenAction.isInvalid() || elseAction.isInvalid())
+ break;
+
+ if (!thenAction.isModified() && !elseAction.isModified())
+ valueFlowGenericForward(condTok, analyzer, settings);
+ else if (condAction.isRead())
+ break;
+ // If the condition modifies the variable then bail
+ if (condAction.isModified())
+ break;
+ tok = condTok->astTop()->previous();
+ continue;
+ }
+ if (tok->str() == "{") {
+ if (tok->previous() &&
+ (Token::simpleMatch(tok->previous(), "do") ||
+ (tok->strAt(-1) == ")" && Token::Match(tok->linkAt(-1)->previous(), "for|while (")))) {
+ Analyzer::Action action = analyzeRange(tok, tok->link());
+ if (action.isModified())
+ break;
+ }
+ if (Token::simpleMatch(tok->tokAt(-2), "} else {"))
+ tok = tok->linkAt(-2);
+ if (Token::simpleMatch(tok->previous(), ") {"))
+ tok = tok->previous()->link();
+ continue;
+ }
+ if (Token* next = isUnevaluated(tok)) {
+ tok = next;
+ continue;
+ }
+ if (Token* parent = isDeadCode(tok)) {
+ tok = parent;
+ continue;
+ }
+ if (!update(tok))
+ break;
+ }
+ }
+
+ static Token* assignExpr(Token* tok)
+ {
+ while (tok->astParent() && (astIsRHS(tok) || !tok->astParent()->isBinaryOp())) {
+ if (tok->astParent()->isAssignmentOp())
+ return tok->astParent();
+ tok = tok->astParent();
+ }
+ return nullptr;
+ }
+
+ static Token* isUnevaluated(Token* tok)
+ {
+ if (Token::Match(tok, ")|>") && tok->link()) {
+ Token* start = tok->link();
+ if (Token::Match(start->previous(), "sizeof|decltype ("))
+ return start->previous();
+ if (Token::simpleMatch(start, "<"))
+ return start;
+ }
+ return nullptr;
+ }
+};
+
+void valueFlowGenericReverse(Token* start, const ValuePtr& a, const Settings* settings)
+{
+ ReverseTraversal rt{a, settings};
+ rt.traverse(start);
+}
diff --git a/lib/reverseanalyzer.h b/lib/reverseanalyzer.h
new file mode 100644
index 000000000..82a258148
--- /dev/null
+++ b/lib/reverseanalyzer.h
@@ -0,0 +1,30 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2020 Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#ifndef reverseanalyzerH
+#define reverseanalyzerH
+
+struct Analyzer;
+class Settings;
+class Token;
+template
+class ValuePtr;
+
+void valueFlowGenericReverse(Token* start, const ValuePtr& a, const Settings* settings);
+
+#endif
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 50a7c917c..b57316726 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -77,6 +77,7 @@
#include "valueflow.h"
+#include "analyzer.h"
#include "astutils.h"
#include "errorlogger.h"
#include "forwardanalyzer.h"
@@ -85,6 +86,7 @@
#include "path.h"
#include "platform.h"
#include "programmemory.h"
+#include "reverseanalyzer.h"
#include "settings.h"
#include "standards.h"
#include "symboldatabase.h"
@@ -1738,12 +1740,12 @@ static void valueFlowGlobalStaticVar(TokenList *tokenList, const Settings *setti
}
}
-static ForwardAnalyzer::Action valueFlowForwardVariable(Token* const startToken,
- const Token* const endToken,
- const Variable* const var,
- std::list values,
- TokenList* const tokenlist,
- const Settings* const settings);
+static Analyzer::Action valueFlowForwardVariable(Token* const startToken,
+ const Token* const endToken,
+ const Variable* const var,
+ std::list values,
+ TokenList* const tokenlist,
+ const Settings* const settings);
// Old deprecated version
static void valueFlowForward(Token* startToken,
@@ -1756,279 +1758,13 @@ static void valueFlowForward(Token* startToken,
ErrorLogger* const errorLogger,
const Settings* settings);
-static void valueFlowReverse(TokenList *tokenlist,
- Token *tok,
- const Token * const varToken,
+static void valueFlowReverse(TokenList* tokenlist,
+ Token* tok,
+ const Token* const varToken,
ValueFlow::Value val,
ValueFlow::Value val2,
- ErrorLogger *errorLogger,
- const Settings *settings)
-{
- const MathLib::bigint num = val.intvalue;
- const Variable * const var = varToken->variable();
- if (!var)
- return;
-
- const int varid = varToken->varId();
- const Token * const startToken = var->nameToken();
-
- for (Token *tok2 = tok->previous(); ; tok2 = tok2->previous()) {
- if (!tok2 || tok2 == startToken ||
- (tok2->str() == "{" &&
- (tok2->scope()->type == Scope::ScopeType::eFunction || tok2->scope()->type == Scope::ScopeType::eLambda))) {
- break;
- }
-
- if (tok2->varId() == varid) {
- if (tok2->hasKnownValue())
- break;
- // bailout: assignment
- if (Token::Match(tok2->previous(), "!!* %name% =")) {
- Token* assignTok = const_cast(tok2->next()->astOperand2());
- if (!assignTok->hasKnownValue()) {
- setTokenValue(assignTok, val, settings);
- const std::string info = "Assignment from '" + assignTok->expressionString() + "'";
- val.errorPath.emplace_back(assignTok, info);
- std::list values = {val};
- if (val2.condition) {
- val2.errorPath.emplace_back(assignTok, info);
- setTokenValue(assignTok, val2, settings);
- values.push_back(val2);
- }
- const Token* startForwardToken = nextAfterAstRightmostLeaf(tok2->next());
- const Token* endForwardToken = tok->scope() ? tok->scope()->bodyEnd : tok;
- valueFlowForward(const_cast(startForwardToken),
- endForwardToken,
- assignTok,
- values,
- false,
- false,
- tokenlist,
- errorLogger,
- settings);
- // Only reverse analysis supported with variables
- if (assignTok->varId() > 0)
- valueFlowReverse(tokenlist, tok2->previous(), assignTok, val, val2, errorLogger, settings);
- }
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "assignment of " + tok2->str());
- break;
- }
-
- // increment/decrement
- int inc = 0;
- if (Token::Match(tok2->previous(), "[;{}] %name% ++|-- ;"))
- inc = (tok2->strAt(1)=="++") ? -1 : 1;
- else if (Token::Match(tok2->tokAt(-2), "[;{}] ++|-- %name% ;"))
- inc = (tok2->strAt(-1)=="++") ? -1 : 1;
- else if (Token::Match(tok2->previous(), "++|-- %name%") || Token::Match(tok2, "%name% ++|--")) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "increment/decrement of " + tok2->str());
- break;
- }
- if (inc != 0) {
- val.intvalue += inc;
- const std::string info(tok2->str() + " is " + std::string(inc==1 ? "decremented" : "incremented") + ", before this " + (inc==1?"decrement":"increment") + " the value is " + val.infoString());
- val.errorPath.emplace_back(tok2, info);
- }
-
- // compound assignment
- if (Token::Match(tok2->previous(), "[;{}] %var% %assign%") && tok2->next()->str() != "=") {
- const Token * const assignToken = tok2->next();
- const Token * const rhsToken = assignToken->astOperand2();
- if (!rhsToken || !rhsToken->hasKnownIntValue()) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "compound assignment, rhs value is not known");
- break;
- }
- const MathLib::bigint rhsValue = rhsToken->values().front().intvalue;
- if (assignToken->str() == "+=")
- val.intvalue -= rhsValue;
- else if (assignToken->str() == "-=")
- val.intvalue += rhsValue;
- else if (assignToken->str() == "*=" && rhsValue != 0)
- val.intvalue /= rhsValue;
- else {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "compound assignment " + tok2->str());
- break;
- }
-
- const std::string info("Compound assignment '" + assignToken->str() + "', before assignment value is " + val.infoString());
- val.errorPath.emplace_back(tok2, info);
- }
-
- // bailout: variable is used in rhs in assignment to itself
- if (bailoutSelfAssignment(tok2)) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "variable " + tok2->str() + " is used in rhs in assignment to itself");
- break;
- }
-
- if (Token::Match(tok2->previous(), "sizeof|.")) {
- const Token *prev = tok2->previous();
- while (Token::Match(prev,"%name%|.") && prev->str() != "sizeof")
- prev = prev->previous();
- if (prev && prev->str() == "sizeof")
- continue;
- }
-
- // assigned by subfunction?
- bool inconclusive = false;
- if (isVariableChangedByFunctionCall(tok2, std::max(val.indirect, val2.indirect), settings, &inconclusive)) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction");
- break;
- }
- // Impossible values can't be inconclusive
- if (val.isImpossible() || val2.isImpossible())
- break;
- val.setInconclusive(inconclusive);
- val2.setInconclusive(inconclusive);
-
- // skip if variable is conditionally used in ?: expression
- if (const Token *parent = skipValueInConditionalExpression(tok2)) {
- if (settings->debugwarnings)
- bailout(tokenlist,
- errorLogger,
- tok2,
- "no simplification of " + tok2->str() + " within " + (Token::Match(parent,"[?:]") ? "?:" : parent->str()) + " expression");
- continue;
- }
-
- // do-while condition, break in the loop body
- {
- const Token *parent = tok2->astParent();
- while (parent && !Token::simpleMatch(parent->previous(), "while ("))
- parent = parent->astParent();
- if (parent && Token::simpleMatch(parent->tokAt(-2), "} while (") && Token::simpleMatch(parent->linkAt(-2)->previous(), "do {")) {
- bool breakBailout = false;
- for (const Token *iftok = parent->linkAt(-2); iftok != parent; iftok = iftok->next()) {
- if (!Token::simpleMatch(iftok, "if ("))
- continue;
- if (!Token::simpleMatch(iftok->linkAt(1), ") { break"))
- continue;
- ProgramMemory programMemory;
- programMemory.setIntValue(varid, num);
- if (conditionIsTrue(iftok->next()->astOperand2(), programMemory)) {
- breakBailout = true;
- break;
- }
- }
- if (breakBailout) {
- if (settings->debugwarnings)
- bailout(tokenlist,
- errorLogger,
- tok2,
- "no simplification of " + tok2->str() + " in do-while condition since there is a break in the loop body");
- break;
- }
- }
- }
-
- setTokenValue(tok2, val, settings);
- if (val2.condition)
- setTokenValue(tok2,val2, settings);
- if (tok2 == var->nameToken())
- break;
- }
-
- // skip sizeof etc..
- if (tok2->str() == ")" && Token::Match(tok2->link()->previous(), "sizeof|typeof|typeid ("))
- tok2 = tok2->link();
-
- // goto label
- if (Token::Match(tok2, "[;{}] %name% :")) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2->next(), "variable " + var->name() + " stopping on goto label");
- break;
- }
-
- if (tok2->str() == "}") {
- const Token* condTok = getCondTokFromEnd(tok2);
- // Evaluate condition of for and while loops first
- if (condTok && condTok->astTop() && Token::Match(condTok->astTop()->previous(), "for|while (")) {
- const Token* startTok = nullptr;
- const Token* endTok = nullptr;
- std::tie(startTok, endTok) = condTok->findExpressionStartEndTokens();
- if (!isVariableChanged(startTok, endTok, varid, false, settings, true)) {
- std::list values = {val};
- if (val2.condition) {
- values.push_back(val2);
- }
- const Token *expr = Token::findmatch(tok2, "%varid%", varid);
- valueFlowForward(const_cast(startTok),
- endTok,
- expr,
- values,
- false,
- false,
- tokenlist,
- errorLogger,
- settings);
- }
- }
- const Token *vartok = Token::findmatch(tok2->link(), "%varid%", tok2, varid);
- while (Token::Match(vartok, "%name% = %num% ;") && !vartok->tokAt(2)->getValue(num))
- vartok = Token::findmatch(vartok->next(), "%varid%", tok2, varid);
- if (vartok) {
- if (settings->debugwarnings) {
- std::string errmsg = "variable ";
- errmsg += var->name() + " ";
- errmsg += "stopping on }";
- bailout(tokenlist, errorLogger, tok2, errmsg);
- }
- break;
- } else {
- tok2 = tok2->link();
- if (Token::simpleMatch(tok2->previous(), ") {") && Token::Match(tok2->previous()->link()->previous(), "for|while ("))
- tok2 = tok2->previous()->link();
- }
- } else if (tok2->str() == "{") {
- // if variable is assigned in loop don't look before the loop
- if (tok2->previous() &&
- (Token::simpleMatch(tok2->previous(), "do") ||
- (tok2->strAt(-1) == ")" && Token::Match(tok2->linkAt(-1)->previous(), "for|while (")))) {
-
- const Token *start = tok2;
- const Token *end = start->link();
- if (isVariableChanged(start,end,varid,var->isGlobal(),settings, tokenlist->isCPP())) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in loop. so valueflow analysis bailout when start of loop is reached.");
- break;
- }
- }
-
- // Global variable : stop when leaving the function scope
- if (!var->isLocal()) {
- if (!Token::Match(tok2->previous(), ")|else|do {"))
- break;
- if ((tok2->previous()->str() == ")") &&
- !Token::Match(tok2->linkAt(-1)->previous(), "if|for|while ("))
- break;
- }
- } else if (tok2->str() == ";") {
- const Token *parent = tok2->previous();
- while (parent && !Token::Match(parent, "return|break|continue|goto"))
- parent = parent->astParent();
- // reaching a break/continue/return
- if (parent) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " stopping on " + parent->str());
- break;
- }
- }
-
- if (Token::Match(tok2, "%name% (") && !Token::simpleMatch(tok2->linkAt(1), ") {")) {
- // bailout: global non-const variables
- if (!(var->isLocal() || var->isArgument()) && !var->isConst()) {
- if (settings->debugwarnings)
- bailout(tokenlist, errorLogger, tok, "global variable " + var->name());
- return;
- }
- }
- }
-}
+ ErrorLogger* errorLogger,
+ const Settings* settings);
static bool isConditionKnown(const Token* tok, bool then)
{
@@ -2169,6 +1905,18 @@ static void valueFlowAST(Token *tok, nonneg int varid, const ValueFlow::Value &v
valueFlowAST(tok->astOperand2(), varid, value, settings);
}
+static const std::string& invertAssign(const std::string& assign)
+{
+ static std::unordered_map lookup = {
+ {"+=", "-="}, {"-=", "+="}, {"*=", "/="}, {"/=", "*="}, {"<<=", ">>="}, {">>=", "<<="}, {"^=", "^="}};
+ static std::string empty = "";
+ auto it = lookup.find(assign);
+ if (it == lookup.end())
+ return empty;
+ else
+ return it->second;
+}
+
static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign, const ValueFlow::Value &rhsValue)
{
if (lhsValue.isIntValue()) {
@@ -2309,17 +2057,13 @@ struct SelectMapValues {
}
};
-struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
+struct ValueFlowAnalyzer : Analyzer {
const TokenList* tokenlist;
ProgramMemoryState pms;
- ValueFlowForwardAnalyzer()
- : tokenlist(nullptr), pms()
- {}
+ ValueFlowAnalyzer() : tokenlist(nullptr), pms() {}
- ValueFlowForwardAnalyzer(const TokenList* t)
- : tokenlist(t), pms()
- {}
+ ValueFlowAnalyzer(const TokenList* t) : tokenlist(t), pms() {}
virtual const ValueFlow::Value* getValue(const Token* tok) const = 0;
virtual ValueFlow::Value* getValue(const Token* tok) = 0;
@@ -2372,6 +2116,14 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
if (isVariableChanged(tok, getIndirect(tok), getSettings(), isCPP())) {
if (Token::Match(tok->astParent(), "*|[|.|++|--"))
return read | Action::Invalid;
+ const ValueFlow::Value* value = getValue(tok);
+ // Check if its assigned to the same value
+ if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) &&
+ astIsIntegral(tok->astParent()->astOperand2(), false)) {
+ std::vector result = evaluate(tok->astParent()->astOperand2());
+ if (!result.empty() && value->equalTo(result.front()))
+ return Action::Idempotent;
+ }
return Action::Invalid;
}
return read;
@@ -2390,7 +2142,8 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
return Action::None;
}
- virtual Action isWritable(const Token* tok) const {
+ virtual Action isWritable(const Token* tok, Direction d) const
+ {
const ValueFlow::Value* value = getValue(tok);
if (!value)
return Action::None;
@@ -2400,6 +2153,9 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
if (parent && parent->isAssignmentOp() && astIsLHS(tok) &&
parent->astOperand2()->hasKnownValue()) {
+ // If the operator is invertible
+ if (d == Direction::Reverse && (parent->str() == "&=" || parent->str() == "|=" || parent->str() == "%="))
+ return Action::None;
const Token* rhs = parent->astOperand2();
const ValueFlow::Value* rhsValue = getKnownValue(rhs, ValueFlow::Value::ValueType::INT);
Action a;
@@ -2407,8 +2163,12 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
a = Action::Invalid;
else
a = Action::Write;
- if (parent->str() != "=")
+ if (parent->str() != "=") {
a |= Action::Read;
+ } else {
+ if (rhsValue && !value->isImpossible() && value->equalValue(*rhsValue))
+ a = Action::Idempotent;
+ }
return a;
}
@@ -2419,7 +2179,16 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
return Action::None;
}
- virtual void writeValue(ValueFlow::Value* value, const Token* tok) const {
+ static const std::string& getAssign(const Token* tok, Direction d)
+ {
+ if (d == Direction::Forward)
+ return tok->str();
+ else
+ return invertAssign(tok->str());
+ }
+
+ virtual void writeValue(ValueFlow::Value* value, const Token* tok, Direction d) const
+ {
if (!value)
return;
if (!tok->astParent())
@@ -2427,7 +2196,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
if (tok->astParent()->isAssignmentOp()) {
// TODO: Check result
if (evalAssignment(*value,
- tok->astParent()->str(),
+ getAssign(tok->astParent(), d),
*getKnownValue(tok->astParent()->astOperand2(), ValueFlow::Value::ValueType::INT))) {
const std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " +
value->infoString());
@@ -2439,30 +2208,33 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
value->intvalue = 0;
}
} else if (tok->astParent()->tokType() == Token::eIncDecOp) {
- const bool inc = tok->astParent()->str() == "++";
+ bool inc = tok->astParent()->str() == "++";
+ std::string opName(inc ? "incremented" : "decremented");
+ if (d == Direction::Reverse)
+ inc = !inc;
value->intvalue += (inc ? 1 : -1);
- const std::string info(tok->str() + " is " + std::string(inc ? "incremented" : "decremented") +
- "', new value is " + value->infoString());
+ const std::string info(tok->str() + " is " + opName + "', new value is " + value->infoString());
value->errorPath.emplace_back(tok, info);
}
}
- virtual Action analyze(const Token* tok) const OVERRIDE {
+ virtual Action analyze(const Token* tok, Direction d) const OVERRIDE
+ {
if (invalid())
return Action::Invalid;
bool inconclusive = false;
if (match(tok)) {
const Token* parent = tok->astParent();
if (astIsPointer(tok) && (Token::Match(parent, "*|[") || (parent && parent->originalName() == "->")) && getIndirect(tok) <= 0)
- return Action::Read;
+ return Action::Read | Action::Match;
// Action read = Action::Read;
- Action w = isWritable(tok);
+ Action w = isWritable(tok, d);
if (w != Action::None)
- return w;
+ return w | Action::Match;
// Check for modifications by function calls
- return isModified(tok);
+ return isModified(tok) | Action::Match;
} else if (tok->isUnaryOp("*")) {
const Token* lifeTok = nullptr;
for (const ValueFlow::Value& v:tok->astOperand1()->values()) {
@@ -2502,10 +2274,19 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
return {static_cast(tok->values().front().intvalue)};
std::vector result;
ProgramMemory pm = pms.get(tok, getProgramState());
- if (conditionIsTrue(tok, pm))
- result.push_back(1);
- if (conditionIsFalse(tok, pm))
- result.push_back(0);
+ if (Token::Match(tok, "&&|%oror%")) {
+ if (conditionIsTrue(tok, pm))
+ result.push_back(1);
+ if (conditionIsFalse(tok, pm))
+ result.push_back(0);
+ } else {
+ MathLib::bigint out = 0;
+ bool error = false;
+ execute(tok, &pm, &out, &error);
+ if (!error)
+ result.push_back(out);
+ }
+
return result;
}
@@ -2525,32 +2306,37 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
makeConditional();
}
- virtual void update(Token* tok, Action a) OVERRIDE {
+ virtual void update(Token* tok, Action a, Direction d) OVERRIDE
+ {
ValueFlow::Value* value = getValue(tok);
if (!value)
return;
- if (a.isRead())
+ // Read first when moving forward
+ if (d == Direction::Forward && a.isRead())
setTokenValue(tok, *value, getSettings());
if (a.isInconclusive())
lowerToInconclusive();
if (a.isWrite() && tok->astParent()) {
- writeValue(value, tok);
+ writeValue(value, tok, d);
}
+ // Read last when moving in reverse
+ if (d == Direction::Reverse && a.isRead())
+ setTokenValue(tok, *value, getSettings());
}
+
+ virtual ValuePtr reanalyze(Token*, const std::string&) const OVERRIDE { return {}; }
};
-struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
+ValuePtr makeAnalyzer(Token* exprTok, const ValueFlow::Value& value, const TokenList* tokenlist);
+
+struct SingleValueFlowAnalyzer : ValueFlowAnalyzer {
std::unordered_map varids;
std::unordered_map aliases;
ValueFlow::Value value;
- SingleValueFlowForwardAnalyzer()
- : ValueFlowForwardAnalyzer()
- {}
+ SingleValueFlowAnalyzer() : ValueFlowAnalyzer() {}
- SingleValueFlowForwardAnalyzer(const ValueFlow::Value& v, const TokenList* t)
- : ValueFlowForwardAnalyzer(t), value(v)
- {}
+ SingleValueFlowAnalyzer(const ValueFlow::Value& v, const TokenList* t) : ValueFlowAnalyzer(t), value(v) {}
const std::unordered_map& getVars() const {
return varids;
@@ -2645,17 +2431,26 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
return false;
}
+
+ virtual ValuePtr reanalyze(Token* tok, const std::string& msg) const OVERRIDE
+ {
+ ValueFlow::Value newValue = value;
+ newValue.errorPath.emplace_back(tok, msg);
+ return makeAnalyzer(tok, newValue, tokenlist);
+ }
};
-struct VariableForwardAnalyzer : SingleValueFlowForwardAnalyzer {
+struct VariableAnalyzer : SingleValueFlowAnalyzer {
const Variable* var;
- VariableForwardAnalyzer()
- : SingleValueFlowForwardAnalyzer(), var(nullptr)
- {}
+ VariableAnalyzer() : SingleValueFlowAnalyzer(), var(nullptr) {}
- VariableForwardAnalyzer(const Variable* v, const ValueFlow::Value& val, std::vector paliases, const TokenList* t)
- : SingleValueFlowForwardAnalyzer(val, t), var(v) {
+ VariableAnalyzer(const Variable* v,
+ const ValueFlow::Value& val,
+ std::vector paliases,
+ const TokenList* t)
+ : SingleValueFlowAnalyzer(val, t), var(v)
+ {
varids[var->declarationId()] = var;
for (const Variable* av:paliases) {
if (!av)
@@ -2704,28 +2499,28 @@ static std::vector getAliasesFromValues(std::list values,
- std::vector aliases,
- TokenList* const tokenlist,
- const Settings* const settings)
+static Analyzer::Action valueFlowForwardVariable(Token* const startToken,
+ const Token* const endToken,
+ const Variable* const var,
+ std::list values,
+ std::vector aliases,
+ TokenList* const tokenlist,
+ const Settings* const settings)
{
- ForwardAnalyzer::Action actions;
+ Analyzer::Action actions;
for (ValueFlow::Value& v : values) {
- VariableForwardAnalyzer a(var, v, aliases, tokenlist);
+ VariableAnalyzer a(var, v, aliases, tokenlist);
actions |= valueFlowGenericForward(startToken, endToken, a, settings);
}
return actions;
}
-static ForwardAnalyzer::Action valueFlowForwardVariable(Token* const startToken,
- const Token* const endToken,
- const Variable* const var,
- std::list values,
- TokenList* const tokenlist,
- const Settings* const settings)
+static Analyzer::Action valueFlowForwardVariable(Token* const startToken,
+ const Token* const endToken,
+ const Variable* const var,
+ std::list values,
+ TokenList* const tokenlist,
+ const Settings* const settings)
{
return valueFlowForwardVariable(
startToken, endToken, var, std::move(values), getAliasesFromValues(values), tokenlist, settings);
@@ -2747,17 +2542,16 @@ static bool valueFlowForwardVariable(Token* const startToken,
return true;
}
-struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer {
+struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
const Token* expr;
bool local;
bool unknown;
- ExpressionForwardAnalyzer()
- : SingleValueFlowForwardAnalyzer(), expr(nullptr), local(true), unknown(false)
- {}
+ ExpressionAnalyzer() : SingleValueFlowAnalyzer(), expr(nullptr), local(true), unknown(false) {}
- ExpressionForwardAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t)
- : SingleValueFlowForwardAnalyzer(val, t), expr(e), local(true), unknown(false) {
+ ExpressionAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t)
+ : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false)
+ {
setupExprVarIds();
}
@@ -2815,16 +2609,16 @@ struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer {
}
};
-static ForwardAnalyzer::Action valueFlowForwardExpression(Token* startToken,
- const Token* endToken,
- const Token* exprTok,
- const std::list& values,
- const TokenList* const tokenlist,
- const Settings* settings)
+static Analyzer::Action valueFlowForwardExpression(Token* startToken,
+ const Token* endToken,
+ const Token* exprTok,
+ const std::list& values,
+ const TokenList* const tokenlist,
+ const Settings* settings)
{
- ForwardAnalyzer::Action actions;
+ Analyzer::Action actions;
for (const ValueFlow::Value& v : values) {
- ExpressionForwardAnalyzer a(exprTok, v, tokenlist);
+ ExpressionAnalyzer a(exprTok, v, tokenlist);
actions |= valueFlowGenericForward(startToken, endToken, a, settings);
}
return actions;
@@ -2891,12 +2685,23 @@ static const Token* solveExprValues(const Token* expr, std::list values,
- TokenList* const tokenlist,
- const Settings* settings)
+ValuePtr makeAnalyzer(Token* exprTok, const ValueFlow::Value& value, const TokenList* tokenlist)
+{
+ std::list values = {value};
+ const Token* expr = solveExprValues(exprTok, values);
+ if (expr->variable()) {
+ return VariableAnalyzer(expr->variable(), value, getAliasesFromValues(values), tokenlist);
+ } else {
+ return ExpressionAnalyzer(expr, value, tokenlist);
+ }
+}
+
+static Analyzer::Action valueFlowForward(Token* startToken,
+ const Token* endToken,
+ const Token* exprTok,
+ std::list values,
+ TokenList* const tokenlist,
+ const Settings* settings)
{
const Token* expr = solveExprValues(exprTok, values);
if (expr->variable()) {
@@ -2920,6 +2725,25 @@ static void valueFlowForward(Token* startToken,
valueFlowForward(startToken, endToken, exprTok, std::move(values), tokenlist, settings);
}
+static void valueFlowReverse(TokenList* tokenlist,
+ Token* tok,
+ const Token* const varToken,
+ ValueFlow::Value val,
+ ValueFlow::Value val2,
+ ErrorLogger* errorLogger,
+ const Settings* settings)
+{
+ std::list values = {val};
+ if (val2.varId != 0)
+ values.push_back(val2);
+ const Variable* var = varToken->variable();
+ auto aliases = getAliasesFromValues(values);
+ for (ValueFlow::Value& v : values) {
+ VariableAnalyzer a(var, v, aliases, tokenlist);
+ valueFlowGenericReverse(tok, a, settings);
+ }
+}
+
static int getArgumentPos(const Variable *var, const Function *f)
{
auto arg_it = std::find_if(f->argumentList.begin(), f->argumentList.end(), [&](const Variable &v) {
@@ -5092,16 +4916,15 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas
}
}
-struct MultiValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
+struct MultiValueFlowAnalyzer : ValueFlowAnalyzer {
std::unordered_map values;
std::unordered_map vars;
- MultiValueFlowForwardAnalyzer()
- : ValueFlowForwardAnalyzer(), values(), vars()
- {}
+ MultiValueFlowAnalyzer() : ValueFlowAnalyzer(), values(), vars() {}
- MultiValueFlowForwardAnalyzer(const std::unordered_map& args, const TokenList* t)
- : ValueFlowForwardAnalyzer(t), values(), vars() {
+ MultiValueFlowAnalyzer(const std::unordered_map& args, const TokenList* t)
+ : ValueFlowAnalyzer(t), values(), vars()
+ {
for (const auto& p:args) {
values[p.first->declarationId()] = p.second;
vars[p.first->declarationId()] = p.first;
@@ -5285,7 +5108,7 @@ static void valueFlowInjectParameter(TokenList* tokenlist, ErrorLogger* errorLog
}
if (skip)
continue;
- MultiValueFlowForwardAnalyzer a(arg, tokenlist);
+ MultiValueFlowAnalyzer a(arg, tokenlist);
valueFlowGenericForward(const_cast(functionScope->bodyStart), functionScope->bodyEnd, a, settings);
}
}
@@ -5937,21 +5760,26 @@ static void valueFlowContainerReverse(Token *tok, nonneg int containerId, const
}
}
-struct ContainerVariableForwardAnalyzer : VariableForwardAnalyzer {
- ContainerVariableForwardAnalyzer()
- : VariableForwardAnalyzer()
- {}
+struct ContainerVariableAnalyzer : VariableAnalyzer {
+ ContainerVariableAnalyzer() : VariableAnalyzer() {}
- ContainerVariableForwardAnalyzer(const Variable* v, const ValueFlow::Value& val, std::vector paliases, const TokenList* t)
- : VariableForwardAnalyzer(v, val, std::move(paliases), t) {}
+ ContainerVariableAnalyzer(const Variable* v,
+ const ValueFlow::Value& val,
+ std::vector paliases,
+ const TokenList* t)
+ : VariableAnalyzer(v, val, std::move(paliases), t)
+ {}
virtual bool match(const Token* tok) const OVERRIDE {
return tok->varId() == var->declarationId() || (astIsIterator(tok) && isAliasOf(tok, var->declarationId()));
}
- virtual Action isWritable(const Token* tok) const OVERRIDE {
+ virtual Action isWritable(const Token* tok, Direction d) const OVERRIDE
+ {
if (astIsIterator(tok))
return Action::None;
+ if (d == Direction::Reverse)
+ return Action::None;
const ValueFlow::Value* value = getValue(tok);
if (!value)
return Action::None;
@@ -5977,7 +5805,10 @@ struct ContainerVariableForwardAnalyzer : VariableForwardAnalyzer {
return Action::None;
}
- virtual void writeValue(ValueFlow::Value* value, const Token* tok) const OVERRIDE {
+ virtual void writeValue(ValueFlow::Value* value, const Token* tok, Direction d) const OVERRIDE
+ {
+ if (d == Direction::Reverse)
+ return;
if (!value)
return;
if (!tok->astParent())
@@ -6021,19 +5852,19 @@ struct ContainerVariableForwardAnalyzer : VariableForwardAnalyzer {
}
};
-static ForwardAnalyzer::Action valueFlowContainerForward(Token* tok,
- const Token* endToken,
- const Variable* var,
- ValueFlow::Value value,
- TokenList* tokenlist)
+static Analyzer::Action valueFlowContainerForward(Token* tok,
+ const Token* endToken,
+ const Variable* var,
+ ValueFlow::Value value,
+ TokenList* tokenlist)
{
- ContainerVariableForwardAnalyzer a(var, value, getAliasesFromValues({value}), tokenlist);
+ ContainerVariableAnalyzer a(var, value, getAliasesFromValues({value}), tokenlist);
return valueFlowGenericForward(tok, endToken, a, tokenlist->getSettings());
}
-static ForwardAnalyzer::Action valueFlowContainerForward(Token* tok,
- const Variable* var,
- ValueFlow::Value value,
- TokenList* tokenlist)
+static Analyzer::Action valueFlowContainerForward(Token* tok,
+ const Variable* var,
+ ValueFlow::Value value,
+ TokenList* tokenlist)
{
const Token * endOfVarScope = nullptr;
if (var->isLocal() || var->isArgument())
diff --git a/lib/valueflow.h b/lib/valueflow.h
index 4c823952b..bcee6de31 100644
--- a/lib/valueflow.h
+++ b/lib/valueflow.h
@@ -25,8 +25,10 @@
#include "mathlib.h"
#include "utils.h"
+#include
#include
#include
+#include
#include
#include
@@ -51,6 +53,14 @@ namespace ValueFlow {
x--;
}
};
+
+ struct equalVisitor {
+ template
+ void operator()(bool& result, T x, U y) const
+ {
+ result = !(x > y || x < y);
+ }
+ };
class CPPCHECKLIB Value {
public:
typedef std::pair ErrorPathItem;
@@ -111,19 +121,20 @@ namespace ValueFlow {
return true;
}
- template
- void visitValue(F f) {
- switch (valueType) {
+ template
+ static void visitValue(T& self, F f)
+ {
+ switch (self.valueType) {
case ValueType::INT:
case ValueType::BUFFER_SIZE:
case ValueType::CONTAINER_SIZE:
case ValueType::ITERATOR_START:
case ValueType::ITERATOR_END: {
- f(intvalue);
+ f(self.intvalue);
break;
}
case ValueType::FLOAT: {
- f(floatValue);
+ f(self.floatValue);
break;
}
case ValueType::UNINIT:
@@ -151,11 +162,19 @@ namespace ValueFlow {
return !(*this == rhs);
}
+ template )>
+ bool equalTo(const T& x) const
+ {
+ bool result = false;
+ visitValue(*this, std::bind(equalVisitor{}, std::ref(result), x, std::placeholders::_1));
+ return result;
+ }
+
void decreaseRange() {
if (bound == Bound::Lower)
- visitValue(increment{});
+ visitValue(*this, increment{});
else if (bound == Bound::Upper)
- visitValue(decrement{});
+ visitValue(*this, decrement{});
}
void invertBound() {
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index 8bfd849a5..c105d8b7b 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -3453,6 +3453,20 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
+ check("struct a {\n"
+ " int *b();\n"
+ "};\n"
+ "bool g(a c, a* d) {\n"
+ " a *v, *e = v = &c;\n"
+ " if (!v)\n"
+ " return true;\n"
+ " int *f = v->b();\n"
+ " if (f)\n"
+ " v = nullptr;\n"
+ " if (v == nullptr && e) {}\n"
+ " return d;\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
}
void alwaysTrueInfer() {
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index 6a4883413..66f75ffdb 100644
--- a/test/testnullpointer.cpp
+++ b/test/testnullpointer.cpp
@@ -101,6 +101,10 @@ private:
TEST_CASE(nullpointer58); // #9807
TEST_CASE(nullpointer59); // #9897
TEST_CASE(nullpointer60); // #9842
+ TEST_CASE(nullpointer61);
+ TEST_CASE(nullpointer62);
+ TEST_CASE(nullpointer63);
+ TEST_CASE(nullpointer64);
TEST_CASE(nullpointer_addressOf); // address of
TEST_CASE(nullpointerSwitch); // #2626
TEST_CASE(nullpointer_cast); // #4692
@@ -692,13 +696,16 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
- check("void foo(int *p)\n"
+ check("void foo(int *p, bool x)\n"
"{\n"
" int var1 = x ? *p : 5;\n"
" if (!p)\n"
" ;\n"
"}");
- ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str());
+ TODO_ASSERT_EQUALS(
+ "[test.cpp:4] -> [test.cpp:3]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n",
+ "",
+ errout.str());
// while
check("void f(int *p) {\n"
@@ -1892,6 +1899,122 @@ private:
ASSERT_EQUALS("", errout.str());
}
+ void nullpointer61()
+ {
+ check("struct a {\n"
+ " int *e;\n"
+ "};\n"
+ "struct f {\n"
+ " a *g() const;\n"
+ "};\n"
+ "void h() {\n"
+ " for (f b;;) {\n"
+ " a *c = b.g();\n"
+ " int *d = c->e;\n"
+ " if (d)\n"
+ " ;\n"
+ " }\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+
+ check("struct A {\n"
+ " A* g() const;\n"
+ " A* h() const;\n"
+ "};\n"
+ "void f(A* a) {\n"
+ " if (!a->h())\n"
+ " return;\n"
+ " const A *b = a;\n"
+ " while (b && !b->h())\n"
+ " b = b->g();\n"
+ " if (!b || b == b->g()->h())\n"
+ " return;\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+ }
+
+ void nullpointer62()
+ {
+ check("struct A {\n"
+ " bool f()() const;\n"
+ "};\n"
+ "void a(A *x) {\n"
+ " std::string b = x && x->f() ? \"\" : \"\";\n"
+ " if (x) {}\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+
+ check("struct A {\n"
+ " bool f()() const;\n"
+ "};\n"
+ "void a(A *x) {\n"
+ " std::string b = (!x || x->f()) ? \"\" : \"\";\n"
+ " if (x) {}\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+
+ check("struct A {\n"
+ " A * aa;\n"
+ "};\n"
+ "void b(A*);\n"
+ "void a(A *x) {\n"
+ " b(x ? x->aa : nullptr);\n"
+ " if (!x) {}\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+ }
+
+ void nullpointer63()
+ {
+ check("struct A {\n"
+ " A* a() const;\n"
+ " A* b() const;\n"
+ "};\n"
+ "A* f(A*);\n"
+ "void g(const A* x) {\n"
+ " A *d = x->a();\n"
+ " d = f(d->b()) ? d->a() : nullptr;\n"
+ " if (d && f(d->b())) {}\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+ }
+
+ void nullpointer64()
+ {
+ check("struct A {\n"
+ " A* f() const;\n"
+ " int g() const;\n"
+ "};\n"
+ "bool a;\n"
+ "bool b(A* c) {\n"
+ " if (c->g() == 0)\n"
+ " ;\n"
+ " A *aq = c;\n"
+ " if (c->g() == 0)\n"
+ " c = c->f();\n"
+ " if (c)\n"
+ " for (A *d = c; d != aq; d = d->f()) {}\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+
+ check("struct A {\n"
+ " A* g() const;\n"
+ " A* h() const;\n"
+ "};\n"
+ "bool i(A*);\n"
+ "void f(A* x) {\n"
+ " if (i(x->g())) {\n"
+ " A *y = x->g();\n"
+ " x = x->g()->h();\n"
+ " if (x && x->g()) {\n"
+ " y = x->g()->h();\n"
+ " }\n"
+ " if (!y) {}\n"
+ " }\n"
+ "}\n");
+ ASSERT_EQUALS("", errout.str());
+ }
+
void nullpointer_addressOf() { // address of
check("void f() {\n"
" struct X *x = 0;\n"
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index 897a28d1c..06aa3509f 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -1082,7 +1082,7 @@ private:
" if (y == 32) {}"
"}\n";
ASSERT_EQUALS("5,Assuming that condition 'y==32' is not redundant\n"
- "4,Compound assignment '+=', before assignment value is 20\n"
+ "4,Compound assignment '+=', assigned value is 20\n"
"2,Assignment 'x=y', assigned value is 20\n",
getErrorPathForX(code, 3U));
@@ -1191,9 +1191,6 @@ private:
" if (x == 4);\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 3));
- ASSERT_EQUALS("4,Assuming that condition 'x==4' is not redundant\n"
- "3,x is incremented, before this increment the value is 3\n",
- getErrorPathForX(code, 2U));
// compound assignment += , -= , ...
code = "void f(int x) {\n"
@@ -1222,15 +1219,16 @@ private:
" x /= 5;\n"
" if (x == 42);\n"
"}";
- ASSERT(tokenValues(code, "x ;").empty());
+ ASSERT_EQUALS(true, testValueOfX(code, 2U, 210));
// bailout: assignment
bailout("void f(int x) {\n"
" x = y;\n"
" if (x == 123) {}\n"
"}");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n"
- "[test.cpp:2]: (debug) valueflow.cpp::valueFlowReverse bailout: assignment of x\n", errout.str());
+ ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+ "[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n",
+ errout.str());
}
void valueFlowBeforeConditionAndAndOrOrGuard() { // guarding by &&
@@ -1345,14 +1343,14 @@ private:
bailout("void f(int x) {\n"
" y = ((x<0) ? x : ((x==2)?3:4));\n"
"}");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n"
- "[test.cpp:2]: (debug) valueflow.cpp:1113:valueFlowReverse bailout: no simplification of x within ?: expression\n", errout.str());
+ ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+ "[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n",
+ errout.str());
bailout("int f(int x) {\n"
" int r = x ? 1 / x : 0;\n"
" if (x == 0) {}\n"
"}");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:1113:valueFlowReverse bailout: no simplification of x within ?: expression\n", errout.str());
code = "void f(int x) {\n"
" int a =v x;\n"
@@ -1410,15 +1408,23 @@ private:
" if (x != 123) { b = x; }\n"
" if (x == 123) {}\n"
"}");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable b\n"
- "[test.cpp:2]: (debug) valueflow.cpp:1144:valueFlowReverse bailout: variable x stopping on }\n", errout.str());
+ ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+ "[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable b\n",
+ errout.str());
- code = "void f(int x) {\n"
+ code = "void f(int x, bool abc) {\n"
" a = x;\n"
- " if (abc) { x = 1; }\n" // <- condition must be false if x is 7 in next line
+ " if (abc) { x = 1; }\n" // <- condition must be false if x is 7 in next line
" if (x == 7) { }\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 7));
+
+ code = "void f(int x, bool abc) {\n"
+ " a = x;\n"
+ " if (abc) { x = 7; }\n" // <- condition is probably true
+ " if (x == 7) { }\n"
+ "}";
+ ASSERT_EQUALS(false, testValueOfX(code, 2U, 7));
}
void valueFlowBeforeConditionGlobalVariables() {
@@ -1451,8 +1457,9 @@ private:
" case 2: if (x==5) {} break;\n"
" };\n"
"}");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
- "[test.cpp:3]: (debug) valueflow.cpp:1180:valueFlowReverse bailout: variable x stopping on break\n", errout.str());
+ ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+ "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
+ errout.str());
bailout("void f(int x, int y) {\n"
" switch (y) {\n"
@@ -1460,8 +1467,9 @@ private:
" case 2: if (x==5) {} break;\n"
" };\n"
"}");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
- "[test.cpp:3]: (debug) valueflow.cpp:1180:valueFlowReverse bailout: variable x stopping on return\n", errout.str());
+ ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+ "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
+ errout.str());
}
void valueFlowBeforeConditionMacro() {
@@ -1492,8 +1500,7 @@ private:
" if (x==123){}\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
- "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
- "[test.cpp:4]: (debug) valueflow.cpp:1131:valueFlowReverse bailout: variable x stopping on goto label\n",
+ "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
errout.str());
// #5721 - FP
@@ -1507,10 +1514,6 @@ private:
"out:\n"
" if (abc) {}\n"
"}\n");
- ASSERT_EQUALS_WITHOUT_LINENUMBERS(
- "[test.cpp:2]: (debug) valueflow.cpp:1035:valueFlowReverse bailout: assignment of abc\n"
- "[test.cpp:8]: (debug) valueflow.cpp:1131:valueFlowReverse bailout: variable abc stopping on goto label\n",
- errout.str());
}
void valueFlowBeforeConditionForward() {