Skip to content

Commit 765b016

Browse files
committed
Unify C file-scope redeclarations in variable identities
1 parent 8bb772d commit 765b016

8 files changed

Lines changed: 345 additions & 11 deletions

File tree

‎lib/symboldatabase.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,53 @@ void SymbolDatabase::createSymbolDatabaseVariableInfo()
893893
for (Scope& scope : scopeList) {
894894
// find variables
895895
scope.getVariableList();
896+
897+
if (mTokenizer.isC() && scope.type == ScopeType::eGlobal) {
898+
// Canonicalize before assigning Variable pointers to tokens. C
899+
// declarations may contribute an initializer or complete an array
900+
// type without introducing another object.
901+
std::unordered_map<nonneg int, Variable*> variables;
902+
nonneg int index = 0;
903+
for (auto it = scope.varlist.begin(); it != scope.varlist.end();) {
904+
const nonneg int id = it->declarationId();
905+
const auto previous = variables.find(id);
906+
if (id == 0 || previous == variables.end()) {
907+
it->mIndex = index++;
908+
if (id != 0)
909+
variables.emplace(id, &*it);
910+
++it;
911+
continue;
912+
}
913+
914+
Variable& var = *previous->second;
915+
const bool isStatic = var.isStatic() || it->isStatic();
916+
const bool isExtern = var.isExtern() && it->isExtern();
917+
const bool maybeUnused = var.isMaybeUnused() || it->isMaybeUnused();
918+
auto dimensions = var.dimensions();
919+
if (dimensions.size() == it->dimensions().size()) {
920+
for (std::size_t i = 0; i < dimensions.size(); ++i) {
921+
const Dimension& dimension = it->dimensions()[i];
922+
if (!dimensions[i].known && !dimensions[i].tok)
923+
dimensions[i] = dimension;
924+
}
925+
}
926+
927+
// Keep the initialized definition, or a tentative definition
928+
// in preference to an extern-only declaration.
929+
if ((!var.isInit() && it->isInit()) ||
930+
(!var.isInit() && var.isExtern() && !it->isExtern())) {
931+
const nonneg int originalIndex = var.index();
932+
var = *it;
933+
var.mIndex = originalIndex;
934+
}
935+
var.setFlag(Variable::fIsStatic, isStatic);
936+
var.setFlag(Variable::fIsExtern, isExtern);
937+
var.setFlag(Variable::fIsMaybeUnused, maybeUnused);
938+
if (dimensions.size() == var.dimensions().size())
939+
var.setDimensions(dimensions);
940+
it = scope.varlist.erase(it);
941+
}
942+
}
896943
}
897944

898945
// fill in function arguments

‎lib/tokenize.cpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4042,6 +4042,10 @@ void Tokenizer::arraySizeAfterValueFlow()
40424042
continue;
40434043
if (!Token::Match(var->nameToken(), "%name% [ ] = { ["))
40444044
continue;
4045+
// A C redeclaration may inherit a complete bound from an earlier
4046+
// declaration. The initializer does not shrink that composite type.
4047+
if (!var->dimensions().empty() && (var->dimensions().front().known || var->dimensions().front().tok))
4048+
continue;
40454049
MathLib::bigint maxIndex = -1;
40464050
const Token* const startToken = var->nameToken()->tokAt(4);
40474051
const Token* const endToken = startToken->link();
@@ -4324,7 +4328,7 @@ namespace {
43244328
VariableMap() = default;
43254329
void enterScope();
43264330
bool leaveScope();
4327-
void addVariable(const std::string& varname, bool globalNamespace);
4331+
void addVariable(const std::string& varname, bool globalNamespace, bool reuseGlobal = false);
43284332
bool hasVariable(const std::string& varname) const {
43294333
return mVariableId.find(varname) != mVariableId.end();
43304334
}
@@ -4362,9 +4366,13 @@ bool VariableMap::leaveScope()
43624366
return true;
43634367
}
43644368

