Skip to content
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
15 changes: 7 additions & 8 deletions src/ILInspector.Decompiler.Tests/CompileBackGateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ public class CompileBackGateTests
/// decompiler docket. Each is a tracked defect or a benign over-render; the gate
/// tolerates these but fails if a NEW method joins the set. Shrink this list as
/// fixes land. Tracked defects include StaleFieldRead (issue #605), Shadowed
/// (a dropped this.field load), the .ctor field-initializer ordering, and the
/// definite-assignment default-init over-render (TryFinallyAdd, PowerOfTwo,
/// CatchEverything, SwitchCase, WhileLoop, ClassicLock, TryFinallyTwoReturns,
/// ManualDisposeAsyncInFinally).
/// (a dropped this.field load), and the definite-assignment default-init
/// over-render (TryFinallyAdd, PowerOfTwo, CatchEverything, SwitchCase,
/// WhileLoop, ClassicLock, TryFinallyTwoReturns, ManualDisposeAsyncInFinally).
/// </summary>
static readonly HashSet<string> KnownDiffs = new(StringComparer.Ordinal)
{
".ctor",
"BothPositive",
"CatchEverything",
"ClassicLock",
Expand All @@ -48,10 +46,11 @@ public class CompileBackGateTests
/// <summary>
/// Methods a prior compile-back fix turned opcode-exact. Pinning them guards the
/// fix durably: CheckedAdd must keep the overflow check (#604), UnsignedShift
/// must keep dropping the redundant width mask (#606), and Shadowed must keep
/// qualifying the shadowed this.field load (#607).
/// must keep dropping the redundant width mask (#606), Shadowed must keep
/// qualifying the shadowed this.field load (#607), and .ctor must keep lifting
/// its field initializer ahead of the base call to the field declaration.
/// </summary>
static readonly string[] PinnedExact = { "CheckedAdd", "UnsignedShift", "Shadowed" };
static readonly string[] PinnedExact = { "CheckedAdd", "UnsignedShift", "Shadowed", ".ctor" };

static IReadOnlyList<CompileBack.CompileBackResult> EvaluateFixtures()
{
Expand Down
48 changes: 43 additions & 5 deletions src/ILInspector.Decompiler.Tests/IrImporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -960,17 +960,20 @@ static string ReturnOf(string sourceType, string returnType)
[Fact]
public void Constructor_ImplicitBaseCall_IsSuppressed()
{
// A no-argument base-constructor call is implicit in C#; only the
// field initializer remains in the body. (Argumentful base(...)
// still prints until constructor initializers are modeled.)
// A no-argument base-constructor call is implicit in C#, and the field
// initializer (`_shadowed = 1`) the compiler emits before that base call
// lifts to the field declaration — not the body, where it would
// recompile to AFTER the base call.
using var source = MetadataSource.Open(typeof(CfgSampleClass).Assembly.Location);
var function = IrImporter.Import(source, typeof(CfgSampleClass).FullName!, ".ctor");

Assert.NotNull(function);
string output = CSharpPrinter.Print(function).Output!;
var result = CSharpPrinter.Print(function);
string output = result.Output!;
Assert.DoesNotContain("base(", output);
Assert.DoesNotContain(".ctor", output);
Assert.Contains("_shadowed = 1;", output);
Assert.DoesNotContain("_shadowed", output);
Assert.Contains(("_shadowed", "1"), result.FieldInitializers);
}

[Fact]
Expand Down Expand Up @@ -2118,6 +2121,41 @@ public void ThisDelegation_LiftsToInitializer()
Assert.DoesNotContain("base(", chain);
Assert.DoesNotContain("..ctor", chain);
}

static DecompilerResult RaiseDefaultCtor(Type type)
{
using var source = MetadataSource.Open(type.Assembly.Location);
var function = IrImporter.Import(source, type.FullName!, ".ctor");
Assert.NotNull(function);
var result = CSharpPrinter.PrintRaised(function);
Assert.True(result.Succeeded);
return result;
}

[Fact]
public void ConstantFieldInitializer_LiftsToFieldDeclaration()
{
// CfgSampleClass declares `int _shadowed = 1;`. The compiler emits that
// store before the (implicit) base call, so it is a field initializer,
// not a body assignment — lift it to the field declaration and drop it
// from the body (where it would recompile to AFTER the base call).
var result = RaiseDefaultCtor(typeof(CfgSampleClass));

Assert.Contains(("_shadowed", "1"), result.FieldInitializers);
Assert.DoesNotContain("_shadowed", result.Output);
}

[Fact]
public void NewObjectFieldInitializer_LiftsToFieldDeclaration()
{
// LockFixtureSamples declares `readonly object _root = new();`. The
// `new object()` initializer is self-contained (no this/parameter/local
// load), so it is legal in field-declaration context and lifts there.
var result = RaiseDefaultCtor(typeof(LockFixtureSamples));

Assert.Contains(("_root", "new object()"), result.FieldInitializers);
Assert.DoesNotContain("_root", result.Output);
}
}

public class IdentityConvertTests
Expand Down
9 changes: 9 additions & 0 deletions src/ILInspector.Decompiler/DecompilerResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ public sealed record DecompilerResult(
/// </summary>
public string? ConstructorChain { get; init; }

/// <summary>
/// For a constructor, the field initializers (<c>field</c>, <c>value</c>
/// pairs) lifted out of <see cref="Output"/> — <c>this.field = value</c>
/// stores the IL placed before the base call, which C# spells on the field
/// declaration, not in the body. The formatter that renders the field
/// declarations places them; empty when there are none.
/// </summary>
public IReadOnlyList<(string Field, string Value)> FieldInitializers { get; init; } = [];

public static DecompilerResult Success(string output)
=> new(output, DecompilationFidelity.Full, []);

Expand Down
75 changes: 64 additions & 11 deletions src/ILInspector.Decompiler/Pipeline/Ir/CSharpPrinter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public static DecompilerResult Print(IrFunction function)
return new DecompilerResult(output, function.Fidelity, [.. function.Diagnostics])
{
ConstructorChain = printer._constructorChain,
FieldInitializers = printer._fieldInitializers,
};
}
catch (Exception ex)
Expand All @@ -63,23 +64,41 @@ public static DecompilerResult Print(IrFunction function)
string? _constructorChain;
IrNode? _chainStatement;

/// <summary>Field initializers (<c>this.f = value</c> stores preceding the base call) lifted out of a constructor body to the field declarations, keyed in source order.</summary>
readonly List<(string Field, string Value)> _fieldInitializers = [];
readonly HashSet<IrNode> _fieldInitStores = [];

