Skip to content

Improve handling bad metadata in EE #76878

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 23, 2025
Merged

Improve handling bad metadata in EE #76878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0622442
Rename test utilities project
tmat Jan 22, 2025
11ca5c8
Better handling of modules with invalid metadata
tmat Jan 23, 2025
86436aa
Pass around ModuleId instead of Guid
tmat Jan 23, 2025
a287175
Feedback
tmat Jan 23, 2025
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
2 changes: 1 addition & 1 deletion Roslyn.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.CodeAnalysis.Expr
EndProject
Project("{778DAE3C-4631-46EA-AA77-85C1314464D9}") = "Microsoft.CodeAnalysis.VisualBasic.ResultProvider.UnitTests", "src\ExpressionEvaluator\VisualBasic\Test\ResultProvider\Microsoft.CodeAnalysis.VisualBasic.ResultProvider.UnitTests.vbproj", "{ACE53515-482C-4C6A-E2D2-4242A687DFEE}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.CodeAnalysis.ExpressionCompiler.Utilities", "src\ExpressionEvaluator\Core\Test\ExpressionCompiler\Microsoft.CodeAnalysis.ExpressionCompiler.Utilities.csproj", "{21B80A31-8FF9-4E3A-8403-AABD635AEED9}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.CodeAnalysis.ExpressionCompiler.Test.Utilities", "src\ExpressionEvaluator\Core\Test\ExpressionCompiler\Microsoft.CodeAnalysis.ExpressionCompiler.Test.Utilities.csproj", "{21B80A31-8FF9-4E3A-8403-AABD635AEED9}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.CodeAnalysis.ResultProvider.Utilities", "src\ExpressionEvaluator\Core\Test\ResultProvider\Microsoft.CodeAnalysis.ResultProvider.Utilities.csproj", "{ABDBAC1E-350E-4DC3-BB45-3504404545EE}"
EndProject
Expand Down
36 changes: 19 additions & 17 deletions src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpExpressionCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,29 @@ internal override DkmCompilerId CompilerId
internal delegate MetadataContext<CSharpMetadataContext> GetMetadataContextDelegate<TAppDomain>(TAppDomain appDomain);
internal delegate void SetMetadataContextDelegate<TAppDomain>(TAppDomain appDomain, MetadataContext<CSharpMetadataContext> metadataContext, bool report);

/// <exception cref="BadMetadataModuleException">Module wasn't included in the compilation due to bad metadata.</exception>
internal override EvaluationContextBase CreateTypeContext(
DkmClrAppDomain appDomain,
ImmutableArray<MetadataBlock> metadataBlocks,
Guid moduleVersionId,
ModuleId moduleId,
int typeToken,
bool useReferencedModulesOnly)
{
return CreateTypeContext(
appDomain,
ad => ad.GetMetadataContext<CSharpMetadataContext>(),
metadataBlocks,
moduleVersionId,
moduleId,
typeToken,
GetMakeAssemblyReferencesKind(useReferencedModulesOnly));
}

/// <exception cref="BadMetadataModuleException">Module wasn't included in the compilation due to bad metadata.</exception>
internal static EvaluationContext CreateTypeContext<TAppDomain>(
TAppDomain appDomain,
GetMetadataContextDelegate<TAppDomain> getMetadataContext,
ImmutableArray<MetadataBlock> metadataBlocks,
Guid moduleVersionId,
ModuleId moduleId,
int typeToken,
MakeAssemblyReferencesKind kind)
{
Expand All @@ -63,14 +65,14 @@ internal static EvaluationContext CreateTypeContext<TAppDomain>(
{
// Avoid using the cache for referenced assemblies only
// since this should be the exceptional case.
compilation = metadataBlocks.ToCompilationReferencedModulesOnly(moduleVersionId);
compilation = metadataBlocks.ToCompilationReferencedModulesOnly(moduleId);
return EvaluationContext.CreateTypeContext(
compilation,
moduleVersionId,
moduleId,
typeToken);
}

var contextId = MetadataContextId.GetContextId(moduleVersionId, kind);
var contextId = MetadataContextId.GetContextId(moduleId, kind);
var previous = getMetadataContext(appDomain);
CSharpMetadataContext previousMetadataContext = default;
if (previous.Matches(metadataBlocks))
Expand All @@ -80,11 +82,11 @@ internal static EvaluationContext CreateTypeContext<TAppDomain>(

// Re-use the previous compilation if possible.
compilation = previousMetadataContext.Compilation;
compilation ??= metadataBlocks.ToCompilation(moduleVersionId, kind);
compilation ??= metadataBlocks.ToCompilation(moduleId, kind);

var context = EvaluationContext.CreateTypeContext(
compilation,
moduleVersionId,
moduleId,
typeToken);

// New type context is not attached to the AppDomain since it is less
Expand All @@ -101,7 +103,7 @@ internal override EvaluationContextBase CreateMethodContext(
ImmutableArray<MetadataBlock> metadataBlocks,
Lazy<ImmutableArray<AssemblyReaders>> unusedLazyAssemblyReaders,
object? symReader,
Guid moduleVersionId,
ModuleId moduleId,
int methodToken,
int methodVersion,
uint ilOffset,
Expand All @@ -114,7 +116,7 @@ internal override EvaluationContextBase CreateMethodContext(
(ad, mc, report) => ad.SetMetadataContext<CSharpMetadataContext>(mc, report),
metadataBlocks,
symReader,
moduleVersionId,
moduleId,
methodToken,
methodVersion,
ilOffset,
Expand All @@ -128,7 +130,7 @@ internal static EvaluationContext CreateMethodContext<TAppDomain>(
SetMetadataContextDelegate<TAppDomain> setMetadataContext,
ImmutableArray<MetadataBlock> metadataBlocks,
object? symReader,
Guid moduleVersionId,
ModuleId moduleId,
int methodToken,
int methodVersion,
uint ilOffset,
Expand All @@ -142,18 +144,18 @@ internal static EvaluationContext CreateMethodContext<TAppDomain>(
{
// Avoid using the cache for referenced assemblies only
// since this should be the exceptional case.
compilation = metadataBlocks.ToCompilationReferencedModulesOnly(moduleVersionId);
compilation = metadataBlocks.ToCompilationReferencedModulesOnly(moduleId);
return EvaluationContext.CreateMethodContext(
compilation,
symReader,
moduleVersionId,
moduleId,
methodToken,
methodVersion,
offset,
localSignatureToken);
}

var contextId = MetadataContextId.GetContextId(moduleVersionId, kind);
var contextId = MetadataContextId.GetContextId(moduleId, kind);
var previous = getMetadataContext(appDomain);
var assemblyContexts = previous.Matches(metadataBlocks) ? previous.AssemblyContexts : ImmutableDictionary<MetadataContextId, CSharpMetadataContext>.Empty;
CSharpMetadataContext previousMetadataContext;
Expand All @@ -167,20 +169,20 @@ internal static EvaluationContext CreateMethodContext<TAppDomain>(
var previousContext = previousMetadataContext.EvaluationContext;
if (previousContext != null &&
previousContext.MethodContextReuseConstraints.HasValue &&
previousContext.MethodContextReuseConstraints.GetValueOrDefault().AreSatisfied(moduleVersionId, methodToken, methodVersion, offset))
previousContext.MethodContextReuseConstraints.GetValueOrDefault().AreSatisfied(moduleId, methodToken, methodVersion, offset))
{
return previousContext;
}
}
else
{
compilation = metadataBlocks.ToCompilation(moduleVersionId, kind);
compilation = metadataBlocks.ToCompilation(moduleId, kind);
}

var context = EvaluationContext.CreateMethodContext(
compilation,
symReader,
moduleVersionId,
moduleId,
methodToken,
methodVersion,
offset,
Expand Down
8 changes: 4 additions & 4 deletions src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpInstructionDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,20 @@ internal override ImmutableArray<TypeParameterSymbol> GetAllTypeParameters(Metho
internal override CSharpCompilation GetCompilation(DkmClrModuleInstance moduleInstance)
{
var appDomain = moduleInstance.AppDomain;
var moduleVersionId = moduleInstance.Mvid;
var moduleId = moduleInstance.GetModuleId();
var previous = appDomain.GetMetadataContext<CSharpMetadataContext>();
var metadataBlocks = moduleInstance.RuntimeInstance.GetMetadataBlocks(appDomain, previous.MetadataBlocks);

var kind = GetMakeAssemblyReferencesKind();
var contextId = MetadataContextId.GetContextId(moduleVersionId, kind);
var contextId = MetadataContextId.GetContextId(moduleId, kind);
var assemblyContexts = previous.Matches(metadataBlocks) ? previous.AssemblyContexts : ImmutableDictionary<MetadataContextId, CSharpMetadataContext>.Empty;
CSharpMetadataContext previousContext;
assemblyContexts.TryGetValue(contextId, out previousContext);

var compilation = previousContext.Compilation;
if (compilation == null)
{
compilation = metadataBlocks.ToCompilation(moduleVersionId, kind);
compilation = metadataBlocks.ToCompilation(moduleId, kind);
appDomain.SetMetadataContext(
new MetadataContext<CSharpMetadataContext>(
metadataBlocks,
Expand All @@ -178,7 +178,7 @@ internal override CSharpCompilation GetCompilation(DkmClrModuleInstance moduleIn
internal override MethodSymbol GetMethod(CSharpCompilation compilation, DkmClrInstructionAddress instructionAddress)
{
var methodHandle = (MethodDefinitionHandle)MetadataTokens.Handle(instructionAddress.MethodId.Token);
return compilation.GetSourceMethod(instructionAddress.ModuleInstance.Mvid, methodHandle);
return compilation.GetSourceMethod(instructionAddress.ModuleInstance.GetModuleId(), methodHandle);
}

internal override TypeNameDecoder<PEModuleSymbol, TypeSymbol> GetTypeNameDecoder(CSharpCompilation compilation, MethodSymbol method)
Expand Down
29 changes: 16 additions & 13 deletions src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ private static PENamedTypeSymbol GetType(PEModuleSymbol module, TypeDefinitionHa
return (PENamedTypeSymbol)metadataDecoder.GetTypeOfToken(typeHandle);
}

internal static PENamedTypeSymbol GetType(this CSharpCompilation compilation, Guid moduleVersionId, int typeToken)
/// <exception cref="BadMetadataModuleException">Module wasn't included in the compilation due to bad metadata.</exception>
internal static PENamedTypeSymbol GetType(this CSharpCompilation compilation, ModuleId moduleId, int typeToken)
{
return GetType(compilation.GetModule(moduleVersionId), (TypeDefinitionHandle)MetadataTokens.Handle(typeToken));
return GetType(compilation.GetModule(moduleId), (TypeDefinitionHandle)MetadataTokens.Handle(typeToken));
}

internal static PEMethodSymbol GetSourceMethod(this CSharpCompilation compilation, Guid moduleVersionId, MethodDefinitionHandle methodHandle)
internal static PEMethodSymbol GetSourceMethod(this CSharpCompilation compilation, ModuleId moduleId, MethodDefinitionHandle methodHandle)
{
var method = GetMethod(compilation, moduleVersionId, methodHandle);
var method = GetMethod(compilation, moduleId, methodHandle);
var metadataDecoder = new MetadataDecoder((PEModuleSymbol)method.ContainingModule);
var containingType = method.ContainingType;
if (GeneratedNameParser.TryParseSourceMethodNameFromGeneratedName(containingType.Name, GeneratedNameKind.StateMachineType, out var sourceMethodName))
Expand All @@ -49,17 +50,19 @@ internal static PEMethodSymbol GetSourceMethod(this CSharpCompilation compilatio
return method;
}

internal static PEMethodSymbol GetMethod(this CSharpCompilation compilation, Guid moduleVersionId, MethodDefinitionHandle methodHandle)
/// <exception cref="BadMetadataModuleException">Module wasn't included in the compilation due to bad metadata.</exception>
internal static PEMethodSymbol GetMethod(this CSharpCompilation compilation, ModuleId moduleId, MethodDefinitionHandle methodHandle)
{
var module = compilation.GetModule(moduleVersionId);
var module = compilation.GetModule(moduleId);
var reader = module.Module.MetadataReader;
var typeHandle = reader.GetMethodDefinition(methodHandle).GetDeclaringType();
var type = GetType(module, typeHandle);
var method = (PEMethodSymbol)new MetadataDecoder(module, type).GetMethodSymbolForMethodDefOrMemberRef(methodHandle, type);
return method;
}

internal static PEModuleSymbol GetModule(this CSharpCompilation compilation, Guid moduleVersionId)
/// <exception cref="BadMetadataModuleException">Module wasn't included in the compilation due to bad metadata.</exception>
internal static PEModuleSymbol GetModule(this CSharpCompilation compilation, ModuleId moduleId)
{
foreach (var pair in compilation.GetBoundReferenceManager().GetReferencedAssemblies())
{
Expand All @@ -68,24 +71,24 @@ internal static PEModuleSymbol GetModule(this CSharpCompilation compilation, Gui
{
var m = (PEModuleSymbol)module;
var id = m.Module.GetModuleVersionIdOrThrow();
if (id == moduleVersionId)
if (id == moduleId.Id)
{
return m;
}
}
}

throw new ArgumentException($"No module found with MVID '{moduleVersionId}'", nameof(moduleVersionId));
throw new BadMetadataModuleException(moduleId);
}

internal static CSharpCompilation ToCompilationReferencedModulesOnly(this ImmutableArray<MetadataBlock> metadataBlocks, Guid moduleVersionId)
internal static CSharpCompilation ToCompilationReferencedModulesOnly(this ImmutableArray<MetadataBlock> metadataBlocks, ModuleId moduleId)
{
return ToCompilation(metadataBlocks, moduleVersionId, kind: MakeAssemblyReferencesKind.DirectReferencesOnly);
return ToCompilation(metadataBlocks, moduleId, kind: MakeAssemblyReferencesKind.DirectReferencesOnly);
}

internal static CSharpCompilation ToCompilation(this ImmutableArray<MetadataBlock> metadataBlocks, Guid moduleVersionId, MakeAssemblyReferencesKind kind)
internal static CSharpCompilation ToCompilation(this ImmutableArray<MetadataBlock> metadataBlocks, ModuleId moduleId, MakeAssemblyReferencesKind kind)
{
var references = metadataBlocks.MakeAssemblyReferences(moduleVersionId, IdentityComparer, kind, out var referencesBySimpleName);
var references = metadataBlocks.MakeAssemblyReferences(moduleId, IdentityComparer, kind, out var referencesBySimpleName);
var options = s_compilationOptions;
if (referencesBySimpleName != null)
{
Expand Down
Loading
Loading