diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 244eeb002..73bf973f6 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1591,7 +1591,8 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 return false; } } else { - if (tok1->function() && !tok1->function()->isConst() && !tok1->function()->isAttributeConst() && !tok1->function()->isAttributePure()) + if (!tok1->function()->isConst() && !tok1->function()->isAttributeConst() && + !tok1->function()->isAttributePure()) return false; } } diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 6563bae3d..dd59f682a 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1965,7 +1965,7 @@ void CheckStl::string_c_str() string_c_strError(tok); } else if (printPerformance && tok->tokAt(1)->astOperand2() && Token::Match(tok->tokAt(1)->astOperand2()->tokAt(-3), "%var% . c_str|data ( ) ;")) { const Token* vartok = tok->tokAt(1)->astOperand2()->tokAt(-3); - if (tok->variable() && tok->variable()->isStlStringType() && vartok->variable() && vartok->variable()->isStlStringType()) + if (tok->variable()->isStlStringType() && vartok->variable() && vartok->variable()->isStlStringType()) string_c_strAssignment(tok); } } else if (printPerformance && tok->function() && Token::Match(tok, "%name% ( !!)") && tok->str() != scope.className) { diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 89ae97428..0d038c08c 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1612,7 +1612,7 @@ void CheckUninitVar::valueFlowUninit() bool uninitderef = false; if (tok->variable()) { bool unknown; - const bool isarray = !tok->variable() || tok->variable()->isArray(); + const bool isarray = tok->variable()->isArray(); const bool ispointer = astIsPointer(tok) && !isarray; const bool deref = CheckNullPointer::isPointerDeRef(tok, unknown, mSettings); if (ispointer && v->indirect == 1 && !deref) diff --git a/lib/infer.cpp b/lib/infer.cpp index fcf611e16..8904def34 100644 --- a/lib/infer.cpp +++ b/lib/infer.cpp @@ -269,6 +269,12 @@ static void addToErrorPath(ValueFlow::Value& value, const std::vectordebugPath.cbegin(), + ref->debugPath.cend(), + std::back_inserter(value.debugPath), + [&](const ErrorPathItem& e) { + return locations.insert(e.first).second; + }); } } diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 19272cd55..1ff598483 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -6138,7 +6138,7 @@ void SymbolDatabase::setValueType(Token* tok, const ValueType& valuetype, Source if (!parent->astOperand1()) return; - const ValueType *vt1 = parent->astOperand1() ? parent->astOperand1()->valueType() : nullptr; + const ValueType *vt1 = parent->astOperand1()->valueType(); const ValueType *vt2 = parent->astOperand2() ? parent->astOperand2()->valueType() : nullptr; if (vt1 && Token::Match(parent, "<<|>>")) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 09bc36ef8..3bf26415d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -508,6 +508,24 @@ static bool isComputableValue(const Token* parent, const ValueFlow::Value& value return true; } +static Library::Container::Yield getContainerYield(Token* tok, const Settings* settings, Token** parent = nullptr) +{ + if (Token::Match(tok, ". %name% (") && tok->astParent() == tok->tokAt(2) && tok->astOperand1() && + tok->astOperand1()->valueType()) { + const Library::Container* c = getLibraryContainer(tok->astOperand1()); + if (parent) + *parent = tok->astParent(); + return c ? c->getYield(tok->strAt(1)) : Library::Container::Yield::NO_YIELD; + } else if (Token::Match(tok->previous(), "%name% (")) { + if (parent) + *parent = tok; + if (const Library::Function* f = settings->library.getFunction(tok->previous())) { + return f->containerYield; + } + } + return Library::Container::Yield::NO_YIELD; +} + /** Set token value for cast */ static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings); @@ -663,36 +681,25 @@ static void setTokenValue(Token* tok, } } } - else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) && - parent->astOperand1() && parent->astOperand1()->valueType()) { - const Library::Container* c = getLibraryContainer(parent->astOperand1()); - const Library::Container::Yield yields = c ? c->getYield(parent->strAt(1)) : Library::Container::Yield::NO_YIELD; - if (yields == Library::Container::Yield::SIZE) { - ValueFlow::Value v(value); - v.valueType = ValueFlow::Value::ValueType::INT; - setTokenValue(parent->astParent(), std::move(v), settings); - } else if (yields == Library::Container::Yield::EMPTY) { - ValueFlow::Value v(value); - v.valueType = ValueFlow::Value::ValueType::INT; - if (value.isImpossible() && value.intvalue == 0) + Token* next = nullptr; + const Library::Container::Yield yields = getContainerYield(parent, settings, &next); + if (yields == Library::Container::Yield::SIZE) { + ValueFlow::Value v(value); + v.valueType = ValueFlow::Value::ValueType::INT; + setTokenValue(next, std::move(v), settings); + } else if (yields == Library::Container::Yield::EMPTY) { + ValueFlow::Value v(value); + v.valueType = ValueFlow::Value::ValueType::INT; + v.bound = ValueFlow::Value::Bound::Point; + if (value.isImpossible()) { + if (value.intvalue == 0) v.setKnown(); else - v.intvalue = !v.intvalue; - setTokenValue(parent->astParent(), std::move(v), settings); - } - } else if (Token::Match(parent->previous(), "%name% (")) { - if (const Library::Function* f = settings->library.getFunction(parent->previous())) { - if (f->containerYield == Library::Container::Yield::SIZE) { - ValueFlow::Value v(value); - v.valueType = ValueFlow::Value::ValueType::INT; - setTokenValue(parent, std::move(v), settings); - } else if (f->containerYield == Library::Container::Yield::EMPTY) { - ValueFlow::Value v(value); - v.intvalue = !v.intvalue; - v.valueType = ValueFlow::Value::ValueType::INT; - setTokenValue(parent, std::move(v), settings); - } + v.setPossible(); + } else { + v.intvalue = !v.intvalue; } + setTokenValue(next, std::move(v), settings); } return; } @@ -917,7 +924,10 @@ static void setTokenValue(Token* tok, if (val.isImpossible() && val.intvalue != 0) continue; ValueFlow::Value v(val); - v.intvalue = !v.intvalue; + if (val.isImpossible()) + v.setKnown(); + else + v.intvalue = !v.intvalue; setTokenValue(parent, std::move(v), settings); } } @@ -1859,7 +1869,7 @@ static void valueFlowImpossibleValues(TokenList* tokenList, const Settings* sett ValueFlow::Value value{0}; value.setImpossible(); setTokenValue(tok->linkAt(1)->next(), std::move(value), settings); - } else if (tokenList->isCPP() && Token::simpleMatch(tok, "this")) { + } else if ((tokenList->isCPP() && Token::simpleMatch(tok, "this")) || tok->isUnaryOp("&")) { ValueFlow::Value value{0}; value.setImpossible(); setTokenValue(tok, std::move(value), settings); @@ -6666,16 +6676,7 @@ static void valueFlowInferCondition(TokenList* tokenlist, continue; if (tok->hasKnownIntValue()) continue; - if (tok->variable() && (Token::Match(tok->astParent(), "?|&&|!|%oror%") || - Token::Match(tok->astParent()->previous(), "if|while ("))) { - std::vector result = infer(IntegralInferModel{}, "!=", tok->values(), 0); - if (result.size() != 1) - continue; - ValueFlow::Value value = result.front(); - value.intvalue = 1; - value.bound = ValueFlow::Value::Bound::Point; - setTokenValue(tok, std::move(value), settings); - } else if (Token::Match(tok, "%comp%|-") && tok->astOperand1() && tok->astOperand2()) { + if (Token::Match(tok, "%comp%|-") && tok->astOperand1() && tok->astOperand2()) { if (astIsIterator(tok->astOperand1()) || astIsIterator(tok->astOperand2())) { static const std::array, 2> iteratorModels = {EndIteratorInferModel{}, StartIteratorInferModel{}}; @@ -6694,6 +6695,15 @@ static void valueFlowInferCondition(TokenList* tokenlist, setTokenValue(tok, std::move(value), settings); } } + } else if (Token::Match(tok->astParent(), "?|&&|!|%oror%") || + Token::Match(tok->astParent()->previous(), "if|while (")) { + std::vector result = infer(IntegralInferModel{}, "!=", tok->values(), 0); + if (result.size() != 1) + continue; + ValueFlow::Value value = result.front(); + value.intvalue = 1; + value.bound = ValueFlow::Value::Bound::Point; + setTokenValue(tok, std::move(value), settings); } } } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index bfe80b7e9..8b64d8d4e 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -353,11 +353,14 @@ private: ASSERT_EQUALS("", errout.str()); // no crash on unary operator& (#5643) + // #11610 check("SdrObject* ApplyGraphicToObject() {\n" " if (&rHitObject) {}\n" " else if (rHitObject.IsClosedObj() && !&rHitObject) { }\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Condition '&rHitObject' is always true\n" + "[test.cpp:3]: (style) Condition '!&rHitObject' is always false\n", + errout.str()); // #5695: increment check("void f(int a0, int n) {\n" @@ -3800,6 +3803,7 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + // TODO: if (!v) is a known condition as well check("struct a {\n" " int *b();\n" "};\n" @@ -3813,7 +3817,7 @@ private: " if (v == nullptr && e) {}\n" " return d;\n" "}\n"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:11]: (style) Condition 'e' is always true\n", errout.str()); // #10037 check("struct a {\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 82d1735d6..26f828d13 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1072,6 +1072,13 @@ private: "}"; ASSERT_EQUALS(false, testValueOfX(code, 3U, 0)); + code = "void f(int i) {\n" + " int * p = &i;\n" + " bool x = !p || i;\n" + " bool a = x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 1)); + code = "bool f(const uint16_t * const p) {\n" " const uint8_t x = (uint8_t)(*p & 0x01E0U) >> 5U;\n" " return x != 0;\n"