Skip to content

Commit ca039ab

Browse files
danmardanmar
committed
Fix #12210 (Cppcheck hang in SymbolDatabase::createSymbolDatabaseExprIds)
1 parent d1b42d0 commit ca039ab

8 files changed

Lines changed: 321 additions & 177 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ test/testvaarg.o: test/testvaarg.cpp lib/addoninfo.h lib/check.h lib/checkvaarg.
887887
test/testvalueflow.o: test/testvalueflow.cpp lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
888888
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvalueflow.cpp
889889

890-
test/testvarid.o: test/testvarid.cpp lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
890+
test/testvarid.o: test/testvarid.cpp lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
891891
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvarid.cpp
892892

893893
externals/simplecpp/simplecpp.o: externals/simplecpp/simplecpp.cpp externals/simplecpp/simplecpp.h

‎lib/symboldatabase.cpp‎

Lines changed: 112 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,16 +1571,75 @@ static std::string getIncompleteNameID(const Token* tok)
15711571
return result + tok->expressionString();
15721572
}
15731573

1574+
namespace {
1575+
struct ExprIdKey {
1576+
std::string parentOp;
1577+
nonneg int operand1;
1578+
nonneg int operand2;
1579+
bool operator<(const ExprIdKey& k) const {
1580+
return std::tie(parentOp, operand1, operand2) < std::tie(k.parentOp, k.operand1, k.operand2);
1581+
}
1582+
};
1583+
using ExprIdMap = std::map<ExprIdKey, nonneg int>;
1584+
void setParentExprId(Token* tok, ExprIdMap& exprIdMap, nonneg int &id) {
1585+
if (!tok->astParent() || tok->astParent()->isControlFlowKeyword())
1586+
return;
1587+
const Token* op1 = tok->astParent()->astOperand1();
1588+
if (op1 && op1->exprId() == 0)
1589+
return;
1590+
const Token* op2 = tok->astParent()->astOperand2();
1591+
if (op2 && op2->exprId() == 0)
1592+
return;
1593+
1594+
if (tok->astParent()->isExpandedMacro()) {
1595+
tok->astParent()->exprId(id);
1596+
++id;
1597+
setParentExprId(tok->astParent(), exprIdMap, id);
1598+
return;
1599+
}
1600+
1601+
ExprIdKey key;
1602+
key.parentOp = tok->astParent()->str();
1603+
1604+
const auto& refs1 = followAllReferences(op1);
1605+
const auto& refs2 = followAllReferences(op2);
1606+
1607+
for (int i1 = 0; i1 < 1 + refs1.size(); ++i1) {
1608+
for (int i2 = 0; i2 < 1 + refs2.size(); ++i2) {
1609+
key.operand1 = op1 ? (i1 >= refs1.size() ? op1->exprId() : refs1[i1].token->exprId()) : 0;
1610+
key.operand2 = op2 ? (i2 >= refs2.size() ? op2->exprId() : refs2[i2].token->exprId()) : 0;
1611+
1612+
if (key.operand1 > key.operand2 && key.operand2 && key.parentOp == "+")
1613+
std::swap(key.operand1, key.operand2);
1614+
1615+
const auto it = exprIdMap.find(key);
1616+
if (it == exprIdMap.end()) {
1617+
if (i1 < refs1.size() || i2 < refs2.size())
1618+
// try to find another reference
1619+
continue;
1620+
exprIdMap[key] = id;
1621+
tok->astParent()->exprId(id);
1622+
++id;
1623+
} else {
1624+
tok->astParent()->exprId(it->second);
1625+
}
1626+
setParentExprId(tok->astParent(), exprIdMap, id);
1627+
i1 = 1 + refs1.size();
1628+
break;
1629+
}
1630+
}
1631+
}
1632+
}
1633+
15741634
void SymbolDatabase::createSymbolDatabaseExprIds()
15751635
{
1576-
nonneg int base = 0;
15771636
// Find highest varId
1578-
for (const Variable *var : mVariableList) {
1579-
if (!var)
1580-
continue;
1581-
base = std::max<MathLib::bigint>(base, var->declarationId());
1637+
nonneg int maximumVarId = 0;
1638+
for (const Token* tok = mTokenizer.list.front(); tok; tok = tok->next()) {
1639+
if (tok->varId() > maximumVarId)
1640+
maximumVarId = tok->varId();
15821641
}
1583-
nonneg int id = base + 1;
1642+
nonneg int id = maximumVarId + 1;
15841643
// Find incomplete vars that are used in constant context
15851644
std::unordered_map<std::string, nonneg int> unknownConstantIds;
15861645
const Token* inConstExpr = nullptr;
@@ -1611,7 +1670,6 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
16111670
});
16121671

16131672
for (const Scope * scope : exprScopes) {
1614-
nonneg int thisId = 0;
16151673
std::unordered_map<std::string, std::vector<Token*>> exprs;
16161674

16171675
std::unordered_map<std::string, nonneg int> unknownIds;
@@ -1638,62 +1696,65 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
16381696
}
16391697

