Browse Source

Fix #2541: Add explicit `unchecked()` around non-constant cast of constant to `nint`

pull/2560/head
Daniel Grunwald 4 years ago
parent
commit
7f985757a7
  1. 2
      ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs
  2. 12
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/NativeInts.cs
  3. 2
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs
  4. 33
      ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs
  5. 17
      ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs

2
ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs

@ -103,14 +103,12 @@ namespace ICSharpCode.Decompiler.Tests
} }
[Test] [Test]
[Ignore("See https://github.com/icsharpcode/ILSpy/issues/2541 - Waiting for https://github.com/dotnet/roslyn/issues/45929")]
public void ExplicitConversions_With_NativeInts() public void ExplicitConversions_With_NativeInts()
{ {
RunWithOutput("Random Tests\\TestCases", "ExplicitConversions.exe", LanguageVersion.CSharp9_0); RunWithOutput("Random Tests\\TestCases", "ExplicitConversions.exe", LanguageVersion.CSharp9_0);
} }
[Test] [Test]
[Ignore("See https://github.com/icsharpcode/ILSpy/issues/2541 - Waiting for https://github.com/dotnet/roslyn/issues/45929")]
public void ExplicitConversions_32_With_NativeInts() public void ExplicitConversions_32_With_NativeInts()
{ {
RunWithOutput("Random Tests\\TestCases", "ExplicitConversions_32.exe", LanguageVersion.CSharp9_0); RunWithOutput("Random Tests\\TestCases", "ExplicitConversions_32.exe", LanguageVersion.CSharp9_0);

12
ICSharpCode.Decompiler.Tests/TestCases/Pretty/NativeInts.cs

@ -205,5 +205,17 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
{ {
return (nint)(0 - x); return (nint)(0 - x);
} }
public nint SignedNotFittingIn32Bits()
{
// Explicit `unchecked` is necessary when casting oversized constant to nint
return unchecked((nint)9123123123123L);
}
public nuint UnsignedNotFittingIn32Bits()
{
// Explicit `unchecked` is necessary when casting oversized constant to nuint
return unchecked((nuint)9123123123123uL);
}
} }
} }

2
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -120,10 +120,12 @@ namespace ICSharpCode.Decompiler.CSharp
else if (rr.Type.IsCSharpSmallIntegerType()) else if (rr.Type.IsCSharpSmallIntegerType())
{ {
expr = new CastExpression(new PrimitiveType(KnownTypeReference.GetCSharpNameByTypeCode(rr.Type.GetDefinition().KnownTypeCode)), expr); expr = new CastExpression(new PrimitiveType(KnownTypeReference.GetCSharpNameByTypeCode(rr.Type.GetDefinition().KnownTypeCode)), expr);
// Note: no unchecked annotation necessary, because the constant was folded to be in-range
} }
else if (rr.Type.IsCSharpNativeIntegerType()) else if (rr.Type.IsCSharpNativeIntegerType())
{ {
expr = new CastExpression(new PrimitiveType(rr.Type.Name), expr); expr = new CastExpression(new PrimitiveType(rr.Type.Name), expr);
// Note: no unchecked annotation necessary, because the rr wouldn't be a constant if the value wasn't in-range on 32bit
} }
} }
var exprRR = expr.Annotation<ResolveResult>(); var exprRR = expr.Annotation<ResolveResult>();

33
ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs

