Skip to content

Commit 56697bf

Browse files
michaelnebelmichaelnebel
committed
C#: Extract compound operator assignments as is.
1 parent 21f2c81 commit 56697bf

File tree

85 files changed

+986
-1244
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+986
-1244
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Assignment.cs‎

Lines changed: 5 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,12 @@ public static Assignment Create(ExpressionNodeInfo info)
2222

2323
protected override void PopulateExpression(TextWriter trapFile)
2424
{
25-
var operatorKind = OperatorKind;
26-
if (operatorKind.HasValue)
27-
{
28-
// Convert assignment such as `a += b` into `a = a + b`.
29-
var simpleAssignExpr = new Expression(new ExpressionInfo(Context, Type, Location, ExprKind.SIMPLE_ASSIGN, this, 2, isCompilerGenerated: true, null));
30-
Create(Context, Syntax.Left, simpleAssignExpr, 1);
31-
var opexpr = new Expression(new ExpressionInfo(Context, Type, Location, operatorKind.Value, simpleAssignExpr, 0, isCompilerGenerated: true, null));
32-
Create(Context, Syntax.Left, opexpr, 0, isCompilerGenerated: true);
33-
Create(Context, Syntax.Right, opexpr, 1);
34-
opexpr.OperatorCall(trapFile, Syntax);
35-
}
36-
else
37-
{
38-
Create(Context, Syntax.Left, this, 1);
39-
Create(Context, Syntax.Right, this, 0);
25+
Create(Context, Syntax.Left, this, 0);
26+
Create(Context, Syntax.Right, this, 1);
4027

41-
if (Kind == ExprKind.ADD_EVENT || Kind == ExprKind.REMOVE_EVENT)
42-
{
43-
OperatorCall(trapFile, Syntax);
44-
}
28+
if (Kind != ExprKind.SIMPLE_ASSIGN && Kind != ExprKind.ASSIGN_COALESCE)
29+
{
30+
OperatorCall(trapFile, Syntax);
4531
}
4632
}
4733

@@ -108,56 +94,5 @@ private static ExprKind GetKind(Context cx, AssignmentExpressionSyntax syntax)
10894

10995
return kind;
11096
}
111-
112-
/// <summary>
113-
/// Gets the kind of this assignment operator (<code>null</code> if the
114-
/// assignment is not an assignment operator). For example, the operator
115-
/// kind of `*=` is `*`.
116-
/// </summary>
117-
private ExprKind? OperatorKind
118-
{
119-
get
120-
{
121-
var kind = Kind;
122-
if (kind == ExprKind.REMOVE_EVENT || kind == ExprKind.ADD_EVENT || kind == ExprKind.SIMPLE_ASSIGN)
123-
return null;
124-
125-
if (CallType.AdjustKind(kind) == ExprKind.OPERATOR_INVOCATION)
126-
return ExprKind.OPERATOR_INVOCATION;
127-
128-
switch (kind)
129-
{
130-
case ExprKind.ASSIGN_ADD:
131-
return ExprKind.ADD;
132-
case ExprKind.ASSIGN_AND:
133-
return ExprKind.BIT_AND;
134-
case ExprKind.ASSIGN_DIV:
135-
return ExprKind.DIV;
136-
case ExprKind.ASSIGN_LSHIFT:
137-
return ExprKind.LSHIFT;
138-
case ExprKind.ASSIGN_MUL:
139-
return ExprKind.MUL;
140-
case ExprKind.ASSIGN_OR:
141-
return ExprKind.BIT_OR;
142-
case ExprKind.ASSIGN_REM:
143-
return ExprKind.REM;
144-
case ExprKind.ASSIGN_RSHIFT:
145-
return ExprKind.RSHIFT;
146-
case ExprKind.ASSIGN_URSHIFT:
147-
return ExprKind.URSHIFT;
148-
case ExprKind.ASSIGN_SUB:
149-
return ExprKind.SUB;
150-
case ExprKind.ASSIGN_XOR:
151-
return ExprKind.BIT_XOR;
152-
case ExprKind.ASSIGN_COALESCE:
153-
return ExprKind.NULL_COALESCING;
154-
default:
155-
Context.ModelError(Syntax, $"Couldn't unfold assignment of type {kind}");
156-
return ExprKind.UNKNOWN;
157-
}
158-
}
159-
}
160-
161-
public new CallType CallType => GetCallType(Context, Syntax);
16297
}
16398
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Initializer.cs‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,30 +83,31 @@ protected override void PopulateExpression(TextWriter trapFile)
8383
{
8484
var assignmentInfo = new ExpressionNodeInfo(Context, init, this, child++).SetKind(ExprKind.SIMPLE_ASSIGN);
8585
var assignmentEntity = new Expression(assignmentInfo);
86-
var typeInfoRight = Context.GetTypeInfo(assignment.Right);
87-
if (typeInfoRight.Type is null)
88-
// The type may be null for nested initializers such as
89-
// ```csharp
90-
// new ClassWithArrayField() { As = { [0] = a } }
91-
// ```
92-
// In this case we take the type from the assignment
93-
// `As = { [0] = a }` instead
94-
typeInfoRight = assignmentInfo.TypeInfo;
95-
CreateFromNode(new ExpressionNodeInfo(Context, assignment.Right, assignmentEntity, 0, typeInfoRight));
96-
9786
var target = Context.GetSymbolInfo(assignment.Left);
9887

9988
// If the target is null, then assume that this is an array initializer (of the form `[...] = ...`)
100-
10189
var access = target.Symbol is null ?
102-
new Expression(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 1).SetKind(ExprKind.ARRAY_ACCESS)) :
103-
Access.Create(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 1), target.Symbol, false, Context.CreateEntity(target.Symbol));
90+
new Expression(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 0).SetKind(ExprKind.ARRAY_ACCESS)) :
91+
Access.Create(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 0), target.Symbol, false, Context.CreateEntity(target.Symbol));
10492

