diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 86d5e35273d..bb2cec13efa 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -96,10 +96,25 @@ const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossib return nullptr; } +ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossible) +{ + if (mValues->empty()) + return nullptr; + + // TODO: avoid copy if no value is found + copyOnWrite(); + + const auto it = find(exprid); + const bool found = it != mValues->cend() && (impossible || !it->second.isImpossible()); + if (found) + return &it->second; + return nullptr; +} + // cppcheck-suppress unusedFunction -bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint& result) const +bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint& result, bool impossible) const { - const ValueFlow::Value* value = getValue(exprid); + const ValueFlow::Value* value = getValue(exprid, impossible); if (value && value->isIntValue()) { result = value->intvalue; return true; @@ -115,9 +130,9 @@ void ProgramMemory::setIntValue(const Token* expr, MathLib::bigint value, bool i setValue(expr, v); } -bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const +bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result, bool impossible) const { - const ValueFlow::Value* value = getValue(exprid); + const ValueFlow::Value* value = getValue(exprid, impossible); if (value && value->isTokValue()) { result = value->tokvalue; return true; @@ -126,18 +141,18 @@ bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const } // cppcheck-suppress unusedFunction -bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint& result) const +bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint& result, bool impossible) const { - const ValueFlow::Value* value = getValue(exprid); + const ValueFlow::Value* value = getValue(exprid, impossible); if (value && value->isContainerSizeValue()) { result = value->intvalue; return true; } return false; } -bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result) const +bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result, bool impossible) const { - const ValueFlow::Value* value = getValue(exprid, true); + const ValueFlow::Value* value = getValue(exprid, impossible); if (value && value->isContainerSizeValue()) { if (value->isImpossible() && value->intvalue == 0) { result = false; @@ -166,28 +181,10 @@ void ProgramMemory::setUnknown(const Token* expr) { (*mValues)[expr].valueType = ValueFlow::Value::ValueType::UNINIT; } -bool ProgramMemory::hasValue(nonneg int exprid) const +bool ProgramMemory::hasValue(nonneg int exprid, bool impossible) const { const auto it = find(exprid); - return it != mValues->cend(); -} - -const ValueFlow::Value& ProgramMemory::at(nonneg int exprid) const { - const auto it = find(exprid); - if (it == mValues->cend()) { - throw std::out_of_range("ProgramMemory::at"); - } - return it->second; -} - -ValueFlow::Value& ProgramMemory::at(nonneg int exprid) { - copyOnWrite(); - - const auto it = find(exprid); - if (it == mValues->end()) { - throw std::out_of_range("ProgramMemory::at"); - } - return it->second; + return it != mValues->cend() && (impossible || !it->second.isImpossible()); } void ProgramMemory::erase_if(const std::function& pred) @@ -253,6 +250,7 @@ ProgramMemory::Map::const_iterator ProgramMemory::find(nonneg int exprid) const return cvalues.find(ExprIdToken::create(exprid)); } +// need to call copyOnWrite() before calling this ProgramMemory::Map::iterator ProgramMemory::find(nonneg int exprid) { return mValues->find(ExprIdToken::create(exprid)); @@ -1350,10 +1348,9 @@ namespace { ValueFlow::Value executeMultiCondition(bool b, const Token* expr) { - if (pm->hasValue(expr->exprId())) { - const ValueFlow::Value& v = utils::as_const(*pm).at(expr->exprId()); - if (v.isIntValue()) - return v; + if (const ValueFlow::Value* v = utils::as_const(*pm).getValue(expr->exprId(), true)) { + if (v->isIntValue()) + return *v; } // Evaluate recursively if there are no exprids @@ -1482,18 +1479,18 @@ namespace { if (rhs.isUninitValue()) return unknown(); if (expr->str() != "=") { - if (!pm->hasValue(expr->astOperand1()->exprId())) + ValueFlow::Value* lhs = pm->getValue(expr->astOperand1()->exprId(), true); + if (!lhs) return unknown(); - ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); - rhs = evaluate(removeAssign(expr->str()), lhs, rhs); - if (lhs.isIntValue()) - ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1)); - else if (lhs.isFloatValue()) + rhs = evaluate(removeAssign(expr->str()), *lhs, rhs); + if (lhs->isIntValue()) + ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs->intvalue), std::placeholders::_1)); + else if (lhs->isFloatValue()) ValueFlow::Value::visitValue(rhs, - std::bind(assign{}, std::ref(lhs.floatValue), std::placeholders::_1)); + std::bind(assign{}, std::ref(lhs->floatValue), std::placeholders::_1)); else return unknown(); - return lhs; + return *lhs; } pm->setValue(expr->astOperand1(), rhs); return rhs; @@ -1505,20 +1502,20 @@ namespace { execute(expr->astOperand1()); return execute(expr->astOperand2()); } else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) { - if (!pm->hasValue(expr->astOperand1()->exprId())) + ValueFlow::Value* lhs = pm->getValue(expr->astOperand1()->exprId(), true); + if (!lhs) return ValueFlow::Value::unknown(); - ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId()); - if (!lhs.isIntValue()) + if (!lhs->isIntValue()) return unknown(); // overflow - if (!lhs.isImpossible() && lhs.intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1())) + if (!lhs->isImpossible() && lhs->intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1())) return unknown(); if (expr->str() == "++") - lhs.intvalue++; + lhs->intvalue++; else - lhs.intvalue--; - return lhs; + lhs->intvalue--; + return *lhs; } else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { const Token* tokvalue = nullptr; if (!pm->getTokValue(expr->astOperand1()->exprId(), tokvalue)) { @@ -1609,13 +1606,16 @@ namespace { } return execute(expr->astOperand1()); } - if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) { - ValueFlow::Value result = utils::as_const(*pm).at(expr->exprId()); - if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) { - result.intvalue = !result.intvalue; - result.setKnown(); + if (expr->exprId() > 0) { + if (const ValueFlow::Value* v = utils::as_const(*pm).getValue(expr->exprId(), true)) + { + ValueFlow::Value result = *v; + if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) { + result.intvalue = !result.intvalue; + result.setKnown(); + } + return result; } - return result; } if (Token::Match(expr->previous(), ">|%name% {|(")) { @@ -1665,14 +1665,16 @@ namespace { } // Check if function modifies argument visitAstNodes(expr->astOperand2(), [&](const Token* child) { - if (child->exprId() > 0 && pm->hasValue(child->exprId())) { - ValueFlow::Value& v = pm->at(child->exprId()); - if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { - if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings)) - v = unknown(); - } else if (v.valueType != ValueFlow::Value::ValueType::UNINIT) { - if (isVariableChanged(child, v.indirect, settings)) - v = unknown(); + if (child->exprId() > 0) { + if (ValueFlow::Value* v = pm->getValue(child->exprId(), true)) + { + if (v->valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) { + if (ValueFlow::isContainerSizeChanged(child, v->indirect, settings)) + *v = unknown(); + } else if (v->valueType != ValueFlow::Value::ValueType::UNINIT) { + if (isVariableChanged(child, v->indirect, settings)) + *v = unknown(); + } } } return ChildrenToVisit::op1_and_op2; @@ -1724,9 +1726,12 @@ namespace { return v; if (!expr) return v; - if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) { - if (updateValue(v, utils::as_const(*pm).at(expr->exprId()))) - return v; + if (expr->exprId() > 0) { + if (const ValueFlow::Value* val = utils::as_const(*pm).getValue(expr->exprId(), true)) + { + if (updateValue(v, *val)) + return v; + } } // Find symbolic values for (const ValueFlow::Value& value : expr->values()) { @@ -1734,12 +1739,14 @@ namespace { continue; if (!value.isKnown()) continue; - if (value.tokvalue->exprId() > 0 && !pm->hasValue(value.tokvalue->exprId())) + if (value.tokvalue->exprId() == 0) + continue; + const ValueFlow::Value* v_p = utils::as_const(*pm).getValue(value.tokvalue->exprId(), true); + if (!v_p) continue; - const ValueFlow::Value& v_ref = utils::as_const(*pm).at(value.tokvalue->exprId()); - if (!v_ref.isIntValue() && value.intvalue != 0) + if (!v_p->isIntValue() && value.intvalue != 0) continue; - ValueFlow::Value v2 = v_ref; + ValueFlow::Value v2 = *v_p; v2.intvalue += value.intvalue; return v2; } diff --git a/lib/programmemory.h b/lib/programmemory.h index bc0a5537f7b..c31738fd56c 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -110,21 +110,19 @@ struct CPPCHECKLIB ProgramMemory { void setValue(const Token* expr, const ValueFlow::Value& value); const ValueFlow::Value* getValue(nonneg int exprid, bool impossible = false) const; + ValueFlow::Value* getValue(nonneg int exprid, bool impossible = false); - bool getIntValue(nonneg int exprid, MathLib::bigint& result) const; + bool getIntValue(nonneg int exprid, MathLib::bigint& result, bool impossible = false) const; void setIntValue(const Token* expr, MathLib::bigint value, bool impossible = false); - bool getContainerSizeValue(nonneg int exprid, MathLib::bigint& result) const; - bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result) const; + bool getContainerSizeValue(nonneg int exprid, MathLib::bigint& result, bool impossible = false) const; + bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint& result, bool impossible = false) const; void setContainerSizeValue(const Token* expr, MathLib::bigint value, bool isEqual = true); void setUnknown(const Token* expr); - bool getTokValue(nonneg int exprid, const Token*& result) const; - bool hasValue(nonneg int exprid) const; - - const ValueFlow::Value& at(nonneg int exprid) const; - ValueFlow::Value& at(nonneg int exprid); + bool getTokValue(nonneg int exprid, const Token*& result, bool impossible = false) const; + bool hasValue(nonneg int exprid, bool impossible = true) const; void erase_if(const std::function& pred); diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 1b70b4446d1..e94ade65189 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -715,7 +715,8 @@ struct ValueFlowAnalyzer : Analyzer { return {value->intvalue == 0}; ProgramMemory pm = pms.get(tok, ctx, getProgramState()); MathLib::bigint out = 0; - if (pm.getContainerEmptyValue(tok->exprId(), out)) + // TODO: do we really went to return an impossible value? + if (pm.getContainerEmptyValue(tok->exprId(), out, true)) return {static_cast(out)}; return {}; } diff --git a/test/testprogrammemory.cpp b/test/testprogrammemory.cpp index 13a5f8e639a..a73090360b6 100644 --- a/test/testprogrammemory.cpp +++ b/test/testprogrammemory.cpp @@ -32,7 +32,6 @@ class TestProgramMemory : public TestFixture { TEST_CASE(copyOnWrite); TEST_CASE(hasValue); TEST_CASE(getValue); - TEST_CASE(at); } void copyOnWrite() const { @@ -42,11 +41,11 @@ class TestProgramMemory : public TestFixture { tok->exprId(id); ProgramMemory pm; - const ValueFlow::Value* v = pm.getValue(id); + const ValueFlow::Value* v = utils::as_const(pm).getValue(id, false); ASSERT(!v); pm.setValue(tok, ValueFlow::Value{41}); - v = pm.getValue(id); + v = utils::as_const(pm).getValue(id, false); ASSERT(v); ASSERT_EQUALS(41, v->intvalue); @@ -54,7 +53,7 @@ class TestProgramMemory : public TestFixture { ProgramMemory pm2 = pm; // make sure the value was copied - v = pm2.getValue(id); + v = utils::as_const(pm2).getValue(id, false); ASSERT(v); ASSERT_EQUALS(41, v->intvalue); @@ -68,17 +67,17 @@ class TestProgramMemory : public TestFixture { pm3.setValue(tok, ValueFlow::Value{43}); // make sure the value was set - v = pm2.getValue(id); + v = utils::as_const(pm2).getValue(id, false); ASSERT(v); ASSERT_EQUALS(42, v->intvalue); // make sure the value was set - v = pm3.getValue(id); + v = utils::as_const(pm3).getValue(id, false); ASSERT(v); ASSERT_EQUALS(43, v->intvalue); // make sure the original value remains unchanged - v = pm.getValue(id); + v = utils::as_const(pm).getValue(id, false); ASSERT(v); ASSERT_EQUALS(41, v->intvalue); } @@ -90,13 +89,7 @@ class TestProgramMemory : public TestFixture { void getValue() const { ProgramMemory pm; - ASSERT(!pm.getValue(123)); - } - - void at() const { - ProgramMemory pm; - ASSERT_THROW_EQUALS_2(pm.at(123), std::out_of_range, "ProgramMemory::at"); - ASSERT_THROW_EQUALS_2(utils::as_const(pm).at(123), std::out_of_range, "ProgramMemory::at"); + ASSERT(!utils::as_const(pm).getValue(123, false)); } };