Skip to content

C++: Take into account the delta at the final sink in cpp/invalid-pointer-deref#13320

Merged
MathiasVP merged 5 commits intogithub:mainfrom
jketema:ptr-deref-dedup
May 31, 2023
Merged

C++: Take into account the delta at the final sink in cpp/invalid-pointer-deref#13320
MathiasVP merged 5 commits intogithub:mainfrom
jketema:ptr-deref-dedup

Conversation

@jketema
Copy link
Copy Markdown
Contributor

@jketema jketema commented May 30, 2023

While here, fix the dataflow configuration names as they occur in the comments, and add the nodes query predicate, which seemed to be missing. Commit-by-commit review is recommended.

@github-actions github-actions Bot added the C++ label May 30, 2023
MathiasVP
MathiasVP previously approved these changes May 30, 2023
Copy link
Copy Markdown
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 once DCA is happy!

joinOn2(node1.asPathNode3(), node2.asSinkNode(), _, _)
}

query predicate nodes(MergedPathNode n, string key, string val) {
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.

FWIW nodes is implicitly generated by the CLI in a path-problem query based on the edges predicate. So strictly speaking this query predicate is redundant (since we're not using it to modify the string representation of the nodes). However, I do like having the nodes predicate in here since it can be useful to append concat(n.getAQlClass(), ", ") for debugging purposes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, learnt something new. Thanks.

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.

I don't blame you. I don't think this is documented anywhere 😅

@jketema
Copy link
Copy Markdown
Contributor Author

jketema commented May 31, 2023

DCA alert changes look correct, except for the CMake one, but this one is related to an FP (without k offset) on the same line. I've added a test case for this FP to this PR.

@jketema jketema force-pushed the ptr-deref-dedup branch from 287c1b3 to ace7b6b Compare May 31, 2023 09:55
@jketema jketema marked this pull request as ready for review May 31, 2023 09:55
@jketema jketema requested a review from a team as a code owner May 31, 2023 09:55
Copy link
Copy Markdown
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!

@MathiasVP MathiasVP merged commit 3d9c282 into github:main May 31, 2023
@jketema jketema deleted the ptr-deref-dedup branch May 31, 2023 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants