diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll index 414cfc2d50ae..3427673cdf87 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,21 @@ private predicate typePattern(PatternMatch pm, TypePatternExpr tpe, Type t) { t = pm.getExpr().getType() } +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 +434,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 6be79a17be25..01251d839526 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll @@ -96,7 +96,15 @@ private Element getAChild(Element p) { /** An AST node. */ class AstNode extends Element, TAstNode { - AstNode() { this = getAChild*(any(@top_level_exprorstmt_parent p | not p instanceof Attribute)) } + AstNode() { + exists(@top_level_exprorstmt_parent p | not p instanceof Attribute | + this = p + or + this = getAChild(p) + ) + or + this = getAChild(any(AstNode parent)) + } 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 1b3de63495f0..176e8f3191de 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,15 @@ predicate storeStep(Node node1, ContentSet c, Node node2) { storeStepDelegateCall(node1, c, node2) } +private predicate isAssignExprLValue(Expr e) { + e = any(AssignExpr ae).getLValue() + or + exists(Expr parent | + isAssignExprLValue(parent) and + e = parent.getAChildExpr() + ) +} + private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration { ReadStepConfiguration() { this = "ReadStepConfiguration" } @@ -2432,7 +2442,7 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration scope = any(AssignExpr ae | ae = defTo.(AssignableDefinitions::TupleAssignmentDefinition).getAssignment() and - e = ae.getLValue().getAChildExpr*().(TupleExpr) and + isAssignExprLValue(e.(TupleExpr)) and exactScope = false and isSuccessor = true ) @@ -2488,7 +2498,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 + isPatternExprAncestor(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 ada010652588..e2dedf350555 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,23 @@ class Expr extends ControlFlowElement, @expr { /** Gets the enclosing callable of this expression, if any. */ override Callable getEnclosingCallable() { enclosingCallable(this, result) } + private predicate isExpandedAssignmentRValue() { + this = + any(AssignOperation op).getExpandedAssignment().getRValue().getChildExpr(0).getAChildExpr() + or + exists(Expr parent | + parent.isExpandedAssignmentRValue() 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.isExpandedAssignmentRValue() } /** @@ -1133,7 +1143,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 isPatternExprAncestor(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 000000000000..44fdd5cdd524 --- /dev/null +++ b/csharp/ql/lib/semmle/code/csharp/exprs/internal/Expr.qll @@ -0,0 +1,10 @@ +private import csharp + +predicate isPatternExprAncestor(Expr e) { + e instanceof PatternExpr + or + exists(Expr parent | + isPatternExprAncestor(parent) and + e = parent.getAChildExpr() + ) +}