Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Use dedicated lock type where applicable. #18090

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/csharp-qltest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,22 @@ on:
paths:
- "csharp/**"
- "shared/**"
- "misc/bazel/**"
- .github/actions/fetch-codeql/action.yml
- codeql-workspace.yml
- "MODULE.bazel"
branches:
- main
- "rc/*"
pull_request:
paths:
- "csharp/**"
- "shared/**"
- "misc/bazel/**"
- .github/workflows/csharp-qltest.yml
- .github/actions/fetch-codeql/action.yml
- codeql-workspace.yml
- "MODULE.bazel"
branches:
- main
- "rc/*"
Expand Down
18 changes: 17 additions & 1 deletion csharp/.vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"preLaunchTask": "dotnet: build",
"program": "${workspaceFolder}/autobuilder/Semmle.Autobuild.CSharp/bin/Debug/net9.0/Semmle.Autobuild.CSharp.dll",
// Set the path to the folder that should be extracted:
"cwd": "${workspaceFolder}/ql/integration-tests/all-platforms/autobuild",
"cwd": "cwd": "${workspaceFolder}/ql/integration-tests/all-platforms/autobuild",
"stopAtEntry": true,
"args": [],
"env": {
Expand All @@ -61,5 +61,21 @@
],
"env": {}
},
{
"name": "C#: Tracing Debug",
"type": "coreclr",
"request": "launch",
"preLaunchTask": "dotnet: build",
"program": "${workspaceFolder}/extractor/Semmle.Extraction.CSharp.Driver/bin/Debug/net9.0/Semmle.Extraction.CSharp.Driver.dll",
// Set the path to the folder that should be extracted:
"cwd": "${workspaceFolder}/ql/test/library-tests/dataflow/local",
"stopAtEntry": true,
"args": [
"LocalDataFlow.cs",
"/r:System.Private.CoreLib.dll",
"/r:System.Collections.dll",
],
"env": {}
},
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Security.Cryptography;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
Expand Down Expand Up @@ -264,7 +263,7 @@ private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<Dep

var isWindows = fileContent.UseWindowsForms || fileContent.UseWpf;

