From 925a4e1e650c541b160c53b377ec15e72078e086 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Fri, 26 Jun 2020 23:42:10 +0200 Subject: [PATCH] #2050: Don't put ldflda/ldelema with immediate exceptions into StObj.TargetSlot. The C# translation of StObj will always apply delayed exceptions in these two cases, so putting an instruction with delayed exceptions in that slot would change program semantics. --- .../TestCases/Correctness/NullableTests.cs | 29 +++++++++++++++++++ ICSharpCode.Decompiler/IL/Instructions.cs | 3 +- ICSharpCode.Decompiler/IL/Instructions.tt | 12 ++++++-- .../IL/Instructions/LdFlda.cs | 21 ++++++++++++++ .../IL/Transforms/ILInlining.cs | 4 +++ .../Transforms/TransformArrayInitializers.cs | 2 +- 6 files changed, 67 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs index 6659f1503..2883a0088 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs @@ -26,6 +26,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness { AvoidLifting(); BitNot(); + FieldAccessOrderOfEvaluation(null); } static void AvoidLifting() @@ -82,5 +83,33 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness if (!b) throw new InvalidOperationException(); } + + + static int GetInt() + { + Console.WriteLine("got int"); + return 42; + } + + int intField; + + static void FieldAccessOrderOfEvaluation(NullableTests c) + { + Console.WriteLine("GetInt, then NRE:"); + try { + c.intField = GetInt(); + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + Console.WriteLine("NRE before GetInt:"); + try { +#if CS60 + ref int i = ref c.intField; + i = GetInt(); +#endif + } catch (Exception ex) { + Console.WriteLine(ex.Message); + } + } } } diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 7fd43ab93..5a7a63d75 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2014 Daniel Grunwald +// Copyright (c) 2014-2020 Daniel Grunwald // // Permission is hereby granted, free of charge, to any person obtaining a copy of this // software and associated documentation files (the "Software"), to deal in the Software @@ -4117,6 +4117,7 @@ namespace ICSharpCode.Decompiler.IL base.CheckInvariant(phase); Debug.Assert(target.ResultType == StackType.Ref || target.ResultType == StackType.I); Debug.Assert(value.ResultType == type.GetStackType()); + Debug.Assert(IsValidTarget(target)); } } } diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 69c2640eb..e08e1b6b3 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -1,4 +1,4 @@ -// Copyright (c) 2014 Daniel Grunwald +// Copyright (c) 2014-2020 Daniel Grunwald // // Permission is hereby granted, free of charge, to any person obtaining a copy of this // software and associated documentation files (the "Software"), to deal in the Software @@ -253,7 +253,8 @@ new OpCode("stobj", "Indirect store (store to ref/pointer)." + Environment.NewLine + "Evaluates to the value that was stored (when using type byte/short: evaluates to the truncated value, sign/zero extended back to I4 based on type.GetSign())", CustomClassName("StObj"), CustomArguments(("target", new[] { "Ref", "I" }), ("value", new[] { "type.GetStackType()" })), HasTypeOperand, MemoryAccess, CustomWriteToButKeepOriginal, - SupportsVolatilePrefix, SupportsUnalignedPrefix, MayThrow, ResultType("type.GetStackType()")), + SupportsVolatilePrefix, SupportsUnalignedPrefix, MayThrow, ResultType("type.GetStackType()"), + CustomInvariant("Debug.Assert(IsValidTarget(target));")), new OpCode("box", "Boxes a value.", Unary, HasTypeOperand, MemoryAccess, MayThrow, ResultType("O")), @@ -1158,6 +1159,13 @@ protected override void Disconnected() opCode.WriteOpCodePrefix.Add("if (IsReadOnly)" + Environment.NewLine + "\toutput.Write(\"readonly.\");"); opCode.PerformMatchConditions.Add("IsReadOnly == o.IsReadOnly"); }; + + static Action CustomInvariant(string code) + { + return opCode => { + opCode.Invariants.Add(code); + }; + } static Action Pattern = opCode => { BaseClass("PatternInstruction")(opCode); diff --git a/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs b/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs index 6fdd3e382..8088a4390 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs @@ -43,4 +43,25 @@ namespace ICSharpCode.Decompiler.IL } } } + + public sealed partial class StObj + { + /// + /// For a store to a field or array element, C# will only throw NullReferenceException/IndexOfBoundsException + /// after the value-to-be-stored has been computed. + /// This means a LdFlda/LdElema used as target for StObj must have DelayExceptions==true to allow a translation to C# + /// without changing the program semantics. See https://github.com/icsharpcode/ILSpy/issues/2050 + /// + /// This is part of the StObj invariant. + public static bool IsValidTarget(ILInstruction inst) + { + switch (inst.OpCode) { + case OpCode.LdElema: + case OpCode.LdFlda: + return !inst.HasDirectFlag(InstructionFlags.MayThrow); + default: + return true; + } + } + } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index ca20dbbb7..d180411ba 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -581,6 +581,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms return FindResult.Stop; if (expr.MatchLdLoc(v) || expr.MatchLdLoca(v)) { // Match found, we can inline + if (expr.SlotInfo == StObj.TargetSlot && !StObj.IsValidTarget(expressionBeingMoved)) { + // special case: the StObj.TargetSlot does not accept some kinds of expressions + return FindResult.Stop; + } return FindResult.Found(expr); } else if (expr is Block block) { // Inlining into inline-blocks? diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs index 6a9d149c1..f4d9b6b20 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs @@ -670,7 +670,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (type.GetStackType() != value.ResultType) { value = new Conv(value, type.ToPrimitiveType(), false, Sign.None); } - return new StObj(new LdElema(type, array, indices), value, type); + return new StObj(new LdElema(type, array, indices) { DelayExceptions = true }, value, type); } internal static ILInstruction GetNullExpression(IType elementType)