From c85f889bf2042fc53c7fa78ff407f0b9783ea305 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Berder?=
 <18538310+francois-berder@users.noreply.github.com>
Date: Wed, 26 Jun 2024 18:01:41 +0200
Subject: [PATCH 1/4] Truncate value of increment operator (#6342)

This PR fixes a similar bug than
[#11591](https://trac.cppcheck.net/ticket/11591) fixed in PR
https://github.com/danmar/cppcheck/pull/6331.

---------

Signed-off-by: Francois Berder <fberder@outlook.fr>
---
 lib/valueflow.cpp        | 14 ++------------
 lib/vf_common.cpp        | 14 ++++++++++++++
 lib/vf_common.h          |  2 ++
 lib/vf_settokenvalue.cpp | 36 ++++++++++++++++++++++++++++++++----
 test/testcondition.cpp   | 15 +++++++++++++++
 test/testvalueflow.cpp   | 22 ++++++++++++++++++++--
 6 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index fe6b4d045d4..0bd0b70d16f 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -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)
@@ -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) {
@@ -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;
 }
diff --git a/lib/vf_common.cpp b/lib/vf_common.cpp
index 7505e89c36d..ae1f4a43d8f 100644
--- a/lib/vf_common.cpp
+++ b/lib/vf_common.cpp
@@ -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);
diff --git a/lib/vf_common.h b/lib/vf_common.h
index 050b9960969..d67dfc6e722 100644
--- a/lib/vf_common.h
+++ b/lib/vf_common.h
@@ -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);
diff --git a/lib/vf_settokenvalue.cpp b/lib/vf_settokenvalue.cpp
index b869c5427fa..4bd7bfd8f37 100644
--- a/lib/vf_settokenvalue.cpp
+++ b/lib/vf_settokenvalue.cpp
@@ -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;
                 }
@@ -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;
                 }
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index e7709c7fa5e..8808699bff2 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -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__)
@@ -6077,6 +6078,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)
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index f417a643b4a..b39822365e3 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -150,7 +150,7 @@ class TestValueFlow : public TestFixture {
         TEST_CASE(valueFlowIdempotent);
         TEST_CASE(valueFlowUnsigned);
         TEST_CASE(valueFlowMod);
-        TEST_CASE(valueFlowInc);
+        TEST_CASE(valueFlowIncDec);
         TEST_CASE(valueFlowNotNull);
         TEST_CASE(valueFlowSymbolic);
         TEST_CASE(valueFlowSymbolicIdentity);
@@ -7925,7 +7925,7 @@ class TestValueFlow : public TestFixture {
         ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 1));
     }
 
-    void valueFlowInc() {
+    void valueFlowIncDec() {
         const char *code;
         std::list<ValueFlow::Value> values;
 
@@ -7939,6 +7939,24 @@ class TestValueFlow : public TestFixture {
         values = tokenValues(code, "i ]");
         ASSERT_EQUALS(1U, values.size());
         ASSERT_EQUALS(0LLU, values.back().intvalue);
+
+        code = "int f() {\n"
+               "    const int a[1] = {};\n"
+               "    unsigned char i = 255;\n"
+               "    return a[++i];\n"
+               "}\n";
+        values = tokenValues(code, "++");
+        ASSERT_EQUALS(1U, values.size());
+        ASSERT_EQUALS(0LLU, values.back().intvalue);
+
+        code = "int f() {\n"
+               "     const int a[128] = {};\n"
+               "     char b = -128;\n"
+               "     return a[--b];\n"
+               "}\n";
+        values = tokenValues(code, "--");
+        ASSERT_EQUALS(1U, values.size());
+        ASSERT_EQUALS(127LLU, values.back().intvalue);
     }
 
     void valueFlowNotNull()

From 781ef2acadf70ab05c983eff74811092d0cabaeb Mon Sep 17 00:00:00 2001
From: orbitcowboy <orbitcowboy@web.de>
Date: Wed, 26 Jun 2024 20:36:06 +0200
Subject: [PATCH 2/4] Added test for #11175 - FN: zerodiv (#6550)

Reference: https://trac.cppcheck.net/ticket/11175
---
 test/testother.cpp | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/testother.cpp b/test/testother.cpp
index d71fd173d63..018806a87d0 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -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
 
@@ -661,6 +662,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"

From 16ebe0ba70d4d58856fcc75514ef47285bc5c56d Mon Sep 17 00:00:00 2001
From: orbitcowboy <orbitcowboy@web.de>
Date: Thu, 27 Jun 2024 18:45:44 +0200
Subject: [PATCH 3/4] windows.cfg: Added support for obsolete
 UpdateTraceW()/UpdateTraceA(). (#6552)

References:
-
https://learn.microsoft.com/en-us/windows/win32/api/evntrace/nf-evntrace-updatetracea
-
https://learn.microsoft.com/en-us/windows/win32/api/evntrace/nf-evntrace-updatetracew
---
 cfg/windows.cfg      | 15 +++++++++++++++
 test/cfg/windows.cpp | 12 ++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/cfg/windows.cfg b/cfg/windows.cfg
index e061752ca66..aa464b6c40e 100644
--- a/cfg/windows.cfg
+++ b/cfg/windows.cfg
@@ -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,
diff --git a/test/cfg/windows.cpp b/test/cfg/windows.cpp
index 1e25ccfd5e5..b6544fc4f64 100644
--- a/test/cfg/windows.cpp
+++ b/test/cfg/windows.cpp
@@ -13,6 +13,7 @@
 #include <WinCon.h>
 #include <cstdio>
 #include <direct.h>
+#include <evntrace.h>
 #include <cstdlib>
 #include <ctime>
 #include <memory.h>
@@ -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);

From a8ee10a507f28daf4a7ef128dc74a8939c46cfb6 Mon Sep 17 00:00:00 2001
From: chrchr-github <78114321+chrchr-github@users.noreply.github.com>
Date: Fri, 28 Jun 2024 14:32:24 +0200
Subject: [PATCH 4/4] Fix #11642 FN redundantAssignment with mutual assignment
 (#6536)

---
 lib/checkother.cpp | 25 ++++++++++++++++++++++---
 lib/checkother.h   |  9 ++++++++-
 test/testother.cpp | 30 ++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 08ca9bbeb3f..086874b90ee 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -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);
 
@@ -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);
+                }
             }
         }
     }
@@ -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)
@@ -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())
@@ -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);
+                        }
                     }
                 }
             }
diff --git a/lib/checkother.h b/lib/checkother.h
index f2726bfd4e3..9295787207b 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -27,6 +27,7 @@
 #include "errortypes.h"
 #include "tokenize.h"
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -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();
@@ -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);
@@ -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;
 };
 /// @}
 //---------------------------------------------------------------------------
diff --git a/test/testother.cpp b/test/testother.cpp
index 018806a87d0..23baf92285c 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -215,6 +215,7 @@ class TestOther : public TestFixture {
         TEST_CASE(redundantVarAssignment_switch_break);
         TEST_CASE(redundantInitialization);
         TEST_CASE(redundantMemWrite);
+        TEST_CASE(redundantAssignmentSameValue);
 
         TEST_CASE(varFuncNullUB);
 
@@ -10189,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); }");