Skip to content

Commit 52edaae

Browse files
committed
Fix initialization through singleton owned arrow writes
1 parent ce2ff87 commit 52edaae

5 files changed

Lines changed: 283 additions & 1 deletion

File tree

‎lib/astutils.cpp‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3512,6 +3512,88 @@ bool isLeafDot(const Token* tok)
35123512
return isLeafDot(parent);
35133513
}
35143514

3515+
static const Variable* singlePlainDataMember(const Scope* scope)
3516+
{
3517+
if (!scope || (scope->type != ScopeType::eStruct && scope->type != ScopeType::eClass) ||
3518+
!scope->definedType || !scope->definedType->derivedFrom.empty() || scope->numConstructors != 0)
3519+
return nullptr;
3520+
if (std::any_of(scope->functionList.cbegin(), scope->functionList.cend(), [](const Function& function) {
3521+
return function.hasVirtualSpecifier() || function.hasOverrideSpecifier();
3522+
}))
3523+
return nullptr;
3524+
if (std::any_of(scope->nestedList.cbegin(), scope->nestedList.cend(), [](const Scope* nested) {
3525+
return nested->type == ScopeType::eUnion;
3526+
}))
3527+
return nullptr;
3528+
const Variable* member = nullptr;
3529+
for (const Variable& var : scope->varlist) {
3530+
if (var.isStatic())
3531+
continue;
3532+
if (member || !var.isPublic() || var.isArray() || var.isPointer() || var.isReference() ||
3533+
var.isRValueReference() || var.isVolatile() || var.hasDefault())
3534+
return nullptr;
3535+
member = &var;
3536+
}
3537+
return member;
3538+
}
3539+
3540+
const Variable* getSingleMemberArrowWriteTarget(const Token* tok)
3541+
{
3542+
if (!Token::Match(tok, ". %name%") || tok->originalName() != "->" || !tok->astOperand1())
3543+
return nullptr;
3544+
// Exclude bindings, conditional/unevaluated operands and nested writes
3545+
// whose evaluation order is not established by this projection.
3546+
const Token* operation = tok->astParent();
3547+
if (!operation || operation->astParent() || operation->astOperand1() != tok ||
3548+
(!operation->isAssignmentOp() && !Token::Match(operation, "++|--")))
3549+
return nullptr;
3550+
const Variable* receiver = tok->astOperand1()->variable();
3551+
if (!receiver || !receiver->isLocal() || receiver->isPointer() || receiver->isArray() || receiver->isReference() ||
3552+
receiver->isRValueReference() || receiver->isVolatile())
3553+
return nullptr;
3554+
// A deferred lambda body does not initialize a captured outer object.
3555+
for (const Scope* enclosing = tok->scope(); enclosing && enclosing != receiver->scope(); enclosing = enclosing->nestedIn) {
3556+
if (enclosing->type == ScopeType::eLambda || enclosing->type == ScopeType::eFunction)
3557+
return nullptr;
3558+
}
3559+
const Scope* scope = receiver->typeScope();
3560+
const Variable* member = singlePlainDataMember(scope);
3561+
if (!member)
3562+
return nullptr;
3563+
const Variable* leaf = singlePlainDataMember(member->typeScope());
3564+
if (!leaf || !leaf->valueType() || !leaf->valueType()->isPrimitive() ||
3565+
tok->astOperand2() != tok->next() || tok->strAt(1) != leaf->name() ||
3566+
(tok->next()->variable() && tok->next()->variable() != leaf))
3567+
return nullptr;
3568+
3569+
const auto operators = scope->functionMap.equal_range("operator->");
3570+
if (operators.first == operators.second)
3571+
return nullptr;
3572+
for (auto it = operators.first; it != operators.second; ++it) {
3573+
const Function* function = it->second;
3574+
if (!function->functionScope || !Function::returnsPointer(function) ||
3575+
function->retType != member->type() || function->argCount() != 0 || function->isVolatile())
3576+
return nullptr;
3577+
const Token* body = function->functionScope->bodyStart;
3578+
if (!Token::simpleMatch(body, "{ return &") || !body->tokAt(2)->isUnaryOp("&"))
3579+
return nullptr;
3580+
const Token* memberToken = body->tokAt(3);
3581+
if (Token::simpleMatch(memberToken, "this ."))
3582+
memberToken = memberToken->tokAt(2);
3583+
if (!Token::Match(memberToken, "%var% ; }") || memberToken->variable() != member ||
3584+
memberToken->tokAt(2) != function->functionScope->bodyEnd)
3585+
return nullptr;
3586+
}
3587+
3588+
// A member, free or friend operator& can change the returned address.
3589+
// Keep the proof independent of overload resolution for address-of.
3590+
if (std::any_of(scope->symdb.scopeList.cbegin(), scope->symdb.scopeList.cend(), [](const Scope& candidate) {
3591+
return candidate.functionMap.count("operator&") != 0;
3592+
}))
3593+
return nullptr;
3594+
return member;
3595+
}
3596+
35153597
ExprUsage getExprUsage(const Token* tok, int indirect, const Settings& settings)
35163598
{
35173599
const Token* parent = tok->astParent();

‎lib/astutils.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,11 @@ bool isConstVarExpression(const Token* tok, const std::function<bool(const Token
439439

440440
bool isLeafDot(const Token* tok);
441441

442+
// Identify a standalone scalar write through a pure overloaded arrow when the
443+
// receiver's only member contains only that scalar. This is not a general alias
444+
// summary: reads, bindings, nested expressions and captured writes are excluded.
445+
const Variable* getSingleMemberArrowWriteTarget(const Token* tok);
446+
442447
enum class ExprUsage : std::uint8_t { None, NotUsed, PassedByReference, Used, Inconclusive };
443448

444449
ExprUsage getExprUsage(const Token* tok, int indirect, const Settings& settings);

‎lib/checkuninitvar.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1441,6 +1441,12 @@ int CheckUninitVarImpl::isFunctionParUsage(const Token *vartok, bool pointer, Al
14411441

14421442
bool CheckUninitVarImpl::isMemberVariableAssignment(const Token *tok, const std::string &membervar) const
14431443
{
1444+
if (const Variable* member = getSingleMemberArrowWriteTarget(tok->astParent())) {
1445+
const Token* access = tok->astParent();
1446+
if (access->astOperand1() == tok && member->name() == membervar &&
1447+
Token::simpleMatch(access->astParent(), "=") && astIsLHS(access))
1448+
return true;
1449+
}
14441450
if (Token::Match(tok, "%name% . %name%") && tok->strAt(2) == membervar) {
14451451
if (Token::Match(tok->tokAt(3), "[=.[]"))
14461452
return true;
@@ -1671,7 +1677,13 @@ void CheckUninitVarImpl::valueFlowUninit()
16711677
(tok->astParent()->next()->variable() || tok->astParent()->next()->isEnumerator()))
16721678
continue;
16731679
}
1674-
const ExprUsage usage = getExprUsage(tok, v->indirect, mSettings);
1680+
// For a proven singleton accessor, the scalar operation also
1681+
// describes the receiver's initialization state (including ++).
1682+
const Token* usageToken = tok;
1683+
if (v->indirect == 0 && getSingleMemberArrowWriteTarget(tok->astParent()) &&
1684+
tok->astParent()->astOperand1() == tok)
1685+
usageToken = tok->astParent();
1686+
const ExprUsage usage = getExprUsage(usageToken, v->indirect, mSettings);
16751687
if (usage == ExprUsage::NotUsed || usage == ExprUsage::Inconclusive)
16761688
continue;
16771689
if (!v->subexpressions.empty() && usage == ExprUsage::PassedByReference)

‎lib/vf_analyzers.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,14 @@ struct ValueFlowAnalyzer : Analyzer {
570570

571571
Action analyzeMatch(const Token* tok, Direction d) const {
572572
const Token* parent = tok->astParent();
573+
const ValueFlow::Value* value = getValue(tok);
574+
if (value && value->isUninitValue() && value->indirect == 0 &&
575+
getSingleMemberArrowWriteTarget(parent) && parent->astOperand1() == tok) {
576+
// The accessor only takes an address. For this singleton layout,
577+
// the selected scalar and the receiver have the same init state.
578+
const Token* operation = parent->astParent();
579+
return operation->str() == "=" ? Action::Invalid : Action::Read | Action::Invalid;
580+
}
573581
if (d == Direction::Reverse && isGlobal() && !dependsOnThis() && Token::Match(parent, ". %name% (")) {
574582
Action a = isGlobalModified(parent->next());
575583
if (a != Action::None)
@@ -1505,6 +1513,8 @@ struct MemberExpressionAnalyzer : SubExpressionAnalyzer {
15051513
{
15061514
if (!Token::Match(tok, ". %var%"))
15071515
return false;
1516+
if (const Variable* member = getSingleMemberArrowWriteTarget(tok))
1517+
return !exact || member->name() == varname;
15081518
if (!exact)
15091519
return true;
15101520
return tok->strAt(1) == varname;

‎test/testuninitvar.cpp‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ class TestUninitVar : public TestFixture {
4343
TEST_CASE(uninitvar_alloc); // data is allocated but not initialized
4444
TEST_CASE(uninitvar_arrays); // arrays
4545
TEST_CASE(uninitvar_class); // class/struct
46+
TEST_CASE(uninitvar_ownedArrow);
47+
TEST_CASE(uninitvar_ownedArrowEscapes);
48+
TEST_CASE(valueFlowUninit_ownedArrow);
49+
TEST_CASE(valueFlowUninit_ownedArrowWrites);
50+
TEST_CASE(valueFlowUninit_ownedArrowReadModify);
51+
TEST_CASE(valueFlowUninit_ownedArrowReceivers);
52+
TEST_CASE(valueFlowUninit_ownedArrowConditional);
4653
TEST_CASE(uninitvar_enum); // enum variables
4754
TEST_CASE(uninitvar_if); // handling if
4855
TEST_CASE(uninitvar_loops); // handling for/while
@@ -3677,6 +3684,172 @@ class TestUninitVar : public TestFixture {
36773684
(checkuninitvar.valueFlowUninit)();
36783685
}
36793686

3687+
void uninitvar_ownedArrow() { // #6572
3688+
checkUninitVar("struct CCommitPointer { int m_place; };\n"
3689+
"struct iterator {\n"
3690+
" CCommitPointer m_ptr;\n"
3691+
" CCommitPointer& operator*() { return m_ptr; }\n"
3692+
" CCommitPointer* operator->() { return &m_ptr; }\n"
3693+
" iterator& operator++() { ++m_ptr.m_place; return *this; }\n"
3694+
"};\n"
3695+
"iterator begin() {\n"
3696+
" iterator it;\n"
3697+
" it->m_place = 0;\n"
3698+
" return it;\n"
3699+
"}\n");
3700+
ASSERT_EQUALS("", errout_str());
3701+
}
3702+
3703+
void uninitvar_ownedArrowEscapes() {
3704+
checkUninitVar("struct Item { int value; };\n"
3705+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3706+
"Cursor f(bool b) {\n"
3707+
" Cursor it;\n"
3708+
" b && (it->value = 1);\n"
3709+
" return it;\n"
3710+
"}\n");
3711+
ASSERT_EQUALS("[test.cpp:6:12]: (error) Uninitialized struct member: it.item [uninitStructMember]\n", errout_str());
3712+
3713+
checkUninitVar("struct Item { int value; };\n"
3714+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3715+
"Cursor f() {\n"
3716+
" Cursor it;\n"
3717+
" (void)noexcept(it->value = 1);\n"
3718+
" return it;\n"
3719+
"}\n");
3720+
ASSERT_EQUALS("[test.cpp:6:12]: (error) Uninitialized struct member: it.item [uninitStructMember]\n", errout_str());
3721+
3722+
checkUninitVar("struct Item { int value; };\n"
3723+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3724+
"Cursor f() {\n"
3725+
" Cursor it;\n"
3726+
" int& r = it->value;\n"
3727+
" ++r;\n"
3728+
" return it;\n"
3729+
"}\n");
3730+
ASSERT_EQUALS("[test.cpp:7:12]: (error) Uninitialized struct member: it.item [uninitStructMember]\n", errout_str());
3731+
3732+
3733+
}
3734+
3735+
void valueFlowUninit_ownedArrow() { // #6572
3736+
valueFlowUninit("struct CCommitPointer { int m_place; };\n"
3737+
"struct iterator {\n"
3738+
" CCommitPointer m_ptr;\n"
3739+
" CCommitPointer& operator*() { return m_ptr; }\n"
3740+
" CCommitPointer* operator->() { return &m_ptr; }\n"
3741+
" iterator& operator++() { ++m_ptr.m_place; return *this; }\n"
3742+
"};\n"
3743+
"iterator begin() {\n"
3744+
" iterator it;\n"
3745+
" it->m_place = 0;\n"
3746+
" return it;\n"
3747+
"}\n");
3748+
ASSERT_EQUALS("", errout_str());
3749+
}
3750+
3751+
void valueFlowUninit_ownedArrowWrites() {
3752+
valueFlowUninit("struct Item { int value; };\n"
3753+
"struct Cursor { Item item; Item* operator->() { return &this->item; }\n"
3754+
" const Item* operator->() const { return &item; } };\n"
3755+
"int f(bool b) {\n"
3756+
" Cursor it;\n"
3757+
" if (b) it->value = 1; else it->value = 2;\n"
3758+
" it->value += 1;\n"
3759+
" return it->value;\n"
3760+
"}\n");
3761+
ASSERT_EQUALS("", errout_str());
3762+
}
3763+
3764+
void valueFlowUninit_ownedArrowReadModify() {
3765+
valueFlowUninit("struct Item { int value; };\n"
3766+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3767+
"Cursor f() {\n"
3768+
" Cursor it;\n"
3769+
" it->value++;\n"
3770+
" return it;\n"
3771+
"}\n");
3772+
ASSERT_EQUALS("[test.cpp:5:5]: (error) Uninitialized variable: it [uninitvar]\n", errout_str());
3773+
3774+
valueFlowUninit("struct Item { int value; };\n"
3775+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3776+
"Cursor f() {\n"
3777+
" Cursor it;\n"
3778+
" --it->value;\n"
3779+
" return it;\n"
3780+
"}\n");
3781+
ASSERT_EQUALS("[test.cpp:5:7]: (error) Uninitialized variable: it [uninitvar]\n", errout_str());
3782+
3783+
valueFlowUninit("struct Item { int value; };\n"
3784+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3785+
"Cursor f() {\n"
3786+
" Cursor it;\n"
3787+
" it->value += 1;\n"
3788+
" return it;\n"
3789+
"}\n");
3790+
ASSERT_EQUALS("[test.cpp:5:5]: (error) Uninitialized variable: it [uninitvar]\n", errout_str());
3791+
}
3792+
3793+
void valueFlowUninit_ownedArrowReceivers() {
3794+
valueFlowUninit("struct Item { int value; };\n"
3795+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3796+
"Cursor f() {\n"
3797+
" Cursor it;\n"
3798+
" it->value = it->value + 1;\n"
3799+
" return it;\n"
3800+
"}\n");
3801+
ASSERT_EQUALS("[test.cpp:5:17]: (error) Uninitialized variable: it [uninitvar]\n", errout_str());
3802+
3803+
valueFlowUninit("struct Item { int value; };\n"
3804+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3805+
"Cursor f() {\n"
3806+
" Cursor first, second;\n"
3807+
" first->value = second->value;\n"
3808+
" return first;\n"
3809+
"}\n");
3810+
ASSERT_EQUALS("[test.cpp:5:20]: (error) Uninitialized variable: second [uninitvar]\n", errout_str());
3811+
3812+
valueFlowUninit("struct Item { int value; };\n"
3813+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3814+
"Cursor f() {\n"
3815+
" Cursor first, second;\n"
3816+
" first->value = 1;\n"
3817+
" second->value = first->value;\n"
3818+
" return second;\n"
3819+
"}\n");
3820+
ASSERT_EQUALS("", errout_str());
3821+
}
3822+
3823+
void valueFlowUninit_ownedArrowConditional() {
3824+
valueFlowUninit("struct Item { int value; };\n"
3825+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3826+
"Cursor f(bool b) {\n"
3827+
" Cursor it;\n"
3828+
" if (b) it->value = 1;\n"
3829+
" return it;\n"
3830+
"}\n");
3831+
ASSERT_EQUALS("[test.cpp:5:9] -> [test.cpp:6:12]: (warning) Uninitialized variable: it.item [uninitvar]\n", errout_str());
3832+
3833+
valueFlowUninit("struct Item { int value; };\n"
3834+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3835+
"Cursor f() {\n"
3836+
" Cursor it;\n"
3837+
" auto l = [&it] { it->value = 1; };\n"
3838+
" return it;\n"
3839+
"}\n");
3840+
ASSERT_EQUALS("[test.cpp:6:12]: (error) Uninitialized variable: it [uninitvar]\n", errout_str());
3841+
3842+
// Reference writes are not projected into the owning object.
3843+
valueFlowUninit("struct Item { int value; };\n"
3844+
"struct Cursor { Item item; Item* operator->() { return &item; } };\n"
3845+
"void f() {\n"
3846+
" Cursor it;\n"
3847+
" int& r = it->value;\n"
3848+
" ++r;\n"
3849+
"}\n");
3850+
ASSERT_EQUALS("[test.cpp:5:14]: (error) Uninitialized variable: it [uninitvar]\n", errout_str());
3851+
}
3852+
36803853
void uninitvar15() { // #13685
36813854
const char code[] = "int f() {\n"
36823855
" int x;\n"

0 commit comments

Comments
 (0)