diff --git a/change-notes/1.26/analysis-csharp.md b/change-notes/1.26/analysis-csharp.md index 202c939553be..5b65481c9254 100644 --- a/change-notes/1.26/analysis-csharp.md +++ b/change-notes/1.26/analysis-csharp.md @@ -21,6 +21,10 @@ The following changes in version 1.26 affect C# analysis in all applications. * Partial method bodies are extracted. Previously, partial method bodies were skipped completely. * Inferring the lengths of implicitely sized arrays is fixed. Previously, multidimensional arrays were always extracted with the same length for each dimension. With the fix, the array sizes `2` and `1` are extracted for `new int[,]{{1},{2}}`. Previously `2` and `2` were extracted. +* The extractor is now assembly-insensitive by default. This means that two entities with the same + fully-qualified name are now mapped to the same entity in the resulting database, regardless of + whether they belong to different assemblies. Assembly sensitivity can be reenabled by passing + `--assemblysensitivetrap` to the extractor. ## Changes to libraries diff --git a/csharp/extractor/Semmle.Extraction.CIL/Entities/Assembly.cs b/csharp/extractor/Semmle.Extraction.CIL/Entities/Assembly.cs index 0368888caa7d..f633af96db7a 100644 --- a/csharp/extractor/Semmle.Extraction.CIL/Entities/Assembly.cs +++ b/csharp/extractor/Semmle.Extraction.CIL/Entities/Assembly.cs @@ -141,7 +141,7 @@ public static void ExtractCIL(Layout layout, string assemblyPath, ILogger logger trapFile = trapWriter.TrapFile; if (nocache || !System.IO.File.Exists(trapFile)) { - var cx = extractor.CreateContext(null, trapWriter, null); + var cx = extractor.CreateContext(null, trapWriter, null, false); ExtractCIL(cx, assemblyPath, extractPdbs); extracted = true; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Analyser.cs b/csharp/extractor/Semmle.Extraction.CSharp/Analyser.cs index 544a1819117a..34dece8e160e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Analyser.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Analyser.cs @@ -25,9 +25,12 @@ public class Analyser : IDisposable public readonly ILogger Logger; - public Analyser(IProgressMonitor pm, ILogger logger) + public readonly bool AddAssemblyTrapPrefix; + + public Analyser(IProgressMonitor pm, ILogger logger, bool addAssemblyTrapPrefix) { Logger = logger; + AddAssemblyTrapPrefix = addAssemblyTrapPrefix; Logger.Log(Severity.Info, "EXTRACTION STARTING at {0}", DateTime.Now); stopWatch.Start(); progressMonitor = pm; @@ -231,7 +234,7 @@ void DoAnalyseCompilation(string cwd, string[] args) var projectLayout = layout.LookupProjectOrDefault(assemblyPath); var trapWriter = projectLayout.CreateTrapWriter(Logger, assemblyPath, true, options.TrapCompression); compilationTrapFile = trapWriter; // Dispose later - var cx = extractor.CreateContext(compilation.Clone(), trapWriter, new AssemblyScope(assembly, assemblyPath, true)); + var cx = extractor.CreateContext(compilation.Clone(), trapWriter, new AssemblyScope(assembly, assemblyPath, true), AddAssemblyTrapPrefix); compilationEntity = new Entities.Compilation(cx, cwd, args); } @@ -286,7 +289,7 @@ void DoAnalyseAssembly(PortableExecutableReference r) if (assembly != null) { - var cx = extractor.CreateContext(c, trapWriter, new AssemblyScope(assembly, assemblyPath, false)); + var cx = extractor.CreateContext(c, trapWriter, new AssemblyScope(assembly, assemblyPath, false), AddAssemblyTrapPrefix); foreach (var module in assembly.Modules) { @@ -372,7 +375,7 @@ void DoExtractTree(SyntaxTree tree) if (!upToDate) { - Context cx = extractor.CreateContext(compilation.Clone(), trapWriter, new SourceScope(tree)); + Context cx = extractor.CreateContext(compilation.Clone(), trapWriter, new SourceScope(tree), AddAssemblyTrapPrefix); Populators.CompilationUnit.Extract(cx, tree.GetRoot()); cx.PopulateAll(); cx.ExtractComments(cx.CommentGenerator); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs index 98c021c8a6d6..cc713b5ad2f3 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs @@ -77,7 +77,7 @@ public override void Populate(TextWriter trapFile) } public new static Accessor Create(Context cx, IMethodSymbol symbol) => - AccessorFactory.Instance.CreateEntity(cx, symbol); + AccessorFactory.Instance.CreateEntityFromSymbol(cx, symbol); class AccessorFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentBlock.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentBlock.cs index 0e851df1e6b2..ffc829885976 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentBlock.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentBlock.cs @@ -35,7 +35,7 @@ public void BindTo(Label entity, CommentBinding binding) Context.TrapWriter.Writer.commentblock_binding(this, entity, binding); } - public static CommentBlock Create(Context cx, ICommentBlock block) => CommentBlockFactory.Instance.CreateEntity(cx, block); + public static CommentBlock Create(Context cx, ICommentBlock block) => CommentBlockFactory.Instance.CreateEntity(cx, block, block); class CommentBlockFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentLine.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentLine.cs index a3ba9896ef1c..e08830a6c3ad 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentLine.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/CommentLine.cs @@ -129,7 +129,11 @@ public override void WriteId(TextWriter trapFile) trapFile.Write(";commentline"); } - static CommentLine Create(Context cx, Microsoft.CodeAnalysis.Location loc, CommentLineType type, string text, string raw) => CommentLineFactory.Instance.CreateEntity(cx, loc, type, text, raw); + static CommentLine Create(Context cx, Microsoft.CodeAnalysis.Location loc, CommentLineType type, string text, string raw) + { + var init = (loc, type, text, raw); + return CommentLineFactory.Instance.CreateEntity(cx, init, init); + } class CommentLineFactory : ICachedEntityFactory<(Microsoft.CodeAnalysis.Location, CommentLineType, string, string), CommentLine> { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs index 298f838cfb93..a9baacbf7287 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs @@ -104,7 +104,7 @@ ConstructorDeclarationSyntax Syntax { case MethodKind.StaticConstructor: case MethodKind.Constructor: - return ConstructorFactory.Instance.CreateEntity(cx, constructor); + return ConstructorFactory.Instance.CreateEntityFromSymbol(cx, constructor); default: throw new InternalError(constructor, "Attempt to create a Constructor from a symbol that isn't a constructor"); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Conversion.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Conversion.cs index e365ca53ae0c..98758fb773f6 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Conversion.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Conversion.cs @@ -11,7 +11,7 @@ class Conversion : UserOperator : base(cx, init) { } public new static Conversion Create(Context cx, IMethodSymbol symbol) => - ConversionFactory.Instance.CreateEntity(cx, symbol); + ConversionFactory.Instance.CreateEntityFromSymbol(cx, symbol); public override Microsoft.CodeAnalysis.Location ReportingLocation { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Destructor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Destructor.cs index 33f3a330f944..65557b16b67f 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Destructor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Destructor.cs @@ -24,7 +24,7 @@ public override void Populate(TextWriter trapFile) } public new static Destructor Create(Context cx, IMethodSymbol symbol) => - DestructorFactory.Instance.CreateEntity(cx, symbol); + DestructorFactory.Instance.CreateEntityFromSymbol(cx, symbol); class DestructorFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs index 224c81b9524b..59f937a6331e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs @@ -60,7 +60,7 @@ public override void Populate(TextWriter trapFile) TypeMention.Create(Context, syntaxType, this, type); } - public static Event Create(Context cx, IEventSymbol symbol) => EventFactory.Instance.CreateEntity(cx, symbol); + public static Event Create(Context cx, IEventSymbol symbol) => EventFactory.Instance.CreateEntityFromSymbol(cx, symbol); class EventFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/EventAccessor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/EventAccessor.cs index 7dc0a4dfcffa..da858b689dc3 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/EventAccessor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/EventAccessor.cs @@ -53,7 +53,7 @@ public override void Populate(TextWriter trapFile) } public new static EventAccessor Create(Context cx, IMethodSymbol symbol) => - EventAccessorFactory.Instance.CreateEntity(cx, symbol); + EventAccessorFactory.Instance.CreateEntityFromSymbol(cx, symbol); class EventAccessorFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs index 7ae4572cd7d0..5f881338200c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs @@ -18,7 +18,7 @@ class Field : CachedSymbol, IExpressionParentEntity type = new Lazy(() => Entities.Type.Create(cx, symbol.GetAnnotatedType())); } - public static Field Create(Context cx, IFieldSymbol field) => FieldFactory.Instance.CreateEntity(cx, field); + public static Field Create(Context cx, IFieldSymbol field) => FieldFactory.Instance.CreateEntityFromSymbol(cx, field); // Do not populate backing fields. // Populate Tuple fields. @@ -101,6 +101,8 @@ public override void Populate(TextWriter trapFile) public override void WriteId(TextWriter trapFile) { + trapFile.WriteSubId(Type.Type); + trapFile.Write(" "); trapFile.WriteSubId(ContainingType); trapFile.Write('.'); trapFile.Write(symbol.Name); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs index b9ece3d8c8e1..af07c41dd6b7 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs @@ -71,7 +71,7 @@ public override void Populate(TextWriter trapFile) TypeMention.Create(Context, syntax.Type, this, type); } - public static new Indexer Create(Context cx, IPropertySymbol prop) => IndexerFactory.Instance.CreateEntity(cx, prop); + public static new Indexer Create(Context cx, IPropertySymbol prop) => IndexerFactory.Instance.CreateEntityFromSymbol(cx, prop); public override void WriteId(TextWriter trapFile) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalFunction.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalFunction.cs index e570802ab3d8..8b90f6905dd3 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalFunction.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalFunction.cs @@ -22,7 +22,7 @@ public override void WriteQuotedId(TextWriter trapFile) trapFile.Write('*'); } - public static new LocalFunction Create(Context cx, IMethodSymbol field) => LocalFunctionFactory.Instance.CreateEntity(cx, field); + public static new LocalFunction Create(Context cx, IMethodSymbol field) => LocalFunctionFactory.Instance.CreateEntityFromSymbol(cx, field); class LocalFunctionFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalVariable.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalVariable.cs index 6d0f14a014eb..02d89d08a2f7 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalVariable.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalVariable.cs @@ -43,7 +43,7 @@ symbol is ILocalSymbol l ? public static LocalVariable Create(Context cx, ISymbol local) { - return LocalVariableFactory.Instance.CreateEntity(cx, local); + return LocalVariableFactory.Instance.CreateEntityFromSymbol(cx, local); } void DefineConstantValue(TextWriter trapFile) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs index aeb57609b8a1..2527920ca3af 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs @@ -108,6 +108,9 @@ public void Overrides(TextWriter trapFile) /// protected static void BuildMethodId(Method m, TextWriter trapFile) { + m.symbol.ReturnType.BuildOrWriteId(m.Context, trapFile, m.symbol); + trapFile.Write(" "); + trapFile.WriteSubId(m.ContainingType); AddExplicitInterfaceQualifierToId(m.Context, trapFile, m.symbol.ExplicitInterfaceImplementations); @@ -129,7 +132,7 @@ protected static void BuildMethodId(Method m, TextWriter trapFile) // Type arguments with different nullability can result in // a constructed method with different nullability of its parameters and return type, // so we need to create a distinct database entity for it. - trapFile.BuildList(",", m.symbol.GetAnnotatedTypeArguments(), (ta, tb0) => { AddSignatureTypeToId(m.Context, tb0, m.symbol, ta.Symbol); trapFile.Write((int)ta.Nullability); }); + trapFile.BuildList(",", m.symbol.GetAnnotatedTypeArguments(), (ta, tb0) => { ta.Symbol.BuildOrWriteId(m.Context, tb0, m.symbol); trapFile.Write((int)ta.Nullability); }); trapFile.Write('>'); } } @@ -163,65 +166,21 @@ public override void WriteId(TextWriter trapFile) BuildMethodId(this, trapFile); } - /// - /// Adds an appropriate label ID to the trap builder - /// for the type belonging to the signature of method - /// . - /// - /// For methods without type parameters this will always add the key of the - /// corresponding type. - /// - /// For methods with type parameters, this will add the key of the - /// corresponding type if the type does *not* contain one of the method - /// type parameters, otherwise it will add a textual representation of - /// the type. This distinction is required because type parameter IDs - /// refer to their declaring methods. - /// - /// Example: - /// - /// - /// int Count<T>(IEnumerable items) - /// - /// - /// The label definitions for Count (#4) and T - /// (#5) will look like: - /// - /// - /// #1=<label for System.Int32> - /// #2=<label for type containing Count> - /// #3=<label for IEnumerable`1> - /// #4=@"{#1} {#2}.Count`2(#3);method" - /// #5=@"{#4}T;typeparameter" - /// - /// - /// Note how int is referenced in the label definition #3 for - /// Count, while T[] is represented textually in order - /// to make the reference to #3 in the label definition #4 for - /// T valid. - /// - protected static void AddSignatureTypeToId(Context cx, TextWriter trapFile, IMethodSymbol method, ITypeSymbol type) - { - if (type.ContainsTypeParameters(cx, method)) - type.BuildTypeId(cx, trapFile, (cx0, tb0, type0) => AddSignatureTypeToId(cx, tb0, method, type0)); - else - trapFile.WriteSubId(Type.Create(cx, type)); - } - protected static void AddParametersToId(Context cx, TextWriter trapFile, IMethodSymbol method) { trapFile.Write('('); int index = 0; - if (method.MethodKind == MethodKind.ReducedExtension) - { - trapFile.WriteSeparator(",", ref index); - AddSignatureTypeToId(cx, trapFile, method, method.ReceiverType); - } + var @params = method.MethodKind == MethodKind.ReducedExtension + ? method.ReducedFrom.Parameters + : method.Parameters; - foreach (var param in method.Parameters) + foreach (var param in @params) { trapFile.WriteSeparator(",", ref index); - AddSignatureTypeToId(cx, trapFile, method, param.Type); + param.Type.BuildOrWriteId(cx, trapFile, method); + trapFile.Write(" "); + trapFile.Write(param.Name); switch (param.RefKind) { case RefKind.Out: @@ -245,9 +204,7 @@ protected static void AddParametersToId(Context cx, TextWriter trapFile, IMethod public static void AddExplicitInterfaceQualifierToId(Context cx, System.IO.TextWriter trapFile, IEnumerable explicitInterfaceImplementations) { if (explicitInterfaceImplementations.Any()) - { trapFile.AppendList(",", explicitInterfaceImplementations.Select(impl => cx.CreateEntity(impl.ContainingType))); - } } public virtual string Name => symbol.Name; diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Modifier.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Modifier.cs index 4de6318786ce..61c5fad4ff4d 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Modifier.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Modifier.cs @@ -5,37 +5,16 @@ namespace Semmle.Extraction.CSharp.Entities { - /// - /// Provide a "Key" object to allow modifiers to exist as entities in the extractor - /// hash map. (Raw strings would work as keys but might clash with other types). - /// - class ModifierKey : Object + class Modifier : Extraction.CachedEntity { - public readonly string name; - - public ModifierKey(string m) - { - name = m; - } - - public override bool Equals(Object obj) - { - return obj.GetType() == GetType() && name == ((ModifierKey)obj).name; - } - - public override int GetHashCode() => 13 * name.GetHashCode(); - } - - class Modifier : Extraction.CachedEntity - { - Modifier(Context cx, ModifierKey init) + Modifier(Context cx, string init) : base(cx, init) { } public override Microsoft.CodeAnalysis.Location ReportingLocation => null; public override void WriteId(TextWriter trapFile) { - trapFile.Write(symbol.name); + trapFile.Write(symbol); trapFile.Write(";modifier"); } @@ -43,7 +22,7 @@ public override void WriteId(TextWriter trapFile) public override void Populate(TextWriter trapFile) { - trapFile.modifiers(Label, symbol.name); + trapFile.modifiers(Label, symbol); } public static string AccessbilityModifier(Accessibility access) @@ -152,17 +131,22 @@ public static void ExtractModifiers(Context cx, TextWriter trapFile, IEntity key } } - public static Modifier Create(Context cx, string modifier) => - ModifierFactory.Instance.CreateEntity(cx, new ModifierKey(modifier)); + public static Modifier Create(Context cx, string modifier) + { + return ModifierFactory.Instance.CreateEntity(cx, (typeof(Modifier), modifier), modifier); + } - public static Modifier Create(Context cx, Accessibility access) => - ModifierFactory.Instance.CreateEntity(cx, new ModifierKey(AccessbilityModifier(access))); + public static Modifier Create(Context cx, Accessibility access) + { + var modifier = AccessbilityModifier(access); + return ModifierFactory.Instance.CreateEntity(cx, (typeof(Modifier), modifier), modifier); + } - class ModifierFactory : ICachedEntityFactory + class ModifierFactory : ICachedEntityFactory { public static readonly ModifierFactory Instance = new ModifierFactory(); - public Modifier Create(Context cx, ModifierKey init) => new Modifier(cx, init); + public Modifier Create(Context cx, string init) => new Modifier(cx, init); } public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.OptionalLabel; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs index 1d663b1b5616..cbc6d2f8b26b 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs @@ -34,7 +34,7 @@ public override void WriteId(TextWriter trapFile) trapFile.Write(";namespace"); } - public static Namespace Create(Context cx, INamespaceSymbol ns) => NamespaceFactory.Instance.CreateEntity2(cx, ns); + public static Namespace Create(Context cx, INamespaceSymbol ns) => NamespaceFactory.Instance.CreateEntityFromSymbol(cx, ns); class NamespaceFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs index 23aa8af97215..ad41afd41895 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs @@ -51,7 +51,7 @@ public override void Populate(TextWriter trapFile) ExtractCompilerGenerated(trapFile); } - public new static OrdinaryMethod Create(Context cx, IMethodSymbol method) => OrdinaryMethodFactory.Instance.CreateEntity(cx, method); + public new static OrdinaryMethod Create(Context cx, IMethodSymbol method) => OrdinaryMethodFactory.Instance.CreateEntityFromSymbol(cx, method); class OrdinaryMethodFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs index 9a64115f24b2..fd74a1c1e9cf 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs @@ -66,10 +66,10 @@ Kind ParamKind } public static Parameter Create(Context cx, IParameterSymbol param, IEntity parent, Parameter original = null) => - ParameterFactory.Instance.CreateEntity(cx, param, parent, original); + ParameterFactory.Instance.CreateEntity(cx, param, (param, parent, original)); public static Parameter Create(Context cx, IParameterSymbol param) => - ParameterFactory.Instance.CreateEntity(cx, param, null, null); + ParameterFactory.Instance.CreateEntity(cx, param, (param, null, null)); public override void WriteId(TextWriter trapFile) { @@ -202,7 +202,7 @@ public override bool Equals(object obj) return obj != null && obj.GetType() == typeof(VarargsType); } - public static VarargsType Create(Context cx) => VarargsTypeFactory.Instance.CreateNullableEntity(cx, null); + public static VarargsType Create(Context cx) => VarargsTypeFactory.Instance.CreateEntity(cx, typeof(VarargsType), null); class VarargsTypeFactory : ICachedEntityFactory { @@ -237,7 +237,7 @@ public override bool Equals(object obj) return obj != null && obj.GetType() == typeof(VarargsParam); } - public static VarargsParam Create(Context cx, Method method) => VarargsParamFactory.Instance.CreateEntity(cx, method); + public static VarargsParam Create(Context cx, Method method) => VarargsParamFactory.Instance.CreateEntity(cx, typeof(VarargsParam), method); class VarargsParamFactory : ICachedEntityFactory { @@ -264,19 +264,8 @@ public override void Populate(TextWriter trapFile) trapFile.param_location(this, Original.Location); } - public override int GetHashCode() => symbol.GetHashCode() + 31 * ConstructedType.GetHashCode(); - - public override bool Equals(object obj) - { - var other = obj as ConstructedExtensionParameter; - if (other == null || other.GetType() != typeof(ConstructedExtensionParameter)) - return false; - - return SymbolEqualityComparer.Default.Equals(symbol, other.symbol) && SymbolEqualityComparer.Default.Equals(ConstructedType, other.ConstructedType); - } - public static ConstructedExtensionParameter Create(Context cx, Method method, Parameter parameter) => - ExtensionParamFactory.Instance.CreateEntity(cx, (method, parameter)); + ExtensionParamFactory.Instance.CreateEntity(cx, (new SymbolEqualityWrapper(parameter.symbol), new SymbolEqualityWrapper(method.symbol.ReceiverType)), (method, parameter)); class ExtensionParamFactory : ICachedEntityFactory<(Method, Parameter), ConstructedExtensionParameter> { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs index e5e2cc8ab261..a7b778b2b81e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs @@ -1,3 +1,4 @@ +using System; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Semmle.Extraction.CSharp.Entities.Expressions; @@ -11,10 +12,18 @@ namespace Semmle.Extraction.CSharp.Entities class Property : CachedSymbol, IExpressionParentEntity { protected Property(Context cx, IPropertySymbol init) - : base(cx, init) { } + : base(cx, init) + { + type = new Lazy(() => Type.Create(Context, symbol.Type)); + } + + readonly Lazy type; + Type Type => type.Value; public override void WriteId(TextWriter trapFile) { + trapFile.WriteSubId(Type); + trapFile.Write(" "); trapFile.WriteSubId(ContainingType); trapFile.Write('.'); Method.AddExplicitInterfaceQualifierToId(Context, trapFile, symbol.ExplicitInterfaceImplementations); @@ -31,7 +40,7 @@ public override void Populate(TextWriter trapFile) PopulateNullability(trapFile, symbol.GetAnnotatedType()); PopulateRefKind(trapFile, symbol.RefKind); - var type = Type.Create(Context, symbol.Type); + var type = Type; trapFile.properties(this, symbol.GetName(), ContainingType, type.TypeRef, Create(Context, symbol.OriginalDefinition)); var getter = symbol.GetMethod; @@ -113,7 +122,7 @@ public static Property Create(Context cx, IPropertySymbol prop) { bool isIndexer = prop.IsIndexer || prop.Parameters.Any(); - return isIndexer ? Indexer.Create(cx, prop) : PropertyFactory.Instance.CreateEntity(cx, prop); + return isIndexer ? Indexer.Create(cx, prop) : PropertyFactory.Instance.CreateEntityFromSymbol(cx, prop); } public void VisitDeclaration(Context cx, PropertyDeclarationSyntax p) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/ArrayType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/ArrayType.cs index c4a79a7764b0..9e2116603441 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/ArrayType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/ArrayType.cs @@ -36,7 +36,7 @@ public override void WriteId(TextWriter trapFile) trapFile.Write(";type"); } - public static ArrayType Create(Context cx, IArrayTypeSymbol symbol) => ArrayTypeFactory.Instance.CreateEntity(cx, symbol); + public static ArrayType Create(Context cx, IArrayTypeSymbol symbol) => ArrayTypeFactory.Instance.CreateEntityFromSymbol(cx, symbol); class ArrayTypeFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/DynamicType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/DynamicType.cs index f5b6bf36c440..bfaba1cff85c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/DynamicType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/DynamicType.cs @@ -9,7 +9,7 @@ class DynamicType : Type DynamicType(Context cx, IDynamicTypeSymbol init) : base(cx, init) { } - public static DynamicType Create(Context cx, IDynamicTypeSymbol type) => DynamicTypeFactory.Instance.CreateEntity(cx, type); + public static DynamicType Create(Context cx, IDynamicTypeSymbol type) => DynamicTypeFactory.Instance.CreateEntityFromSymbol(cx, type); public override Microsoft.CodeAnalysis.Location ReportingLocation => Context.Compilation.ObjectType.Locations.FirstOrDefault(); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs index 93c13726750e..25c830e62930 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs @@ -17,7 +17,7 @@ class NamedType : Type typeArgumentsLazy = new Lazy(() => symbol.TypeArguments.Select(t => Create(cx, t)).ToArray()); } - public static NamedType Create(Context cx, INamedTypeSymbol type) => NamedTypeFactory.Instance.CreateEntity(cx, type); + public static NamedType Create(Context cx, INamedTypeSymbol type) => NamedTypeFactory.Instance.CreateEntityFromSymbol(cx, type); public override bool NeedsPopulation => base.NeedsPopulation || symbol.TypeKind == TypeKind.Error; @@ -29,7 +29,8 @@ public override void Populate(TextWriter trapFile) return; } - trapFile.typeref_type((NamedTypeRef)TypeRef, this); + if (UsesTypeRef) + trapFile.typeref_type((NamedTypeRef)TypeRef, this); if (symbol.IsGenericType) { @@ -106,10 +107,25 @@ public override IEnumerable Locations public override Microsoft.CodeAnalysis.Location ReportingLocation => GetLocations(symbol).FirstOrDefault(); + bool IsAnonymousType() => symbol.IsAnonymousType || symbol.Name.Contains("__AnonymousType"); + public override void WriteId(TextWriter trapFile) { - symbol.BuildTypeId(Context, trapFile, (cx0, tb0, sub) => tb0.WriteSubId(Create(cx0, sub))); - trapFile.Write(";type"); + if (IsAnonymousType()) + trapFile.Write('*'); + else + { + symbol.BuildTypeId(Context, trapFile, symbol); + trapFile.Write(";type"); + } + } + + public override void WriteQuotedId(TextWriter trapFile) + { + if (IsAnonymousType()) + trapFile.Write('*'); + else + base.WriteQuotedId(trapFile); } /// @@ -148,7 +164,13 @@ class NamedTypeFactory : ICachedEntityFactory public NamedType Create(Context cx, INamedTypeSymbol init) => new NamedType(cx, init); } - public override Type TypeRef => NamedTypeRef.Create(Context, symbol); + // Do not create typerefs of constructed generics as they are always in the current trap file. + // Create typerefs for constructed error types in case they are fully defined elsewhere. + // We cannot use `!this.NeedsPopulation` because this would not be stable as it would depend on + // the assembly that was being extracted at the time. + bool UsesTypeRef => symbol.TypeKind == TypeKind.Error || SymbolEqualityComparer.Default.Equals(symbol.OriginalDefinition, symbol); + + public override Type TypeRef => UsesTypeRef ? (Type)NamedTypeRef.Create(Context, symbol) : this; } class NamedTypeRef : Type @@ -161,7 +183,9 @@ public NamedTypeRef(Context cx, INamedTypeSymbol symbol) : base(cx, symbol) } public static NamedTypeRef Create(Context cx, INamedTypeSymbol type) => - NamedTypeRefFactory.Instance.CreateEntity2(cx, type); + // We need to use a different cache key than `type` to avoid mixing up + // `NamedType`s and `NamedTypeRef`s + NamedTypeRefFactory.Instance.CreateEntity(cx, (typeof(NamedTypeRef), new SymbolEqualityWrapper(type)), type); class NamedTypeRefFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NullType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NullType.cs index 7ef079b932e4..6e97233d596a 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NullType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NullType.cs @@ -27,7 +27,7 @@ public override bool Equals(object obj) return obj != null && obj.GetType() == typeof(NullType); } - public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateNullableEntity(cx, null), NullableAnnotation.None); + public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateEntity(cx, typeof(NullType), null), NullableAnnotation.None); class NullTypeFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Nullability.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Nullability.cs index 05a4300133a4..5bd952ea93ea 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Nullability.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Nullability.cs @@ -127,7 +127,7 @@ public override void WriteId(TextWriter trapFile) symbol.WriteId(trapFile); } - public static NullabilityEntity Create(Context cx, Nullability init) => NullabilityFactory.Instance.CreateEntity(cx, init); + public static NullabilityEntity Create(Context cx, Nullability init) => NullabilityFactory.Instance.CreateEntity(cx, init, init); class NullabilityFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/PointerType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/PointerType.cs index db8b5ff8c23b..229666e6260e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/PointerType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/PointerType.cs @@ -29,7 +29,7 @@ public override void Populate(TextWriter trapFile) public Type PointedAtType { get; private set; } - public static PointerType Create(Context cx, IPointerTypeSymbol symbol) => PointerTypeFactory.Instance.CreateEntity(cx, symbol); + public static PointerType Create(Context cx, IPointerTypeSymbol symbol) => PointerTypeFactory.Instance.CreateEntityFromSymbol(cx, symbol); class PointerTypeFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TupleType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TupleType.cs index de6ea2170849..f1dfa7e1fc8a 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TupleType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TupleType.cs @@ -13,7 +13,7 @@ namespace Semmle.Extraction.CSharp.Entities /// class TupleType : Type { - public static TupleType Create(Context cx, INamedTypeSymbol type) => TupleTypeFactory.Instance.CreateEntity(cx, type); + public static TupleType Create(Context cx, INamedTypeSymbol type) => TupleTypeFactory.Instance.CreateEntityFromSymbol(cx, type); class TupleTypeFactory : ICachedEntityFactory { @@ -32,7 +32,7 @@ class TupleTypeFactory : ICachedEntityFactory public override void WriteId(TextWriter trapFile) { - symbol.BuildTypeId(Context, trapFile, (cx0, tb0, sub) => tb0.WriteSubId(Create(cx0, sub))); + symbol.BuildTypeId(Context, trapFile, symbol); trapFile.Write(";tuple"); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs index 7d9f5e3651e8..5899af36f38c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs @@ -100,19 +100,14 @@ protected void PopulateType(TextWriter trapFile) // Visit base types var baseTypes = new List(); - if (symbol.BaseType != null) + if (symbol.GetNonObjectBaseType(Context) is INamedTypeSymbol @base) { - Type baseKey = Create(Context, symbol.BaseType); + Type baseKey = Create(Context, @base); trapFile.extend(this, baseKey.TypeRef); if (symbol.TypeKind != TypeKind.Struct) baseTypes.Add(baseKey); } - if (symbol.TypeKind == TypeKind.Interface) - { - trapFile.extend(this, Create(Context, Context.Compilation.ObjectType)); - } - if (!(base.symbol is IArrayTypeSymbol)) { foreach (var t in base.symbol.Interfaces.Select(i => Create(Context, i))) @@ -298,7 +293,9 @@ class DelegateTypeParameter : Parameter : base(cx, init, parent, original) { } new public static DelegateTypeParameter Create(Context cx, IParameterSymbol param, IEntity parent, Parameter original = null) => - DelegateTypeParameterFactory.Instance.CreateEntity(cx, (param, parent, original)); + // We need to use a different cache key than `param` to avoid mixing up + // `DelegateTypeParameter`s and `Parameter`s + DelegateTypeParameterFactory.Instance.CreateEntity(cx, (typeof(DelegateTypeParameter), new SymbolEqualityWrapper(param)), (param, parent, original)); class DelegateTypeParameterFactory : ICachedEntityFactory<(IParameterSymbol, IEntity, Parameter), DelegateTypeParameter> { @@ -324,6 +321,14 @@ public virtual IEnumerable TypeMentions } public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.NoLabel; + + public override bool Equals(object obj) + { + var other = obj as Type; + return other?.GetType() == GetType() && SymbolEqualityComparer.IncludeNullability.Equals(other.symbol, symbol); + } + + public override int GetHashCode() => SymbolEqualityComparer.IncludeNullability.GetHashCode(symbol); } abstract class Type : Type where T : ITypeSymbol diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameter.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameter.cs index 721583784548..8e2cbdb0e1f5 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameter.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/TypeParameter.cs @@ -38,17 +38,11 @@ public override void Populate(TextWriter trapFile) if (symbol.HasUnmanagedTypeConstraint) trapFile.general_type_parameter_constraints(constraints, 4); - ITypeSymbol baseType = symbol.HasValueTypeConstraint ? - Context.Compilation.GetTypeByMetadataName(valueTypeName) : - Context.Compilation.ObjectType; - if (symbol.ReferenceTypeConstraintNullableAnnotation == NullableAnnotation.Annotated) trapFile.general_type_parameter_constraints(constraints, 5); foreach (var abase in symbol.GetAnnotatedTypeConstraints()) { - if (abase.Symbol.TypeKind != TypeKind.Interface) - baseType = abase.Symbol; var t = Create(Context, abase.Symbol); trapFile.specific_type_parameter_constraints(constraints, t.TypeRef); if (!abase.HasObliviousNullability()) @@ -56,7 +50,6 @@ public override void Populate(TextWriter trapFile) } trapFile.types(this, Kinds.TypeKind.TYPE_PARAMETER, symbol.Name); - trapFile.extend(this, Create(Context, baseType).TypeRef); Namespace parentNs = Namespace.Create(Context, symbol.TypeParameterKind == TypeParameterKind.Method ? Context.Compilation.GlobalNamespace : symbol.ContainingNamespace); trapFile.parent_namespace(this, parentNs); @@ -88,7 +81,7 @@ public override void Populate(TextWriter trapFile) } static public TypeParameter Create(Context cx, ITypeParameterSymbol p) => - TypeParameterFactory.Instance.CreateEntity(cx, p); + TypeParameterFactory.Instance.CreateEntityFromSymbol(cx, p); /// /// The variance of this type parameter. diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/UserOperator.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/UserOperator.cs index 2a64a29fa0d2..0d3bf96c799c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/UserOperator.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/UserOperator.cs @@ -49,13 +49,6 @@ public override Type ContainingType } } - public override void WriteId(TextWriter trapFile) - { - AddSignatureTypeToId(Context, trapFile, symbol, symbol.ReturnType); // Needed for op_explicit(), which differs only by return type. - trapFile.Write(' '); - BuildMethodId(this, trapFile); - } - /// /// For some reason, some operators are missing from the Roslyn database of mscorlib. /// This method returns true for such operators. @@ -68,7 +61,7 @@ bool IsImplicitOperator(out ITypeSymbol containingType) if (containingType != null) { var containingNamedType = containingType as INamedTypeSymbol; - return containingNamedType == null || !containingNamedType.MemberNames.Contains(symbol.Name); + return containingNamedType == null || !containingNamedType.GetMembers(symbol.Name).Contains(symbol); } var pointerType = symbol.Parameters.Select(p => p.Type).OfType().FirstOrDefault(); @@ -190,7 +183,7 @@ public static string OperatorSymbol(Context cx, string methodName) return result; } - public new static UserOperator Create(Context cx, IMethodSymbol symbol) => UserOperatorFactory.Instance.CreateEntity(cx, symbol); + public new static UserOperator Create(Context cx, IMethodSymbol symbol) => UserOperatorFactory.Instance.CreateEntityFromSymbol(cx, symbol); class UserOperatorFactory : ICachedEntityFactory { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor.cs index 94fe70f035f2..1b5bf15d1872 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor.cs @@ -76,7 +76,7 @@ public static ExitCode Run(string[] args) return ExitCode.Ok; } - using (var analyser = new Analyser(new LogProgressMonitor(logger), logger)) + using (var analyser = new Analyser(new LogProgressMonitor(logger), logger, commandLineArguments.AssemblySensitiveTrap)) using (var references = new BlockingCollection()) { try @@ -318,7 +318,7 @@ public static void ExtractStandalone( ILogger logger, CommonOptions options) { - using (var analyser = new Analyser(pm, logger)) + using (var analyser = new Analyser(pm, logger, false)) using (var references = new BlockingCollection()) { try diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Options.cs b/csharp/extractor/Semmle.Extraction.CSharp/Options.cs index 8fa7be08b675..319c2db68c71 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Options.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Options.cs @@ -26,6 +26,11 @@ public sealed class Options : CommonOptions /// public bool ClrTracer = false; + /// + /// Holds if assembly information should be prefixed to TRAP labels. + /// + public bool AssemblySensitiveTrap = false; + public static Options CreateWithEnvironment(string[] arguments) { var options = new Options(); @@ -75,6 +80,9 @@ public override bool handleFlag(string flag, bool value) case "clrtracer": ClrTracer = value; return true; + case "assemblysensitivetrap": + AssemblySensitiveTrap = value; + return true; default: return base.handleFlag(flag, value); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs index 6050ad910a52..9b8e4a266659 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs @@ -85,41 +85,62 @@ public static IEnumerable GetSourceLevelModifiers(this ISymbol symbol) } /// - /// Holds if this type symbol contains a type parameter from the - /// declaring generic . + /// Holds if the ID generated for `dependant` will contain a reference to + /// the ID for `symbol`. If this is the case, then the ID for `symbol` must + /// not contain a reference back to `dependant`. /// - public static bool ContainsTypeParameters(this ITypeSymbol type, Context cx, ISymbol declaringGeneric) + public static bool IdDependsOn(this ITypeSymbol dependant, Context cx, ISymbol symbol) { - using (cx.StackGuard) + var seen = new HashSet(SymbolEqualityComparer.Default); + + bool IdDependsOnImpl(ITypeSymbol type) { - switch (type.TypeKind) + if (SymbolEqualityComparer.Default.Equals(type, symbol)) + return true; + + if (type is null || seen.Contains(type)) + return false; + + seen.Add(type); + + using (cx.StackGuard) { - case TypeKind.Array: - var array = (IArrayTypeSymbol)type; - return array.ElementType.ContainsTypeParameters(cx, declaringGeneric); - case TypeKind.Class: - case TypeKind.Interface: - case TypeKind.Struct: - case TypeKind.Enum: - case TypeKind.Delegate: - case TypeKind.Error: - var named = (INamedTypeSymbol)type; - if (named.IsTupleType) - named = named.TupleUnderlyingType; - if (named.ContainingType != null && named.ContainingType.ContainsTypeParameters(cx, declaringGeneric)) - return true; - return named.TypeArguments.Any(arg => arg.ContainsTypeParameters(cx, declaringGeneric)); - case TypeKind.Pointer: - var ptr = (IPointerTypeSymbol)type; - return ptr.PointedAtType.ContainsTypeParameters(cx, declaringGeneric); - case TypeKind.TypeParameter: - var tp = (ITypeParameterSymbol)type; - var declaringGen = tp.TypeParameterKind == TypeParameterKind.Method ? tp.DeclaringMethod : (ISymbol)tp.DeclaringType; - return SymbolEqualityComparer.Default.Equals(declaringGen, declaringGeneric); - default: - return false; + switch (type.TypeKind) + { + case TypeKind.Array: + var array = (IArrayTypeSymbol)type; + return IdDependsOnImpl(array.ElementType); + case TypeKind.Class: + case TypeKind.Interface: + case TypeKind.Struct: + case TypeKind.Enum: + case TypeKind.Delegate: + case TypeKind.Error: + var named = (INamedTypeSymbol)type; + if (named.IsTupleType) + named = named.TupleUnderlyingType; + if (IdDependsOnImpl(named.ContainingType)) + return true; + if (IdDependsOnImpl(named.GetNonObjectBaseType(cx))) + return true; + if (IdDependsOnImpl(named.ConstructedFrom)) + return true; + return named.TypeArguments.Any(IdDependsOnImpl); + case TypeKind.Pointer: + var ptr = (IPointerTypeSymbol)type; + return IdDependsOnImpl(ptr.PointedAtType); + case TypeKind.TypeParameter: + var tp = (ITypeParameterSymbol)type; + return tp.ContainingSymbol is ITypeSymbol cont + ? IdDependsOnImpl(cont) + : SymbolEqualityComparer.Default.Equals(tp.ContainingSymbol, symbol); + default: + return false; + } } } + + return IdDependsOnImpl(dependant); } /// @@ -130,27 +151,19 @@ public static bool ContainsTypeParameters(this ITypeSymbol type, Context cx, ISy /// /// The extraction context. /// The trap builder used to store the result. - /// The action to apply to syntactic sub terms of this type. - public static void BuildTypeId(this ITypeSymbol type, Context cx, TextWriter trapFile, Action subTermAction) - { - if (type.SpecialType != SpecialType.None) - { - /* - * Use the keyword ("int" etc) for the built-in types. - * This makes the IDs shorter and means that all built-in types map to - * the same entities (even when using multiple versions of mscorlib). - */ - trapFile.Write(type.ToDisplayString()); - return; - } + /// The outer symbol being defined (to avoid recursive ids). + public static void BuildTypeId(this ITypeSymbol type, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined) => + type.BuildTypeId(cx, trapFile, symbolBeingDefined, true); + static void BuildTypeId(this ITypeSymbol type, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined, bool addBaseClass) + { using (cx.StackGuard) { switch (type.TypeKind) { case TypeKind.Array: var array = (IArrayTypeSymbol)type; - subTermAction(cx, trapFile, array.ElementType); + array.ElementType.BuildOrWriteId(cx, trapFile, symbolBeingDefined, addBaseClass); array.BuildArraySuffix(trapFile); return; case TypeKind.Class: @@ -160,15 +173,17 @@ public static void BuildTypeId(this ITypeSymbol type, Context cx, TextWriter tra case TypeKind.Delegate: case TypeKind.Error: var named = (INamedTypeSymbol)type; - named.BuildNamedTypeId(cx, trapFile, subTermAction); + named.BuildNamedTypeId(cx, trapFile, symbolBeingDefined, addBaseClass); return; case TypeKind.Pointer: var ptr = (IPointerTypeSymbol)type; - subTermAction(cx, trapFile, ptr.PointedAtType); + ptr.PointedAtType.BuildOrWriteId(cx, trapFile, symbolBeingDefined, addBaseClass); trapFile.Write('*'); return; case TypeKind.TypeParameter: var tp = (ITypeParameterSymbol)type; + tp.ContainingSymbol.BuildOrWriteId(cx, trapFile, symbolBeingDefined, addBaseClass); + trapFile.Write('_'); trapFile.Write(tp.Name); return; case TypeKind.Dynamic: @@ -180,6 +195,42 @@ public static void BuildTypeId(this ITypeSymbol type, Context cx, TextWriter tra } } + static void BuildOrWriteId(this ISymbol symbol, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined, bool addBaseClass) + { + // We need to keep track of the symbol being defined in order to avoid cyclic labels. + // For example, in + // + // ```csharp + // class C : IEnumerable { } + // ``` + // + // when we generate the label for ``C`1``, the base class `IEnumerable` has `T` as a type + // argument, which will be qualified with `__self__` instead of the label we are defining. + // In effect, the label will (simplified) look like + // + // ``` + // #123 = @"C`1 : IEnumerable<__self___T>" + // ``` + if (SymbolEqualityComparer.Default.Equals(symbol, symbolBeingDefined)) + trapFile.Write("__self__"); + else if (symbol is ITypeSymbol type && type.IdDependsOn(cx, symbolBeingDefined)) + type.BuildTypeId(cx, trapFile, symbolBeingDefined, addBaseClass); + else + trapFile.WriteSubId(CreateEntity(cx, symbol)); + } + + /// + /// Adds an appropriate ID to the trap builder + /// for the symbol belonging to + /// . + /// + /// This will either write a reference to the ID of the entity belonging to + /// (`{#label}`), or if that will lead to cyclic IDs, + /// it will generate an appropriate ID that encodes the signature of + /// . + /// + public static void BuildOrWriteId(this ISymbol symbol, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined) => + symbol.BuildOrWriteId(cx, trapFile, symbolBeingDefined, true); /// /// Constructs an array suffix string for this array type symbol. @@ -211,11 +262,8 @@ private static void BuildAssembly(IAssemblySymbol asm, TextWriter trapFile, bool trapFile.Write("::"); } - static void BuildNamedTypeId(this INamedTypeSymbol named, Context cx, TextWriter trapFile, Action subTermAction) + static void BuildNamedTypeId(this INamedTypeSymbol named, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined, bool addBaseClass) { - bool prefixAssembly = true; - if (named.ContainingAssembly is null) prefixAssembly = false; - if (named.IsTupleType) { trapFile.Write('('); @@ -224,48 +272,71 @@ static void BuildNamedTypeId(this INamedTypeSymbol named, Context cx, TextWriter { trapFile.Write(f.Name); trapFile.Write(":"); - subTermAction(cx, tb0, f.Type); + f.Type.BuildOrWriteId(cx, tb0, symbolBeingDefined, addBaseClass); } ); trapFile.Write(")"); return; } - if (named.ContainingType != null) + void AddContaining() { - subTermAction(cx, trapFile, named.ContainingType); - trapFile.Write('.'); - } - else if (named.ContainingNamespace != null) - { - if (prefixAssembly) - BuildAssembly(named.ContainingAssembly, trapFile); - named.ContainingNamespace.BuildNamespace(cx, trapFile); + if (named.ContainingType != null) + { + named.ContainingType.BuildOrWriteId(cx, trapFile, symbolBeingDefined, addBaseClass); + trapFile.Write('.'); + } + else if (named.ContainingNamespace != null) + { + if (cx.ShouldAddAssemblyTrapPrefix && named.ContainingAssembly is object) + BuildAssembly(named.ContainingAssembly, trapFile); + named.ContainingNamespace.BuildNamespace(cx, trapFile); + } } - if (named.IsAnonymousType) - named.BuildAnonymousName(cx, trapFile, subTermAction, true); - else if (named.TypeParameters.IsEmpty) + if (named.TypeParameters.IsEmpty) + { + AddContaining(); trapFile.Write(named.Name); - else if (IsReallyUnbound(named)) + } + else if (named.IsReallyUnbound()) { + AddContaining(); trapFile.Write(named.Name); trapFile.Write("`"); trapFile.Write(named.TypeParameters.Length); } else { - subTermAction(cx, trapFile, named.ConstructedFrom); + named.ConstructedFrom.BuildOrWriteId(cx, trapFile, symbolBeingDefined, addBaseClass); trapFile.Write('<'); // Encode the nullability of the type arguments in the label. // Type arguments with different nullability can result in // a constructed type with different nullability of its members and methods, // so we need to create a distinct database entity for it. trapFile.BuildList(",", named.GetAnnotatedTypeArguments(), - (ta, tb0) => subTermAction(cx, tb0, ta.Symbol) + (ta, tb0) => ta.Symbol.BuildOrWriteId(cx, tb0, symbolBeingDefined, addBaseClass) ); trapFile.Write('>'); } + + if (addBaseClass && named.GetNonObjectBaseType(cx) is INamedTypeSymbol @base) + { + // We need to limit unfolding of base classes. For example, in + // + // ```csharp + // class C1 { } + // class C2 : C1> { } + // class C3 : C1> { } + // class C4 : C3 { } + // ``` + // + // when we generate the label for `C4`, the base class `C3` has itself `C1>` as + // a base class, which in turn has `C1>` as a base class. The latter has the original + // base class `C3` as a type argument, which would lead to infinite unfolding. + trapFile.Write(" : "); + @base.BuildOrWriteId(cx, trapFile, symbolBeingDefined, addBaseClass: false); + } } static void BuildNamespace(this INamespaceSymbol ns, Context cx, TextWriter trapFile) @@ -274,22 +345,14 @@ static void BuildNamespace(this INamespaceSymbol ns, Context cx, TextWriter trap trapFile.Write('.'); } - static void BuildAnonymousName(this ITypeSymbol type, Context cx, TextWriter trapFile, Action subTermAction, bool includeParamName) + static void BuildAnonymousName(this INamedTypeSymbol type, Context cx, TextWriter trapFile) { - var buildParam = includeParamName - ? (prop, tb0) => - { - tb0.Write(prop.Name); - tb0.Write(' '); - subTermAction(cx, tb0, prop.Type); - } - : (Action)((prop, tb0) => subTermAction(cx, tb0, prop.Type)); int memberCount = type.GetMembers().OfType().Count(); int hackTypeNumber = memberCount == 1 ? 1 : 0; trapFile.Write("<>__AnonType"); trapFile.Write(hackTypeNumber); trapFile.Write('<'); - trapFile.BuildList(",", type.GetMembers().OfType(), buildParam); + trapFile.BuildList(",", type.GetMembers().OfType(), (prop, tb0) => BuildDisplayName(prop.Type, cx, tb0)); trapFile.Write('>'); } @@ -354,11 +417,9 @@ public static void BuildNamedTypeDisplayName(this INamedTypeSymbol namedType, Co } if (namedType.IsAnonymousType) - { - namedType.BuildAnonymousName(cx, trapFile, (cx0, tb0, sub) => sub.BuildDisplayName(cx0, tb0), false); - } - - trapFile.Write(namedType.Name); + namedType.BuildAnonymousName(cx, trapFile); + else + trapFile.Write(namedType.Name); if (namedType.IsGenericType && namedType.TypeKind != TypeKind.Error && namedType.TypeArguments.Any()) { trapFile.Write('<'); @@ -434,6 +495,13 @@ public static bool IsSourceDeclaration(this IParameterSymbol parameter) return true; } + /// + /// Gets the base type of `symbol`. Unlike `symbol.BaseType`, this excludes effective base + /// types of type parameters as well as `object` base types. + /// + public static INamedTypeSymbol GetNonObjectBaseType(this ITypeSymbol symbol, Context cx) => + symbol is ITypeParameterSymbol || SymbolEqualityComparer.Default.Equals(symbol.BaseType, cx.Compilation.ObjectType) ? null : symbol.BaseType; + public static IEntity CreateEntity(this Context cx, ISymbol symbol) { if (symbol == null) return null; diff --git a/csharp/extractor/Semmle.Extraction/Context.cs b/csharp/extractor/Semmle.Extraction/Context.cs index 85813400846c..76097a0c12c6 100644 --- a/csharp/extractor/Semmle.Extraction/Context.cs +++ b/csharp/extractor/Semmle.Extraction/Context.cs @@ -41,42 +41,12 @@ public SemanticModel GetModel(SyntaxNode node) /// public readonly TrapWriter TrapWriter; - int GetNewId() => TrapWriter.IdCounter++; - - /// - /// Creates a new entity using the factory. - /// - /// The entity factory. - /// The initializer for the entity. - /// The new/existing entity. - public Entity CreateEntity(ICachedEntityFactory factory, Type init) where Entity : ICachedEntity where Type : struct - { - return CreateNonNullEntity(factory, init); - } - /// - /// Creates a new entity using the factory. + /// Holds if assembly information should be prefixed to TRAP labels. /// - /// The entity factory. - /// The initializer for the entity. - /// The new/existing entity. - public Entity CreateNullableEntity(ICachedEntityFactory factory, Type init) where Entity : ICachedEntity - { - return init == null ? CreateEntity2(factory, init) : CreateNonNullEntity(factory, init); - } + public readonly bool ShouldAddAssemblyTrapPrefix; - /// - /// Creates a new entity using the factory. - /// - /// The entity factory. - /// The initializer for the entity. - /// The new/existing entity. - public Entity CreateEntityFromSymbol(ICachedEntityFactory factory, Type init) - where Entity : ICachedEntity - where Type : ISymbol - { - return init == null ? CreateEntity2(factory, init) : CreateNonNullEntity(factory, init); - } + int GetNewId() => TrapWriter.IdCounter++; // A recursion guard against writing to the trap file whilst writing an id to the trap file. bool WritingLabel = false; @@ -102,47 +72,6 @@ public void DefineLabel(IEntity entity, TextWriter trapFile, IExtractor extracto } } - /// - /// Creates a new entity using the factory. - /// Uses a different cache to , - /// and can store null values. - /// - /// The entity factory. - /// The initializer for the entity. - /// The new/existing entity. - public Entity CreateEntity2(ICachedEntityFactory factory, Type init) where Entity : ICachedEntity - { - using (StackGuard) - { - var entity = factory.Create(this, init); - - if (entityLabelCache.TryGetValue(entity, out var label)) - { - entity.Label = label; - } - else - { - label = GetNewLabel(); - entity.Label = label; - entityLabelCache[entity] = label; - - DefineLabel(entity, TrapWriter.Writer, Extractor); - - if (entity.NeedsPopulation) - Populate(init as ISymbol, entity); -#if DEBUG_LABELS - using (var id = new StringWriter()) - { - entity.WriteId(id); - CheckEntityHasUniqueLabel(id.ToString(), entity); - } -#endif - - } - return entity; - } - } - #if DEBUG_LABELS private void CheckEntityHasUniqueLabel(string id, ICachedEntity entity) { @@ -159,12 +88,27 @@ private void CheckEntityHasUniqueLabel(string id, ICachedEntity entity) public Label GetNewLabel() => new Label(GetNewId()); - public Entity CreateNonNullEntity(ICachedEntityFactory factory, Type init) + public Entity CreateEntity(ICachedEntityFactory factory, object cacheKey, Type init) + where Entity : ICachedEntity => + cacheKey is ISymbol s ? CreateEntity(factory, s, init, symbolEntityCache) : CreateEntity(factory, cacheKey, init, objectEntityCache); + + public Entity CreateEntityFromSymbol(ICachedEntityFactory factory, Type init) + where Type : ISymbol + where Entity : ICachedEntity => CreateEntity(factory, init, init, symbolEntityCache); + + /// + /// Creates and populates a new entity, or returns the existing one from the cache. + /// + /// The entity factory. + /// The key used for caching. + /// The initializer for the entity. + /// The dictionary to use for caching. + /// The new/existing entity. + Entity CreateEntity(ICachedEntityFactory factory, CacheKeyType cacheKey, Type init, IDictionary dictionary) + where CacheKeyType : notnull where Entity : ICachedEntity { - if (init is null) throw new ArgumentException("Unexpected null value", nameof(init)); - - if (objectEntityCache.TryGetValue(init, out var cached)) + if (dictionary.TryGetValue(cacheKey, out var cached)) return (Entity)cached; using (StackGuard) @@ -173,7 +117,7 @@ public Entity CreateNonNullEntity(ICachedEntityFactory idLabelCache = new Dictionary(); #endif - readonly Dictionary objectEntityCache = new Dictionary(); - readonly Dictionary entityLabelCache = new Dictionary(); + + readonly IDictionary objectEntityCache = new Dictionary(); + readonly IDictionary symbolEntityCache = new Dictionary(10000, SymbolEqualityComparer.IncludeNullability); readonly HashSet