4365-
void VariableMap::addVariable(const std::string& varname, bool globalNamespace)
4369+
void VariableMap::addVariable(const std::string& varname, bool globalNamespace, bool reuseGlobal)
43664370
{
43674371
if (mScopeInfo.empty()) {
4372+
// C file-scope redeclarations name the same object. Parameters and
4373+
// local declarations enter their own VariableMap scope.
4374+
if (reuseGlobal && globalNamespace && hasVariable(varname))
4375+
return;
43684376
mVariableId[varname].id = ++mVarId;
43694377
if (globalNamespace)
43704378
mVariableId_global[varname] = mVariableId[varname];
@@ -5039,7 +5047,7 @@ void Tokenizer::setVarIdPass1()
50395047
if (decl) {
50405048
if (isC() && Token::Match(prev2->previous(), "&|&&"))
50415049
syntaxErrorC(prev2, prev2->strAt(-2) + prev2->strAt(-1) + " " + prev2->str());
5042-
variableMap.addVariable(prev2->str(), scopeStack.size() <= 1);
5050+
variableMap.addVariable(prev2->str(), scopeStack.size() <= 1, isC());
50435051

50445052
if (Token::simpleMatch(tok->previous(), "for (") && Token::Match(prev2, "%name% [=[({,]")) {
50455053
for (const Token *tok3 = prev2->next(); tok3 && tok3->str() != ";"; tok3 = tok3->next()) {

‎lib/valueflow.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,6 +1246,7 @@ static void valueFlowGlobalStaticVar(TokenList& tokenList, const Settings& setti
12461246
{
12471247
// Get variable values...
12481248
std::map<const Variable*, ValueFlow::Value> vars;
1249+
std::unordered_set<const Variable*> modified;
12491250
for (const Token* tok = tokenList.front(); tok; tok = tok->next()) {
12501251
if (!tok->variable())
12511252
continue;
@@ -1254,22 +1255,30 @@ static void valueFlowGlobalStaticVar(TokenList& tokenList, const Settings& setti
12541255
tok->valueType() && tok->valueType()->isIntegral() && tok->valueType()->pointer == 0 &&
12551256
tok->valueType()->constness == 0 && Token::Match(tok, "%name% =") && tok->next()->astOperand2() &&
12561257
tok->next()->astOperand2()->hasKnownIntValue()) {
1257-
vars[tok->variable()] = *tok->next()->astOperand2()->getKnownValue(ValueFlow::Value::ValueType::INT);
1258+
// A C definition can follow uses of a tentative declaration.
1259+
// Do not forget writes encountered before the initializer.
1260+
if (modified.count(tok->variable()) == 0)
1261+
vars[tok->variable()] = *tok->next()->astOperand2()->getKnownValue(ValueFlow::Value::ValueType::INT);
12581262
} else {
12591263
// If variable is written anywhere in TU then remove it from vars
12601264
if (!tok->astParent())
12611265
continue;
1266+
bool changed = false;
12621267
if (Token::Match(tok->astParent(), "++|--|&") && !tok->astParent()->astOperand2())
1263-
vars.erase(tok->variable());
1268+
changed = true;
12641269
else if (tok->astParent()->isAssignmentOp()) {
12651270
if (tok == tok->astParent()->astOperand1())
1266-
vars.erase(tok->variable());
1271+
changed = true;
12671272
else if (tok->isCpp() && Token::Match(tok->astParent()->tokAt(-2), "& %name% ="))
1268-
vars.erase(tok->variable());
1273+
changed = true;
12691274
} else if (isLikelyStreamRead(tok->astParent())) {
1270-
vars.erase(tok->variable());
1275+
changed = true;
12711276
} else if (Token::Match(tok->astParent(), "[(,]"))
1277+
changed = true;
1278+
if (changed) {
12721279
vars.erase(tok->variable());
1280+
modified.insert(tok->variable());
1281+
}
12731282
}
12741283
}
12751284

‎test/testbufferoverrun.cpp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class TestBufferOverrun : public TestFixture {
9393
TEST_CASE(sizeof3);
9494

9595
TEST_CASE(array_index_1);
96+
TEST_CASE(array_redeclaration_c);
9697
TEST_CASE(array_index_2);
9798
TEST_CASE(array_index_3);
9899
TEST_CASE(array_index_4);
@@ -386,6 +387,30 @@ class TestBufferOverrun : public TestFixture {
386387
ASSERT_EQUALS("", errout_str());
387388
}
388389

390+
void array_redeclaration_c() {
391+
check("extern int a[];\n"
392+
"int before(void) { return a[4]; }\n"
393+
"int a[4];\n"
394+
"int after(void) { return a[4]; }\n", dinit(CheckOptions, $.cpp = false));
395+
ASSERT_EQUALS("[test.c:2:28]: (error) Array 'a[4]' accessed at index 4, which is out of bounds. [arrayIndexOutOfBounds]\n"
396+
"[test.c:4:27]: (error) Array 'a[4]' accessed at index 4, which is out of bounds. [arrayIndexOutOfBounds]\n", errout_str());
397+
398+
check("int a[4];\n"
399+
"int a[] = {1};\n"
400+
"int f(void) { return a[3]; }\n", dinit(CheckOptions, $.cpp = false));
401+
ASSERT_EQUALS("", errout_str());
402+
403+
check("extern int a[4];\n"
404+
"int a[] = {1};\n"
405+
"int f(void) { return a[3]; }\n", dinit(CheckOptions, $.cpp = false));
406+
ASSERT_EQUALS("", errout_str());
407+
408+
check("enum { I = 0 }; int a[4];\n"
409+
"int a[] = {[I] = 1};\n"
410+
"int f(void) { return a[3]; }\n", dinit(CheckOptions, $.cpp = false));
411+
ASSERT_EQUALS("", errout_str());
412+
}
413+
389414
void array_index_1() {
390415
check("void f()\n"
391416
"{\n"

‎test/testother.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13370,7 +13370,7 @@ class TestOther : public TestFixture {
1337013370
" int a;\n"
1337113371
" return 0;\n"
1337213372
"}\n", dinit(CheckOptions, $.cpp = false));
13373-
ASSERT_EQUALS("[test.c:1:12] -> [test.c:4:9]: (style) Local variable 'a' shadows outer variable [shadowVariable]\n", errout_str());
13373+
ASSERT_EQUALS("[test.c:2:5] -> [test.c:4:9]: (style) Local variable 'a' shadows outer variable [shadowVariable]\n", errout_str());
1337413374

1337513375
check("int f() {\n" // #12591
1337613376
" int g = 0;\n"

‎test/testsymboldatabase.cpp‎

Lines changed: 160 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,10 @@ class TestSymbolDatabase : public TestFixture {
282282
TEST_CASE(hasGlobalVariables1);
283283
TEST_CASE(hasGlobalVariables2);
284284
TEST_CASE(hasGlobalVariables3);
285+
TEST_CASE(globalVariableRedeclarations); // #6418
286+
TEST_CASE(globalVariableRedeclarationMetadata);
287+
TEST_CASE(globalVariableRedeclarationArrayBounds);
288+
TEST_CASE(globalVariableRedeclarationScopes);
285289

286290
TEST_CASE(checkTypeStartEndToken1);
287291
TEST_CASE(checkTypeStartEndToken2); // handling for unknown macro: 'void f() MACRO {..'
@@ -1858,9 +1862,11 @@ class TestSymbolDatabase : public TestFixture {
18581862
GET_SYMBOL_DB_C("extern alignas(16) int x;\n"
18591863
"alignas(16) int x;\n");
18601864
ASSERT(db);
1861-
ASSERT_EQUALS(2, db->scopeList.front().varlist.size());
1865+
ASSERT_EQUALS(1, db->scopeList.front().varlist.size());
18621866
const Variable *x1 = Token::findsimplematch(tokenizer.tokens(), "x")->variable();
18631867
ASSERT(x1 && Token::simpleMatch(x1->typeStartToken(), "int x ;"));
1868+
const Token *x2 = findToken(tokenizer, "x ;", 2);
1869+
ASSERT(x2 && x2->variable() == x1);
18641870
}
18651871

18661872
void memberVar1() {
@@ -2696,6 +2702,159 @@ class TestSymbolDatabase : public TestFixture {
26962702
ASSERT(var->typeStartToken()->str() == "int");
26972703
}
26982704

2705+
void globalVariableRedeclarations() { // #6418
2706+
GET_SYMBOL_DB_C("int tentative;\n"
2707+
"int tentative;\n"
2708+
"extern int defined;\n"
2709+
"int before(void) { return defined; }\n"
2710+
"int defined;\n"
2711+
"extern int defined;\n"
2712+
"int after(void) { return defined; }\n"
2713+
"int reverse;\n"
2714+
"extern int reverse;\n");
2715+
ASSERT(db);
2716+
ASSERT_EQUALS(3, db->scopeList.front().varlist.size());
2717+
for (const Variable& var : db->scopeList.front().varlist) {
2718+
ASSERT(var.isGlobal());
2719+
ASSERT(!var.isExtern());
2720+
ASSERT(var.declarationId() != 0);
2721+
ASSERT(db->getVariableFromVarId(var.declarationId()) == &var);
2722+
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
2723+
if (tok->str() != var.name())
2724+
continue;
2725+
ASSERT_EQUALS(var.declarationId(), tok->varId());
2726+
ASSERT(tok->variable() == &var);
2727+
}
2728+
}
2729+
}
2730+
2731+
void globalVariableRedeclarationMetadata() {
2732+
GET_SYMBOL_DB_C("extern int value;\n"
2733+
"int value = 7;\n"
2734+
"extern int value;\n"
2735+
"int reverse = 9;\n"
2736+
"extern int reverse;\n"
2737+
"extern int data[];\n"
2738+
"int before(void) { return data[0]; }\n"
2739+
"int data[4];\n"
2740+
"extern int data[];\n"
2741+
"int after(void) { return data[0]; }\n"
2742+
"static int internal;\n"
2743+
"extern int internal = 1;\n");
2744+
ASSERT(db);
2745+
ASSERT_EQUALS(4, db->scopeList.front().varlist.size());
2746+
2747+
const Variable* value = Token::findsimplematch(tokenizer.tokens(), "value")->variable();
2748+
ASSERT(value && value->isInit() && !value->isExtern());
2749+
ASSERT_EQUALS(2, value->nameToken()->linenr());
2750+
const Variable* reverse = Token::findsimplematch(tokenizer.tokens(), "reverse")->variable();
2751+
ASSERT(reverse && reverse->isInit() && !reverse->isExtern());
2752+
ASSERT_EQUALS(4, reverse->nameToken()->linenr());
2753+
2754+
const Variable* data = Token::findsimplematch(tokenizer.tokens(), "data")->variable();
2755+
ASSERT(data && data->isArray() && !data->isExtern());
2756+
ASSERT_EQUALS(1U, data->dimensions().size());
2757+
ASSERT(data->dimensions()[0].known);
2758+
ASSERT_EQUALS(4, data->dimension(0));
2759+
const Variable* internal = Token::findsimplematch(tokenizer.tokens(), "internal")->variable();
2760+
ASSERT(internal && internal->isStatic() && internal->isInit());
2761+
ASSERT_EQUALS(12, internal->nameToken()->linenr());
2762+
2763+
for (const Variable& var : db->scopeList.front().varlist) {
2764+
ASSERT(db->getVariableFromVarId(var.declarationId()) == &var);
2765+
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
2766+
if (tok->str() == var.name())
2767+
ASSERT(tok->variable() == &var);
2768+
}
2769+
}
2770+
}
2771+
2772+
void globalVariableRedeclarationArrayBounds() {
2773+
GET_SYMBOL_DB_C("int a[4];\n"
2774+
"int a[] = { 1 };\n"
2775+
"extern int b[4];\n"
2776+
"int b[] = { 1 };\n");
2777+
ASSERT(db);
2778+
ASSERT_EQUALS(2, db->scopeList.front().varlist.size());
2779+
for (const Variable& var : db->scopeList.front().varlist) {
2780+
ASSERT(var.isArray() && var.isInit() && !var.isExtern());
2781+
ASSERT_EQUALS(1U, var.dimensions().size());
2782+
ASSERT(var.dimensions()[0].known);
2783+
ASSERT_EQUALS(4, var.dimension(0));
2784+
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
2785+
if (tok->str() == var.name())
2786+
ASSERT(tok->variable() == &var);
2787+
}
2788+
}
2789+
}
2790+
2791+
void globalVariableRedeclarationScopes() {
2792+
{
2793+
GET_SYMBOL_DB_C("int x;\n"
2794+
"struct A { int x; };\n"
2795+
"struct B { int x; };\n"
2796+
"void f(void) { int x; { int x; } }\n"
2797+
"void g(void) { int x; }\n"
2798+
"int x;\n");
2799+
ASSERT(db);
2800+
ASSERT_EQUALS(1, db->scopeList.front().varlist.size());
2801+
const Variable* global = &db->scopeList.front().varlist.front();
2802+
std::set<const Variable*> variables;
2803+
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
2804+
if (tok->str() != "x")
2805+
continue;
2806+
ASSERT(tok->variable());
2807+
variables.insert(tok->variable());
2808+
if (tok->linenr() == 1 || tok->linenr() == 6)
2809+
ASSERT(tok->variable() == global);
2810+
else
2811+
ASSERT(tok->variable() != global);
2812+
}
2813+
ASSERT_EQUALS(6, variables.size());
2814+
}
2815+
{
2816+
GET_SYMBOL_DB_C("int x;\n"
2817+
"int f(int x);\n"
2818+
"int f(int x) { return x; }\n"
2819+
"int x;\n");
2820+
ASSERT(db);
2821+
ASSERT_EQUALS(1, db->scopeList.front().varlist.size());
2822+
const Variable* global = &db->scopeList.front().varlist.front();
2823+
const Token* prototypeArg = findToken(tokenizer, "x )", 2);
2824+
ASSERT(prototypeArg && prototypeArg->varId() != 0);
2825+
ASSERT(prototypeArg->varId() != global->declarationId());
2826+
const Token* arg = findToken(tokenizer, "x )", 3);
2827+
const Token* use = findToken(tokenizer, "x ;", 3);
2828+
ASSERT(arg && arg->variable() && arg->variable()->isArgument());
2829+
ASSERT(arg->variable() != global);
2830+
ASSERT(use && use->variable() == arg->variable());
2831+
}
2832+
{
2833+
GET_SYMBOL_DB_C("int x;\n"
2834+
"struct { int x; } s;\n"
2835+
"int x;\n");
2836+
ASSERT(db);
2837+
ASSERT_EQUALS(2, db->scopeList.front().varlist.size());
2838+
const Token* global = findToken(tokenizer, "x ;", 1);
2839+
const Token* member = findToken(tokenizer, "x ;", 2);
2840+
const Token* redeclaration = findToken(tokenizer, "x ;", 3);
2841+
ASSERT(global && global->variable() && global->variable()->isGlobal());
2842+
ASSERT(member && member->variable() && member->variable()->isMember());
2843+
ASSERT(redeclaration && redeclaration->variable() == global->variable());
2844+
ASSERT(member->variable() != global->variable());
2845+
}
2846+
{
2847+
// Preserve the existing C++ handling.
2848+
GET_SYMBOL_DB("extern int x; int x;\n");
2849+
ASSERT_EQUALS(2, db->scopeList.front().varlist.size());
2850+
const Token* first = Token::findsimplematch(tokenizer.tokens(), "x ;");
2851+
ASSERT(first && first->variable());
2852+
const Token* second = Token::findsimplematch(first->next(), "x ;");
2853+
ASSERT(second && second->variable());
2854+
ASSERT(first->variable() != second->variable());
2855+
}
2856+
}
2857+
26992858
void checkTypeStartEndToken1() {
27002859
GET_SYMBOL_DB("static std::string i;\n"
27012860
"static const std::string j;\n"

0 commit comments

Comments
 (0)