Title: [160063] trunk/Source
Revision
160063
Author
mark....@apple.com
Date
2013-12-03 18:19:09 -0800 (Tue, 03 Dec 2013)

Log Message

testapi test crashes on Windows in WTF::Vector<wchar_t,64,WTF::UnsafeVectorOverflow>::size().
https://bugs.webkit.org/show_bug.cgi?id=121972.

Reviewed by Brent Fulgham.

Source/_javascript_Core: 

* interpreter/JSStack.cpp:
(JSC::JSStack::~JSStack):
- Reverting the change from r160004 since it's better to fix OSAllocatorWin
  to be consistent with OSAllocatorPosix.

Source/WTF: 

* wtf/OSAllocatorWin.cpp:
(WTF::OSAllocator::decommit):
(WTF::OSAllocator::releaseDecommitted):
- Added a check to ensure that the bytes to decommit / release is not 0.
  On Windows, a 0 length passed to VirtualFree() has a special meaning,
  and it's not "decommit / release nothing" as one would expect. Adding
  this check makes OSAllocatorWin consistent with OSAllocatorPosix for
  these 2 functions.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (160062 => 160063)


--- trunk/Source/_javascript_Core/ChangeLog	2013-12-04 01:18:19 UTC (rev 160062)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-12-04 02:19:09 UTC (rev 160063)
@@ -1,5 +1,17 @@
 2013-12-03  Mark Lam  <mark....@apple.com>
 
+        testapi test crashes on Windows in WTF::Vector<wchar_t,64,WTF::UnsafeVectorOverflow>::size().
+        https://bugs.webkit.org/show_bug.cgi?id=121972.
+
+        Reviewed by Brent Fulgham.
+
+        * interpreter/JSStack.cpp:
+        (JSC::JSStack::~JSStack):
+        - Reverting the change from r160004 since it's better to fix OSAllocatorWin
+          to be consistent with OSAllocatorPosix.
+
+2013-12-03  Mark Lam  <mark....@apple.com>
+
         Fix LLINT_C_LOOP build for Win64.
         https://bugs.webkit.org/show_bug.cgi?id=125186.
 

Modified: trunk/Source/_javascript_Core/interpreter/JSStack.cpp (160062 => 160063)


--- trunk/Source/_javascript_Core/interpreter/JSStack.cpp	2013-12-04 01:18:19 UTC (rev 160062)
+++ trunk/Source/_javascript_Core/interpreter/JSStack.cpp	2013-12-04 02:19:09 UTC (rev 160063)
@@ -63,10 +63,8 @@
 JSStack::~JSStack()
 {
     void* highAddress = reinterpret_cast<void*>(static_cast<char*>(m_reservation.base()) + m_reservation.size());
-    if (highAddress > m_commitEnd) {
-        m_reservation.decommit(reinterpret_cast<void*>(m_commitEnd), reinterpret_cast<intptr_t>(highAddress) - reinterpret_cast<intptr_t>(m_commitEnd));
-        addToCommittedByteCount(-(reinterpret_cast<intptr_t>(highAddress) - reinterpret_cast<intptr_t>(m_commitEnd)));
-    }
+    m_reservation.decommit(reinterpret_cast<void*>(m_commitEnd), reinterpret_cast<intptr_t>(highAddress) - reinterpret_cast<intptr_t>(m_commitEnd));
+    addToCommittedByteCount(-(reinterpret_cast<intptr_t>(highAddress) - reinterpret_cast<intptr_t>(m_commitEnd)));
     m_reservation.deallocate();
 }
 

Modified: trunk/Source/WTF/ChangeLog (160062 => 160063)


--- trunk/Source/WTF/ChangeLog	2013-12-04 01:18:19 UTC (rev 160062)
+++ trunk/Source/WTF/ChangeLog	2013-12-04 02:19:09 UTC (rev 160063)
@@ -1,3 +1,19 @@
+2013-12-03  Mark Lam  <mark....@apple.com>
+
+        testapi test crashes on Windows in WTF::Vector<wchar_t,64,WTF::UnsafeVectorOverflow>::size().
+        https://bugs.webkit.org/show_bug.cgi?id=121972.
+
+        Reviewed by Brent Fulgham.
+
+        * wtf/OSAllocatorWin.cpp:
+        (WTF::OSAllocator::decommit):
+        (WTF::OSAllocator::releaseDecommitted):
+        - Added a check to ensure that the bytes to decommit / release is not 0.
+          On Windows, a 0 length passed to VirtualFree() has a special meaning,
+          and it's not "decommit / release nothing" as one would expect. Adding
+          this check makes OSAllocatorWin consistent with OSAllocatorPosix for
+          these 2 functions.
+
 2013-12-02  Mark Lam  <mark....@apple.com>
 
         Build failure when disabling JIT, YARR_JIT, and ASSEMBLER.

Modified: trunk/Source/WTF/wtf/OSAllocatorWin.cpp (160062 => 160063)


--- trunk/Source/WTF/wtf/OSAllocatorWin.cpp	2013-12-04 01:18:19 UTC (rev 160062)
+++ trunk/Source/WTF/wtf/OSAllocatorWin.cpp	2013-12-04 02:19:09 UTC (rev 160063)
@@ -65,6 +65,14 @@
 
 void OSAllocator::decommit(void* address, size_t bytes)
 {
+    // According to http://msdn.microsoft.com/en-us/library/aa366892(VS.85).aspx,
+    // bytes (i.e. dwSize) being 0 when dwFreeType is MEM_DECOMMIT means that we'll
+    // decommit the entire region allocated by VirtualAlloc() instead of decommitting
+    // nothing as we would expect. Hence, we should check if bytes is 0 and handle it
+    // appropriately before calling VirtualFree().
+    // See: https://bugs.webkit.org/show_bug.cgi?id=121972.
+    if (!bytes)
+        return;
     bool result = VirtualFree(address, bytes, MEM_DECOMMIT);
     if (!result)
         CRASH();
@@ -72,6 +80,10 @@
 
 void OSAllocator::releaseDecommitted(void* address, size_t bytes)
 {
+    // See comment in OSAllocator::decommit(). Similarly, when bytes is 0, we
+    // don't want to release anything. So, don't call VirtualFree() below.
+    if (!bytes)
+        return;
     // According to http://msdn.microsoft.com/en-us/library/aa366892(VS.85).aspx,
     // dwSize must be 0 if dwFreeType is MEM_RELEASE.
     bool result = VirtualFree(address, 0, MEM_RELEASE);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to