var sync = new object();
var sync = new Lock();
var projectGroups = projects.GroupBy(Path.GetDirectoryName);
Parallel.ForEach(projectGroups, new ParallelOptions { MaxDegreeOfParallelism = DependencyManager.Threads }, projectGroup =>
{
Expand Down Expand Up @@ -346,7 +345,7 @@ private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<Dep
compilationInfoContainer.CompilationInfos.Add(("Fallback nuget restore", notYetDownloadedPackages.Count.ToString()));

var successCount = 0;
var sync = new object();
var sync = new Lock();

Parallel.ForEach(notYetDownloadedPackages, new ParallelOptions { MaxDegreeOfParallelism = DependencyManager.Threads }, package =>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System;
using System.IO;
using Semmle.Util;
using Semmle.Util.Logging;
using Semmle.Extraction.CSharp.DependencyFetching;
using System;

namespace Semmle.Extraction.CSharp.Standalone
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
using System;
using System.Collections.Generic;
using Semmle.Util.Logging;
using Semmle.Extraction.CSharp.DependencyFetching;

namespace Semmle.Extraction.CSharp.Standalone
{
public class Program
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,5 +642,11 @@ 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 or protected.
/// </summary>
public static bool IsPublicOrProtected(this ISymbol symbol) =>
symbol.DeclaredAccessibility == Accessibility.Public || symbol.DeclaredAccessibility == Accessibility.Protected;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -225,29 +225,33 @@
}

/// <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.
/// <param name="onlyEffectivelyPublic">If true, only extract members that are effectively public otherwise extract all members.</param>
/// </summary>
public void ExtractRecursive()
public void ExtractRecursive(bool onlyEffectivelyPublic = false)
{
foreach (var l in Symbol.DeclaringSyntaxReferences.Select(s => s.GetSyntax().GetLocation()))
{
Context.BindComments(this, l);
}

foreach (var member in Symbol.GetMembers())
{
if (onlyEffectivelyPublic && !member.IsPublicOrProtected())
continue;

switch (member.Kind)
{
case SymbolKind.NamedType:
Create(Context, (ITypeSymbol)member).ExtractRecursive();
Create(Context, (ITypeSymbol)member).ExtractRecursive(onlyEffectivelyPublic);
break;
default:
Context.CreateEntity(member);
break;
}
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Where Note

This foreach loop
implicitly filters its target sequence
- consider filtering the sequence explicitly using '.Where(...)'.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,13 @@
AnalyseNamespace(cx, memberNamespace);
}

foreach (var memberType in ns.GetTypeMembers())
{
Entities.Type.Create(cx, memberType).ExtractRecursive();
if (memberType.IsPublicOrProtected())
{
Entities.Type.Create(cx, memberType).ExtractRecursive(onlyEffectivelyPublic: true);
}
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Where Note

This foreach loop
implicitly filters its target sequence
- consider filtering the sequence explicitly using '.Where(...)'.
}

private int IncrementTaskCount()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Threading;
using Semmle.Util.Logging;
using CompilationInfo = (string key, string value);

Expand Down Expand Up @@ -38,7 +39,7 @@ public ExtractionContext(string cwd, string[] args, string outputPath, IEnumerab
// to handle pathological cases.
private const int maxErrors = 1000;

private readonly object mutex = new object();
private readonly Lock mutex = new();

public void Message(Message msg)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions csharp/extractor/Semmle.Util/Logging/PidStreamWriter.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System.IO;
using System.Diagnostics;
using System.Threading;

namespace Semmle.Util.Logging
{
Expand Down Expand Up @@ -33,6 +33,6 @@ public override void WriteLine(string? format, params object?[] args)
WriteLine(format is null ? format : string.Format(format, args));
}

private readonly object mutex = new object();
private readonly Lock mutex = new();
}
}
2 changes: 1 addition & 1 deletion csharp/extractor/Testrunner/Testrunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
/// </summary>
public class Testrunner
{
private static readonly object ConsoleLock = new();
private static readonly Lock ConsoleLock = new();

private static readonly ManualResetEvent Finished = new(false);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
| Assembly1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null | no compilation |
| Locations, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null | has compilation |
| System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 | no compilation |
| System.Console, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a | no compilation |
| System.Console, Version=9.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a | no compilation |
| System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 | no compilation |
| System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e | no compilation |
| System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a | no compilation |
| System.Runtime.Extensions, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a | no compilation |
| System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e | no compilation |
| System.Runtime, Version=9.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a | no compilation |
| System.Runtime.Extensions, Version=9.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a | no compilation |
| mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 | no compilation |
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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,Byte*,Int32,CleanupWorkListElement ref,Void> | Void | 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 |
Expand All @@ -25,6 +26,10 @@ 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,Byte*,Int32,CleanupWorkListElement ref,Void> | 0 | file://:0:0:0:0 | | ref byte! |
| file://:0:0:0:0 | delegate* default<Byte ref,Byte*,Int32,CleanupWorkListElement ref,Void> | 1 | file://:0:0:0:0 | `1 | byte*! |
| file://:0:0:0:0 | delegate* default<Byte ref,Byte*,Int32,CleanupWorkListElement ref,Void> | 2 | file://:0:0:0:0 | `2 | int! |
| file://:0:0:0:0 | delegate* default<Byte ref,Byte*,Int32,CleanupWorkListElement ref,Void> | 3 | file://:0:0:0:0 | `3 | ref CleanupWorkListElement |
| 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? |
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
| Console | Write(string, ReadOnlySpan<object>) | arg |
| Console | Write(string, object) | arg0 |
| Console | Write(string, object, object) | arg0 |
| Console | Write(string, object, object) | arg1 |
| Console | Write(string, object, object, object) | arg0 |
| Console | Write(string, object, object, object) | arg1 |
| Console | Write(string, object, object, object) | arg2 |
| Console | Write(string, params Object[]) | arg |
| Console | WriteLine(string, ReadOnlySpan<object>) | arg |
| Console | WriteLine(string, object) | arg0 |
| Console | WriteLine(string, object, object) | arg0 |
| Console | WriteLine(string, object, object) | arg1 |
Expand All @@ -15,13 +17,15 @@
| Debug | Assert(bool, string, string, params Object[]) | args |
| Debug | Print(string, params Object[]) | args |
| Debug | WriteLine(string, params Object[]) | args |
| StringBuilder | AppendFormat(IFormatProvider, string, ReadOnlySpan<object>) | args |
| StringBuilder | AppendFormat(IFormatProvider, string, object) | arg0 |
| StringBuilder | AppendFormat(IFormatProvider, string, object, object) | arg0 |
| StringBuilder | AppendFormat(IFormatProvider, string, object, object) | arg1 |
| StringBuilder | AppendFormat(IFormatProvider, string, object, object, object) | arg0 |
| StringBuilder | AppendFormat(IFormatProvider, string, object, object, object) | arg1 |
| StringBuilder | AppendFormat(IFormatProvider, string, object, object, object) | arg2 |
| StringBuilder | AppendFormat(IFormatProvider, string, params Object[]) | args |
| StringBuilder | AppendFormat(string, ReadOnlySpan<object>) | args |
| StringBuilder | AppendFormat(string, object) | arg0 |
| StringBuilder | AppendFormat(string, object, object) | arg0 |
| StringBuilder | AppendFormat(string, object, object) | arg1 |
Expand All @@ -30,27 +34,31 @@
| StringBuilder | AppendFormat(string, object, object, object) | arg2 |
| StringBuilder | AppendFormat(string, params Object[]) | args |
| Strings | MyStringFormat(string, params Object[]) | args |
| TextWriter | Write(string, ReadOnlySpan<object>) | arg |
| TextWriter | Write(string, object) | arg0 |
| TextWriter | Write(string, object, object) | arg0 |
| TextWriter | Write(string, object, object) | arg1 |
| TextWriter | Write(string, object, object, object) | arg0 |
| TextWriter | Write(string, object, object, object) | arg1 |
| TextWriter | Write(string, object, object, object) | arg2 |
| TextWriter | Write(string, params Object[]) | arg |
| TextWriter | WriteLine(string, ReadOnlySpan<object>) | arg |
| TextWriter | WriteLine(string, object) | arg0 |
| TextWriter | WriteLine(string, object, object) | arg0 |
| TextWriter | WriteLine(string, object, object) | arg1 |
| TextWriter | WriteLine(string, object, object, object) | arg0 |
| TextWriter | WriteLine(string, object, object, object) | arg1 |
| TextWriter | WriteLine(string, object, object, object) | arg2 |
| TextWriter | WriteLine(string, params Object[]) | arg |
| string | Format(IFormatProvider, string, ReadOnlySpan<object>) | args |
| string | Format(IFormatProvider, string, object) | arg0 |
| string | Format(IFormatProvider, string, object, object) | arg0 |
| string | Format(IFormatProvider, string, object, object) | arg1 |
| string | Format(IFormatProvider, string, object, object, object) | arg0 |
| string | Format(IFormatProvider, string, object, object, object) | arg1 |
| string | Format(IFormatProvider, string, object, object, object) | arg2 |
| string | Format(IFormatProvider, string, params Object[]) | args |
| string | Format(string, ReadOnlySpan<object>) | args |
| string | Format(string, object) | arg0 |
| string | Format(string, object, object) | arg0 |
| string | Format(string, object, object) | arg1 |
Expand Down
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/generics/Generics.ql
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ query predicate test26(ConstructedGeneric cg, string s) {
}

query predicate test27(ConstructedType ct, UnboundGenericType ugt, UnboundGenericType sourceDecl) {
ct.fromSource() and
ct instanceof NestedType and
ugt = ct.getUnboundGeneric() and
sourceDecl = ct.getUnboundDeclaration() and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
| NonPublicClass.cs:9:9:9:31 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicClass.cs:9:9:9:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicClass.cs:14:9:14:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicClass.cs:19:9:19:51 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicClass.cs:19:33:19:50 | call to method ReadLine | System | Console | ReadLine | () | true | System.Console | 8.0.0.0 | neutral | source |
| PublicClass.cs:19:33:19:50 | call to method ReadLine | System | Console | ReadLine | () | true | System.Console | 8.0.0.0 | source | source |
| PublicClass.cs:24:9:24:46 | call to method Write | System | Console | Write | (System.Object) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicClass.cs:30:9:30:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicGenericClass.cs:9:9:9:30 | call to method WriteLine | System | Console | WriteLine | (System.Object) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicGenericClass.cs:14:9:14:30 | call to method WriteLine | System | Console | WriteLine | (System.Object) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicGenericInterface.cs:13:9:13:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| PublicInterface.cs:13:9:13:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 8.0.0.0 | neutral | source |
| NonPublicClass.cs:9:9:9:31 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicClass.cs:9:9:9:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicClass.cs:14:9:14:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicClass.cs:19:9:19:51 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicClass.cs:19:33:19:50 | call to method ReadLine | System | Console | ReadLine | () | true | System.Console | 9.0.0.0 | neutral | source |
| PublicClass.cs:19:33:19:50 | call to method ReadLine | System | Console | ReadLine | () | true | System.Console | 9.0.0.0 | source | source |
| PublicClass.cs:24:9:24:46 | call to method Write | System | Console | Write | (System.Object) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicClass.cs:30:9:30:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicGenericClass.cs:9:9:9:30 | call to method WriteLine | System | Console | WriteLine | (System.Object) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicGenericClass.cs:14:9:14:30 | call to method WriteLine | System | Console | WriteLine | (System.Object) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicGenericInterface.cs:13:9:13:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
| PublicInterface.cs:13:9:13:30 | call to method WriteLine | System | Console | WriteLine | (System.String) | true | System.Console | 9.0.0.0 | neutral | source |
2 changes: 1 addition & 1 deletion misc/bazel/csharp.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ load("@rules_dotnet//dotnet:defs.bzl", "csharp_binary", "csharp_library", "cshar
load("@rules_pkg//pkg:mappings.bzl", "strip_prefix")
load("//misc/bazel:pkg.bzl", "codeql_pkg_files")

TARGET_FRAMEWORK = "net8.0"
TARGET_FRAMEWORK = "net9.0"

def _gen_assembly_info(name):
assembly_info_gen = name + "-assembly-info"
Expand Down