hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 03:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

C#: Improve AccessorCall::getArgument()

Browse Source 
- Handle tuple assignments.
- Handle compound `+=` assignments.
This commit is contained in:
Tom Hvitved
2018-11-07 10:34:10 +01:00
parent 7423916214
commit 5d8162cc8b
5 changed files with 146 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

251
csharp/ql/src/semmle/code/csharp/Assignable.qll
View File

@@ -183,54 +183,16 @@ class AssignableWrite extends AssignableAccess {
}
}
private cached module AssignableDefinitionImpl {
cached newtype TAssignableDefinition =
TAssignmentDefinition(Assignment a) {
not a.getLValue() instanceof TupleExpr
}
or
TTupleAssignmentDefinition(AssignExpr ae, Expr leaf) {
exists(TupleExpr te |
ae.getLValue() = te and
te.getAnArgument+() = leaf and
// `leaf` is either an assignable access or a local variable declaration
not leaf instanceof TupleExpr
)
}
or
TOutRefDefinition(AssignableAccess aa) {
aa.isOutArgument()
or
isRelevantRefCall(_, aa)
}
or
TMutationDefinition(MutatorOperation mo)
or
TLocalVariableDefinition(LocalVariableDeclExpr lvde) {
not lvde.hasInitializer() and
not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
not lvde = any(IsPatternExpr ipe).getVariableDeclExpr() and
not lvde = any(TypeCase tc).getVariableDeclExpr()
}
or
TImplicitParameterDefinition(Parameter p) {
exists(Callable c |
p = c.getAParameter() |
c.hasBody() or
c.(Constructor).hasInitializer()
)
}
or
TAddressOfDefinition(AddressOfExpr aoe)
or
TIsPatternDefinition(IsPatternExpr ipe)
or
TTypeCasePatternDefinition(TypeCase tc)
or
TInitializer(Assignable a, Expr e) {
e = a.(Field).getInitializer() or
e = a.(Property).getInitializer()
}
/** INTERNAL: Do not use. */
module AssignableInternal {
private predicate tupleAssignmentDefinition(AssignExpr ae, Expr leaf) {
exists(TupleExpr te |
ae.getLValue() = te and
te.getAnArgument+() = leaf and
// `leaf` is either an assignable access or a local variable declaration
not leaf instanceof TupleExpr
)
}
/**
* Holds if `ae` is a tuple assignment, and `left` is a sub expression
@@ -238,7 +200,7 @@ private cached module AssignableDefinitionImpl {
* right-hand side `right`.
*/
private predicate tupleAssignmentPair(AssignExpr ae, Expr left, Expr right) {
exists(TTupleAssignmentDefinition(ae, _)) and
tupleAssignmentDefinition(ae, _) and
left = ae.getLValue() and
right = ae.getRValue()
or
@@ -249,16 +211,6 @@ private cached module AssignableDefinitionImpl {
)
}
/**
* Gets the source expression assigned in tuple definition `def`, if any.
*/
cached Expr getTupleSource(TTupleAssignmentDefinition def) {
exists(AssignExpr ae, Expr leaf |
def = TTupleAssignmentDefinition(ae, leaf) |
tupleAssignmentPair(ae, leaf, result)
)
}
/**
* Holds if the `ref` assignment to `aa` via call `c` is relevant.
*/
@@ -341,19 +293,6 @@ private cached module AssignableDefinitionImpl {
not result = TImplicitParameterDefinition(_)
}
/**
* Holds if the `ref` assignment to `aa` via call `c` is uncertain.
*/
cached predicate isUncertainRefCall(Call c, AssignableAccess aa) {
isRelevantRefCall(c, aa)
and
exists(ControlFlow::BasicBlock bb, Parameter p |
isAnalyzableRefCall(c, aa, p) |
parameterReachesWithoutDef(p, bb) and
bb.getLastNode() = p.getCallable().getExitPoint()
)
}
/**
* Holds if `p` is an analyzable `ref` parameter and there is a path from the
* entry point of `p`'s callable to basic block `bb` without passing through
@@ -378,51 +317,137 @@ private cached module AssignableDefinitionImpl {
bb.getANode() = getAnAnalyzableRefDef(_, _, p).getAControlFlowNode()
}
// Not defined by dispatch in order to avoid too conservative negative recursion error
cached Assignable getTarget(AssignableDefinition def) {
result = def.getTargetAccess().getTarget()
or
exists(Expr leaf |
def = TTupleAssignmentDefinition(_, leaf) |
result = leaf.(LocalVariableDeclExpr).getVariable()
)
or
def = any(AssignableDefinitions::ImplicitParameterDefinition p |
result = p.getParameter()
)
or
def = any(AssignableDefinitions::LocalVariableDefinition decl |
result = decl.getDeclaration().getVariable()
)
or
def = any(AssignableDefinitions::IsPatternDefinition is |
result = is.getDeclaration().getVariable()
)
or
def = any(AssignableDefinitions::TypeCasePatternDefinition case |
result = case.getDeclaration().getVariable()
)
or
def = any(AssignableDefinitions::InitializerDefinition init |
result = init.getAssignable()
)
}
private cached module Cached {
cached newtype TAssignableDefinition =
TAssignmentDefinition(Assignment a) {
not a.getLValue() instanceof TupleExpr
}
or
TTupleAssignmentDefinition(AssignExpr ae, Expr leaf) {
tupleAssignmentDefinition(ae, leaf)
}
or
TOutRefDefinition(AssignableAccess aa) {
aa.isOutArgument()
or
isRelevantRefCall(_, aa)
}
or
TMutationDefinition(MutatorOperation mo)
or
TLocalVariableDefinition(LocalVariableDeclExpr lvde) {
not lvde.hasInitializer() and
not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
not lvde = any(IsPatternExpr ipe).getVariableDeclExpr() and
not lvde = any(TypeCase tc).getVariableDeclExpr()
}
or
TImplicitParameterDefinition(Parameter p) {
exists(Callable c |
p = c.getAParameter() |
c.hasBody() or
c.(Constructor).hasInitializer()
)
}
or
TAddressOfDefinition(AddressOfExpr aoe)
or
TIsPatternDefinition(IsPatternExpr ipe)
or
TTypeCasePatternDefinition(TypeCase tc)
or
TInitializer(Assignable a, Expr e) {
e = a.(Field).getInitializer() or
e = a.(Property).getInitializer()
}
// Not defined by dispatch in order to avoid too conservative negative recursion error
cached AssignableAccess getTargetAccess(AssignableDefinition def) {
def = TAssignmentDefinition(any(Assignment a | a.getLValue() = result))
or
def = TTupleAssignmentDefinition(_, result)
or
def = TOutRefDefinition(result)
or
def = TMutationDefinition(any(MutatorOperation mo | mo.getOperand() = result))
or
def = TAddressOfDefinition(any(AddressOfExpr aoe | aoe.getOperand() = result))
/**
* Gets the source expression assigned in tuple definition `def`, if any.
*/
cached Expr getTupleSource(TTupleAssignmentDefinition def) {
exists(AssignExpr ae, Expr leaf |
def = TTupleAssignmentDefinition(ae, leaf) |
tupleAssignmentPair(ae, leaf, result)
)
}
/**
* Holds if the `ref` assignment to `aa` via call `c` is uncertain.
*/
cached predicate isUncertainRefCall(Call c, AssignableAccess aa) {
isRelevantRefCall(c, aa)
and
exists(ControlFlow::BasicBlock bb, Parameter p |
isAnalyzableRefCall(c, aa, p) |
parameterReachesWithoutDef(p, bb) and
bb.getLastNode() = p.getCallable().getExitPoint()
)
}
// Not defined by dispatch in order to avoid too conservative negative recursion error
cached Assignable getTarget(AssignableDefinition def) {
result = def.getTargetAccess().getTarget()
or
exists(Expr leaf |
def = TTupleAssignmentDefinition(_, leaf) |
result = leaf.(LocalVariableDeclExpr).getVariable()
)
or
def = any(AssignableDefinitions::ImplicitParameterDefinition p |
result = p.getParameter()
)
or
def = any(AssignableDefinitions::LocalVariableDefinition decl |
result = decl.getDeclaration().getVariable()
)
or
def = any(AssignableDefinitions::IsPatternDefinition is |
result = is.getDeclaration().getVariable()
)
or
def = any(AssignableDefinitions::TypeCasePatternDefinition case |
result = case.getDeclaration().getVariable()
)
or
def = any(AssignableDefinitions::InitializerDefinition init |
result = init.getAssignable()
)
}
// Not defined by dispatch in order to avoid too conservative negative recursion error
cached AssignableAccess getTargetAccess(AssignableDefinition def) {
def = TAssignmentDefinition(any(Assignment a | a.getLValue() = result))
or
def = TTupleAssignmentDefinition(_, result)
or
def = TOutRefDefinition(result)
or
def = TMutationDefinition(any(MutatorOperation mo | mo.getOperand() = result))
or
def = TAddressOfDefinition(any(AddressOfExpr aoe | aoe.getOperand() = result))
}
/**
* Gets the argument for the implicit `value` parameter in the accessor call
* `ac`, if any.
*/
cached Expr getAccessorCallValueArgument(AccessorCall ac) {
exists(AssignExpr ae |
tupleAssignmentDefinition(ae, ac) |
tupleAssignmentPair(ae, ac, result)
)
or
exists(Assignment a |
ac = a.getLValue() |
result = a.getRValue() and
not a.(AssignOperation).hasExpandedAssignment()
)
}
}
import Cached
}
private import AssignableDefinitionImpl
private import AssignableInternal
/**
* An assignable definition.

6
csharp/ql/src/semmle/code/csharp/exprs/Call.qll
View File

@@ -643,8 +643,7 @@ class PropertyCall extends AccessorCall, PropertyAccessExpr {
override Expr getArgument(int i) {
i = 0 and
this instanceof AssignableWrite and
exists(Assignment a | a.getLValue() = this and result = a.getRValue())
result = AssignableInternal::getAccessorCallValueArgument(this)
}
override string toString() {
@@ -681,9 +680,8 @@ class IndexerCall extends AccessorCall, IndexerAccessExpr {
override Expr getArgument(int i) {
result = this.(ElementAccess).getIndex(i)
or
this instanceof AssignableWrite and
i = count(this.(ElementAccess).getAnIndex()) and
exists(Assignment a | a.getLValue() = this and result = a.getRValue())
result = AssignableInternal::getAccessorCallValueArgument(this)
}
override string toString() { result = IndexerAccessExpr.super.toString() }

4
csharp/ql/test/library-tests/arguments/argumentByName.expected
View File

@@ -21,10 +21,11 @@
| arguments.cs:55:9:55:12 | access to property Prop | arguments.cs:55:16:55:25 | access to indexer | value |
| arguments.cs:55:16:55:25 | access to indexer | arguments.cs:55:21:55:21 | 1 | a |
| arguments.cs:55:16:55:25 | access to indexer | arguments.cs:55:24:55:24 | 2 | b |
| arguments.cs:56:10:56:13 | access to property Prop | arguments.cs:56:31:56:31 | 5 | value |
| arguments.cs:56:16:56:25 | access to indexer | arguments.cs:56:21:56:21 | 3 | a |
| arguments.cs:56:16:56:25 | access to indexer | arguments.cs:56:24:56:24 | 4 | b |
| arguments.cs:56:16:56:25 | access to indexer | arguments.cs:56:34:56:34 | 6 | value |
| arguments.cs:58:9:58:12 | access to property Prop | arguments.cs:58:9:58:17 | ... + ... | value |
| arguments.cs:58:9:58:12 | access to property Prop | arguments.cs:58:17:58:17 | 7 | value |
| arguments.cs:59:9:59:18 | access to indexer | arguments.cs:59:14:59:14 | 8 | a |
| arguments.cs:59:9:59:18 | access to indexer | arguments.cs:59:17:59:17 | 9 | b |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:9:60:26 | ... + ... | value |
@@ -32,6 +33,5 @@
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:14:60:15 | 10 | a |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:18:60:19 | 11 | b |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:18:60:19 | 11 | b |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:25:60:26 | 12 | value |
| arguments.cs:62:16:62:27 | access to indexer | arguments.cs:62:21:62:22 | 15 | a |
| arguments.cs:62:16:62:27 | access to indexer | arguments.cs:62:25:62:26 | 16 | b |

4
csharp/ql/test/library-tests/arguments/argumentByParameter.expected
View File

@@ -21,10 +21,11 @@
| arguments.cs:55:9:55:12 | access to property Prop | arguments.cs:55:16:55:25 | access to indexer | arguments.cs:48:21:48:23 | value |
| arguments.cs:55:16:55:25 | access to indexer | arguments.cs:55:21:55:21 | 1 | arguments.cs:50:18:50:18 | a |
| arguments.cs:55:16:55:25 | access to indexer | arguments.cs:55:24:55:24 | 2 | arguments.cs:50:25:50:25 | b |
| arguments.cs:56:10:56:13 | access to property Prop | arguments.cs:56:31:56:31 | 5 | arguments.cs:48:21:48:23 | value |
| arguments.cs:56:16:56:25 | access to indexer | arguments.cs:56:21:56:21 | 3 | arguments.cs:50:18:50:18 | a |
| arguments.cs:56:16:56:25 | access to indexer | arguments.cs:56:24:56:24 | 4 | arguments.cs:50:25:50:25 | b |
| arguments.cs:56:16:56:25 | access to indexer | arguments.cs:56:34:56:34 | 6 | arguments.cs:50:44:50:46 | value |
| arguments.cs:58:9:58:12 | access to property Prop | arguments.cs:58:9:58:17 | ... + ... | arguments.cs:48:21:48:23 | value |
| arguments.cs:58:9:58:12 | access to property Prop | arguments.cs:58:17:58:17 | 7 | arguments.cs:48:21:48:23 | value |
| arguments.cs:59:9:59:18 | access to indexer | arguments.cs:59:14:59:14 | 8 | arguments.cs:50:18:50:18 | a |
| arguments.cs:59:9:59:18 | access to indexer | arguments.cs:59:14:59:14 | 8 | arguments.cs:50:18:50:18 | a |
| arguments.cs:59:9:59:18 | access to indexer | arguments.cs:59:17:59:17 | 9 | arguments.cs:50:25:50:25 | b |
@@ -34,6 +35,5 @@
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:14:60:15 | 10 | arguments.cs:50:18:50:18 | a |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:18:60:19 | 11 | arguments.cs:50:25:50:25 | b |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:18:60:19 | 11 | arguments.cs:50:25:50:25 | b |
| arguments.cs:60:9:60:20 | access to indexer | arguments.cs:60:25:60:26 | 12 | arguments.cs:50:44:50:46 | value |
| arguments.cs:62:16:62:27 | access to indexer | arguments.cs:62:21:62:22 | 15 | arguments.cs:50:18:50:18 | a |
| arguments.cs:62:16:62:27 | access to indexer | arguments.cs:62:25:62:26 | 16 | arguments.cs:50:25:50:25 | b |

4
csharp/ql/test/library-tests/arguments/parameterGetArguments.expected
View File

@@ -19,8 +19,8 @@
| arguments.cs:33:33:33:36 | args | arguments.cs:40:18:40:35 | array creation of type Int32[] |
| arguments.cs:48:21:48:23 | value | arguments.cs:54:16:54:16 | 0 |
| arguments.cs:48:21:48:23 | value | arguments.cs:55:16:55:25 | access to indexer |
| arguments.cs:48:21:48:23 | value | arguments.cs:56:31:56:31 | 5 |
| arguments.cs:48:21:48:23 | value | arguments.cs:58:9:58:17 | ... + ... |
| arguments.cs:48:21:48:23 | value | arguments.cs:58:17:58:17 | 7 |
| arguments.cs:50:18:50:18 | a | arguments.cs:55:21:55:21 | 1 |
| arguments.cs:50:18:50:18 | a | arguments.cs:56:21:56:21 | 3 |
| arguments.cs:50:18:50:18 | a | arguments.cs:59:14:59:14 | 8 |
@@ -35,5 +35,5 @@
| arguments.cs:50:25:50:25 | b | arguments.cs:60:18:60:19 | 11 |
| arguments.cs:50:25:50:25 | b | arguments.cs:60:18:60:19 | 11 |
| arguments.cs:50:25:50:25 | b | arguments.cs:62:25:62:26 | 16 |
| arguments.cs:50:44:50:46 | value | arguments.cs:56:34:56:34 | 6 |
| arguments.cs:50:44:50:46 | value | arguments.cs:60:9:60:26 | ... + ... |
| arguments.cs:50:44:50:46 | value | arguments.cs:60:25:60:26 | 12 |