Skip to content

Commit

Permalink
Fix #12210 (Cppcheck hang in SymbolDatabase::createSymbolDatabaseExpr…
Browse files Browse the repository at this point in the history 
…Ids)
  • Loading branch information
@danmar
danmar committed Nov 24, 2023
1 parent 331db40 commit cda7660
Show file tree
Hide file tree
Showing 7 changed files with 326 additions and 169 deletions.
134 changes: 86 additions & 48 deletions lib/symboldatabase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1576,16 +1576,59 @@ static std::string getIncompleteNameID(const Token* tok)
return result + tok->expressionString();
}

namespace {
struct ExprIdKey {
std::string parentOp;
nonneg int operand1;
nonneg int operand2;
bool operator<(const ExprIdKey& k) const {
if (operand1 != k.operand1)
return operand1 < k.operand1;
if (operand2 != k.operand2)
return operand2 < k.operand2;
return parentOp < k.parentOp;
}
};
using ExprIdMap = std::map<ExprIdKey, nonneg int>;
void setParentExprId(Token* tok, ExprIdMap& exprIdMap, nonneg int &id) {
if (!tok->astParent() || tok->astParent()->isControlFlowKeyword())
return;
const Token* op1 = tok->astParent()->astOperand1();
if (op1 && op1->exprId() == 0)
return;
const Token* op2 = tok->astParent()->astOperand2();
if (op2 && op2->exprId() == 0)
return;

ExprIdKey key;
key.parentOp = tok->astParent()->str();
key.operand1 = op1 ? op1->exprId() : 0;
key.operand2 = op2 ? op2->exprId() : 0;

if (op1 && op2 && key.parentOp == "+" && key.operand1 > key.operand2)
std::swap(key.operand1, key.operand2);

const auto it = exprIdMap.find(key);
if (it == exprIdMap.end()) {
exprIdMap[key] = id;
tok->astParent()->exprId(id);
++id;
} else {
tok->astParent()->exprId(it->second);
}
setParentExprId(tok->astParent(), exprIdMap, id);
}
}

void SymbolDatabase::createSymbolDatabaseExprIds()
{
nonneg int base = 0;
// Find highest varId
for (const Variable *var : mVariableList) {
if (!var)
continue;
base = std::max<MathLib::bigint>(base, var->declarationId());
nonneg int maximumVarId = 0;
for (const Token* tok = mTokenizer.list.front(); tok; tok = tok->next()) {
if (tok->varId() > maximumVarId)
maximumVarId = tok->varId();
}
nonneg int id = base + 1;
nonneg int id = maximumVarId + 1;
// Find incomplete vars that are used in constant context
std::unordered_map<std::string, nonneg int> unknownConstantIds;
const Token* inConstExpr = nullptr;
Expand Down Expand Up @@ -1616,7 +1659,6 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
});

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

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

// Assign IDs
ExprIdMap exprIdMap;
std::map<std::string, nonneg int> baseIds;
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
if (tok->varId() > 0) {
tok->exprId(tok->varId());
} else if (isExpression(tok)) {
exprs[tok->str()].push_back(tok);
tok->exprId(id++);
setParentExprId(tok, exprIdMap, id);
} else if (tok->astParent() && !tok->astOperand1() && !tok->astOperand2()) {
if (tok->tokType() == Token::Type::eBracket)
continue;
if (tok->astParent()->isAssignmentOp())
continue;
if (tok->isControlFlowKeyword())
continue;

if (id == std::numeric_limits<nonneg int>::max() / 4) {
throw InternalError(nullptr, "Ran out of expression ids.", InternalError::INTERNAL);
}
} else if (isCPP() && Token::simpleMatch(tok, "this")) {
if (thisId == 0)
thisId = id++;
tok->exprId(thisId);
}
}

