Skip to content

C++: Reduce duplication in cpp/uncontrolled-process-operation #20059

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

Conversation

MathiasVP
Copy link
Contributor

Having both asExpr() and asIndirectExpr() doesn't appear to change any results. This was a leftover from back when we ported the default-taint-tracking queries over, and we still had a few remaining bugs in asExpr and asIndirectExpr, and probably some remaining pointer sources that needed to be converted to pointee sources.

I've manually verified that all the removed results from DCA are due to deduplication.

@Copilot Copilot AI review requested due to automatic review settings July 15, 2025 15:58
@MathiasVP MathiasVP requested a review from a team as a code owner July 15, 2025 15:58
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Jul 15, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the UncontrolledProcessOperation.ql query by removing redundant code that was checking for process operation arguments using both direct and indirect expression patterns. The change eliminates duplication by keeping only the asIndirectExpr() method, as having both methods doesn't change the query results and was a leftover from earlier porting work.

Key Changes

  • Removed redundant expression matching pattern in process operation detection
  • Simplified argument matching to use only indirect expressions

@github-actions github-actions bot added the C++ label Jul 15, 2025
Copy link
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

@jketema jketema merged commit 58aa758 into github:main Jul 15, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ 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