From cb0b639b856a877fe318ec2bde81338016aca2a0 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 25 Oct 2024 13:02:21 +0200 Subject: [PATCH] C#: Simplify delegate read and store steps (remove dependency on parameter). --- .../dataflow/internal/DataFlowPrivate.qll | 69 +++++-------------- .../dataflow/internal/DataFlowPublic.qll | 59 ++++------------ .../modelgenerator/internal/CaptureModels.qll | 17 +++-- 3 files changed, 42 insertions(+), 103 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 8660bb871e135..ff3531e677ded 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -1148,17 +1148,10 @@ private module Cached { p.getCallable() instanceof PrimaryConstructor } or TCapturedVariableContent(VariableCapture::CapturedVariable v) or - TDelegateCallArgumentContent(Parameter p, int i) { - i = - [0 .. p.getType() - .getUnboundDeclaration() - .(SystemLinqExpressions::DelegateExtType) - .getDelegateType() - .getNumberOfParameters() - 1] + TDelegateCallArgumentContent(int i) { + i = [0 .. max(int j | j = any(DelegateCall dc).getNumberOfArguments())] } or - TDelegateCallReturnContent(Parameter p) { - p.getType().getUnboundDeclaration() instanceof SystemLinqExpressions::DelegateExtType - } + TDelegateCallReturnContent() cached newtype TContentSet = @@ -1175,12 +1168,8 @@ private module Cached { firstChar = approximatePrimaryConstructorParameterContent(_) } or TCapturedVariableContentApprox(VariableCapture::CapturedVariable v) or - TDelegateCallArgumentApproxContent(string firstChar) { - firstChar = approximateDelegateCallArgumentContent(_) - } or - TDelegateCallReturnApproxContent(string firstChar) { - firstChar = approximateDelegateCallReturnContent(_) - } + TDelegateCallArgumentApproxContent() or + TDelegateCallReturnApproxContent() pragma[nomagic] private predicate commonSubTypeGeneral(DataFlowTypeOrUnifiable t1, RelevantGvnType t2) { @@ -2296,14 +2285,13 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) { * the content set `c` of a delegate call. * * If there is a delegate call f(x), then we store "x" on "f" - * using a delegate parameter content set. + * using a delegate argument content set. */ private predicate storeStepDelegateCall(Node node1, ContentSet c, Node node2) { - exists(DelegateCall call, Parameter p, int i | + exists(DelegateCall call, int i | node1.asExpr() = call.getArgument(i) and node2.(PostUpdateNode).getPreUpdateNode().asExpr() = call.getExpr() and - call.getExpr() = p.getAnAccess() and - c.isDelegateCallArgument(p, i) + c.isDelegateCallArgument(i) ) } @@ -2465,15 +2453,14 @@ private predicate readContentStep(Node node1, Content c, Node node2) { * Holds if data can flow from `node1` to `node2` via an assignment to * the content set `c` of a delegate call. * - * If there is a delegate call f(x), then we read the result of the delegate + * If there is a delegate call f(x), then we read the return of the delegate * call. */ private predicate readStepDelegateCall(Node node1, ContentSet c, Node node2) { - exists(DelegateCall call, Parameter p | + exists(DelegateCall call | node1.asExpr() = call.getExpr() and node2.asExpr() = call and - call.getExpr() = p.getAnAccess() and - c.isDelegateCallReturn(p) + c.isDelegateCallReturn() ) } @@ -3092,15 +3079,11 @@ class ContentApprox extends TContentApprox { this = TCapturedVariableContentApprox(v) and result = "captured " + v ) or - exists(string firstChar | - this = TDelegateCallArgumentApproxContent(firstChar) and - result = "approximated delegate call argument " + firstChar - ) + this = TDelegateCallArgumentApproxContent() and + result = "approximated delegate call argument" or - exists(string firstChar | - this = TDelegateCallReturnApproxContent(firstChar) and - result = "approximated delegate call return " + firstChar - ) + this = TDelegateCallReturnApproxContent() and + result = "approximated delegate call return" } } @@ -3122,22 +3105,6 @@ private string approximatePrimaryConstructorParameterContent(PrimaryConstructorP result = pc.getParameter().getName().prefix(1) } -private string getApproximateParameterName(Parameter p) { - exists(string name | name = p.getName() | - name = "" and result = "" - or - result = name.prefix(1) - ) -} - -private string approximateDelegateCallArgumentContent(DelegateCallArgumentContent dc) { - result = getApproximateParameterName(dc.getParameter()) -} - -private string approximateDelegateCallReturnContent(DelegateCallReturnContent dc) { - result = getApproximateParameterName(dc.getParameter()) -} - /** Gets an approximated value for content `c`. */ pragma[nomagic] ContentApprox getContentApprox(Content c) { @@ -3154,9 +3121,11 @@ ContentApprox getContentApprox(Content c) { or result = TCapturedVariableContentApprox(VariableCapture::getCapturedVariableContent(c)) or - result = TDelegateCallArgumentApproxContent(approximateDelegateCallArgumentContent(c)) + c instanceof DelegateCallArgumentContent and + result = TDelegateCallArgumentApproxContent() or - result = TDelegateCallReturnApproxContent(approximateDelegateCallReturnContent(c)) + c instanceof DelegateCallReturnContent and + result = TDelegateCallReturnApproxContent() } /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll index c0f0f3a08fd88..c5460652746bd 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll @@ -240,58 +240,27 @@ class PropertyContent extends Content, TPropertyContent { } /** - * A reference to the index of an argument of a delegate call - * (where the delegate is a parameter) + * A reference to the index of an argument of a delegate call. */ class DelegateCallArgumentContent extends Content, TDelegateCallArgumentContent { - private Parameter p; private int i; - DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(p, i) } + DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(i) } - /** Gets the underlying parameter. */ - Parameter getParameter() { result = p } + override string toString() { result = "delegate argument at position " + i } - /** - * Gets the type of the `i`th parameter of the underlying parameter `p` - * (which is of delegate type). - */ - Type getType() { - result = - p.getType() - .(SystemLinqExpressions::DelegateExtType) - .getDelegateType() - .getParameter(i) - .getType() - } - - override string toString() { result = "delegate parameter " + p.getName() + " at position " + i } - - override Location getLocation() { result = p.getLocation() } + override Location getLocation() { result instanceof EmptyLocation } } /** - * A reference to the return of a delegate call - * (where the delegate is a parameter) + * A reference to the return of a delegate call. */ class DelegateCallReturnContent extends Content, TDelegateCallReturnContent { - private Parameter p; + DelegateCallReturnContent() { this = TDelegateCallReturnContent() } - DelegateCallReturnContent() { this = TDelegateCallReturnContent(p) } + override string toString() { result = "delegate return" } - /** Gets the underlying parameter. */ - Parameter getParameter() { result = p } - - /** - * Gets the return type of the underlying parameter `p` (which is of delegate type). - */ - Type getType() { - result = p.getType().(SystemLinqExpressions::DelegateExtType).getDelegateType().getReturnType() - } - - override string toString() { result = "delegate parameter " + p.getName() + " result" } - - override Location getLocation() { result = p.getLocation() } + override Location getLocation() { result instanceof EmptyLocation } } /** @@ -356,18 +325,14 @@ class ContentSet extends TContentSet { predicate isProperty(Property p) { this = TPropertyContentSet(p) } /** - * Holds if this content set represents the `i`th argument of - * the parameter `p` of delegate type in a delegate call. + * Holds if this content set represents the `i`th argument of a delegate call. */ - predicate isDelegateCallArgument(Parameter p, int i) { - this.isSingleton(TDelegateCallArgumentContent(p, i)) - } + predicate isDelegateCallArgument(int i) { this.isSingleton(TDelegateCallArgumentContent(i)) } /** - * Holds if this content set represents the return of a delegate call - * of parameter `p` (which is of delegate type). + * Holds if this content set represents the return of a delegate call. */ - predicate isDelegateCallReturn(Parameter p) { this.isSingleton(TDelegateCallReturnContent(p)) } + predicate isDelegateCallReturn() { this.isSingleton(TDelegateCallReturnContent()) } /** Holds if this content set represents the field `f`. */ predicate isField(Field f) { this.isSingleton(TFieldContent(f)) } diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll index e815b38ef9e32..7f5598f2506d2 100644 --- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll +++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll @@ -176,10 +176,15 @@ module ModelGeneratorInput implements ModelGeneratorInputSig