Python: Use TUnknown as the result of calls to methods with unknown return types
#2915
Conversation
To ease the rollout of this test, currently we only report missing points-to information for nodes that either - appear as an argument in a call to a function named `check`, or - appear inside a scope where the first line is annotated with a comment ending in "check". The idea behind the second version is that once we have points-to running at a level where no node inside a scope that _ought_ to have points-to is missing this information, we can simply remove all uses of `check(...)` from inside this scope, and annotate the entire scope with `# check`. Once this has been done for the entire file, we can then remove all the comments and just require _everything_ to be checked. Note that I don't expect all nodes to have the need for points-to information. For instance, there are nodes representing scope entry and exit, and for these it doesn't make sense to require that they "point-to" anything. Similarly, `NameNode` appearing in a "store" (i.e. as the left hand side of an assignment) do not strictly need to have points-to information, although it might be more intuitive if they did. Thus, the `relevant_node` predicate will almost certainly need to be extended to exclude these kinds of nodes.
|
I'm actually curious if this would have any performance implications. I don't have an intuitive feel for it, but I'm guessing that anything that changes points-to analysis might be worth investigating? 😄 |
|
You're right. I was considering this myself. I'll set up a differences job to see how it fares. |
tausbn
added
the
Awaiting evaluation
|
Dist-compare report: https://git.semmle.com/gist/taus/269cda979d7c3d34ebbbf12d4f860eca |
|
Okay, having now looked at the results, I see some problems with them. Currently, we do not consider an unknown instance of |
tausbn
changed the title
object as default return type for built-ins.TUnknown as the result of calls to methods with unknown return types
|
Note: I expect tests to start failing again, since a bunch of |
|
I think it looks good, but could we do an other dist-compare after the |
|
Indeed. That's why I left the "Awaiting Evaluation" label. Hopefully we should have fresh results tomorrow. 🙂 |
|
Latest report: https://git.semmle.com/gist/taus/07875fdb07c028c31a0a7c06a26eab44 |
|
Except for that merge conflict of course 😄 (probably just got introduced since I just merged 2 PRs) |
|
Oh no, tests are failing 😢 @tausbn |
tausbn
deleted the
python-add-points-to-for-missing-builtin-return-types
branch
TL;DR: We were not handling calls to methods like
dict.getcorrectly, and thus they would not receive a correspondingValue. The change is fairly simple: If we don't know the precise return type, just assume it'sobject. The pointed-toValuewill then be an unknown instance ofobject, which is fine.