Skip to content

Commit

Permalink
fix #13098: False negative: memory leak not found when deallocation i…
Browse files Browse the repository at this point in the history
…s conditional (regression) (#6989)
  • Loading branch information
ludviggunne authored Nov 6, 2024
1 parent bdfaff6 commit 363a596
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
14 changes: 12 additions & 2 deletions lib/checkleakautovar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,16 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken,

// if/else
else if (Token::simpleMatch(tok, "if (")) {

bool skipIfBlock = false;
bool skipElseBlock = false;
const Token *condTok = tok->astSibling();

if (condTok->hasKnownIntValue()) {
skipIfBlock = !condTok->getKnownIntValue();
skipElseBlock = !skipIfBlock;
}

// Parse function calls inside the condition

const Token * closingParenthesis = tok->linkAt(1);
Expand Down Expand Up @@ -565,13 +575,13 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken,
return ChildrenToVisit::none;
});

if (!checkScope(closingParenthesis->next(), varInfo1, notzero, recursiveCount)) {
if (!skipIfBlock && !checkScope(closingParenthesis->next(), varInfo1, notzero, recursiveCount)) {
varInfo.clear();
continue;
}
closingParenthesis = closingParenthesis->linkAt(1);
if (Token::simpleMatch(closingParenthesis, "} else {")) {
if (!checkScope(closingParenthesis->tokAt(2), varInfo2, notzero, recursiveCount)) {
if (!skipElseBlock && !checkScope(closingParenthesis->tokAt(2), varInfo2, notzero, recursiveCount)) {
varInfo.clear();
return false;
}
Expand Down
15 changes: 14 additions & 1 deletion test/testleakautovar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class TestLeakAutoVar : public TestFixture {
TEST_CASE(return8);
TEST_CASE(return9);
TEST_CASE(return10);
TEST_CASE(return11); // #13098

// General tests: variable type, allocation type, etc
TEST_CASE(test1);
Expand Down Expand Up @@ -1686,7 +1687,7 @@ class TestLeakAutoVar : public TestFixture {
" free(p);\n"
" if (q == NULL)\n"
" return;\n"
" free(q)\n"
" free(q);\n"
"}");
ASSERT_EQUALS("[test.c:3] -> [test.c:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout_str());
}
Expand Down Expand Up @@ -2745,6 +2746,18 @@ class TestLeakAutoVar : public TestFixture {
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Returning/dereferencing 'p' after it is deallocated / released\n", errout_str());
}

void return11() { // #13098
check("char malloc_memleak(void) {\n"
" bool flag = false;\n"
" char *ptr = malloc(10);\n"
" if (flag) {\n"
" free(ptr);\n"
" }\n"
" return 'a';\n"
"}\n", true);
ASSERT_EQUALS("[test.cpp:7]: (error) Memory leak: ptr\n", errout_str());
}

void test1() {
check("void f(double*&p) {\n" // 3809
" p = malloc(0x100);\n"
Expand Down

0 comments on commit 363a596

Please sign in to comment.