Skip to content

Commit

Permalink
don't try to underflow comparisons to very large numbers (#413)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jamiras
Jamiras authored Sep 13, 2023
1 parent 1759639 commit 0eb86e2
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 34 deletions.
67 changes: 58 additions & 9 deletions Source/Parser/Expressions/IntegerConstantExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ public bool IsZero
/// <summary>
/// Returns <c>true</c> if the constant is numerically negative
/// </summary>
public bool IsNegative
public virtual bool IsNegative
{
get { return Value < 0; }
}

/// <summary>
/// Returns <c>true</c> if the constant is numerically positive
/// </summary>
public bool IsPositive
public virtual bool IsPositive
{
get { return Value > 0; }
}
Expand Down Expand Up @@ -91,36 +91,52 @@ public ExpressionBase Combine(ExpressionBase right, MathematicOperation operatio
var integerExpression = right as IntegerConstantExpression;
if (integerExpression != null)
{
var newValue = 0;
switch (operation)
{
case MathematicOperation.Add:
return new IntegerConstantExpression(Value + integerExpression.Value);
newValue = Value + integerExpression.Value;
break;

case MathematicOperation.Subtract:
return new IntegerConstantExpression(Value - integerExpression.Value);
newValue = Value - integerExpression.Value;
break;

case MathematicOperation.Multiply:
return new IntegerConstantExpression(Value * integerExpression.Value);
newValue = Value * integerExpression.Value;
break;

case MathematicOperation.Divide:
if (integerExpression.Value == 0)
return new ErrorExpression("Division by zero");
return new IntegerConstantExpression(Value / integerExpression.Value);
newValue = Value / integerExpression.Value;
break;

case MathematicOperation.Modulus:
if (integerExpression.Value == 0)
return new ErrorExpression("Division by zero");
return new IntegerConstantExpression(Value % integerExpression.Value);
newValue = Value % integerExpression.Value;
break;

case MathematicOperation.BitwiseAnd:
return new IntegerConstantExpression(Value & integerExpression.Value);
newValue = Value & integerExpression.Value;
break;

case MathematicOperation.BitwiseXor:
return new IntegerConstantExpression(Value ^ integerExpression.Value);
newValue = Value ^ integerExpression.Value;
break;

default:
break;
}

if (right is UnsignedIntegerConstantExpression ||
this is UnsignedIntegerConstantExpression)
{
return new UnsignedIntegerConstantExpression((uint)newValue);
}

return new IntegerConstantExpression(newValue);
}

if (right is FloatConstantExpression)
Expand Down Expand Up @@ -182,4 +198,37 @@ public ExpressionBase NormalizeComparison(ExpressionBase right, ComparisonOperat
return null;
}
}

internal class UnsignedIntegerConstantExpression : IntegerConstantExpression
{
public UnsignedIntegerConstantExpression(uint value)
: base((int)value)
{
}

/// <summary>
/// Returns <c>true</c> if the constant is numerically negative
/// </summary>
public override bool IsNegative
{
get { return false; }
}

/// <summary>
/// Returns <c>true</c> if the constant is numerically positive
/// </summary>
public override bool IsPositive
{
get { return Value != 0; }
}

/// <summary>
/// Appends the textual representation of this expression to <paramref name="builder" />.
/// </summary>
internal override void AppendString(StringBuilder builder)
{
builder.Append((uint)Value);
builder.Append('U');
}
}
}
14 changes: 12 additions & 2 deletions Source/Parser/Expressions/Trigger/MemoryValueExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ private ExpressionBase EnsureSingleExpressionOnRightHandSide(ExpressionBase righ
return new ComparisonExpression(cloneLeft, operation, constant);
}

