From 271f32e80eb28884d6c2759a7581d4f725d12be0 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 30 Jun 2025 10:21:31 +0100 Subject: [PATCH 01/28] Move missing/multiple calls to init/del queries to folder --- .../CallsToInitDel/MethodCallOrder.qll | 77 +++++++++++++++++++ .../{ => CallsToInitDel}/MissingCallToDel.py | 0 .../MissingCallToDel.qhelp | 0 .../{ => CallsToInitDel}/MissingCallToDel.ql | 0 .../{ => CallsToInitDel}/MissingCallToInit.py | 0 .../MissingCallToInit.qhelp | 0 .../{ => CallsToInitDel}/MissingCallToInit.ql | 0 .../SuperclassDelCalledMultipleTimes.py | 0 .../SuperclassDelCalledMultipleTimes.qhelp | 0 .../SuperclassDelCalledMultipleTimes.ql | 0 .../SuperclassDelCalledMultipleTimes2.py | 0 .../SuperclassInitCalledMultipleTimes.py | 0 .../SuperclassInitCalledMultipleTimes.qhelp | 0 .../SuperclassInitCalledMultipleTimes.ql | 0 .../SuperclassInitCalledMultipleTimes2.py | 0 python/ql/src/Classes/MethodCallOrder.qll | 2 + 16 files changed, 79 insertions(+) create mode 100644 python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll rename python/ql/src/Classes/{ => CallsToInitDel}/MissingCallToDel.py (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/MissingCallToDel.qhelp (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/MissingCallToDel.ql (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/MissingCallToInit.py (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/MissingCallToInit.qhelp (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/MissingCallToInit.ql (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassDelCalledMultipleTimes.py (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassDelCalledMultipleTimes.qhelp (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassDelCalledMultipleTimes.ql (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassDelCalledMultipleTimes2.py (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassInitCalledMultipleTimes.py (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassInitCalledMultipleTimes.qhelp (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassInitCalledMultipleTimes.ql (100%) rename python/ql/src/Classes/{ => CallsToInitDel}/SuperclassInitCalledMultipleTimes2.py (100%) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll new file mode 100644 index 000000000000..b911ee0183e1 --- /dev/null +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -0,0 +1,77 @@ +deprecated module; + +import python + +// Helper predicates for multiple call to __init__/__del__ queries. +pragma[noinline] +private predicate multiple_invocation_paths_helper( + FunctionInvocation top, FunctionInvocation i1, FunctionInvocation i2, FunctionObject multi +) { + i1 != i2 and + i1 = top.getACallee+() and + i2 = top.getACallee+() and + i1.getFunction() = multi +} + +pragma[noinline] +private predicate multiple_invocation_paths( + FunctionInvocation top, FunctionInvocation i1, FunctionInvocation i2, FunctionObject multi +) { + multiple_invocation_paths_helper(top, i1, i2, multi) and + i2.getFunction() = multi +} + +/** Holds if `self.name` calls `multi` by multiple paths, and thus calls it more than once. */ +predicate multiple_calls_to_superclass_method(ClassObject self, FunctionObject multi, string name) { + exists(FunctionInvocation top, FunctionInvocation i1, FunctionInvocation i2 | + multiple_invocation_paths(top, i1, i2, multi) and + top.runtime(self.declaredAttribute(name)) and + self.getASuperType().declaredAttribute(name) = multi + | + // Only called twice if called from different functions, + // or if one call-site can reach the other. + i1.getCall().getScope() != i2.getCall().getScope() + or + i1.getCall().strictlyReaches(i2.getCall()) + ) +} + +/** 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 +} + +/** 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) + ) +} + +/** 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) +} diff --git a/python/ql/src/Classes/MissingCallToDel.py b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.py similarity index 100% rename from python/ql/src/Classes/MissingCallToDel.py rename to python/ql/src/Classes/CallsToInitDel/MissingCallToDel.py diff --git a/python/ql/src/Classes/MissingCallToDel.qhelp b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp similarity index 100% rename from python/ql/src/Classes/MissingCallToDel.qhelp rename to python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp diff --git a/python/ql/src/Classes/MissingCallToDel.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql similarity index 100% rename from python/ql/src/Classes/MissingCallToDel.ql rename to python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql diff --git a/python/ql/src/Classes/MissingCallToInit.py b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.py similarity index 100% rename from python/ql/src/Classes/MissingCallToInit.py rename to python/ql/src/Classes/CallsToInitDel/MissingCallToInit.py diff --git a/python/ql/src/Classes/MissingCallToInit.qhelp b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.qhelp similarity index 100% rename from python/ql/src/Classes/MissingCallToInit.qhelp rename to python/ql/src/Classes/CallsToInitDel/MissingCallToInit.qhelp diff --git a/python/ql/src/Classes/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql similarity index 100% rename from python/ql/src/Classes/MissingCallToInit.ql rename to python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql diff --git a/python/ql/src/Classes/SuperclassDelCalledMultipleTimes.py b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.py similarity index 100% rename from python/ql/src/Classes/SuperclassDelCalledMultipleTimes.py rename to python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.py diff --git a/python/ql/src/Classes/SuperclassDelCalledMultipleTimes.qhelp b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp similarity index 100% rename from python/ql/src/Classes/SuperclassDelCalledMultipleTimes.qhelp rename to python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp diff --git a/python/ql/src/Classes/SuperclassDelCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql similarity index 100% rename from python/ql/src/Classes/SuperclassDelCalledMultipleTimes.ql rename to python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql diff --git a/python/ql/src/Classes/SuperclassDelCalledMultipleTimes2.py b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes2.py similarity index 100% rename from python/ql/src/Classes/SuperclassDelCalledMultipleTimes2.py rename to python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes2.py diff --git a/python/ql/src/Classes/SuperclassInitCalledMultipleTimes.py b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.py similarity index 100% rename from python/ql/src/Classes/SuperclassInitCalledMultipleTimes.py rename to python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.py diff --git a/python/ql/src/Classes/SuperclassInitCalledMultipleTimes.qhelp b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.qhelp similarity index 100% rename from python/ql/src/Classes/SuperclassInitCalledMultipleTimes.qhelp rename to python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.qhelp diff --git a/python/ql/src/Classes/SuperclassInitCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql similarity index 100% rename from python/ql/src/Classes/SuperclassInitCalledMultipleTimes.ql rename to python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql diff --git a/python/ql/src/Classes/SuperclassInitCalledMultipleTimes2.py b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes2.py similarity index 100% rename from python/ql/src/Classes/SuperclassInitCalledMultipleTimes2.py rename to python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes2.py diff --git a/python/ql/src/Classes/MethodCallOrder.qll b/python/ql/src/Classes/MethodCallOrder.qll index 620cb8028780..b911ee0183e1 100644 --- a/python/ql/src/Classes/MethodCallOrder.qll +++ b/python/ql/src/Classes/MethodCallOrder.qll @@ -1,3 +1,5 @@ +deprecated module; + import python // Helper predicates for multiple call to __init__/__del__ queries. From a2fc14af2e85c2293a738aeb93534e47ece4003c Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 30 Jun 2025 16:21:17 +0100 Subject: [PATCH 02/28] Update missing call to init --- .../CallsToInitDel/MethodCallOrder.qll | 99 +++++++++++++------ .../CallsToInitDel/MissingCallToInit.ql | 31 +++--- 2 files changed, 86 insertions(+), 44 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index b911ee0183e1..b14c6138015f 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) +predicate superCall(DataFlow::MethodCallNode call, string name) { + exists(DataFlow::Node sup | + call.calls(sup, name) and + sup = API::builtin("super").getACall() + ) } -/** 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 callsSuper(Function meth) { + exists(DataFlow::MethodCallNode call | + call.getScope() = meth and + superCall(call, meth.getName()) ) } -/** 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 +predicate callsMethodOnClassWithSelf( + Function meth, DataFlow::MethodCallNode call, Class target, 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) + 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 4f5d3d90e84a..a8f1c5b6179c 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() From 6f9983a431ab8d1ddfd3c690022305da75e564cd Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 30 Jun 2025 16:59:51 +0100 Subject: [PATCH 03/28] Add missing call to del --- .../CallsToInitDel/MissingCallToDel.ql | 29 ++++++++++++------- .../CallsToInitDel/MissingCallToInit.ql | 2 +- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql index be49dc48b5f4..1c0bf6f6ec4a 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql @@ -1,6 +1,6 @@ /** - * @name Missing call to `__del__` during object destruction - * @description An omitted call to a super-class `__del__` method may lead to class instances not being cleaned up properly. + * @name Missing call to superclass `__del__` during object destruction + * @description An omitted call to a superclass `__del__` method may lead to class instances not being cleaned up properly. * @kind problem * @tags quality * reliability @@ -15,12 +15,21 @@ import python import MethodCallOrder -from ClassObject self, FunctionObject missing +predicate missingCallToSuperclassDel(Function base, Function shouldCall, Class mroStart) { + missingCallToSuperclassMethod(base, shouldCall, mroStart, "__del__") +} + +from Function base, Function shouldCall, Class mroStart, string msg where - missing_call_to_superclass_method(self, _, missing, "__del__") and - not missing.neverReturns() and - not self.failedInference() and - not missing.isBuiltin() -select self, - "Class " + self.getName() + " may not be cleaned up properly as $@ is not called during deletion.", - missing, missing.descriptiveString() + missingCallToSuperclassDel(base, shouldCall, mroStart) and + ( + // Simple case: the method that should be called is directly overridden + mroStart = base.getScope() and + msg = "This deletion method does not call $@, which may leave $@ not properly cleaned up." + or + // Only alert for a different mro base if there are no alerts for direct overrides + not missingCallToSuperclassDel(base, _, base.getScope()) and + msg = + "This deletion method does not call $@, which follows it in the MRO of $@, leaving it not properly cleaned up." + ) +select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName() diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql index a8f1c5b6179c..9adcad3e46a1 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql @@ -1,6 +1,6 @@ /** * @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. + * @description An omitted call to a superclass `__init__` method may lead to objects of this class not being fully initialized. * @kind problem * @tags quality * reliability From adcfdf1da4388cde06ea614f21782cc7595eaf70 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Tue, 1 Jul 2025 11:25:06 +0100 Subject: [PATCH 04/28] Modernize multple calls to init/del --- .../new/internal/DataFlowDispatch.qll | 13 +++-- .../CallsToInitDel/MethodCallOrder.qll | 49 +++++++++---------- .../SuperclassDelCalledMultipleTimes.ql | 27 +++++----- .../SuperclassInitCalledMultipleTimes.ql | 28 +++++------ 4 files changed, 59 insertions(+), 58 deletions(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll index 1a38593bce48..ce778eff85d2 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll @@ -851,9 +851,14 @@ Class getNextClassInMroKnownStartingClass(Class cls, Class startingClass) { ) } -private Function findFunctionAccordingToMroKnownStartingClass( - Class cls, Class startingClass, string name -) { +/** + * Gets a potential definition of the function `name` of the class `cls` according to our approximation of + * MRO for the class `startingCls` (see `getNextClassInMroKnownStartingClass` for more information). + * + * Note: this is almost the same as `findFunctionAccordingToMro`, except we know the + * `startingClass`, which can give slightly more precise results. + */ +Function findFunctionAccordingToMroKnownStartingClass(Class cls, Class startingClass, string name) { result = cls.getAMethod() and result.getName() = name and cls = getADirectSuperclass*(startingClass) @@ -866,7 +871,7 @@ private Function findFunctionAccordingToMroKnownStartingClass( /** * Gets a potential definition of the function `name` according to our approximation of - * MRO for the class `cls` (see `getNextClassInMroKnownStartingClass` for more information). + * MRO for the class `startingCls` (see `getNextClassInMroKnownStartingClass` for more information). * * Note: this is almost the same as `findFunctionAccordingToMro`, except we know the * `startingClass`, which can give slightly more precise results. diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index b14c6138015f..253360bf2275 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -4,37 +4,32 @@ import python import semmle.python.ApiGraphs import semmle.python.dataflow.new.internal.DataFlowDispatch -// Helper predicates for multiple call to __init__/__del__ queries. -pragma[noinline] -private predicate multiple_invocation_paths_helper( - FunctionInvocation top, FunctionInvocation i1, FunctionInvocation i2, FunctionObject multi -) { - i1 != i2 and - i1 = top.getACallee+() and - i2 = top.getACallee+() and - i1.getFunction() = multi -} - -pragma[noinline] -private predicate multiple_invocation_paths( - FunctionInvocation top, FunctionInvocation i1, FunctionInvocation i2, FunctionObject multi -) { - multiple_invocation_paths_helper(top, i1, i2, multi) and - i2.getFunction() = multi +predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, string name) { + exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls | + meth.getName() = name and + meth.getScope() = cls and + not call1 = call2 and + calledMulti = getASuperCallTarget(cls, meth, call1) and + calledMulti = getASuperCallTarget(cls, meth, call2) and + nonTrivial(calledMulti) + ) } -/** Holds if `self.name` calls `multi` by multiple paths, and thus calls it more than once. */ -predicate multiple_calls_to_superclass_method(ClassObject self, FunctionObject multi, string name) { - exists(FunctionInvocation top, FunctionInvocation i1, FunctionInvocation i2 | - multiple_invocation_paths(top, i1, i2, multi) and - top.runtime(self.declaredAttribute(name)) and - self.getASuperType().declaredAttribute(name) = multi +Function getASuperCallTarget(Class mroBase, Function meth, DataFlow::MethodCallNode call) { + meth = call.getScope() and + getADirectSuperclass*(mroBase) = meth.getScope() and + call.calls(_, meth.getName()) and + exists(Function target, Class nextMroBase | + (result = target or result = getASuperCallTarget(nextMroBase, target, _)) | - // Only called twice if called from different functions, - // or if one call-site can reach the other. - i1.getCall().getScope() != i2.getCall().getScope() + superCall(call, _) and + nextMroBase = mroBase and + target = + findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(meth.getScope(), + mroBase), mroBase, meth.getName()) or - i1.getCall().strictlyReaches(i2.getCall()) + callsMethodOnClassWithSelf(meth, call, nextMroBase, _) and + target = findFunctionAccordingToMro(nextMroBase, meth.getName()) ) } diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql index 019da4257aa0..d75d948809dc 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql @@ -1,6 +1,6 @@ /** * @name Multiple calls to `__del__` during object destruction - * @description A duplicated call to a super-class `__del__` method may lead to class instances not be cleaned up properly. + * @description A duplicated call to a superclass `__del__` method may lead to class instances not be cleaned up properly. * @kind problem * @tags quality * reliability @@ -14,16 +14,17 @@ import python import MethodCallOrder -from ClassObject self, FunctionObject multi +predicate multipleCallsToSuperclassDel(Function meth, Function calledMulti) { + multipleCallsToSuperclassMethod(meth, calledMulti, "__sel__") +} + +from Function meth, Function calledMulti where - multiple_calls_to_superclass_method(self, multi, "__del__") and - not multiple_calls_to_superclass_method(self.getABaseType(), multi, "__del__") and - not exists(FunctionObject better | - multiple_calls_to_superclass_method(self, better, "__del__") and - better.overrides(multi) - ) and - not self.failedInference() -select self, - "Class " + self.getName() + - " may not be cleaned up properly as $@ may be called multiple times during destruction.", multi, - multi.descriptiveString() + multipleCallsToSuperclassDel(meth, calledMulti) and + // Don't alert for multiple calls to a superclass del when a subclass will do. + not exists(Function subMulti | + multipleCallsToSuperclassDel(meth, subMulti) and + calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope()) + ) +select meth, "This delete method calls $@ multiple times.", calledMulti, + calledMulti.getQualifiedName() diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql index 6251ef274dac..1f6f61222bfe 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql @@ -1,6 +1,6 @@ /** * @name Multiple calls to `__init__` during object initialization - * @description A duplicated call to a super-class `__init__` method may lead to objects of this class not being properly initialized. + * @description A duplicated call to a superclass `__init__` method may lead to objects of this class not being properly initialized. * @kind problem * @tags quality * reliability @@ -14,17 +14,17 @@ import python import MethodCallOrder -from ClassObject self, FunctionObject multi +predicate multipleCallsToSuperclassInit(Function meth, Function calledMulti) { + multipleCallsToSuperclassMethod(meth, calledMulti, "__init__") +} + +from Function meth, Function calledMulti where - multi != theObjectType().lookupAttribute("__init__") and - multiple_calls_to_superclass_method(self, multi, "__init__") and - not multiple_calls_to_superclass_method(self.getABaseType(), multi, "__init__") and - not exists(FunctionObject better | - multiple_calls_to_superclass_method(self, better, "__init__") and - better.overrides(multi) - ) and - not self.failedInference() -select self, - "Class " + self.getName() + - " may not be initialized properly as $@ may be called multiple times during initialization.", - multi, multi.descriptiveString() + multipleCallsToSuperclassInit(meth, calledMulti) and + // Don't alert for multiple calls to a superclass init when a subclass will do. + not exists(Function subMulti | + multipleCallsToSuperclassInit(meth, subMulti) and + calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope()) + ) +select meth, "This initializer method calls $@ multiple times.", calledMulti, + calledMulti.getQualifiedName() From caddec474ce6f74fbfd4ee21f5a8f75b44f64e41 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Tue, 1 Jul 2025 11:38:12 +0100 Subject: [PATCH 05/28] Update alert messages --- python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql | 4 ++-- python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql | 2 +- .../CallsToInitDel/SuperclassInitCalledMultipleTimes.ql | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql index 1c0bf6f6ec4a..4cc72ee9facd 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql @@ -25,11 +25,11 @@ where ( // Simple case: the method that should be called is directly overridden mroStart = base.getScope() and - msg = "This deletion method does not call $@, which may leave $@ not properly cleaned up." + msg = "This delete method does not call $@, which may leave $@ not properly cleaned up." or // Only alert for a different mro base if there are no alerts for direct overrides not missingCallToSuperclassDel(base, _, base.getScope()) and msg = - "This deletion method does not call $@, which follows it in the MRO of $@, leaving it not properly cleaned up." + "This delete method does not call super().__del__, which may cause $@ to be missed during the cleanup of $@." ) select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName() diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql index 9adcad3e46a1..56c6bd258cd2 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql @@ -29,6 +29,6 @@ where // 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." + "This initialization method does not call super().__init__, which may cause $@ to be missed during the initialization of $@." ) select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName() diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql index 1f6f61222bfe..4f577dc4a764 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql @@ -26,5 +26,5 @@ where multipleCallsToSuperclassInit(meth, subMulti) and calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope()) ) -select meth, "This initializer method calls $@ multiple times.", calledMulti, +select meth, "This initialization method calls $@ multiple times.", calledMulti, calledMulti.getQualifiedName() From 71d1179877a80cdfb574bf52c9533bf95b2f42a8 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Tue, 1 Jul 2025 16:39:12 +0100 Subject: [PATCH 06/28] Fix FPs and typo --- python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll | 2 +- .../Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 253360bf2275..5fd28ef9ddf7 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -8,7 +8,7 @@ predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, s exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls | meth.getName() = name and meth.getScope() = cls and - not call1 = call2 and + call1.asExpr() != call2.asExpr() and calledMulti = getASuperCallTarget(cls, meth, call1) and calledMulti = getASuperCallTarget(cls, meth, call2) and nonTrivial(calledMulti) diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql index d75d948809dc..7aca3dee1892 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql @@ -15,7 +15,7 @@ import python import MethodCallOrder predicate multipleCallsToSuperclassDel(Function meth, Function calledMulti) { - multipleCallsToSuperclassMethod(meth, calledMulti, "__sel__") + multipleCallsToSuperclassMethod(meth, calledMulti, "__del__") } from Function meth, Function calledMulti From 085df269a327dfc01d4a5dfa29902482914df0cf Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Wed, 2 Jul 2025 10:09:51 +0100 Subject: [PATCH 07/28] Move tests and add inline expectation postprocessing --- .../query-tests/Classes/missing-del/MissingCallToDel.qlref | 3 ++- .../query-tests/Classes/missing-init/MissingCallToInit.qlref | 3 ++- .../Classes/multiple/SuperclassDelCalledMultipleTimes.qlref | 1 - .../Classes/multiple/SuperclassInitCalledMultipleTimes.qlref | 1 - .../SuperclassDelCalledMultipleTimes.expected | 0 .../multiple-del/SuperclassDelCalledMultipleTimes.qlref | 2 ++ .../Classes/multiple/{ => multiple-del}/multiple_del.py | 0 .../SuperclassInitCalledMultipleTimes.expected | 0 .../multiple-init/SuperclassInitCalledMultipleTimes.qlref | 2 ++ .../Classes/multiple/{ => multiple-init}/multiple_init.py | 2 +- 10 files changed, 9 insertions(+), 5 deletions(-) delete mode 100644 python/ql/test/query-tests/Classes/multiple/SuperclassDelCalledMultipleTimes.qlref delete mode 100644 python/ql/test/query-tests/Classes/multiple/SuperclassInitCalledMultipleTimes.qlref rename python/ql/test/query-tests/Classes/multiple/{ => multiple-del}/SuperclassDelCalledMultipleTimes.expected (100%) create mode 100644 python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.qlref rename python/ql/test/query-tests/Classes/multiple/{ => multiple-del}/multiple_del.py (100%) rename python/ql/test/query-tests/Classes/multiple/{ => multiple-init}/SuperclassInitCalledMultipleTimes.expected (100%) create mode 100644 python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.qlref rename python/ql/test/query-tests/Classes/multiple/{ => multiple-init}/multiple_init.py (98%) diff --git a/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.qlref b/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.qlref index 8bf1811d0fab..b7ff00870542 100644 --- a/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.qlref +++ b/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.qlref @@ -1 +1,2 @@ -Classes/MissingCallToDel.ql +query: Classes/CallsToInitDel/MissingCallToDel.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.qlref b/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.qlref index b8635a5f8d92..86700427963a 100644 --- a/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.qlref +++ b/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.qlref @@ -1 +1,2 @@ -Classes/MissingCallToInit.ql +query: Classes/CallsToInitDel/MissingCallToInit.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/python/ql/test/query-tests/Classes/multiple/SuperclassDelCalledMultipleTimes.qlref b/python/ql/test/query-tests/Classes/multiple/SuperclassDelCalledMultipleTimes.qlref deleted file mode 100644 index 560d7b7dc416..000000000000 --- a/python/ql/test/query-tests/Classes/multiple/SuperclassDelCalledMultipleTimes.qlref +++ /dev/null @@ -1 +0,0 @@ -Classes/SuperclassDelCalledMultipleTimes.ql diff --git a/python/ql/test/query-tests/Classes/multiple/SuperclassInitCalledMultipleTimes.qlref b/python/ql/test/query-tests/Classes/multiple/SuperclassInitCalledMultipleTimes.qlref deleted file mode 100644 index 042ddb76904c..000000000000 --- a/python/ql/test/query-tests/Classes/multiple/SuperclassInitCalledMultipleTimes.qlref +++ /dev/null @@ -1 +0,0 @@ -Classes/SuperclassInitCalledMultipleTimes.ql diff --git a/python/ql/test/query-tests/Classes/multiple/SuperclassDelCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected similarity index 100% rename from python/ql/test/query-tests/Classes/multiple/SuperclassDelCalledMultipleTimes.expected rename to python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.qlref b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.qlref new file mode 100644 index 000000000000..8fa7d25474b0 --- /dev/null +++ b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.qlref @@ -0,0 +1,2 @@ +query: Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/python/ql/test/query-tests/Classes/multiple/multiple_del.py b/python/ql/test/query-tests/Classes/multiple/multiple-del/multiple_del.py similarity index 100% rename from python/ql/test/query-tests/Classes/multiple/multiple_del.py rename to python/ql/test/query-tests/Classes/multiple/multiple-del/multiple_del.py diff --git a/python/ql/test/query-tests/Classes/multiple/SuperclassInitCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected similarity index 100% rename from python/ql/test/query-tests/Classes/multiple/SuperclassInitCalledMultipleTimes.expected rename to python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.qlref b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.qlref new file mode 100644 index 000000000000..9a2b156acd35 --- /dev/null +++ b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.qlref @@ -0,0 +1,2 @@ +query: Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/python/ql/test/query-tests/Classes/multiple/multiple_init.py b/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py similarity index 98% rename from python/ql/test/query-tests/Classes/multiple/multiple_init.py rename to python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py index 6a97ef67f6ca..28dd16eeed1e 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple_init.py +++ b/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py @@ -2,7 +2,7 @@ class Base(object): def __init__(self): - pass + print("Base init") class C1(Base): From 2faf67d08c8892f45d02388c7a108b0c93e2bbe8 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Wed, 2 Jul 2025 10:53:28 +0100 Subject: [PATCH 08/28] Update test outputs + fix semantics --- .../CallsToInitDel/MethodCallOrder.qll | 11 +++--- .../SuperclassDelCalledMultipleTimes.expected | 4 +- .../multiple/multiple-del/multiple_del.py | 19 ++++++++-- ...SuperclassInitCalledMultipleTimes.expected | 4 +- .../multiple/multiple-init/multiple_init.py | 38 ++++++++----------- 5 files changed, 40 insertions(+), 36 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 5fd28ef9ddf7..58330eeb9999 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -19,17 +19,16 @@ Function getASuperCallTarget(Class mroBase, Function meth, DataFlow::MethodCallN meth = call.getScope() and getADirectSuperclass*(mroBase) = meth.getScope() and call.calls(_, meth.getName()) and - exists(Function target, Class nextMroBase | - (result = target or result = getASuperCallTarget(nextMroBase, target, _)) - | + exists(Function target | (result = target or result = getASuperCallTarget(mroBase, target, _)) | superCall(call, _) and - nextMroBase = mroBase and target = findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(meth.getScope(), mroBase), mroBase, meth.getName()) or - callsMethodOnClassWithSelf(meth, call, nextMroBase, _) and - target = findFunctionAccordingToMro(nextMroBase, meth.getName()) + exists(Class called | + callsMethodOnClassWithSelf(meth, call, called, _) and + target = findFunctionAccordingToMroKnownStartingClass(called, mroBase, meth.getName()) + ) ) } diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected index 210f24c7525d..ad9858397dff 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected +++ b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected @@ -1,2 +1,2 @@ -| multiple_del.py:17:1:17:17 | class Y3 | Class Y3 may not be cleaned up properly as $@ may be called multiple times during destruction. | multiple_del.py:9:5:9:22 | Function __del__ | method Y1.__del__ | -| multiple_del.py:34:1:34:17 | class Z3 | Class Z3 may not be cleaned up properly as $@ may be called multiple times during destruction. | multiple_del.py:26:5:26:22 | Function __del__ | method Z1.__del__ | +| multiple_del.py:21:5:21:22 | Function __del__ | This delete method calls $@ multiple times. | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | +| multiple_del.py:43:5:43:22 | Function __del__ | This delete method calls $@ multiple times. | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-del/multiple_del.py b/python/ql/test/query-tests/Classes/multiple/multiple-del/multiple_del.py index 284f6bf6969f..f72e7c5d00ad 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-del/multiple_del.py +++ b/python/ql/test/query-tests/Classes/multiple/multiple-del/multiple_del.py @@ -2,37 +2,48 @@ class Base(object): def __del__(self): - pass + print("Base del") class Y1(Base): def __del__(self): + print("Y1 del") super(Y1, self).__del__() class Y2(Base): def __del__(self): + print("Y2 del") super(Y2, self).__del__() #When `type(self) == Y3` this calls `Y1.__del__` class Y3(Y2, Y1): - def __del__(self): + def __del__(self): # $ Alert + print("Y3 del") Y1.__del__(self) Y2.__del__(self) +a = Y3() +del a + #Calling a method multiple times by using explicit calls when a base inherits from other base class Z1(object): def __del__(self): - pass + print("Z1 del") class Z2(Z1): def __del__(self): + print("Z2 del") Z1.__del__(self) class Z3(Z2, Z1): - def __del__(self): + def __del__(self): # $ Alert + print("Z3 del") Z1.__del__(self) Z2.__del__(self) + +b = Z3() +del b diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected index 04ce8c0f3730..42d019e7f710 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected +++ b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected @@ -1,2 +1,2 @@ -| multiple_init.py:17:1:17:17 | class C3 | Class C3 may not be initialized properly as $@ may be called multiple times during initialization. | multiple_init.py:9:5:9:23 | Function __init__ | method C1.__init__ | -| multiple_init.py:34:1:34:17 | class D3 | Class D3 may not be initialized properly as $@ may be called multiple times during initialization. | multiple_init.py:26:5:26:23 | Function __init__ | method D1.__init__ | +| multiple_init.py:21:5:21:23 | Function __init__ | This initialization method calls $@ multiple times. | multiple_init.py:9:5:9:23 | Function __init__ | C1.__init__ | +| multiple_init.py:42:5:42:23 | Function __init__ | This initialization method calls $@ multiple times. | multiple_init.py:31:5:31:23 | Function __init__ | D1.__init__ | diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py b/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py index 28dd16eeed1e..cba8b24523fa 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py +++ b/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py @@ -7,70 +7,64 @@ def __init__(self): class C1(Base): def __init__(self): + print("C1 init") super(C1, self).__init__() class C2(Base): def __init__(self): + print("C2 init") super(C2, self).__init__() #When `type(self) == C3` this calls `C1.__init__` class C3(C2, C1): - def __init__(self): + def __init__(self): # $ Alert + print("C3 init") C1.__init__(self) C2.__init__(self) +C3() + #Calling a method multiple times by using explicit calls when a base inherits from other base class D1(object): def __init__(self): - pass + print("D1 init") class D2(D1): def __init__(self): + print("D2 init") D1.__init__(self) class D3(D2, D1): - def __init__(self): + def __init__(self): # $ Alert + print("D3 init") D1.__init__(self) D2.__init__(self) +D3() + #OK to call object.__init__ multiple times class E1(object): def __init__(self): + print("E1 init") super(E1, self).__init__() class E2(object): def __init__(self): + print("E2 init") object.__init__(self) class E3(E2, E1): - def __init__(self): + def __init__(self): # OK: builtin init methods are fine. E1.__init__(self) E2.__init__(self) -#Two invocations, but can only be called once -class F1(Base): - - def __init__(self, cond): - if cond: - Base.__init__(self) - else: - Base.__init__(self) - -#Single call, splitting causes what seems to be multiple invocations. -class F2(Base): - - def __init__(self, cond): - if cond: - pass - if cond: - pass - Base.__init__(self) +E3() From 1b4e2feb60cdb1a3221b305242fe486edeff20bb Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 3 Jul 2025 14:22:04 +0100 Subject: [PATCH 09/28] Change implenetation of missing calls to use getASuperCallTarget, and change alerts to alert on the class and provide clearer information, using optional location links. --- .../CallsToInitDel/MethodCallOrder.qll | 128 +++++++++++++----- .../CallsToInitDel/MissingCallToDel.ql | 42 ++++-- .../CallsToInitDel/MissingCallToInit.ql | 41 +++--- 3 files changed, 146 insertions(+), 65 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 58330eeb9999..1ebde88083a8 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -3,32 +3,38 @@ import python import semmle.python.ApiGraphs import semmle.python.dataflow.new.internal.DataFlowDispatch +import codeql.util.Option predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, string name) { exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls | meth.getName() = name and meth.getScope() = cls and call1.asExpr() != call2.asExpr() and - calledMulti = getASuperCallTarget(cls, meth, call1) and - calledMulti = getASuperCallTarget(cls, meth, call2) and + calledMulti = getASuperCallTargetFromCall(cls, meth, call1, name) and + calledMulti = getASuperCallTargetFromCall(cls, meth, call2, name) and nonTrivial(calledMulti) ) } -Function getASuperCallTarget(Class mroBase, Function meth, DataFlow::MethodCallNode call) { +Function getASuperCallTargetFromCall( + Class mroBase, Function meth, DataFlow::MethodCallNode call, string name +) { meth = call.getScope() and getADirectSuperclass*(mroBase) = meth.getScope() and - call.calls(_, meth.getName()) and - exists(Function target | (result = target or result = getASuperCallTarget(mroBase, target, _)) | + meth.getName() = name and + call.calls(_, name) and + exists(Class targetCls | result = getASuperCallTargetFromClass(mroBase, targetCls, name) | superCall(call, _) and - target = - findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(meth.getScope(), - mroBase), mroBase, meth.getName()) + targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase) or - exists(Class called | - callsMethodOnClassWithSelf(meth, call, called, _) and - target = findFunctionAccordingToMroKnownStartingClass(called, mroBase, meth.getName()) - ) + callsMethodOnClassWithSelf(meth, call, targetCls, _) + ) +} + +Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) { + exists(Function target | + target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and + (result = target or result = getASuperCallTargetFromCall(mroBase, target, _, name)) ) } @@ -78,31 +84,83 @@ predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) { ) } -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 -) { +predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string name) { base.getName() = name and shouldCall.getName() = name and - not callsSuper(base) and - not callsMethodOnUnknownClassWithSelf(base, name) and + base = getADirectSuperclass*(base.getScope()) and + not shouldCall = getASuperCallTargetFromClass(base, 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) + // "Benefit of the doubt" - if somewhere in the chain we call an unknown superclass, assume all the necessary parent methods are called from it + not callsMethodOnUnknownClassWithSelf(getASuperCallTargetFromClass(base, base, name), name) +} + +predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCall, string name) { + missingCallToSuperclassMethod(base, shouldCall, name) and + not exists(Class subBase | + subBase = getADirectSubclass+(base) and + missingCallToSuperclassMethod(subBase, shouldCall, name) + ) and + not exists(Function superShouldCall | + superShouldCall.getScope() = getADirectSuperclass+(shouldCall.getScope()) and + missingCallToSuperclassMethod(base, superShouldCall, name) + ) +} + +Function getPossibleMissingSuper(Class base, Function shouldCall, string name) { + missingCallToSuperclassMethod(base, shouldCall, name) and + exists(Function baseMethod | + baseMethod.getScope() = base and + baseMethod.getName() = name and + // the base method calls super, so is presumably expecting every method called in the MRO chain to do so + callsSuper(baseMethod) and + // result is something that does get called in the chain + result = getASuperCallTargetFromClass(base, base, name) and + // it doesn't call super + not callsSuper(result) and + // if it did call super, it would resolve to the missing method + shouldCall = + findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(result + .getScope(), base), base, name) ) } + +private module FunctionOption = Option; + +class FunctionOption extends FunctionOption::Option { + /** + * Holds if this element is at the specified location. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. + * For more information, see + * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). + */ + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + this.asSome() + .getLocation() + .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + or + this.isNone() and + filepath = "" and + startline = 0 and + startcolumn = 0 and + endline = 0 and + endcolumn = 0 + } + + string getQualifiedName() { + result = this.asSome().getQualifiedName() + or + this.isNone() and + result = "" + } +} + +bindingset[name] +FunctionOption getPossibleMissingSuperOption(Class base, Function shouldCall, string name) { + result.asSome() = getPossibleMissingSuper(base, shouldCall, name) + or + not exists(getPossibleMissingSuper(base, shouldCall, name)) and + result.isNone() +} diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql index 4cc72ee9facd..3d742d45a006 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql @@ -15,21 +15,35 @@ import python import MethodCallOrder -predicate missingCallToSuperclassDel(Function base, Function shouldCall, Class mroStart) { - missingCallToSuperclassMethod(base, shouldCall, mroStart, "__del__") +Function getDelMethod(Class c) { + result = c.getAMethod() and + result.getName() = "__del__" } -from Function base, Function shouldCall, Class mroStart, string msg +from Class base, Function shouldCall, FunctionOption possibleIssue, string msg where - missingCallToSuperclassDel(base, shouldCall, mroStart) and - ( - // Simple case: the method that should be called is directly overridden - mroStart = base.getScope() and - msg = "This delete method does not call $@, which may leave $@ not properly cleaned up." - or - // Only alert for a different mro base if there are no alerts for direct overrides - not missingCallToSuperclassDel(base, _, base.getScope()) and - msg = - "This delete method does not call super().__del__, which may cause $@ to be missed during the cleanup of $@." + not exists(Function newMethod | newMethod = base.getAMethod() and newMethod.getName() = "__new__") and + exists(FunctionOption possiblyMissingSuper | + missingCallToSuperclassMethodRestricted(base, shouldCall, "__del__") and + possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__del__") and + ( + not possiblyMissingSuper.isNone() and + possibleIssue = possiblyMissingSuper and + msg = + "This class does not call $@ during destruction. ($@ may be missing a call to super().__del__)" + or + possiblyMissingSuper.isNone() and + ( + possibleIssue.asSome() = getDelMethod(base) and + msg = + "This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__)" + or + not getDelMethod(base) and + possibleIssue.isNone() and + msg = + "This class does not call $@ during destruction. (The class lacks an __del__ method to ensure every base class __del__ is called.)" + ) + ) ) -select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName() +select base, msg, shouldCall, shouldCall.getQualifiedName(), possibleIssue, + possibleIssue.getQualifiedName() diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql index 56c6bd258cd2..1b13fef46c73 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql @@ -14,21 +14,30 @@ import python import MethodCallOrder -predicate missingCallToSuperclassInit(Function base, Function shouldCall, Class mroStart) { - missingCallToSuperclassMethod(base, shouldCall, mroStart, "__init__") -} - -from Function base, Function shouldCall, Class mroStart, string msg +from Class base, Function shouldCall, FunctionOption possibleIssue, string msg where - 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 super().__init__, which may cause $@ to be missed during the initialization of $@." + not exists(Function newMethod | newMethod = base.getAMethod() and newMethod.getName() = "__new__") and + exists(FunctionOption possiblyMissingSuper | + missingCallToSuperclassMethodRestricted(base, shouldCall, "__init__") and + possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__init__") and + ( + not possiblyMissingSuper.isNone() and + possibleIssue = possiblyMissingSuper and + msg = + "This class does not call $@ during initialization. ($@ may be missing a call to super().__init__)" + or + possiblyMissingSuper.isNone() and + ( + possibleIssue.asSome() = base.getInitMethod() and + msg = + "This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__)" + or + not exists(base.getInitMethod()) and + possibleIssue.isNone() and + msg = + "This class does not call $@ during initialization. (The class lacks an __init__ method to ensure every base class __init__ is called.)" + ) + ) ) -select base, msg, shouldCall, shouldCall.getQualifiedName(), mroStart, mroStart.getName() +select base, msg, shouldCall, shouldCall.getQualifiedName(), possibleIssue, + possibleIssue.getQualifiedName() From 16b90a176b3fcd00318a1468b8c46c1b364ad8fe Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 3 Jul 2025 15:32:54 +0100 Subject: [PATCH 10/28] Fixes --- python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll | 3 +-- python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql | 2 +- python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 1ebde88083a8..f5853e686734 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -85,9 +85,8 @@ predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) { } predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string name) { - base.getName() = name and shouldCall.getName() = name and - base = getADirectSuperclass*(base.getScope()) and + shouldCall.getScope() = getADirectSuperclass+(base) and not shouldCall = getASuperCallTargetFromClass(base, base, name) and nonTrivial(shouldCall) and // "Benefit of the doubt" - if somewhere in the chain we call an unknown superclass, assume all the necessary parent methods are called from it diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql index 3d742d45a006..a6ade1f1d312 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql @@ -38,7 +38,7 @@ where msg = "This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__)" or - not getDelMethod(base) and + not exists(getDelMethod(base)) and possibleIssue.isNone() and msg = "This class does not call $@ during destruction. (The class lacks an __del__ method to ensure every base class __del__ is called.)" diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql index 1b13fef46c73..9081960a75bb 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql @@ -21,8 +21,7 @@ where missingCallToSuperclassMethodRestricted(base, shouldCall, "__init__") and possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__init__") and ( - not possiblyMissingSuper.isNone() and - possibleIssue = possiblyMissingSuper and + possibleIssue.asSome() = possiblyMissingSuper.asSome() and msg = "This class does not call $@ during initialization. ($@ may be missing a call to super().__init__)" or From b3056fc5a749643f00b5fa8abef72a591e84af21 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 3 Jul 2025 15:48:52 +0100 Subject: [PATCH 11/28] Update tests for calls to init + fixes --- .../CallsToInitDel/MethodCallOrder.qll | 14 +- .../missing-init/MissingCallToInit.expected | 8 +- .../Classes/missing-init/missing_init.py | 124 +++++++++--------- 3 files changed, 77 insertions(+), 69 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index f5853e686734..191243e03328 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -95,13 +95,15 @@ predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCall, string name) { missingCallToSuperclassMethod(base, shouldCall, name) and - not exists(Class subBase | - subBase = getADirectSubclass+(base) and - missingCallToSuperclassMethod(subBase, shouldCall, name) + not exists(Class superBase | + // Alert only on the highest base class that has the issue + superBase = getADirectSuperclass+(base) and + missingCallToSuperclassMethod(superBase, shouldCall, name) ) and - not exists(Function superShouldCall | - superShouldCall.getScope() = getADirectSuperclass+(shouldCall.getScope()) and - missingCallToSuperclassMethod(base, superShouldCall, name) + not exists(Function subShouldCall | + // Mention in the alert only the lowest method we're missing the call to + subShouldCall.getScope() = getADirectSubclass+(shouldCall.getScope()) and + missingCallToSuperclassMethod(base, subShouldCall, name) ) } diff --git a/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected b/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected index 6cb92041a630..90ca4bf49e77 100644 --- a/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected +++ b/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected @@ -1,3 +1,5 @@ -| missing_init.py:12:1:12:13 | class B3 | Class B3 may not be initialized properly as $@ is not called from its $@. | missing_init.py:9:5:9:23 | Function __init__ | method B2.__init__ | missing_init.py:14:5:14:23 | Function __init__ | __init__ method | -| missing_init.py:39:1:39:21 | class IUVT | Class IUVT may not be initialized properly as $@ is not called from its $@. | missing_init.py:30:5:30:23 | Function __init__ | method UT.__init__ | missing_init.py:26:5:26:23 | Function __init__ | __init__ method | -| missing_init.py:72:1:72:13 | class AB | Class AB may not be initialized properly as $@ is not called from its $@. | missing_init.py:69:5:69:23 | Function __init__ | method AA.__init__ | missing_init.py:75:5:75:23 | Function __init__ | __init__ method | +| missing_init.py:14:5:14:23 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:9:5:9:23 | Function __init__ | B2.__init__ | missing_init.py:13:1:13:13 | Class B3 | B3 | +| missing_init.py:29:5:29:23 | Function __init__ | This initialization method does not call super().__init__, which may cause $@ to be missed during the initialization of $@. | missing_init.py:33:5:33:23 | Function __init__ | UT.__init__ | missing_init.py:42:1:42:21 | Class IUVT | IUVT | +| missing_init.py:70:5:70:23 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:64:5:64:23 | Function __init__ | AA.__init__ | missing_init.py:67:1:67:13 | Class AB | AB | +| missing_init.py:124:9:124:27 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:117:5:117:23 | Function __init__ | DA.__init__ | missing_init.py:122:5:122:17 | Class DC | DC | +| missing_init.py:134:5:134:23 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:117:5:117:23 | Function __init__ | DA.__init__ | missing_init.py:132:1:132:13 | Class DD | DD | diff --git a/python/ql/test/query-tests/Classes/missing-init/missing_init.py b/python/ql/test/query-tests/Classes/missing-init/missing_init.py index b1651557759f..68c5b1fad7b5 100644 --- a/python/ql/test/query-tests/Classes/missing-init/missing_init.py +++ b/python/ql/test/query-tests/Classes/missing-init/missing_init.py @@ -2,18 +2,21 @@ class B1(object): def __init__(self): - do_something() + print("B1 init") class B2(B1): def __init__(self): + print("B2 init") B1.__init__(self) -class B3(B2): - - def __init__(self): +class B3(B2): # $ Alert + def __init__(self): + print("B3 init") B1.__init__(self) +B3() + #OK if superclass __init__ is builtin as #builtin classes tend to rely on __new__ class MyException(Exception): @@ -23,11 +26,11 @@ def __init__(self): #ODASA-4107 class IUT(object): - def __init__(self): + def __init__(self): print("IUT init") class UT(object): - def __init__(self): + def __init__(self): print("UT init") class PU(object): @@ -36,150 +39,151 @@ class PU(object): class UVT(UT, PU): pass -class IUVT(IUT, UVT): +class IUVT(IUT, UVT): # $ Alert pass -#False positive +print("IUVT") +IUVT() + class M1(object): def __init__(self): - print("A") + print("M1 init") class M2(object): pass class Mult(M2, M1): def __init__(self): - super(Mult, self).__init__() # Calls M1.__init__ - -class X: - def __init__(self): - do_something() - -class Y(X): - @decorated - def __init__(self): - X.__init__(self) + print("Mult init") + super(Mult, self).__init__() # OK - Calls M1.__init__ -class Z(Y): - def __init__(self): - Y.__init__(self) +Mult() class AA(object): def __init__(self): - do_something() + print("AA init") -class AB(AA): +class AB(AA): # $ Alert - #Don't call super class init - def __init__(self): - do_something() + # Doesn't call super class init + def __init__(self): + print("AB init") class AC(AB): def __init__(self): - #Missing call to AA.__init__ but not AC's fault. + # Doesn't call AA init, but we don't alert here as the issue is with AB. + print("AC init") super(AC, self).__init__() +AC() + import six import abc class BA(object): def __init__(self): - do_something() + print("BA init") @six.add_metaclass(abc.ABCMeta) class BB(BA): def __init__(self): + print("BB init") super(BB,self).__init__() +BB() + @six.add_metaclass(abc.ABCMeta) class CA(object): def __init__(self): - do_something() + print("CA init") -class CB(BA): +class CB(CA): def __init__(self): + print("CB init") super(CB,self).__init__() +CB() + #ODASA-5799 class DA(object): def __init__(self): - do_something() + print("DA init") class DB(DA): - class DC(DA): + class DC(DA): # $ SPURIOUS: Alert # We only consider direct super calls, so have an FP here - def __init__(self): + def __init__(self): + print("DC init") sup = super(DB.DC, self) sup.__init__() +DB.DC() + #Simpler variants -class DD(DA): +class DD(DA): # $ SPURIOUS: Alert # We only consider direct super calls, so have an FP here - def __init__(self): + def __init__(self): + print("DD init") sup = super(DD, self) sup.__init__() +DD() + class DE(DA): - class DF(DA): + class DF(DA): # No alert here - def __init__(self): + def __init__(self): + print("DF init") sup = super(DE.DF, self).__init__() +DE.DF() + class FA(object): def __init__(self): - pass + pass # does nothing, thus is considered a trivial method and ok to not call class FB(object): def __init__(self): - do_something() + print("FB init") class FC(FA, FB): def __init__(self): - #OK to skip call to FA.__init__ as that does nothing. + # No alert here - ok to skip call to trivial FA init FB.__init__(self) #Potential false positives. class ConfusingInit(B1): - def __init__(self): + def __init__(self): # We track this correctly and don't alert. super_call = super(ConfusingInit, self).__init__ super_call() -# Library class -import collections - -class G1(collections.Counter): - +class G1: def __init__(self): - collections.Counter.__init__(self) - -class G2(G1): + print("G1 init") +class G2: def __init__(self): - super(G2, self).__init__() + print("G2 init") -class G3(collections.Counter): - - def __init__(self): - super(G3, self).__init__() - -class G4(G3): - - def __init__(self): - G3.__init__(self) +class G3(G1, G2): + def __init__(self): + print("G3 init") + for cls in self.__class__.__bases__: + cls.__init__(self) # We dont track which classes this could refer to, but assume it calls all required init methods and don't alert. From 73057d3b6fbe2667afa18f42559e077e7e78bbf8 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 3 Jul 2025 15:56:28 +0100 Subject: [PATCH 12/28] Add additional test case + update missing del tests --- .../missing-del/MissingCallToDel.expected | 2 +- .../Classes/missing-del/missing_del.py | 9 +++++++-- .../missing-init/MissingCallToInit.expected | 11 ++++++----- .../Classes/missing-init/missing_init.py | 16 ++++++++++++++++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected b/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected index 7f080b1d729b..80e02da0e132 100644 --- a/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected +++ b/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected @@ -1 +1 @@ -| missing_del.py:12:1:12:13 | class X3 | Class X3 may not be cleaned up properly as $@ is not called during deletion. | missing_del.py:9:5:9:22 | Function __del__ | method X2.__del__ | +| missing_del.py:13:1:13:13 | Class X3 | This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__) | missing_del.py:9:5:9:22 | Function __del__ | X2.__del__ | missing_del.py:15:5:15:22 | Function __del__ | X3.__del__ | diff --git a/python/ql/test/query-tests/Classes/missing-del/missing_del.py b/python/ql/test/query-tests/Classes/missing-del/missing_del.py index 5d4e30e681d2..6548bb1fa3b4 100644 --- a/python/ql/test/query-tests/Classes/missing-del/missing_del.py +++ b/python/ql/test/query-tests/Classes/missing-del/missing_del.py @@ -2,14 +2,19 @@ class X1(object): def __del__(self): - pass + print("X1 del") class X2(X1): def __del__(self): + print("X2 del") X1.__del__(self) -class X3(X2): +class X3(X2): # $ Alert - skips X2 del def __del__(self): + print("X3 del") X1.__del__(self) + +a = X3() +del a diff --git a/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected b/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected index 90ca4bf49e77..c0f35be3ff9b 100644 --- a/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected +++ b/python/ql/test/query-tests/Classes/missing-init/MissingCallToInit.expected @@ -1,5 +1,6 @@ -| missing_init.py:14:5:14:23 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:9:5:9:23 | Function __init__ | B2.__init__ | missing_init.py:13:1:13:13 | Class B3 | B3 | -| missing_init.py:29:5:29:23 | Function __init__ | This initialization method does not call super().__init__, which may cause $@ to be missed during the initialization of $@. | missing_init.py:33:5:33:23 | Function __init__ | UT.__init__ | missing_init.py:42:1:42:21 | Class IUVT | IUVT | -| missing_init.py:70:5:70:23 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:64:5:64:23 | Function __init__ | AA.__init__ | missing_init.py:67:1:67:13 | Class AB | AB | -| missing_init.py:124:9:124:27 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:117:5:117:23 | Function __init__ | DA.__init__ | missing_init.py:122:5:122:17 | Class DC | DC | -| missing_init.py:134:5:134:23 | Function __init__ | This initialization method does not call $@, which may leave $@ partially initialized. | missing_init.py:117:5:117:23 | Function __init__ | DA.__init__ | missing_init.py:132:1:132:13 | Class DD | DD | +| missing_init.py:13:1:13:13 | Class B3 | This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__) | missing_init.py:9:5:9:23 | Function __init__ | B2.__init__ | missing_init.py:14:5:14:23 | Function __init__ | B3.__init__ | +| missing_init.py:42:1:42:21 | Class IUVT | This class does not call $@ during initialization. (The class lacks an __init__ method to ensure every base class __init__ is called.) | missing_init.py:33:5:33:23 | Function __init__ | UT.__init__ | file://:0:0:0:0 | (none) | | +| missing_init.py:67:1:67:13 | Class AB | This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__) | missing_init.py:64:5:64:23 | Function __init__ | AA.__init__ | missing_init.py:70:5:70:23 | Function __init__ | AB.__init__ | +| missing_init.py:122:5:122:17 | Class DC | This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__) | missing_init.py:117:5:117:23 | Function __init__ | DA.__init__ | missing_init.py:124:9:124:27 | Function __init__ | DB.DC.__init__ | +| missing_init.py:132:1:132:13 | Class DD | This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__) | missing_init.py:117:5:117:23 | Function __init__ | DA.__init__ | missing_init.py:134:5:134:23 | Function __init__ | DD.__init__ | +| missing_init.py:200:1:200:17 | Class H3 | This class does not call $@ during initialization. ($@ may be missing a call to super().__init__) | missing_init.py:197:5:197:23 | Function __init__ | H2.__init__ | missing_init.py:193:5:193:23 | Function __init__ | H1.__init__ | diff --git a/python/ql/test/query-tests/Classes/missing-init/missing_init.py b/python/ql/test/query-tests/Classes/missing-init/missing_init.py index 68c5b1fad7b5..68498765f75f 100644 --- a/python/ql/test/query-tests/Classes/missing-init/missing_init.py +++ b/python/ql/test/query-tests/Classes/missing-init/missing_init.py @@ -187,3 +187,19 @@ def __init__(self): for cls in self.__class__.__bases__: cls.__init__(self) # We dont track which classes this could refer to, but assume it calls all required init methods and don't alert. +G3() + +class H1: + def __init__(self): + print("H1 init") + +class H2: + def __init__(self): + print("H2 init") + +class H3(H1, H2): # $ Alert # The alert should also mention that H1.__init__ may be missing a call to super().__init__ + def __init__(self): + print("H3 init") + super().__init__() + +H3() \ No newline at end of file From 2e6f35b29017a50827558213233375bd534ba71d Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 3 Jul 2025 16:07:17 +0100 Subject: [PATCH 13/28] Remove case excluding classes with a __new__ method; as it doesn't make much sense (__init__ is still called anyway) --- python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql index 9081960a75bb..566d8320a29b 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql @@ -16,7 +16,6 @@ import MethodCallOrder from Class base, Function shouldCall, FunctionOption possibleIssue, string msg where - not exists(Function newMethod | newMethod = base.getAMethod() and newMethod.getName() = "__new__") and exists(FunctionOption possiblyMissingSuper | missingCallToSuperclassMethodRestricted(base, shouldCall, "__init__") and possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__init__") and From c5b79fa622b7e6e690cdb11d0457f861104cd483 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 10:30:20 +0100 Subject: [PATCH 14/28] Update multiple calls queries to include call targets in alert message --- .../CallsToInitDel/MethodCallOrder.qll | 38 +++++++++++++++---- .../SuperclassDelCalledMultipleTimes.ql | 33 ++++++++++++---- .../SuperclassInitCalledMultipleTimes.ql | 33 ++++++++++++---- 3 files changed, 81 insertions(+), 23 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 191243e03328..02d60ca420e0 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -5,11 +5,14 @@ import semmle.python.ApiGraphs import semmle.python.dataflow.new.internal.DataFlowDispatch import codeql.util.Option -predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, string name) { - exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls | +predicate multipleCallsToSuperclassMethod( + Function meth, Function calledMulti, DataFlow::MethodCallNode call1, + DataFlow::MethodCallNode call2, string name +) { + exists(Class cls | meth.getName() = name and meth.getScope() = cls and - call1.asExpr() != call2.asExpr() and + call1.getLocation().toString() < call2.getLocation().toString() and calledMulti = getASuperCallTargetFromCall(cls, meth, call1, name) and calledMulti = getASuperCallTargetFromCall(cls, meth, call2, name) and nonTrivial(calledMulti) @@ -18,23 +21,44 @@ predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, s Function getASuperCallTargetFromCall( Class mroBase, Function meth, DataFlow::MethodCallNode call, string name +) { + exists(Function target | target = getDirectSuperCallTargetFromCall(mroBase, meth, call, name) | + result = target + or + result = getASuperCallTargetFromCall(mroBase, target, _, name) + ) +} + +Function getDirectSuperCallTargetFromCall( + Class mroBase, Function meth, DataFlow::MethodCallNode call, string name ) { meth = call.getScope() and getADirectSuperclass*(mroBase) = meth.getScope() and meth.getName() = name and call.calls(_, name) and - exists(Class targetCls | result = getASuperCallTargetFromClass(mroBase, targetCls, name) | + mroBase = getADirectSubclass*(meth.getScope()) and + exists(Class targetCls | + // the differences between 0-arg and 2-arg super is not considered; we assume each super uses the mro of the instance `self` superCall(call, _) and - targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase) + targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase) and + result = findFunctionAccordingToMroKnownStartingClass(targetCls, mroBase, name) or - callsMethodOnClassWithSelf(meth, call, targetCls, _) + // targetCls is the mro base for this lookup. + // note however that if the call we find uses super(), that still uses the mro of the instance `self` will sill be used + // assuming it's 0-arg or is 2-arg with `self` as second arg. + callsMethodOnClassWithSelf(meth, call, targetCls, _) and + result = findFunctionAccordingToMroKnownStartingClass(targetCls, targetCls, name) ) } Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) { exists(Function target | target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and - (result = target or result = getASuperCallTargetFromCall(mroBase, target, _, name)) + ( + result = target + or + result = getASuperCallTargetFromCall(mroBase, target, _, name) + ) ) } diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql index 7aca3dee1892..7772aa153730 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql @@ -14,17 +14,34 @@ import python import MethodCallOrder -predicate multipleCallsToSuperclassDel(Function meth, Function calledMulti) { - multipleCallsToSuperclassMethod(meth, calledMulti, "__del__") +predicate multipleCallsToSuperclassDel( + Function meth, Function calledMulti, DataFlow::MethodCallNode call1, + DataFlow::MethodCallNode call2 +) { + multipleCallsToSuperclassMethod(meth, calledMulti, call1, call2, "__del__") } -from Function meth, Function calledMulti +from + Function meth, Function calledMulti, DataFlow::MethodCallNode call1, + DataFlow::MethodCallNode call2, Function target1, Function target2, string msg where - multipleCallsToSuperclassDel(meth, calledMulti) and - // Don't alert for multiple calls to a superclass del when a subclass will do. + multipleCallsToSuperclassDel(meth, calledMulti, call1, call2) and + // Only alert for the lowest method in the hierarchy that both calls will call. not exists(Function subMulti | - multipleCallsToSuperclassDel(meth, subMulti) and + multipleCallsToSuperclassDel(meth, subMulti, _, _) and calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope()) + ) and + target1 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call1, _) and + target2 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call2, _) and + ( + target1 != target2 and + msg = + "This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively." + or + target1 = target2 and + // The targets themselves are called multiple times (either is calledMulti, or something earlier in the MRO) + // Mentioning them again would be redundant. + msg = "This deletion method calls $@ multiple times, via $@ and $@." ) -select meth, "This delete method calls $@ multiple times.", calledMulti, - calledMulti.getQualifiedName() +select meth, msg, calledMulti, calledMulti.getQualifiedName(), call1, "this call", call2, + "this call", target1, target1.getQualifiedName(), target2, target2.getQualifiedName() diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql index 4f577dc4a764..04c226aa1951 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql @@ -14,17 +14,34 @@ import python import MethodCallOrder -predicate multipleCallsToSuperclassInit(Function meth, Function calledMulti) { - multipleCallsToSuperclassMethod(meth, calledMulti, "__init__") +predicate multipleCallsToSuperclassInit( + Function meth, Function calledMulti, DataFlow::MethodCallNode call1, + DataFlow::MethodCallNode call2 +) { + multipleCallsToSuperclassMethod(meth, calledMulti, call1, call2, "__init__") } -from Function meth, Function calledMulti +from + Function meth, Function calledMulti, DataFlow::MethodCallNode call1, + DataFlow::MethodCallNode call2, Function target1, Function target2, string msg where - multipleCallsToSuperclassInit(meth, calledMulti) and - // Don't alert for multiple calls to a superclass init when a subclass will do. + multipleCallsToSuperclassInit(meth, calledMulti, call1, call2) and + // Only alert for the lowest method in the hierarchy that both calls will call. not exists(Function subMulti | - multipleCallsToSuperclassInit(meth, subMulti) and + multipleCallsToSuperclassInit(meth, subMulti, _, _) and calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope()) + ) and + target1 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call1, _) and + target2 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call2, _) and + ( + target1 != target2 and + msg = + "This initialization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively." + or + target1 = target2 and + // The targets themselves are called multiple times (either is calledMulti, or something earlier in the MRO) + // Mentioning them again would be redundant. + msg = "This initialization method calls $@ multiple times, via $@ and $@." ) -select meth, "This initialization method calls $@ multiple times.", calledMulti, - calledMulti.getQualifiedName() +select meth, msg, calledMulti, calledMulti.getQualifiedName(), call1, "this call", call2, + "this call", target1, target1.getQualifiedName(), target2, target2.getQualifiedName() From 804b9ef941447cbdef5c0a4f8a668b68bdb88163 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 11:08:33 +0100 Subject: [PATCH 15/28] Update tests and add an additional test --- .../SuperclassDelCalledMultipleTimes.expected | 4 ++-- ...SuperclassInitCalledMultipleTimes.expected | 5 +++-- .../multiple/multiple-init/multiple_init.py | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected index ad9858397dff..0df0670b2191 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected +++ b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected @@ -1,2 +1,2 @@ -| multiple_del.py:21:5:21:22 | Function __del__ | This delete method calls $@ multiple times. | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | -| multiple_del.py:43:5:43:22 | Function __del__ | This delete method calls $@ multiple times. | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | +| multiple_del.py:21:5:21:22 | Function __del__ | This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | multiple_del.py:23:9:23:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:24:9:24:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | multiple_del.py:15:5:15:22 | Function __del__ | Y2.__del__ | +| multiple_del.py:43:5:43:22 | Function __del__ | This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | multiple_del.py:45:9:45:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:46:9:46:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | multiple_del.py:37:5:37:22 | Function __del__ | Z2.__del__ | diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected index 42d019e7f710..e2a0934e9027 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected +++ b/python/ql/test/query-tests/Classes/multiple/multiple-init/SuperclassInitCalledMultipleTimes.expected @@ -1,2 +1,3 @@ -| multiple_init.py:21:5:21:23 | Function __init__ | This initialization method calls $@ multiple times. | multiple_init.py:9:5:9:23 | Function __init__ | C1.__init__ | -| multiple_init.py:42:5:42:23 | Function __init__ | This initialization method calls $@ multiple times. | multiple_init.py:31:5:31:23 | Function __init__ | D1.__init__ | +| multiple_init.py:21:5:21:23 | Function __init__ | This initialization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_init.py:9:5:9:23 | Function __init__ | C1.__init__ | multiple_init.py:23:9:23:25 | ControlFlowNode for Attribute() | this call | multiple_init.py:24:9:24:25 | ControlFlowNode for Attribute() | this call | multiple_init.py:9:5:9:23 | Function __init__ | C1.__init__ | multiple_init.py:15:5:15:23 | Function __init__ | C2.__init__ | +| multiple_init.py:42:5:42:23 | Function __init__ | This initialization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_init.py:31:5:31:23 | Function __init__ | D1.__init__ | multiple_init.py:44:9:44:25 | ControlFlowNode for Attribute() | this call | multiple_init.py:45:9:45:25 | ControlFlowNode for Attribute() | this call | multiple_init.py:31:5:31:23 | Function __init__ | D1.__init__ | multiple_init.py:36:5:36:23 | Function __init__ | D2.__init__ | +| multiple_init.py:84:5:84:23 | Function __init__ | This initialization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_init.py:80:5:80:23 | Function __init__ | F3.__init__ | multiple_init.py:86:9:86:25 | ControlFlowNode for Attribute() | this call | multiple_init.py:87:9:87:25 | ControlFlowNode for Attribute() | this call | multiple_init.py:75:5:75:23 | Function __init__ | F2.__init__ | multiple_init.py:80:5:80:23 | Function __init__ | F3.__init__ | diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py b/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py index cba8b24523fa..59efb28e691e 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py +++ b/python/ql/test/query-tests/Classes/multiple/multiple-init/multiple_init.py @@ -68,3 +68,22 @@ def __init__(self): # OK: builtin init methods are fine. E3() +class F1: + pass + +class F2(F1): + def __init__(self): + print("F2 init") + super().__init__() + +class F3(F1): + def __init__(self): + print("F3 init") + +class F4(F2, F3): + def __init__(self): # $ Alert # F2's super call calls F3 + print("F4 init") + F2.__init__(self) + F3.__init__(self) + +F4() \ No newline at end of file From 6ca4f32ce0702415c0b94ec4a14444393e10f892 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 11:32:39 +0100 Subject: [PATCH 16/28] qhelp: move examples to subfolder --- .../Classes/CallsToInitDel/{ => examples}/MissingCallToDel.py | 0 .../CallsToInitDel/{ => examples}/MissingCallToInit.py | 4 ++-- .../{ => examples}/SuperclassDelCalledMultipleTimes.py | 0 .../{ => examples}/SuperclassDelCalledMultipleTimes2.py | 0 .../{ => examples}/SuperclassInitCalledMultipleTimes.py | 0 .../{ => examples}/SuperclassInitCalledMultipleTimes2.py | 0 6 files changed, 2 insertions(+), 2 deletions(-) rename python/ql/src/Classes/CallsToInitDel/{ => examples}/MissingCallToDel.py (100%) rename python/ql/src/Classes/CallsToInitDel/{ => examples}/MissingCallToInit.py (85%) rename python/ql/src/Classes/CallsToInitDel/{ => examples}/SuperclassDelCalledMultipleTimes.py (100%) rename python/ql/src/Classes/CallsToInitDel/{ => examples}/SuperclassDelCalledMultipleTimes2.py (100%) rename python/ql/src/Classes/CallsToInitDel/{ => examples}/SuperclassInitCalledMultipleTimes.py (100%) rename python/ql/src/Classes/CallsToInitDel/{ => examples}/SuperclassInitCalledMultipleTimes2.py (100%) diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.py b/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToDel.py similarity index 100% rename from python/ql/src/Classes/CallsToInitDel/MissingCallToDel.py rename to python/ql/src/Classes/CallsToInitDel/examples/MissingCallToDel.py diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.py b/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToInit.py similarity index 85% rename from python/ql/src/Classes/CallsToInitDel/MissingCallToInit.py rename to python/ql/src/Classes/CallsToInitDel/examples/MissingCallToInit.py index 1b3e0e3aee59..14d7c5a80fd3 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.py +++ b/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToInit.py @@ -10,14 +10,14 @@ def __init__(self): Vehicle.__init__(self) self.car_init() -#Car.__init__ is missed out. +# BAD: Car.__init__ is not called. class SportsCar(Car, Vehicle): def __init__(self): Vehicle.__init__(self) self.sports_car_init() -#Fix SportsCar by calling Car.__init__ +# GOOD: Car.__init__ is called correctly. class FixedSportsCar(Car, Vehicle): def __init__(self): diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes.py similarity index 100% rename from python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.py rename to python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes.py diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes2.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes2.py similarity index 100% rename from python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes2.py rename to python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes2.py diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes.py similarity index 100% rename from python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.py rename to python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes.py diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes2.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes2.py similarity index 100% rename from python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes2.py rename to python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes2.py From 2e5f470d742dbe36eb6bd4b2945c699bca181c72 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 15:44:37 +0100 Subject: [PATCH 17/28] Update qhelp + alert messages --- .../CallsToInitDel/MissingCallToDel.qhelp | 38 ++++++------ .../CallsToInitDel/MissingCallToDel.ql | 6 +- .../CallsToInitDel/MissingCallToInit.qhelp | 40 ++++++------- .../SuperclassDelCalledMultipleTimes.qhelp | 49 +++++++-------- .../SuperclassDelCalledMultipleTimes.ql | 4 +- .../SuperclassInitCalledMultipleTimes.qhelp | 60 ++++++++++++------- .../examples/MissingCallToDel.py | 4 +- .../SuperclassDelCalledMultipleTimes.py | 21 ++++--- .../SuperclassDelCalledMultipleTimes2.py | 32 ---------- .../SuperclassInitCalledMultipleTimes.py | 36 ----------- .../SuperclassInitCalledMultipleTimes2.py | 38 ------------ .../SuperclassInitCalledMultipleTimesBad1.py | 20 +++++++ .../SuperclassInitCalledMultipleTimesBad3.py | 22 +++++++ .../SuperclassInitCalledMultipleTimesGood2.py | 22 +++++++ .../missing-del/MissingCallToDel.expected | 2 +- .../SuperclassDelCalledMultipleTimes.expected | 4 +- 16 files changed, 186 insertions(+), 212 deletions(-) delete mode 100644 python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes2.py delete mode 100644 python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes.py delete mode 100644 python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes2.py create mode 100644 python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad1.py create mode 100644 python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad3.py create mode 100644 python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp index 864ddd1b56b8..a9897d3c682e 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp @@ -4,47 +4,49 @@ -

Python, unlike statically typed languages such as Java, allows complete freedom when calling methods during object destruction. -However, standard object-oriented principles apply to Python classes using deep inheritance hierarchies. -Therefore the developer has responsibility for ensuring that objects are properly cleaned up when -there are multiple __del__ methods that need to be called. +

+Python, unlike some other object-oriented languages such as Java, allows the developer complete freedom in +when and how superclass finalizers are called during object finalization. +However, the developer has responsibility for ensuring that objects are properly cleaned up, and that all superclass __del__ +methods are called.

-If the __del__ method of a superclass is not called during object destruction it is likely that +Classes with a __del__ method (a finalizer) typically hold some resource such as a file handle that needs to be cleaned up. +If the __del__ method of a superclass is not called during object finalization, it is likely that that resources may be leaked.

-

A call to the __del__ method of a superclass during object destruction may be omitted: +

A call to the __init__ method of a superclass during object initialization may be unintentionally skipped:

    -
  • When a subclass calls the __del__ method of the wrong class.
    • -
  • When a call to the __del__ method of one its base classes is omitted.
    • +
  • If a subclass calls the __del__ method of the wrong class.
    • +
  • If a call to the __del__ method of one its base classes is omitted.
    • +
  • If a call to super().__del__ is used, but not all __del__ methods in the Method Resolution Order (MRO) + chain themselves call super(). This in particular arises more often in cases of multiple inheritance.
-

Either be careful to explicitly call the __del__ of the correct base class, or -use super() throughout the inheritance hierarchy.

- -

Alternatively refactor one or more of the classes to use composition rather than inheritance.

+

Ensure that all superclass __del__ methods are properly called. +Either each base class's finalize method should be explicitly called, or super() calls +should be consistently used throughout the inheritance hierarchy.

 -

In this example, explicit calls to __del__ are used, but SportsCar erroneously calls +

In the following example, explicit calls to __del__ are used, but SportsCar erroneously calls Vehicle.__del__. This is fixed in FixedSportsCar by calling Car.__del__.

- + -
  • Python Tutorial: Classes.
    • -
  • Python Standard Library: super.
    • -
  • Artima Developer: Things to Know About Python Super.
    • -
  • Wikipedia: Composition over inheritance.
    • +
  • Python Reference: __del__.
    • +
  • Python Standard Library: super.
    • +
  • Python Glossary: Method resolution order.
    •      diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql index a6ade1f1d312..7d8e11b569d7 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql @@ -30,18 +30,18 @@ where not possiblyMissingSuper.isNone() and possibleIssue = possiblyMissingSuper and msg = - "This class does not call $@ during destruction. ($@ may be missing a call to super().__del__)" + "This class does not call $@ during finalization. ($@ may be missing a call to super().__del__)" or possiblyMissingSuper.isNone() and ( possibleIssue.asSome() = getDelMethod(base) and msg = - "This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__)" + "This class does not call $@ during finalization. ($@ may be missing a call to a base class __del__)" or not exists(getDelMethod(base)) and possibleIssue.isNone() and msg = - "This class does not call $@ during destruction. (The class lacks an __del__ method to ensure every base class __del__ is called.)" + "This class does not call $@ during finalization. (The class lacks an __del__ method to ensure every base class __del__ is called.)" ) ) ) diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.qhelp b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.qhelp index 31ad3d693a5d..76121446f99e 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.qhelp @@ -4,49 +4,47 @@ -

    Python, unlike statically typed languages such as Java, allows complete freedom when calling methods during object initialization. -However, standard object-oriented principles apply to Python classes using deep inheritance hierarchies. -Therefore the developer has responsibility for ensuring that objects are properly initialized when -there are multiple __init__ methods that need to be called. +

    Python, unlike some other object-oriented languages such as Java, allows the developer complete freedom in +when and how superclass initializers are called during object initialization. +However, the developer has responsibility for ensuring that objects are properly initialized, and that all superclass __init__ +methods are called.

    -If the __init__ method of a superclass is not called during object initialization it is likely that -that object will end up in an incorrect state. +If the __init__ method of a superclass is not called during object initialization, this can lead to errors due to +the object not being fully initialized, such as having missing attributes.

    -

    A call to the __init__ method of a superclass during object initialization may be omitted: +

    A call to the __init__ method of a superclass during object initialization may be unintentionally skipped:

      -
    • When a subclass calls the __init__ method of the wrong class.
      • -
    • When a call to the __init__ method of one its base classes is omitted.
      • -
    • When multiple inheritance is used and a class inherits from several base classes, - and at least one of those does not use super() in its own __init__ method.
      • +
    • If a subclass calls the __init__ method of the wrong class.
      • +
    • If a call to the __init__ method of one its base classes is omitted.
      • +
    • If a call to super().__init__ is used, but not all __init__ methods in the Method Resolution Order (MRO) + chain themselves call super(). This in particular arises more often in cases of multiple inheritance.
    -

    Either be careful to explicitly call the __init__ of the correct base class, or -use super() throughout the inheritance hierarchy.

    - -

    Alternatively refactor one or more of the classes to use composition rather than inheritance.

    +

    Ensure that all superclass __init__ methods are properly called. +Either each base class's initialize method should be explicitly called, or super() calls +should be consistently used throughout the inheritance hierarchy.

     -

    In this example, explicit calls to __init__ are used, but SportsCar erroneously calls +

    In the following example, explicit calls to __init__ are used, but SportsCar erroneously calls Vehicle.__init__. This is fixed in FixedSportsCar by calling Car.__init__.

    - +     -
  • Python Tutorial: Classes.
    • -
  • Python Standard Library: super.
    • -
  • Artima Developer: Things to Know About Python Super.
    • -
  • Wikipedia: Composition over inheritance.
    • +
  • Python Reference: __init__.
    • +
  • Python Standard Library: super.
    • +
  • Python Glossary: Method resolution order.
    •      diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp index d9514b2c68c4..f828cfb8e648 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp @@ -4,55 +4,52 @@ -

    Python, unlike statically typed languages such as Java, allows complete freedom when calling methods during object destruction. -However, standard object-oriented principles apply to Python classes using deep inheritance hierarchies. -Therefore the developer has responsibility for ensuring that objects are properly cleaned up when -there are multiple __del__ methods that need to be called. +

    +Python, unlike some other object-oriented languages such as Java, allows the developer complete freedom in +when and how superclass finalizers are called during object finalization. +However, the developer has responsibility for ensuring that objects are properly cleaned up.

    -Calling a __del__ method more than once during object destruction risks resources being released multiple -times. The relevant __del__ method may not be designed to be called more than once. +Objects with a __del__ method (a finalizer) often hold resources such as file handles that need to be cleaned up. +If a superclass finalizer is called multiple times, this may lead to errors such as closing an already closed file, and lead to objects not being +cleaned up properly as expected.

    There are a number of ways that a __del__ method may be be called more than once.

    • There may be more than one explicit call to the method in the hierarchy of __del__ methods.
      • -
    • A class using multiple inheritance directly calls the __del__ methods of its base types. - One or more of those base types uses super() to pass down the inheritance chain.
      • +
    • In situations involving multiple inheritance, an finalization method may call the finalizers of each of its base types, + which themselves both call the finalizer of a shared base type. (This is an example of the Diamond Inheritance problem)
      • +
    • Another situation involving multiple inheritance arises when a subclass calls the __del__ methods of each of its base classes, + one of which calls super().__del__. This super call resolves to the next class in the Method Resolution Order (MRO) of the subclass, + which may be another base class that already has its initializer explicitly called.
     -

    Either be careful not to explicitly call a __del__ method more than once, or -use super() throughout the inheritance hierarchy.

    - -

    Alternatively refactor one or more of the classes to use composition rather than inheritance.

    +

    Ensure that each finalizer method is called exactly once during finalization. +This can be ensured by calling super().__del__ for each finalizer methid in the inheritance chain. +

     -

    In the first example, explicit calls to __del__ are used, but SportsCar erroneously calls -both Vehicle.__del__ and Car.__del__. -This can be fixed by removing the call to Vehicle.__del__, as shown in FixedSportsCar. -

    - - -

    In the second example, there is a mixture of explicit calls to __del__ and calls using super(). -To fix this example, super() should be used throughout. +

    In the following example, there is a mixture of explicit calls to __del__ and calls using super(), resulting in Vehicle.__del__ +being called twice. +FixedSportsCar.__del__ fixes this by using super() consistently with the other delete methods.

    - -
  • Python Tutorial: Classes.
    • -
  • Python Standard Library: super.
    • -
  • Artima Developer: Things to Know About Python Super.
    • -
  • Wikipedia: Composition over inheritance.
    • - + +
  • Python Reference: __del__.
    • +
  • Python Standard Library: super.
    • +
  • Python Glossary: Method resolution order.
    • +
  • Wikipedia: The Diamond Problem.
    •      diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql index 7772aa153730..dfdc3545e64f 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql @@ -36,12 +36,12 @@ where ( target1 != target2 and msg = - "This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively." + "This finalization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively." or target1 = target2 and // The targets themselves are called multiple times (either is calledMulti, or something earlier in the MRO) // Mentioning them again would be redundant. - msg = "This deletion method calls $@ multiple times, via $@ and $@." + msg = "This finalization method calls $@ multiple times, via $@ and $@." ) select meth, msg, calledMulti, calledMulti.getQualifiedName(), call1, "this call", call2, "this call", target1, target1.getQualifiedName(), target2, target2.getQualifiedName() diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.qhelp b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.qhelp index f1140d68b891..d7060adef8d9 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.qhelp @@ -4,54 +4,70 @@ -

    Python, unlike statically typed languages such as Java, allows complete freedom when calling methods during object initialization. -However, standard object-oriented principles apply to Python classes using deep inheritance hierarchies. -Therefore the developer has responsibility for ensuring that objects are properly initialized when -there are multiple __init__ methods that need to be called. +

    Python, unlike some other object-oriented languages such as Java, allows the developer complete freedom in +when and how superclass initializers are called during object initialization. +However, the developer has responsibility for ensuring that objects are properly initialized.

    -Calling an __init__ method more than once during object initialization risks the object being incorrectly initialized. -It is unlikely that the relevant __init__ method is designed to be called more than once. +Calling an __init__ method more than once during object initialization risks the object being incorrectly +initialized, as the method and the rest of the inheritance chain may not have been written with the expectation +that it could be called multiple times. For example, it may set attributes to a default value in a way that unexpectedly overwrites +values setting those attributes in a subclass.

    There are a number of ways that an __init__ method may be be called more than once.

    • There may be more than one explicit call to the method in the hierarchy of __init__ methods.
      • -
    • A class using multiple inheritance directly calls the __init__ methods of its base types. - One or more of those base types uses super() to pass down the inheritance chain.
      • +
    • In situations involving multiple inheritance, an initialization method may call the initializers of each of its base types, + which themselves both call the initializer of a shared base type. (This is an example of the Diamond Inheritance problem)
      • +
    • Another situation involving multiple inheritance arises when a subclass calls the __init__ methods of each of its base classes, + one of which calls super().__init__. This super call resolves to the next class in the Method Resolution Order (MRO) of the subclass, + which may be another base class that already has its initializer explicitly called.
     -

    Either be careful not to explicitly call an __init__ method more than once, or -use super() throughout the inheritance hierarchy.

    +

    +Take care whenever possible not to call an an initializer multiple times. If each __init__ method in the hierarchy +calls super().__init__(), then each initializer will be called exactly once according to the MRO of the subclass. + +When explicitly calling base class initializers (such as to pass different arguments to different initializers), +ensure this is done consistently throughout, rather than using super() calls in the base classes. +

    +

    +In some cases, it may not be possible to avoid calling a base initializer multiple times without significant refactoring. +In this case, carefully check that the initializer does not interfere with subclass initializers + when called multiple times (such as by overwriting attributes), and ensure this behavior is documented. +

    -

    Alternatively refactor one or more of the classes to use composition rather than inheritance.

     -

    In the first example, explicit calls to __init__ are used, but SportsCar erroneously calls -both Vehicle.__init__ and Car.__init__. -This can be fixed by removing the call to Vehicle.__init__, as shown in FixedSportsCar. +

    In the following (BAD) example, the class D calls B.__init__ and C.__init__, +which each call A.__init__. This results in self.state being set to None as +A.__init__ is called again after B.__init__ had finished. This may lead to unexpected results.

    - + -

    In the second example, there is a mixture of explicit calls to __init__ and calls using super(). -To fix this example, super() should be used throughout. +

    In the following (GOOD) example, a call to super().__init__ is made in each class +in the inheritance hierarchy, ensuring each initializer is called exactly once.

    - + + +

    In the following (BAD) example, explicit base class calls are mixed with super() calls, and C.__init__ is called twice.

    +     -
  • Python Tutorial: Classes.
    • -
  • Python Standard Library: super.
    • -
  • Artima Developer: Things to Know About Python Super.
    • -
  • Wikipedia: Composition over inheritance.
    • +
  • Python Reference: __init__.
    • +
  • Python Standard Library: super.
    • +
  • Python Glossary: Method resolution order.
    • +
  • Wikipedia: The Diamond Problem.
    •  diff --git a/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToDel.py b/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToDel.py index 37520071b3ae..296d36be7d8f 100644 --- a/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToDel.py +++ b/python/ql/src/Classes/CallsToInitDel/examples/MissingCallToDel.py @@ -10,14 +10,14 @@ def __del__(self): recycle(self.car_parts) Vehicle.__del__(self) -#Car.__del__ is missed out. +#BAD: Car.__del__ is not called. class SportsCar(Car, Vehicle): def __del__(self): recycle(self.sports_car_parts) Vehicle.__del__(self) -#Fix SportsCar by calling Car.__del__ +#GOOD: Car.__del__ is called correctly. class FixedSportsCar(Car, Vehicle): def __del__(self): diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes.py index 0ee6e61bcb1a..f48f325f8b57 100644 --- a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes.py +++ b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes.py @@ -1,29 +1,32 @@ -#Calling a method multiple times by using explicit calls when a base inherits from other base + +#Calling a method multiple times by using explicit calls when a base uses super() class Vehicle(object): - + def __del__(self): recycle(self.base_parts) - + super(Vehicle, self).__del__() class Car(Vehicle): def __del__(self): recycle(self.car_parts) - Vehicle.__del__(self) - - + super(Car, self).__del__() + + class SportsCar(Car, Vehicle): - # Vehicle.__del__ will get called twice + # BAD: Vehicle.__del__ will get called twice def __del__(self): recycle(self.sports_car_parts) Car.__del__(self) Vehicle.__del__(self) -#Fix SportsCar by only calling Car.__del__ +# GOOD: super() is used ensuring each del method is called once. class FixedSportsCar(Car, Vehicle): def __del__(self): recycle(self.sports_car_parts) - Car.__del__(self) + super(SportsCar, self).__del__() + + diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes2.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes2.py deleted file mode 100644 index 8cb1433ac0c4..000000000000 --- a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassDelCalledMultipleTimes2.py +++ /dev/null @@ -1,32 +0,0 @@ - -#Calling a method multiple times by using explicit calls when a base uses super() -class Vehicle(object): - - def __del__(self): - recycle(self.base_parts) - super(Vehicle, self).__del__() - -class Car(Vehicle): - - def __del__(self): - recycle(self.car_parts) - super(Car, self).__del__() - - -class SportsCar(Car, Vehicle): - - # Vehicle.__del__ will get called twice - def __del__(self): - recycle(self.sports_car_parts) - Car.__del__(self) - Vehicle.__del__(self) - - -#Fix SportsCar by using super() -class FixedSportsCar(Car, Vehicle): - - def __del__(self): - recycle(self.sports_car_parts) - super(SportsCar, self).__del__() - - diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes.py deleted file mode 100644 index 050d5d389d61..000000000000 --- a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes.py +++ /dev/null @@ -1,36 +0,0 @@ -#Calling a method multiple times by using explicit calls when a base inherits from other base -class Vehicle(object): - - def __init__(self): - self.mobile = True - -class Car(Vehicle): - - def __init__(self): - Vehicle.__init__(self) - self.car_init() - - def car_init(self): - pass - -class SportsCar(Car, Vehicle): - - # Vehicle.__init__ will get called twice - def __init__(self): - Vehicle.__init__(self) - Car.__init__(self) - self.sports_car_init() - - def sports_car_init(self): - pass - -#Fix SportsCar by only calling Car.__init__ -class FixedSportsCar(Car, Vehicle): - - def __init__(self): - Car.__init__(self) - self.sports_car_init() - - def sports_car_init(self): - pass - diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes2.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes2.py deleted file mode 100644 index e12e86a079ee..000000000000 --- a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimes2.py +++ /dev/null @@ -1,38 +0,0 @@ - -#Calling a method multiple times by using explicit calls when a base uses super() -class Vehicle(object): - - def __init__(self): - super(Vehicle, self).__init__() - self.mobile = True - -class Car(Vehicle): - - def __init__(self): - super(Car, self).__init__() - self.car_init() - - def car_init(self): - pass - -class SportsCar(Car, Vehicle): - - # Vehicle.__init__ will get called twice - def __init__(self): - Vehicle.__init__(self) - Car.__init__(self) - self.sports_car_init() - - def sports_car_init(self): - pass - -#Fix SportsCar by using super() -class FixedSportsCar(Car, Vehicle): - - def __init__(self): - super(SportsCar, self).__init__() - self.sports_car_init() - - def sports_car_init(self): - pass - diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad1.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad1.py new file mode 100644 index 000000000000..0f595a534dac --- /dev/null +++ b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad1.py @@ -0,0 +1,20 @@ +class A: + def __init__(self): + self.state = None + +class B(A): + def __init__(self): + A.__init__(self) + self.state = "B" + self.b = 3 + +class C(A): + def __init__(self): + A.__init__(self) + self.c = 2 + +class D(B,C): + def __init__(self): + B.__init__(self) + C.__init__(self) # BAD: This calls A.__init__ a second time, setting self.state to None. + diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad3.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad3.py new file mode 100644 index 000000000000..9a70876e7a85 --- /dev/null +++ b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesBad3.py @@ -0,0 +1,22 @@ +class A: + def __init__(self): + print("A") + self.state = None + +class B(A): + def __init__(self): + print("B") + super().__init__() # When called from D, this calls C.__init__ + self.state = "B" + self.b = 3 + +class C(A): + def __init__(self): + print("C") + super().__init__() + self.c = 2 + +class D(B,C): + def __init__(self): + B.__init__(self) + C.__init__(self) # BAD: C.__init__ is called a second time \ No newline at end of file diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py new file mode 100644 index 000000000000..ab8d98116aaf --- /dev/null +++ b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py @@ -0,0 +1,22 @@ +class A: + def __init__(self): + self.state = None + +class B(A): + def __init__(self): + super().__init__() + self.state = "B" + self.b = 3 + +class C(A): + def __init__(self): + super().__init__() + self.c = 2 + +class D(B,C): + def __init__(self): # GOOD: Each method calls super, so each init method runs once. self.stat =e will be set to "B". + super().__init__() + self.d = 1 + + + diff --git a/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected b/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected index 80e02da0e132..2ec5e1352584 100644 --- a/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected +++ b/python/ql/test/query-tests/Classes/missing-del/MissingCallToDel.expected @@ -1 +1 @@ -| missing_del.py:13:1:13:13 | Class X3 | This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__) | missing_del.py:9:5:9:22 | Function __del__ | X2.__del__ | missing_del.py:15:5:15:22 | Function __del__ | X3.__del__ | +| missing_del.py:13:1:13:13 | Class X3 | This class does not call $@ during finalization. ($@ may be missing a call to a base class __del__) | missing_del.py:9:5:9:22 | Function __del__ | X2.__del__ | missing_del.py:15:5:15:22 | Function __del__ | X3.__del__ | diff --git a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected index 0df0670b2191..b7ee48feba78 100644 --- a/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected +++ b/python/ql/test/query-tests/Classes/multiple/multiple-del/SuperclassDelCalledMultipleTimes.expected @@ -1,2 +1,2 @@ -| multiple_del.py:21:5:21:22 | Function __del__ | This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | multiple_del.py:23:9:23:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:24:9:24:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | multiple_del.py:15:5:15:22 | Function __del__ | Y2.__del__ | -| multiple_del.py:43:5:43:22 | Function __del__ | This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | multiple_del.py:45:9:45:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:46:9:46:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | multiple_del.py:37:5:37:22 | Function __del__ | Z2.__del__ | +| multiple_del.py:21:5:21:22 | Function __del__ | This finalization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | multiple_del.py:23:9:23:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:24:9:24:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:9:5:9:22 | Function __del__ | Y1.__del__ | multiple_del.py:15:5:15:22 | Function __del__ | Y2.__del__ | +| multiple_del.py:43:5:43:22 | Function __del__ | This finalization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively. | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | multiple_del.py:45:9:45:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:46:9:46:24 | ControlFlowNode for Attribute() | this call | multiple_del.py:32:5:32:22 | Function __del__ | Z1.__del__ | multiple_del.py:37:5:37:22 | Function __del__ | Z2.__del__ | From d2c68dedc9f09aaf6a5b1881abd3d2a133e2092e Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 15:53:02 +0100 Subject: [PATCH 18/28] Update integration test output --- .../query-suite/python-code-quality-extended.qls.expected | 8 ++++---- .../query-suite/python-code-quality.qls.expected | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/python/ql/integration-tests/query-suite/python-code-quality-extended.qls.expected b/python/ql/integration-tests/query-suite/python-code-quality-extended.qls.expected index 960972c508c8..efd34ac3db63 100644 --- a/python/ql/integration-tests/query-suite/python-code-quality-extended.qls.expected +++ b/python/ql/integration-tests/query-suite/python-code-quality-extended.qls.expected @@ -1,14 +1,14 @@ +ql/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +ql/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +ql/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +ql/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql ql/python/ql/src/Classes/ConflictingAttributesInBaseClasses.ql ql/python/ql/src/Classes/DefineEqualsWhenAddingAttributes.ql ql/python/ql/src/Classes/EqualsOrHash.ql ql/python/ql/src/Classes/InconsistentMRO.ql ql/python/ql/src/Classes/InitCallsSubclass/InitCallsSubclassMethod.ql -ql/python/ql/src/Classes/MissingCallToDel.ql -ql/python/ql/src/Classes/MissingCallToInit.ql ql/python/ql/src/Classes/MutatingDescriptor.ql ql/python/ql/src/Classes/SubclassShadowing.ql -ql/python/ql/src/Classes/SuperclassDelCalledMultipleTimes.ql -ql/python/ql/src/Classes/SuperclassInitCalledMultipleTimes.ql ql/python/ql/src/Classes/WrongNameForArgumentInClassInstantiation.ql ql/python/ql/src/Classes/WrongNumberArgumentsInClassInstantiation.ql ql/python/ql/src/Exceptions/CatchingBaseException.ql diff --git a/python/ql/integration-tests/query-suite/python-code-quality.qls.expected b/python/ql/integration-tests/query-suite/python-code-quality.qls.expected index 960972c508c8..efd34ac3db63 100644 --- a/python/ql/integration-tests/query-suite/python-code-quality.qls.expected +++ b/python/ql/integration-tests/query-suite/python-code-quality.qls.expected @@ -1,14 +1,14 @@ +ql/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +ql/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +ql/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +ql/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql ql/python/ql/src/Classes/ConflictingAttributesInBaseClasses.ql ql/python/ql/src/Classes/DefineEqualsWhenAddingAttributes.ql ql/python/ql/src/Classes/EqualsOrHash.ql ql/python/ql/src/Classes/InconsistentMRO.ql ql/python/ql/src/Classes/InitCallsSubclass/InitCallsSubclassMethod.ql -ql/python/ql/src/Classes/MissingCallToDel.ql -ql/python/ql/src/Classes/MissingCallToInit.ql ql/python/ql/src/Classes/MutatingDescriptor.ql ql/python/ql/src/Classes/SubclassShadowing.ql -ql/python/ql/src/Classes/SuperclassDelCalledMultipleTimes.ql -ql/python/ql/src/Classes/SuperclassInitCalledMultipleTimes.ql ql/python/ql/src/Classes/WrongNameForArgumentInClassInstantiation.ql ql/python/ql/src/Classes/WrongNumberArgumentsInClassInstantiation.ql ql/python/ql/src/Exceptions/CatchingBaseException.ql From f1026e436a14f10d8c48fdb28b0760f0a3de4601 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 16:03:50 +0100 Subject: [PATCH 19/28] Add change note --- .../2025-06-04-missing-multiple-calls-to-init-del.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md diff --git a/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md b/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md new file mode 100644 index 000000000000..fe9a0cc110df --- /dev/null +++ b/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md @@ -0,0 +1,4 @@ +--- +minorAnalysis +--- +The queries `py/missing-call-to-init`, `py/missing-calls-to-del`, `py/multiple-calls-to-init`, and `py/multiple-calls-to-del` queries have been modernized; no longer relying on outdated libraries, and producing more precise results with more descriptive alert messages, and improved documentation. \ No newline at end of file From c47e6e3a0f3c249dab0a547a52c96c0adf07f93e Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 16:26:07 +0100 Subject: [PATCH 20/28] Add qldoc --- .../CallsToInitDel/MethodCallOrder.qll | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 02d60ca420e0..321da002fdc8 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -5,6 +5,7 @@ import semmle.python.ApiGraphs import semmle.python.dataflow.new.internal.DataFlowDispatch import codeql.util.Option +/** Holds if `meth` is a method named `name` that transitively calls `calledMulti` of the same name via the calls `call1` and `call2`. */ predicate multipleCallsToSuperclassMethod( Function meth, Function calledMulti, DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, string name @@ -19,6 +20,7 @@ predicate multipleCallsToSuperclassMethod( ) } +/** Gets a method transitively called by `meth` named `name` with `call` that it overrides, with `mroBase` as the type determining the MRO to search. */ Function getASuperCallTargetFromCall( Class mroBase, Function meth, DataFlow::MethodCallNode call, string name ) { @@ -29,6 +31,7 @@ Function getASuperCallTargetFromCall( ) } +/** Gets the method called by `meth` named `name` with `call`, with `mroBase` as the type determining the MRO to search. */ Function getDirectSuperCallTargetFromCall( Class mroBase, Function meth, DataFlow::MethodCallNode call, string name ) { @@ -51,6 +54,7 @@ Function getDirectSuperCallTargetFromCall( ) } +/** Gets a method that is transitively called by a call to `cls.`, with `mroBase` as the type determining the MRO to search. */ Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) { exists(Function target | target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and @@ -62,6 +66,7 @@ Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) { ) } +/** Holds if `meth` does something besides calling a superclass method. */ predicate nonTrivial(Function meth) { exists(Stmt s | s = meth.getAStmt() | not s instanceof Pass and @@ -74,6 +79,7 @@ predicate nonTrivial(Function meth) { exists(meth.getANormalExit()) // doesn't always raise an exception } +/** Holds if `call` is a call to `super().`. No distinction is made btween 0- and 2- arg super calls. */ predicate superCall(DataFlow::MethodCallNode call, string name) { exists(DataFlow::Node sup | call.calls(sup, name) and @@ -81,6 +87,7 @@ predicate superCall(DataFlow::MethodCallNode call, string name) { ) } +/** Holds if `meth` calls `super().` where `name` is the name of the method. */ predicate callsSuper(Function meth) { exists(DataFlow::MethodCallNode call | call.getScope() = meth and @@ -88,6 +95,7 @@ predicate callsSuper(Function meth) { ) } +/** Holds if `meth` calls `target.(self, ...)` with the call `call`. */ predicate callsMethodOnClassWithSelf( Function meth, DataFlow::MethodCallNode call, Class target, string name ) { @@ -99,6 +107,7 @@ predicate callsMethodOnClassWithSelf( ) } +/** Holds if `meth` calls a method named `name` passing its `self` argument as its first parameter, but the class it refers to is unknown. */ predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) { exists(DataFlow::MethodCallNode call, DataFlow::Node callTarget, DataFlow::ParameterNode self | call.calls(callTarget, name) and @@ -108,6 +117,7 @@ predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) { ) } +/** Holds if `base` does not call a superclass method `shouldCall` named `name` when it appears it should. */ predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string name) { shouldCall.getName() = name and shouldCall.getScope() = getADirectSuperclass+(base) and @@ -117,6 +127,9 @@ predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string not callsMethodOnUnknownClassWithSelf(getASuperCallTargetFromClass(base, base, name), name) } +/** Holds if `base` does not call a superclass method `shouldCall` named `name` when it appears it should. + * Results are restricted to hold only for the highest `base` class and the lowest `shouldCall` method in the heirarchy for which this applies. + */ predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCall, string name) { missingCallToSuperclassMethod(base, shouldCall, name) and not exists(Class superBase | @@ -131,6 +144,11 @@ predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCal ) } +/** + * If `base` contains a `super()` call, gets a method in the inheritence heirarchy of `name` in the MRO of `base` + * that does not contain a `super()` call, but would call `shouldCall` if it did, which does not otherwise get called + * during a call to `base.`. + * */ Function getPossibleMissingSuper(Class base, Function shouldCall, string name) { missingCallToSuperclassMethod(base, shouldCall, name) and exists(Function baseMethod | @@ -151,6 +169,7 @@ Function getPossibleMissingSuper(Class base, Function shouldCall, string name) { private module FunctionOption = Option; +/** An optional `Function`. */ class FunctionOption extends FunctionOption::Option { /** * Holds if this element is at the specified location. @@ -174,6 +193,7 @@ class FunctionOption extends FunctionOption::Option { endcolumn = 0 } + /** Gets the qualified name of this function. */ string getQualifiedName() { result = this.asSome().getQualifiedName() or @@ -182,6 +202,7 @@ class FunctionOption extends FunctionOption::Option { } } +/** Gets the result of `getPossibleMissingSuper`, or None if none exists. */ bindingset[name] FunctionOption getPossibleMissingSuperOption(Class base, Function shouldCall, string name) { result.asSome() = getPossibleMissingSuper(base, shouldCall, name) From 4b49ac3c3ef794a7211e0d601cb3f979a84c35ea Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 16:32:49 +0100 Subject: [PATCH 21/28] Fix changenote formatting --- .../2025-06-04-missing-multiple-calls-to-init-del.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md b/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md index fe9a0cc110df..5dfe5c2b8413 100644 --- a/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md +++ b/python/ql/src/change-notes/2025-06-04-missing-multiple-calls-to-init-del.md @@ -1,4 +1,4 @@ --- -minorAnalysis +category: minorAnalysis --- -The queries `py/missing-call-to-init`, `py/missing-calls-to-del`, `py/multiple-calls-to-init`, and `py/multiple-calls-to-del` queries have been modernized; no longer relying on outdated libraries, and producing more precise results with more descriptive alert messages, and improved documentation. \ No newline at end of file +* The queries `py/missing-call-to-init`, `py/missing-calls-to-del`, `py/multiple-calls-to-init`, and `py/multiple-calls-to-del` queries have been modernized; no longer relying on outdated libraries, producing more precise results with more descriptive alert messages, and improved documentation. \ No newline at end of file From d163bdf981b6956c7953d9bb66ef5be47cc67ffd Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 4 Jul 2025 16:35:52 +0100 Subject: [PATCH 22/28] Fix typos --- .../CallsToInitDel/MethodCallOrder.qll | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 321da002fdc8..6506cb1a1933 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -31,7 +31,7 @@ Function getASuperCallTargetFromCall( ) } -/** Gets the method called by `meth` named `name` with `call`, with `mroBase` as the type determining the MRO to search. */ +/** Gets the method called by `meth` named `name` with `call`, with `mroBase` as the type determining the MRO to search. */ Function getDirectSuperCallTargetFromCall( Class mroBase, Function meth, DataFlow::MethodCallNode call, string name ) { @@ -54,7 +54,7 @@ Function getDirectSuperCallTargetFromCall( ) } -/** Gets a method that is transitively called by a call to `cls.`, with `mroBase` as the type determining the MRO to search. */ +/** Gets a method that is transitively called by a call to `cls.`, with `mroBase` as the type determining the MRO to search. */ Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) { exists(Function target | target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and @@ -79,7 +79,7 @@ predicate nonTrivial(Function meth) { exists(meth.getANormalExit()) // doesn't always raise an exception } -/** Holds if `call` is a call to `super().`. No distinction is made btween 0- and 2- arg super calls. */ +/** Holds if `call` is a call to `super().`. No distinction is made between 0- and 2- arg super calls. */ predicate superCall(DataFlow::MethodCallNode call, string name) { exists(DataFlow::Node sup | call.calls(sup, name) and @@ -127,8 +127,9 @@ predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string not callsMethodOnUnknownClassWithSelf(getASuperCallTargetFromClass(base, base, name), name) } -/** Holds if `base` does not call a superclass method `shouldCall` named `name` when it appears it should. - * Results are restricted to hold only for the highest `base` class and the lowest `shouldCall` method in the heirarchy for which this applies. +/** + * Holds if `base` does not call a superclass method `shouldCall` named `name` when it appears it should. + * Results are restricted to hold only for the highest `base` class and the lowest `shouldCall` method in the hierarchy for which this applies. */ predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCall, string name) { missingCallToSuperclassMethod(base, shouldCall, name) and @@ -144,11 +145,11 @@ predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCal ) } -/** - * If `base` contains a `super()` call, gets a method in the inheritence heirarchy of `name` in the MRO of `base` +/** + * If `base` contains a `super()` call, gets a method in the inheritance hierarchy of `name` in the MRO of `base` * that does not contain a `super()` call, but would call `shouldCall` if it did, which does not otherwise get called - * during a call to `base.`. - * */ + * during a call to `base.`. + */ Function getPossibleMissingSuper(Class base, Function shouldCall, string name) { missingCallToSuperclassMethod(base, shouldCall, name) and exists(Function baseMethod | From e8a65b8555738570c3205ad6585117e4ad99c169 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 7 Jul 2025 09:49:23 +0100 Subject: [PATCH 23/28] Update integration test outout and fix qhelp --- .../query-suite/python-security-and-quality.qls.expected | 8 ++++---- .../ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp | 2 +- .../CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/python/ql/integration-tests/query-suite/python-security-and-quality.qls.expected b/python/ql/integration-tests/query-suite/python-security-and-quality.qls.expected index 170d9f442f92..564304a7b539 100644 --- a/python/ql/integration-tests/query-suite/python-security-and-quality.qls.expected +++ b/python/ql/integration-tests/query-suite/python-security-and-quality.qls.expected @@ -1,3 +1,7 @@ +ql/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql +ql/python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql +ql/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql +ql/python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql ql/python/ql/src/Classes/ConflictingAttributesInBaseClasses.ql ql/python/ql/src/Classes/DefineEqualsWhenAddingAttributes.ql ql/python/ql/src/Classes/EqualsOrHash.ql @@ -5,16 +9,12 @@ ql/python/ql/src/Classes/EqualsOrNotEquals.ql ql/python/ql/src/Classes/IncompleteOrdering.ql ql/python/ql/src/Classes/InconsistentMRO.ql ql/python/ql/src/Classes/InitCallsSubclass/InitCallsSubclassMethod.ql -ql/python/ql/src/Classes/MissingCallToDel.ql -ql/python/ql/src/Classes/MissingCallToInit.ql ql/python/ql/src/Classes/MutatingDescriptor.ql ql/python/ql/src/Classes/OverwritingAttributeInSuperClass.ql ql/python/ql/src/Classes/PropertyInOldStyleClass.ql ql/python/ql/src/Classes/SlotsInOldStyleClass.ql ql/python/ql/src/Classes/SubclassShadowing.ql ql/python/ql/src/Classes/SuperInOldStyleClass.ql -ql/python/ql/src/Classes/SuperclassDelCalledMultipleTimes.ql -ql/python/ql/src/Classes/SuperclassInitCalledMultipleTimes.ql ql/python/ql/src/Classes/WrongNameForArgumentInClassInstantiation.ql ql/python/ql/src/Classes/WrongNumberArgumentsInClassInstantiation.ql ql/python/ql/src/Diagnostics/ExtractedFiles.ql diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp index a9897d3c682e..e461aebdc028 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp @@ -13,7 +13,7 @@ methods are called.

    Classes with a __del__ method (a finalizer) typically hold some resource such as a file handle that needs to be cleaned up. If the __del__ method of a superclass is not called during object finalization, it is likely that -that resources may be leaked. +resources may be leaked.

    A call to the __init__ method of a superclass during object initialization may be unintentionally skipped: diff --git a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp index f828cfb8e648..df9c073fcceb 100644 --- a/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.qhelp @@ -30,7 +30,7 @@ cleaned up properly as expected.

    Ensure that each finalizer method is called exactly once during finalization. -This can be ensured by calling super().__del__ for each finalizer methid in the inheritance chain. +This can be ensured by calling super().__del__ for each finalizer method in the inheritance chain.

    @@ -41,7 +41,7 @@ being called twice. FixedSportsCar.__del__ fixes this by using super() consistently with the other delete methods.

    - + From f5066c7050b4d970fd6eab49e5f88e3cc546768f Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 7 Jul 2025 11:02:48 +0100 Subject: [PATCH 24/28] Remove tostring --- .../ql/src/Classes/CallsToInitDel/MethodCallOrder.qll | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 6506cb1a1933..6bb5a5862535 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -13,13 +13,21 @@ predicate multipleCallsToSuperclassMethod( exists(Class cls | meth.getName() = name and meth.getScope() = cls and - call1.getLocation().toString() < call2.getLocation().toString() and + locationBefore(call1.getLocation(), call2.getLocation()) and calledMulti = getASuperCallTargetFromCall(cls, meth, call1, name) and calledMulti = getASuperCallTargetFromCall(cls, meth, call2, name) and nonTrivial(calledMulti) ) } +/** Holds if l1 comes before l2, assuming they're in the same file. */ +private predicate locationBefore(Location l1, Location l2) { + l1.getStartLine() < l2.getStartLine() + or + l1.getStartLine() = l2.getStartLine() and + l1.getStartColumn() < l2.getStartColumn() +} + /** Gets a method transitively called by `meth` named `name` with `call` that it overrides, with `mroBase` as the type determining the MRO to search. */ Function getASuperCallTargetFromCall( Class mroBase, Function meth, DataFlow::MethodCallNode call, string name From 2c93e2c3697f962b63d8b60bee483e6e841b06b4 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 14 Jul 2025 13:18:59 +0100 Subject: [PATCH 25/28] Inline locationBefore --- python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 6bb5a5862535..98a1d70915b2 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -21,6 +21,7 @@ predicate multipleCallsToSuperclassMethod( } /** Holds if l1 comes before l2, assuming they're in the same file. */ +pragma[inline] private predicate locationBefore(Location l1, Location l2) { l1.getStartLine() < l2.getStartLine() or From 7dad89f44b75560fbc6c0c4faa6359f7ef7d0467 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 14 Jul 2025 13:30:56 +0100 Subject: [PATCH 26/28] Adress review suggestions - cleanups --- python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll index 98a1d70915b2..bd98f3fb0fb7 100644 --- a/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll +++ b/python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll @@ -45,7 +45,6 @@ Function getDirectSuperCallTargetFromCall( Class mroBase, Function meth, DataFlow::MethodCallNode call, string name ) { meth = call.getScope() and - getADirectSuperclass*(mroBase) = meth.getScope() and meth.getName() = name and call.calls(_, name) and mroBase = getADirectSubclass*(meth.getScope()) and @@ -56,7 +55,7 @@ Function getDirectSuperCallTargetFromCall( result = findFunctionAccordingToMroKnownStartingClass(targetCls, mroBase, name) or // targetCls is the mro base for this lookup. - // note however that if the call we find uses super(), that still uses the mro of the instance `self` will sill be used + // note however that if the call we find uses super(), that still uses the mro of the instance `self` // assuming it's 0-arg or is 2-arg with `self` as second arg. callsMethodOnClassWithSelf(meth, call, targetCls, _) and result = findFunctionAccordingToMroKnownStartingClass(targetCls, targetCls, name) @@ -96,7 +95,7 @@ predicate superCall(DataFlow::MethodCallNode call, string name) { ) } -/** Holds if `meth` calls `super().` where `name` is the name of the method. */ +/** Holds if `meth` calls a `super()` method of the same name. */ predicate callsSuper(Function meth) { exists(DataFlow::MethodCallNode call | call.getScope() = meth and @@ -122,7 +121,7 @@ predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) { 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)) + not callTarget = classTracker(any(Class target)) ) } From d2a8e5d65174e36dd6f2c4a4d50305cf3083cdb1 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 18 Jul 2025 14:18:03 +0100 Subject: [PATCH 27/28] Fix typo in example --- .../examples/SuperclassInitCalledMultipleTimesGood2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py index ab8d98116aaf..9e4bbdea0584 100644 --- a/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py +++ b/python/ql/src/Classes/CallsToInitDel/examples/SuperclassInitCalledMultipleTimesGood2.py @@ -14,7 +14,7 @@ def __init__(self): self.c = 2 class D(B,C): - def __init__(self): # GOOD: Each method calls super, so each init method runs once. self.stat =e will be set to "B". + def __init__(self): # GOOD: Each method calls super, so each init method runs once. self.state will be set to "B". super().__init__() self.d = 1 From b33a1c2a099e95a18ec0329de8a424465dae1105 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 21 Jul 2025 15:54:36 +0100 Subject: [PATCH 28/28] Fix doc typo --- python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp index e461aebdc028..83aac7629eba 100644 --- a/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp +++ b/python/ql/src/Classes/CallsToInitDel/MissingCallToDel.qhelp @@ -16,7 +16,7 @@ If the __del__ method of a superclass is not called during object resources may be leaked.

    -

    A call to the __init__ method of a superclass during object initialization may be unintentionally skipped: +

    A call to the __del__ method of a superclass during object initialization may be unintentionally skipped:

    • If a subclass calls the __del__ method of the wrong class.