Skip to content

Enable validateVariables(), fix fuzzing crash #6118

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
chrchr-github merged 5 commits into from
Mar 14, 2024
Merged

Enable validateVariables(), fix fuzzing crash #6118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25c562d
Enable validateVariables(), fix fuzzing crash
chrchr-github Mar 12, 2024
bef8765
syntaxError
chrchr-github Mar 12, 2024
ad2e8a8
Simplify
chrchr-github Mar 12, 2024
4a280ed
Comment
chrchr-github Mar 12, 2024
8a990c5
Merge branch 'main' into chr_varscope
chrchr-github Mar 13, 2024
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
28 changes: 13 additions & 15 deletions lib/symboldatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2146,18 +2146,18 @@ void SymbolDatabase::validateExecutableScopes() const
}

namespace {
const Function* getFunctionForArgumentvariable(const Variable * const var, const std::vector<const Scope *>& functionScopes)
const Function* getFunctionForArgumentvariable(const Variable * const var)
{
const std::size_t functions = functionScopes.size();
for (std::size_t i = 0; i < functions; ++i) {
const Scope* const scope = functionScopes[i];
const Function* const function = scope->function;
if (function) {
for (std::size_t arg=0; arg < function->argCount(); ++arg) {
if (var==function->getArgumentVar(arg))
return function;
if (const Scope* scope = var->nameToken()->scope()) {
Comment thread
danmar marked this conversation as resolved.
auto it = std::find_if(scope->functionList.begin(), scope->functionList.end(), [&](const Function& function) {
for (std::size_t arg = 0; arg < function.argCount(); ++arg) {
if (var == function.getArgumentVar(arg))
return true;
}
}
return false;
});
if (it != scope->functionList.end())
return &*it;
}
return nullptr;
}
Expand All @@ -2169,10 +2169,9 @@ void SymbolDatabase::validateVariables() const
const Variable * const var = *iter;
if (var) {
if (!var->scope()) {
const Function* function = getFunctionForArgumentvariable(var, functionScopes);
if (!var->isArgument() || (function && function->hasBody())) {
const Function* function = getFunctionForArgumentvariable(var);
if (!var->isArgument() || (!function || function->hasBody())) { // variables which only appear in a function declaration do not have a scope
throw InternalError(var->nameToken(), "Analysis failed (variable without scope). If the code is valid then please report this failure.", InternalError::INTERNAL);
//std::cout << "!!!Variable found without scope: " << var->nameToken()->str() << std::endl;
}
}
}
Expand All @@ -2184,8 +2183,7 @@ void SymbolDatabase::validate() const
if (mSettings.debugwarnings) {
validateExecutableScopes();
}
// TODO
//validateVariables();
validateVariables();
}

void SymbolDatabase::clangSetVariables(const std::vector<const Variable *> &variableList)
Expand Down
12 changes: 6 additions & 6 deletions lib/symboldatabase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1392,12 +1392,6 @@ class CPPCHECKLIB SymbolDatabase {
*/
void validate() const;

void validateExecutableScopes() const;
/**
* @brief Check variable list, e.g. variables w/o scope
*/
void validateVariables() const;

/** Set valuetype in provided tokenlist */
void setValueTypeInTokenList(bool reportDebugWarnings, Token *tokens=nullptr);

Expand Down Expand Up @@ -1465,6 +1459,12 @@ class CPPCHECKLIB SymbolDatabase {
void setValueType(Token* tok, const Variable& var, const SourceLocation &loc = SourceLocation::current());
void setValueType(Token* tok, const Enumerator& enumerator, const SourceLocation &loc = SourceLocation::current());

void validateExecutableScopes() const;
/**
* @brief Check variable list, e.g. variables w/o scope
*/
void validateVariables() const;

Tokenizer& mTokenizer;
const Settings &mSettings;
ErrorLogger *mErrorLogger;
Expand Down
3 changes: 3 additions & 0 deletions lib/tokenize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8406,6 +8406,9 @@ void Tokenizer::findGarbageCode() const
else if (Token::Match(tok, "%assign% [") && Token::simpleMatch(tok->linkAt(1), "] ;"))
syntaxError(tok, tok->str() + "[...];");

else if (Token::Match(tok, "[({<] %assign%"))
Copy link
Copy Markdown
Collaborator

@danmar danmar Mar 13, 2024

Choose a reason for hiding this comment

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

isn't this executed before the symboldatabase? I wonder if the changes in symboldatabase+testgarbage is technically separated?

Copy link
Copy Markdown
Collaborator Author

@chrchr-github chrchr-github Mar 13, 2024

Choose a reason for hiding this comment

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

Yes, those are separate changes. validateVariables() was (a) deactivated and (b) not doing sufficient checking to prevent the crash, so I added the syntaxError.

syntaxError(tok);

// UNKNOWN_MACRO(return)
if (tok->isKeyword() && Token::Match(tok, "throw|return )") && Token::Match(tok->linkAt(1)->previous(), "%name% ("))
unknownMacroError(tok->linkAt(1)->previous());
Expand Down
1 change: 1 addition & 0 deletions test/cli/fuzz-crash/crash-2490dbc1880f2d7883c1b634deee8da3805186f8
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
i a;m t(=a[]);m e(){a[]=0}
34 changes: 17 additions & 17 deletions test/testgarbage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1644,23 +1644,23 @@ class TestGarbage : public TestFixture {
}

void garbageCode205() {
checkCode("class CodeSnippetsEvent : public wxCommandEvent {\n"
"public :\n"
" CodeSnippetsEvent ( wxEventType commandType = wxEventType , int id = 0 ) ;\n"
" CodeSnippetsEvent ( const CodeSnippetsEvent & event ) ;\n"
"virtual wxEvent * Clone ( ) const { return new CodeSnippetsEvent ( * this ) ; }\n"
"private :\n"
" int m_SnippetID ;\n"
"} ;\n"
"const wxEventType wxEVT_CODESNIPPETS_GETFILELINKS = wxNewEventType ( )\n"
"CodeSnippetsEvent :: CodeSnippetsEvent ( wxEventType commandType , int id )\n"
": wxCommandEvent ( commandType , id ) {\n"
"}\n"
"CodeSnippetsEvent :: CodeSnippetsEvent ( const CodeSnippetsEvent & Event )\n"
": wxCommandEvent ( Event )\n"
", m_SnippetID ( 0 ) {\n"
"}"); // don't crash
(void)errout_str(); // we are not interested in the output
ASSERT_THROW(checkCode("class CodeSnippetsEvent : public wxCommandEvent {\n"
"public :\n"
" CodeSnippetsEvent ( wxEventType commandType = wxEventType , int id = 0 ) ;\n"
" CodeSnippetsEvent ( const CodeSnippetsEvent & event ) ;\n"
"virtual wxEvent * Clone ( ) const { return new CodeSnippetsEvent ( * this ) ; }\n"
"private :\n"
" int m_SnippetID ;\n"
"} ;\n"
"const wxEventType wxEVT_CODESNIPPETS_GETFILELINKS = wxNewEventType ( )\n"
"CodeSnippetsEvent :: CodeSnippetsEvent ( wxEventType commandType , int id )\n"
": wxCommandEvent ( commandType , id ) {\n"
"}\n"
"CodeSnippetsEvent :: CodeSnippetsEvent ( const CodeSnippetsEvent & Event )\n"
": wxCommandEvent ( Event )\n"
", m_SnippetID ( 0 ) {\n"
"}"),
InternalError);
}

void garbageCode206() {
Expand Down