// Apply CSE
for (const auto& p:exprs) {
const std::vector<Token*>& tokens = p.second;
const std::size_t N = tokens.size();
for (std::size_t i = 0; i < N; ++i) {
Token* const tok1 = tokens[i];
for (std::size_t j = i + 1; j < N; ++j) {
Token* const tok2 = tokens[j];
if (tok1->exprId() == tok2->exprId())
continue;
if (!isSameExpression(isCPP(), true, tok1, tok2, mSettings.library, false, false))
continue;
nonneg int const cid = std::min(tok1->exprId(), tok2->exprId());
tok1->exprId(cid);
tok2->exprId(cid);
if (Token::Match(tok, "%name% <") && tok->next()->link()) {
baseIds[tok->str()] = id;
tok->exprId(id);
++id;
} else {
const auto it = baseIds.find(tok->str());
if (it != baseIds.end()) {
tok->exprId(it->second);
} else {
baseIds[tok->str()] = id;
tok->exprId(id);
++id;
}
}

setParentExprId(tok, exprIdMap, id);
}
}

// Mark expressions that are unique
std::unordered_map<nonneg int, Token*> exprMap;
std::vector<std::pair<Token*, int>> uniqueExprId((std::size_t)(id+10));
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
if (tok->exprId() == 0)
const auto id2 = tok->exprId();
if (id2 == 0)
continue;
auto p = exprMap.emplace(tok->exprId(), tok);
// Already exists so set it to null
if (!p.second) {
p.first->second = nullptr;
}
uniqueExprId[id2].first = tok;
uniqueExprId[id2].second++;
}
for (const auto& p : exprMap) {
if (!p.second)
for (const auto& p : uniqueExprId) {
if (!p.first || p.second != 1)
continue;
if (p.second->variable()) {
const Variable* var = p.second->variable();
if (var->nameToken() != p.second)
if (p.first->variable()) {
const Variable* var = p.first->variable();
if (var->nameToken() != p.first)
continue;
}
p.second->setUniqueExprId();
p.first->setUniqueExprId();
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion lib/token.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,9 @@ std::string Token::stringify(const stringifyOptions& options) const
} else if (options.exprid && mImpl->mExprId != 0) {
ret += '@';
ret += (options.idtype ? "expr" : "");
ret += std::to_string(mImpl->mExprId);
if ((mImpl->mExprId & (1U << efIsUnique)) != 0)
ret += "UNIQUE";
ret += std::to_string(mImpl->mExprId & ~(1U << efIsUnique));
}

return ret;
Expand Down
5 changes: 1 addition & 4 deletions lib/token.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -907,10 +907,7 @@ class CPPCHECKLIB Token {

bool isUniqueExprId() const
{
if (mImpl->mExprId > 0) {
return (mImpl->mExprId & (1 << efIsUnique)) != 0;
}
return false;
return (mImpl->mExprId & (1 << efIsUnique)) != 0;
}

/**
Expand Down
109 changes: 0 additions & 109 deletions test/cli/test-other.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,115 +153,6 @@ def test_progress_j(tmpdir):
assert stderr == ""


@pytest.mark.timeout(10)
def test_slow_array_many_floats(tmpdir):
# 11649
# cppcheck valueflow takes a long time when an array has many floats
filename = os.path.join(tmpdir, 'hang.c')
with open(filename, 'wt') as f:
f.write("const float f[] = {\n")
for i in range(20000):
f.write(' 13.6f,\n')
f.write("};\n")
cppcheck([filename]) # should not take more than ~1 second


@pytest.mark.timeout(10)
def test_slow_array_many_strings(tmpdir):
# 11901
# cppcheck valueflow takes a long time when analyzing a file with many strings
filename = os.path.join(tmpdir, 'hang.c')
with open(filename, 'wt') as f:
f.write("const char *strings[] = {\n")
for i in range(20000):
f.write(' "abc",\n')
f.write("};\n")
cppcheck([filename]) # should not take more than ~1 second


@pytest.mark.timeout(10)
def test_slow_long_line(tmpdir):
# simplecpp #314
filename = os.path.join(tmpdir, 'hang.c')
with open(filename, 'wt') as f:
f.write("#define A() static const int a[] = {\\\n")
for i in range(5000):
f.write(" -123, 456, -789,\\\n")
f.write("};\n")
cppcheck([filename]) # should not take more than ~1 second


@pytest.mark.timeout(60)
def test_slow_large_constant_expression(tmpdir):
# 12182
filename = os.path.join(tmpdir, 'hang.c')
with open(filename, 'wt') as f:
f.write("""
#define FLAG1 0
#define FLAG2 0
#define FLAG3 0
#define FLAG4 0
#define FLAG5 0
#define FLAG6 0
#define FLAG7 0
#define FLAG8 0
#define FLAG9 0
#define FLAG10 0
#define FLAG11 0
#define FLAG12 0
#define FLAG13 0
#define FLAG14 0
#define FLAG15 0
#define FLAG16 0
#define FLAG17 0
#define FLAG18 0
#define FLAG19 0
#define FLAG20 0
#define FLAG21 0
#define FLAG22 0
#define FLAG23 0
#define FLAG24 0
#define maxval(x, y) ((x) > (y) ? (x) : (y))
#define E_SAMPLE_SIZE maxval( FLAG1, \
maxval( FLAG2, \
maxval( FLAG3, \
maxval( FLAG4, \
maxval( FLAG5, \
maxval( FLAG6, \
maxval( FLAG7, \
maxval( FLAG8, \
maxval( FLAG9, \
maxval( FLAG10, \
maxval( FLAG11, \
maxval( FLAG12, \
maxval( FLAG13, \
maxval( FLAG14, \
FLAG15 ))))))))))))))
#define SAMPLE_SIZE maxval( E_SAMPLE_SIZE, \
maxval( sizeof(st), \
maxval( FLAG16, \
maxval( FLAG17, \
maxval( FLAG18, \
maxval( FLAG19, \
maxval( FLAG20, \
maxval( FLAG21, \
maxval( FLAG22, \
maxval( FLAG23, \
FLAG24 ))))))))))
typedef struct {
int n;
} st;
x = SAMPLE_SIZE;
""")

cppcheck([filename])


def test_execute_addon_failure(tmpdir):
test_file = os.path.join(tmpdir, 'test.cpp')
with open(test_file, 'wt') as f:
Expand Down
Loading

0 comments on commit cda7660

Please sign in to comment.