ATM: undo unsound performance optimizations #8470
Draft
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.
For an ordinary path-problem query, it is a requirement that at least one sink exists, otherwise there is nothing to alert on.
Thus the optimization with checking
isSink(_, this, _)
pruning inConfiguration::hasFlow
is sound.For programs without any relevant sinks (which is likely to be the case when ATM is used), the
Configuration::hasFlow
calls will not hold due to the above pruning step.The optimizations removed in this commit are thus unsound for programs without any relevant sinks.
Relevant pings: @henrymercer @adityasharad
Hopefully performance works out without this 🤞🏻🤞🏻.
Impact
This means that it now is possible to predict sinks in programs without any known sinks(!!!).
Testing
Testing this is presumably tricky at the moment due to the required presence of a mlmodel. So I include my own manual test case for completeness and posterity.
Example database sources
Example query that produces no/some results without/with this PR.
Confirming that
hasFlow(...)
works for the full ATM query without any known sinksRun
TaintedPathATM.ql
.