Skip to content

Commit 1b4e2fe

Browse files
joefarebrotherjoefarebrother
committed
Change implenetation of missing calls to use getASuperCallTarget, and change alerts to alert on the class and provide clearer information, using optional location links.
1 parent 2faf67d commit 1b4e2fe

File tree

3 files changed

+146
-65
lines changed

3 files changed

+146
-65
lines changed

‎python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll‎

Lines changed: 93 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,38 @@
33
import python
44
import semmle.python.ApiGraphs
55
import semmle.python.dataflow.new.internal.DataFlowDispatch
6+
import codeql.util.Option
67

78
predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, string name) {
89
exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls |
910
meth.getName() = name and
1011
meth.getScope() = cls and
1112
call1.asExpr() != call2.asExpr() and
12-
calledMulti = getASuperCallTarget(cls, meth, call1) and
13-
calledMulti = getASuperCallTarget(cls, meth, call2) and
13+
calledMulti = getASuperCallTargetFromCall(cls, meth, call1, name) and
14+
calledMulti = getASuperCallTargetFromCall(cls, meth, call2, name) and
1415
nonTrivial(calledMulti)
1516
)
1617
}
1718

18-
Function getASuperCallTarget(Class mroBase, Function meth, DataFlow::MethodCallNode call) {
19+
Function getASuperCallTargetFromCall(
20+
Class mroBase, Function meth, DataFlow::MethodCallNode call, string name
21+
) {
1922
meth = call.getScope() and
2023
getADirectSuperclass*(mroBase) = meth.getScope() and
21-
call.calls(_, meth.getName()) and
22-
exists(Function target | (result = target or result = getASuperCallTarget(mroBase, target, _)) |
24+
meth.getName() = name and
25+
call.calls(_, name) and
26+
exists(Class targetCls | result = getASuperCallTargetFromClass(mroBase, targetCls, name) |
2327
superCall(call, _) and
24-
target =
25-
findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(meth.getScope(),
26-
mroBase), mroBase, meth.getName())
28+
targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase)
2729
or
28-
exists(Class called |
29-
callsMethodOnClassWithSelf(meth, call, called, _) and
30-
target = findFunctionAccordingToMroKnownStartingClass(called, mroBase, meth.getName())
31-
)
30+
callsMethodOnClassWithSelf(meth, call, targetCls, _)
31+
)
32+
}
33+
34+
Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) {
35+
exists(Function target |
36+
target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and
37+
(result = target or result = getASuperCallTargetFromCall(mroBase, target, _, name))
3238
)
3339
}
3440

@@ -78,31 +84,83 @@ predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) {
7884
)
7985
}
8086

81-
predicate mayProceedInMro(Class a, Class b, Class mroStart) {
82-
b = getNextClassInMroKnownStartingClass(a, mroStart)
83-
or
84-
exists(Class mid |
85-
mid = getNextClassInMroKnownStartingClass(a, mroStart) and
86-
mayProceedInMro(mid, b, mroStart)
87-
)
88-
}
89-
90-
predicate missingCallToSuperclassMethod(
91-
Function base, Function shouldCall, Class mroStart, string name
92-
) {
87+
predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string name) {
9388
base.getName() = name and
9489
shouldCall.getName() = name and
95-
not callsSuper(base) and
96-
not callsMethodOnUnknownClassWithSelf(base, name) and
90+
base = getADirectSuperclass*(base.getScope()) and
91+
not shouldCall = getASuperCallTargetFromClass(base, base, name) and
9792
nonTrivial(shouldCall) and
98-
base.getScope() = getADirectSuperclass*(mroStart) and
99-
mayProceedInMro(base.getScope(), shouldCall.getScope(), mroStart) and
100-
not exists(Class called |
101-
(
102-
callsMethodOnClassWithSelf(base, _, called, name)
103-
or
104-
callsMethodOnClassWithSelf(findFunctionAccordingToMro(mroStart, name), _, called, name)
105-
) and
106-
shouldCall.getScope() = getADirectSuperclass*(called)
93+
// "Benefit of the doubt" - if somewhere in the chain we call an unknown superclass, assume all the necessary parent methods are called from it
94+
not callsMethodOnUnknownClassWithSelf(getASuperCallTargetFromClass(base, base, name), name)
95+
}
96+
97+
predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCall, string name) {
98+
missingCallToSuperclassMethod(base, shouldCall, name) and
99+
not exists(Class subBase |
100+
subBase = getADirectSubclass+(base) and
101+
missingCallToSuperclassMethod(subBase, shouldCall, name)
102+
) and
103+
not exists(Function superShouldCall |
104+
superShouldCall.getScope() = getADirectSuperclass+(shouldCall.getScope()) and
105+
missingCallToSuperclassMethod(base, superShouldCall, name)
106+
)
107+
}
108+
109+
Function getPossibleMissingSuper(Class base, Function shouldCall, string name) {
110+
missingCallToSuperclassMethod(base, shouldCall, name) and
111+
exists(Function baseMethod |
112+
baseMethod.getScope() = base and
113+
baseMethod.getName() = name and
114+
// the base method calls super, so is presumably expecting every method called in the MRO chain to do so
115+
callsSuper(baseMethod) and
116+
// result is something that does get called in the chain
117+
result = getASuperCallTargetFromClass(base, base, name) and
118+
// it doesn't call super
119+
not callsSuper(result) and
120+
// if it did call super, it would resolve to the missing method
121+
shouldCall =
122+
findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(result
123+
.getScope(), base), base, name)
107124
)
108125
}
126+
127+
private module FunctionOption = Option<Function>;
128+
129+
class FunctionOption extends FunctionOption::Option {
130+
/**
131+
* Holds if this element is at the specified location.
132+
* The location spans column `startcolumn` of line `startline` to
133+
* column `endcolumn` of line `endline` in file `filepath`.
134+
* For more information, see
135+
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
136+
*/
137+
predicate hasLocationInfo(
138+
string filepath, int startline, int startcolumn, int endline, int endcolumn
139+
) {
140+
this.asSome()
141+
.getLocation()
142+
.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
143+
or
144+
this.isNone() and
145+
filepath = "" and
146+
startline = 0 and
147+
startcolumn = 0 and
148+
endline = 0 and
149+
endcolumn = 0
150+
}
151+
152+
string getQualifiedName() {
153+
result = this.asSome().getQualifiedName()
154+
or
155+
this.isNone() and
156+
result = ""
157+
}
158+
}
159+
160+
bindingset[name]
161+
FunctionOption getPossibleMissingSuperOption(Class base, Function shouldCall, string name) {
162+
result.asSome() = getPossibleMissingSuper(base, shouldCall, name)
163+
or
164+
not exists(getPossibleMissingSuper(base, shouldCall, name)) and
165+
result.isNone()
166+
}

‎python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,35 @@
1515
import python
1616
import MethodCallOrder
1717

18-
predicate missingCallToSuperclassDel(Function base, Function shouldCall, Class mroStart) {
19-
missingCallToSuperclassMethod(base, shouldCall, mroStart, "__del__")
18+
Function getDelMethod(Class c) {
19+
result = c.getAMethod() and
20+
result.getName() = "__del__"
2021
}
2122

22-
from Function base, Function shouldCall, Class mroStart, string msg
23+
from Class base, Function shouldCall, FunctionOption possibleIssue, string msg
2324
where
24-
missingCallToSuperclassDel(base, shouldCall, mroStart) and
25-
(
26-
// Simple case: the method that should be called is directly overridden
27-
mroStart = base.getScope() and
28-
msg = "This delete method does not call $@, which may leave $@ not properly cleaned up."
29-
or
30-
// Only alert for a different mro base if there are no alerts for direct overrides
31-
not missingCallToSuperclassDel(base, _, base.getScope()) and
32-
msg =
33-
"This delete method does not call super().__del__, which may cause $@ to be missed during the cleanup of $@."
25+
not exists(Function newMethod | newMethod = base.getAMethod() and newMethod.getName() = "__new__") and
26+
exists(FunctionOption possiblyMissingSuper |
27+
missingCallToSuperclassMethodRestricted(base, shouldCall, "__del__") and
28+
possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__del__") and
29+
(
30+
not possiblyMissingSuper.isNone() and
31+
possibleIssue = possiblyMissingSuper and
32+
msg =
33+
"This class does not call $@ during destruction. ($@ may be missing a call to super().__del__)"
34+
or
35+
possiblyMissingSuper.isNone() and
36+
(
37+
possibleIssue.asSome() = getDelMethod(base) and
38+
msg =
39+
"This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__)"
40+
or
41+
not getDelMethod(base) and
42+
possibleIssue.isNone() and
43+
msg =
44+
"This class does not call $@ during destruction. (The class lacks an __del__ method to ensure every base class __del__ is called.)"
45+
)
46+
)
3447
)
35-
select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName()
48+
select base, msg, shouldCall, shouldCall.getQualifiedName(), possibleIssue,
49+
possibleIssue.getQualifiedName()

‎python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,30 @@
1414
import python
1515
import MethodCallOrder
1616

17-
predicate missingCallToSuperclassInit(Function base, Function shouldCall, Class mroStart) {
18-
missingCallToSuperclassMethod(base, shouldCall, mroStart, "__init__")
19-
}
20-
21-
from Function base, Function shouldCall, Class mroStart, string msg
17+
from Class base, Function shouldCall, FunctionOption possibleIssue, string msg
2218
where
23-
missingCallToSuperclassInit(base, shouldCall, mroStart) and
24-
(
25-
// Simple case: the method that should be called is directly overridden
26-
mroStart = base.getScope() and
27-
msg = "This initialization method does not call $@, which may leave $@ partially initialized."
28-
or
29-
// Only alert for a different mro base if there are no alerts for direct overrides
30-
not missingCallToSuperclassInit(base, _, base.getScope()) and
31-
msg =
32-
"This initialization method does not call super().__init__, which may cause $@ to be missed during the initialization of $@."
19+
not exists(Function newMethod | newMethod = base.getAMethod() and newMethod.getName() = "__new__") and
20+
exists(FunctionOption possiblyMissingSuper |
21+
missingCallToSuperclassMethodRestricted(base, shouldCall, "__init__") and
22+
possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__init__") and
23+
(
24+
not possiblyMissingSuper.isNone() and
25+
possibleIssue = possiblyMissingSuper and
26+
msg =
27+
"This class does not call $@ during initialization. ($@ may be missing a call to super().__init__)"
28+
or
29+
possiblyMissingSuper.isNone() and
30+
(
31+
possibleIssue.asSome() = base.getInitMethod() and
32+
msg =
33+
"This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__)"
34+
or
35+
not exists(base.getInitMethod()) and
36+
possibleIssue.isNone() and
37+
msg =
38+
"This class does not call $@ during initialization. (The class lacks an __init__ method to ensure every base class __init__ is called.)"
39+
)
40+
)
3341
)
34-
select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName()
42+
select base, msg, shouldCall, shouldCall.getQualifiedName(), possibleIssue,
43+
possibleIssue.getQualifiedName()

0 commit comments

Comments
 (0)