Skip to content

Commit 2414019

Browse files
tamasvajktamasvajk
authored
Merge pull request #5242 from tamasvajk/feature/tuple-df
C#: Add tuple dataflow
2 parents 8d6b835 + 2704819 commit 2414019

35 files changed

+1200
-369
lines changed

‎csharp/change-notes/2021-02-26-tuple-dataflow.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Data flow for tuples has been improved to track data flowing into and out of
3+
tuple fields. Tuple extraction was changed to extract non-named tuple elements.

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,22 @@ namespace Semmle.Extraction.CSharp.Entities
1212
internal class Expression : FreshEntity, IExpressionParentEntity
1313
{
1414
private readonly IExpressionInfo info;
15-
public AnnotatedTypeSymbol? Type { get; }
15+
public AnnotatedTypeSymbol? Type { get; private set; }
1616
public Extraction.Entities.Location Location { get; }
1717
public ExprKind Kind { get; }
1818

19-
internal Expression(IExpressionInfo info)
19+
internal Expression(IExpressionInfo info, bool shouldPopulate = true)
2020
: base(info.Context)
2121
{
2222
this.info = info;
2323
Location = info.Location;
2424
Kind = info.Kind;
2525
Type = info.Type;
2626

27-
TryPopulate();
27+
if (shouldPopulate)
28+
{
29+
TryPopulate();
30+
}
2831
}
2932

3033
protected sealed override void Populate(TextWriter trapFile)
@@ -59,6 +62,14 @@ protected sealed override void Populate(TextWriter trapFile)
5962

6063
public override Location? ReportingLocation => Location.Symbol;
6164

65+
internal void SetType(ITypeSymbol? type)
66+
{
67+
if (type is not null)
68+
{
69+
Type = new AnnotatedTypeSymbol(type, type.NullableAnnotation);
70+
}
71+
}
72+
6273
bool IExpressionParentEntity.IsTopLevelParent => false;
6374

6475
/// <summary>

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

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
using Microsoft.CodeAnalysis.CSharp.Syntax;
44
using Semmle.Extraction.CSharp.Populators;
55
using Semmle.Extraction.Kinds;
6-
using Semmle.Extraction.Entities;
6+
using System.Collections.Generic;
7+
using System.Linq;
8+
using System.Collections.Immutable;
79

810
namespace Semmle.Extraction.CSharp.Entities.Expressions
911
{
@@ -47,69 +49,91 @@ private static VariableDeclaration CreateSingle(Context cx, DeclarationExpressio
4749
/// Create a tuple expression representing a parenthesized variable declaration.
4850
/// That is, we consider `var (x, y) = ...` to be equivalent to `(var x, var y) = ...`.
4951
/// </summary>
50-
public static Expression CreateParenthesized(Context cx, DeclarationExpressionSyntax node, ParenthesizedVariableDesignationSyntax designation, IExpressionParentEntity parent, int child)
52+
public static Expression CreateParenthesized(Context cx, DeclarationExpressionSyntax node, ParenthesizedVariableDesignationSyntax designation, IExpressionParentEntity parent, int child, INamedTypeSymbol? t)
5153
{
52-
AnnotatedTypeSymbol? type = null; // Should ideally be a corresponding tuple type
54+
var type = t is null ? (AnnotatedTypeSymbol?)null : new AnnotatedTypeSymbol(t, t.NullableAnnotation);
5355
var tuple = new Expression(new ExpressionInfo(cx, type, cx.CreateLocation(node.GetLocation()), ExprKind.TUPLE, parent, child, false, null));
5456

5557
cx.Try(null, null, () =>
5658
{
57-
var child0 = 0;
58-
foreach (var variable in designation.Variables)
59-
Create(cx, node, variable, tuple, child0++);
59+
for (var child0 = 0; child0 < designation.Variables.Count; child0++)
60+
{
61+
Create(cx, node, designation.Variables[child0], tuple, child0, t?.TypeArguments[child0] as INamedTypeSymbol);
62+
}
6063
});
6164

6265
return tuple;
6366
}
6467

6568
public static Expression CreateParenthesized(Context cx, VarPatternSyntax varPattern, ParenthesizedVariableDesignationSyntax designation, IExpressionParentEntity parent, int child)
6669
{
67-
AnnotatedTypeSymbol? type = null; // Should ideally be a corresponding tuple type
68-
var tuple = new Expression(new ExpressionInfo(cx, type, cx.CreateLocation(varPattern.GetLocation()), ExprKind.TUPLE, parent, child, false, null));
70+
var tuple = new Expression(
71+
new ExpressionInfo(cx, null, cx.CreateLocation(varPattern.GetLocation()), ExprKind.TUPLE, parent, child, false, null),
72+
shouldPopulate: false);
6973

74+
var elementTypes = new List<ITypeSymbol?>();
7075
cx.Try(null, null, () =>
7176
{
7277
var child0 = 0;
7378
foreach (var variable in designation.Variables)
7479
{
80+
Expression sub;
7581
switch (variable)
7682
{
7783
case ParenthesizedVariableDesignationSyntax paren:
78-
CreateParenthesized(cx, varPattern, paren, tuple, child0++);
84+
sub = CreateParenthesized(cx, varPattern, paren, tuple, child0++);
7985
break;
8086
case SingleVariableDesignationSyntax single:
8187
if (cx.GetModel(variable).GetDeclaredSymbol(single) is ILocalSymbol local)
8288
{
8389
var decl = Create(cx, variable, local.GetAnnotatedType(), tuple, child0++);
8490
var l = LocalVariable.Create(cx, local);
8591
l.PopulateManual(decl, true);
92+
sub = decl;
8693
}
8794
else
8895
{
8996
throw new InternalError(single, "Failed to access local variable");
9097
}
9198
break;
9299
case DiscardDesignationSyntax discard:
93-
new Discard(cx, discard, tuple, child0++);
100+
sub = new Discard(cx, discard, tuple, child0++);
101+
if (!sub.Type.HasValue || sub.Type.Value.Symbol is null)
102+
{
103+
// The type is only updated in memory, it will not be written to the trap file.
104+
sub.SetType(cx.Compilation.GetSpecialType(SpecialType.System_Object));
105+
}
94106
break;
95107
default:
96108
throw new InternalError(variable, "Unhandled designation type");
97109
}
110+
111+
elementTypes.Add(sub.Type.HasValue && sub.Type.Value.Symbol?.Kind != SymbolKind.ErrorType
112+
? sub.Type.Value.Symbol
113+
: null);
98114
}
99115
});
100116

117+
INamedTypeSymbol? tupleType = null;
118+
if (!elementTypes.Any(et => et is null))
119+
{
120+
tupleType = cx.Compilation.CreateTupleTypeSymbol(elementTypes.ToImmutableArray()!);
121+
}
122+
123+
tuple.SetType(tupleType);
124+
tuple.TryPopulate();
125+
101126
return tuple;
102127
}
103128

104-
105-
private static Expression Create(Context cx, DeclarationExpressionSyntax node, VariableDesignationSyntax? designation, IExpressionParentEntity parent, int child)
129+
private static Expression Create(Context cx, DeclarationExpressionSyntax node, VariableDesignationSyntax? designation, IExpressionParentEntity parent, int child, INamedTypeSymbol? declarationType)
106130
{
107131
switch (designation)
108132
{
109133
case SingleVariableDesignationSyntax single:
110134
return CreateSingle(cx, node, single, parent, child);
111135
case ParenthesizedVariableDesignationSyntax paren:
112-
return CreateParenthesized(cx, node, paren, parent, child);
136+
return CreateParenthesized(cx, node, paren, parent, child, declarationType);
113137
case DiscardDesignationSyntax discard:
114138
var type = cx.GetType(discard);
115139
return Create(cx, node, type, parent, child);
@@ -120,7 +144,7 @@ private static Expression Create(Context cx, DeclarationExpressionSyntax node, V
120144
}
121145

122146
public static Expression Create(Context cx, DeclarationExpressionSyntax node, IExpressionParentEntity parent, int child) =>
123-
Create(cx, node, node.Designation, parent, child);
147+
Create(cx, node, node.Designation, parent, child, cx.GetTypeInfo(node).Type.DisambiguateType() as INamedTypeSymbol);
124148

125149
public static VariableDeclaration Create(Context cx, CSharpSyntaxNode c, AnnotatedTypeSymbol? type, IExpressionParentEntity parent, int child) =>
126150
new VariableDeclaration(new ExpressionInfo(cx, type, cx.CreateLocation(c.FixedLocation()), ExprKind.LOCAL_VAR_DECL, parent, child, false, null));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private Field(Context cx, IFieldSymbol init)
1717
type = new Lazy<Type>(() => Entities.Type.Create(cx, Symbol.Type));
1818
}
1919

20-
public static Field Create(Context cx, IFieldSymbol field) => FieldFactory.Instance.CreateEntityFromSymbol(cx, field);
20+
public static Field Create(Context cx, IFieldSymbol field) => FieldFactory.Instance.CreateEntityFromSymbol(cx, field.CorrespondingTupleField ?? field);
2121

2222
// Do not populate backing fields.
2323
// Populate Tuple fields.

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TupleType.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Semmle.Extraction.CSharp.Entities
1313
/// </summary>
1414
internal class TupleType : Type<INamedTypeSymbol>
1515
{
16-
public static TupleType Create(Context cx, INamedTypeSymbol type) => TupleTypeFactory.Instance.CreateEntityFromSymbol(cx, type);
16+
public static TupleType Create(Context cx, INamedTypeSymbol type) => TupleTypeFactory.Instance.CreateEntityFromSymbol(cx, type.TupleUnderlyingType ?? type);
1717

1818
private class TupleTypeFactory : CachedEntityFactory<INamedTypeSymbol, TupleType>
1919
{
@@ -41,7 +41,7 @@ public override void Populate(TextWriter trapFile)
4141
PopulateType(trapFile);
4242
PopulateGenerics();
4343

44-
var underlyingType = NamedType.CreateNamedTypeFromTupleType(Context, Symbol.TupleUnderlyingType ?? Symbol);
44+
var underlyingType = NamedType.CreateNamedTypeFromTupleType(Context, Symbol);
4545

4646
trapFile.tuple_underlying_type(this, underlyingType);
4747

‎csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ private static void BuildNamedTypeId(this INamedTypeSymbol named, Context cx, Te
295295
trapFile.BuildList(",", named.TupleElements,
296296
(f, tb0) =>
297297
{
298-
trapFile.Write(f.Name);
298+
trapFile.Write((f.CorrespondingTupleField ?? f).Name);
299299
trapFile.Write(":");
300300
f.Type.BuildOrWriteId(cx, tb0, symbolBeingDefined, addBaseClass);
301301
}

‎csharp/extractor/Semmle.Extraction/Entities/Base/FreshEntity.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ protected FreshEntity(Context cx) : base(cx)
1313

1414
protected abstract void Populate(TextWriter trapFile);
1515

16-
protected void TryPopulate()
16+
public void TryPopulate()
1717
{
1818
Context.Try(null, null, () => Populate(Context.TrapWriter.Writer));
1919
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,9 @@ module AssignableDefinitions {
537537
/** Gets the underlying assignment. */
538538
AssignExpr getAssignment() { result = ae }
539539

540+
/** Gets the leaf expression. */
541+
Expr getLeaf() { result = leaf }
542+
540543
/**
541544
* Gets the evaluation order of this definition among the other definitions
542545
* in the compound tuple assignment. For example, in `(x, (y, z)) = ...` the

‎csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,16 @@ module LocalFlow {
220220
e1 = we.getInitializer() and
221221
e2 = we
222222
)
223+
or
224+
scope = any(AssignExpr ae | ae.getLValue().(TupleExpr) = e2 and ae.getRValue() = e1) and
225+
isSuccessor = false
226+
or
227+
isSuccessor = true and
228+
exists(ControlFlowElement cfe | cfe = e2.(TupleExpr).(PatternExpr).getPatternMatch() |
229+
cfe.(IsExpr).getExpr() = e1 and scope = cfe
230+
or
231+
exists(Switch sw | sw.getACase() = cfe and sw.getExpr() = e1 and scope = sw)
232+
)
223233
)
224234
}
225235

@@ -483,6 +493,18 @@ private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, bool
483493
src = mi.getRValue() and
484494
postUpdate = false
485495
)
496+
or
497+
// Tuple element, `(..., src, ...)` `f` is `ItemX` of tuple `q`
498+
e =
499+
any(TupleExpr te |
500+
exists(int i |
501+
e = q and
502+
src = te.getArgument(i) and
503+
te.isConstruction() and
504+
f = q.getType().(TupleType).getElement(i) and
505+
postUpdate = false
506+
)
507+
)
486508
)
487509
}
488510

@@ -495,7 +517,7 @@ private predicate overridesOrImplementsSourceDecl(Property p1, Property p2) {
495517

496518
/**
497519
* Holds if `e2` is an expression that reads field or property `c` from
498-
* expresion `e1`. This takes overriding into account for properties written
520+
* expression `e1`. This takes overriding into account for properties written
499521
* from library code.
500522
*/
501523
private predicate fieldOrPropertyRead(Expr e1, Content c, FieldOrPropertyRead e2) {
@@ -792,6 +814,37 @@ private module Cached {
792814
hasNodePath(x, node1, node2) and
793815
node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
794816
c = getResultContent()
817+
or
818+
// node1 = (..., node2, ...)
819+
// node1.ItemX flows to node2
820+
exists(TupleExpr te, int i, Expr item |
821+
te = node1.asExpr() and
822+
not te.isConstruction() and
823+
c.(FieldContent).getField() = te.getType().(TupleType).getElement(i).getUnboundDeclaration() and
824+
// node1 = (..., item, ...)
825+
te.getArgument(i) = item
826+
|
827+
// item = (..., ..., ...) in node1 = (..., (..., ..., ...), ...)
828+
node2.asExpr().(TupleExpr) = item and
829+
hasNodePath(x, node1, node2)
830+
or
831+
// item = variable in node1 = (..., variable, ...)
832+
exists(AssignableDefinitions::TupleAssignmentDefinition tad, Ssa::ExplicitDefinition def |
833+
node2.(SsaDefinitionNode).getDefinition() = def and
834+
def.getADefinition() = tad and
835+
tad.getLeaf() = item and
836+
hasNodePath(x, node1, node2)
837+
)
838+
or
839+
// item = variable in node1 = (..., variable, ...) in a case/is var (..., ...)
840+
te = any(PatternExpr pe).getAChildExpr*() and
841+
exists(AssignableDefinitions::LocalVariableDefinition lvd, Ssa::ExplicitDefinition def |
842+
node2.(SsaDefinitionNode).getDefinition() = def and
843+
def.getADefinition() = lvd and
844+
lvd.getDeclaration() = item and
845+
hasNodePath(x, node1, node2)
846+
)
847+
)
795848
)
796849
or
797850
FlowSummaryImpl::Private::readStep(node1, c, node2)
@@ -1731,6 +1784,11 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
17311784
e1 = e2.(AwaitExpr).getExpr() and
17321785
scope = e2 and
17331786
isSuccessor = true
1787+
or
1788+
exactScope = false and
1789+
e2 = e1.(TupleExpr).getAnArgument() and
1790+
scope = e1 and
1791+
isSuccessor = false
17341792
}
17351793

17361794
override predicate candidateDef(
@@ -1752,6 +1810,22 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
17521810
scope = fs.getVariableDeclExpr() and
17531811
exactScope = false
17541812
)
1813+
or
1814+
scope =
1815+
any(AssignExpr ae |
1816+
ae = defTo.(AssignableDefinitions::TupleAssignmentDefinition).getAssignment() and
1817+
e = ae.getLValue().getAChildExpr*().(TupleExpr) and
1818+
exactScope = false and
1819+
isSuccessor = true
1820+
)
1821+
or
1822+
scope =
1823+
any(TupleExpr te |
1824+
te.getAnArgument() = defTo.(AssignableDefinitions::LocalVariableDefinition).getDeclaration() and
1825+
e = te and
1826+
exactScope = false and
1827+
isSuccessor = false
1828+
)
17551829
}
17561830
}
17571831

‎csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,25 +62,9 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
6262
e1 = e2.(BinaryLogicalOperation).getAnOperand() and
6363
scope = e2
6464
or
65-
// Taint from tuple argument
66-
e2 =
67-
any(TupleExpr te |
68-
e1 = te.getAnArgument() and
69-
te.isReadAccess() and
70-
scope = e2
71-
)
72-
or
7365
e1 = e2.(InterpolatedStringExpr).getAChild() and
7466
scope = e2
7567
or
76-
// Taint from tuple expression
77-
e2 =
78-
any(MemberAccess ma |
79-
ma.getQualifier().getType() instanceof TupleType and
80-
e1 = ma.getQualifier() and
81-
scope = e2
82-
)
83-
or
8468
e2 =
8569
any(OperatorCall oc |
8670
oc.getTarget().(ConversionOperator).fromLibrary() and

0 commit comments

Comments
 (0)