string PrintBody(IrFunction function)
{
var sb = new StringBuilder();
_labelTargets = CollectBranchTargets(function);
CollectDeclaringStores(function);
_readBeforeAssign = ComputeReadBeforeAssign(function);

// An explicit base(...)/this(...) chain opens a constructor body; lift
// it to a signature initializer so the body stays valid C# (a base/this
// call as a body statement is CS0175).
if (function.Body.Blocks is [{ Children: [var first, ..] }, ..]
&& first is ExpressionStatement { Expression: Call { Callee: { Name: ".ctor", HasThis: true } callee } call }
&& call.Arguments is [LoadArgument { Index: 0 }, ..]
&& ConstructorChainText(callee, call) is { } chain)
// A constructor prologue is leading field-initializer stores
// (this.f = value) followed by the base(...)/this(...) chain call. C#
// emits field initializers before the base call, so a this-field store
// preceding the chain call is a field initializer — not a body
// assignment — and the chain call is invalid as a body statement
// (CS0175). Lift both out of the body so they render where they belong:
// the initializers on the field declarations, the chain on the
// signature.
if (function.Body.Blocks is [{ } entry, ..]
&& ChainCallIndex(entry) is { } chainIndex
&& entry.Children.Take(chainIndex).All(IsFieldInitializerStore))
{
_constructorChain = chain.TrimEnd(';');
_chainStatement = first;
foreach (var store in entry.Children.Take(chainIndex).Cast<StoreField>())
{
_fieldInitStores.Add(store);
_fieldInitializers.Add((store.Field.Name, Expression(store.Value)));
}

var chainCall = (Call)((ExpressionStatement)entry.Children[chainIndex]).Expression;
if (ConstructorChainText(chainCall.Callee, chainCall) is { } chain)
{
_constructorChain = chain.TrimEnd(';');
_chainStatement = entry.Children[chainIndex];
}
}

// Remaining locals and slots declare up front, current-style.
Expand Down Expand Up @@ -107,8 +126,8 @@ void AppendContainer(StringBuilder sb, BlockContainer container, int indent, boo
bool labeledReturnOnly = _labelTargets.Contains(block.StartOffset) && block.Children.Count == 1;
foreach (var statement in block.Children)
{
if (ReferenceEquals(statement, _chainStatement))
continue; // lifted to the signature initializer
if (ReferenceEquals(statement, _chainStatement) || _fieldInitStores.Contains(statement))
continue; // lifted to the signature initializer / field declarations
bool isLast = topLevel && i == blocks.Count - 1 && ReferenceEquals(statement, block.Children[^1]);
if (isLast && !labeledReturnOnly && statement is Return { Value: null })
break;
Expand Down Expand Up @@ -687,6 +706,40 @@ void AppendStatement(StringBuilder sb, IrNode node, int indent)
return $"{(isThis ? "this" : "base")}({Arguments(arguments, callee.ParameterTypes, callee.ParameterRefKinds)});";
}

/// <summary>The index of the base/this <c>.ctor</c> chain call in the entry block, or null when the body has none (a struct ctor, a static method, a body that never chains).</summary>
static int? ChainCallIndex(Block entry)
{
for (int i = 0; i < entry.Children.Count; i++)
{
if (entry.Children[i] is ExpressionStatement { Expression: Call { Callee: { Name: ".ctor", HasThis: true } } call }
&& call.Arguments is [LoadArgument { Index: 0 }, ..])
{
return i;
}
}
return null;
}

/// <summary>
/// A <c>this.field = value</c> store whose value is a field initializer:
/// self-contained (no <c>this</c>, parameter, local, or slot load), so it is
/// legal in field-declaration context. C# field initializers cannot read the
/// instance or constructor parameters, which is exactly the place-load ban.
/// </summary>
static bool IsFieldInitializerStore(IrNode node)
=> node is StoreField { HasInstance: true, Instance: LoadArgument { Index: 0 } } store
&& !ReferencesPlace(store.Value);

static bool ReferencesPlace(IrExpression value)
{
foreach (var node in (IEnumerable<IrNode>)[value, .. value.Descendants])
{
if (node is LoadArgument or LoadLocal or LoadStackSlot or LoadLocalAddress or LoadArgumentAddress)
return true;
}
return false;
}

/// <summary>Baseline-style clause headers: bare <c>catch</c> for object (the catch-all), the variable form when the entry store folded into the clause.</summary>
string CatchHeader(CatchClause clause)
=> clause.ExceptionType is { Namespace: "System", Name: "Object" }
Expand Down
37 changes: 24 additions & 13 deletions tools/DecompilerHarness/CompileBack.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ static void EvaluateType(
continue;
string? body;
string? chain;
try { var printed = CSharpPrinter.PrintRaised(function); body = printed.Output; chain = printed.ConstructorChain; }
IReadOnlyList<(string Field, string Value)> fieldInits;
try { var printed = CSharpPrinter.PrintRaised(function); body = printed.Output; chain = printed.ConstructorChain; fieldInits = printed.FieldInitializers; }
catch { continue; }
if (body is null)
continue;
Expand All @@ -180,7 +181,7 @@ static void EvaluateType(
string origText = string.Join(" ", origOps);

string unit;
try { unit = BuildUnit(reader, mh, body, chain); }
try { unit = BuildUnit(reader, mh, body, chain, fieldInits); }
catch
{
results.Add(new(fullType, name, overload, CompileBackStatus.ContextFail, origText, "", "skeleton-emit"));
Expand Down Expand Up @@ -259,7 +260,8 @@ static void RunType(
continue;
string? body;
string? chain;
try { var printed = CSharpPrinter.PrintRaised(function); body = printed.Output; chain = printed.ConstructorChain; }
IReadOnlyList<(string Field, string Value)> fieldInits;
try { var printed = CSharpPrinter.PrintRaised(function); body = printed.Output; chain = printed.ConstructorChain; fieldInits = printed.FieldInitializers; }
catch { continue; }
if (body is null)
continue;
Expand All @@ -273,7 +275,7 @@ static void RunType(
var origOps = original.Select(i => CanonicalOpcode(i.OpCodeName)).ToList();

string unit;
try { unit = BuildUnit(reader, mh, body, chain); }
try { unit = BuildUnit(reader, mh, body, chain, fieldInits); }
catch { contextFail++; continue; }

var tree = CSharpSyntaxTree.ParseText(unit, parseOptions);
Expand Down Expand Up @@ -355,7 +357,8 @@ static void Report(
/// body's references to same-assembly types and members all bind — so a
/// dropped or mis-bound access surfaces as a true opcode diff, not CS0234/CS0122.
/// </summary>
static string BuildUnit(MetadataReader reader, MethodDefinitionHandle target, string targetBody, string? targetChain)
static string BuildUnit(MetadataReader reader, MethodDefinitionHandle target, string targetBody, string? targetChain,
IReadOnlyList<(string Field, string Value)> targetFieldInits)
{
var sb = new StringBuilder();
sb.AppendLine("#pragma warning disable");
Expand All @@ -372,12 +375,12 @@ static string BuildUnit(MetadataReader reader, MethodDefinitionHandle target, st
{
sb.AppendLine($"namespace {ns}");
sb.AppendLine("{");
EmitType(reader, typeHandle, target, targetBody, targetChain, sb, 1);
EmitType(reader, typeHandle, target, targetBody, targetChain, targetFieldInits, sb, 1);
sb.AppendLine("}");
}
else
{
EmitType(reader, typeHandle, target, targetBody, targetChain, sb, 0);
EmitType(reader, typeHandle, target, targetBody, targetChain, targetFieldInits, sb, 0);
}
}
return sb.ToString();
Expand Down Expand Up @@ -407,7 +410,8 @@ static string BaseClause(MetadataReader reader, TypeDefinition typeDef, TypeKind
}

static void EmitType(MetadataReader reader, TypeDefinitionHandle typeHandle,
MethodDefinitionHandle target, string targetBody, string? targetChain, StringBuilder sb, int indent)
MethodDefinitionHandle target, string targetBody, string? targetChain,
IReadOnlyList<(string Field, string Value)> targetFieldInits, StringBuilder sb, int indent)
{
var typeDef = reader.GetTypeDefinition(typeHandle);
var kind = ShapeOf(reader, typeDef);
Expand Down Expand Up @@ -440,8 +444,13 @@ static void EmitType(MetadataReader reader, TypeDefinitionHandle typeHandle,
sb.AppendLine($"{pad}public unsafe {keyword} {Identifier(name)}{genParams}{baseClause}");
sb.AppendLine($"{pad}{{");

// Field initializers lifted from the target ctor apply to this type's
// fields only when it declares that ctor.
bool declaresTarget = !target.IsNil && typeDef.GetMethods().Any(mh => mh == target);
var fieldInits = declaresTarget ? targetFieldInits : [];

foreach (var fh in typeDef.GetFields())
EmitField(reader, fh, typeContext, sb, pad + " ");
EmitField(reader, fh, typeContext, fieldInits, sb, pad + " ");

foreach (var mh in typeDef.GetMethods())
EmitMethod(reader, typeHandle, mh, mh == target ? targetBody : null, mh == target ? targetChain : null, sb, pad + " ");
Expand All @@ -450,7 +459,7 @@ static void EmitType(MetadataReader reader, TypeDefinitionHandle typeHandle,
{
if (reader.GetString(reader.GetTypeDefinition(nested).Name).Contains('<'))
continue; // compiler-generated (display class, iterator) — not valid C#
EmitType(reader, nested, target, targetBody, targetChain, sb, indent + 1);
EmitType(reader, nested, target, targetBody, targetChain, targetFieldInits, sb, indent + 1);
}

sb.AppendLine($"{pad}}}");
Expand Down Expand Up @@ -531,7 +540,7 @@ static void EmitInterface(MetadataReader reader, TypeDefinitionHandle typeHandle
{
if (reader.GetString(reader.GetTypeDefinition(nested).Name).Contains('<'))
continue;
EmitType(reader, nested, default, "", null, sb, indent + 1);
EmitType(reader, nested, default, "", null, [], sb, indent + 1);
}

sb.AppendLine($"{pad}}}");
Expand Down Expand Up @@ -563,7 +572,7 @@ static void EmitEnum(MetadataReader reader, TypeDefinition typeDef, string name,
}

static void EmitField(MetadataReader reader, FieldDefinitionHandle fh, GenericContext context,
StringBuilder sb, string pad)
IReadOnlyList<(string Field, string Value)> fieldInits, StringBuilder sb, string pad)
{
var field = reader.GetFieldDefinition(fh);
string name = reader.GetString(field.Name);
Expand All @@ -583,7 +592,9 @@ static void EmitField(MetadataReader reader, FieldDefinitionHandle fh, GenericCo
sb.AppendLine($"{pad}public const {Clean(type)} {Identifier(name)} = {value};");
return;
}
sb.AppendLine($"{pad}public {(isStatic ? "static " : "")}{Clean(type)} {Identifier(name)};");
string? initializer = fieldInits.FirstOrDefault(fi => fi.Field == name).Value;
string suffix = initializer is not null && !isStatic ? $" = {initializer}" : "";
sb.AppendLine($"{pad}public {(isStatic ? "static " : "")}{Clean(type)} {Identifier(name)}{suffix};");
}

static void EmitMethod(MetadataReader reader, TypeDefinitionHandle typeHandle,
Expand Down
Loading