Dataflow: Strengthen tracked types.#13083
Conversation
| */ | ||
| predicate expectsContent(Node n, ContentSet c) { none() } | ||
|
|
||
| predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() } |
There was a problem hiding this comment.
It would be super helpful to have some QLDoc to explain what "strong" means in this context. Is it (roughly) equivalent to saying that t1 is a strict subtype of t2?
There was a problem hiding this comment.
Yes. I postponed qldoc a bit, since I'll hopefully soon rewrite the language-to-library api to go through a signature, such that the entire library can be a parameterised module in a qlpack - at that point all these predicates will get language-agnostic qldoc in the signature.
c069df3 to
7497b5b
Compare
Click to show differences in coveragejavaGenerated file changes for java
- java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2,1
+ java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,3, |
Click to show differences in coveragejavaGenerated file changes for java
- java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2,1
+ java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,3, |
Click to show differences in coveragejavaGenerated file changes for java
- java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2,1
+ java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,3, |
b218ac6 to
6351b68
Compare
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,3,679,168,39,,9,,,13
+ Java Standard Library,``java.*``,3,678,168,39,,9,,,13
- Totals,,246,9119,1957,174,10,113,33,1,361
+ Totals,,246,9118,1957,174,10,113,33,1,361
- java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2,1
+ java.sql,13,,2,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2, |
hvitved
left a comment
There was a problem hiding this comment.
LGTM. Two minor comments.
| if castingNodeEx(node) | ||
| then | ||
| exists(Typ nt | nt = node.getDataFlowType() | | ||
| if typeStrongerThan(nt, t0) then t = nt else (compatibleTypes(nt, t0) and t = t0) | ||
| ) | ||
| else t = t0 |
There was a problem hiding this comment.
Perhaps factor this duplicated pattern out into a helper predicate?
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,3,679,168,39,,9,,,13
+ Java Standard Library,``java.*``,3,678,168,39,,9,,,13
- Totals,,246,9119,1957,174,10,113,33,1,361
+ Totals,,246,9118,1957,174,10,113,33,1,361
- java.sql,13,,3,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2,1
+ java.sql,13,,2,,,,,,,,4,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,2, |
hvitved
left a comment
There was a problem hiding this comment.
🎉 Let's wait for a final QA run before merging.
1db8ccc to
78b5020
Compare
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,3,683,172,64,,9,,,17
+ Java Standard Library,``java.*``,3,682,172,64,,9,,,17
- Totals,,255,9190,1975,244,10,122,33,1,382
+ Totals,,255,9189,1975,244,10,122,33,1,382
- java.sql,13,,3,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2,1
+ java.sql,13,,2,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2, |
78b5020 to
92b07dc
Compare
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,3,683,172,64,,9,,,17
+ Java Standard Library,``java.*``,3,682,172,64,,9,,,17
- Totals,,255,9194,1985,251,10,122,33,1,385
+ Totals,,255,9193,1985,251,10,122,33,1,385
- java.sql,13,,3,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2,1
+ java.sql,13,,2,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2, |
92b07dc to
f2b9e01
Compare
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,3,683,184,76,,9,,,17
+ Java Standard Library,``java.*``,3,682,184,76,,9,,,17
- Totals,,255,9199,1997,263,10,122,33,1,385
+ Totals,,255,9198,1997,263,10,122,33,1,385
- java.sql,13,,3,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2,1
+ java.sql,13,,2,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2, |
f2b9e01 to
68f1e40
Compare
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,3,683,184,76,,9,,,17
+ Java Standard Library,``java.*``,3,682,184,76,,9,,,17
- Totals,,255,9199,1997,263,10,122,33,1,385
+ Totals,,255,9198,1997,263,10,122,33,1,385
- java.sql,13,,3,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2,1
+ java.sql,13,,2,,,,,,,,,,,,,,,,,,,,,,,,,4,,9,,,,,,,,2, |
yoff
left a comment
There was a problem hiding this comment.
Proxy approval after rebase
This adds support for type strengthening in the data flow library. Resolves https://github.com/github/codeql-team/issues/1773.
Initial tests indicated that strengthening types already from stage 3 when type pruning is introduced came at a potentially steep performance cost, thus stage 3 is kept as-is and strengthening is introduced in stage 4, which appeared to perform much better. Note that we cannot delay further than stage 4, since stage 5 constructs a depth 2 access path approximation that makes use of the types that are traced to store steps in stage 4, so from that point and onwards the traced types must be consistent between stages.