From f3d9755e652d4ff8391e425be8b711129db7df09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 3 Oct 2021 18:12:29 +0200 Subject: [PATCH] UninitVar: too many warnings (pointer dereference) --- lib/astutils.cpp | 5 +++++ lib/astutils.h | 2 ++ lib/checkuninitvar.cpp | 5 ----- lib/forwardanalyzer.cpp | 8 ++++++-- test/testvalueflow.cpp | 29 +++++++++++++++++++++++------ 5 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index d32701112..cf6d8a496 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -3186,3 +3186,8 @@ bool FwdAnalysis::isEscapedAlias(const Token* expr) } return false; } + +bool isSizeOfEtc(const Token *tok) +{ + return Token::Match(tok, "sizeof|typeof|offsetof|decltype|__typeof__ ("); +} diff --git a/lib/astutils.h b/lib/astutils.h index 87102f9be..f3364c6cd 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -376,4 +376,6 @@ private: bool mValueFlowKnown; }; +bool isSizeOfEtc(const Token *tok); + #endif // astutilsH diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 579594ec6..c3c6f2ca0 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -50,11 +50,6 @@ namespace { //--------------------------------------------------------------------------- -bool isSizeOfEtc(const Token *tok) -{ - return Token::Match(tok, "sizeof|typeof|offsetof|decltype|__typeof__ ("); -} - // get ast parent, skip possible address-of and casts static const Token *getAstParentSkipPossibleCastAndAddressOf(const Token *vartok, bool *unknown) { diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index d050f84e3..76a362a6a 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -5,6 +5,7 @@ #include "symboldatabase.h" #include "token.h" #include "valueptr.h" +#include "checknullpointer.h" // UninitValue => isPointerDeref #include #include @@ -12,8 +13,6 @@ #include #include -bool isSizeOfEtc(const Token *tok); - struct OnExit { std::function f; @@ -200,6 +199,11 @@ struct ForwardTraversal { // uninit value => skip further analysis auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); if (v != tok->values().end()) { + if (tok->valueType() && tok->valueType()->pointer > 0) { + bool unknown = false; + if (CheckNullPointer::isPointerDeRef(tok, unknown, settings)) + return Break(Analyzer::Terminate::Modified); + } if (Token::Match(tok->astParent(), "[,(]") && !isSizeOfEtc(tok->astParent()->previous())) return Break(Analyzer::Terminate::Modified); } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 04f347782..d46fa4bd1 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -4661,9 +4661,9 @@ private: "};\n" "\n" "void copy_wcs(wcsstruct *wcsin) {\n" - " wcsstruct *x;\n" - " memcpy(wcsin, x, sizeof(wcsstruct));\n" // <- True positive - " x->wcsprm = NULL;\n" // <- False positive + " wcsstruct *x;\n" + " memcpy(wcsin, x, sizeof(wcsstruct));\n" // <- True positive + " x->wcsprm = NULL;\n" // <- False positive "}"; values = tokenValues(code, "x . wcsprm", ValueFlow::Value::ValueType::UNINIT); ASSERT_EQUALS(0, values.size()); @@ -4673,12 +4673,29 @@ private: "};\n" "\n" "void copy_wcs(wcsstruct *wcsin) {\n" - " wcsstruct *x;\n" - " sizeof(x);\n" - " x->wcsprm = NULL;\n" // <- Warn + " wcsstruct *x;\n" + " sizeof(x);\n" + " x->wcsprm = NULL;\n" // <- Warn "}"; values = tokenValues(code, "x . wcsprm", ValueFlow::Value::ValueType::UNINIT); ASSERT_EQUALS(1, values.size()); + + code = "struct wcsstruct {\n" + " int *wcsprm;\n" + "};\n" + "\n" + "void init_wcs(wcsstruct *x) { if (x->wcsprm != NULL); }\n" // <- False positive + "\n" + "void copy_wcs() {\n" + " wcsstruct *x;\n" + " x->wcsprm = NULL;\n" // <- True positive + " init_wcs(x);\n" // <- False positive + "}"; + values = tokenValues(code, "x . wcsprm != NULL", ValueFlow::Value::ValueType::UNINIT); + ASSERT_EQUALS(0, values.size()); + values = tokenValues(code, "x )", ValueFlow::Value::ValueType::UNINIT); + ASSERT_EQUALS(0, values.size()); + } void valueFlowConditionExpressions() {