Skip to content

Fix issue 9302: FP uninitvar - struct accessed via pointer #2121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danmar merged 1 commit into from
Aug 29, 2019
Merged

Fix issue 9302: FP uninitvar - struct accessed via pointer #2121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/astutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,24 @@ std::vector<const Token*> astFlatten(const Token* tok, const char* op)
}


bool astHasToken(const Token* root, const Token * tok)
{
if (!root)
return false;
if (root == tok)
return true;
return astHasToken(root->astOperand1(), tok) || astHasToken(root->astOperand2(), tok);
}

bool astHasVar(const Token * tok, nonneg int varid)
{
if (!tok)
return false;
if (tok->varId() == varid)
return true;
return astHasVar(tok->astOperand1(), varid) || astHasVar(tok->astOperand2(), varid);
}

static bool astIsCharWithSign(const Token *tok, ValueType::Sign sign)
{
if (!tok)
Expand Down
4 changes: 4 additions & 0 deletions lib/astutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ void visitAstNodes(const Token *ast, std::function<ChildrenToVisit(const Token *

std::vector<const Token*> astFlatten(const Token* tok, const char* op);

bool astHasToken(const Token* root, const Token * tok);

bool astHasVar(const Token * tok, nonneg int varid);

/** Is expression a 'signed char' if no promotion is used */
bool astIsSignedChar(const Token *tok);
/** Is expression a 'char' if no promotion is used? */
Expand Down
56 changes: 56 additions & 0 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2025,6 +2025,52 @@ static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign
return true;
}

static bool isAliasOf(const Token *tok, nonneg int varid)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
if (!astIsPointer(tok))
return false;
for (const ValueFlow::Value &val : tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
if (val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
continue;
if (val.tokvalue->varId() == varid)
return true;
}
return false;
}

// Check if its an alias of the variable or is being aliased to this variable
static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list<ValueFlow::Value>& values)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
if (isAliasOf(tok, varid))
return true;
if (!var->isPointer())
return false;
// Search through non value aliases
for (const ValueFlow::Value &val : values) {
if (!val.isNonValue())
continue;
if (val.isLifetimeValue() && !val.isLocalLifetimeValue())
continue;
if (val.isLifetimeValue() && val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
continue;
if (!Token::Match(val.tokvalue, ".|&|*|%var%"))
continue;
if (astHasVar(val.tokvalue, tok->varId()))
return true;
}
return false;
}

static bool valueFlowForward(Token * const startToken,
const Token * const endToken,
const Variable * const var,
Expand Down Expand Up @@ -2708,6 +2754,15 @@ static bool valueFlowForward(Token * const startToken,
if (isVariableChanged(tok2, settings, tokenlist->isCPP())) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isUninitValue));
}
} else if (isAliasOf(var, tok2, varid, values) && isVariableChanged(tok2, settings, tokenlist->isCPP())) {
if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "Alias variable was modified.");
// Bail at the end of the statement if its in an assignment
const Token * top = tok2->astTop();
if (Token::Match(top, "%assign%") && astHasToken(top->astOperand1(), tok2))
returnStatement = true;
else
return false;
}

// Lambda function
Expand Down Expand Up @@ -5030,6 +5085,7 @@ static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatab
ValueFlow::Value uninitValue;
uninitValue.setKnown();
uninitValue.valueType = ValueFlow::Value::UNINIT;
uninitValue.tokvalue = vardecl;
std::list<ValueFlow::Value> values;
values.push_back(uninitValue);

Expand Down
13 changes: 13 additions & 0 deletions test/testuninitvar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4120,6 +4120,19 @@ class TestUninitVar : public TestFixture {
" return d(&c);\n"
"}\n");
ASSERT_EQUALS("", errout.str());

// # 9302
valueFlowUninit("struct VZ {\n"
" double typ;\n"
"};\n"
"void read() {\n"
" struct VZ vz;\n"
" struct VZ* pvz = &vz;\n"
" vz.typ = 42;\n"
" if (pvz->typ == 0)\n"
Copy link
Copy Markdown
Collaborator

@amai2012 amai2012 Aug 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the value for pvz->typ is still unknown but there is no warning?

Copy link
Copy Markdown
Contributor Author

@pfultz2 pfultz2 Aug 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? pvz->typ is equal to 42 although we dont track that value. We just ensure that it wont be set to Uninit.

Copy link
Copy Markdown
Collaborator

@amai2012 amai2012 Aug 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see

" return;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}

void uninitvar_memberfunction() {
Expand Down