JS: Follow use-use flow after a post-update #17535
Open
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
asgerf
force-pushed
the
jss/use-use-flow
branch
from
7fbde8e
to
4afd36a
Compare
asgerf
force-pushed
the
jss/use-use-flow
branch
4 times, most recently
from
8ffa01a
to
a6f18a1
Compare
asgerf
force-pushed
the
jss/use-use-flow
branch
from
dcd8364
to
685e917
Compare
We need these to return the dominator instead of declaring it in the parameter list, so that we can use it directly to fulfill part of the signature for the SSA library. We can't rewrite it with an inline predicate since the SSA module calls with a transitive closure '*', which does not permit inline predicates.
JS: Remove with statement comment
…public This exposes the predicates publicly, but will be hidden again in the next commit.
Indentation will be fixed in next commit
Only formatting changes
asgerf
force-pushed
the
jss/use-use-flow
branch
from
685e917
to
3910699
Compare
asgerf
force-pushed
the
jss/use-use-flow
branch
from
6eb467c
to
17b8e4e
Compare
For a constructor call, the return value acts as the post-update node for the 'this' argument. The fact that constructor calls are sometimes PostUpdateNodes causes some of these harmless alerts. The warnings have disappeared in some cases because we no longer target getALocalSource() so the target is no longer the constructor call.
Fixed in the next commit
The Immutable model uses the 'd' and 'f' properties to model Map content, but the test doesn't actually mention those properties, so they were missing from the PropertyName class. The flow was previously found spuriously by the regular Map model, which also adds flow through the get/set calls. This flow is however no longer found since it relied on a step from post-update back to getALocalSource which is no longer present.
This causes less wobbles in test outputs
…ot tracked precisely With the capture library we sometimes bails out of handling certain functions for scalability reasons. This means we have a notion of "captured but imprecisely-tracked" variables and 'this'. In these cases we go back to propagating flow from a post-update node to the local source.
This is a bit of a bandaid to cover issues with the push() method on next/router being treated as an array push, which causes it to flow into other taint sources.
These were marked as 'NOT OK' in the test file, but weren't previously flagged for some reason
Only changes to nodes/edges for various reasons, no actual result changes
asgerf
force-pushed
the
jss/use-use-flow
branch
from
4410401
to
2fb1084
Compare
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.
Previously there was flow from a post-update node back to
getALocalSource(), mirroring how side effects were tracked in the legacy data flow library. This PR changes it so use-use flow is followed after a post-update node. The use-use chain is represented as a new type of data flow node, which is hidden so as not to get confusing data flow paths.Note however that there is still direct flow from the definition site to all uses/refinements; the use-use chain is only followed after a post-update.
The use-use chain is constructed using the shared SSA library. But we can't remove the old SSA library yet, as it is directly depended on by various components such as type inference. This is also why still have def-use flow, and only switch to use-use after a post-update.
The PR also contains some other changes that were needed to make this switch feasible:
thisis now tracked as a captured variable, alongside regular variables.thisnow have an explicit data flow node, so they can be part of the use-use chain.supercalls and instance field initialisers contain an implicitthisuse.Evaluation: