Skip to content

Commit 5503abc

Browse files
hvitvedhvitved
authored
Merge pull request github#7772 from hvitved/csharp/event-accessor-event-null
C#: Guard against `AssociatedSymbol` not being an `IEventSymbol`
2 parents 56ac990 + 2354281 commit 5503abc

File tree

10 files changed

+8235
-50
lines changed

10 files changed

+8235
-50
lines changed

‎csharp/downgrades/ba2201248071b2bf0bb52909b35014091d2e18a6/old.dbscheme

Lines changed: 2046 additions & 0 deletions
Large diffs are not rendered by default.

‎csharp/downgrades/ba2201248071b2bf0bb52909b35014091d2e18a6/semmlecode.csharp.dbscheme

Lines changed: 2046 additions & 0 deletions
Large diffs are not rendered by default.

‎csharp/downgrades/ba2201248071b2bf0bb52909b35014091d2e18a6/upgrade.properties

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
description: Support for compiler-generated event accessors.
2+
compatibility: backwards

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

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@ namespace Semmle.Extraction.CSharp.Entities
66
{
77
internal class Accessor : Method
88
{
9-
protected Accessor(Context cx, IMethodSymbol init)
10-
: base(cx, init) { }
9+
private readonly IPropertySymbol property;
10+
protected Accessor(Context cx, IMethodSymbol init, IPropertySymbol property)
11+
: base(cx, init)
12+
{
13+
this.property = property;
14+
}
1115

1216
/// <summary>
13-
/// Gets the property symbol associated accessor `symbol`, or `null`
17+
/// Gets the property symbol associated with accessor `symbol`, or `null`
1418
/// if there is no associated symbol.
1519
/// </summary>
1620
public static IPropertySymbol? GetPropertySymbol(IMethodSymbol symbol)
@@ -26,39 +30,26 @@ protected Accessor(Context cx, IMethodSymbol init)
2630
return props.SingleOrDefault();
2731
}
2832

29-
/// <summary>
30-
/// Gets the property symbol associated with this accessor.
31-
/// </summary>
32-
private IPropertySymbol? PropertySymbol => GetPropertySymbol(Symbol);
33-
34-
public new Accessor OriginalDefinition => Create(Context, Symbol.OriginalDefinition);
35-
3633
public override void Populate(TextWriter trapFile)
3734
{
3835
PopulateMethod(trapFile);
3936
PopulateModifiers(trapFile);
4037
ContainingType!.PopulateGenerics();
4138

42-
var prop = PropertySymbol;
43-
if (prop is null)
44-
{
45-
var type = Symbol.AssociatedSymbol?.GetType().ToString() ?? "null";
46-
Context.ModelError(Symbol, $"Unhandled accessor associated symbol of type {type}");
47-
return;
48-
}
49-
50-
var parent = Property.Create(Context, prop);
39+
var parent = Property.Create(Context, property);
5140
int kind;
5241
Accessor unboundAccessor;
53-
if (SymbolEqualityComparer.Default.Equals(Symbol, prop.GetMethod))
42+
if (SymbolEqualityComparer.Default.Equals(Symbol, property.GetMethod))
5443
{
5544
kind = 1;
56-
unboundAccessor = Create(Context, prop.OriginalDefinition.GetMethod!);
45+
var orig = property.OriginalDefinition;
46+
unboundAccessor = Create(Context, orig.GetMethod!, orig);
5747
}
58-
else if (SymbolEqualityComparer.Default.Equals(Symbol, prop.SetMethod))
48+
else if (SymbolEqualityComparer.Default.Equals(Symbol, property.SetMethod))
5949
{
6050
kind = 2;
61-
unboundAccessor = Create(Context, prop.OriginalDefinition.SetMethod!);
51+
var orig = property.OriginalDefinition;
52+
unboundAccessor = Create(Context, orig.SetMethod!, orig);
6253
}
6354
else
6455
{
@@ -84,14 +75,14 @@ public override void Populate(TextWriter trapFile)
8475
}
8576
}
8677

87-
public static new Accessor Create(Context cx, IMethodSymbol symbol) =>
88-
AccessorFactory.Instance.CreateEntityFromSymbol(cx, symbol);
78+
public static Accessor Create(Context cx, IMethodSymbol symbol, IPropertySymbol prop) =>
79+
AccessorFactory.Instance.CreateEntity(cx, symbol, (symbol, prop));
8980

90-
private class AccessorFactory : CachedEntityFactory<IMethodSymbol, Accessor>
81+
private class AccessorFactory : CachedEntityFactory<(IMethodSymbol, IPropertySymbol), Accessor>
9182
{
9283
public static AccessorFactory Instance { get; } = new AccessorFactory();
9384

94-
public override Accessor Create(Context cx, IMethodSymbol init) => new Accessor(cx, init);
85+
public override Accessor Create(Context cx, (IMethodSymbol, IPropertySymbol) init) => new(cx, init.Item1, init.Item2);
9586
}
9687
}
9788
}

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

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,45 +3,46 @@
33

