From 0cb742701d0628f9187d0439e1b7884aa0dc763d Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 8 Dec 2022 13:10:58 -0600 Subject: [PATCH] Fix 11415: FP containerOutOfBounds for container initialized in virtual method (#4622) --- lib/programmemory.cpp | 2 +- lib/valueflow.cpp | 32 +++++++++++++++++++++----------- lib/valueflow.h | 2 +- test/teststl.cpp | 19 +++++++++++++++++++ 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index de55ca118..14b6882cb 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -1349,7 +1349,7 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const if (child->exprId() > 0 && pm.hasValue(child->exprId())) { ValueFlow::Value& v = pm.at(child->exprId()); if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { - if (isContainerSizeChanged(child, settings)) + if (isContainerSizeChanged(child, v.indirect, settings)) v = unknown; } else if (v.valueType != ValueFlow::Value::ValueType::UNINIT) { if (isVariableChanged(child, v.indirect, settings, true)) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 85f900854..061634479 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7540,10 +7540,14 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba static bool isContainerSizeChanged(nonneg int varId, const Token* start, const Token* end, + int indirect, const Settings* settings = nullptr, int depth = 20); -static bool isContainerSizeChangedByFunction(const Token* tok, const Settings* settings = nullptr, int depth = 20) +static bool isContainerSizeChangedByFunction(const Token* tok, + int indirect, + const Settings* settings = nullptr, + int depth = 20) { if (!tok->valueType()) return false; @@ -7566,12 +7570,13 @@ static bool isContainerSizeChangedByFunction(const Token* tok, const Settings* s if (!ftok) return false; // not a function => variable not changed const Function * fun = ftok->function(); - if (fun && !fun->hasVirtualSpecifier()) { + if (fun && !fun->isImplicitlyVirtual()) { const Variable *arg = fun->getArgumentVar(narg); if (arg) { - if (!arg->isReference() && !addressOf) + const bool isPointer = addressOf || indirect > 0; + if (!arg->isReference() && !isPointer) return false; - if (!addressOf && arg->isConst()) + if (!isPointer && arg->isConst()) return false; if (arg->valueType() && arg->valueType()->constness == 1) return false; @@ -7581,8 +7586,12 @@ static bool isContainerSizeChangedByFunction(const Token* tok, const Settings* s if (!arg->nameToken()) return false; if (depth > 0) - return isContainerSizeChanged( - arg->declarationId(), scope->bodyStart, scope->bodyEnd, settings, depth - 1); + return isContainerSizeChanged(arg->declarationId(), + scope->bodyStart, + scope->bodyEnd, + addressOf ? indirect + 1 : indirect, + settings, + depth - 1); } // Don't know => Safe guess return true; @@ -7590,7 +7599,7 @@ static bool isContainerSizeChangedByFunction(const Token* tok, const Settings* s } bool inconclusive = false; - const bool isChanged = isVariableChangedByFunctionCall(tok, 0, settings, &inconclusive); + const bool isChanged = isVariableChangedByFunctionCall(tok, indirect, settings, &inconclusive); return (isChanged || inconclusive); } @@ -7685,7 +7694,7 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer { return Action::Invalid; if (isLikelyStreamRead(isCPP(), tok->astParent())) return Action::Invalid; - if (astIsContainer(tok) && isContainerSizeChanged(tok, getSettings())) + if (astIsContainer(tok) && isContainerSizeChanged(tok, getIndirect(tok), getSettings())) return read | Action::Invalid; return read; } @@ -7787,7 +7796,7 @@ ValuePtr makeReverseAnalyzer(const Token* exprTok, const ValueFlow::Va return ExpressionAnalyzer(exprTok, value, tokenlist); } -bool isContainerSizeChanged(const Token* tok, const Settings* settings, int depth) +bool isContainerSizeChanged(const Token* tok, int indirect, const Settings* settings, int depth) { if (!tok) return false; @@ -7823,7 +7832,7 @@ bool isContainerSizeChanged(const Token* tok, const Settings* settings, int dept case Library::Container::Action::CHANGE_INTERNAL: break; } - if (isContainerSizeChangedByFunction(tok, settings, depth)) + if (isContainerSizeChangedByFunction(tok, indirect, settings, depth)) return true; return false; } @@ -7831,13 +7840,14 @@ bool isContainerSizeChanged(const Token* tok, const Settings* settings, int dept static bool isContainerSizeChanged(nonneg int varId, const Token* start, const Token* end, + int indirect, const Settings* settings, int depth) { for (const Token *tok = start; tok != end; tok = tok->next()) { if (tok->varId() != varId) continue; - if (isContainerSizeChanged(tok, settings, depth)) + if (isContainerSizeChanged(tok, indirect, settings, depth)) return true; } return false; diff --git a/lib/valueflow.h b/lib/valueflow.h index ca5202f84..2bda7269b 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -467,7 +467,7 @@ namespace ValueFlow { ValueFlow::Value asImpossible(ValueFlow::Value v); -bool isContainerSizeChanged(const Token* tok, const Settings* settings = nullptr, int depth = 20); +bool isContainerSizeChanged(const Token* tok, int indirect, const Settings* settings = nullptr, int depth = 20); struct LifetimeToken { const Token* token; diff --git a/test/teststl.cpp b/test/teststl.cpp index 88a5b3aca..da12fd545 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -867,6 +867,25 @@ private: " return m.at(1);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + checkNormal("struct A {\n" + " virtual void init_v(std::vector *v) = 0;\n" + "};\n" + "A* create_a();\n" + "struct B {\n" + " B() : a(create_a()) {}\n" + " void init_v(std::vector *v) {\n" + " a->init_v(v);\n" + " }\n" + " A* a;\n" + "};\n" + "void f() {\n" + " B b;\n" + " std::vector v;\n" + " b.init_v(&v);\n" + " v[0];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void outOfBoundsSymbolic()