Skip to content
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

C++: Improve handling of re-use expressions #16289

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Conversation

jketema
Copy link
Contributor

@jketema jketema commented Apr 19, 2024

This will slightly brake IR generation for databases created with CodeQL 2.17.1. There are two changes:

  • Some type are different in the IR. This seems inconsequential.
  • The IR becomes inconsistent when a null pointer is being deleted. This should hopefully not occur at all in real world code.

@jketema jketema requested a review from a team as a code owner April 19, 2024 12:14
@jketema jketema added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Apr 19, 2024
@github-actions github-actions bot added the C++ label Apr 19, 2024
@jketema jketema added no-change-note-required This PR does not need a change note and removed C++ labels Apr 19, 2024
@jketema jketema changed the title C++: Improve handling of re-used qualifier in delete expressions C++: Improve handling of re-use expressions Apr 20, 2024
@github-actions github-actions bot added the C++ label Apr 21, 2024
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!

@jketema
Copy link
Contributor Author

jketema commented Apr 22, 2024

@MathiasVP note I added some additional text to the top-level comment.

@MathiasVP
Copy link
Contributor

@MathiasVP note I added some additional text to the top-level comment.

Thanks for adding that piece of context. I think that's fine. AFAIK, we don't have any queries that:

  • Use the IR, and
  • Look for null pointer dereferences like that.

So, as you said yourself, I don't think this will have any impact on real-world DBs.

@jketema jketema merged commit c5bdd5b into github:main Apr 22, 2024
10 of 14 checks passed
@jketema jketema deleted the reuse-improve branch April 22, 2024 08:34
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