Skip to content

C++: Fix implicit reads on cpp/cleartext-storage-database#11768

Merged
MathiasVP merged 1 commit intogithub:mathiasvp/replace-ast-with-ir-use-usedataflowfrom
MathiasVP:fix-implicit-reads-in-cleartext-sqlite-database
Dec 22, 2022
Merged

C++: Fix implicit reads on cpp/cleartext-storage-database#11768
MathiasVP merged 1 commit intogithub:mathiasvp/replace-ast-with-ir-use-usedataflowfrom
MathiasVP:fix-implicit-reads-in-cleartext-sqlite-database

Conversation

@MathiasVP
Copy link
Copy Markdown
Contributor

When there happened to be a conversion on the sink node (such as a conversion to void*) the implicit reads added by allowImplicitRead wouldn't fire since the type of the node wouldn't be a Class (but instead, it would be whatever the result of the conversion was).

This PR fixes the last FNs on the cpp/cleartext-storage-database query in the internal repo.

@MathiasVP MathiasVP requested a review from a team as a code owner December 21, 2022 09:47
@github-actions github-actions Bot added the C++ label Dec 21, 2022
@MathiasVP MathiasVP added no-change-note-required This PR does not need a change note depends on internal PR This PR should only be merged in sync with an internal Semmle PR labels Dec 21, 2022
@MathiasVP MathiasVP changed the title C++: Use fully converted sinks in cpp/cleartext-storage-database C++: Fix implicit reads on cpp/cleartext-storage-database Dec 21, 2022
Copy link
Copy Markdown
Contributor

@jketema jketema 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
Copy link
Copy Markdown
Contributor Author

The slowdown on DCA seems completely unrelated to this PR. It's a 15% slowdown on 1 project (which will be fixed by #11781). The stage timing report shows that all runs of cpp/cleartext-storage-database actually got faster, but basically every other query got slower. Since this cannot possibly be caused by this PR I'll merge it now with the expectation that the performance wobbliness will be fixed by #11781.

@MathiasVP MathiasVP merged commit 83d751b into github:mathiasvp/replace-ast-with-ir-use-usedataflow Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants