Skip to content

Commit

Permalink
Merge branch 'chr_12858' of https://github.com/chrchr-github/cppcheck
Browse files Browse the repository at this point in the history
…into chr_12858
  • Loading branch information
chrchr-github committed Jun 29, 2024
2 parents 98fe91f + 1d8d90a commit 5665817
Show file tree
Hide file tree
Showing 11 changed files with 183 additions and 22 deletions.
15 changes: 15 additions & 0 deletions cfg/windows.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -6775,6 +6775,21 @@ HFONT CreateFont(
<not-bool/>
</arg>
</function>
<!-- ULONG UpdateTraceA(CONTROLTRACE_ID TraceId, LPCSTR InstanceName, PEVENT_TRACE_PROPERTIES Properties); -->
<!-- ULONG UpdateTraceW(CONTROLTRACE_ID TraceId, LPCWSTR InstanceName, PEVENT_TRACE_PROPERTIES Properties); -->
<function name="UpdateTraceA,UpdateTraceW">
<noreturn>false</noreturn>
<returnValue type="ULONG"/>
<use-retval/>
<warn severity="style" alternatives="ControlTrace" reason="Obsolete">This function is obsolete. The ControlTrace function supersedes this function.</warn>
<arg nr="1" direction="in">
<not-uninit/>
</arg>
<arg nr="2" direction="in">
<not-uninit/>
</arg>
<arg nr="3" direction="out"/>
</function>
<!-- BOOL DeviceIoControl(
_In_ HANDLE hDevice,
_In_ DWORD dwIoControlCode,
Expand Down
25 changes: 22 additions & 3 deletions lib/checkother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,10 @@ void CheckOther::checkRedundantAssignment()
tokenToCheck = tempToken;
}

if (start->hasKnownSymbolicValue(tokenToCheck) && Token::simpleMatch(start->astParent(), "=") && !diag(tok)) {
redundantAssignmentSameValueError(start, tokenToCheck, tok->astOperand1()->expressionString());
}

// Get next assignment..
const Token *nextAssign = fwdAnalysis.reassign(tokenToCheck, start, scope->bodyEnd);

Expand All @@ -541,8 +545,10 @@ void CheckOther::checkRedundantAssignment()
redundantAssignmentInSwitchError(tok, nextAssign, tok->astOperand1()->expressionString());
else if (isInitialization)
redundantInitializationError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
else
else {
diag(nextAssign);
redundantAssignmentError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
}
}
}
}
Expand Down Expand Up @@ -587,6 +593,16 @@ void CheckOther::redundantAssignmentInSwitchError(const Token *tok1, const Token
"Variable '$symbol' is reassigned a value before the old one has been used. 'break;' missing?", CWE563, Certainty::normal);
}

void CheckOther::redundantAssignmentSameValueError(const Token *tok1, const Token* tok2, const std::string &var)
{
const ValueFlow::Value* val = tok1->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC);
auto errorPath = val->errorPath;
errorPath.emplace_back(tok2, "");
reportError(errorPath, Severity::style, "redundantAssignment",
"$symbol:" + var + "\n"
"Variable '$symbol' is assigned an expression that holds the same value.", CWE563, Certainty::normal);
}


//---------------------------------------------------------------------------
// switch (x)
Expand Down Expand Up @@ -2485,7 +2501,8 @@ void CheckOther::checkDuplicateExpression()
tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString()) {
bool differentDomain = false;
const Scope * varScope = var1->scope() ? var1->scope() : scope;
for (const Token *assignTok = Token::findsimplematch(var2, ";"); assignTok && assignTok != varScope->bodyEnd; assignTok = assignTok->next()) {
const Token* assignTok = Token::findsimplematch(var2, ";");
for (; assignTok && assignTok != varScope->bodyEnd; assignTok = assignTok->next()) {
if (!Token::Match(assignTok, "%assign%|%comp%"))
continue;
if (!assignTok->astOperand1())
Expand Down Expand Up @@ -2516,8 +2533,10 @@ void CheckOther::checkDuplicateExpression()
}
if (!differentDomain && !isUniqueExpression(tok->astOperand2()))
duplicateAssignExpressionError(var1, var2, false);
else if (mSettings->certainty.isEnabled(Certainty::inconclusive))
else if (mSettings->certainty.isEnabled(Certainty::inconclusive)) {
diag(assignTok);
duplicateAssignExpressionError(var1, var2, true);
}
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion lib/checkother.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "errortypes.h"
#include "tokenize.h"

#include <set>
#include <string>
#include <vector>

Expand Down Expand Up @@ -75,11 +76,11 @@ class CPPCHECKLIB CheckOther : public Check {
checkOther.warningOldStylePointerCast();
checkOther.invalidPointerCast();
checkOther.checkCharVariable();
checkOther.checkRedundantAssignment();
checkOther.redundantBitwiseOperationInSwitchError();
checkOther.checkSuspiciousCaseInSwitch();
checkOther.checkDuplicateBranch();
checkOther.checkDuplicateExpression();
checkOther.checkRedundantAssignment();
checkOther.checkUnreachableCode();
checkOther.checkSuspiciousSemicolon();
checkOther.checkVariableScope();
Expand Down Expand Up @@ -253,6 +254,7 @@ class CPPCHECKLIB CheckOther : public Check {
void redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive);
void redundantInitializationError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive);
void redundantAssignmentInSwitchError(const Token *tok1, const Token *tok2, const std::string &var);
void redundantAssignmentSameValueError(const Token *tok1, const Token *tok2, const std::string &var);
void redundantCopyError(const Token *tok1, const Token* tok2, const std::string& var);
void redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname);
void suspiciousCaseInSwitchError(const Token* tok, const std::string& operatorString);
Expand Down Expand Up @@ -426,6 +428,11 @@ class CPPCHECKLIB CheckOther : public Check {
"- calculating modulo of one.\n"
"- known function argument, suspicious calculation.\n";
}

bool diag(const Token* tok) {
return !mRedundantAssignmentDiag.emplace(tok).second;
}
std::set<const Token*> mRedundantAssignmentDiag;
};
/// @}
//---------------------------------------------------------------------------
Expand Down
14 changes: 2 additions & 12 deletions lib/valueflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,16 +411,6 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val
result.path = value1.path;
}

static long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign)
{
const MathLib::biguint unsignedMaxValue = (1ULL << (value_size * 8)) - 1ULL;
const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1);
value &= unsignedMaxValue;
if (dst_sign == ValueType::Sign::SIGNED && (value & signBit))
value |= ~unsignedMaxValue;

return value;
}

template<class F>
static size_t accumulateStructMembers(const Scope* scope, F f)
Expand Down Expand Up @@ -1879,7 +1869,7 @@ struct ValueFlowAnalyzer : Analyzer {
if (dst) {
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
if (sz > 0 && sz < sizeof(MathLib::biguint)) {
long long newvalue = truncateIntValue(value->intvalue, sz, dst->sign);
long long newvalue = ValueFlow::truncateIntValue(value->intvalue, sz, dst->sign);

/* Handle overflow/underflow for value bounds */
if (value->bound != ValueFlow::Value::Bound::Point) {
Expand Down Expand Up @@ -5105,7 +5095,7 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> va
}

if (value.isIntValue() && sz > 0 && sz < sizeof(MathLib::biguint))
value.intvalue = truncateIntValue(value.intvalue, sz, dst->sign);
value.intvalue = ValueFlow::truncateIntValue(value.intvalue, sz, dst->sign);
}
return values;
}
Expand Down
14 changes: 14 additions & 0 deletions lib/vf_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,20 @@ namespace ValueFlow
return true;
}

long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign)
{
if (value_size == 0)
return value;

const MathLib::biguint unsignedMaxValue = std::numeric_limits<MathLib::biguint>::max() >> ((sizeof(unsignedMaxValue) - value_size) * 8);
const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1);
value &= unsignedMaxValue;
if (dst_sign == ValueType::Sign::SIGNED && (value & signBit))
value |= ~unsignedMaxValue;

return value;
}

static nonneg int getSizeOfType(const Token *typeTok, const Settings &settings)
{
const ValueType &valueType = ValueType::parseDecl(typeTok, settings);
Expand Down
2 changes: 2 additions & 0 deletions lib/vf_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ namespace ValueFlow
{
bool getMinMaxValues(const ValueType* vt, const Platform& platform, MathLib::bigint& minValue, MathLib::bigint& maxValue);

long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign);

Token * valueFlowSetConstantValue(Token *tok, const Settings &settings);

Value castValue(Value value, const ValueType::Sign sign, nonneg int bit);
Expand Down
36 changes: 32 additions & 4 deletions lib/vf_settokenvalue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,22 @@ namespace ValueFlow
continue;
Value v(val);
if (parent == tok->previous()) {
if (v.isIntValue() || v.isSymbolicValue())
v.intvalue = v.intvalue + 1;
if (v.isIntValue() || v.isSymbolicValue()) {
const ValueType *dst = tok->valueType();
if (dst) {
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
long long newvalue = ValueFlow::truncateIntValue(v.intvalue + 1, sz, dst->sign);
if (v.bound != ValueFlow::Value::Bound::Point) {
if (newvalue < v.intvalue) {
v.invertBound();
newvalue -= 2;
}
}
v.intvalue = newvalue;
} else {
v.intvalue = v.intvalue + 1;
}
}
else
v.floatValue = v.floatValue + 1.0;
}
Expand All @@ -656,8 +670,22 @@ namespace ValueFlow
continue;
Value v(val);
if (parent == tok->previous()) {
if (v.isIntValue() || v.isSymbolicValue())
v.intvalue = v.intvalue - 1;
if (v.isIntValue() || v.isSymbolicValue()) {
const ValueType *dst = tok->valueType();
if (dst) {
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
long long newvalue = ValueFlow::truncateIntValue(v.intvalue - 1, sz, dst->sign);
if (v.bound != ValueFlow::Value::Bound::Point) {
if (newvalue > v.intvalue) {
v.invertBound();
newvalue += 2;
}
}
v.intvalue = newvalue;
} else {
v.intvalue = v.intvalue - 1;
}
}
else
v.floatValue = v.floatValue - 1.0;
}
Expand Down
12 changes: 12 additions & 0 deletions test/cfg/windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <WinCon.h>
#include <cstdio>
#include <direct.h>
#include <evntrace.h>
#include <cstdlib>
#include <ctime>
#include <memory.h>
Expand All @@ -22,6 +23,17 @@
#include <atlstr.h>
#include <string>

bool UpdateTraceACalled(TRACEHANDLE traceHandle, LPCSTR loggerName, EVENT_TRACE_PROPERTIES* pProperties)
{
// cppcheck-suppress UpdateTraceACalled
return UpdateTraceA(traceHandle, loggerName, pProperties) != ERROR_SUCCESS;
}
bool UpdateTraceWCalled(TRACEHANDLE traceHandle, LPCWSTR loggerName, EVENT_TRACE_PROPERTIES* pProperties)
{
// cppcheck-suppress UpdateTraceWCalled
return UpdateTraceW(traceHandle, loggerName, pProperties) != ERROR_SUCCESS;
}

void invalidHandle_CreateFile(LPCWSTR lpFileName)
{
HANDLE file = CreateFile(lpFileName, GENERIC_READ, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
Expand Down
15 changes: 15 additions & 0 deletions test/testcondition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class TestCondition : public TestFixture {
TEST_CASE(knownConditionCast); // #9976
TEST_CASE(knownConditionIncrementLoop); // #9808
TEST_CASE(knownConditionAfterBailout); // #12526
TEST_CASE(knownConditionIncDecOperator);
}

#define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
Expand Down Expand Up @@ -6097,6 +6098,20 @@ class TestCondition : public TestFixture {
);
ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:18]: (style) Condition 'mS.b' is always false\n", errout_str());
}

void knownConditionIncDecOperator() {
check(
"void f() {\n"
" unsigned int d = 0;\n"
" for (int i = 0; i < 4; ++i) {\n"
" if (i < 3)\n"
" ++d;\n"
" else if (--d == 0)\n"
" ;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout_str());
}
};

REGISTER_TEST(TestCondition)
41 changes: 41 additions & 0 deletions test/testother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class TestOther : public TestFixture {
TEST_CASE(zeroDiv17); // #9931
TEST_CASE(zeroDiv18);
TEST_CASE(zeroDiv19);
TEST_CASE(zeroDiv20); // #11175

TEST_CASE(zeroDivCond); // division by zero / useless condition

Expand Down Expand Up @@ -214,6 +215,7 @@ class TestOther : public TestFixture {
TEST_CASE(redundantVarAssignment_switch_break);
TEST_CASE(redundantInitialization);
TEST_CASE(redundantMemWrite);
TEST_CASE(redundantAssignmentSameValue);

TEST_CASE(varFuncNullUB);

Expand Down Expand Up @@ -661,6 +663,16 @@ class TestOther : public TestFixture {
ASSERT_EQUALS("[test.cpp:3]: (error) Division by zero.\n", errout_str());
}

void zeroDiv20()
{
check("uint16_t f(void)\n" // #11175
"{\n"
" uint16_t x = 0xFFFFU;\n" // UINT16_MAX=0xFFFF
" return 42/(++x);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Division by zero.\n", errout_str());
}

void zeroDivCond() {
check("void f(unsigned int x) {\n"
" int y = 17 / x;\n"
Expand Down Expand Up @@ -10178,6 +10190,35 @@ class TestOther : public TestFixture {
TODO_ASSERT_EQUALS("error", "", errout_str());
}

void redundantAssignmentSameValue() {
check("int main() {\n" // #11642
" int a = 0;\n"
" int b = a;\n"
" int c = 1;\n"
" a = b;\n"
" return a * b * c;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Variable 'a' is assigned an expression that holds the same value.\n", errout_str());

check("int main() {\n"
" int a = 0;\n"
" int b = a;\n"
" int c = 1;\n"
" a = b + 1;\n"
" return a * b * c;\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("int main() {\n"
" int a = 0;\n"
" int b = a;\n"
" int c = 1;\n"
" a = b = 5;\n"
" return a * b * c;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Redundant initialization for 'b'. The initialized value is overwritten before it is read.\n", errout_str());
}

void varFuncNullUB() { // #4482
check("void a(...);\n"
"void b() { a(NULL); }");
Expand Down
Loading

0 comments on commit 5665817

Please sign in to comment.