diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs index 4ad05eea3833..affb8b224a3d 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs @@ -72,15 +72,14 @@ public static Assembly CreateOutputAssembly(Context cx) public override void WriteId(EscapingTextWriter trapFile) { - if (isOutputAssembly && Context.ExtractionContext.IsStandalone) + if (Context.ExtractionContext.IsStandalone) { - trapFile.Write("buildlessOutputAssembly"); - } - else - { - trapFile.Write(assembly.ToString()); + WriteStarId(trapFile); + return; } + trapFile.Write(assembly.ToString()); + if (assemblyPath is not null) { trapFile.Write("#file:///"); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs index c3ce2bb6d29e..dfab00846d83 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs @@ -29,7 +29,10 @@ public override void Populate(TextWriter trapFile) ContainingType!.PopulateGenerics(); trapFile.constructors(this, Symbol.ContainingType.Name, ContainingType, (Constructor)OriginalDefinition); - trapFile.constructor_location(this, Location); + if (Context.ExtractLocation(Symbol)) + { + trapFile.constructor_location(this, Location); + } if (MakeSynthetic) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/EmptyLocation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/EmptyLocation.cs new file mode 100644 index 000000000000..1ceff87b6b17 --- /dev/null +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/EmptyLocation.cs @@ -0,0 +1,58 @@ +using System.IO; +using System.Threading; + +namespace Semmle.Extraction.CSharp.Entities +{ + public class EmptyLocation : SourceLocation + { + private readonly File generatedFile; + + // The Ql library assumes the presence of a single "empty" location. + private static EmptyLocation? instance; + private static readonly Lock instanceLock = new(); + + private EmptyLocation(Context cx) + : base(cx, null) + { + generatedFile = GeneratedFile.Create(cx); + } + + public override void Populate(TextWriter trapFile) + { + trapFile.locations_default(this, generatedFile, 0, 0, 0, 0); + } + + public override void WriteId(EscapingTextWriter trapFile) + { + if (Context.ExtractionContext.IsStandalone) + { + WriteStarId(trapFile); + return; + } + + trapFile.Write("loc,"); + trapFile.WriteSubId(generatedFile); + trapFile.Write(",0,0,0,0"); + } + + public override int GetHashCode() => 98732567; + + public override bool Equals(object? obj) => obj is not null && obj.GetType() == typeof(EmptyLocation); + + public static EmptyLocation Create(Context cx) + { + lock (instanceLock) + { + instance ??= EmptyLocationFactory.Instance.CreateEntity(cx, typeof(EmptyLocation), null); + return instance; + } + } + + private class EmptyLocationFactory : CachedEntityFactory + { + public static EmptyLocationFactory Instance { get; } = new EmptyLocationFactory(); + + public override EmptyLocation Create(Context cx, string? init) => new EmptyLocation(cx); + } + } +} diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/GeneratedLocation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/GeneratedLocation.cs deleted file mode 100644 index d12f1ca51e00..000000000000 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/GeneratedLocation.cs +++ /dev/null @@ -1,40 +0,0 @@ -using System.IO; - -namespace Semmle.Extraction.CSharp.Entities -{ - public class GeneratedLocation : SourceLocation - { - private readonly File generatedFile; - - private GeneratedLocation(Context cx) - : base(cx, null) - { - generatedFile = GeneratedFile.Create(cx); - } - - public override void Populate(TextWriter trapFile) - { - trapFile.locations_default(this, generatedFile, 0, 0, 0, 0); - } - - public override void WriteId(EscapingTextWriter trapFile) - { - trapFile.Write("loc,"); - trapFile.WriteSubId(generatedFile); - trapFile.Write(",0,0,0,0"); - } - - public override int GetHashCode() => 98732567; - - public override bool Equals(object? obj) => obj is not null && obj.GetType() == typeof(GeneratedLocation); - - public static GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, typeof(GeneratedLocation), null); - - private class GeneratedLocationFactory : CachedEntityFactory - { - public static GeneratedLocationFactory Instance { get; } = new GeneratedLocationFactory(); - - public override GeneratedLocation Create(Context cx, string? init) => new GeneratedLocation(cx); - } - } -} diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/Location.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/Location.cs index 9f9e15e33f33..0ba8aeaa4187 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/Location.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/Location.cs @@ -5,7 +5,24 @@ public abstract class Location : CachedEntity { #nullable restore warnings protected Location(Context cx, Microsoft.CodeAnalysis.Location? init) - : base(cx, init) { } + : base(cx, init) + { } + + protected static void WriteStarId(EscapingTextWriter writer) + { + writer.Write('*'); + } + + public sealed override void WriteQuotedId(EscapingTextWriter writer) + { + if (Context.ExtractionContext.IsStandalone) + { + WriteStarId(writer); + return; + } + + base.WriteQuotedId(writer); + } public override Microsoft.CodeAnalysis.Location? ReportingLocation => Symbol; diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/NonGeneratedSourceLocation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/NonGeneratedSourceLocation.cs index 69e9ea4e9dc7..22a8277b9495 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/NonGeneratedSourceLocation.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Locations/NonGeneratedSourceLocation.cs @@ -42,6 +42,12 @@ public File FileEntity public override void WriteId(EscapingTextWriter trapFile) { + if (Context.ExtractionContext.IsStandalone) + { + WriteStarId(trapFile); + return; + } + trapFile.Write("loc,"); trapFile.WriteSubId(FileEntity); trapFile.Write(','); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs index bd3a637a624c..edd20db63d63 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/OrdinaryMethod.cs @@ -43,8 +43,11 @@ public override void Populate(TextWriter trapFile) } } - foreach (var l in Locations) - trapFile.method_location(this, l); + if (Context.ExtractLocation(Symbol)) + { + foreach (var l in Locations) + trapFile.method_location(this, l); + } PopulateGenerics(trapFile); Overrides(trapFile); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs index 76d518776caa..5c86f4904100 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Parameter.cs @@ -247,7 +247,7 @@ public override void Populate(TextWriter trapFile) var typeKey = VarargsType.Create(Context); // !! Maybe originaldefinition is wrong trapFile.@params(this, "", typeKey, Ordinal, Kind.None, Parent!, this); - trapFile.param_location(this, GeneratedLocation.Create(Context)); + trapFile.param_location(this, EmptyLocation.Create(Context)); } protected override int Ordinal => ((Method)Parent!).OriginalDefinition.Symbol.Parameters.Length; diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs index 3ea99a0d772c..4c8660c172ab 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs @@ -238,6 +238,9 @@ private void DoAnalyseCompilation() compilationEntity = Entities.Compilation.Create(cx); + // Ensure that the empty location is always created. + Entities.EmptyLocation.Create(cx); + ExtractionContext.CompilationInfos.ForEach(ci => trapWriter.Writer.compilation_info(compilationEntity, ci.key, ci.value)); ReportProgressTaskDone(currentTaskId, assemblyPath, trapWriter.TrapFile, stopwatch.Elapsed, AnalysisAction.Extracted); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs index f231c8238a96..b7160cd1f638 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs @@ -550,6 +550,10 @@ public bool Defines(ISymbol symbol) => !SymbolEqualityComparer.Default.Equals(symbol, symbol.OriginalDefinition) || scope.InScope(symbol); + public bool ExtractLocation(ISymbol symbol) => + SymbolEqualityComparer.Default.Equals(symbol, symbol.OriginalDefinition) && + scope.InScope(symbol); + /// /// Runs the given action , guarding for trap duplication /// based on key . @@ -582,14 +586,14 @@ public void WithDuplicationGuard(Key key, Action a) public Entities.Location CreateLocation() { return SourceTree is null - ? Entities.GeneratedLocation.Create(this) + ? Entities.EmptyLocation.Create(this) : CreateLocation(Microsoft.CodeAnalysis.Location.Create(SourceTree, Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(0, 0))); } public Entities.Location CreateLocation(Microsoft.CodeAnalysis.Location? location) { return (location is null || location.Kind == LocationKind.None) - ? Entities.GeneratedLocation.Create(this) + ? Entities.EmptyLocation.Create(this) : location.IsInSource ? Entities.NonGeneratedSourceLocation.Create(this, location) : Entities.Assembly.Create(this, location); diff --git a/csharp/ql/lib/semmle/code/csharp/Callable.qll b/csharp/ql/lib/semmle/code/csharp/Callable.qll index 7e17f853913a..ef0d0673ce2a 100644 --- a/csharp/ql/lib/semmle/code/csharp/Callable.qll +++ b/csharp/ql/lib/semmle/code/csharp/Callable.qll @@ -265,7 +265,7 @@ class Method extends Callable, Virtualizable, Attributable, @method { result = Virtualizable.super.getAnUltimateImplementor() } - override Location getALocation() { method_location(this, result) } + override Location getALocation() { method_location(this.getUnboundDeclaration(), result) } /** Holds if this method is an extension method. */ predicate isExtensionMethod() { this.getParameter(0).hasExtensionMethodModifier() } diff --git a/csharp/ql/test/library-tests/locations/A.cs b/csharp/ql/test/library-tests/locations/A.cs new file mode 100644 index 000000000000..fe3a90057e5b --- /dev/null +++ b/csharp/ql/test/library-tests/locations/A.cs @@ -0,0 +1,18 @@ +using System; + +public abstract class A +{ + public void Apply(T t) { } + public abstract object ToObject(T t); +} + +public class A2 : A +{ + public override object ToObject(string t) => t; + + public void M() + { + A2 other = new(); + other.Apply(""); + } +} diff --git a/csharp/ql/test/library-tests/locations/B.cs b/csharp/ql/test/library-tests/locations/B.cs new file mode 100644 index 000000000000..86f45e1d50b0 --- /dev/null +++ b/csharp/ql/test/library-tests/locations/B.cs @@ -0,0 +1,6 @@ +using System; + +public class B : A +{ + public override object ToObject(int t) => t; +} diff --git a/csharp/ql/test/library-tests/locations/C.cs b/csharp/ql/test/library-tests/locations/C.cs new file mode 100644 index 000000000000..a4063646d53f --- /dev/null +++ b/csharp/ql/test/library-tests/locations/C.cs @@ -0,0 +1,12 @@ +using System; + +class C +{ + public void M() + { + B b = new B(); + b.Apply(0); + A2 a2 = new A2(); + a2.Apply(""); + } +} diff --git a/csharp/ql/test/library-tests/locations/locations.expected b/csharp/ql/test/library-tests/locations/locations.expected new file mode 100644 index 000000000000..9b1111e40068 --- /dev/null +++ b/csharp/ql/test/library-tests/locations/locations.expected @@ -0,0 +1,10 @@ +| A.cs:3:23:3:26 | A | A.cs:5:17:5:21 | Apply | A.cs:5:17:5:21 | A.cs:5:17:5:21 | +| A.cs:3:23:3:26 | A | A.cs:6:28:6:35 | ToObject | A.cs:6:28:6:35 | A.cs:6:28:6:35 | +| A.cs:3:23:3:26 | A | A.cs:5:17:5:21 | Apply | A.cs:5:17:5:21 | A.cs:5:17:5:21 | +| A.cs:3:23:3:26 | A | A.cs:6:28:6:35 | ToObject | A.cs:6:28:6:35 | A.cs:6:28:6:35 | +| A.cs:3:23:3:26 | A`1 | A.cs:5:17:5:21 | Apply | A.cs:5:17:5:21 | A.cs:5:17:5:21 | +| A.cs:3:23:3:26 | A`1 | A.cs:6:28:6:35 | ToObject | A.cs:6:28:6:35 | A.cs:6:28:6:35 | +| A.cs:9:14:9:15 | A2 | A.cs:11:28:11:35 | ToObject | A.cs:11:28:11:35 | A.cs:11:28:11:35 | +| A.cs:9:14:9:15 | A2 | A.cs:13:17:13:17 | M | A.cs:13:17:13:17 | A.cs:13:17:13:17 | +| B.cs:3:14:3:14 | B | B.cs:5:28:5:35 | ToObject | B.cs:5:28:5:35 | B.cs:5:28:5:35 | +| C.cs:3:7:3:7 | C | C.cs:5:17:5:17 | M | C.cs:5:17:5:17 | C.cs:5:17:5:17 | diff --git a/csharp/ql/test/library-tests/locations/locations.ql b/csharp/ql/test/library-tests/locations/locations.ql new file mode 100644 index 000000000000..796b3776dd48 --- /dev/null +++ b/csharp/ql/test/library-tests/locations/locations.ql @@ -0,0 +1,5 @@ +import csharp + +from Type t, Method m, SourceLocation l +where t = m.getDeclaringType() and l = m.getLocation() and not l instanceof EmptyLocation +select t, m, l diff --git a/csharp/ql/test/library-tests/locations/options b/csharp/ql/test/library-tests/locations/options new file mode 100644 index 000000000000..7ba3811b2afb --- /dev/null +++ b/csharp/ql/test/library-tests/locations/options @@ -0,0 +1 @@ +semmle-extractor-options: --standalone