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 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..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 @@ -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" } } @@ -1129,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 " | @@ -1633,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 @@ -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 { @@ -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) } @@ -1701,16 +1703,17 @@ 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() } 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 @@ -1738,6 +1741,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(Declaration f, ParameterPosition pos) { + this.getFunction() = f and + exists(int argumentIndex | + pos.(DirectPosition).getArgumentIndex() = argumentIndex and + instr.hasIndex(argumentIndex) + ) + } } abstract private class AbstractExplicitParameterNode extends AbstractDirectParameterNode { } @@ -1746,15 +1761,12 @@ abstract private class AbstractExplicitParameterNode extends AbstractDirectParam private class ExplicitParameterInstructionNode extends AbstractExplicitParameterNode, InstructionDirectParameterNode { - ExplicitParameterInstructionNode() { exists(instr.getParameter()) } - - override predicate isSourceParameterOf(Function f, ParameterPosition pos) { - f.getParameter(pos.(DirectPosition).getArgumentIndex()) = instr.getParameter() + ExplicitParameterInstructionNode() { + // We don't model catch parameters as parameter nodes. + exists(instr.getParameter().getFunction()) } override string toStringImpl() { result = instr.getParameter().toString() } - - override Parameter getParameter() { result = instr.getParameter() } } /** @@ -1782,9 +1794,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 } @@ -1795,10 +1807,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) } 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) + ) } /** 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..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,47 +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-> | -| 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/dataflow-tests/type-bugs.ql b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql index 3e5f9165ef8..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 @@ -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 necessarily have + // the source code in the database. + not node instanceof FlowSummaryNode and n = count(node.getType()) and location = node.getLocation() and n != 1 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 | 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 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/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() ) 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/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/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. 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.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..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,31 +1,47 @@ +| 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 | | 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/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 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 | 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