C#: Add binlog support to buildless with source generator support #16747

tamasvajk · 2024-06-13T07:53:53Z

This PR is adding MsBuild binary log extraction support to buildless. There's another variant of this in #16672. The main difference is that this version also runs source generators and extracts the additional ASTs.

I've compared traced databases to binlog extracted ones, and found that the alert retention rate (in the nightly query suite) is around 97.5% on dotnet/efcore, 99% on dotnet/msbuild, and 99% on dotnet/roslyn. The latter is an approximation, because Roslyn uses source generators, and the generated files are at different locations in the traced and binlog extracted DBs, so the alerts are not matching in these files.

Database creation times are faster with binlog extraction. Speedups (on a single measurement): roslyn: 43.2%, efcore: 35%, msbuild: 64.6%.

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs

csharp/extractor/Semmle.Extraction/TrapWriter.cs

+            catch (Exception ex)
+            {
+                // If this happened, it was probably because
+                // - the same file was compiled multiple times, or
+                // - the file doesn't exist (due to wrong #line directive or because it's an in-memory source generated AST).
+                // In any case, this is not a fatal error.
+                logger.LogWarning("Problem archiving " + dest + ": " + ex);
+            }


csharp/extractor/Semmle.Extraction/Entities/File.cs

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs

michaelnebel · 2024-06-20T08:27:52Z

To make sure I understand: The idea with this change is that the user should be able to provide a path to a directory containing a "binary log"; A binary log is created during a previous compilation of the source code and contains information about the compiler calls and the generated syntax trees?

tamasvajk · 2024-06-20T09:14:01Z

To make sure I understand: The idea with this change is that the user should be able to provide a path to a directory containing a "binary log"; A binary log is created during a previous compilation of the source code and contains information about the compiler calls and the generated syntax trees?

Yes, more or less. The user would pass a .binlog file to the database creation, for example:

dotnet build /t:rebuild /bl:xyz.binlog
codeql database create DB_xyz --language=csharp --build-mode=none -Obinlog=xyz.binlog

The binary log contains information about the compiler calls. The generated ASTs are produced by basic.compilerlog.util based on the arguments of the compiler calls.

michaelnebel · 2024-06-20T09:15:58Z

To make sure I understand: The idea with this change is that the user should be able to provide a path to a directory containing a "binary log"; A binary log is created during a previous compilation of the source code and contains information about the compiler calls and the generated syntax trees?

Yes, more or less. The user would pass a .binlog file to the database creation, for example:
dotnet build /t:rebuild /bl:xyz.binlog
codeql database create DB_xyz --language=csharp --build-mode=none -Obinlog=xyz.binlog
The binary log contains information about the compiler calls. The generated ASTs are produced by basic.compilerlog.util based on the arguments of the compiler calls.

But there is no new compilation attempt right?

tamasvajk · 2024-06-20T09:20:41Z

But there is no new compilation attempt right?

No, there's no new compilation, other than the usual AST creation and symbol resolution based on the compiler call arguments. There's some detail in #16346 regarding how things work.

I edited this answer a couple of times. I think what you're interested in is that whether there's a new call to csc. There's no new call. We're using basic.compilerlog.util to get to the compilation object and the ASTs. I had another variant of this feature in #16672, which is more straightforward, but ignores source generators.

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs

csharp/extractor/Semmle.Extraction/Extractor/BinaryLogExtractionContext.cs

jaredpar · 2024-06-24T03:36:07Z

csharp/extractor/Semmle.Extraction/Extractor/BinaryLogExtractionContext.cs

+            // Compute a unique folder name for the generated files:
+            generatedFolderName = "generated";
+
+            if (Directory.Exists(generatedFolderName))


This code will check for generated in the current working directory. Did you mean to do something like Path.Combine(cwd, generatedFolderName)?

Yes, this is on purpose. The current working directory will point to the --source-root specified in the codeql database creation command. In practice this will be the repo root that you're analysing. All the processed source files are stored in the source archive in the codeql database. And this logic is making sure that we're assigning the generated ASTs paths that don't collide with any already existing source files.

The integration test results in csharp/ql/integration-tests/all-platforms/binlog/Files.expected show some sample paths, for example:

generated/b/test.csproj (net8.0)/System.Text.RegularExpressions.Generator/System.Text.RegularExpressions.Generator.RegexGenerator/RegexGenerator.g.cs

We could also use Path.Combine(cwd, generatedFolderName), then the paths would point to locations closer to their original .csproj files, such as

b/generated/test.csproj (net8.0)/System.Text.RegularExpressions.Generator/System.Text.RegularExpressions.Generator.RegexGenerator/RegexGenerator.g.cs

I think the root level generated folder should work, and then all these generated ASTs are assigned to a single folder.

tamasvajk · 2024-06-26T09:57:32Z

I rebased this PR after upgrading the Microsoft.CodeAnalysis dependency from 4.8.0 to 4.9.2 in #16832. This allowed me to upgrade basic.compilerlog.util to the latest version. I pushed an extra commit to start using compilationData.GetGeneratedSyntaxTrees().

michaelnebel

Looks good to me.
Very nice work @tamasvajk!

tamasvajk · 2024-06-27T08:11:32Z

I rebased to fix a merge conflict.

michaelnebel

LGTM!

tamasvajk · 2024-06-27T12:51:34Z

