Fix handling integer overflow in constant folding
Integer operations that overflow are defined to wrap in the ESSL
3.00.6 spec. Constant folding that happens inside the shader
translator should also follow the wrapping rules.
The new implementations of wrapping integer addition and subtraction
use unsigned integers to perform calculations. Unsigned integers are
defined to implement arithmetic in modulo 2^n in the C++ spec. This
behavior is also leveraged to implement wrapping unsigned integer
multiplication.
The implementation of wrapping signed integer multiplication is
slightly trickier. The operands are casted to a wider type to perform
the multiplication in a way that doesn't overflow, and then the result
is truncated and casted back to the narrower integer type.
Incorrect tests that expected errors to be generated from integer
overflow in constant folding are removed.
BUG=chromium:637050
TEST=angle_unittests
Change-Id: I0de7e25881d254803455fbf22907c192f49d09ff
Reviewed-on: https://chromium-review.googlesource.com/390252
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/ConstantUnion.cpp b/src/compiler/translator/ConstantUnion.cpp
index d2cd422..90a0042 100644
--- a/src/compiler/translator/ConstantUnion.cpp
+++ b/src/compiler/translator/ConstantUnion.cpp
@@ -61,6 +61,38 @@
return result.ValueOrDefault(0);
}
+// Unsigned types are defined to do arithmetic modulo 2^n in C++. For signed types, overflow
+// behavior is undefined.
+
+template <typename T>
+T WrappingSum(T lhs, T rhs)
+{
+ uint32_t lhsUnsigned = static_cast<uint32_t>(lhs);
+ uint32_t rhsUnsigned = static_cast<uint32_t>(rhs);
+ return static_cast<T>(lhsUnsigned + rhsUnsigned);
+}
+
+template <typename T>
+T WrappingDiff(T lhs, T rhs)
+{
+ uint32_t lhsUnsigned = static_cast<uint32_t>(lhs);
+ uint32_t rhsUnsigned = static_cast<uint32_t>(rhs);
+ return static_cast<T>(lhsUnsigned - rhsUnsigned);
+}
+
+int32_t WrappingMul(int32_t lhs, int32_t rhs)
+{
+ int64_t lhsWide = static_cast<int64_t>(lhs);
+ int64_t rhsWide = static_cast<int64_t>(rhs);
+ // The multiplication is guaranteed not to overflow.
+ int64_t resultWide = lhsWide * rhsWide;
+ // Implement the desired wrapping behavior by masking out the high-order 32 bits.
+ resultWide = resultWide & 0xffffffffll;
+ // Casting to a narrower signed type is fine since the casted value is representable in the
+ // narrower type.
+ return static_cast<int32_t>(resultWide);
+}
+
} // anonymous namespace
TConstantUnion::TConstantUnion()
@@ -283,10 +315,10 @@
switch (lhs.type)
{
case EbtInt:
- returnValue.setIConst(CheckedSum<int>(lhs.iConst, rhs.iConst, diag, line));
+ returnValue.setIConst(WrappingSum<int>(lhs.iConst, rhs.iConst));
break;
case EbtUInt:
- returnValue.setUConst(CheckedSum<unsigned int>(lhs.uConst, rhs.uConst, diag, line));
+ returnValue.setUConst(WrappingSum<unsigned int>(lhs.uConst, rhs.uConst));
break;
case EbtFloat:
returnValue.setFConst(CheckedSum<float>(lhs.fConst, rhs.fConst, diag, line));
@@ -309,10 +341,10 @@
switch (lhs.type)
{
case EbtInt:
- returnValue.setIConst(CheckedDiff<int>(lhs.iConst, rhs.iConst, diag, line));
+ returnValue.setIConst(WrappingDiff<int>(lhs.iConst, rhs.iConst));
break;
case EbtUInt:
- returnValue.setUConst(CheckedDiff<unsigned int>(lhs.uConst, rhs.uConst, diag, line));
+ returnValue.setUConst(WrappingDiff<unsigned int>(lhs.uConst, rhs.uConst));
break;
case EbtFloat:
returnValue.setFConst(CheckedDiff<float>(lhs.fConst, rhs.fConst, diag, line));
@@ -335,10 +367,12 @@
switch (lhs.type)
{
case EbtInt:
- returnValue.setIConst(CheckedMul<int>(lhs.iConst, rhs.iConst, diag, line));
+ returnValue.setIConst(WrappingMul(lhs.iConst, rhs.iConst));
break;
case EbtUInt:
- returnValue.setUConst(CheckedMul<unsigned int>(lhs.uConst, rhs.uConst, diag, line));
+ // Unsigned integer math in C++ is defined to be done in modulo 2^n, so we rely on that
+ // to implement wrapping multiplication.
+ returnValue.setUConst(lhs.uConst * rhs.uConst);
break;
case EbtFloat:
returnValue.setFConst(CheckedMul<float>(lhs.fConst, rhs.fConst, diag, line));