Skip to content

Fix #11311 Do not search for null pointer in dead code#6442

Merged
chrchr-github merged 5 commits intocppcheck-opensource:mainfrom
francois-berder:fix-11311
Jun 25, 2024
Merged

Fix #11311 Do not search for null pointer in dead code#6442
chrchr-github merged 5 commits intocppcheck-opensource:mainfrom
francois-berder:fix-11311

Conversation

@francois-berder
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread lib/checknullpointer.h Outdated
@francois-berder
Copy link
Copy Markdown
Collaborator Author

@chrchr-github
I fixed this typo.

Unfortunately, I could not find a way to skip tokens of the "false" branch when processing the ternary operator. I can see a function named skipTernaryOp but that does not seem to do what I want. Any suggestions how to skip tokens of a ternary operator branch safely?

@chrchr-github
Copy link
Copy Markdown
Collaborator

What kind of code do you have in mind?
There is no warning for e.g.

int f(const int* p) {
	if (p)
		return 0;
	return 0 ? *p : 1;
}

@pfultz2
Copy link
Copy Markdown
Contributor

pfultz2 commented May 27, 2024

findTokensSkipDeadCode is the function you want to use. It should skip the ternary operator as well.

@francois-berder francois-berder marked this pull request as draft May 31, 2024 13:53
@francois-berder
Copy link
Copy Markdown
Collaborator Author

Converting this PR to draft.
I am trying to handle more scenarios such as else if branch, nested branches, etc.

Comment thread lib/checknullpointer.cpp Outdated
@pfultz2
Copy link
Copy Markdown
Contributor

pfultz2 commented Jun 1, 2024

There should be no need to reinvent the wheel here. findTokensSkipDeadCode should work for nested branches, else if, early returns, lambda captures, and ternary operator, and it can be easily extended to handle unevaluated functions as well.

@francois-berder
Copy link
Copy Markdown
Collaborator Author

There should be no need to reinvent the wheel here. findTokensSkipDeadCode should work for nested branches, else if, early returns, lambda captures, and ternary operator, and it can be easily extended to handle unevaluated functions as well.

You are right, using findTokensSkipDeadCode makes it much easier to fix this bug.

@francois-berder francois-berder marked this pull request as ready for review June 2, 2024 19:23
@francois-berder
Copy link
Copy Markdown
Collaborator Author

@pfultz2 @chrchr-github I think this PR is now again ready for review.

Comment thread lib/checknullpointer.cpp Outdated
return true;

while (tok) {
if (isUnevaluated(tok->previous()))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could you clarify what will tok->previous() be. if there is parenthesis like ...)tok.. or something then you want to return true?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I want to make sure that the following code does not trigger a warning.

void f(type* p) {
   x(sizeof ( (p[0])));
    if (!p)
        ;
}

I need to check whether the variable p is unevaluated so I used a similar method found in checkuninitvar.cpp (see method CheckUninitVar::checkExpr).

Comment thread lib/checknullpointer.cpp Outdated
void CheckNullPointer::nullPointerByDeRefAndChec()
static bool isPointerUnevaluated(const Token *tok)
{
if (isUnevaluated(tok))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be checked in findTokensSkipDeadCodeImpl function.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@danmar @pfultz2 I updated my PR to move this in findTokensSkipDeadAndUnevaluatedCode function because I was not very happy with the way I detected unevaluated pointers.

@francois-berder
Copy link
Copy Markdown
Collaborator Author

@danmar @pfultz2 @chrchr-github ping

@chrchr-github chrchr-github merged commit 1668b0b into cppcheck-opensource:main Jun 25, 2024
@francois-berder francois-berder deleted the fix-11311 branch June 27, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants