From cee48e5e19ad01d62217247a9b16b518a9a05d9c Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 12 Jun 2022 00:13:42 -0500 Subject: [PATCH] Add backtrace to valueflow in debug mode (#4195) --- .github/workflows/selfcheck.yml | 4 +- Makefile | 2 +- cmake/cxx11.cmake | 2 +- lib/sourcelocation.h | 71 ++++++++++++++++ lib/valueflow.cpp | 138 ++++++++++++++++++++++---------- lib/valueflow.h | 2 + 6 files changed, 174 insertions(+), 45 deletions(-) create mode 100644 lib/sourcelocation.h diff --git a/.github/workflows/selfcheck.yml b/.github/workflows/selfcheck.yml index d223d0ba3..8501fd9d0 100644 --- a/.github/workflows/selfcheck.yml +++ b/.github/workflows/selfcheck.yml @@ -54,7 +54,7 @@ jobs: - name: Self check (unusedFunction) if: false # TODO: fails with preprocessorErrorDirective - see #10667 run: | - ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__GNUC__ -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 --inconclusive --enable=unusedFunction --exception-handling -rp=. --project=cmake.output/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr + ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__CPPCHECK__ -D__GNUC__ -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 --inconclusive --enable=unusedFunction --exception-handling -rp=. --project=cmake.output/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr env: DISABLE_VALUEFLOW: 1 @@ -76,6 +76,6 @@ jobs: # TODO: find a way to report unmatched suppressions without need to add information checks - name: Self check (unusedFunction / no test) run: | - ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__GNUC__ -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 --inconclusive --enable=unusedFunction --exception-handling -rp=. --project=cmake.output.notest/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr + ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__CPPCHECK__ -D__GNUC__ -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 --inconclusive --enable=unusedFunction --exception-handling -rp=. --project=cmake.output.notest/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr env: DISABLE_VALUEFLOW: 1 diff --git a/Makefile b/Makefile index 9e6face80..c58838692 100644 --- a/Makefile +++ b/Makefile @@ -561,7 +561,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/analyzer.h lib/astutils.h lib/calculate.h lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/infer.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 +$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/analyzer.h lib/astutils.h lib/calculate.h lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/infer.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/programmemory.h lib/reverseanalyzer.h lib/settings.h lib/sourcelocation.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 externals/tinyxml2/tinyxml2.h lib/check.h lib/color.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/cmake/cxx11.cmake b/cmake/cxx11.cmake index 89cf3103b..176214ab1 100644 --- a/cmake/cxx11.cmake +++ b/cmake/cxx11.cmake @@ -6,7 +6,7 @@ macro(use_cxx11) set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") endif () else () - set (CMAKE_CXX_STANDARD 11) + set (CMAKE_CXX_STANDARD 11 CACHE STRING "C++ standard to use") set (CMAKE_CXX_STANDARD_REQUIRED ON) if (POLICY CMP0025) cmake_policy(SET CMP0025 NEW) diff --git a/lib/sourcelocation.h b/lib/sourcelocation.h new file mode 100644 index 000000000..9a0581abb --- /dev/null +++ b/lib/sourcelocation.h @@ -0,0 +1,71 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2022 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 sourcelocationH +#define sourcelocationH + +#ifdef __CPPCHECK__ +#define CPPCHECK_HAS_SOURCE_LOCATION 0 +#define CPPCHECK_HAS_SOURCE_LOCATION_TS 0 +#elif defined(__has_include) +#if __has_include() && __cplusplus >= 202003L +#define CPPCHECK_HAS_SOURCE_LOCATION 1 +#else +#define CPPCHECK_HAS_SOURCE_LOCATION 0 +#endif +#if __has_include() && __cplusplus >= 201402L +#define CPPCHECK_HAS_SOURCE_LOCATION_TS 1 +#else +#define CPPCHECK_HAS_SOURCE_LOCATION_TS 0 +#endif +#else +#define CPPCHECK_HAS_SOURCE_LOCATION 0 +#define CPPCHECK_HAS_SOURCE_LOCATION_TS 0 +#endif + +#if CPPCHECK_HAS_SOURCE_LOCATION +#include +using SourceLocation = std::source_location; +#elif CPPCHECK_HAS_SOURCE_LOCATION_TS +#include +using SourceLocation = std::experimental::source_location; +#else +struct SourceLocation { + static SourceLocation current() { + return SourceLocation(); + } + std::uint_least32_t m_line = 0; + std::uint_least32_t m_column = 0; + const char* m_file_name = ""; + const char* m_function_name = ""; + std::uint_least32_t line() const { + return m_line; + } + std::uint_least32_t column() const { + return m_column; + } + const char* file_name() const { + return m_file_name; + } + const char* function_name() const { + return m_function_name; + } +}; +#endif + +#endif diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 681521d6c..55ff70960 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -93,6 +93,7 @@ #include "programmemory.h" #include "reverseanalyzer.h" #include "settings.h" +#include "sourcelocation.h" #include "standards.h" #include "symboldatabase.h" #include "token.h" @@ -137,6 +138,38 @@ static void bailoutInternal(const std::string& type, TokenList *tokenlist, Error #define bailoutIncompleteVar(tokenlist, errorLogger, tok, what) bailout2("valueFlowBailoutIncompleteVar", tokenlist, errorLogger, tok, what) +static std::string debugString(const ValueFlow::Value& v) +{ + std::string kind; + switch (v.valueKind) { + + case ValueFlow::Value::ValueKind::Impossible: + case ValueFlow::Value::ValueKind::Known: + kind = "always"; + break; + case ValueFlow::Value::ValueKind::Inconclusive: + kind = "inconclusive"; + break; + case ValueFlow::Value::ValueKind::Possible: + kind = "possible"; + break; + } + return kind + " " + v.toString(); +} + +static void setSourceLocation(ValueFlow::Value& v, + SourceLocation ctx, + const Token* tok, + SourceLocation local = SourceLocation::current()) +{ + std::string file = ctx.file_name(); + if (file.empty()) + return; + std::string s = Path::stripDirectoryPart(file) + ":" + MathLib::toString(ctx.line()) + ": " + ctx.function_name() + + " => " + local.function_name() + ": " + debugString(v); + v.debugPath.emplace_back(tok, s); +} + static void changeKnownToPossible(std::list &values, int indirect=-1) { for (ValueFlow::Value& v: values) { @@ -619,7 +652,10 @@ static ValueFlow::Value truncateImplicitConversion(Token* parent, const ValueFlo } /** set ValueFlow value and perform calculations if possible */ -static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* settings) +static void setTokenValue(Token* tok, + ValueFlow::Value value, + const Settings* settings, + SourceLocation loc = SourceLocation::current()) { // Skip setting values that are too big since its ambiguous if (!value.isImpossible() && value.isIntValue() && value.intvalue < 0 && astIsUnsigned(tok) && @@ -629,6 +665,9 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se if (!value.isImpossible() && value.isIntValue()) value = truncateImplicitConversion(tok->astParent(), value, settings); + if (settings->debugnormal) + setSourceLocation(value, loc, tok); + if (!tok->addValue(value)) return; @@ -1907,43 +1946,58 @@ ValuePtr makeReverseAnalyzer(const Token* exprTok, const ValueFlow::Va static Analyzer::Result valueFlowForward(Token* startToken, const Token* endToken, const Token* exprTok, - const ValueFlow::Value& value, - TokenList* const tokenlist) + ValueFlow::Value value, + TokenList* const tokenlist, + SourceLocation loc = SourceLocation::current()) { - return valueFlowGenericForward(startToken, endToken, makeAnalyzer(exprTok, value, tokenlist), tokenlist->getSettings()); + if (tokenlist->getSettings()->debugnormal) + setSourceLocation(value, loc, startToken); + return valueFlowGenericForward(startToken, + endToken, + makeAnalyzer(exprTok, std::move(value), tokenlist), + tokenlist->getSettings()); } static Analyzer::Result valueFlowForward(Token* startToken, const Token* endToken, const Token* exprTok, - const std::list& values, - TokenList* const tokenlist) + std::list values, + TokenList* const tokenlist, + SourceLocation loc = SourceLocation::current()) { Analyzer::Result result{}; - for (const ValueFlow::Value& v : values) { - result.update(valueFlowForward(startToken, endToken, exprTok, v, tokenlist)); + for (ValueFlow::Value& v : values) { + result.update(valueFlowForward(startToken, endToken, exprTok, std::move(v), tokenlist, loc)); } return result; } template -static Analyzer::Result valueFlowForward(Token* startToken, const Token* exprTok, const ValueOrValues& v, TokenList* tokenlist) +static Analyzer::Result valueFlowForward(Token* startToken, + const Token* exprTok, + ValueOrValues v, + TokenList* tokenlist, + SourceLocation loc = SourceLocation::current()) { const Token* endToken = nullptr; const Function* f = Scope::nestedInFunction(startToken->scope()); if (f && f->functionScope) endToken = f->functionScope->bodyEnd; - return valueFlowForward(startToken, endToken, exprTok, v, tokenlist); + return valueFlowForward(startToken, endToken, exprTok, std::move(v), tokenlist, loc); } static Analyzer::Result valueFlowForwardRecursive(Token* top, const Token* exprTok, - const std::list& values, - TokenList* const tokenlist) + std::list values, + TokenList* const tokenlist, + SourceLocation loc = SourceLocation::current()) { Analyzer::Result result{}; - for (const ValueFlow::Value& v : values) { - result.update(valueFlowGenericForward(top, makeAnalyzer(exprTok, v, tokenlist), tokenlist->getSettings())); + for (ValueFlow::Value& v : values) { + if (tokenlist->getSettings()->debugnormal) + setSourceLocation(v, loc, top); + result.update( + valueFlowGenericForward(top, makeAnalyzer(exprTok, std::move(v), tokenlist), tokenlist->getSettings())); } return result; } @@ -1951,10 +2005,13 @@ static Analyzer::Result valueFlowForwardRecursive(Token* top, static void valueFlowReverse(Token* tok, const Token* const endToken, const Token* const varToken, - const std::list& values, - TokenList* tokenlist) + std::list values, + TokenList* tokenlist, + SourceLocation loc = SourceLocation::current()) { - for (const ValueFlow::Value& v : values) { + for (ValueFlow::Value& v : values) { + if (tokenlist->getSettings()->debugnormal) + setSourceLocation(v, loc, tok); valueFlowGenericReverse(tok, endToken, makeReverseAnalyzer(varToken, v, tokenlist), tokenlist->getSettings()); } } @@ -1966,12 +2023,13 @@ static void valueFlowReverse(TokenList* tokenlist, ValueFlow::Value val, const ValueFlow::Value& val2, ErrorLogger* /*errorLogger*/, - const Settings* = nullptr) + const Settings* = nullptr, + SourceLocation loc = SourceLocation::current()) { std::list values = {val}; if (val2.varId != 0) values.push_back(val2); - valueFlowReverse(tok, nullptr, varToken, values, tokenlist); + valueFlowReverse(tok, nullptr, varToken, values, tokenlist, loc); } static bool isConditionKnown(const Token* tok, bool then) @@ -4740,7 +4798,11 @@ static const Token* findIncompleteVar(const Token* start, const Token* end) return nullptr; } -static ValueFlow::Value makeConditionValue(long long val, const Token* condTok, bool assume) +static ValueFlow::Value makeConditionValue(long long val, + const Token* condTok, + bool assume, + const Settings* settings = nullptr, + SourceLocation loc = SourceLocation::current()) { ValueFlow::Value v(val); v.setKnown(); @@ -4749,6 +4811,8 @@ static ValueFlow::Value makeConditionValue(long long val, const Token* condTok, v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true"); else v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false"); + if (settings && settings->debugnormal) + setSourceLocation(v, loc, condTok); return v; } @@ -5559,26 +5623,29 @@ struct ConditionHandler { const Token* stop, const Token* exprTok, const std::list& values, - TokenList* tokenlist) const + TokenList* tokenlist, + SourceLocation loc = SourceLocation::current()) const { - return valueFlowForward(start->next(), stop, exprTok, values, tokenlist); + return valueFlowForward(start->next(), stop, exprTok, values, tokenlist, loc); } virtual Analyzer::Result forward(Token* top, const Token* exprTok, const std::list& values, - TokenList* tokenlist) const + TokenList* tokenlist, + SourceLocation loc = SourceLocation::current()) const { - return valueFlowForwardRecursive(top, exprTok, values, tokenlist); + return valueFlowForwardRecursive(top, exprTok, values, tokenlist, loc); } virtual void reverse(Token* start, const Token* endToken, const Token* exprTok, const std::list& values, - TokenList* tokenlist) const + TokenList* tokenlist, + SourceLocation loc = SourceLocation::current()) const { - return valueFlowReverse(start, endToken, exprTok, values, tokenlist); + return valueFlowReverse(start, endToken, exprTok, values, tokenlist, loc); } void traverseCondition(TokenList* tokenlist, @@ -8353,23 +8420,12 @@ static void valueFlowDebug(TokenList* tokenlist, ErrorLogger* errorLogger) for (Token* tok = tokenlist->front(); tok; tok = tok->next()) { if (tok->getTokenDebug() != TokenDebug::ValueFlow) continue; + if (tok->astParent() && tok->astParent()->getTokenDebug() == TokenDebug::ValueFlow) + continue; for (const ValueFlow::Value& v : tok->values()) { - std::string kind; - switch (v.valueKind) { - - case ValueFlow::Value::ValueKind::Impossible: - case ValueFlow::Value::ValueKind::Known: - kind = "always"; - break; - case ValueFlow::Value::ValueKind::Inconclusive: - kind = "inconclusive"; - break; - case ValueFlow::Value::ValueKind::Possible: - kind = "possible"; - break; - } - std::string msg = "The value is " + kind + " " + v.toString(); + std::string msg = "The value is " + debugString(v); ErrorPath errorPath = v.errorPath; + errorPath.insert(errorPath.end(), v.debugPath.begin(), v.debugPath.end()); errorPath.emplace_back(tok, ""); errorLogger->reportErr({errorPath, tokenlist, Severity::debug, "valueFlow", msg, CWE{0}, Certainty::normal}); } diff --git a/lib/valueflow.h b/lib/valueflow.h index bca23a8cd..8f699dc6a 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -342,6 +342,8 @@ namespace ValueFlow { ErrorPath errorPath; + ErrorPath debugPath; + /** For calculated values - varId that calculated value depends on */ nonneg int varId;