10593
if (assignment.Left is ImplicitElementAccessSyntax iea)
10694
{
10795
// An array/indexer initializer of the form `[...] = ...`
10896
access.PopulateArguments(trapFile, iea.ArgumentList.Arguments, 0);
10997
}
98+
99+
var typeInfoRight = Context.GetTypeInfo(assignment.Right);
100+
if (typeInfoRight.Type is null)
101+
{
102+
// The type may be null for nested initializers such as
103+
// ```csharp
104+
// new ClassWithArrayField() { As = { [0] = a } }
105+
// ```
106+
// In this case we take the type from the assignment
107+
// `As = { [0] = a }` instead
108+
typeInfoRight = assignmentInfo.TypeInfo;
109+
}
110+
CreateFromNode(new ExpressionNodeInfo(Context, assignment.Right, assignmentEntity, 1, typeInfoRight));
110111
}
111112
else
112113
{

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation/AnonymousObjectCreation.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ protected override void PopulateExpression(TextWriter trapFile)
4141
var loc = Context.CreateLocation(init.GetLocation());
4242

4343
var assignment = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.SIMPLE_ASSIGN, objectInitializer, child++, isCompilerGenerated: false, null));
44-
Create(Context, init.Expression, assignment, 0);
4544
Property.Create(Context, property);
46-
47-
var access = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.PROPERTY_ACCESS, assignment, 1, isCompilerGenerated: false, null));
45+
var access = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.PROPERTY_ACCESS, assignment, 0, isCompilerGenerated: false, null));
4846
trapFile.expr_access(access, propEntity);
47+
48+
Create(Context, init.Expression, assignment, 1);
4949
}
5050
}
5151
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Query.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,12 @@ protected Expression DeclareRangeVariable(Context cx, IExpressionParentEntity pa
9494
child
9595
);
9696

