From aa7a73004139f02ed14fb6180a5d6bf9889372f4 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 19 Feb 2026 11:18:58 +0100 Subject: [PATCH] C#: Remove some unnecessary TCs --- .../semmle/code/csharp/controlflow/Guards.qll | 38 +++++++++++-------- .../internal/ControlFlowGraphImpl.qll | 12 +++++- .../dataflow/internal/DataFlowPrivate.qll | 15 +++++++- .../ql/lib/semmle/code/csharp/exprs/Expr.qll | 17 +++++++-- .../code/csharp/exprs/internal/Expr.qll | 11 ++++++ 5 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 csharp/ql/lib/semmle/code/csharp/exprs/internal/Expr.qll diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll index 414cfc2d50a..40ec3722edd 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll @@ -142,6 +142,7 @@ private module GuardsInput implements } } + pragma[nomagic] predicate equalityTest(Expr eqtest, Expr left, Expr right, boolean polarity) { exists(ComparisonTest ct | ct.getExpr() = eqtest and @@ -410,6 +411,22 @@ private predicate typePattern(PatternMatch pm, TypePatternExpr tpe, Type t) { t = pm.getExpr().getType() } +pragma[nomagic] +private predicate dereferenceableExpr(Expr e, boolean isNullableType) { + exists(Type t | t = e.getType() | + t instanceof NullableType and + isNullableType = true + or + t instanceof RefType and + isNullableType = false + ) + or + exists(Expr parent | + dereferenceableExpr(parent, isNullableType) and + e = getNullEquivParent(parent) + ) +} + /** * An expression that evaluates to a value that can be dereferenced. That is, * an expression that may evaluate to `null`. @@ -418,21 +435,12 @@ class DereferenceableExpr extends Expr { private boolean isNullableType; DereferenceableExpr() { - exists(Expr e, Type t | - // There is currently a bug in the extractor: the type of `x?.Length` is - // incorrectly `int`, while it should have been `int?`. We apply - // `getNullEquivParent()` as a workaround - this = getNullEquivParent*(e) and - t = e.getType() and - not this instanceof SwitchCaseExpr and - not this instanceof PatternExpr - | - t instanceof NullableType and - isNullableType = true - or - t instanceof RefType and - isNullableType = false - ) + // There is currently a bug in the extractor: the type of `x?.Length` is + // incorrectly `int`, while it should have been `int?`. We apply + // `getNullEquivParent()` as a workaround + dereferenceableExpr(this, isNullableType) and + not this instanceof SwitchCaseExpr and + not this instanceof PatternExpr } /** Holds if this expression has a nullable type `T?`. */ diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll index 6be79a17be2..576d0734ef6 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll @@ -94,9 +94,19 @@ private Element getAChild(Element p) { result = p.(AssignOperation).getExpandedAssignment() } +pragma[nomagic] +private predicate astNode(Element e) { + e = any(@top_level_exprorstmt_parent p | not p instanceof Attribute) + or + exists(Element parent | + astNode(parent) and + e = getAChild(parent) + ) +} + /** An AST node. */ class AstNode extends Element, TAstNode { - AstNode() { this = getAChild*(any(@top_level_exprorstmt_parent p | not p instanceof Attribute)) } + AstNode() { astNode(this) } int getId() { idOf(this, result) } } 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 1b3de63495f..efed0846053 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -7,6 +7,7 @@ private import FlowSummaryImpl as FlowSummaryImpl private import semmle.code.csharp.dataflow.FlowSummary as FlowSummary private import semmle.code.csharp.dataflow.internal.ExternalFlow private import semmle.code.csharp.Conversion +private import semmle.code.csharp.exprs.internal.Expr private import semmle.code.csharp.dataflow.internal.SsaImpl as SsaImpl private import semmle.code.csharp.ExprOrStmtParent private import semmle.code.csharp.Unification @@ -2377,6 +2378,16 @@ predicate storeStep(Node node1, ContentSet c, Node node2) { storeStepDelegateCall(node1, c, node2) } +pragma[nomagic] +private predicate isAssignExprLValueDescendant(Expr e) { + e = any(AssignExpr ae).getLValue() + or + exists(Expr parent | + isAssignExprLValueDescendant(parent) and + e = parent.getAChildExpr() + ) +} + private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration { ReadStepConfiguration() { this = "ReadStepConfiguration" } @@ -2432,7 +2443,7 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration scope = any(AssignExpr ae | ae = defTo.(AssignableDefinitions::TupleAssignmentDefinition).getAssignment() and - e = ae.getLValue().getAChildExpr*().(TupleExpr) and + isAssignExprLValueDescendant(e.(TupleExpr)) and exactScope = false and isSuccessor = true ) @@ -2488,7 +2499,7 @@ private predicate readContentStep(Node node1, Content c, Node node2) { ) or // item = variable in node1 = (..., variable, ...) in a case/is var (..., ...) - te = any(PatternExpr pe).getAChildExpr*() and + isPatternExprDescendant(te) and exists(AssignableDefinitions::LocalVariableDefinition lvd | node2.(AssignableDefinitionNode).getDefinition() = lvd and lvd.getDeclaration() = item and diff --git a/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll b/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll index ada01065258..42d9320bfa8 100644 --- a/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll +++ b/csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll @@ -21,6 +21,7 @@ import semmle.code.csharp.Type private import semmle.code.csharp.ExprOrStmtParent private import semmle.code.csharp.frameworks.System private import semmle.code.csharp.TypeRef +private import internal.Expr /** * An expression. Either an access (`Access`), a call (`Call`), an object or @@ -64,14 +65,24 @@ class Expr extends ControlFlowElement, @expr { /** Gets the enclosing callable of this expression, if any. */ override Callable getEnclosingCallable() { enclosingCallable(this, result) } + pragma[nomagic] + private predicate isExpandedAssignmentRValueDescendant() { + this = + any(AssignOperation op).getExpandedAssignment().getRValue().getChildExpr(0).getAChildExpr() + or + exists(Expr parent | + parent.isExpandedAssignmentRValueDescendant() and + this = parent.getAChildExpr() + ) + } + /** * Holds if this expression is generated by the compiler and does not appear * explicitly in the source code. */ final predicate isImplicit() { compiler_generated(this) or - this = - any(AssignOperation op).getExpandedAssignment().getRValue().getChildExpr(0).getAChildExpr+() + this.isExpandedAssignmentRValueDescendant() } /** @@ -1133,7 +1144,7 @@ class TupleExpr extends Expr, @tuple_expr { /** Holds if this expression is a tuple construction. */ predicate isConstruction() { not this = getAnAssignOrForeachChild() and - not this = any(PatternExpr pe).getAChildExpr*() + not isPatternExprDescendant(this) } override string getAPrimaryQlClass() { result = "TupleExpr" } diff --git a/csharp/ql/lib/semmle/code/csharp/exprs/internal/Expr.qll b/csharp/ql/lib/semmle/code/csharp/exprs/internal/Expr.qll new file mode 100644 index 00000000000..94dfac72193 --- /dev/null +++ b/csharp/ql/lib/semmle/code/csharp/exprs/internal/Expr.qll @@ -0,0 +1,11 @@ +private import csharp + +pragma[nomagic] +predicate isPatternExprDescendant(Expr e) { + e instanceof PatternExpr + or + exists(Expr parent | + isPatternExprDescendant(parent) and + e = parent.getAChildExpr() + ) +}