From eb35fa0d5e0d290d378eae4a2e8ffa174ca8686d Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 25 Mar 2026 08:43:49 +0000 Subject: [PATCH 01/13] C++: Unify 'isSourceParameterOf' for this parameters with the implementation for positional parameters. --- .../ir/dataflow/internal/DataFlowNodes.qll | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll index afec2384b23..1d4bb3f8ab5 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll @@ -850,11 +850,6 @@ module Public { { ThisParameterInstructionNode() { instr.getIRVariable() instanceof IRThisVariable } - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { - pos.(DirectPosition).getArgumentIndex() = -1 and - instr.getEnclosingFunction() = f - } - override string toStringImpl() { result = "this" } } @@ -1659,6 +1654,11 @@ abstract private class AbstractParameterNode extends Node { /** Gets the `Parameter` associated with this node, if it exists. */ Parameter getParameter() { none() } // overridden by subclasses + + /** + * Holds if this node represents an implicit `this` parameter, if it exists. + */ + predicate isThis() { none() } // overridden by subclasses } abstract private class AbstractIndirectParameterNode extends AbstractParameterNode { @@ -1701,9 +1701,10 @@ private class IndirectInstructionParameterNode extends AbstractIndirectParameter ) } - /** Gets the parameter whose indirection is initialized. */ override Parameter getParameter() { result = init.getParameter() } + override predicate isThis() { init.hasIndex(-1) } + override DataFlowCallable getEnclosingCallable() { result.asSourceCallable() = this.getFunction() } @@ -1738,6 +1739,18 @@ abstract class InstructionDirectParameterNode extends InstructionNode, AbstractD * Gets the `IRVariable` that this parameter references. */ final IRVariable getIRVariable() { result = instr.getIRVariable() } + + override predicate isThis() { instr.hasIndex(-1) } + + override Parameter getParameter() { result = instr.getParameter() } + + override predicate isSourceParameterOf(Function f, ParameterPosition pos) { + this.getFunction() = f and + exists(int argumentIndex | + pos.(DirectPosition).getArgumentIndex() = argumentIndex and + instr.hasIndex(argumentIndex) + ) + } } abstract private class AbstractExplicitParameterNode extends AbstractDirectParameterNode { } @@ -1748,13 +1761,7 @@ private class ExplicitParameterInstructionNode extends AbstractExplicitParameter { ExplicitParameterInstructionNode() { exists(instr.getParameter()) } - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { - f.getParameter(pos.(DirectPosition).getArgumentIndex()) = instr.getParameter() - } - override string toStringImpl() { result = instr.getParameter().toString() } - - override Parameter getParameter() { result = instr.getParameter() } } /** From 9cb8edb41a2133467d105969b4ba802cb3a811c0 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 25 Mar 2026 08:45:18 +0000 Subject: [PATCH 02/13] C++: Change 'Function' to 'Declaration' in a few places to handle enclosing callables being fields. --- .../cpp/ir/dataflow/internal/DataFlowNodes.qll | 16 ++++++++-------- .../cpp/ir/dataflow/internal/DataFlowPrivate.qll | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll index 1d4bb3f8ab5..637d8f73eb9 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll @@ -1124,7 +1124,7 @@ class IndirectArgumentOutNode extends PostUpdateNodeImpl { /** * Gets the `Function` that the call targets, if this is statically known. */ - Function getStaticCallTarget() { result = this.getCallInstruction().getStaticCallTarget() } + Declaration getStaticCallTarget() { result = this.getCallInstruction().getStaticCallTarget() } override string toStringImpl() { exists(string prefix | if indirectionIndex > 0 then prefix = "" else prefix = "pointer to " | @@ -1628,7 +1628,7 @@ abstract private class AbstractParameterNode extends Node { * implicit `this` parameter is considered to have position `-1`, and * pointer-indirection parameters are at further negative positions. */ - predicate isSourceParameterOf(Function f, ParameterPosition pos) { none() } + predicate isSourceParameterOf(Declaration f, ParameterPosition pos) { none() } /** * Holds if this node is the parameter of `sc` at the specified position. The @@ -1711,7 +1711,7 @@ private class IndirectInstructionParameterNode extends AbstractIndirectParameter override Declaration getFunction() { result = init.getEnclosingFunction() } - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { + override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) { this.getFunction() = f and exists(int argumentIndex, int indirectionIndex | indirectPositionHasArgumentIndexAndIndex(pos, argumentIndex, indirectionIndex) and @@ -1744,7 +1744,7 @@ abstract class InstructionDirectParameterNode extends InstructionNode, AbstractD override Parameter getParameter() { result = instr.getParameter() } - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { + override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) { this.getFunction() = f and exists(int argumentIndex | pos.(DirectPosition).getArgumentIndex() = argumentIndex and @@ -1789,9 +1789,9 @@ private class DirectBodyLessParameterNode extends AbstractExplicitParameterNode, { DirectBodyLessParameterNode() { indirectionIndex = 0 } - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { + override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) { this.getFunction() = f and - f.getParameter(pos.(DirectPosition).getArgumentIndex()) = p + f.(Function).getParameter(pos.(DirectPosition).getArgumentIndex()) = p } override Parameter getParameter() { result = p } @@ -1802,10 +1802,10 @@ private class IndirectBodyLessParameterNode extends AbstractIndirectParameterNod { IndirectBodyLessParameterNode() { not this instanceof DirectBodyLessParameterNode } - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { + override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) { exists(int argumentPosition | this.getFunction() = f and - f.getParameter(argumentPosition) = p and + f.(Function).getParameter(argumentPosition) = p and indirectPositionHasArgumentIndexAndIndex(pos, argumentPosition, indirectionIndex) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 6dd953b16ab..83f240ddae5 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -1170,7 +1170,7 @@ class DataFlowCall extends TDataFlowCall { /** * Gets the `Function` that the call targets, if this is statically known. */ - Function getStaticCallSourceTarget() { none() } + Declaration getStaticCallSourceTarget() { none() } /** * Gets the target of this call. We use the following strategy for deciding @@ -1182,7 +1182,7 @@ class DataFlowCall extends TDataFlowCall { * whether is it manual or generated. */ final DataFlowCallable getStaticCallTarget() { - exists(Function target | target = this.getStaticCallSourceTarget() | + exists(Declaration target | target = this.getStaticCallSourceTarget() | // Don't use the source callable if there is a manual model for the // target not exists(SummarizedCallable sc | @@ -1242,7 +1242,7 @@ private class NormalCall extends DataFlowCall, TNormalCall { override CallTargetOperand getCallTargetOperand() { result = call.getCallTargetOperand() } - override Function getStaticCallSourceTarget() { result = call.getStaticCallTarget() } + override Declaration getStaticCallSourceTarget() { result = call.getStaticCallTarget() } override ArgumentOperand getArgumentOperand(int index) { result = call.getArgumentOperand(index) } From 599b7a66534fcb147f300f001f00e45961cdd5e1 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 25 Mar 2026 08:46:57 +0000 Subject: [PATCH 03/13] C++: Handle fields in 'getThisType'. --- .../code/cpp/ir/dataflow/internal/SsaImplCommon.qll | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll index 4d109c0716d..e4734f285fa 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll @@ -11,13 +11,18 @@ private import TypeFlow private import semmle.code.cpp.ir.ValueNumbering /** - * Gets the C++ type of `this` in the member function `f`. + * Gets the C++ type of `this` in an `IRFunction` generated from `f`. * The result is a glvalue if `isGLValue` is true, and * a prvalue if `isGLValue` is false. */ bindingset[isGLValue] -private CppType getThisType(Cpp::MemberFunction f, boolean isGLValue) { - result.hasType(f.getTypeOfThis(), isGLValue) +private CppType getThisType(Cpp::Declaration f, boolean isGLValue) { + result.hasType(f.(Cpp::MemberFunction).getTypeOfThis(), isGLValue) + or + exists(Cpp::PointerType pt | + pt.getBaseType() = f.(Cpp::Field).getDeclaringType() and + result.hasType(pt, isGLValue) + ) } /** From 503c15334a6545eaa3167ee8d35a80fd65d155df Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 25 Mar 2026 08:47:45 +0000 Subject: [PATCH 04/13] C++: Accept test changes. --- .../dataflow-tests/test-source-sink.expected | 1 + .../dataflow/dataflow-tests/test.cpp | 2 +- .../dataflow-tests/type-bugs.expected | 6 ----- .../test/library-tests/dataflow/fields/C.cpp | 2 +- .../dataflow/fields/ir-path-flow.expected | 24 +++++++++++++++++++ 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected index 420bd110e1f..5ee2ca86cbc 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected @@ -391,6 +391,7 @@ irFlow | test.cpp:1308:7:1308:12 | call to source | test.cpp:1309:8:1309:16 | ... ++ | | test.cpp:1312:7:1312:12 | call to source | test.cpp:1313:8:1313:24 | ... ? ... : ... | | test.cpp:1312:7:1312:12 | call to source | test.cpp:1314:8:1314:8 | x | +| test.cpp:1318:13:1318:18 | call to source | test.cpp:1327:10:1327:10 | i | | test.cpp:1329:11:1329:16 | call to source | test.cpp:1330:10:1330:10 | i | | true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x | | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index ca240eb7b2b..892d49b0085 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1324,7 +1324,7 @@ struct nsdmi { void nsdmi_test() { nsdmi x; - sink(x.i); // $ MISSING: ir ast + sink(x.i); // $ ir MISSING: ast nsdmi y(source()); sink(y.i); // $ ir ast diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected index b5f4db887b6..87ebdc9e83a 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected @@ -36,12 +36,6 @@ irTypeBugs | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] read: Argument[this].Element[*] in operator-> | | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] to write: ReturnValue[**] in operator-> | | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] to write: ReturnValue[*] in operator-> | -| test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | field | -| test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | field | -| test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | field | -| test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | i | -| test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | i | -| test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | i | incorrectBaseType | clang.cpp:22:8:22:20 | *& ... | Expected 'Node.getType()' to be int, but it was int * | | clang.cpp:23:17:23:29 | *& ... | Expected 'Node.getType()' to be int, but it was int * | diff --git a/cpp/ql/test/library-tests/dataflow/fields/C.cpp b/cpp/ql/test/library-tests/dataflow/fields/C.cpp index 6e5165caa9a..0c092928272 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/C.cpp +++ b/cpp/ql/test/library-tests/dataflow/fields/C.cpp @@ -27,7 +27,7 @@ public: void func() { sink(s1); // $ ast,ir - sink(s2); // $ MISSING: ast,ir + sink(s2); // $ ir MISSING: ast sink(s3); // $ ast,ir sink(s4); // $ MISSING: ast,ir } diff --git a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected index cc8cd2826bf..2e38382150f 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected +++ b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected @@ -187,23 +187,34 @@ edges | B.cpp:46:7:46:10 | *this [post update] [*box1, elem2] | B.cpp:44:5:44:8 | *this [Return] [*box1, elem2] | provenance | | | B.cpp:46:7:46:21 | *... = ... [elem1] | B.cpp:46:7:46:10 | *this [post update] [*box1, elem1] | provenance | | | B.cpp:46:7:46:21 | *... = ... [elem2] | B.cpp:46:7:46:10 | *this [post update] [*box1, elem2] | provenance | | +| C.cpp:10:15:10:16 | *s2 [post update] [s2] | C.cpp:10:15:10:16 | *this [Return] [s2] | provenance | | +| C.cpp:10:15:10:16 | *this [Return] [s2] | C.cpp:22:3:22:3 | s2 output argument [s2] | provenance | | +| C.cpp:10:20:10:29 | new | C.cpp:10:15:10:16 | *s2 [post update] [s2] | provenance | | +| C.cpp:10:20:10:29 | new | C.cpp:10:20:10:29 | new | provenance | | | C.cpp:18:12:18:18 | *new [s1] | C.cpp:19:5:19:5 | *c [s1] | provenance | | +| C.cpp:18:12:18:18 | *new [s2] | C.cpp:19:5:19:5 | *c [s2] | provenance | | | C.cpp:18:12:18:18 | *new [s3] | C.cpp:19:5:19:5 | *c [s3] | provenance | | | C.cpp:18:12:18:18 | call to C [s1] | C.cpp:18:12:18:18 | *new [s1] | provenance | | +| C.cpp:18:12:18:18 | call to C [s2] | C.cpp:18:12:18:18 | *new [s2] | provenance | | | C.cpp:18:12:18:18 | call to C [s3] | C.cpp:18:12:18:18 | *new [s3] | provenance | | | C.cpp:19:5:19:5 | *c [s1] | C.cpp:27:8:27:11 | *this [s1] | provenance | | +| C.cpp:19:5:19:5 | *c [s2] | C.cpp:27:8:27:11 | *this [s2] | provenance | | | C.cpp:19:5:19:5 | *c [s3] | C.cpp:27:8:27:11 | *this [s3] | provenance | | | C.cpp:22:3:22:3 | *C [post update] [s1] | C.cpp:22:3:22:3 | *this [Return] [s1] | provenance | | | C.cpp:22:3:22:3 | *this [Return] [s1] | C.cpp:18:12:18:18 | call to C [s1] | provenance | | +| C.cpp:22:3:22:3 | *this [Return] [s2] | C.cpp:18:12:18:18 | call to C [s2] | provenance | | | C.cpp:22:3:22:3 | *this [Return] [s3] | C.cpp:18:12:18:18 | call to C [s3] | provenance | | +| C.cpp:22:3:22:3 | s2 output argument [s2] | C.cpp:22:3:22:3 | *this [Return] [s2] | provenance | | | C.cpp:22:12:22:21 | new | C.cpp:22:3:22:3 | *C [post update] [s1] | provenance | | | C.cpp:22:12:22:21 | new | C.cpp:22:12:22:21 | new | provenance | | | C.cpp:24:5:24:8 | *this [post update] [s3] | C.cpp:22:3:22:3 | *this [Return] [s3] | provenance | | | C.cpp:24:5:24:25 | ... = ... | C.cpp:24:5:24:8 | *this [post update] [s3] | provenance | | | C.cpp:24:16:24:25 | new | C.cpp:24:5:24:25 | ... = ... | provenance | | | C.cpp:27:8:27:11 | *this [s1] | C.cpp:29:10:29:11 | *this [s1] | provenance | | +| C.cpp:27:8:27:11 | *this [s2] | C.cpp:30:10:30:11 | *this [s2] | provenance | | | C.cpp:27:8:27:11 | *this [s3] | C.cpp:31:10:31:11 | *this [s3] | provenance | | | C.cpp:29:10:29:11 | *this [s1] | C.cpp:29:10:29:11 | s1 | provenance | | +| C.cpp:30:10:30:11 | *this [s2] | C.cpp:30:10:30:11 | s2 | provenance | | | C.cpp:31:10:31:11 | *this [s3] | C.cpp:31:10:31:11 | s3 | provenance | | | D.cpp:10:11:10:17 | *this [elem] | D.cpp:10:30:10:33 | *this [elem] | provenance | | | D.cpp:10:30:10:33 | *this [elem] | D.cpp:10:30:10:33 | elem | provenance | | @@ -1116,24 +1127,36 @@ nodes | B.cpp:46:7:46:10 | *this [post update] [*box1, elem2] | semmle.label | *this [post update] [*box1, elem2] | | B.cpp:46:7:46:21 | *... = ... [elem1] | semmle.label | *... = ... [elem1] | | B.cpp:46:7:46:21 | *... = ... [elem2] | semmle.label | *... = ... [elem2] | +| C.cpp:10:15:10:16 | *s2 [post update] [s2] | semmle.label | *s2 [post update] [s2] | +| C.cpp:10:15:10:16 | *this [Return] [s2] | semmle.label | *this [Return] [s2] | +| C.cpp:10:20:10:29 | new | semmle.label | new | +| C.cpp:10:20:10:29 | new | semmle.label | new | | C.cpp:18:12:18:18 | *new [s1] | semmle.label | *new [s1] | +| C.cpp:18:12:18:18 | *new [s2] | semmle.label | *new [s2] | | C.cpp:18:12:18:18 | *new [s3] | semmle.label | *new [s3] | | C.cpp:18:12:18:18 | call to C [s1] | semmle.label | call to C [s1] | +| C.cpp:18:12:18:18 | call to C [s2] | semmle.label | call to C [s2] | | C.cpp:18:12:18:18 | call to C [s3] | semmle.label | call to C [s3] | | C.cpp:19:5:19:5 | *c [s1] | semmle.label | *c [s1] | +| C.cpp:19:5:19:5 | *c [s2] | semmle.label | *c [s2] | | C.cpp:19:5:19:5 | *c [s3] | semmle.label | *c [s3] | | C.cpp:22:3:22:3 | *C [post update] [s1] | semmle.label | *C [post update] [s1] | | C.cpp:22:3:22:3 | *this [Return] [s1] | semmle.label | *this [Return] [s1] | +| C.cpp:22:3:22:3 | *this [Return] [s2] | semmle.label | *this [Return] [s2] | | C.cpp:22:3:22:3 | *this [Return] [s3] | semmle.label | *this [Return] [s3] | +| C.cpp:22:3:22:3 | s2 output argument [s2] | semmle.label | s2 output argument [s2] | | C.cpp:22:12:22:21 | new | semmle.label | new | | C.cpp:22:12:22:21 | new | semmle.label | new | | C.cpp:24:5:24:8 | *this [post update] [s3] | semmle.label | *this [post update] [s3] | | C.cpp:24:5:24:25 | ... = ... | semmle.label | ... = ... | | C.cpp:24:16:24:25 | new | semmle.label | new | | C.cpp:27:8:27:11 | *this [s1] | semmle.label | *this [s1] | +| C.cpp:27:8:27:11 | *this [s2] | semmle.label | *this [s2] | | C.cpp:27:8:27:11 | *this [s3] | semmle.label | *this [s3] | | C.cpp:29:10:29:11 | *this [s1] | semmle.label | *this [s1] | | C.cpp:29:10:29:11 | s1 | semmle.label | s1 | +| C.cpp:30:10:30:11 | *this [s2] | semmle.label | *this [s2] | +| C.cpp:30:10:30:11 | s2 | semmle.label | s2 | | C.cpp:31:10:31:11 | *this [s3] | semmle.label | *this [s3] | | C.cpp:31:10:31:11 | s3 | semmle.label | s3 | | D.cpp:10:11:10:17 | *getElem | semmle.label | *getElem | @@ -1958,6 +1981,7 @@ subpaths | B.cpp:9:10:9:24 | elem1 | B.cpp:6:15:6:24 | new | B.cpp:9:10:9:24 | elem1 | elem1 flows from $@ | B.cpp:6:15:6:24 | new | new | | B.cpp:19:10:19:24 | elem2 | B.cpp:15:15:15:27 | new | B.cpp:19:10:19:24 | elem2 | elem2 flows from $@ | B.cpp:15:15:15:27 | new | new | | C.cpp:29:10:29:11 | s1 | C.cpp:22:12:22:21 | new | C.cpp:29:10:29:11 | s1 | s1 flows from $@ | C.cpp:22:12:22:21 | new | new | +| C.cpp:30:10:30:11 | s2 | C.cpp:10:20:10:29 | new | C.cpp:30:10:30:11 | s2 | s2 flows from $@ | C.cpp:10:20:10:29 | new | new | | C.cpp:31:10:31:11 | s3 | C.cpp:24:16:24:25 | new | C.cpp:31:10:31:11 | s3 | s3 flows from $@ | C.cpp:24:16:24:25 | new | new | | D.cpp:22:10:22:33 | call to getElem | D.cpp:28:15:28:24 | new | D.cpp:22:10:22:33 | call to getElem | call to getElem flows from $@ | D.cpp:28:15:28:24 | new | new | | D.cpp:22:10:22:33 | call to getElem | D.cpp:35:15:35:24 | new | D.cpp:22:10:22:33 | call to getElem | call to getElem flows from $@ | D.cpp:35:15:35:24 | new | new | From 78c0c7cb76ff3a1e78aba140e222e616ce492618 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 25 Mar 2026 14:17:10 +0000 Subject: [PATCH 05/13] C++: Exclude flow summaries from 'irTypeBugs'. --- .../dataflow-tests/type-bugs.expected | 36 ------------------- .../dataflow/dataflow-tests/type-bugs.ql | 4 +++ 2 files changed, 4 insertions(+), 36 deletions(-) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected index 87ebdc9e83a..2ba0cf2928b 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected @@ -1,41 +1,5 @@ astTypeBugs irTypeBugs -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary param] *0 in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary param] this in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[****] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[***] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[**] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[*] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[****] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[***] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[**] in iterator | -| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[*] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary param] *0 in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary param] this in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[****] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[***] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[**] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[*] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[****] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[***] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[**] in iterator | -| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[*] in iterator | -| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary param] this in operator* | -| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] read: Argument[this].Element in operator* | -| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] read: Argument[this].Element[*] in operator* | -| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] to write: ReturnValue[**] in operator* | -| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] to write: ReturnValue[*] in operator* | -| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary param] this in operator-> | -| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] read: Argument[this].Element in operator-> | -| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] read: Argument[this].Element[*] in operator-> | -| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] to write: ReturnValue[**] in operator-> | -| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] to write: ReturnValue[*] in operator-> | incorrectBaseType | clang.cpp:22:8:22:20 | *& ... | Expected 'Node.getType()' to be int, but it was int * | | clang.cpp:23:17:23:29 | *& ... | Expected 'Node.getType()' to be int, but it was int * | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql index 3e5f9165ef8..84fd3e6bbe4 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql @@ -17,9 +17,13 @@ import AstTest module IrTest { private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil + private import semmle.code.cpp.ir.dataflow.internal.DataFlowNodes query predicate irTypeBugs(Location location, Node node) { exists(int n | + // Flow summary nodes don't have a type since we don't (necessary) have + // the source code in the database. + not node instanceof FlowSummaryNode and n = count(node.getType()) and location = node.getLocation() and n != 1 From 29768bbed481cb4fc2c61123d0ed25d5955818bd Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 30 Mar 2026 11:26:24 +0100 Subject: [PATCH 06/13] Update cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql index 84fd3e6bbe4..3fcf39ef1c5 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql @@ -21,7 +21,7 @@ module IrTest { query predicate irTypeBugs(Location location, Node node) { exists(int n | - // Flow summary nodes don't have a type since we don't (necessary) have + // Flow summary nodes don't have a type since we don't necessarily have // the source code in the database. not node instanceof FlowSummaryNode and n = count(node.getType()) and From 9247e6af0c5b9890da02913d62419a84afcc903e Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 30 Mar 2026 11:30:17 +0100 Subject: [PATCH 07/13] C++: Add change note. --- cpp/ql/lib/change-notes/2026-03-30-nsdmi-dataflow.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2026-03-30-nsdmi-dataflow.md diff --git a/cpp/ql/lib/change-notes/2026-03-30-nsdmi-dataflow.md b/cpp/ql/lib/change-notes/2026-03-30-nsdmi-dataflow.md new file mode 100644 index 00000000000..8bf87900330 --- /dev/null +++ b/cpp/ql/lib/change-notes/2026-03-30-nsdmi-dataflow.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added dataflow through members initialized via non-static data member initialization (NSDMI). \ No newline at end of file From 5db069eb5689773f760f8bc854bddb2efe2689b4 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 30 Mar 2026 12:08:08 +0100 Subject: [PATCH 08/13] C++: Fix more consistency errors. --- .../code/cpp/ir/dataflow/internal/DataFlowNodes.qll | 9 +++++++-- .../syntax-zoo/dataflow-ir-consistency.expected | 2 -- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll index 637d8f73eb9..bcf6a0d512c 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll @@ -1687,7 +1687,9 @@ private class IndirectInstructionParameterNode extends AbstractIndirectParameter InitializeParameterInstruction init; IndirectInstructionParameterNode() { - IndirectInstruction.super.hasInstructionAndIndirectionIndex(init, _) + IndirectInstruction.super.hasInstructionAndIndirectionIndex(init, _) and + // We don't model catch parameters as parameter nodes + not exists(init.getParameter().getCatchBlock()) } int getArgumentIndex() { init.hasIndex(result) } @@ -1759,7 +1761,10 @@ abstract private class AbstractExplicitParameterNode extends AbstractDirectParam private class ExplicitParameterInstructionNode extends AbstractExplicitParameterNode, InstructionDirectParameterNode { - ExplicitParameterInstructionNode() { exists(instr.getParameter()) } + ExplicitParameterInstructionNode() { + // We don't model catch parameters as parameter nodes. + exists(instr.getParameter().getFunction()) + } override string toStringImpl() { result = instr.getParameter().toString() } } diff --git a/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected index 984335d1251..e6556b1a89c 100644 --- a/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected +++ b/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected @@ -29,8 +29,6 @@ postWithInFlow | try_catch.cpp:7:8:7:8 | call to exception | PostUpdateNode should not be the target of local flow. | viableImplInCallContextTooLarge uniqueParameterNodeAtPosition -| ir.cpp:726:6:726:13 | TryCatch | *0 | ir.cpp:737:22:737:22 | *s | Parameters with overlapping positions. | -| ir.cpp:726:6:726:13 | TryCatch | *0 | ir.cpp:740:24:740:24 | *e | Parameters with overlapping positions. | uniqueParameterNodePosition uniqueContentApprox identityLocalStep From 40366042a5c5cef8b8a8584818008225cd3a5082 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 30 Mar 2026 15:24:32 +0200 Subject: [PATCH 09/13] C#: Fix inconsistent casing of Cs/CS. --- .../code/csharp/dataflow/internal/DataFlowPrivate.qll | 4 ++-- .../code/csharp/dataflow/internal/FlowSummaryImpl.qll | 4 ++-- .../lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll | 1 - csharp/ql/lib/utils/test/InlineMadTest.qll | 8 ++++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index afb09f85835..109c27de67b 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -274,7 +274,7 @@ module VariableCapture { } private module CaptureInput implements Shared::InputSig { - private import csharp as Cs + private import csharp as CS private import semmle.code.csharp.controlflow.ControlFlowGraph as Cfg private import TaintTrackingPrivate as TaintTrackingPrivate @@ -391,7 +391,7 @@ module VariableCapture { } } - class Callable extends Cs::Callable { + class Callable extends CS::Callable { predicate isConstructor() { this instanceof Constructor } } } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll index 6f9b621ff40..ec6bbf81fee 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll @@ -215,9 +215,9 @@ private module StepsInput implements Impl::Private::StepsInputSig { module SourceSinkInterpretationInput implements Impl::Private::External::SourceSinkInterpretationInputSig { - private import csharp as Cs + private import csharp as CS - class Element = Cs::Element; + class Element = CS::Element; predicate sourceElement( Element e, string output, string kind, Public::Provenance provenance, string model diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll index 7a592bebff0..2809f8187b9 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll @@ -1042,7 +1042,6 @@ string getToStringPrefix(Definition def) { } private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig { - private import csharp as Cs private import semmle.code.csharp.controlflow.BasicBlocks private import codeql.util.Boolean diff --git a/csharp/ql/lib/utils/test/InlineMadTest.qll b/csharp/ql/lib/utils/test/InlineMadTest.qll index b614fda41db..acc1df8eab3 100644 --- a/csharp/ql/lib/utils/test/InlineMadTest.qll +++ b/csharp/ql/lib/utils/test/InlineMadTest.qll @@ -1,14 +1,14 @@ -private import csharp as Cs +private import csharp as CS private import codeql.mad.test.InlineMadTest private module InlineMadTestLang implements InlineMadTestLangSig { - class Callable = Cs::Callable; + class Callable = CS::Callable; string getComment(Callable c) { - exists(Cs::CommentBlock block, Cs::Element after | after = block.getAfter() | + exists(CS::CommentBlock block, CS::Element after | after = block.getAfter() | ( after = c or - after = c.(Cs::Accessor).getDeclaration() + after = c.(CS::Accessor).getDeclaration() ) and result = block.getALine() ) From 3c78d8a737cebd1933fcee8858ea40da79c7fcb9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 30 Mar 2026 14:50:44 +0000 Subject: [PATCH 10/13] update codeql documentation --- .../codeql-changelog/codeql-cli-2.25.1.rst | 30 +++++++++++++++++++ .../codeql-changelog/index.rst | 1 + 2 files changed, 31 insertions(+) create mode 100644 docs/codeql/codeql-overview/codeql-changelog/codeql-cli-2.25.1.rst diff --git a/docs/codeql/codeql-overview/codeql-changelog/codeql-cli-2.25.1.rst b/docs/codeql/codeql-overview/codeql-changelog/codeql-cli-2.25.1.rst new file mode 100644 index 00000000000..1164b8d90c3 --- /dev/null +++ b/docs/codeql/codeql-overview/codeql-changelog/codeql-cli-2.25.1.rst @@ -0,0 +1,30 @@ +.. _codeql-cli-2.25.1: + +========================== +CodeQL 2.25.1 (2026-03-27) +========================== + +.. contents:: Contents + :depth: 2 + :local: + :backlinks: none + +This is an overview of changes in the CodeQL CLI and relevant CodeQL query and library packs. For additional updates on changes to the CodeQL code scanning experience, check out the `code scanning section on the GitHub blog `__, `relevant GitHub Changelog updates `__, `changes in the CodeQL extension for Visual Studio Code `__, and the `CodeQL Action changelog `__. + +Security Coverage +----------------- + +CodeQL 2.25.1 runs a total of 491 security queries when configured with the Default suite (covering 166 CWE). The Extended suite enables an additional 135 queries (covering 35 more CWE). + +CodeQL CLI +---------- + +Bug Fixes +~~~~~~~~~ + +* Fixed a bug where extraction could fail on YAML files containing emoji. + +Miscellaneous +~~~~~~~~~~~~~ + +* Upgraded snakeyaml (which is a dependency of jackson-dataformat-yaml) from 2.3 to 2.6. diff --git a/docs/codeql/codeql-overview/codeql-changelog/index.rst b/docs/codeql/codeql-overview/codeql-changelog/index.rst index d880a7e56d8..fc3c5247328 100644 --- a/docs/codeql/codeql-overview/codeql-changelog/index.rst +++ b/docs/codeql/codeql-overview/codeql-changelog/index.rst @@ -11,6 +11,7 @@ A list of queries for each suite and language `is available here Date: Fri, 27 Mar 2026 15:08:21 +0100 Subject: [PATCH 11/13] C#: Simplify the ConstantCondition query. --- .../Control-Flow/ConstantCondition.ql | 113 ++++-------------- .../ConstantCondition/ConstantCondition.cs | 10 +- .../ConstantCondition.expected | 16 +-- .../ConstantConditionalExpressionCondition.cs | 2 +- .../ConstantCondition/ConstantDoCondition.cs | 4 +- .../ConstantCondition/ConstantForCondition.cs | 2 +- .../ConstantCondition/ConstantIfCondition.cs | 7 +- .../ConstantIsNullOrEmpty.cs | 11 +- .../ConstantWhileCondition.cs | 2 +- .../ConstantCondition/ConstantCondition.cs | 2 +- .../ConstantCondition.expected | 2 - 11 files changed, 51 insertions(+), 120 deletions(-) diff --git a/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql b/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql index a58e19049ff..c1eb996863c 100644 --- a/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql +++ b/csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql @@ -41,7 +41,9 @@ module ConstCondInput implements ConstCond::InputSig { module ConstCondImpl = ConstCond::Make; predicate nullCheck(Expr e, boolean direct) { - exists(QualifiableExpr qe | qe.isConditional() and qe.getQualifier() = e and direct = true) + exists(QualifiableExpr qe | qe.isConditional() and direct = true | + qe.getQualifier() = e or qe.(ExtensionMethodCall).getArgument(0) = e + ) or exists(NullCoalescingOperation nce | nce.getLeftOperand() = e and direct = true) or @@ -108,57 +110,14 @@ class ConstantGuard extends ConstantCondition { class ConstantBooleanCondition extends ConstantCondition { boolean b; - ConstantBooleanCondition() { isConstantCondition(this, b) } + ConstantBooleanCondition() { isConstantComparison(this, b) } override string getMessage() { result = "Condition always evaluates to '" + b + "'." } - - override predicate isWhiteListed() { - // E.g. `x ?? false` - this.(BoolLiteral) = any(NullCoalescingOperation nce).getRightOperand() or - // No need to flag logical operations when the operands are constant - isConstantCondition(this.(LogicalNotExpr).getOperand(), _) or - this = - any(LogicalAndExpr lae | - isConstantCondition(lae.getAnOperand(), false) - or - isConstantCondition(lae.getLeftOperand(), true) and - isConstantCondition(lae.getRightOperand(), true) - ) or - this = - any(LogicalOrExpr loe | - isConstantCondition(loe.getAnOperand(), true) - or - isConstantCondition(loe.getLeftOperand(), false) and - isConstantCondition(loe.getRightOperand(), false) - ) - } } -/** A constant condition in an `if` statement or a conditional expression. */ -class ConstantIfCondition extends ConstantBooleanCondition { - ConstantIfCondition() { - this = any(IfStmt is).getCondition().getAChildExpr*() or - this = any(ConditionalExpr ce).getCondition().getAChildExpr*() - } - - override predicate isWhiteListed() { - ConstantBooleanCondition.super.isWhiteListed() - or - // It is a common pattern to use a local constant/constant field to control - // whether code parts must be executed or not - this instanceof AssignableRead and - not this instanceof ParameterRead - } -} - -/** A constant loop condition. */ -class ConstantLoopCondition extends ConstantBooleanCondition { - ConstantLoopCondition() { this = any(LoopStmt ls).getCondition() } - - override predicate isWhiteListed() { - // Clearly intentional infinite loops are allowed - this.(BoolLiteral).getBoolValue() = true - } +private Expr getQualifier(QualifiableExpr e) { + // `e.getQualifier()` does not work for calls to extension methods + result = e.getChildExpr(-1) } /** A constant nullness condition. */ @@ -166,14 +125,23 @@ class ConstantNullnessCondition extends ConstantCondition { boolean b; ConstantNullnessCondition() { - forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() | - exists(ControlFlow::NullnessSuccessor t, ControlFlow::Node s | - s = cfn.getASuccessorByType(t) - | - b = t.getValue() and - not s.isJoin() - ) and - strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1 + nullCheck(this, true) and + exists(Expr stripped | stripped = this.(Expr).stripCasts() | + stripped.getType() = + any(ValueType t | + not t instanceof NullableType and + // Extractor bug: the type of `x?.Length` is reported as `int`, but it should + // be `int?` + not getQualifier*(stripped).(QualifiableExpr).isConditional() + ) and + b = false + or + stripped instanceof NullLiteral and + b = true + or + stripped.hasValue() and + not stripped instanceof NullLiteral and + b = false ) } @@ -184,39 +152,6 @@ class ConstantNullnessCondition extends ConstantCondition { } } -/** A constant matching condition. */ -class ConstantMatchingCondition extends ConstantCondition { - boolean b; - - ConstantMatchingCondition() { - this instanceof Expr and - forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() | - exists(ControlFlow::MatchingSuccessor t | exists(cfn.getASuccessorByType(t)) | - b = t.getValue() - ) and - strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1 - ) - } - - override predicate isWhiteListed() { - exists(Switch se, Case c, int i | - c = se.getCase(i) and - c.getPattern() = this.(DiscardExpr) - | - i > 0 - or - i = 0 and - exists(Expr cond | c.getCondition() = cond and not isConstantCondition(cond, true)) - ) - or - this = any(PositionalPatternExpr ppe).getPattern(_) - } - - override string getMessage() { - if b = true then result = "Pattern always matches." else result = "Pattern never matches." - } -} - from ConstantCondition c, string msg, Guards::Guards::Guard reason, string reasonMsg where msg = c.getMessage() and diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs index 0445e152ec7..95e8cd38c11 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs @@ -59,9 +59,9 @@ class ConstantMatching { switch (1 + 2) { - case 2: // $ Alert + case 2: // Intentionally missing Alert break; - case 3: // $ Alert + case 3: // Intentionally missing Alert break; case int _: // GOOD break; @@ -72,7 +72,7 @@ class ConstantMatching { switch ((object)s) { - case int _: // $ Alert + case int _: // Intentionally missing Alert break; case "": // GOOD break; @@ -92,7 +92,7 @@ class ConstantMatching { return o switch { - _ => o.ToString() // $ Alert + _ => o.ToString() // GOOD, catch-all pattern is fine }; } @@ -138,7 +138,7 @@ class ConstantMatching { switch (i) { - case var _: // $ Alert + case var _: // GOOD, catch-all pattern is fine return "even"; } } diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected index fc310e53fde..7d1d716386c 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected @@ -4,28 +4,18 @@ | ConstantCondition.cs:47:17:47:18 | "" | Expression is never 'null'. | ConstantCondition.cs:47:17:47:18 | "" | dummy | | ConstantCondition.cs:48:13:48:19 | (...) ... | Expression is never 'null'. | ConstantCondition.cs:48:13:48:19 | (...) ... | dummy | | ConstantCondition.cs:49:13:49:14 | "" | Expression is never 'null'. | ConstantCondition.cs:49:13:49:14 | "" | dummy | -| ConstantCondition.cs:62:18:62:18 | 2 | Pattern never matches. | ConstantCondition.cs:62:18:62:18 | 2 | dummy | -| ConstantCondition.cs:64:18:64:18 | 3 | Pattern always matches. | ConstantCondition.cs:64:18:64:18 | 3 | dummy | -| ConstantCondition.cs:75:18:75:20 | access to type Int32 | Pattern never matches. | ConstantCondition.cs:75:18:75:20 | access to type Int32 | dummy | -| ConstantCondition.cs:95:13:95:13 | _ | Pattern always matches. | ConstantCondition.cs:95:13:95:13 | _ | dummy | | ConstantCondition.cs:114:13:114:14 | access to parameter b1 | Condition is always true because of $@. | ConstantCondition.cs:110:14:110:15 | access to parameter b1 | access to parameter b1 | | ConstantCondition.cs:114:19:114:20 | access to parameter b2 | Condition is always true because of $@. | ConstantCondition.cs:112:14:112:15 | access to parameter b2 | access to parameter b2 | -| ConstantCondition.cs:141:22:141:22 | _ | Pattern always matches. | ConstantCondition.cs:141:22:141:22 | _ | dummy | | ConstantConditionBad.cs:5:16:5:20 | ... > ... | Condition always evaluates to 'false'. | ConstantConditionBad.cs:5:16:5:20 | ... > ... | dummy | | ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | Condition always evaluates to 'true'. | ConstantConditionalExpressionCondition.cs:11:22:11:34 | ... == ... | dummy | -| ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | Condition always evaluates to 'false'. | ConstantConditionalExpressionCondition.cs:12:21:12:25 | false | dummy | | ConstantConditionalExpressionCondition.cs:13:21:13:30 | ... == ... | Condition always evaluates to 'true'. | ConstantConditionalExpressionCondition.cs:13:21:13:30 | ... == ... | dummy | -| ConstantForCondition.cs:9:29:9:33 | false | Condition always evaluates to 'false'. | ConstantForCondition.cs:9:29:9:33 | false | dummy | +| ConstantDoCondition.cs:15:22:15:34 | ... == ... | Condition always evaluates to 'true'. | ConstantDoCondition.cs:15:22:15:34 | ... == ... | dummy | +| ConstantDoCondition.cs:32:22:32:31 | ... == ... | Condition always evaluates to 'true'. | ConstantDoCondition.cs:32:22:32:31 | ... == ... | dummy | | ConstantForCondition.cs:11:29:11:34 | ... == ... | Condition always evaluates to 'false'. | ConstantForCondition.cs:11:29:11:34 | ... == ... | dummy | | ConstantIfCondition.cs:11:17:11:29 | ... == ... | Condition always evaluates to 'true'. | ConstantIfCondition.cs:11:17:11:29 | ... == ... | dummy | -| ConstantIfCondition.cs:14:17:14:21 | false | Condition always evaluates to 'false'. | ConstantIfCondition.cs:14:17:14:21 | false | dummy | | ConstantIfCondition.cs:17:17:17:26 | ... == ... | Condition always evaluates to 'true'. | ConstantIfCondition.cs:17:17:17:26 | ... == ... | dummy | -| ConstantIsNullOrEmpty.cs:10:21:10:54 | call to method IsNullOrEmpty | Condition always evaluates to 'false'. | ConstantIsNullOrEmpty.cs:10:21:10:54 | call to method IsNullOrEmpty | dummy | -| ConstantIsNullOrEmpty.cs:46:21:46:46 | call to method IsNullOrEmpty | Condition always evaluates to 'true'. | ConstantIsNullOrEmpty.cs:46:21:46:46 | call to method IsNullOrEmpty | dummy | -| ConstantIsNullOrEmpty.cs:50:21:50:44 | call to method IsNullOrEmpty | Condition always evaluates to 'true'. | ConstantIsNullOrEmpty.cs:50:21:50:44 | call to method IsNullOrEmpty | dummy | -| ConstantIsNullOrEmpty.cs:54:21:54:45 | call to method IsNullOrEmpty | Condition always evaluates to 'false'. | ConstantIsNullOrEmpty.cs:54:21:54:45 | call to method IsNullOrEmpty | dummy | +| ConstantIfCondition.cs:35:20:35:25 | ... >= ... | Condition always evaluates to 'true'. | ConstantIfCondition.cs:35:20:35:25 | ... >= ... | dummy | | ConstantNullCoalescingLeftHandOperand.cs:11:24:11:34 | access to constant NULL_OBJECT | Expression is never 'null'. | ConstantNullCoalescingLeftHandOperand.cs:11:24:11:34 | access to constant NULL_OBJECT | dummy | | ConstantNullCoalescingLeftHandOperand.cs:12:24:12:27 | null | Expression is always 'null'. | ConstantNullCoalescingLeftHandOperand.cs:12:24:12:27 | null | dummy | | ConstantWhileCondition.cs:12:20:12:32 | ... == ... | Condition always evaluates to 'true'. | ConstantWhileCondition.cs:12:20:12:32 | ... == ... | dummy | -| ConstantWhileCondition.cs:16:20:16:24 | false | Condition always evaluates to 'false'. | ConstantWhileCondition.cs:16:20:16:24 | false | dummy | | ConstantWhileCondition.cs:24:20:24:29 | ... == ... | Condition always evaluates to 'true'. | ConstantWhileCondition.cs:24:20:24:29 | ... == ... | dummy | diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs index 4cd56232627..b84e746ae66 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantConditionalExpressionCondition.cs @@ -9,7 +9,7 @@ namespace ConstantConditionalExpression public void Foo() { int i = (ZERO == 1 - 1) ? 0 : 1; // $ Alert - int j = false ? 0 : 1; // $ Alert + int j = false ? 0 : 1; // GOOD, literal false is likely intentional int k = " " == " " ? 0 : 1; // $ Alert int l = (" "[0] == ' ') ? 0 : 1; // Missing Alert int m = Bar() == 0 ? 0 : 1; // GOOD diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantDoCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantDoCondition.cs index 07db7f0c0ee..fadd44fefee 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantDoCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantDoCondition.cs @@ -12,7 +12,7 @@ namespace ConstantDoCondition do { break; - } while (ZERO == 1 - 1); // BAD + } while (ZERO == 1 - 1); // $ Alert do { break; @@ -29,7 +29,7 @@ namespace ConstantDoCondition do { break; - } while (" " == " "); // BAD + } while (" " == " "); // $ Alert do { break; diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs index 2da0589d182..74bc709348a 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantForCondition.cs @@ -6,7 +6,7 @@ namespace ConstantForCondition { public void M() { - for (int i = 0; false; i++) // $ Alert + for (int i = 0; false; i++) // GOOD, literal false is likely intentional ; for (int i = 0; 0 == 1; i++) // $ Alert ; diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs index 04c91cc222d..8da2623e1f4 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIfCondition.cs @@ -11,7 +11,7 @@ namespace ConstantIfCondition if (ZERO == 1 - 1) // $ Alert { } - if (false) // $ Alert + if (false) // GOOD { } if (" " == " ") // $ Alert @@ -30,6 +30,11 @@ namespace ConstantIfCondition return ZERO; } + public void UnsignedCheck(byte n) + { + while (n >= 0) { n--; } // $ Alert + } + } } diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs index 01e8353a20f..97857777574 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantIsNullOrEmpty.cs @@ -7,7 +7,10 @@ namespace ConstantIsNullOrEmpty static void Main(string[] args) { { - if (string.IsNullOrEmpty(nameof(args))) // $ Alert + // All of the IsNullOrEmpty constant checks have been descoped + // from the query as it didn't seem worth the effort to keep them. + + if (string.IsNullOrEmpty(nameof(args))) // Missing Alert (always false) { } @@ -43,15 +46,15 @@ namespace ConstantIsNullOrEmpty { } - if (string.IsNullOrEmpty(null)) // $ Alert + if (string.IsNullOrEmpty(null)) // Missing Alert { } - if (string.IsNullOrEmpty("")) // $ Alert + if (string.IsNullOrEmpty("")) // Missing Alert { } - if (string.IsNullOrEmpty(" ")) // $ Alert + if (string.IsNullOrEmpty(" ")) // Missing Alert { } } diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs index 59575e0de45..f69cf165732 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantWhileCondition.cs @@ -13,7 +13,7 @@ namespace ConstantWhileCondition { break; } - while (false) // $ Alert + while (false) // Silly, but likely intentional { break; } diff --git a/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs b/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs index 6f40759b3e6..caf9d85b653 100644 --- a/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs +++ b/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.cs @@ -12,7 +12,7 @@ class ConstantMatching void M1() { var c1 = new C1(); - if (c1.Prop is int) // $ Alert + if (c1.Prop is int) // Descoped, no longer reported by the query. { } diff --git a/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected b/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected index b00cfb3115e..e69de29bb2d 100644 --- a/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected +++ b/csharp/ql/test/query-tests/standalone/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected @@ -1,2 +0,0 @@ -| ConstantCondition.cs:15:13:15:26 | ... is ... | Condition always evaluates to 'false'. | ConstantCondition.cs:15:13:15:26 | ... is ... | dummy | -| ConstantCondition.cs:15:24:15:26 | access to type Int32 | Pattern never matches. | ConstantCondition.cs:15:24:15:26 | access to type Int32 | dummy | From 2a54dce5cbb1877b30c5763b4f9c9d65cef018ed Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 30 Mar 2026 11:44:02 +0200 Subject: [PATCH 12/13] C#: Remove redundant ConstantComparison.ql query. --- .../csharp-code-quality-extended.qls.expected | 1 - .../csharp-code-quality.qls.expected | 1 - .../csharp-security-and-quality.qls.expected | 1 - .../semmle/code/csharp/commons/Constants.qll | 13 ----- csharp/ql/src/CSI/CompareIdenticalValues.ql | 3 +- .../ql/src/Likely Bugs/ConstantComparison.cs | 2 - .../src/Likely Bugs/ConstantComparison.qhelp | 46 ---------------- .../ql/src/Likely Bugs/ConstantComparison.ql | 22 -------- .../csharp-security-and-quality.qls | 1 - .../ConstantCondition}/ConstantComparison.cs | 54 +++++++++---------- .../ConstantCondition.expected | 26 +++++++++ .../ConstantComparison.expected | 26 --------- .../ConstantComparison.qlref | 1 - .../Likely Bugs/ConstantComparison/options | 2 - 14 files changed, 54 insertions(+), 145 deletions(-) delete mode 100644 csharp/ql/src/Likely Bugs/ConstantComparison.cs delete mode 100644 csharp/ql/src/Likely Bugs/ConstantComparison.qhelp delete mode 100644 csharp/ql/src/Likely Bugs/ConstantComparison.ql rename csharp/ql/test/query-tests/{Likely Bugs/ConstantComparison => Bad Practices/Control-Flow/ConstantCondition}/ConstantComparison.cs (52%) delete mode 100644 csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.expected delete mode 100644 csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.qlref delete mode 100644 csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/options diff --git a/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality-extended.qls.expected b/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality-extended.qls.expected index fdc5e6eae9d..c6361fe69c5 100644 --- a/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality-extended.qls.expected +++ b/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality-extended.qls.expected @@ -65,7 +65,6 @@ ql/csharp/ql/src/Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql ql/csharp/ql/src/Likely Bugs/Collections/ContainerSizeCmpZero.ql ql/csharp/ql/src/Likely Bugs/Collections/ReadOnlyContainer.ql ql/csharp/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql -ql/csharp/ql/src/Likely Bugs/ConstantComparison.ql ql/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql ql/csharp/ql/src/Likely Bugs/Dynamic/BadDynamicCall.ql ql/csharp/ql/src/Likely Bugs/EqualityCheckOnFloats.ql diff --git a/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected b/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected index 6694cc8461b..893eaeb7560 100644 --- a/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected +++ b/csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected @@ -38,7 +38,6 @@ ql/csharp/ql/src/Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql ql/csharp/ql/src/Likely Bugs/Collections/ContainerSizeCmpZero.ql ql/csharp/ql/src/Likely Bugs/Collections/ReadOnlyContainer.ql ql/csharp/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql -ql/csharp/ql/src/Likely Bugs/ConstantComparison.ql ql/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql ql/csharp/ql/src/Likely Bugs/EqualityCheckOnFloats.ql ql/csharp/ql/src/Likely Bugs/EqualsArray.ql diff --git a/csharp/ql/integration-tests/posix/query-suite/csharp-security-and-quality.qls.expected b/csharp/ql/integration-tests/posix/query-suite/csharp-security-and-quality.qls.expected index b520a571fc8..43c14b4281c 100644 --- a/csharp/ql/integration-tests/posix/query-suite/csharp-security-and-quality.qls.expected +++ b/csharp/ql/integration-tests/posix/query-suite/csharp-security-and-quality.qls.expected @@ -69,7 +69,6 @@ ql/csharp/ql/src/Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql ql/csharp/ql/src/Likely Bugs/Collections/ContainerSizeCmpZero.ql ql/csharp/ql/src/Likely Bugs/Collections/ReadOnlyContainer.ql ql/csharp/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql -ql/csharp/ql/src/Likely Bugs/ConstantComparison.ql ql/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql ql/csharp/ql/src/Likely Bugs/Dynamic/BadDynamicCall.ql ql/csharp/ql/src/Likely Bugs/EqualityCheckOnFloats.ql diff --git a/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll b/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll index ab2d9e0eef7..5025202eb21 100644 --- a/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll +++ b/csharp/ql/lib/semmle/code/csharp/commons/Constants.qll @@ -4,19 +4,6 @@ import csharp private import semmle.code.csharp.commons.ComparisonTest private import semmle.code.csharp.commons.StructuralComparison as StructuralComparison -pragma[noinline] -private predicate isConstantCondition0(ControlFlow::Node cfn, boolean b) { - exists(cfn.getASuccessorByType(any(ControlFlow::BooleanSuccessor t | t.getValue() = b))) and - strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1 -} - -/** - * Holds if `e` is a condition that always evaluates to Boolean value `b`. - */ -predicate isConstantCondition(Expr e, boolean b) { - forex(ControlFlow::Node cfn | cfn = e.getAControlFlowNode() | isConstantCondition0(cfn, b)) -} - /** * Holds if comparison operation `co` is constant with the Boolean value `b`. * For example, the comparison `x > x` is constantly `false` in diff --git a/csharp/ql/src/CSI/CompareIdenticalValues.ql b/csharp/ql/src/CSI/CompareIdenticalValues.ql index 503067a8a3e..fe79db08206 100644 --- a/csharp/ql/src/CSI/CompareIdenticalValues.ql +++ b/csharp/ql/src/CSI/CompareIdenticalValues.ql @@ -47,7 +47,6 @@ where not comparesIdenticalValuesNan(ct, _) and msg = "Comparison of identical values." ) and not isMutatingOperation(ct.getAnArgument().getAChild*()) and - not isConstantCondition(e, _) and // Avoid overlap with cs/constant-condition - not isConstantComparison(e, _) and // Avoid overlap with cs/constant-comparison + not isConstantComparison(e, _) and // Avoid overlap with cs/constant-condition not isExprInAssertion(e) select ct, msg diff --git a/csharp/ql/src/Likely Bugs/ConstantComparison.cs b/csharp/ql/src/Likely Bugs/ConstantComparison.cs deleted file mode 100644 index 5b0304b2818..00000000000 --- a/csharp/ql/src/Likely Bugs/ConstantComparison.cs +++ /dev/null @@ -1,2 +0,0 @@ - for (uint order = numberOfOrders; order >= 0; order--) - ProcessOrder(order); diff --git a/csharp/ql/src/Likely Bugs/ConstantComparison.qhelp b/csharp/ql/src/Likely Bugs/ConstantComparison.qhelp deleted file mode 100644 index 5e52142c84e..00000000000 --- a/csharp/ql/src/Likely Bugs/ConstantComparison.qhelp +++ /dev/null @@ -1,46 +0,0 @@ - - - -

- Comparisons which always yield the same result are unnecessary and may indicate a bug in the - logic. This can happen when the data type of one of the operands has a limited range of values. - For example unsigned integers are always greater than or equal to zero, and byte - values are always less than 256. -

- -

The following expressions always have the same result:

-
    -
  • Unsigned < 0 is always false,
  • -
  • 0 > Unsigned is always false,
  • -
  • 0 ≤ Unsigned is always true,
  • -
  • Unsigned ≥ 0 is always true,
  • -
  • Unsigned == -1 is always false,
  • -
  • Byte < 512 is always true.
  • -
-
- - -

- Examine the logic of the program to determine whether the comparison is necessary. - Either change the data types, or remove the unnecessary code. -

-
- - -

The following example attempts to count down from numberOfOrders to 0, - however the loop never terminates because order is an unsigned integer and so the - condition order >= 0 is always true.

- - - -

The solution is to change the type of the variable order.

-
- - -
  • - MSDN Library: C# Operators. -
  • -
    -
    \ No newline at end of file diff --git a/csharp/ql/src/Likely Bugs/ConstantComparison.ql b/csharp/ql/src/Likely Bugs/ConstantComparison.ql deleted file mode 100644 index 98352348214..00000000000 --- a/csharp/ql/src/Likely Bugs/ConstantComparison.ql +++ /dev/null @@ -1,22 +0,0 @@ -/** - * @name Comparison is constant - * @description The result of the comparison is always the same. - * @kind problem - * @problem.severity warning - * @precision high - * @id cs/constant-comparison - * @tags quality - * reliability - * correctness - */ - -import csharp -import semmle.code.csharp.commons.Assertions -import semmle.code.csharp.commons.Constants - -from ComparisonOperation cmp, boolean value -where - isConstantComparison(cmp, value) and - not isConstantCondition(cmp, _) and // Avoid overlap with cs/constant-condition - not isExprInAssertion(cmp) -select cmp, "This comparison is always " + value + "." diff --git a/csharp/ql/src/codeql-suites/csharp-security-and-quality.qls b/csharp/ql/src/codeql-suites/csharp-security-and-quality.qls index 21d39db383d..9700c8b0341 100644 --- a/csharp/ql/src/codeql-suites/csharp-security-and-quality.qls +++ b/csharp/ql/src/codeql-suites/csharp-security-and-quality.qls @@ -22,7 +22,6 @@ - cs/comparison-of-identical-expressions - cs/complex-block - cs/complex-condition - - cs/constant-comparison - cs/constant-condition - cs/coupled-types - cs/dereferenced-value-is-always-null diff --git a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.cs b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantComparison.cs similarity index 52% rename from csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.cs rename to csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantComparison.cs index c31d940e7f2..26f2c457325 100644 --- a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.cs +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantComparison.cs @@ -16,16 +16,16 @@ class Test { bool good, bad; - bad = uintValue < 0; - bad = 0 > uintValue; - bad = 0 <= uintValue; - bad = uintValue >= 0; + bad = uintValue < 0; // $ Alert + bad = 0 > uintValue; // $ Alert + bad = 0 <= uintValue; // $ Alert + bad = uintValue >= 0; // $ Alert - bad = uintValue == -1; - bad = uintValue != -1; - bad = 256 == byteValue; - bad = 256 != byteValue; - bad = 1 != 0; + bad = uintValue == -1; // $ Alert + bad = uintValue != -1; // $ Alert + bad = 256 == byteValue; // $ Alert + bad = 256 != byteValue; // $ Alert + bad = 1 != 0; // $ Alert good = byteValue == 50; good = 50 != byteValue; @@ -35,61 +35,61 @@ class Test good = intValue <= 1u; good = 1u >= intValue; - good = charValue >= '0'; // Regression + good = charValue >= '0'; good = charValue < '0'; // Test ranges - bad = charValue <= 65535; - bad = charValue >= 0; + bad = charValue <= 65535; // $ Alert + bad = charValue >= 0; // $ Alert good = charValue < 255; good = charValue > 0; - bad = byteValue >= byte.MinValue; - bad = byteValue <= byte.MaxValue; + bad = byteValue >= byte.MinValue; // $ Alert + bad = byteValue <= byte.MaxValue; // $ Alert good = byteValue > byte.MinValue; good = byteValue < byte.MaxValue; - bad = sbyteValue >= sbyte.MinValue; - bad = sbyteValue <= sbyte.MaxValue; + bad = sbyteValue >= sbyte.MinValue; // $ Alert + bad = sbyteValue <= sbyte.MaxValue; // $ Alert good = sbyteValue < sbyte.MaxValue; good = sbyteValue > sbyte.MinValue; - bad = shortValue >= short.MinValue; - bad = shortValue <= short.MaxValue; + bad = shortValue >= short.MinValue; // $ Alert + bad = shortValue <= short.MaxValue; // $ Alert good = shortValue > short.MinValue; good = shortValue < short.MaxValue; - bad = ushortValue >= ushort.MinValue; - bad = ushortValue <= ushort.MaxValue; + bad = ushortValue >= ushort.MinValue; // $ Alert + bad = ushortValue <= ushort.MaxValue; // $ Alert good = ushortValue > ushort.MinValue; good = ushortValue < ushort.MaxValue; - bad = intValue >= int.MinValue; - bad = intValue <= int.MaxValue; + bad = intValue >= int.MinValue; // $ Alert + bad = intValue <= int.MaxValue; // $ Alert good = intValue > int.MinValue; good = intValue < int.MaxValue; - bad = uintValue >= uint.MinValue; + bad = uintValue >= uint.MinValue; // $ Alert good = uintValue > uint.MinValue; - bad = ulongValue >= ulong.MinValue; + bad = ulongValue >= ulong.MinValue; // $ Alert good = ulongValue > ulong.MinValue; // Explicit casts can cause large values to be truncated or // to wrap into negative values. good = (sbyte)byteValue >= 0; good = (sbyte)byteValue == -1; - bad = (sbyte)byteValue > 127; - bad = (sbyte)byteValue > (sbyte)127; + bad = (sbyte)byteValue > 127; // $ Alert + bad = (sbyte)byteValue > (sbyte)127; // $ Alert good = (int)uintValue == -1; good = (sbyte)uintValue == -1; - bad = (sbyte)uintValue == 256; + bad = (sbyte)uintValue == 256; // $ Alert System.Diagnostics.Debug.Assert(ulongValue >= ulong.MinValue); // GOOD } diff --git a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected index 7d1d716386c..edf1f87232e 100644 --- a/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected +++ b/csharp/ql/test/query-tests/Bad Practices/Control-Flow/ConstantCondition/ConstantCondition.expected @@ -1,3 +1,29 @@ +| ConstantComparison.cs:19:15:19:27 | ... < ... | Condition always evaluates to 'false'. | ConstantComparison.cs:19:15:19:27 | ... < ... | dummy | +| ConstantComparison.cs:20:15:20:27 | ... > ... | Condition always evaluates to 'false'. | ConstantComparison.cs:20:15:20:27 | ... > ... | dummy | +| ConstantComparison.cs:21:15:21:28 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:21:15:21:28 | ... <= ... | dummy | +| ConstantComparison.cs:22:15:22:28 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:22:15:22:28 | ... >= ... | dummy | +| ConstantComparison.cs:24:15:24:29 | ... == ... | Condition always evaluates to 'false'. | ConstantComparison.cs:24:15:24:29 | ... == ... | dummy | +| ConstantComparison.cs:25:15:25:29 | ... != ... | Condition always evaluates to 'true'. | ConstantComparison.cs:25:15:25:29 | ... != ... | dummy | +| ConstantComparison.cs:26:15:26:30 | ... == ... | Condition always evaluates to 'false'. | ConstantComparison.cs:26:15:26:30 | ... == ... | dummy | +| ConstantComparison.cs:27:15:27:30 | ... != ... | Condition always evaluates to 'true'. | ConstantComparison.cs:27:15:27:30 | ... != ... | dummy | +| ConstantComparison.cs:28:15:28:20 | ... != ... | Condition always evaluates to 'true'. | ConstantComparison.cs:28:15:28:20 | ... != ... | dummy | +| ConstantComparison.cs:42:15:42:32 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:42:15:42:32 | ... <= ... | dummy | +| ConstantComparison.cs:43:15:43:28 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:43:15:43:28 | ... >= ... | dummy | +| ConstantComparison.cs:48:15:48:40 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:48:15:48:40 | ... >= ... | dummy | +| ConstantComparison.cs:49:15:49:40 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:49:15:49:40 | ... <= ... | dummy | +| ConstantComparison.cs:54:15:54:42 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:54:15:54:42 | ... >= ... | dummy | +| ConstantComparison.cs:55:15:55:42 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:55:15:55:42 | ... <= ... | dummy | +| ConstantComparison.cs:60:15:60:42 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:60:15:60:42 | ... >= ... | dummy | +| ConstantComparison.cs:61:15:61:42 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:61:15:61:42 | ... <= ... | dummy | +| ConstantComparison.cs:66:15:66:44 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:66:15:66:44 | ... >= ... | dummy | +| ConstantComparison.cs:67:15:67:44 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:67:15:67:44 | ... <= ... | dummy | +| ConstantComparison.cs:72:15:72:38 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:72:15:72:38 | ... >= ... | dummy | +| ConstantComparison.cs:73:15:73:38 | ... <= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:73:15:73:38 | ... <= ... | dummy | +| ConstantComparison.cs:78:15:78:40 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:78:15:78:40 | ... >= ... | dummy | +| ConstantComparison.cs:81:15:81:42 | ... >= ... | Condition always evaluates to 'true'. | ConstantComparison.cs:81:15:81:42 | ... >= ... | dummy | +| ConstantComparison.cs:88:15:88:36 | ... > ... | Condition always evaluates to 'false'. | ConstantComparison.cs:88:15:88:36 | ... > ... | dummy | +| ConstantComparison.cs:89:15:89:43 | ... > ... | Condition always evaluates to 'false'. | ConstantComparison.cs:89:15:89:43 | ... > ... | dummy | +| ConstantComparison.cs:92:15:92:37 | ... == ... | Condition always evaluates to 'false'. | ConstantComparison.cs:92:15:92:37 | ... == ... | dummy | | ConstantCondition.cs:38:18:38:29 | (...) ... | Expression is always 'null'. | ConstantCondition.cs:38:18:38:29 | (...) ... | dummy | | ConstantCondition.cs:39:18:39:24 | (...) ... | Expression is never 'null'. | ConstantCondition.cs:39:18:39:24 | (...) ... | dummy | | ConstantCondition.cs:46:17:46:26 | (...) ... | Expression is always 'null'. | ConstantCondition.cs:46:17:46:26 | (...) ... | dummy | diff --git a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.expected b/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.expected deleted file mode 100644 index 53f55501895..00000000000 --- a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.expected +++ /dev/null @@ -1,26 +0,0 @@ -| ConstantComparison.cs:19:15:19:27 | ... < ... | This comparison is always false. | -| ConstantComparison.cs:20:15:20:27 | ... > ... | This comparison is always false. | -| ConstantComparison.cs:21:15:21:28 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:22:15:22:28 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:24:15:24:29 | ... == ... | This comparison is always false. | -| ConstantComparison.cs:25:15:25:29 | ... != ... | This comparison is always true. | -| ConstantComparison.cs:26:15:26:30 | ... == ... | This comparison is always false. | -| ConstantComparison.cs:27:15:27:30 | ... != ... | This comparison is always true. | -| ConstantComparison.cs:28:15:28:20 | ... != ... | This comparison is always true. | -| ConstantComparison.cs:42:15:42:32 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:43:15:43:28 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:48:15:48:40 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:49:15:49:40 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:54:15:54:42 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:55:15:55:42 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:60:15:60:42 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:61:15:61:42 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:66:15:66:44 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:67:15:67:44 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:72:15:72:38 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:73:15:73:38 | ... <= ... | This comparison is always true. | -| ConstantComparison.cs:78:15:78:40 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:81:15:81:42 | ... >= ... | This comparison is always true. | -| ConstantComparison.cs:88:15:88:36 | ... > ... | This comparison is always false. | -| ConstantComparison.cs:89:15:89:43 | ... > ... | This comparison is always false. | -| ConstantComparison.cs:92:15:92:37 | ... == ... | This comparison is always false. | diff --git a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.qlref b/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.qlref deleted file mode 100644 index 8566e49f6cc..00000000000 --- a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/ConstantComparison.qlref +++ /dev/null @@ -1 +0,0 @@ -Likely Bugs/ConstantComparison.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/options b/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/options deleted file mode 100644 index 75c39b4541b..00000000000 --- a/csharp/ql/test/query-tests/Likely Bugs/ConstantComparison/options +++ /dev/null @@ -1,2 +0,0 @@ -semmle-extractor-options: /nostdlib /noconfig -semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj From 29500c7eb7a9b0616fec66841ba2570e11209d48 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 31 Mar 2026 09:29:28 +0200 Subject: [PATCH 13/13] C#: Add change note. --- .../src/change-notes/2026-03-31-constantcondition-simplify.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/src/change-notes/2026-03-31-constantcondition-simplify.md diff --git a/csharp/ql/src/change-notes/2026-03-31-constantcondition-simplify.md b/csharp/ql/src/change-notes/2026-03-31-constantcondition-simplify.md new file mode 100644 index 00000000000..a1051d4c00f --- /dev/null +++ b/csharp/ql/src/change-notes/2026-03-31-constantcondition-simplify.md @@ -0,0 +1,4 @@ +--- +category: majorAnalysis +--- +* The `cs/constant-condition` query has been simplified. The query no longer reports trivially constant conditions as they were found to generally be intentional. As a result, it should now produce fewer false positives. Additionally, the simplification means that it now reports all the results that `cs/constant-comparison` used to report, and as consequence, that query has been deleted.