DFG::IntegerCheckCombiningPhase's wrap-around check shouldn't trigger C++ undef behavior on wrap-around

Filip Pizlo · Filip Pizlo · commit 1bdfa970dbce · 2015-04-08T20:22:53.000Z
https://bugs.webkit.org/show_bug.cgi?id=143532 Reviewed by Gavin Barraclough. Oh the irony! We were protecting an optimization that only worked if there was no wrap-around in JavaScript. But the C++ code had wrap-around, which is undef in C++. So, if the compiler was smart enough, our compiler would think that there never was wrap-around. This fixes a failure in stress/tricky-array-boiunds-checks.js when JSC is compiled with bleeding-edge clang. * dfg/DFGIntegerCheckCombiningPhase.cpp: (JSC::DFG::IntegerCheckCombiningPhase::isValid): Canonical link: https://commits.webkit.org/161560@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182562 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Makefile.shared b/Makefile.shared
@@ -12,6 +12,8 @@ ifneq (,$(ARCHS))
 	XCODE_OPTIONS += ONLY_ACTIVE_ARCH=NO
 endif
 
+XCODE_OPTIONS += TOOLCHAINS=com.apple.dt.toolchain.OSX10_11
+
 DEFAULT_VERBOSITY := $(shell defaults read org.webkit.BuildConfiguration BuildTranscriptVerbosity 2>/dev/null || echo "default")
 VERBOSITY ?= $(DEFAULT_VERBOSITY)
 
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2015-04-08  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG::IntegerCheckCombiningPhase's wrap-around check shouldn't trigger C++ undef behavior on wrap-around
+        https://bugs.webkit.org/show_bug.cgi?id=143532
+
+        Reviewed by Gavin Barraclough.
+        
+        Oh the irony!  We were protecting an optimization that only worked if there was no wrap-around in JavaScript.
+        But the C++ code had wrap-around, which is undef in C++.  So, if the compiler was smart enough, our compiler
+        would think that there never was wrap-around.
+        
+        This fixes a failure in stress/tricky-array-boiunds-checks.js when JSC is compiled with bleeding-edge clang.
+
+        * dfg/DFGIntegerCheckCombiningPhase.cpp:
+        (JSC::DFG::IntegerCheckCombiningPhase::isValid):
+
 2015-04-07  Michael Saboff  <msaboff@apple.com>
 
         Lazily initialize LogToSystemConsole flag to reduce memory usage
diff --git a/Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp b/Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp
@@ -355,8 +355,17 @@ class IntegerCheckCombiningPhase : public Phase {
             return false;
         
         switch (key.m_kind) {
-        case ArrayBounds:
-            return (range.m_maxBound - range.m_minBound) >= 0;
+        case ArrayBounds: {
+            // Have to do this carefully because C++ compilers are too smart. But all we're really doing is detecting if
+            // the difference between the bounds is 2^31 or more. If it was, then we'd have to worry about wrap-around.
+            // The way we'd like to write this expression is (range.m_maxBound - range.m_minBound) >= 0, but that is a
+            // signed subtraction and compare, which allows the C++ compiler to do anything it wants in case of
+            // wrap-around.
+            uint32_t maxBound = range.m_maxBound;
+            uint32_t minBound = range.m_minBound;
+            uint32_t unsignedDifference = maxBound - minBound;
+            return !(unsignedDifference >> 31);
+        }
             
         default:
             return true;
