Skip to content

Commit cda7660

Browse files
danmardanmar
committed
Fix #12210 (Cppcheck hang in SymbolDatabase::createSymbolDatabaseExprIds)
1 parent 331db40 commit cda7660

7 files changed

Lines changed: 326 additions & 169 deletions

File tree

‎lib/symboldatabase.cpp‎

Lines changed: 86 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,16 +1576,59 @@ static std::string getIncompleteNameID(const Token* tok)
15761576
return result + tok->expressionString();
15771577
}
15781578

1579+
namespace {
1580+
struct ExprIdKey {
1581+
std::string parentOp;
1582+
nonneg int operand1;
1583+
nonneg int operand2;
1584+
bool operator<(const ExprIdKey& k) const {
1585+
if (operand1 != k.operand1)
1586+
return operand1 < k.operand1;
1587+
if (operand2 != k.operand2)
1588+
return operand2 < k.operand2;
1589+
return parentOp < k.parentOp;
1590+
}
1591+
};
1592+
using ExprIdMap = std::map<ExprIdKey, nonneg int>;
1593+
void setParentExprId(Token* tok, ExprIdMap& exprIdMap, nonneg int &id) {
1594+
if (!tok->astParent() || tok->astParent()->isControlFlowKeyword())
1595+
return;
1596+
const Token* op1 = tok->astParent()->astOperand1();
1597+
if (op1 && op1->exprId() == 0)
1598+
return;
1599+
const Token* op2 = tok->astParent()->astOperand2();
1600+
if (op2 && op2->exprId() == 0)
1601+
return;
1602+
1603+
ExprIdKey key;
1604+
key.parentOp = tok->astParent()->str();
1605+
key.operand1 = op1 ? op1->exprId() : 0;
1606+
key.operand2 = op2 ? op2->exprId() : 0;
1607+
1608+
if (op1 && op2 && key.parentOp == "+" && key.operand1 > key.operand2)
1609+
std::swap(key.operand1, key.operand2);
1610+
1611+
const auto it = exprIdMap.find(key);
1612+
if (it == exprIdMap.end()) {
1613+
exprIdMap[key] = id;
1614+
tok->astParent()->exprId(id);
1615+
++id;
1616+
} else {
1617+
tok->astParent()->exprId(it->second);
1618+
}
1619+
setParentExprId(tok->astParent(), exprIdMap, id);
1620+
}
1621+
}
1622+
15791623
void SymbolDatabase::createSymbolDatabaseExprIds()
15801624
{
1581-
nonneg int base = 0;
15821625
// Find highest varId
1583-
for (const Variable *var : mVariableList) {
1584-
if (!var)
1585-
continue;
1586-
base = std::max<MathLib::bigint>(base, var->declarationId());
1626+
nonneg int maximumVarId = 0;
1627+
for (const Token* tok = mTokenizer.list.front(); tok; tok = tok->next()) {
1628+
if (tok->varId() > maximumVarId)
1629+
maximumVarId = tok->varId();
15871630
}
1588-
nonneg int id = base + 1;
1631+
nonneg int id = maximumVarId + 1;
15891632
// Find incomplete vars that are used in constant context
15901633
std::unordered_map<std::string, nonneg int> unknownConstantIds;
15911634
const Token* inConstExpr = nullptr;
@@ -1616,7 +1659,6 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
16161659
});
16171660

16181661
for (const Scope * scope : exprScopes) {
1619-
nonneg int thisId = 0;
16201662
std::unordered_map<std::string, std::vector<Token*>> exprs;
16211663

16221664
std::unordered_map<std::string, nonneg int> unknownIds;
@@ -1643,61 +1685,57 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
16431685
}
16441686

16451687
// Assign IDs
1688+
ExprIdMap exprIdMap;
1689+
std::map<std::string, nonneg int> baseIds;
16461690
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
16471691
if (tok->varId() > 0) {
16481692
tok->exprId(tok->varId());
1649-
} else if (isExpression(tok)) {
1650-
exprs[tok->str()].push_back(tok);
1651-
tok->exprId(id++);
1693+
setParentExprId(tok, exprIdMap, id);
1694+
} else if (tok->astParent() && !tok->astOperand1() && !tok->astOperand2()) {
1695+
if (tok->tokType() == Token::Type::eBracket)
1696+
continue;
1697+
if (tok->astParent()->isAssignmentOp())
1698+
continue;
1699+
if (tok->isControlFlowKeyword())
1700+
continue;
16521701

1653-
if (id == std::numeric_limits<nonneg int>::max() / 4) {
1654-
throw InternalError(nullptr, "Ran out of expression ids.", InternalError::INTERNAL);
1655-
}
1656-
} else if (isCPP() && Token::simpleMatch(tok, "this")) {
1657-
if (thisId == 0)
1658-
thisId = id++;
1659-
tok->exprId(thisId);
1660-
}
1661-
}
1662-
1663-
// Apply CSE
1664-
for (const auto& p:exprs) {
1665-
const std::vector<Token*>& tokens = p.second;
1666-
const std::size_t N = tokens.size();
1667-
for (std::size_t i = 0; i < N; ++i) {
1668-
Token* const tok1 = tokens[i];
1669-
for (std::size_t j = i + 1; j < N; ++j) {
1670-
Token* const tok2 = tokens[j];
1671-
if (tok1->exprId() == tok2->exprId())
1672-
continue;
1673-
if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings.library, false, false))
1674-
continue;
1675-
nonneg int const cid = std::min(tok1->exprId(), tok2->exprId());
1676-
tok1->exprId(cid);
1677-
tok2->exprId(cid);
1702+
if (Token::Match(tok, "%name% <") && tok->next()->link()) {
1703+
baseIds[tok->str()] = id;
1704+
tok->exprId(id);
1705+
++id;
1706+
} else {
1707+
const auto it = baseIds.find(tok->str());
1708+
if (it != baseIds.end()) {
1709+
tok->exprId(it->second);
1710+
} else {
1711+
baseIds[tok->str()] = id;
1712+
tok->exprId(id);
1713+
++id;
1714+
}
16781715
}
1716+
1717+
setParentExprId(tok, exprIdMap, id);
16791718
}
16801719
}
1720+
16811721
// Mark expressions that are unique
1682-
std::unordered_map<nonneg int, Token*> exprMap;
1722+
std::vector<std::pair<Token*, int>> uniqueExprId((std::size_t)(id+10));
16831723
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
1684-
if (tok->exprId() == 0)
1724+
const auto id2 = tok->exprId();
1725+
if (id2 == 0)
16851726
continue;
1686-
auto p = exprMap.emplace(tok->exprId(), tok);
1687-
// Already exists so set it to null
1688-
if (!p.second) {
1689-
p.first->second = nullptr;
1690-
}
1727+
uniqueExprId[id2].first = tok;
1728+
uniqueExprId[id2].second++;
16911729
}
1692-
for (const auto& p : exprMap) {
1693-
if (!p.second)
1730+
for (const auto& p : uniqueExprId) {
1731+
if (!p.first || p.second != 1)
16941732
continue;
1695-
if (p.second->variable()) {
1696-
const Variable* var = p.second->variable();
1697-
if (var->nameToken() != p.second)
1733+
if (p.first->variable()) {
1734+
const Variable* var = p.first->variable();
1735+
if (var->nameToken() != p.first)
16981736
continue;
16991737
}
1700-
p.second->setUniqueExprId();
1738+
p.first->setUniqueExprId();
17011739
}
17021740
}
17031741
}

‎lib/token.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1262,7 +1262,9 @@ std::string Token::stringify(const stringifyOptions& options) const
12621262
} else if (options.exprid && mImpl->mExprId != 0) {
12631263
ret += '@';
12641264
ret += (options.idtype ? "expr" : "");
1265-
ret += std::to_string(mImpl->mExprId);
1265+
if ((mImpl->mExprId & (1U << efIsUnique)) != 0)
1266+
ret += "UNIQUE";
1267+
ret += std::to_string(mImpl->mExprId & ~(1U << efIsUnique));
12661268
}
12671269

12681270
return ret;

‎lib/token.h‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -907,10 +907,7 @@ class CPPCHECKLIB Token {
907907

908908
bool isUniqueExprId() const
909909
{
910-
if (mImpl->mExprId > 0) {
911-
return (mImpl->mExprId & (1 << efIsUnique)) != 0;
912-
}
913-
return false;
910+
return (mImpl->mExprId & (1 << efIsUnique)) != 0;
914911
}
915912

916913
/**

‎test/cli/test-other.py‎

Lines changed: 0 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -153,115 +153,6 @@ def test_progress_j(tmpdir):
153153
assert stderr == ""
154154

155155

156-
@pytest.mark.timeout(10)
157-
def test_slow_array_many_floats(tmpdir):
158-
# 11649
159-
# cppcheck valueflow takes a long time when an array has many floats
160-
filename = os.path.join(tmpdir, 'hang.c')
161-
with open(filename, 'wt') as f:
162-
f.write("const float f[] = {\n")
163-
for i in range(20000):
164-
f.write(' 13.6f,\n')
165-
f.write("};\n")
166-
cppcheck([filename]) # should not take more than ~1 second
167-
168-
169-
@pytest.mark.timeout(10)
170-
def test_slow_array_many_strings(tmpdir):
171-
# 11901
172-
# cppcheck valueflow takes a long time when analyzing a file with many strings
173-
filename = os.path.join(tmpdir, 'hang.c')
174-
with open(filename, 'wt') as f:
175-
f.write("const char *strings[] = {\n")
176-
for i in range(20000):
177-
f.write(' "abc",\n')
178-
f.write("};\n")
179-
cppcheck([filename]) # should not take more than ~1 second
180-
181-
182-
@pytest.mark.timeout(10)
183-
def test_slow_long_line(tmpdir):
184-
# simplecpp #314
185-
filename = os.path.join(tmpdir, 'hang.c')
186-
with open(filename, 'wt') as f:
187-
f.write("#define A() static const int a[] = {\\\n")
188-
for i in range(5000):
189-
f.write(" -123, 456, -789,\\\n")
190-
f.write("};\n")
191-
cppcheck([filename]) # should not take more than ~1 second
192-
193-
194-
@pytest.mark.timeout(60)
195-
def test_slow_large_constant_expression(tmpdir):
196-
# 12182
197-
filename = os.path.join(tmpdir, 'hang.c')
198-
with open(filename, 'wt') as f:
199-
f.write("""
200-
#define FLAG1 0
201-
#define FLAG2 0
202-
#define FLAG3 0
203-
#define FLAG4 0
204-
#define FLAG5 0
205-
#define FLAG6 0
206-
#define FLAG7 0
207-
#define FLAG8 0
208-
#define FLAG9 0
209-
#define FLAG10 0
210-
#define FLAG11 0
211-
#define FLAG12 0
212-
#define FLAG13 0
213-
#define FLAG14 0
214-
#define FLAG15 0
215-
#define FLAG16 0
216-
#define FLAG17 0
217-
#define FLAG18 0
218-
#define FLAG19 0
219-
#define FLAG20 0
220-
#define FLAG21 0
221-
#define FLAG22 0
222-
#define FLAG23 0
223-
#define FLAG24 0
224-
225-
#define maxval(x, y) ((x) > (y) ? (x) : (y))
226-
227-
#define E_SAMPLE_SIZE maxval( FLAG1, \
228-
maxval( FLAG2, \
229-
maxval( FLAG3, \
230-
maxval( FLAG4, \
231-
maxval( FLAG5, \
232-
maxval( FLAG6, \
233-
maxval( FLAG7, \
234-
maxval( FLAG8, \
235-
maxval( FLAG9, \
236-
maxval( FLAG10, \
237-
maxval( FLAG11, \
238-
maxval( FLAG12, \
239-
maxval( FLAG13, \
240-
maxval( FLAG14, \
241-
FLAG15 ))))))))))))))
242-
243-
#define SAMPLE_SIZE maxval( E_SAMPLE_SIZE, \
244-
maxval( sizeof(st), \
245-
maxval( FLAG16, \
246-
maxval( FLAG17, \
247-
maxval( FLAG18, \
248-
maxval( FLAG19, \
249-
maxval( FLAG20, \
250-
maxval( FLAG21, \
251-
maxval( FLAG22, \
252-
maxval( FLAG23, \
253-
FLAG24 ))))))))))
254-
255-
typedef struct {
256-
int n;
257-
} st;
258-
259-
x = SAMPLE_SIZE;
260-
""")
261-
262-
cppcheck([filename])
263-
264-
265156
def test_execute_addon_failure(tmpdir):
266157
test_file = os.path.join(tmpdir, 'test.cpp')
267158
with open(test_file, 'wt') as f:

0 commit comments

Comments
 (0)