Data flow: Track call contexts in parameterValueFlow
#17876
Merged
Conversation
This file contains 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
hvitved
changed the title
parameterFlowparameterValueFlow
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
6dd83e4
to
9037ff0
Compare
aschackmull
reviewed
Nov 11, 2024
Comment on lines
1443
to
1448
| // call may restrict the set of call sites that can be returned to | ||
| ctx2.(CachedCallContextSensitivity::CcReturn).isReturn(callable, call) | ||
| or | ||
| // call does not restrict the set of call sites that can be returned to | ||
| not exists(CachedCallContextSensitivity::CcReturn ret | ret.isReturn(callable, call)) and | ||
| CachedCallContextSensitivity::viableImplNotCallContextReducedReverse(ctx2) |
There was a problem hiding this comment.
This calculation that derives ctx2 from the (callable, call) pair is a deflating calculation, so it'll be more efficient to do it earlier. We can push it all the way into argumentValueFlowsThrough0.
There was a problem hiding this comment.
Also, it already exists as a bindingset-annotated predicate: getCallContextReturn, so no need to duplicate that logic.
aschackmull
reviewed
Nov 11, 2024
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
9037ff0
to
045d424
Compare
aschackmull
reviewed
Nov 20, 2024
| Cand::argumentValueFlowsThroughCand(arg, _, _) | ||
| } | ||
|
|
||
| pragma[nomagic] | ||
| private predicate argumentValueFlowsThrough0( | ||
| DataFlowCall call, ArgNode arg, ReturnKind kind, ReadStepTypesOption read, string model | ||
| DataFlowCall call, DataFlowCallable callable, ArgNode arg, ReturnKind kind, |
There was a problem hiding this comment.
The callable column is unused and can be pushed into the exists.
aschackmull
reviewed
Nov 20, 2024
|
|
||
| pragma[nomagic] | ||
| private predicate argumentValueFlowsThrough( | ||
| DataFlowCall call, DataFlowCallable callable, ArgNode arg, ReadStepTypesOption read, |
There was a problem hiding this comment.
The call column can be pushed into the exists here.
aschackmull
reviewed
Nov 20, 2024
| @@ -628,6 +628,8 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> { | |||
| override string toString() { | |||
| exists(DataFlowCall call | this = TReturn(_, call) | result = "CcReturn(" + call + ")") | |||
| } | |||
|
|
|||
| predicate isReturn(DataFlowCallable c, DataFlowCall call) { this = TReturn(c, call) } | |||
There was a problem hiding this comment.
This can be removed now.
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
045d424
to
fc1bc3a
Compare
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
fc1bc3a
to
3f56fc9
Compare
aschackmull
approved these changes
Nov 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When investigating a Ruby performance issue, I found that we produced some false positive return-through-parameter flow, which is because we are not tracking call contexts in the
parameterValueFlowlogic. This PR adds tracking of call contexts.