Skip to content

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Feb 7, 2025

No description provided.

@firewave
Copy link
Collaborator Author

firewave commented Feb 7, 2025

All tests still pass with the early returns added.

Example output:

TestLeakAutoVar::doublefree9
ExprIdToken - no tok
setValue - no expr - 0@1
setValue - no expr - 0@1
ExprIdToken - no tok
setValue - no expr - 1@2
setValue - no expr - 1@2
setValue - no expr - 1
setValue - no expr - 0

Looks like this is related to unnamed parameters.

@pfultz2 please have a look.

@firewave firewave changed the title do not set values if there is no expression [skip ci] do not set values if there is no expression Feb 8, 2025
@firewave
Copy link
Collaborator Author

firewave commented Feb 8, 2025

Happens with unnamed parameters when a function body exists:

void cb(int) {}

void func()
{
    cb(0);
}
ExprIdToken - no tok
setValue - no expr - 0@1
ExprIdToken - no tok
solveExprValue - no expr
setValue - no expr - 0@1
ExprIdToken - no tok
solveExprValue - no expr


##file scratch_3.cpp
1: void cb ( int ) { }
2:
3: void func ( )
4: {
5: cb (@exprUNIQUE 0 ) ;
6: }



##Value flow
File scratch_3.cpp
Line 5
  0 always 0

Naming the parameter fixes it:

##file scratch_3.cpp
1: void cb ( int i@var1 ) { }
2:
3: void func ( )
4: {
5: cb (@exprUNIQUE 0 ) ;
6: }



##Value flow
File scratch_3.cpp
Line 5
  0 always 0

@firewave firewave force-pushed the expr branch 2 times, most recently from 7920895 to e22ec20 Compare August 8, 2025 12:56
@firewave
Copy link
Collaborator Author

firewave commented Aug 8, 2025

The MultiValueFlowAnalyzer happens in TestAutoVariables::testinvaliddealloc (among others). I assume that should not get such data so it possibly needs to be addressed earlier in the code.

Comment on lines +945 to +946
if (declId == 0)
continue; // TODO: should never happen?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This occurs in e.g. TestAutoVariables::testinvaliddealloc.

The Variable has no mNameToken and points to void* which appears to be the injected parameter for free(). The errorPath in the values seems to support this:

Address of variable taken here.
Calling function 'free', 1st argument '&c2' value is lifetime=c2

Not sure if something with the injecting is lacking data. But I do not think this needs to be solved in the scope of this PR.

@pfultz2 Any thoughts?

@firewave firewave force-pushed the expr branch 2 times, most recently from 5bd5804 to 1569700 Compare August 19, 2025 07:02
@firewave firewave marked this pull request as ready for review August 19, 2025 07:03
Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

spontanously this sounds good to me.
what is the motivation?
is there speedup or change of results or simpler code?

@firewave
Copy link
Collaborator Author

spontanously this sounds good to me. what is the motivation? is there speedup or change of results or simpler code?

I cannot remember how I came across this but I assume it was in the context of some of the other drafts PRs where I try to get rid of unnecessary calls.

Depending on the code this might have performance implications as less values are being generated.

@firewave firewave marked this pull request as draft August 25, 2025 16:12
@firewave firewave force-pushed the expr branch 2 times, most recently from 828da56 to cfb9e74 Compare September 8, 2025 07:50
@firewave firewave marked this pull request as ready for review September 8, 2025 08:07
Copy link

sonarqubecloud bot commented Sep 8, 2025

@firewave
Copy link
Collaborator Author

firewave commented Sep 8, 2025

I tested it with #7800 and there were no differences in the output.

@firewave firewave merged commit a86640d into danmar:main Sep 9, 2025
63 checks passed
@firewave firewave deleted the expr branch September 9, 2025 15:04
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.

2 participants