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

fix: Conditional access mutations could lead to unrecoverable NullReferenceException in compilation #2888

Merged
merged 5 commits into from
Mar 20, 2024
Merged
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
1,292 changes: 653 additions & 639 deletions src/Stryker.Core/Stryker.Core.UnitTest/Mutants/CsharpMutantOrchestratorTests.cs

Large diffs are not rendered by default.

36 changes: 34 additions & 2 deletions src/Stryker.Core/Stryker.Core/Compiling/CSharpRollbackProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Stryker.Core.Compiling
public interface ICSharpRollbackProcess
{
CSharpRollbackProcessResult Start(CSharpCompilation compiler, ImmutableArray<Diagnostic> diagnostics, bool lastAttempt, bool devMode);
SyntaxTree CleanUpFile(SyntaxTree file);
}

/// <summary>
Expand All @@ -39,7 +40,12 @@ public CSharpRollbackProcessResult Start(CSharpCompilation compiler, ImmutableAr
var syntaxTreeMapping = compiler.SyntaxTrees.ToDictionary<SyntaxTree, SyntaxTree, ICollection<Diagnostic>>(syntaxTree => syntaxTree, _ => new Collection<Diagnostic>());

foreach (var diagnostic in diagnostics.Where(x => x.Severity == DiagnosticSeverity.Error))
{
{
if (diagnostic.Location.SourceTree == null)
{
Logger.LogWarning("General compilation error: {0}", diagnostic.GetMessage());
continue;
}
syntaxTreeMapping[diagnostic.Location.SourceTree].Add(diagnostic);
}

Expand Down Expand Up @@ -142,7 +148,7 @@ private static SyntaxNode FindEnclosingMember(SyntaxNode node)
return currentNode;
}
}
// return the all file if not found
// return the whole file if not found
return node.SyntaxTree.GetRoot();
}

Expand Down Expand Up @@ -250,6 +256,32 @@ private Collection<SyntaxNode> ScanForSuspiciousMutations(Diagnostic[] diagnosti
return suspiciousMutations;
}

// removes all mutation from a file
public SyntaxTree CleanUpFile(SyntaxTree file)
{
var rollbackRoot = file.GetRoot();
var scan = ScanAllMutationsIfsAndIds(rollbackRoot);
var suspiciousMutations = new Collection<SyntaxNode>();
foreach (var mutant in scan.Where(mutant => !suspiciousMutations.Contains(mutant.Node)))
{
suspiciousMutations.Add(mutant.Node);
if (mutant.Id != -1)
{
RollBackedIds.Add(mutant.Id.Value);
}
}
// mark the broken mutation nodes to track
var trackedTree = rollbackRoot.TrackNodes(suspiciousMutations);
foreach (var brokenMutation in suspiciousMutations)
{
// find the mutated node in the new tree
var nodeToRemove = trackedTree.GetCurrentNode(brokenMutation);
// remove the mutated node using its MutantPlacer remove method and update the tree
trackedTree = trackedTree.ReplaceNode(nodeToRemove, MutantPlacer.RemoveMutant(nodeToRemove));
}
return file.WithRootAndOptions(trackedTree, file.Options);
}

private string DisplayName(SyntaxNode initNode) =>
initNode switch
{
Expand Down
111 changes: 79 additions & 32 deletions src/Stryker.Core/Stryker.Core/Compiling/CsharpCompilingProcess.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Text;
Expand All @@ -9,6 +11,7 @@
using Microsoft.CodeAnalysis.Emit;
using Microsoft.Extensions.Logging;
using Stryker.Core.Exceptions;
using Stryker.Core.Initialisation;
using Stryker.Core.Initialisation.Buildalyzer;
using Stryker.Core.Logging;
using Stryker.Core.MutationTest;
Expand Down Expand Up @@ -60,7 +63,7 @@ public CompilingProcessResult Compile(IEnumerable<SyntaxTree> syntaxTrees, Strea

// first try compiling
var retryCount = 1;
(var rollbackProcessResult, var emitResult, retryCount) = TryCompilation(ilStream, symbolStream, compilation, null, false, retryCount);
(var rollbackProcessResult, var emitResult, retryCount) = TryCompilation(ilStream, symbolStream, ref compilation, null, false, retryCount);

// If compiling failed and the error has no location, log and throw exception.
if (!emitResult.Success && emitResult.Diagnostics.Any(diagnostic => diagnostic.Location == Location.None && diagnostic.Severity == DiagnosticSeverity.Error))
Expand All @@ -73,8 +76,8 @@ public CompilingProcessResult Compile(IEnumerable<SyntaxTree> syntaxTrees, Strea

for (var count = 1; !emitResult.Success && count < MaxAttempt; count++)
{
// compilation did not succeed. let's compile a couple times more for good measure
(rollbackProcessResult, emitResult, retryCount) = TryCompilation(ilStream, symbolStream, rollbackProcessResult?.Compilation ?? compilation, emitResult, retryCount == MaxAttempt - 1, retryCount);
// compilation did not succeed. let's compile a couple of times more for good measure
(rollbackProcessResult, emitResult, retryCount) = TryCompilation(ilStream, symbolStream, ref compilation, emitResult, retryCount == MaxAttempt - 1, retryCount);
}

if (emitResult.Success)
Expand Down Expand Up @@ -118,15 +121,16 @@ private CSharpCompilation RunSourceGenerators(IAnalyzerResult analyzerResult, Co
.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var diagnostics);

var errors = diagnostics.Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error && diagnostic.Location == Location.None).ToList();
if (errors.Count > 0)
if (errors.Count == 0)
{
foreach (var diagnostic in errors)
{
_logger.LogError("Failed to generate source code for mutated assembly: {0}", diagnostic);
}
throw new CompilationException("Source Generator Failure");
return outputCompilation as CSharpCompilation;
}
return outputCompilation as CSharpCompilation;

foreach (var diagnostic in errors)
{
_logger.LogError("Failed to generate source code for mutated assembly: {0}", diagnostic);
}
throw new CompilationException("Source Generator Failure");
}

private CSharpCompilation GetCSharpCompilation(IEnumerable<SyntaxTree> syntaxTrees)
Expand All @@ -147,43 +151,86 @@ private CSharpCompilation GetCSharpCompilation(IEnumerable<SyntaxTree> syntaxTre
private (CSharpRollbackProcessResult, EmitResult, int) TryCompilation(
Stream ms,
Stream symbolStream,
CSharpCompilation compilation,
ref CSharpCompilation compilation,
EmitResult previousEmitResult,
bool lastAttempt,
int retryCount)
{
CSharpRollbackProcessResult rollbackProcessResult = null;

if (previousEmitResult != null)
{
// remove broken mutations
rollbackProcessResult = _rollbackProcess.Start(compilation, previousEmitResult.Diagnostics, lastAttempt, _options.DevMode);
compilation = rollbackProcessResult.Compilation;
}

// reset the memoryStream
ms.SetLength(0);
symbolStream?.SetLength(0);

_logger.LogDebug("Trying compilation for the {retryCount} time.", ReadableNumber(retryCount));

var emitOptions = symbolStream == null ? null : new EmitOptions(false, DebugInformationFormat.PortablePdb,
_input.SourceProjectInfo.AnalyzerResult.GetSymbolFileName());
var emitResult = compilation.Emit(
ms,
symbolStream,
manifestResources: _input.SourceProjectInfo.AnalyzerResult.GetResources(_logger),
win32Resources: compilation.CreateDefaultWin32Resources(
true, // Important!
false,
null,
null),
options: emitOptions);
EmitResult emitResult = null;
var resourceDescriptions = _input.SourceProjectInfo.AnalyzerResult.GetResources(_logger);
while(emitResult == null)
{
if (previousEmitResult != null)
{
// remove broken mutations
rollbackProcessResult = _rollbackProcess.Start(compilation, previousEmitResult.Diagnostics, lastAttempt, _options.DevMode);
compilation = rollbackProcessResult.Compilation;
}

// reset the memoryStreams
ms.SetLength(0);
symbolStream?.SetLength(0);
try
{
emitResult = compilation.Emit(
ms,
symbolStream,
manifestResources: resourceDescriptions,
win32Resources: compilation.CreateDefaultWin32Resources(
true, // Important!
false,
null,
null),
options: emitOptions);
}
#pragma warning disable S1696 // this catches an exception raised by the C# compiler
catch (NullReferenceException e)
{
_logger.LogError("Roslyn C# compiler raised an NullReferenceException. This is a known Roslyn's issue that may be triggered by invalid usage of conditional access expression.");
_logger.LogInformation(e, "Exception");
_logger.LogError("Stryker will attempt to skip problematic files.");
compilation = ScanForCauseOfException(compilation);
EmbeddedResourcesGenerator.ResetCache();
}
}

LogEmitResult(emitResult);

return (rollbackProcessResult, emitResult, retryCount+1);
}

private CSharpCompilation ScanForCauseOfException(CSharpCompilation compilation)
{
var syntaxTrees = compilation.SyntaxTrees;
// we add each file incrementally until it fails
foreach(var st in syntaxTrees)
{
var local = compilation.RemoveAllSyntaxTrees().AddSyntaxTrees(st);
try
{
using var ms = new MemoryStream();
local.Emit(
ms,
manifestResources: _input.SourceProjectInfo.AnalyzerResult.GetResources(_logger),
options: null);
}
catch(Exception e)
{
_logger.LogError(e, "Failed to compile {0}", st.FilePath);
_logger.LogTrace("source code:\n {0}", st.GetText());
syntaxTrees = syntaxTrees.Where(x => x != st).Append(_rollbackProcess.CleanUpFile(st)).ToImmutableArray();
}
}
_logger.LogError("Please report an issue and provide the source code of the file that caused the exception for analysis.");
return compilation.RemoveAllSyntaxTrees().AddSyntaxTrees(syntaxTrees);
}

private void LogEmitResult(EmitResult result)
{
if (!result.Success)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ namespace Stryker.Core.Initialisation
[ExcludeFromCodeCoverage]
public static class EmbeddedResourcesGenerator
{
private static readonly IDictionary<string, IEnumerable<ResourceDescription>> _resourceDescriptions = new Dictionary<string, IEnumerable<ResourceDescription>>();
private static readonly Dictionary<string, IEnumerable<(ResourceDescription description, object context)>> _resourceDescriptions = new();

public static void ResetCache()
{
_resourceDescriptions.Clear();
}

public static IEnumerable<ResourceDescription> GetManifestResources(string assemblyPath, string projectFilePath, string rootNamespace, IEnumerable<string> embeddedResources)
{
Expand All @@ -31,7 +36,7 @@ public static IEnumerable<ResourceDescription> GetManifestResources(string assem
// Failed to load some or all resources from module, generate missing resources from disk
if (module is not null && _resourceDescriptions[projectFilePath].Count() < embeddedResources.Count())
{
var missingEmbeddedResources = embeddedResources.Where(r => _resourceDescriptions[projectFilePath].Any(fr => GetResourceDescriptionInternalName(fr) == GenerateResourceName(r)));
var missingEmbeddedResources = embeddedResources.Where(r => _resourceDescriptions[projectFilePath].Any(fr => GetResourceDescriptionInternalName(fr.description) == GenerateResourceName(r)));
_resourceDescriptions[projectFilePath] = _resourceDescriptions[projectFilePath].Concat(GenerateManifestResources(projectFilePath, rootNamespace, missingEmbeddedResources));
}

Expand All @@ -42,9 +47,13 @@ public static IEnumerable<ResourceDescription> GetManifestResources(string assem
}
}

foreach (var description in _resourceDescriptions.ContainsKey(projectFilePath) ? _resourceDescriptions[projectFilePath] : Enumerable.Empty<ResourceDescription>())
if (!_resourceDescriptions.TryGetValue(projectFilePath, out var resourcesDescription))
{
yield break;
}
foreach (var description in resourcesDescription)
{
yield return description;
yield return description.description;
}
}

Expand All @@ -67,7 +76,7 @@ private static ModuleDefinition LoadModule(string assemblyPath)
}
}

private static IEnumerable<ResourceDescription> ReadResourceDescriptionsFromModule(ModuleDefinition module)
private static IEnumerable<(ResourceDescription, object)> ReadResourceDescriptionsFromModule(ModuleDefinition module)
{
foreach (var moduleResource in module.Resources.Where(r => r.ResourceType == ResourceType.Embedded).Cast<EmbeddedResource>())
{
Expand All @@ -80,26 +89,26 @@ private static IEnumerable<ResourceDescription> ReadResourceDescriptionsFromModu
resourceStream.Position = 0;
shortLivedBackingStream.Position = 0;

yield return new ResourceDescription(
yield return (new ResourceDescription(
moduleResource.Name,
() => resourceStream,
moduleResource.IsPublic);
moduleResource.IsPublic), resourceStream);
}
}

private static IEnumerable<ResourceDescription> GenerateManifestResources(string projectFilePath, string rootNamespace, IEnumerable<string> embeddedResources)
private static IEnumerable<(ResourceDescription, object)> GenerateManifestResources(string projectFilePath, string rootNamespace, IEnumerable<string> embeddedResources)
{
var resources = new List<ResourceDescription>();
var resources = new List<(ResourceDescription, object)>();
foreach (var embeddedResource in embeddedResources)
{
var resourceFullFilename = Path.Combine(Path.GetDirectoryName(projectFilePath), embeddedResource);

var resourceName = GenerateResourceName(embeddedResource);

resources.Add(new ResourceDescription(
resources.Add((new ResourceDescription(
$"{rootNamespace}.{string.Join(".", resourceName.Split('\\'))}",
() => ProvideResourceData(resourceFullFilename),
true));
true), null));
}

return resources;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ private static List<INodeOrchestrator> BuildOrchestratorList() =>
new MemberAccessExpressionOrchestrator<MemberAccessExpressionSyntax>(),
new MemberAccessExpressionOrchestrator<MemberBindingExpressionSyntax>(),
new MemberAccessExpressionOrchestrator<SimpleNameSyntax>(),
new MemberAccessExpressionOrchestrator<PostfixUnaryExpressionSyntax>(t => t.IsKind(SyntaxKind.SuppressNullableWarningExpression)),
new ConditionalExpressionOrchestrator(),
// ensure static constructs are marked properly
new StaticFieldDeclarationOrchestrator(),
new StaticConstructorOrchestrator(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Stryker.Core.Mutants.CsharpNodeOrchestrators;
internal class ConditionalExpressionOrchestrator :MemberAccessExpressionOrchestrator<ConditionalAccessExpressionSyntax>
{

protected override ExpressionSyntax OrchestrateChildrenMutation(ConditionalAccessExpressionSyntax node, SemanticModel semanticModel,
MutationContext context)
{
var mutated = node.ReplaceNodes(node.ChildNodes(), (original, _) =>
{
if (original != node.WhenNotNull)
{
return context.Mutate(original, semanticModel);
}
// there must be some MemberBindingAccess in the chain, so we assume we are in a member access chain (no mutation injection)
var subContext = context.Enter(MutationControl.MemberAccess);
var result = subContext.Mutate(original, semanticModel);
subContext.Leave();
return result;

});
return mutated;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ protected override PropertyDeclarationSyntax OrchestrateChildrenMutation(Propert
var children = node.ReplaceNodes(node.ChildNodes(), (original, _) =>
{
var determinedContext = original == node.Initializer ? context.EnterStatic() : context;
return determinedContext.FindHandler(original).Mutate(original, semanticModel, determinedContext);
return determinedContext.Mutate(original, semanticModel);
});
return children.WithInitializer(children.Initializer.WithValue(context.PlaceStaticContextMarker(children.Initializer.Value)));
}

}
Loading
Loading