C#: Misc Guards-related cleanup. #20738
Merged
+29
−169
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aschackmull
force-pushed
the
csharp/guards-misc
branch
from
d0f1bf5 to
eb93e8e
Compare
|
Dca is fairly uneventful. I dug into one of the weird alert diffs for NullMaybe, but it turned out to be due to missing types in buildless, so 🤷 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors control flow guard usage across the C# codebase, replacing the deprecated controlsBlock predicate with newer guard APIs. The main changes include:
- Removal of complex split-based control flow analysis from
ControlFlowElement.qll - Simplification of guard-based constant value tracking in dataflow analysis
- Migration to shared guard libraries (
Guards::Guard.valueControls) - Updates to boolean constant detection to use value-based checks
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
ConditionalBypassQuery.qll |
Replaced manual control flow checking with Guard.directlyControls API |
DataFlowPrivate.qll |
Generalized boolean constant tracking to support any constant value type using GuardValue |
Guards.qll |
Extended boolean constant detection beyond just BoolLiteral to any boolean-typed expression with a value |
ControlFlowElement.qll |
Deprecated complex controlsBlock predicate in favor of simpler implementation |
Caching.qll |
Removed caching dependency on deprecated controlsBlock predicate |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll
Show resolved
Hide resolved
aschackmull
removed
the
no-change-note-required
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What the title says.
There's a little bit of semantic impact from the first commit: Since the tweaked
isUnreachableInCallreferred to a slightly larger set of boolean constants than mere literals, I've expanded the input to Guards in the same way to recognize those constants.