Modified: trunk/Source/WTF/ChangeLog (145758 => 145759)
--- trunk/Source/WTF/ChangeLog 2013-03-13 21:50:11 UTC (rev 145758)
+++ trunk/Source/WTF/ChangeLog 2013-03-13 21:51:07 UTC (rev 145759)
@@ -1,3 +1,19 @@
+2013-03-13 Oliver Hunt <[email protected]>
+
+ Simplify Checked<> multiplication
+ https://bugs.webkit.org/show_bug.cgi?id=112286
+
+ Reviewed by James Robinson.
+
+ Trying to correctly identify multiplication by zero complicated the
+ unsigned * unsigned multiply, and still didn't handle all cases correctly.
+ Now we just do the normal division based approach to overflow detection
+ leading to much simpler reasoning.
+
+ Would be nice if we could have a jo style intrinsic one day.
+
+ * wtf/CheckedArithmetic.h:
+
2013-03-13 Dean Jackson <[email protected]>
Unreviewed attempted build fix for Windows. SchedulePair.cpp -> SchedulePairCF.cpp
Modified: trunk/Source/WTF/wtf/CheckedArithmetic.h (145758 => 145759)
--- trunk/Source/WTF/wtf/CheckedArithmetic.h 2013-03-13 21:50:11 UTC (rev 145758)
+++ trunk/Source/WTF/wtf/CheckedArithmetic.h 2013-03-13 21:51:07 UTC (rev 145759)
@@ -321,10 +321,13 @@
static inline bool multiply(LHS lhs, RHS rhs, ResultType& result) WARN_UNUSED_RETURN
{
- ResultType temp = lhs * rhs;
- if (temp < lhs)
- return !rhs;
- result = temp;
+ if (!lhs || !rhs) {
+ result = 0;
+ return true;
+ }
+ if (std::numeric_limits<ResultType>::max() / lhs < rhs)
+ return false;
+ result = lhs * rhs;
return true;
}
Modified: trunk/Tools/ChangeLog (145758 => 145759)
--- trunk/Tools/ChangeLog 2013-03-13 21:50:11 UTC (rev 145758)
+++ trunk/Tools/ChangeLog 2013-03-13 21:51:07 UTC (rev 145759)
@@ -1,3 +1,16 @@
+2013-03-13 Oliver Hunt <[email protected]>
+
+ Simplify Checked<> multiplication
+ https://bugs.webkit.org/show_bug.cgi?id=112286
+
+ Reviewed by James Robinson.
+
+ Add tests for multiplication by zero and max to ensure we don't
+ mess them up should we ever make changes to Checked<> in future.
+
+ * TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp:
+ (TestWebKitAPI):
+
2013-03-13 Simon Hausmann <[email protected]>
[Qt] Unreviewed prospective Windows build fix
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp (145758 => 145759)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp 2013-03-13 21:50:11 UTC (rev 145758)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp 2013-03-13 21:51:07 UTC (rev 145759)
@@ -70,6 +70,41 @@
EXPECT_EQ(true, (value += coerceLiteral(1)).hasOverflowed()); \
EXPECT_EQ(true, value.hasOverflowed()); \
value = 10; \
+ type _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (value * Checked<type, RecordOverflow>(0)).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (Checked<type, RecordOverflow>(0) * value).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidOverflow == (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidOverflow == (Checked<type, RecordOverflow>(std::numeric_limits<type>::max()) * value).safeGet(_value)); \
+ value = 0; \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (Checked<type, RecordOverflow>(std::numeric_limits<type>::max()) * value).safeGet(_value)); \
+ value = 1; \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (Checked<type, RecordOverflow>(std::numeric_limits<type>::max()) * value).safeGet(_value)); \
+ _value = 0; \
+ value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (Checked<type, RecordOverflow>(std::numeric_limits<type>::max()) * (type)0).safeGet(_value)); \
+ _value = 0; \
+ value = 1; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidNotOverflow == (Checked<type, RecordOverflow>(std::numeric_limits<type>::max()) * (type)1).safeGet(_value)); \
+ _value = 0; \
+ value = 2; \
+ EXPECT_EQ(true, CheckedState::DidOverflow == (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).safeGet(_value)); \
+ _value = 0; \
+ EXPECT_EQ(true, CheckedState::DidOverflow == (Checked<type, RecordOverflow>(std::numeric_limits<type>::max()) * (type)2).safeGet(_value)); \
+ value = 10; \
+ EXPECT_EQ(true, (value * Checked<type, RecordOverflow>(std::numeric_limits<type>::max())).hasOverflowed()); \
MixedSignednessTest(EXPECT_EQ(coerceLiteral(0), (value + -10).unsafeGet())); \
MixedSignednessTest(EXPECT_EQ(0U, (value - 10U).unsafeGet())); \
MixedSignednessTest(EXPECT_EQ(coerceLiteral(0), (-10 + value).unsafeGet())); \