@ -35,10 +35,16 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
/// true=checked, false=unchecked /// true=checked, false=unchecked
/// </summary> /// </summary>
public bool IsChecked; public bool IsChecked;
/// <summary>
/// Require an explicit unchecked block (can't rely on the project-level unchecked context)
/// </summary>
public bool IsExplicit;
} }
public static readonly object CheckedAnnotation = new CheckedUncheckedAnnotation { IsChecked = true }; public static readonly object CheckedAnnotation = new CheckedUncheckedAnnotation { IsChecked = true };
public static readonly object UncheckedAnnotation = new CheckedUncheckedAnnotation { IsChecked = false }; public static readonly object UncheckedAnnotation = new CheckedUncheckedAnnotation { IsChecked = false };
public static readonly object ExplicitUncheckedAnnotation = new CheckedUncheckedAnnotation { IsChecked = false, IsExplicit = true };
#endregion #endregion
/* /*
@ -114,7 +120,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
/// <summary> /// <summary>
/// Gets the new cost if an expression with this cost is wrapped in a checked/unchecked expression. /// Gets the new cost if an expression with this cost is wrapped in a checked/unchecked expression.
/// </summary> /// </summary>
/// <returns></returns>
internal Cost WrapInCheckedExpr() internal Cost WrapInCheckedExpr()
{ {
if (Expressions == 0) if (Expressions == 0)
@ -349,13 +354,27 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
CheckedUncheckedAnnotation annotation = expr.Annotation<CheckedUncheckedAnnotation>(); CheckedUncheckedAnnotation annotation = expr.Annotation<CheckedUncheckedAnnotation>();
if (annotation != null) if (annotation != null)
{ {
// If the annotation requires this node to be in a specific context, add a huge cost to the other context if (annotation.IsExplicit)
// That huge cost gives us the option to ignore a required checked/unchecked expression when there wouldn't be any {
// solution otherwise. (e.g. "for (checked(M().x += 1); true; unchecked(M().x += 2)) {}") // We don't yet support distinguishing CostInUncheckedContext vs. CostInExplicitUncheckedContext,
if (annotation.IsChecked) // so we always force an unchecked() expression here.
result.CostInUncheckedContext += new Cost(10000, 0); if (annotation.IsChecked)
throw new NotImplementedException("explicit checked"); // should not be needed
result.CostInCheckedContext = result.CostInUncheckedContext.WrapInCheckedExpr();
result.CostInUncheckedContext = result.CostInUncheckedContext.WrapInCheckedExpr();
result.NodesToInsertInUncheckedContext += new InsertedExpression(expr, annotation.IsChecked);
result.NodesToInsertInCheckedContext = result.NodesToInsertInUncheckedContext;
}
else else
result.CostInCheckedContext += new Cost(10000, 0); {
// If the annotation requires this node to be in a specific context, add a huge cost to the other context
// That huge cost gives us the option to ignore a required checked/unchecked expression when there wouldn't be any
// solution otherwise. (e.g. "for (checked(M().x += 1); true; unchecked(M().x += 2)) {}")
if (annotation.IsChecked)
result.CostInUncheckedContext += new Cost(10000, 0);
else
result.CostInCheckedContext += new Cost(10000, 0);
}
} }
// Embed this node in an checked/unchecked expression: // Embed this node in an checked/unchecked expression:
if (expr.Parent is ExpressionStatement) if (expr.Parent is ExpressionStatement)

17
ICSharpCode.Decompiler/CSharp/TranslatedExpression.cs

@ -569,7 +569,22 @@ namespace ICSharpCode.Decompiler.CSharp
bool needsCheckAnnotation = targetUType.GetStackType().IsIntegerType(); bool needsCheckAnnotation = targetUType.GetStackType().IsIntegerType();
if (needsCheckAnnotation) if (needsCheckAnnotation)
{ {
castExpr.AddAnnotation(checkForOverflow ? AddCheckedBlocks.CheckedAnnotation : AddCheckedBlocks.UncheckedAnnotation); if (checkForOverflow)
{
castExpr.AddAnnotation(AddCheckedBlocks.CheckedAnnotation);
}
else if (ResolveResult.IsCompileTimeConstant && targetUType.IsCSharpNativeIntegerType())
{
// unchecked potentially-overflowing cast of constant to n(u)int:
// Placement in implicitly unchecked context is not good enough when applied to compile-time constant,
// the constant must be placed into an explicit unchecked block.
// (note that non-potentially-overflowing casts will be handled by constant folding and won't get here)
castExpr.AddAnnotation(AddCheckedBlocks.ExplicitUncheckedAnnotation);
}
else
{
castExpr.AddAnnotation(AddCheckedBlocks.UncheckedAnnotation);
}
} }
return castExpr.WithoutILInstruction().WithRR(rr); return castExpr.WithoutILInstruction().WithRR(rr);
} }

Loading…
Cancel
Save