From 9d52861b8cd2632ff47e7fd59910f6936fcbfd34 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 3 Jan 2022 12:23:22 -0600 Subject: [PATCH 1/2] Revert "exprengine; add CONTRACT #define so contract-handling can be enabled/disabled" This reverts commit 33446d0c75bc351efbacdf5c413e3a9fd2a29e3e. --- lib/exprengine.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/exprengine.cpp b/lib/exprengine.cpp index 364a3f5cd74..f35727771b5 100644 --- a/lib/exprengine.cpp +++ b/lib/exprengine.cpp @@ -151,7 +151,6 @@ #endif constexpr auto MAX_BUFFER_SIZE = ~0UL; -#define CONTRACT 1 namespace { struct ExprEngineException { @@ -413,7 +412,6 @@ namespace { return tokenizer->isCPP(); } -#ifdef CONTRACT ExprEngine::ValuePtr executeContract(const Function *function, ExprEngine::ValuePtr (*executeExpression)(const Token*, Data&)) { const auto it = settings->functionContracts.find(function->fullName()); if (it == settings->functionContracts.end()) @@ -441,7 +439,6 @@ namespace { if (value) constraints.push_back(value); } -#endif void assignValue(const Token *tok, unsigned int varId, ExprEngine::ValuePtr value) { if (varId == 0) @@ -620,7 +617,6 @@ namespace { } void addConstraints(ExprEngine::ValuePtr value, const Token *tok) { -#ifdef CONTRACT MathLib::bigint low; if (tok->getCppcheckAttribute(TokenImpl::CppcheckAttributes::Type::LOW, &low)) addConstraint(std::make_shared(">=", value, std::make_shared(std::to_string(low), low, low)), true); @@ -628,7 +624,6 @@ namespace { MathLib::bigint high; if (tok->getCppcheckAttribute(TokenImpl::CppcheckAttributes::Type::HIGH, &high)) addConstraint(std::make_shared("<=", value, std::make_shared(std::to_string(high), high, high)), true); -#endif } void reportError(const Token *tok, @@ -1973,7 +1968,6 @@ static ExprEngine::ValuePtr executeIncDec(const Token *tok, Data &data) #ifdef USE_Z3 static void checkContract(Data &data, const Token *tok, const Function *function, const std::vector &argValues) { -#ifdef CONTRACT ExprData exprData; z3::solver solver(exprData.context); try { @@ -2043,7 +2037,6 @@ static void checkContract(Data &data, const Token *tok, const Function *function true, functionName); } -#endif } #endif @@ -2103,7 +2096,6 @@ static ExprEngine::ValuePtr executeFunctionCall(const Token *tok, Data &data) if (tok->astOperand1()->function()) { const Function *function = tok->astOperand1()->function(); const std::string &functionName = function->fullName(); -#ifdef CONTRACT const auto contractIt = data.settings->functionContracts.find(functionName); if (contractIt != data.settings->functionContracts.end()) { #ifdef USE_Z3 @@ -2116,7 +2108,6 @@ static ExprEngine::ValuePtr executeFunctionCall(const Token *tok, Data &data) if (!bailout) data.addMissingContract(functionName); } -#endif // Execute subfunction.. if (hasBody) { @@ -3005,9 +2996,7 @@ void ExprEngine::executeFunction(const Scope *functionScope, ErrorLogger *errorL for (const Variable &arg : function->argumentList) data.assignValue(functionScope->bodyStart, arg.declarationId(), createVariableValue(arg, data)); -#ifdef CONTRACT data.contractConstraints(function, executeExpression1); -#endif const std::time_t stopTime = data.startTime + data.settings->bugHuntingCheckFunctionMaxTime; From 5811b0e29fc6feeeaf7442f983af03dffdb52199 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 3 Jan 2022 12:23:28 -0600 Subject: [PATCH 2/2] Revert "exprengine: better checking for uninit variables" This reverts commit 33305ef4ec7ed5bcde52640942035721576034c3. --- lib/bughuntingchecks.cpp | 10 +++---- lib/exprengine.cpp | 63 +++++++--------------------------------- lib/exprengine.h | 14 ++++----- test/testexprengine.cpp | 20 ++++++------- 4 files changed, 33 insertions(+), 74 deletions(-) diff --git a/lib/bughuntingchecks.cpp b/lib/bughuntingchecks.cpp index aec4616bc23..cc29463e6ec 100644 --- a/lib/bughuntingchecks.cpp +++ b/lib/bughuntingchecks.cpp @@ -199,7 +199,7 @@ static void divByZero(const Token *tok, const ExprEngine::Value &value, ExprEngi return; if (tok->isImpossibleIntValue(0)) return; - if (value.isUninit(dataBase) && value.type != ExprEngine::ValueType::BailoutValue) + if (value.isUninit() && value.type != ExprEngine::ValueType::BailoutValue) return; float f = getKnownFloatValue(tok, 0.0f); if (f > 0.0f || f < 0.0f) @@ -315,7 +315,7 @@ static void uninit(const Token *tok, const ExprEngine::Value &value, ExprEngine: std::string uninitStructMember; if (const auto* structValue = dynamic_cast(&value)) { - uninitStructMember = structValue->getUninitStructMember(dataBase); + uninitStructMember = structValue->getUninitStructMember(); // uninitialized struct member => is there data copy of struct.. if (!uninitStructMember.empty()) { @@ -325,10 +325,10 @@ static void uninit(const Token *tok, const ExprEngine::Value &value, ExprEngine: } bool uninitData = false; - if (!value.isUninit(dataBase) && uninitStructMember.empty()) { + if (!value.isUninit() && uninitStructMember.empty()) { if (Token::Match(tok->astParent(), "[(,]")) { if (const auto* arrayValue = dynamic_cast(&value)) { - uninitData = arrayValue->data.size() >= 1 && arrayValue->data[0].value->isUninit(dataBase); + uninitData = arrayValue->data.size() >= 1 && arrayValue->data[0].value->isUninit(); } } @@ -627,7 +627,7 @@ static void checkFunctionCall(const Token *tok, const ExprEngine::Value &value, const ExprEngine::ArrayValue &arrayValue = static_cast(value); auto index0 = std::make_shared("0", 0, 0); for (const auto &v: arrayValue.read(index0)) { - if (v.second->isUninit(dataBase)) { + if (v.second->isUninit()) { dataBase->reportError(tok, Severity::SeverityType::error, "bughuntingUninitArg", "There is function call, cannot determine that " + std::to_string(num) + getOrdinalText(num) + " argument is initialized.", CWE_USE_OF_UNINITIALIZED_VARIABLE, false); break; } diff --git a/lib/exprengine.cpp b/lib/exprengine.cpp index f35727771b5..4fd70bddc0b 100644 --- a/lib/exprengine.cpp +++ b/lib/exprengine.cpp @@ -150,8 +150,6 @@ #define Z3_VERSION_INT GET_VERSION_INT(Z3_MAJOR_VERSION, Z3_MINOR_VERSION, Z3_BUILD_NUMBER) #endif -constexpr auto MAX_BUFFER_SIZE = ~0UL; - namespace { struct ExprEngineException { ExprEngineException(const Token *tok, const std::string &what) : tok(tok), what(what) {} @@ -203,7 +201,9 @@ static std::string str(ExprEngine::ValuePtr val) break; } - return val->name + "=" + std::string(typestr) + "(" + val->getRange() + ")"; + std::ostringstream ret; + ret << val->name << "=" << typestr << "(" << val->getRange() << ")"; + return ret.str(); } static size_t extfind(const std::string &str, const std::string &what, size_t pos) @@ -341,7 +341,7 @@ namespace { } void ifSplit(const Token *tok, unsigned int thenIndex, unsigned int elseIndex) { - mMap[tok].push_back("D" + std::to_string(thenIndex) + ": Split. Then:D" + std::to_string(thenIndex) + " Else:D" + std::to_string(elseIndex)); + mMap[tok].push_back(std::to_string(thenIndex) + ": Split. Then:" + std::to_string(thenIndex) + " Else:" + std::to_string(elseIndex)); } private: @@ -536,7 +536,7 @@ namespace { return; const SymbolDatabase * const symbolDatabase = tokenizer->getSymbolDatabase(); std::ostringstream s; - s << "D" << mDataIndex << ":" << "memory:{"; + s << mDataIndex << ":" << "memory:{"; bool first = true; for (auto mem : memory) { ExprEngine::ValuePtr value = mem.second; @@ -1305,32 +1305,6 @@ class ExprData { }; #endif -bool ExprEngine::UninitValue::isUninit(const DataBase *dataBase) const { - const Data *data = dynamic_cast(dataBase); - if (data->constraints.empty()) - return true; -#ifdef USE_Z3 - // Check the value against the constraints - ExprData exprData; - z3::solver solver(exprData.context); - try { - exprData.addConstraints(solver, data); - exprData.addAssertions(solver); - return solver.check() == z3::sat; - } catch (const z3::exception &exception) { - std::cerr << "z3: " << exception << std::endl; - return true; // Safe option is to return true - } catch (const ExprData::BailoutValueException &) { - return true; // Safe option is to return true - } catch (const ExprEngineException &) { - return true; // Safe option is to return true - } -#else - // The value may or may not be uninitialized - return false; -#endif -} - bool ExprEngine::IntRange::isEqual(const DataBase *dataBase, int value) const { if (value < minValue || value > maxValue) @@ -2206,7 +2180,7 @@ static ExprEngine::ValuePtr executeCast(const Token *tok, Data &data) uninitPointer = std::static_pointer_cast(val)->uninitPointer; } - auto bufferSize = std::make_shared(data.getNewSymbolName(), 1, MAX_BUFFER_SIZE); + auto bufferSize = std::make_shared(data.getNewSymbolName(), 1, ~0UL); return std::make_shared(data.getNewSymbolName(), bufferSize, range, true, nullPointer, uninitPointer); } @@ -2267,11 +2241,6 @@ static void streamReadSetValue(const Token *tok, Data &data) { if (!tok || !tok->valueType()) return; - if (tok->varId() > 0 && tok->valueType()->pointer) { - const auto oldValue = data.getValue(tok->varId(), tok->valueType(), tok); - if (oldValue && (oldValue->isUninit(&data))) - call(data.callbacks, tok, oldValue, &data); - } auto rangeValue = getValueRangeFromValueType(tok->valueType(), data); if (rangeValue) assignExprValue(tok, rangeValue, data); @@ -2394,8 +2363,7 @@ static ExprEngine::ValuePtr executeVariable(const Token *tok, Data &data) static ExprEngine::ValuePtr executeKnownMacro(const Token *tok, Data &data) { - const auto intval = tok->getKnownIntValue(); - auto val = std::make_shared(std::to_string(intval), intval, intval); + auto val = std::make_shared(data.getNewSymbolName(), tok->getKnownIntValue(), tok->getKnownIntValue()); call(data.callbacks, tok, val, &data); return val; } @@ -2540,9 +2508,6 @@ static std::string execute(const Token *start, const Token *end, Data &data) if (Token::Match(prev, "[;{}] return|throw")) return data.str(); } - while (Token::simpleMatch(tok, "} catch (") && Token::simpleMatch(tok->linkAt(2), ") {")) { - tok = tok->linkAt(2)->next()->link(); - } if (std::time(nullptr) > stopTime) return ""; } @@ -2565,15 +2530,9 @@ static std::string execute(const Token *start, const Token *end, Data &data) return data.str(); } - if (Token::simpleMatch(tok, "try {") && Token::simpleMatch(tok->linkAt(1), "} catch (")) { - const Token *catchTok = tok->linkAt(1); - while (Token::simpleMatch(catchTok, "} catch (")) { - Data catchData(data); - catchTok = catchTok->linkAt(2)->next(); - execute(catchTok, end, catchData); - catchTok = catchTok->link(); - } - } + if (Token::simpleMatch(tok, "try")) + // TODO this is a bailout + throw ExprEngineException(tok, "Unhandled:" + tok->str()); // Variable declaration.. if (tok->variable() && tok->variable()->nameToken() == tok) { @@ -2812,7 +2771,7 @@ static std::string execute(const Token *start, const Token *end, Data &data) continue; changedVariables.insert(varid); auto oldValue = bodyData.getValue(varid, nullptr, nullptr); - if (oldValue && oldValue->isUninit(&bodyData)) + if (oldValue && oldValue->isUninit()) call(bodyData.callbacks, lhs, oldValue, &bodyData); if (oldValue && oldValue->type == ExprEngine::ValueType::ArrayValue) { // Try to assign "any" value diff --git a/lib/exprengine.h b/lib/exprengine.h index 5bdc0429cc3..600d3ef52bd 100644 --- a/lib/exprengine.h +++ b/lib/exprengine.h @@ -116,8 +116,7 @@ namespace ExprEngine { (void)value; return false; } - virtual bool isUninit(const DataBase *dataBase) const { - (void)dataBase; + virtual bool isUninit() const { return false; } @@ -133,7 +132,9 @@ namespace ExprEngine { (void)value; return true; } - bool isUninit(const DataBase *dataBase) const OVERRIDE; + bool isUninit() const OVERRIDE { + return true; + } }; class IntRange : public Value { @@ -244,9 +245,9 @@ namespace ExprEngine { return (it == member.end()) ? ValuePtr() : it->second; } - std::string getUninitStructMember(const DataBase *dataBase) const { + std::string getUninitStructMember() const { for (auto memberNameValue: member) { - if (memberNameValue.second && memberNameValue.second->isUninit(dataBase)) + if (memberNameValue.second && memberNameValue.second->isUninit()) return memberNameValue.first; } return std::string(); @@ -326,8 +327,7 @@ namespace ExprEngine { bool isEqual(const DataBase * /*dataBase*/, int /*value*/) const OVERRIDE { return true; } - bool isUninit(const DataBase *dataBase) const OVERRIDE { - (void)dataBase; + bool isUninit() const OVERRIDE { return true; } }; diff --git a/test/testexprengine.cpp b/test/testexprengine.cpp index 9b4d01e1778..c4197c78370 100644 --- a/test/testexprengine.cpp +++ b/test/testexprengine.cpp @@ -359,14 +359,14 @@ class TestExprEngine : public TestFixture { ASSERT_EQUALS("1:26: $4=ArrayValue([$3],[:]=$2)\n" "1:26: $3=IntRange(0:2147483647)\n" "1:26: $2=IntRange(-128:127)\n" - "1:27: D0:memory:{s=($4,[$3],[:]=$2)}\n", + "1:27: 0:memory:{s=($4,[$3],[:]=$2)}\n", trackExecution("void foo() { std::string s; }", &settings)); ASSERT_EQUALS("1:52: $4=ArrayValue([$3],[:]=$2)\n" "1:52: $3=IntRange(0:2147483647)\n" "1:52: $2=IntRange(-128:127)\n" - "1:66: D0:memory:{s=($4,[$3],[:]=$2)}\n", + "1:66: 0:memory:{s=($4,[$3],[:]=$2)}\n", trackExecution("std::string getName(int); void foo() { std::string s = getName(1); }", &settings)); } @@ -780,7 +780,7 @@ class TestExprEngine : public TestFixture { ASSERT_EQUALS("2:16: $2:0=IntRange(-2147483648:2147483647)\n" "2:20: $1=ArrayValue([10],[:]=$2)\n" "2:20: $2=IntRange(-2147483648:2147483647)\n" - "2:26: D0:memory:{buf=($1,[10],[:]=$2) x=$2:0}\n", + "2:26: 0:memory:{buf=($1,[10],[:]=$2) x=$2:0}\n", trackExecution(code)); } @@ -791,10 +791,10 @@ class TestExprEngine : public TestFixture { " return buf[0][1][2];\n" "}"; ASSERT_EQUALS("1:14: $1=IntRange(-2147483648:2147483647)\n" - "1:14: D0:memory:{x=$1}\n" + "1:14: 0:memory:{x=$1}\n" "2:7: $2=ArrayValue([3][4][5],[:]=?)\n" - "2:19: D0:memory:{x=$1 buf=($2,[3][4][5],[:]=?)}\n" - "3:20: D0:memory:{x=$1 buf=($2,[3][4][5],[:]=?,[((20)*($1))+(7)]=10)}\n", + "2:19: 0:memory:{x=$1 buf=($2,[3][4][5],[:]=?)}\n" + "3:20: 0:memory:{x=$1 buf=($2,[3][4][5],[:]=?,[((20)*($1))+(7)]=10)}\n", trackExecution(code)); } @@ -815,9 +815,9 @@ class TestExprEngine : public TestFixture { "}"; ASSERT_EQUALS("1:28: $2=ArrayValue([$1],[:]=?,null)\n" "1:28: $1=IntRange(1:ffffffffffffffff)\n" - "1:28: D0:memory:{x=($2,[$1],[:]=?)}\n" - "2:9: D0:memory:{x=($2,[$1],[:]=?,[0]=2)}\n" - "3:9: D0:memory:{x=($2,[$1],[:]=?,[0]=1)}\n", + "1:28: 0:memory:{x=($2,[$1],[:]=?)}\n" + "2:9: 0:memory:{x=($2,[$1],[:]=?,[0]=2)}\n" + "3:9: 0:memory:{x=($2,[$1],[:]=?,[0]=1)}\n", trackExecution(code)); } @@ -903,7 +903,7 @@ class TestExprEngine : public TestFixture { ASSERT_EQUALS("1:36: $3=ArrayValue([$2],[:]=bailout,null)\n" "1:36: $2=IntRange(1:2147483647)\n" "1:36: bailout=BailoutValue(bailout)\n" - "1:46: D0:memory:{p=($3,[$2],[:]=bailout)}\n", + "1:46: 0:memory:{p=($3,[$2],[:]=bailout)}\n", trackExecution("char *foo(int); void bar() { char *p = foo(1); }")); }