Sorry for the constant noise on this PR. I rebased it again to fix some conflicts with #16857.

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs

+            catch (Exception ex)  // lgtm[cs/catch-of-all-exceptions]
+            {
+                logger.LogError($"  Unhandled exception: {ex}");
+                return ExitCode.Errors;
+            }


michaelnebel

LGTM!

michaelnebel

LGTM!

github-actions bot added the C# label Jun 13, 2024

github-advanced-security bot found potential problems Jun 13, 2024

View reviewed changes

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Jun 17, 2024

View reviewed changes

tamasvajk force-pushed the buildless/binary-log-extractor-2 branch 3 times, most recently from 1baaa46 to 4bdef60 Compare June 19, 2024 08:27

tamasvajk marked this pull request as ready for review June 19, 2024 09:21

tamasvajk requested review from a team as code owners June 19, 2024 09:21

tamasvajk mentioned this pull request Jun 19, 2024

Using binary logs to significantly increase CodeQL analysis performance for C# #16346

Open

tamasvajk commented Jun 19, 2024

View reviewed changes

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs Show resolved Hide resolved

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs Outdated Show resolved Hide resolved

tamasvajk commented Jun 20, 2024

View reviewed changes

csharp/extractor/Semmle.Extraction.CSharp/Extractor/Extractor.cs Outdated Show resolved Hide resolved

tamasvajk commented Jun 21, 2024

View reviewed changes

csharp/extractor/Semmle.Extraction/Extractor/BinaryLogExtractionContext.cs Show resolved Hide resolved

jaredpar reviewed Jun 24, 2024

View reviewed changes

tamasvajk force-pushed the buildless/binary-log-extractor-2 branch from 69d89cd to 24089f6 Compare June 26, 2024 09:54

michaelnebel previously approved these changes Jun 26, 2024

View reviewed changes

tamasvajk dismissed michaelnebel’s stale review via 16b63cd June 27, 2024 08:11

tamasvajk force-pushed the buildless/binary-log-extractor-2 branch from 24089f6 to 16b63cd Compare June 27, 2024 08:11

michaelnebel previously approved these changes Jun 27, 2024

View reviewed changes

tamasvajk added 5 commits June 27, 2024 14:48

C#: Add binlog support to buildless with source generator support

31ad195

Improve source archiving to handle non-existent files

dcd84f4

C#: Adjust trap location, database ID and archiving of generated sources

fb0520c

Fix failed extraction handling

b22f5f5

Change binlog test to have colliding generated file locations

1ae40c9

tamasvajk added 2 commits June 27, 2024 14:49

Compute unique identifier (folder path) for each compilation

0b41d51

Use dedicated API to get generated syntax trees

1e2d1ef

tamasvajk dismissed michaelnebel’s stale review via 1e2d1ef June 27, 2024 12:50

tamasvajk force-pushed the buildless/binary-log-extractor-2 branch from 16b63cd to 1e2d1ef Compare June 27, 2024 12:50

github-advanced-security bot found potential problems Jun 27, 2024

View reviewed changes

michaelnebel previously approved these changes Jun 27, 2024

View reviewed changes

tamasvajk added 2 commits June 28, 2024 09:40

Record diagnostics in binlog extraction test

c892744

Add TSP diagnostic for binlog usage

4db586f

tamasvajk dismissed michaelnebel’s stale review via 4db586f June 28, 2024 07:40

michaelnebel approved these changes Jun 28, 2024

View reviewed changes

tamasvajk merged commit 1cf5e89 into github:main Jun 28, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C#: Add binlog support to buildless with source generator support #16747

C#: Add binlog support to buildless with source generator support #16747

tamasvajk commented Jun 13, 2024 •

edited

Loading

michaelnebel commented Jun 20, 2024

tamasvajk commented Jun 20, 2024

michaelnebel commented Jun 20, 2024

tamasvajk commented Jun 20, 2024 •

edited

Loading

jaredpar Jun 24, 2024

tamasvajk Jun 24, 2024

tamasvajk commented Jun 26, 2024

michaelnebel left a comment

tamasvajk commented Jun 27, 2024

michaelnebel left a comment

tamasvajk commented Jun 27, 2024

michaelnebel left a comment

michaelnebel left a comment

C#: Add binlog support to buildless with source generator support #16747

C#: Add binlog support to buildless with source generator support #16747

Conversation

tamasvajk commented Jun 13, 2024 • edited Loading

michaelnebel commented Jun 20, 2024

tamasvajk commented Jun 20, 2024

michaelnebel commented Jun 20, 2024

tamasvajk commented Jun 20, 2024 • edited Loading

jaredpar Jun 24, 2024

Choose a reason for hiding this comment

tamasvajk Jun 24, 2024

Choose a reason for hiding this comment

tamasvajk commented Jun 26, 2024

michaelnebel left a comment

Choose a reason for hiding this comment

tamasvajk commented Jun 27, 2024

michaelnebel left a comment

Choose a reason for hiding this comment

tamasvajk commented Jun 27, 2024

michaelnebel left a comment

Choose a reason for hiding this comment

michaelnebel left a comment

Choose a reason for hiding this comment

tamasvajk commented Jun 13, 2024 •

edited

Loading

tamasvajk commented Jun 20, 2024 •

edited

Loading