16401698
// Assign IDs
1699+
ExprIdMap exprIdMap;
1700+
std::map<std::string, nonneg int> baseIds;
16411701
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
16421702
if (tok->varId() > 0) {
16431703
tok->exprId(tok->varId());
1644-
} else if (isExpression(tok)) {
1645-
exprs[tok->str()].push_back(tok);
1646-
tok->exprId(id++);
1704+
if (tok->astParent() && tok->astParent()->exprId() == 0)
1705+
setParentExprId(tok, exprIdMap, id);
1706+
} else if (tok->astParent() && !tok->astOperand1() && !tok->astOperand2()) {
1707+
if (tok->tokType() == Token::Type::eBracket)
1708+
continue;
1709+
if (tok->astParent()->isAssignmentOp())
1710+
continue;
1711+
if (tok->isControlFlowKeyword())
1712+
continue;
16471713

1648-
if (id == std::numeric_limits<nonneg int>::max() / 4) {
1649-
throw InternalError(nullptr, "Ran out of expression ids.", InternalError::INTERNAL);
1650-
}
1651-
} else if (isCPP() && Token::simpleMatch(tok, "this")) {
1652-
if (thisId == 0)
1653-
thisId = id++;
1654-
tok->exprId(thisId);
1655-
}
1656-
}
1657-
1658-
// Apply CSE
1659-
for (const auto& p:exprs) {
1660-
const std::vector<Token*>& tokens = p.second;
1661-
const std::size_t N = tokens.size();
1662-
for (std::size_t i = 0; i < N; ++i) {
1663-
Token* const tok1 = tokens[i];
1664-
for (std::size_t j = i + 1; j < N; ++j) {
1665-
Token* const tok2 = tokens[j];
1666-
if (tok1->exprId() == tok2->exprId())
1667-
continue;
1668-
if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings.library, false, false))
1669-
continue;
1670-
nonneg int const cid = std::min(tok1->exprId(), tok2->exprId());
1671-
tok1->exprId(cid);
1672-
tok2->exprId(cid);
1714+
if (Token::Match(tok, "%name% <") && tok->next()->link()) {
1715+
tok->exprId(id);
1716+
++id;
1717+
} else {
1718+
const auto it = baseIds.find(tok->str());
1719+
if (it != baseIds.end()) {
1720+
tok->exprId(it->second);
1721+
} else {
1722+
baseIds[tok->str()] = id;
1723+
tok->exprId(id);
1724+
++id;
1725+
}
16731726
}
1727+
1728+
setParentExprId(tok, exprIdMap, id);
16741729
}
16751730
}
1676-
// Mark expressions that are unique
1677-
std::unordered_map<nonneg int, Token*> exprMap;
16781731
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
1679-
if (tok->exprId() == 0)
1680-
continue;
1681-
auto p = exprMap.emplace(tok->exprId(), tok);
1682-
// Already exists so set it to null
1683-
if (!p.second) {
1684-
p.first->second = nullptr;
1732+
if (tok->varId() == 0 && tok->exprId() > 0 && tok->astParent() && !tok->astOperand1() && !tok->astOperand2()) {
1733+
if (tok->isNumber() || tok->isKeyword() || Token::Match(tok->astParent(), ".|::") ||
1734+
(Token::simpleMatch(tok->astParent(), "(") && precedes(tok, tok->astParent())))
1735+
tok->exprId(0);
16851736
}
16861737
}
1687-
for (const auto& p : exprMap) {
1688-
if (!p.second)
1738+
}
1739+
1740+
// Mark expressions that are unique
1741+
std::vector<std::pair<Token*, int>> uniqueExprId(id);
1742+
for (Token* tok = const_cast<Token*>(mTokenizer.list.front()); tok; tok = tok->next()) {
1743+
const auto id2 = tok->exprId();
1744+
if (id2 == 0 || id2 <= maximumVarId)
1745+
continue;
1746+
uniqueExprId[id2].first = tok;
1747+
uniqueExprId[id2].second++;
1748+
}
1749+
for (const auto& p : uniqueExprId) {
1750+
if (!p.first || p.second != 1)
1751+
continue;
1752+
if (p.first->variable()) {
1753+
const Variable* var = p.first->variable();
1754+
if (var->nameToken() != p.first)
16891755
continue;
1690-
if (p.second->variable()) {
1691-
const Variable* var = p.second->variable();
1692-
if (var->nameToken() != p.second)
1693-
continue;
1694-
}
1695-
p.second->setUniqueExprId();
16961756
}
1757+
p.first->setUniqueExprId();
16971758
}
16981759
}
16991760

‎lib/token.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1262,7 +1262,10 @@ 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+
else
1268+
ret += std::to_string(mImpl->mExprId);
12661269
}
12671270

12681271
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: 3 additions & 112 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:
@@ -852,7 +743,7 @@ def test_valueflow_debug(tmpdir):
852743
##file {}
853744
2: void f2 ( )
854745
3: {{
855-
4: int i@var1 ; i@var1 =@expr1073741828 0 ;
746+
4: int i@var1 ; i@var1 = 0 ;
856747
5: }}
857748
858749
##file {}
@@ -861,7 +752,7 @@ def test_valueflow_debug(tmpdir):
861752
2:
862753
3: void f1 ( )
863754
4: {{
864-
5: int i@var2 ; i@var2 =@expr1073741829 0 ;
755+
5: int i@var2 ; i@var2 = 0 ;
865756
6: }}
866757
867758
##file {}
@@ -871,7 +762,7 @@ def test_valueflow_debug(tmpdir):
871762
3:
872763
4: void f ( )
873764
5: {{
874-
6: int i@var3 ; i@var3 =@expr1073741830 0 ;
765+
6: int i@var3 ; i@var3 = 0 ;
875766
7: }}
876767
877768

0 commit comments

Comments
 (0)