private MemoryValueExpression InvertAndMigrateAccessorsTo(ExpressionBase target)
internal MemoryValueExpression InvertAndMigrateAccessorsTo(ExpressionBase target)
{
var value = target as MemoryValueExpression;
if (value == null)
Expand Down Expand Up @@ -1004,6 +1004,7 @@ private static ExpressionBase ApplyUnderflowAdjustment(ComparisonExpression comp
{
Debug.Assert(comparison.Left is MemoryValueExpression);
var leftMemoryValue = (MemoryValueExpression)comparison.Left;
IntegerConstantExpression integerConstant;

var newLeft = leftMemoryValue.Clone();
newLeft.IntegerConstant += underflowAdjustment;
Expand Down Expand Up @@ -1036,7 +1037,7 @@ private static ExpressionBase ApplyUnderflowAdjustment(ComparisonExpression comp
else if (underflowAdjustment < 0)
{
// attempting to clamp adjustment, see if we can clamp it even more
var integerConstant = newRight as IntegerConstantExpression;
integerConstant = newRight as IntegerConstantExpression;
if (integerConstant != null)
{
var newConstant = integerConstant.Value - newLeft.IntegerConstant;
Expand All @@ -1049,6 +1050,12 @@ private static ExpressionBase ApplyUnderflowAdjustment(ComparisonExpression comp
}
}

// if the resulting underflow comparison target is negative, explicitly make it positive
// to prevent further underflow calculations.
integerConstant = newRight as IntegerConstantExpression;
if (integerConstant != null && integerConstant.IsNegative)
newRight = new UnsignedIntegerConstantExpression((uint)integerConstant.Value);

return new ComparisonExpression(newLeft, operation, newRight);
}

Expand All @@ -1063,6 +1070,9 @@ private int GetUnderflowAdjustment(ExpressionBase right)
switch (right.Type)
{
case ExpressionType.IntegerConstant:
if (right is UnsignedIntegerConstantExpression)
return 0; // explicit unsigned in comparison, do not adjust for underflow

integerOffset = ((IntegerConstantExpression)right).Value;
break;

Expand Down
45 changes: 44 additions & 1 deletion Source/Parser/Expressions/Trigger/ModifiedMemoryAccessorExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,50 @@ public ExpressionBase NormalizeComparison(ExpressionBase right, ComparisonOperat
result = newRight.ApplyMathematic(modifier, opposingOperator);

if (result is MathematicExpression)
return new ErrorExpression("Result can never be true using integer math");
{
var zero = new UnsignedIntegerConstantExpression(0U);
var negative = new UnsignedIntegerConstantExpression(0x80000000U);

// left and right are both modified memory expressions. use subsource and check the negative bit
var memoryValue = new MemoryValueExpression(this);
memoryValue.ApplyMathematic(modifiedMemoryAccessor.Clone(), MathematicOperation.Subtract);
switch (operation)
{
case ComparisonOperation.Equal:
case ComparisonOperation.NotEqual:
// A * 3 == B * 4 => A * 3 - B * 4 == 0 => A * 3 - B * 4 == 0
// A * 3 != B * 4 => A * 3 - B * 4 != 0 => A * 3 - B * 4 != 0
return new ComparisonExpression(memoryValue, operation, zero);

case ComparisonOperation.LessThan:
case ComparisonOperation.GreaterThanOrEqual:
// A * 3 < B * 4 => A * 3 - B * 4 < 0 => A * 3 - B * 4 >= 0x80000000
// A * 3 >= B * 4 => A * 3 - B * 4 >= 0 => A * 3 - B * 4 < 0x80000000
var newOperation = ComparisonExpression.GetOppositeComparisonOperation(operation);
return new ComparisonExpression(memoryValue, newOperation, negative);

case ComparisonOperation.LessThanOrEqual:
// A * 3 <= B * 4 => A * 3 - B * 4 <= 0 => A * 3 - B * 4 >= 0x80000000 || == 0
var negativeComparison = new RequirementConditionExpression() { Left = memoryValue, Comparison = ComparisonOperation.GreaterThanOrEqual, Right = negative };
var zeroComparison = new RequirementConditionExpression() { Left = memoryValue, Comparison = ComparisonOperation.Equal, Right = zero };
var lessThanOrEqualClause = new RequirementClauseExpression() { Operation = ConditionalOperation.Or };
lessThanOrEqualClause.AddCondition(negativeComparison);
lessThanOrEqualClause.AddCondition(zeroComparison);
return lessThanOrEqualClause;

case ComparisonOperation.GreaterThan:
// A * 3 > B * 4 => A * 3 - B * 4 > 0 => A * 3 - B * 4 < 0x80000000 && != 0
var positiveComparison = new RequirementConditionExpression() { Left = memoryValue, Comparison = ComparisonOperation.LessThan, Right = negative };
var nonZeroComparison = new RequirementConditionExpression() { Left = memoryValue, Comparison = ComparisonOperation.NotEqual, Right = zero };
var greaterThanClause = new RequirementClauseExpression() { Operation = ConditionalOperation.And };
greaterThanClause.AddCondition(positiveComparison);
greaterThanClause.AddCondition(nonZeroComparison);
return greaterThanClause;

default:
return new ErrorExpression("Result can never be true using integer math");
}
}

// swap so modifier is on left
newRight = newLeft;
Expand Down
51 changes: 39 additions & 12 deletions Source/Parser/Expressions/Trigger/RequirementConditionExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ internal override void AppendString(StringBuilder builder)

// special handling: comparisons use unsigned values
var rightInteger = Right as IntegerConstantExpression;
if (rightInteger != null && rightInteger.Value < 0)
if (rightInteger != null && rightInteger.IsNegative)
builder.Append((uint)rightInteger.Value);
else
Right.AppendString(builder);
Expand Down Expand Up @@ -419,19 +419,26 @@ private static void NormalizeLimits(ref RequirementExpressionBase expression)
}
else if (value > max)
{
switch (condition.Comparison)
if (value > Int32.MaxValue && min < 0)
{
case ComparisonOperation.GreaterThan:
case ComparisonOperation.GreaterThanOrEqual:
case ComparisonOperation.Equal:
expression = new AlwaysFalseExpression();
return;
// value is implicitly negative, ignore it.
}
else
{
switch (condition.Comparison)
{
case ComparisonOperation.GreaterThan:
case ComparisonOperation.GreaterThanOrEqual:
case ComparisonOperation.Equal:
expression = new AlwaysFalseExpression();
return;

case ComparisonOperation.LessThan:
case ComparisonOperation.LessThanOrEqual:
case ComparisonOperation.NotEqual:
expression = new AlwaysTrueExpression();
return;
case ComparisonOperation.LessThan:
case ComparisonOperation.LessThanOrEqual:
case ComparisonOperation.NotEqual:
expression = new AlwaysTrueExpression();
return;
}
}
}
else if (value == max)
Expand Down Expand Up @@ -504,6 +511,26 @@ public ExpressionBase Normalize()
return reversed.Normalize();
}

var integerRight = Right as IntegerConstantExpression;
if (integerRight != null && integerRight.IsNegative && integerRight.Value > -100000)
{
var memoryValue = Left as MemoryValueExpression;
if (memoryValue != null && memoryValue.MemoryAccessors.Any(a => a.CombiningOperator == RequirementType.SubSource))
{
// A - B < -2 => B - A > 2
var newMemoryValue = new MemoryValueExpression();
newMemoryValue = memoryValue.InvertAndMigrateAccessorsTo(newMemoryValue);
var integerConstant = new IntegerConstantExpression(-integerRight.Value);
var normalized = new RequirementConditionExpression
{
Left = newMemoryValue,
Comparison = ComparisonExpression.ReverseComparisonOperation(Comparison),
Right = integerConstant
};
return normalized.Normalize();
}
}

var modifiedMemoryAccessor = Left as ModifiedMemoryAccessorExpression;
if (modifiedMemoryAccessor != null && modifiedMemoryAccessor.ModifyingOperator != RequirementOperator.None)
{
Expand Down
11 changes: 9 additions & 2 deletions Source/Parser/Internal/ExpressionBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,15 @@ private static ExpressionBase ParseNumber(PositionalTokenizer tokenizer, bool is
return new ErrorExpression("Number too large");
}

if (value > Int32.MaxValue && !isUnsigned)
return new ErrorExpression("Number too large");
if (value > Int32.MaxValue)
{
if (!isUnsigned)
return new ErrorExpression("Number too large");

var unsignedIntegerExpression = new UnsignedIntegerConstantExpression(value);
unsignedIntegerExpression.Location = new TextRange(line, column, endLine, endColumn);
return unsignedIntegerExpression;
}

var integerExpression = new IntegerConstantExpression((int)value);
integerExpression.Location = new TextRange(line, column, endLine, endColumn);
Expand Down
14 changes: 12 additions & 2 deletions Tests/Parser/Expressions/Trigger/ModifiedMemoryAcessorExpressionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ public void TestCombine(string left, string operation, string right, ExpressionT
ExpressionType.Comparison, "float(0x001234) != 3.363636")]
[TestCase("byte(0x001234) * 10", "=", "byte(0x002345) * 5",
ExpressionType.Comparison, "byte(0x001234) * 2 == byte(0x002345)")]
[TestCase("byte(0x001234) * 10", "=", "byte(0x002345) * 3",
ExpressionType.Error, "Result can never be true using integer math")]
[TestCase("byte(0x001234) * 10", "=", "byte(0x002345) / 3",
ExpressionType.Comparison, "byte(0x001234) * 30 == byte(0x002345)")]
[TestCase("byte(0x001234) / 10", "=", "byte(0x002345) * 3",
Expand All @@ -178,6 +176,18 @@ public void TestCombine(string left, string operation, string right, ExpressionT
ExpressionType.None, null)] // division cannot be moved; multiplication cannot be extracted
[TestCase("byte(0x001234) / byte(0x002345) * 10", ">", "80",
ExpressionType.Comparison, "byte(0x001234) / byte(0x002345) > 8")]
[TestCase("byte(0x001234) * 10", "=", "byte(0x002345) * 8",
ExpressionType.Comparison, "byte(0x001234) * 10 - byte(0x002345) * 8 == 0U")] // comparison of two modified memory accesors results in subsource chain
[TestCase("byte(0x001234) * 10", "!=", "byte(0x002345) * 8",
ExpressionType.Comparison, "byte(0x001234) * 10 - byte(0x002345) * 8 != 0U")] // comparison of two modified memory accesors results in subsource chain
[TestCase("byte(0x001234) * 10", "<", "byte(0x002345) * 8",
ExpressionType.Comparison, "byte(0x001234) * 10 - byte(0x002345) * 8 >= 2147483648U")] // comparison of two modified memory accesors results in subsource chain
[TestCase("byte(0x001234) * 10", "<=", "byte(0x002345) * 8",
ExpressionType.Requirement, "byte(0x001234) * 10 - byte(0x002345) * 8 >= 2147483648U || byte(0x001234) * 10 - byte(0x002345) * 8 == 0U")] // comparison of two modified memory accesors results in subsource chain
[TestCase("byte(0x001234) * 10", ">", "byte(0x002345) * 8",
ExpressionType.Requirement, "byte(0x001234) * 10 - byte(0x002345) * 8 < 2147483648U && byte(0x001234) * 10 - byte(0x002345) * 8 != 0U")] // comparison of two modified memory accesors results in subsource chain
[TestCase("byte(0x001234) * 10", ">=", "byte(0x002345) * 8",
ExpressionType.Comparison, "byte(0x001234) * 10 - byte(0x002345) * 8 < 2147483648U")] // comparison of two modified memory accesors results in subsource chain
public void TestNormalizeComparison(string left, string operation, string right, ExpressionType expectedType, string expected)
{
ExpressionTests.AssertNormalizeComparison(left, operation, right, expectedType, expected);
Expand Down
2 changes: 0 additions & 2 deletions Tests/Parser/Expressions/Trigger/RequirementClauseExpression_Tests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
using Jamiras.Components;
using NUnit.Framework;
using RATools.Data;
using RATools.Parser;
using RATools.Parser.Expressions;
using RATools.Parser.Expressions.Trigger;
using RATools.Parser.Internal;
using System.Collections.Generic;

namespace RATools.Tests.Parser.Expressions.Trigger
{
Expand Down
Loading

0 comments on commit 0eb86e2

Please sign in to comment.