From 2271229b23243561da3fca66a8c386fbeca705a5 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 22 Nov 2025 22:22:42 +0100 Subject: [PATCH] Fix #3618: Extend validation of declared members and ctor parameters --- .../TestCases/Pretty/Records.cs | 29 ++++ .../CSharp/RecordDecompiler.cs | 132 ++++++++++++++---- 2 files changed, 132 insertions(+), 29 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs index d5016bd5d..4c9e7d424 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs @@ -18,6 +18,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty public record Derived(int B) : Base(B.ToString()); + public record BaseRecordWithObject(object Id); + + public record DerivedRecordWithString : BaseRecordWithObject + { + public DerivedRecordWithString(string Id) + : base(Id) + { + } + } + public record Empty; public record Fields @@ -400,6 +410,25 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty this.A = int.Parse(A); } } + + public record struct RecordCtorChain(int A, string B) + { +#if EXPECTED_OUTPUT + public double C = 0.0; +#else + public double C; +#endif + public RecordCtorChain(int A) + : this(A, "default") + { + C = 3.14; + } + public RecordCtorChain(string B) + : this(42, B) + { + C = 1.41; + } + } } #endif } diff --git a/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs b/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs index f38f2aa5a..a2a7dec3b 100644 --- a/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs @@ -167,6 +167,14 @@ namespace ICSharpCode.Decompiler.CSharp var subst = recordTypeDef.AsParameterizedType().GetSubstitution(); + Dictionary nameToMemberMap = new Dictionary(StringComparer.Ordinal); + Dictionary ctorChainMap = new Dictionary(); + + foreach (IMember m in recordTypeDef.GetMembers(m => m.SymbolKind is SymbolKind.Property or SymbolKind.Field, GetMemberOptions.ReturnMemberDefinitions)) + { + nameToMemberMap[m.Name] = m; + } + IMethod guessedPrimaryCtor = null; foreach (var method in recordTypeDef.Methods) @@ -178,8 +186,21 @@ namespace ICSharpCode.Decompiler.CSharp { continue; } + var body = DecompileBody(method); + if (body == null) + { + continue; + } + IMethod chainedCtor = (IMethod)FindChainedCtor(body)?.MemberDefinition; + ctorChainMap[method] = chainedCtor; + + if (chainedCtor != null && chainedCtor.DeclaringTypeDefinition.Equals(recordTypeDef)) + { + continue; + } + var m = method.Specialize(subst); - if (IsPrimaryConstructor(m, method)) + if (IsPrimaryConstructor(body, m, method)) { if (guessedPrimaryCtor == null) { @@ -194,6 +215,26 @@ namespace ICSharpCode.Decompiler.CSharp } } + if (guessedPrimaryCtor != null) + { + foreach (var (source, target) in ctorChainMap) + { + if (guessedPrimaryCtor.Equals(source)) + continue; + // in a record with a primary ctor every other ctor must call the primary ctor + // at the end of the ctor chain. + // if the target is null or a ctor of another type, we found a ctor that does not + // follow this rule. + // we don't have to check the full chain, because the C# compiler enforces that + // there are no loops in the ctor call graph. + if (target == null || !target.DeclaringTypeDefinition!.Equals(recordTypeDef)) + { + guessedPrimaryCtor = null; + break; + } + } + } + if (guessedPrimaryCtor == null) { primaryCtorParameterToAutoProperty.Clear(); @@ -213,12 +254,22 @@ namespace ICSharpCode.Decompiler.CSharp return guessedPrimaryCtor; - bool IsPrimaryConstructor(IMethod method, IMethod unspecializedMethod) + bool IsPrimaryConstructor(Block body, IMethod method, IMethod unspecializedMethod) { - Debug.Assert(method.IsConstructor); - var body = DecompileBody(method); - if (body == null) - return false; + foreach (IParameter p in unspecializedMethod.Parameters) + { + // ref and out are not valid modifiers in this context + if (p.ReferenceKind is ReferenceKind.Ref or ReferenceKind.Out) + return false; + + // for each positional parameter there must be a field or property of the same name and type + if (!nameToMemberMap.TryGetValue(p.Name, out var member)) + return false; + + var paramType = p.ReferenceKind != ReferenceKind.None ? p.Type.UnwrapByRef() : p.Type; + if (!NormalizeTypeVisitor.TypeErasure.EquivalentTypes(paramType, member.ReturnType)) + return false; + } if (!isStruct) { @@ -236,50 +287,73 @@ namespace ICSharpCode.Decompiler.CSharp if (body.Instructions.Count < addonInst) return false; + int parameterIndex = 0; + for (int i = 0; i < body.Instructions.Count - addonInst; i++) { if (!body.Instructions[i].MatchStFld(out var target, out var field, out var valueInst)) return false; if (!target.MatchLdThis()) return false; - if (i < method.Parameters.Count) + // allow assignments to fields that are not backing fields of auto-properties + if (!backingFieldToAutoProperty.TryGetValue(field, out var property)) + continue; + if (valueInst.MatchLdLoc(out var v)) { - if (method.Parameters[i].ReferenceKind is ReferenceKind.In or ReferenceKind.RefReadOnly) - { - if (!valueInst.MatchLdObj(out valueInst, out _)) - return false; - } - if (!valueInst.MatchLdLoc(out var value)) + if (v.Kind != VariableKind.Parameter || v.Index < parameterIndex) return false; - if (!(value.Kind == VariableKind.Parameter && value.Index >= 0 && value.Index < method.Parameters.Count)) - return false; - - if (!backingFieldToAutoProperty.TryGetValue(field, out var property)) - { + parameterIndex = v.Index.Value; + } + else if (valueInst.MatchLdObj(out valueInst, out _) && valueInst.MatchLdLoc(out v)) + { + if (v.Kind != VariableKind.Parameter || v.Index < parameterIndex) return false; - } - - IParameter parameter = unspecializedMethod.Parameters[i]; - if (primaryCtorParameterToAutoProperty.ContainsKey(parameter)) + parameterIndex = v.Index.Value; + if (method.Parameters[parameterIndex].ReferenceKind is ReferenceKind.None) { return false; } + } + else + { + continue; + } + IParameter parameter = unspecializedMethod.Parameters[parameterIndex]; + if (primaryCtorParameterToAutoProperty.ContainsKey(parameter)) + { + continue; + } - if (recordTypeDef.Kind != TypeKind.Struct) + if (recordTypeDef.Kind != TypeKind.Struct) + { + if (!(property.CanSet && property.Setter.IsInitOnly)) { - if (!(property.CanSet && property.Setter.IsInitOnly)) - { - continue; - } + continue; } - - primaryCtorParameterToAutoProperty.Add(parameter, property); } + primaryCtorParameterToAutoProperty.Add(parameter, property); } var returnInst = body.Instructions.LastOrDefault(); return returnInst != null && returnInst.MatchReturn(out var retVal) && retVal.MatchNop(); } + + IMethod FindChainedCtor(Block body) + { + // look for a call instruction or assignment to a chained constructor + foreach (var inst in body.Instructions) + { + switch (inst) + { + case Call { Method: { IsConstructor: true } ctor }: + return ctor; + case StObj { Value: NewObj { Method: var ctor } } stObj when stObj.Target.MatchLdThis(): + return ctor; + } + } + + return null; + } } static List DetectMemberOrder(ITypeDefinition recordTypeDef, Dictionary backingFieldToAutoProperty)