From 38af3ac92567d257cbb0faaf9ebbfeb361dd583b Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 30 Jun 2025 16:21:17 +0100 Subject: [PATCH] Update missing call to init --- .../CallsToInitDel/MethodCallOrder.qll | 105 ++++++++++++------ .../CallsToInitDel/MissingCallToInit.ql | 31 +++--- 2 files changed, 89 insertions(+), 47 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index b911ee0183e..b14c6138015 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -1,6 +1,8 @@ -deprecated module; +/** Definitions for reasoning about multiple or missing calls to superclass methods. */ import python +import semmle.python.ApiGraphs +import semmle.python.dataflow.new.internal.DataFlowDispatch // Helper predicates for multiple call to __init__/__del__ queries. pragma[noinline] @@ -36,42 +38,77 @@ predicate multiple_calls_to_superclass_method(ClassObject self, FunctionObject m ) } -/** Holds if all attributes called `name` can be inferred to be methods. */ -private predicate named_attributes_not_method(ClassObject cls, string name) { - cls.declaresAttribute(name) and not cls.declaredAttribute(name) instanceof FunctionObject +predicate nonTrivial(Function meth) { + exists(Stmt s | s = meth.getAStmt() | + not s instanceof Pass and + not exists(DataFlow::Node call | call.asExpr() = s.(ExprStmt).getValue() | + superCall(call, meth.getName()) + or + callsMethodOnClassWithSelf(meth, call, _, meth.getName()) + ) + ) and + exists(meth.getANormalExit()) // doesn't always raise an exception } -/** Holds if `f` actually does something. */ -private predicate does_something(FunctionObject f) { - f.isBuiltin() and not f = theObjectType().lookupAttribute("__init__") - or - exists(Stmt s | s = f.getFunction().getAStmt() and not s instanceof Pass) -} - -/** Holds if `meth` looks like it should have a call to `name`, but does not */ -private predicate missing_call(FunctionObject meth, string name) { - exists(CallNode call, AttrNode attr | - call.getScope() = meth.getFunction() and - call.getFunction() = attr and - attr.getName() = name and - not exists(FunctionObject f | f.getACall() = call) +predicate superCall(DataFlow::MethodCallNode call, string name) { + exists(DataFlow::Node sup | + call.calls(sup, name) and + sup = API::builtin("super").getACall() ) } -/** Holds if `self.name` does not call `missing`, even though it is expected to. */ -predicate missing_call_to_superclass_method( - ClassObject self, FunctionObject top, FunctionObject missing, string name -) { - missing = self.getASuperType().declaredAttribute(name) and - top = self.lookupAttribute(name) and - /* There is no call to missing originating from top */ - not top.getACallee*() = missing and - /* Make sure that all named 'methods' are objects that we can understand. */ - not exists(ClassObject sup | - sup = self.getAnImproperSuperType() and - named_attributes_not_method(sup, name) - ) and - not self.isAbstract() and - does_something(missing) and - not missing_call(top, name) +predicate callsSuper(Function meth) { + exists(DataFlow::MethodCallNode call | + call.getScope() = meth and + superCall(call, meth.getName()) + ) +} + +predicate callsMethodOnClassWithSelf( + Function meth, DataFlow::MethodCallNode call, Class target, string name +) { + exists(DataFlow::Node callTarget, DataFlow::ParameterNode self | + call.calls(callTarget, name) and + self.getParameter() = meth.getArg(0) and + self.(DataFlow::LocalSourceNode).flowsTo(call.getArg(0)) and + callTarget = classTracker(target) + ) +} + +predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) { + exists(DataFlow::MethodCallNode call, DataFlow::Node callTarget, DataFlow::ParameterNode self | + call.calls(callTarget, name) and + self.getParameter() = meth.getArg(0) and + self.(DataFlow::LocalSourceNode).flowsTo(call.getArg(0)) and + not exists(Class target | callTarget = classTracker(target)) + ) +} + +predicate mayProceedInMro(Class a, Class b, Class mroStart) { + b = getNextClassInMroKnownStartingClass(a, mroStart) + or + exists(Class mid | + mid = getNextClassInMroKnownStartingClass(a, mroStart) and + mayProceedInMro(mid, b, mroStart) + ) +} + +predicate missingCallToSuperclassMethod( + Function base, Function shouldCall, Class mroStart, string name +) { + base.getName() = name and + shouldCall.getName() = name and + not callsSuper(base) and + not callsMethodOnUnknownClassWithSelf(base, name) and + nonTrivial(shouldCall) and + base.getScope() = getADirectSuperclass*(mroStart) and + mayProceedInMro(base.getScope(), shouldCall.getScope(), mroStart) and + not exists(Class called | + ( + callsMethodOnClassWithSelf(base, _, called, name) + or + callsMethodOnClassWithSelf(findFunctionAccordingToMro(mroStart, name), _, called, name) + ) and + shouldCall.getScope() = getADirectSuperclass*(called) + ) } diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql index 4f5d3d90e84..a8f1c5b6179 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql @@ -1,5 +1,5 @@ /** - * @name Missing call to `__init__` during object initialization + * @name Missing call to superclass `__init__` during object initialization * @description An omitted call to a super-class `__init__` method may lead to objects of this class not being fully initialized. * @kind problem * @tags quality @@ -14,16 +14,21 @@ import python import MethodCallOrder -from ClassObject self, FunctionObject initializer, FunctionObject missing +predicate missingCallToSuperclassInit(Function base, Function shouldCall, Class mroStart) { + missingCallToSuperclassMethod(base, shouldCall, mroStart, "__init__") +} + +from Function base, Function shouldCall, Class mroStart, string msg where - self.lookupAttribute("__init__") = initializer and - missing_call_to_superclass_method(self, initializer, missing, "__init__") and - // If a superclass is incorrect, don't flag this class as well. - not missing_call_to_superclass_method(self.getASuperType(), _, missing, "__init__") and - not missing.neverReturns() and - not self.failedInference() and - not missing.isBuiltin() and - not self.isAbstract() -select self, - "Class " + self.getName() + " may not be initialized properly as $@ is not called from its $@.", - missing, missing.descriptiveString(), initializer, "__init__ method" + missingCallToSuperclassInit(base, shouldCall, mroStart) and + ( + // Simple case: the method that should be called is directly overridden + mroStart = base.getScope() and + msg = "This initialization method does not call $@, which may leave $@ partially initialized." + or + // Only alert for a different mro base if there are no alerts for direct overrides + not missingCallToSuperclassInit(base, _, base.getScope()) and + msg = + "This initialization method does not call $@, which follows it in the MRO of $@, leaving it partially initialized." + ) +select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName()