Merge pull request github#18159 from michaelnebel/csharp/publicprotected

michaelnebel · web-flow · commit 801f696bbbf1 · 2024-12-04T09:48:36.000+01:00
C#: Only extract public and protected members from metadata.
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs
@@ -642,5 +642,40 @@ public static AnnotatedTypeSymbol GetType(this Context cx, Microsoft.CodeAnalysi
         /// </summary>
         public static IEnumerable<AnnotatedTypeSymbol> GetAnnotatedTypeArguments(this INamedTypeSymbol symbol) =>
             symbol.TypeArguments.Zip(symbol.TypeArgumentNullableAnnotations, (t, a) => new AnnotatedTypeSymbol(t, a));
+
+        /// <summary>
+        /// Returns true if the symbol is public, protected or protected internal.
+        /// </summary>
+        public static bool IsPublicOrProtected(this ISymbol symbol) =>
+            symbol.DeclaredAccessibility == Accessibility.Public
+            || symbol.DeclaredAccessibility == Accessibility.Protected
+            || symbol.DeclaredAccessibility == Accessibility.ProtectedOrInternal;
+
+        /// <summary>
+        /// Returns true if the given symbol should be extracted.
+        /// </summary>
+        public static bool ShouldExtractSymbol(this ISymbol symbol)
+        {
+            // Extract all source symbols and public/protected metadata symbols.
+            if (symbol.Locations.Any(x => !x.IsInMetadata) || symbol.IsPublicOrProtected())
+            {
+                return true;
+            }
+            if (symbol is IMethodSymbol method)
+            {
+                return method.ExplicitInterfaceImplementations.Any(m => m.ContainingType.ShouldExtractSymbol());
+            }
+            if (symbol is IPropertySymbol property)
+            {
+                return property.ExplicitInterfaceImplementations.Any(m => m.ContainingType.ShouldExtractSymbol());
+            }
+            return false;
+        }
+
+        /// <summary>
+        /// Returns the symbols that should be extracted.
+        /// </summary>
+        public static IEnumerable<T> ExtractionCandidates<T>(this IEnumerable<T> symbols) where T : ISymbol =>
+            symbols.Where(symbol => symbol.ShouldExtractSymbol());
     }
 }
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs
@@ -101,7 +101,7 @@ public void Overrides(TextWriter trapFile)
                 }
             }
 
-            if (Symbol.OverriddenMethod is not null)
+            if (Symbol.OverriddenMethod is not null && Symbol.OverriddenMethod.ShouldExtractSymbol())
             {
                 trapFile.overrides(this, Method.Create(Context, Symbol.OverriddenMethod));
             }
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs
@@ -225,7 +225,7 @@ private void ExtractParametersForDelegateLikeType(TextWriter trapFile, IMethodSy
         }
 
         /// <summary>
-        /// Called to extract all members and nested types.
+        /// Called to extract members and nested types.
         /// This is called on each member of a namespace,
         /// in either source code or an assembly.
         /// </summary>
@@ -236,7 +236,7 @@ public void ExtractRecursive()
                 Context.BindComments(this, l);
             }
 
-            foreach (var member in Symbol.GetMembers())
+            foreach (var member in Symbol.GetMembers().ExtractionCandidates())
             {
                 switch (member.Kind)
                 {
@@ -262,16 +262,16 @@ public void PopulateGenerics()
 
                 var members = new List<ISymbol>();
 
-                foreach (var member in Symbol.GetMembers())
+                foreach (var member in Symbol.GetMembers().ExtractionCandidates())
                     members.Add(member);
-                foreach (var member in Symbol.GetTypeMembers())
+                foreach (var member in Symbol.GetTypeMembers().ExtractionCandidates())
                     members.Add(member);
 
                 // Mono extractor puts all BASE interface members as members of the current interface.
 
                 if (Symbol.TypeKind == TypeKind.Interface)
                 {
-                    foreach (var baseInterface in Symbol.Interfaces)
+                    foreach (var baseInterface in Symbol.Interfaces.ExtractionCandidates())
                     {
                         foreach (var member in baseInterface.GetMembers())
                             members.Add(member);
@@ -288,7 +288,7 @@ public void PopulateGenerics()
                 if (Symbol.BaseType is not null)
                     Create(Context, Symbol.BaseType).PopulateGenerics();
 
-                foreach (var i in Symbol.Interfaces)
+                foreach (var i in Symbol.Interfaces.ExtractionCandidates())
                 {
                     Create(Context, i).PopulateGenerics();
                 }
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Analyser.cs
@@ -269,7 +269,7 @@ private static void AnalyseNamespace(Context cx, INamespaceSymbol ns)
                 AnalyseNamespace(cx, memberNamespace);
             }
 
-            foreach (var memberType in ns.GetTypeMembers())
+            foreach (var memberType in ns.GetTypeMembers().ExtractionCandidates())
             {
                 Entities.Type.Create(cx, memberType).ExtractRecursive();
             }
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs
@@ -549,7 +549,6 @@ private static ExitCode AnalyseTracing(
                         compilerArguments.CompilationOptions
                             .WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default)
                             .WithStrongNameProvider(new DesktopStrongNameProvider(compilerArguments.KeyFileSearchPaths))
-                            .WithMetadataImportOptions(MetadataImportOptions.All)
                         );
                 },
                 (compilation, options) => analyser.EndInitialize(compilerArguments, options, compilation, cwd, args),
diff --git a/csharp/ql/lib/change-notes/2024-12-03-public-protected-reference.md b/csharp/ql/lib/change-notes/2024-12-03-public-protected-reference.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Only extract *public* and *protected* members from reference assemblies. This yields an approximate average speed-up of around 10% for extraction and query execution. Custom MaD rows using `Field`-based summaries may need to be changed to `SyntheticField`-based flows if they reference private fields.
diff --git a/csharp/ql/test/library-tests/conversion/operator/Operator.expected b/csharp/ql/test/library-tests/conversion/operator/Operator.expected
@@ -21,15 +21,13 @@
 | Int32 | Decimal |
 | Int32 | Index |
 | Int32 | Int128 |
-| Int32 | MetadataToken |
 | Int32 | NFloat |
 | Int64 | Decimal |
 | Int64 | Int128 |
 | Int64 | NFloat |
 | IntPtr | Int128 |
 | IntPtr | NFloat |
 | Memory`1 | ReadOnlyMemory<T> |
-| MetadataToken | Int32 |
 | NFloat | Double |
 | SByte | Decimal |
 | SByte | Half |
@@ -59,4 +57,3 @@
 | UIntPtr | Int128 |
 | UIntPtr | NFloat |
 | UIntPtr | UInt128 |
-| UnixFileMode | Nullable<UnixFileMode> |
diff --git a/csharp/ql/test/library-tests/conversion/reftype/RefType.expected b/csharp/ql/test/library-tests/conversion/reftype/RefType.expected
@@ -183,8 +183,6 @@
 | IReadOnlyList<T4> | dynamic |
 | Int16[] | Object |
 | Int16[] | dynamic |
-| Int32[,] | Object |
-| Int32[,] | dynamic |
 | Int32[] | Object |
 | Int32[] | dynamic |
 | Int64[] | Object |
@@ -223,8 +221,6 @@
 | T5 | C1 |
 | T5 | Object |
 | T5 | dynamic |
-| UInt16[][] | Object |
-| UInt16[][] | dynamic |
 | UInt32[] | Object |
 | UInt32[] | dynamic |
 | UInt64[] | Object |
@@ -283,7 +279,6 @@
 | null | IReadOnlyList<T3> |
 | null | IReadOnlyList<T4> |
 | null | Int16[] |
-| null | Int32[,] |
 | null | Int32[] |
 | null | Int64[] |
 | null | Object |
@@ -294,7 +289,6 @@
 | null | T4 |
 | null | T4[] |
 | null | T5 |
-| null | UInt16[][] |
 | null | UInt32[] |
 | null | UInt64[] |
 | null | dynamic |
diff --git a/csharp/ql/test/library-tests/csharp9/FunctionPointer.expected b/csharp/ql/test/library-tests/csharp9/FunctionPointer.expected
@@ -1,68 +1,33 @@
 type
 | file://:0:0:0:0 | delegate* default<A,B> | B | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<B,A> | A | DefaultCallingConvention |
-| file://:0:0:0:0 | delegate* default<Byte ref,Void> | Void | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | ref int | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | readonly int | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<Int32*,Void*> | Void* | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<Int32> | int | DefaultCallingConvention |
-| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | Void | DefaultCallingConvention |
-| file://:0:0:0:0 | delegate* default<Object,Void> | Void | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<T,Int32> | int | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* default<Void*,Int32*> | int* | DefaultCallingConvention |
-| file://:0:0:0:0 | delegate* default<Void*,Object> | object | DefaultCallingConvention |
 | file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | Void | StdCallCallingConvention |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | Void | UnmanagedCallingConvention |
-| file://:0:0:0:0 | delegate* unmanaged<Char*,IntPtr,Void> | Void | UnmanagedCallingConvention |
-| file://:0:0:0:0 | delegate* unmanaged<Int32,PosixSignal,Int32> | int | UnmanagedCallingConvention |
 | file://:0:0:0:0 | delegate* unmanaged<IntPtr,Int32> | int | UnmanagedCallingConvention |
 | file://:0:0:0:0 | delegate* unmanaged<IntPtr,Void> | Void | UnmanagedCallingConvention |
-| file://:0:0:0:0 | delegate* unmanaged<NoGCRegionCallbackFinalizerWorkItem*,Void> | Void | UnmanagedCallingConvention |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Byte*,Void> | Void | UnmanagedCallingConvention |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | Void | UnmanagedCallingConvention |
 | file://:0:0:0:0 | delegate* unmanaged<Void> | Void | UnmanagedCallingConvention |
 unmanagedCallingConvention
 parameter
 | file://:0:0:0:0 | delegate* default<A,B> | 0 | file://:0:0:0:0 |  | A |
 | file://:0:0:0:0 | delegate* default<B,A> | 0 | file://:0:0:0:0 |  | B |
-| file://:0:0:0:0 | delegate* default<Byte ref,Void> | 0 | file://:0:0:0:0 |  | ref byte! |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 0 | file://:0:0:0:0 |  | ref int! |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 1 | file://:0:0:0:0 | `1 | out object? |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 2 | file://:0:0:0:0 | `2 | readonly int! |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | 0 | file://:0:0:0:0 |  | ref int! |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | 1 | file://:0:0:0:0 | `1 | out object? |
 | file://:0:0:0:0 | delegate* default<Int32*,Void*> | 0 | file://:0:0:0:0 |  | int*! |
-| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | 0 | file://:0:0:0:0 |  | IntPtr! |
-| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | 1 | file://:0:0:0:0 | `1 | ref byte! |
-| file://:0:0:0:0 | delegate* default<IntPtr,Byte ref,PortableTailCallFrame*,Void> | 2 | file://:0:0:0:0 | `2 | PortableTailCallFrame*! |
-| file://:0:0:0:0 | delegate* default<Object,Void> | 0 | file://:0:0:0:0 |  | object |
 | file://:0:0:0:0 | delegate* default<T,Int32> | 0 | file://:0:0:0:0 |  | T |
 | file://:0:0:0:0 | delegate* default<Void*,Int32*> | 0 | file://:0:0:0:0 |  | Void*! |
-| file://:0:0:0:0 | delegate* default<Void*,Object> | 0 | file://:0:0:0:0 |  | Void*! |
 | file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | 0 | file://:0:0:0:0 |  | ref int! |
 | file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | 1 | file://:0:0:0:0 | `1 | out object? |
 | file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | 2 | file://:0:0:0:0 | `2 | T |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 0 | file://:0:0:0:0 |  | byte*! |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 1 | file://:0:0:0:0 | `1 | int! |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 2 | file://:0:0:0:0 | `2 | byte! |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 3 | file://:0:0:0:0 | `3 | long! |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 4 | file://:0:0:0:0 | `4 | long! |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 5 | file://:0:0:0:0 | `5 | EVENT_FILTER_DESCRIPTOR*! |
-| file://:0:0:0:0 | delegate* unmanaged<Byte*,Int32,Byte,Int64,Int64,EVENT_FILTER_DESCRIPTOR*,Void*,Void> | 6 | file://:0:0:0:0 | `6 | Void*! |
-| file://:0:0:0:0 | delegate* unmanaged<Char*,IntPtr,Void> | 0 | file://:0:0:0:0 |  | char*! |
-| file://:0:0:0:0 | delegate* unmanaged<Char*,IntPtr,Void> | 1 | file://:0:0:0:0 | `1 | IntPtr! |
-| file://:0:0:0:0 | delegate* unmanaged<Int32,PosixSignal,Int32> | 0 | file://:0:0:0:0 |  | int! |
-| file://:0:0:0:0 | delegate* unmanaged<Int32,PosixSignal,Int32> | 1 | file://:0:0:0:0 | `1 | PosixSignal! |
 | file://:0:0:0:0 | delegate* unmanaged<IntPtr,Int32> | 0 | file://:0:0:0:0 |  | IntPtr! |
 | file://:0:0:0:0 | delegate* unmanaged<IntPtr,Void> | 0 | file://:0:0:0:0 |  | IntPtr! |
-| file://:0:0:0:0 | delegate* unmanaged<NoGCRegionCallbackFinalizerWorkItem*,Void> | 0 | file://:0:0:0:0 |  | NoGCRegionCallbackFinalizerWorkItem*! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Byte*,Void> | 0 | file://:0:0:0:0 |  | Void*! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Byte*,Void> | 1 | file://:0:0:0:0 | `1 | byte*! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 0 | file://:0:0:0:0 |  | Void*! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 1 | file://:0:0:0:0 | `1 | Void*! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 2 | file://:0:0:0:0 | `2 | Void*! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 3 | file://:0:0:0:0 | `3 | GCConfigurationType! |
-| file://:0:0:0:0 | delegate* unmanaged<Void*,Void*,Void*,GCConfigurationType,Int64,Void> | 4 | file://:0:0:0:0 | `4 | long! |
 invocation
 | FunctionPointer.cs:17:21:17:43 | function pointer call |
 | FunctionPointer.cs:23:13:23:44 | function pointer call |
diff --git a/csharp/ql/test/library-tests/tuples/tuples.expected b/csharp/ql/test/library-tests/tuples/tuples.expected

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ public void Overrides(TextWriter trapFile)`
`101`	`101`	`}`
`102`	`102`	`}`
`103`	`103`
`104`		`- if (Symbol.OverriddenMethod is not null)`
	`104`	`+ if (Symbol.OverriddenMethod is not null && Symbol.OverriddenMethod.ShouldExtractSymbol())`
`105`	`105`	`{`
`106`	`106`	`trapFile.overrides(this, Method.Create(Context, Symbol.OverriddenMethod));`
`107`	`107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -225,7 +225,7 @@ private void ExtractParametersForDelegateLikeType(TextWriter trapFile, IMethodSy`
`225`	`225`	`}`
`226`	`226`
`227`	`227`	`/// <summary>`
`228`		`- /// Called to extract all members and nested types.`
	`228`	`+ /// Called to extract members and nested types.`
`229`	`229`	`/// This is called on each member of a namespace,`
`230`	`230`	`/// in either source code or an assembly.`
`231`	`231`	`/// </summary>`
`@@ -236,7 +236,7 @@ public void ExtractRecursive()`
`236`	`236`	`Context.BindComments(this, l);`
`237`	`237`	`}`
`238`	`238`
`239`		`- foreach (var member in Symbol.GetMembers())`
	`239`	`+ foreach (var member in Symbol.GetMembers().ExtractionCandidates())`
`240`	`240`	`{`
`241`	`241`	`switch (member.Kind)`
`242`	`242`	`{`
`@@ -262,16 +262,16 @@ public void PopulateGenerics()`
`262`	`262`
`263`	`263`	`var members = new List<ISymbol>();`
`264`	`264`
`265`		`- foreach (var member in Symbol.GetMembers())`
	`265`	`+ foreach (var member in Symbol.GetMembers().ExtractionCandidates())`
`266`	`266`	`members.Add(member);`
`267`		`- foreach (var member in Symbol.GetTypeMembers())`
	`267`	`+ foreach (var member in Symbol.GetTypeMembers().ExtractionCandidates())`
`268`	`268`	`members.Add(member);`
`269`	`269`
`270`	`270`	`// Mono extractor puts all BASE interface members as members of the current interface.`
`271`	`271`
`272`	`272`	`if (Symbol.TypeKind == TypeKind.Interface)`
`273`	`273`	`{`
`274`		`- foreach (var baseInterface in Symbol.Interfaces)`
	`274`	`+ foreach (var baseInterface in Symbol.Interfaces.ExtractionCandidates())`
`275`	`275`	`{`
`276`	`276`	`foreach (var member in baseInterface.GetMembers())`
`277`	`277`	`members.Add(member);`
`@@ -288,7 +288,7 @@ public void PopulateGenerics()`
`288`	`288`	`if (Symbol.BaseType is not null)`
`289`	`289`	`Create(Context, Symbol.BaseType).PopulateGenerics();`
`290`	`290`
`291`		`- foreach (var i in Symbol.Interfaces)`
	`291`	`+ foreach (var i in Symbol.Interfaces.ExtractionCandidates())`
`292`	`292`	`{`
`293`	`293`	`Create(Context, i).PopulateGenerics();`
`294`	`294`	`}`
Original file line number	Diff line number	Diff line change
`@@ -269,7 +269,7 @@ private static void AnalyseNamespace(Context cx, INamespaceSymbol ns)`
`269`	`269`	`AnalyseNamespace(cx, memberNamespace);`
`270`	`270`	`}`
`271`	`271`
`272`		`- foreach (var memberType in ns.GetTypeMembers())`
	`272`	`+ foreach (var memberType in ns.GetTypeMembers().ExtractionCandidates())`
`273`	`273`	`{`
`274`	`274`	`Entities.Type.Create(cx, memberType).ExtractRecursive();`
`275`	`275`	`}`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* Only extract *public* and *protected* members from reference assemblies. This yields an approximate average speed-up of around 10% for extraction and query execution. Custom MaD rows using `Field`-based summaries may need to be changed to `SyntheticField`-based flows if they reference private fields.