97-
Expression.Create(cx, Expr, decl, 0);
98-
9997
var nameLoc = cx.CreateLocation(name.GetLocation());
100-
var access = new Expression(new ExpressionInfo(cx, type, nameLoc, ExprKind.LOCAL_VARIABLE_ACCESS, decl, 1, isCompilerGenerated: false, null));
98+
var access = new Expression(new ExpressionInfo(cx, type, nameLoc, ExprKind.LOCAL_VARIABLE_ACCESS, decl, 0, isCompilerGenerated: false, null));
10199
cx.TrapWriter.Writer.expr_access(access, LocalVariable.Create(cx, variableSymbol));
102100

101+
Expression.Create(cx, Expr, decl, 1);
102+
103103
return decl;
104104
}
105105

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/VariableDeclaration.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,11 +176,11 @@ public static VariableDeclaration CreateDeclarator(Context cx, VariableDeclarato
176176

177177
if (d.Initializer is not null)
178178
{
179-
Create(cx, d.Initializer.Value, ret, 0);
180-
181179
// Create an access
182-
var access = new Expression(new ExpressionInfo(cx, type, localVar.Location, ExprKind.LOCAL_VARIABLE_ACCESS, ret, 1, isCompilerGenerated: false, null));
180+
var access = new Expression(new ExpressionInfo(cx, type, localVar.Location, ExprKind.LOCAL_VARIABLE_ACCESS, ret, 0, isCompilerGenerated: false, null));
183181
cx.TrapWriter.Writer.expr_access(access, localVar);
182+
183+
Create(cx, d.Initializer.Value, ret, 1);
184184
}
185185

186186
if (d.Parent is VariableDeclarationSyntax decl)

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,9 @@ private Expression AddInitializerAssignment(TextWriter trapFile, ExpressionSynta
116116
{
117117
var type = Symbol.GetAnnotatedType();
118118
var simpleAssignExpr = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.SIMPLE_ASSIGN, this, child++, isCompilerGenerated: true, constValue));
119-
Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer, simpleAssignExpr, 0));
120-
var access = new Expression(new ExpressionInfo(Context, type, Location, ExprKind.FIELD_ACCESS, simpleAssignExpr, 1, isCompilerGenerated: true, constValue));
119+
var access = new Expression(new ExpressionInfo(Context, type, Location, ExprKind.FIELD_ACCESS, simpleAssignExpr, 0, isCompilerGenerated: true, constValue));
121120
trapFile.expr_access(access, this);
121+
Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer, simpleAssignExpr, 1));
122122
return access;
123123
}
124124

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ public override void Populate(TextWriter trapFile)
9494
var loc = Context.CreateLocation(initializer!.GetLocation());
9595
var annotatedType = AnnotatedTypeSymbol.CreateNotAnnotated(Symbol.Type);
9696
var simpleAssignExpr = new Expression(new ExpressionInfo(Context, annotatedType, loc, ExprKind.SIMPLE_ASSIGN, this, child++, isCompilerGenerated: true, null));
97-
Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer.Value, simpleAssignExpr, 0));
98-
var access = new Expression(new ExpressionInfo(Context, annotatedType, Location, ExprKind.PROPERTY_ACCESS, simpleAssignExpr, 1, isCompilerGenerated: true, null));
97+
var access = new Expression(new ExpressionInfo(Context, annotatedType, Location, ExprKind.PROPERTY_ACCESS, simpleAssignExpr, 0, isCompilerGenerated: true, null));
9998
trapFile.expr_access(access, this);
99+
Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer.Value, simpleAssignExpr, 1));
100100
if (!Symbol.IsStatic)
101101
{
102102
This.CreateImplicit(Context, Symbol.ContainingType, Location, access, -1);

‎csharp/ql/lib/experimental/code/csharp/Cryptography/NonCryptographicHashes.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ private predicate maybeUsedInElfHashFunction(Variable v, Operation xor, Operatio
4848
Expr e1, Expr e2, AssignExpr addAssign, AssignExpr xorAssign, Operation notOp,
4949
AssignExpr notAssign
5050
|
51-
(add instanceof AddExpr or add instanceof AssignAddExpr) and
51+
add instanceof AddOperation and
5252
e1.getAChild*() = add.getAnOperand() and
5353
e1 instanceof BinaryBitwiseOperation and
5454
e2 = e1.(BinaryBitwiseOperation).getLeftOperand() and

‎csharp/ql/lib/semmle/code/csharp/Assignable.qll‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ class AssignableRead extends AssignableAccess {
7878
this.isRefArgument()
7979
or
8080
this = any(AssignableDefinitions::AddressOfDefinition def).getTargetAccess()
81+
or
82+
this = any(AssignableDefinitions::AssignOperationDefinition def).getTargetAccess()
8183
) and
8284
not nameOfChild(_, this)
8385
}
@@ -271,6 +273,8 @@ module AssignableInternal {
271273
def = TAddressOfDefinition(result)
272274
or
273275
def = TPatternDefinition(result)
276+
or
277+
def = TAssignOperationDefinition(result)
274278
}
275279

276280
/** A local variable declaration at the top-level of a pattern. */
@@ -286,7 +290,11 @@ module AssignableInternal {
286290
private module Cached {
287291
cached
288292
newtype TAssignableDefinition =
289-
TAssignmentDefinition(Assignment a) { not a.getLValue() instanceof TupleExpr } or
293+
TAssignmentDefinition(Assignment a) {
294+
not a.getLValue() instanceof TupleExpr and
295+
not a instanceof AssignCompoundOperation and
296+
not a instanceof AssignCoalesceExpr
297+
} or
290298
TTupleAssignmentDefinition(AssignExpr ae, Expr leaf) { tupleAssignmentDefinition(ae, leaf) } or
291299
TOutRefDefinition(AssignableAccess aa) {
292300
aa.isOutArgument()
@@ -309,7 +317,11 @@ module AssignableInternal {
309317
)
310318
} or
311319
TAddressOfDefinition(AddressOfExpr aoe) or
312-
TPatternDefinition(TopLevelPatternDecl tlpd)
320+
TPatternDefinition(TopLevelPatternDecl tlpd) or
321+
TAssignOperationDefinition(AssignOperation ao) {
322+
ao instanceof AssignCompoundOperation or
323+
ao instanceof AssignCoalesceExpr
324+
}
313325

314326
/**
315327
* Gets the source expression assigned in tuple definition `def`, if any.
@@ -355,6 +367,8 @@ module AssignableInternal {
355367
def = TMutationDefinition(any(MutatorOperation mo | mo.getOperand() = result))
356368
or
357369
def = TAddressOfDefinition(any(AddressOfExpr aoe | aoe.getOperand() = result))
370+
or
371+
def = TAssignOperationDefinition(any(AssignOperation ao | ao.getLeftOperand() = result))
358372
}
359373

360374
/**
@@ -369,8 +383,10 @@ module AssignableInternal {
369383
or
370384
exists(Assignment ass | ac = ass.getLValue() |
371385
result = ass.getRValue() and
372-
not ass.(AssignOperation).hasExpandedAssignment()
386+
not ass instanceof AssignOperation
373387
)
388+
or
389+
exists(AssignOperation ao | ac = ao.getLeftOperand() | result = ao)
374390
}
375391
}
376392

@@ -388,8 +404,9 @@ private import AssignableInternal
388404
* a mutation update (`AssignableDefinitions::MutationDefinition`), a local variable
389405
* declaration without an initializer (`AssignableDefinitions::LocalVariableDefinition`),
390406
* an implicit parameter definition (`AssignableDefinitions::ImplicitParameterDefinition`),
391-
* an address-of definition (`AssignableDefinitions::AddressOfDefinition`), or a pattern
392-
* definition (`AssignableDefinitions::PatternDefinition`).
407+
* an address-of definition (`AssignableDefinitions::AddressOfDefinition`), a pattern
408+
* definition (`AssignableDefinitions::PatternDefinition`), or a compound assignment
409+
* operation definition (`AssignableDefinitions::AssignOperationDefinition`)
393410
*/
394411
class AssignableDefinition extends TAssignableDefinition {
395412
/**
@@ -511,7 +528,7 @@ module AssignableDefinitions {
511528

512529
override Expr getSource() {
513530
result = a.getRValue() and
514-
not a instanceof AssignOperation
531+
not a instanceof AddOrRemoveEventExpr
515532
}
516533

517534
override string toString() { result = a.toString() }
@@ -735,4 +752,17 @@ module AssignableDefinitions {
735752
/** Gets the assignable (field or property) being initialized. */
736753
Assignable getAssignable() { result = fieldOrProp }
737754
}
755+
756+
/**
757+
* A definition by a compound assignment operation, for example `x += y`.
758+
*/
759+
class AssignOperationDefinition extends AssignableDefinition, TAssignOperationDefinition {
760+
AssignOperation ao;
761+
762+
AssignOperationDefinition() { this = TAssignOperationDefinition(ao) }
763+
764+
override Expr getSource() { result = ao }
765+
766+
override string toString() { result = ao.toString() }
767+
}
738768
}

‎csharp/ql/lib/semmle/code/csharp/ExprOrStmtParent.qll‎

Lines changed: 3 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class ExprOrStmtParent extends Element, @exprorstmt_parent {
2020

2121
/** Gets the `i`th child expression of this element (zero-based). */
2222
final Expr getChildExpr(int i) {
23-
expr_parent_adjusted(result, i, this) or
23+
expr_parent(result, i, this) or
2424
expr_parent_top_level_adjusted(result, i, this)
2525
}
2626

@@ -119,66 +119,14 @@ private module Cached {
119119
}
120120

121121
/**
122-
* The `expr_parent()` relation adjusted for expandable assignments. For example,
123-
* the assignment `x += y` is extracted as
124-
*
125-
* ```
126-
* +=
127-
* |
128-
* 2
129-
* |
130-
* =
131-
* / \
132-
* 1 0
133-
* / \
134-
* x +
135-
* / \
136-
* 1 0
137-
* / \
138-
* x y
139-
* ```
140-
*
141-
* in order to be able to retrieve the expanded assignment `x = x + y` as the 2nd
142-
* child. This predicate changes the diagram above into
143-
*
144-
* ```
145-
* +=
146-
* / \
147-
* 1 0
148-
* / \
149-
* x y
150-
* ```
122+
* Use `expr_parent` instead.
151123
*/
152124
cached
153-
predicate expr_parent_adjusted(Expr child, int i, ControlFlowElement parent) {
154-
if parent instanceof AssignOperation
155-
then
156-
parent =
157-
any(AssignOperation ao |
158-
exists(AssignExpr ae | ae = ao.getExpandedAssignment() |
159-
i = 0 and
160-
exists(Expr right |
161-
// right = `x + y`
162-
expr_parent(right, 0, ae)
163-
|
164-
expr_parent(child, 1, right)
165-
)
166-
or
167-
i = 1 and
168-
expr_parent(child, 1, ae)
169-
)
170-
or
171-
not ao.hasExpandedAssignment() and
172-
expr_parent(child, i, parent)
173-
)
174-
else expr_parent(child, i, parent)
175-
}
125+
deprecated predicate expr_parent_adjusted(Expr child, int i, ControlFlowElement parent) { none() }
176126

177127
private Expr getAChildExpr(ExprOrStmtParent parent) {
178128
result = parent.getAChildExpr() and
179129
not result = parent.(DeclarationWithGetSetAccessors).getExpressionBody()
180-
or
181-
result = parent.(AssignOperation).getExpandedAssignment()
182130
}
183131

184132
private ControlFlowElement getAChild(ExprOrStmtParent parent) {

0 commit comments

Comments
 (0)