Skip to content

Commit 5077663

Browse files
pfultz2pfultz2
authored
Fix issue 9979: false positive: containerOutOfBounds with conditional resize (#3136)
1 parent 5c102a0 commit 5077663

File tree

11 files changed

+460
-169
lines changed

11 files changed

+460
-169
lines changed

‎Makefile‎

Lines changed: 63 additions & 63 deletions
Large diffs are not rendered by default.

‎addons/test/misra/misra-test.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ void misra_7_2(void) {
241241
unsigned short h = 0x8000U;
242242
unsigned int i = 0x80000000; // 7.2
243243
unsigned int j = 0x80000000U;
244-
unsigned long long k = 0x8000000000000000; // 7.2
244+
unsigned long long k = 0x8000000000000000; // TODO 7.2
245245
unsigned long long l = 0x8000000000000000ULL;
246246

247247
unsigned int m = 1 + 0x80000000; // 7.2 10.4

‎lib/astutils.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ bool astIsIntegral(const Token *tok, bool unknown)
167167
return vt->isIntegral() && vt->pointer == 0U;
168168
}
169169

170+
bool astIsUnsigned(const Token* tok)
171+
{
172+
return tok && tok->valueType() && tok->valueType()->sign == ValueType::UNSIGNED;
173+
}
174+
170175
bool astIsFloat(const Token *tok, bool unknown)
171176
{
172177
const ValueType *vt = tok ? tok->valueType() : nullptr;

‎lib/astutils.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ bool astIsSignedChar(const Token *tok);
7070
bool astIsUnknownSignChar(const Token *tok);
7171
/** Is expression of integral type? */
7272
bool astIsIntegral(const Token *tok, bool unknown);
73+
bool astIsUnsigned(const Token* tok);
7374
/** Is expression of floating point type? */
7475
bool astIsFloat(const Token *tok, bool unknown);
7576
/** Is expression of boolean type? */

‎lib/programmemory.cpp‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "mathlib.h"
55
#include "symboldatabase.h"
66
#include "token.h"
7+
#include "valueflow.h"
78
#include <algorithm>
89
#include <cassert>
910
#include <cstdio>
@@ -424,20 +425,34 @@ void execute(const Token *expr,
424425

425426
else if (expr->isComparisonOp()) {
426427
MathLib::bigint result1(0), result2(0);
427-
execute(expr->astOperand1(), programMemory, &result1, error);
428-
execute(expr->astOperand2(), programMemory, &result2, error);
429-
if (expr->str() == "<")
430-
*result = result1 < result2;
431-
else if (expr->str() == "<=")
432-
*result = result1 <= result2;
433-
else if (expr->str() == ">")
434-
*result = result1 > result2;
435-
else if (expr->str() == ">=")
436-
*result = result1 >= result2;
437-
else if (expr->str() == "==")
438-
*result = result1 == result2;
439-
else if (expr->str() == "!=")
440-
*result = result1 != result2;
428+
bool error1 = 0;
429+
bool error2 = 0;
430+
execute(expr->astOperand1(), programMemory, &result1, &error1);
431+
execute(expr->astOperand2(), programMemory, &result2, &error2);
432+
if (error1 && error2) {
433+
*error = true;
434+
} else if (error1 && !error2) {
435+
ValueFlow::Value v = inferCondition(expr->str(), expr->astOperand1(), result2);
436+
*error = !v.isKnown();
437+
*result = v.intvalue;
438+
} else if (!error1 && error2) {
439+
ValueFlow::Value v = inferCondition(expr->str(), result1, expr->astOperand2());
440+
*error = !v.isKnown();
441+
*result = v.intvalue;
442+
} else {
443+
if (expr->str() == "<")
444+
*result = result1 < result2;
445+
else if (expr->str() == "<=")
446+
*result = result1 <= result2;
447+
else if (expr->str() == ">")
448+
*result = result1 > result2;
449+
else if (expr->str() == ">=")
450+
*result = result1 >= result2;
451+
else if (expr->str() == "==")
452+
*result = result1 == result2;
453+
else if (expr->str() == "!=")
454+
*result = result1 != result2;
455+
}
441456
}
442457

443458
else if (expr->isAssignmentOp()) {

‎lib/token.cpp‎

Lines changed: 115 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@
2424
#include "symboldatabase.h"
2525
#include "tokenlist.h"
2626
#include "utils.h"
27+
#include "valueflow.h"
2728

2829
#include <algorithm>
2930
#include <cassert>
3031
#include <cctype>
3132
#include <cstring>
33+
#include <functional>
3234
#include <iostream>
3335
#include <map>
3436
#include <set>
@@ -1947,6 +1949,25 @@ const Token *Token::getValueTokenDeadPointer() const
19471949
return nullptr;
19481950
}
19491951

1952+
static bool isAdjacent(const ValueFlow::Value& x, const ValueFlow::Value& y)
1953+
{
1954+
if (x.bound != ValueFlow::Value::Bound::Point && x.bound == y.bound)
1955+
return true;
1956+
if (x.valueType == ValueFlow::Value::ValueType::FLOAT)
1957+
return false;
1958+
return std::abs(x.intvalue - y.intvalue) == 1;
1959+
}
1960+
1961+
static bool removePointValue(std::list<ValueFlow::Value>& values, ValueFlow::Value& x)
1962+
{
1963+
const bool isPoint = x.bound == ValueFlow::Value::Bound::Point;
1964+
if (!isPoint)
1965+
x.decreaseRange();
1966+
else
1967+
values.remove(x);
1968+
return isPoint;
1969+
}
1970+
19501971
static bool removeContradiction(std::list<ValueFlow::Value>& values)
19511972
{
19521973
bool result = false;
@@ -1962,26 +1983,110 @@ static bool removeContradiction(std::list<ValueFlow::Value>& values)
19621983
continue;
19631984
if (x.isImpossible() == y.isImpossible())
19641985
continue;
1965-
if (!x.equalValue(y))
1986+
if (!x.equalValue(y)) {
1987+
auto compare = [](const ValueFlow::Value& x, const ValueFlow::Value& y) {
1988+
return x.compareValue(y, ValueFlow::less{});
1989+
};
1990+
const ValueFlow::Value& maxValue = std::max(x, y, compare);
1991+
const ValueFlow::Value& minValue = std::min(x, y, compare);
1992+
// TODO: Adjust non-points instead of removing them
1993+
if (maxValue.isImpossible() && maxValue.bound == ValueFlow::Value::Bound::Upper) {
1994+
values.remove(minValue);
1995+
return true;
1996+
}
1997+
if (minValue.isImpossible() && minValue.bound == ValueFlow::Value::Bound::Lower) {
1998+
values.remove(maxValue);
1999+
return true;
2000+
}
19662001
continue;
1967-
if (x.bound == y.bound ||
1968-
(x.bound != ValueFlow::Value::Bound::Point && y.bound != ValueFlow::Value::Bound::Point)) {
1969-
const bool removex = !x.isImpossible() || y.isKnown();
1970-
const bool removey = !y.isImpossible() || x.isKnown();
2002+
}
2003+
const bool removex = !x.isImpossible() || y.isKnown();
2004+
const bool removey = !y.isImpossible() || x.isKnown();
2005+
if (x.bound == y.bound) {
19712006
if (removex)
19722007
values.remove(x);
19732008
if (removey)
19742009
values.remove(y);
19752010
return true;
1976-
} else if (x.bound == ValueFlow::Value::Bound::Point) {
1977-
y.decreaseRange();
1978-
result = true;
2011+
} else {
2012+
result = removex || removey;
2013+
bool bail = false;
2014+
if (removex && removePointValue(values, x))
2015+
bail = true;
2016+
if (removey && removePointValue(values, y))
2017+
bail = true;
2018+
if (bail)
2019+
return true;
19792020
}
19802021
}
19812022
}
19822023
return result;
19832024
}
19842025

2026+
using ValueIterator = std::list<ValueFlow::Value>::iterator;
2027+
2028+
template <class Iterator>
2029+
static ValueIterator removeAdjacentValues(std::list<ValueFlow::Value>& values, ValueIterator x, Iterator start, Iterator last)
2030+
{
2031+
if (!isAdjacent(*x, **start))
2032+
return std::next(x);
2033+
auto it = std::adjacent_find(start, last, [](ValueIterator x, ValueIterator y) { return !isAdjacent(*x, *y); });
2034+
if (it == last)
2035+
it--;
2036+
(*it)->bound = x->bound;
2037+
std::for_each(start, it, [&](ValueIterator y) { values.erase(y); });
2038+
return values.erase(x);
2039+
}
2040+
2041+
static void mergeAdjacent(std::list<ValueFlow::Value>& values)
2042+
{
2043+
for (auto x = values.begin(); x != values.end();) {
2044+
if (x->isNonValue()) {
2045+
x++;
2046+
continue;
2047+
}
2048+
if (x->bound == ValueFlow::Value::Bound::Point) {
2049+
x++;
2050+
continue;
2051+
}
2052+
std::vector<ValueIterator> adjValues;
2053+
for (auto y = values.begin(); y != values.end(); y++) {
2054+
if (x == y)
2055+
continue;
2056+
if (y->isNonValue())
2057+
continue;
2058+
if (x->valueType != y->valueType)
2059+
continue;
2060+
if (x->valueKind != y->valueKind)
2061+
continue;
2062+
if (x->bound != y->bound) {
2063+
// No adjacent points for floating points
2064+
if (x->valueType == ValueFlow::Value::ValueType::FLOAT)
2065+
continue;
2066+
if (y->bound != ValueFlow::Value::Bound::Point)
2067+
continue;
2068+
}
2069+
if (x->bound == ValueFlow::Value::Bound::Lower && !y->compareValue(*x, ValueFlow::less{}))
2070+
continue;
2071+
if (x->bound == ValueFlow::Value::Bound::Upper && !x->compareValue(*y, ValueFlow::less{}))
2072+
continue;
2073+
adjValues.push_back(y);
2074+
}
2075+
if (adjValues.empty()) {
2076+
x++;
2077+
continue;
2078+
}
2079+
std::sort(adjValues.begin(), adjValues.end(), [&values](ValueIterator xx, ValueIterator yy) {
2080+
assert(xx != values.end() && yy != values.end());
2081+
return xx->compareValue(*yy, ValueFlow::less{});
2082+
});
2083+
if (x->bound == ValueFlow::Value::Bound::Lower)
2084+
x = removeAdjacentValues(values, x, adjValues.rbegin(), adjValues.rend());
2085+
else if (x->bound == ValueFlow::Value::Bound::Upper)
2086+
x = removeAdjacentValues(values, x, adjValues.begin(), adjValues.end());
2087+
}
2088+
}
2089+
19852090
static void removeOverlaps(std::list<ValueFlow::Value>& values)
19862091
{
19872092
for (ValueFlow::Value& x : values) {
@@ -2005,12 +2110,14 @@ static void removeOverlaps(std::list<ValueFlow::Value>& values)
20052110
return true;
20062111
});
20072112
}
2113+
mergeAdjacent(values);
20082114
}
20092115

20102116
// Removing contradictions is an NP-hard problem. Instead we run multiple
20112117
// passes to try to catch most contradictions
20122118
static void removeContradictions(std::list<ValueFlow::Value>& values)
20132119
{
2120+
removeOverlaps(values);
20142121
for (int i = 0; i < 4; i++) {
20152122
if (!removeContradiction(values))
20162123
return;

0 commit comments

Comments
 (0)