44
namespace Semmle.Extraction.CSharp.Entities
55
{
6-
internal class EventAccessor : Accessor
6+
internal class EventAccessor : Method
77
{
8-
private EventAccessor(Context cx, IMethodSymbol init)
9-
: base(cx, init) { }
8+
private readonly IEventSymbol @event;
9+
10+
private EventAccessor(Context cx, IMethodSymbol init, IEventSymbol @event)
11+
: base(cx, init)
12+
{
13+
this.@event = @event;
14+
}
1015

1116
/// <summary>
12-
/// Gets the event symbol associated with this accessor.
17+
/// Gets the event symbol associated with accessor `symbol`, or `null`
18+
/// if there is no associated symbol.
1319
/// </summary>
14-
private IEventSymbol? EventSymbol => Symbol.AssociatedSymbol as IEventSymbol;
20+
public static IEventSymbol? GetEventSymbol(IMethodSymbol symbol) =>
21+
symbol.AssociatedSymbol as IEventSymbol;
1522

1623
public override void Populate(TextWriter trapFile)
1724
{
1825
PopulateMethod(trapFile);
1926
ContainingType!.PopulateGenerics();
2027

21-
var @event = EventSymbol;
22-
if (@event is null)
23-
{
24-
var type = Symbol.AssociatedSymbol?.GetType().ToString() ?? "null";
25-
Context.ModelError(Symbol, $"Unhandled event accessor associated symbol of type {type}");
26-
return;
27-
}
28-
2928
var parent = Event.Create(Context, @event);
3029
int kind;
3130
EventAccessor unboundAccessor;
3231
if (SymbolEqualityComparer.Default.Equals(Symbol, @event.AddMethod))
3332
{
3433
kind = 1;
35-
unboundAccessor = Create(Context, @event.OriginalDefinition.AddMethod!);
34+
var orig = @event.OriginalDefinition;
35+
unboundAccessor = Create(Context, orig.AddMethod!, orig);
3636
}
3737
else if (SymbolEqualityComparer.Default.Equals(Symbol, @event.RemoveMethod))
3838
{
3939
kind = 2;
40-
unboundAccessor = Create(Context, @event.OriginalDefinition.RemoveMethod!);
40+
var orig = @event.OriginalDefinition;
41+
unboundAccessor = Create(Context, orig.RemoveMethod!, orig);
4142
}
4243
else
4344
{
44-
Context.ModelError(Symbol, "Undhandled event accessor kind");
45+
Context.ModelError(Symbol, $"Undhandled event accessor kind {Symbol.ToDisplayString()}");
4546
return;
4647
}
4748

@@ -51,16 +52,21 @@ public override void Populate(TextWriter trapFile)
5152
trapFile.event_accessor_location(this, l);
5253

5354
Overrides(trapFile);
55+
56+
if (Symbol.FromSource() && Block is null)
57+
{
58+
trapFile.compiler_generated(this);
59+
}
5460
}
5561

56-
public static new EventAccessor Create(Context cx, IMethodSymbol symbol) =>
57-
EventAccessorFactory.Instance.CreateEntityFromSymbol(cx, symbol);
62+
public static EventAccessor Create(Context cx, IMethodSymbol symbol, IEventSymbol @event) =>
63+
EventAccessorFactory.Instance.CreateEntity(cx, symbol, (symbol, @event));
5864

59-
private class EventAccessorFactory : CachedEntityFactory<IMethodSymbol, EventAccessor>
65+
private class EventAccessorFactory : CachedEntityFactory<(IMethodSymbol, IEventSymbol), EventAccessor>
6066
{
6167
public static EventAccessorFactory Instance { get; } = new EventAccessorFactory();
6268

63-
public override EventAccessor Create(Context cx, IMethodSymbol init) => new EventAccessor(cx, init);
69+
public override EventAccessor Create(Context cx, (IMethodSymbol, IEventSymbol) init) => new EventAccessor(cx, init.Item1, init.Item2);
6470
}
6571
}
6672
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,10 @@ public static void AddExplicitInterfaceQualifierToId(Context cx, EscapingTextWri
262262
return Destructor.Create(cx, methodDecl);
263263
case MethodKind.PropertyGet:
264264
case MethodKind.PropertySet:
265-
return Accessor.GetPropertySymbol(methodDecl) is null ? OrdinaryMethod.Create(cx, methodDecl) : (Method)Accessor.Create(cx, methodDecl);
265+
return Accessor.GetPropertySymbol(methodDecl) is IPropertySymbol prop ? Accessor.Create(cx, methodDecl, prop) : OrdinaryMethod.Create(cx, methodDecl);
266266
case MethodKind.EventAdd:
267267
case MethodKind.EventRemove:
268-
return EventAccessor.Create(cx, methodDecl);
268+
return EventAccessor.GetEventSymbol(methodDecl) is IEventSymbol @event ? EventAccessor.Create(cx, methodDecl, @event) : OrdinaryMethod.Create(cx, methodDecl);
269269
case MethodKind.UserDefinedOperator:
270270
case MethodKind.BuiltinOperator:
271271
return UserOperator.Create(cx, methodDecl);

‎csharp/ql/lib/semmlecode.csharp.dbscheme

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ has_modifiers(
648648
int id: @modifiable_direct ref,
649649
int mod_id: @modifier ref);
650650

651-
compiler_generated(unique int id: @modifiable_direct ref);
651+
compiler_generated(unique int id: @modifiable ref);
652652

653653
/** MEMBERS **/
654654

0 commit comments

Comments
 (0)