Skip to content

C++: Define the argv flow source in terms the input parameter #11693

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.

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
merged 6 commits into from
Dec 20, 2022

Conversation

jketema
Copy link
Contributor

@jketema jketema commented Dec 14, 2022

This is currently missing some updates to expected test results, but I'd like to see the impact on DCA before fixing those.

I manually verified that the spurious cases from #11677 (comment) are now gone, and that the alert there only has the argv parameter as its source (as was to be expected).

The issue with main that is fixed in one of the test cases caused the argv parameter each of the mains to be reported as flow source in the other mains. The same issue should not occur in real codebases, as in those each main function and its parameters should be in a separate link target.

@github-actions github-actions bot added the C++ label Dec 14, 2022
@jketema jketema marked this pull request as ready for review December 15, 2022 14:03
@jketema jketema requested a review from a team as a code owner December 15, 2022 14:03
MathiasVP
MathiasVP previously approved these changes Dec 15, 2022
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM (with one minor comment).

@jketema jketema force-pushed the argv-param-flowsource branch from c9f2540 to fab912b Compare December 15, 2022 14:27
@jketema jketema force-pushed the argv-param-flowsource branch from fab912b to edd29f4 Compare December 19, 2022 12:51
@jketema jketema requested a review from a team December 19, 2022 12:51
@jketema jketema requested review from a team as code owners December 19, 2022 12:51
@jketema jketema changed the base branch from mathiasvp/replace-ast-with-ir-use-usedataflow to main December 19, 2022 12:51
@jketema jketema dismissed MathiasVP’s stale review December 19, 2022 12:51

The base branch was changed.

@jketema
Copy link
Contributor Author

jketema commented Dec 19, 2022

Rebased and now targeting main. Will re-run DCA.

@jketema
Copy link
Contributor Author

jketema commented Dec 19, 2022

Apologies for pinging many people.

@jketema jketema removed request for a team December 19, 2022 12:54
@jketema jketema removed the request for review from a team December 19, 2022 12:54
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

Awesome!

@MathiasVP MathiasVP merged commit cbe330e into github:main Dec 20, 2022
@jketema jketema deleted the argv-param-flowsource branch December 20, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants