Title: [164500] trunk/Source/_javascript_Core
- Revision
- 164500
- Author
- mark....@apple.com
- Date
- 2014-02-21 14:27:27 -0800 (Fri, 21 Feb 2014)
Log Message
gatherFromOtherThread() needs to align the sp before gathering roots.
<https://webkit.org/b/129169>
Reviewed by Geoffrey Garen.
The GC scans the stacks of other threads using MachineThreads::gatherFromOtherThread().
gatherFromOtherThread() defines the range of the other thread's stack as
being bounded by the other thread's stack pointer and stack base. While
the stack base will always be aligned to sizeof(void*), the stack pointer
may not be. This is because the other thread may have just pushed a 32-bit
value on its stack before we suspended it for scanning.
The fix is to round the stack pointer up to the next aligned address of
sizeof(void*) and start scanning from there. On 64-bit systems, we will
effectively ignore the 32-bit word at the bottom of the stack (top of the
stack for stacks growing up) because it cannot be a 64-bit pointer anyway.
64-bit pointers should always be stored on 64-bit aligned boundaries (our
conservative scan algorithm already depends on this assumption).
On 32-bit systems, the rounding is effectively a no-op.
* heap/ConservativeRoots.cpp:
(JSC::ConservativeRoots::genericAddSpan):
- Hardened somne assertions so that we can catch misalignment issues on
release builds as well.
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::gatherFromOtherThread):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (164499 => 164500)
--- trunk/Source/_javascript_Core/ChangeLog 2014-02-21 22:26:37 UTC (rev 164499)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-02-21 22:27:27 UTC (rev 164500)
@@ -1,3 +1,33 @@
+2014-02-21 Mark Lam <mark....@apple.com>
+
+ gatherFromOtherThread() needs to align the sp before gathering roots.
+ <https://webkit.org/b/129169>
+
+ Reviewed by Geoffrey Garen.
+
+ The GC scans the stacks of other threads using MachineThreads::gatherFromOtherThread().
+ gatherFromOtherThread() defines the range of the other thread's stack as
+ being bounded by the other thread's stack pointer and stack base. While
+ the stack base will always be aligned to sizeof(void*), the stack pointer
+ may not be. This is because the other thread may have just pushed a 32-bit
+ value on its stack before we suspended it for scanning.
+
+ The fix is to round the stack pointer up to the next aligned address of
+ sizeof(void*) and start scanning from there. On 64-bit systems, we will
+ effectively ignore the 32-bit word at the bottom of the stack (top of the
+ stack for stacks growing up) because it cannot be a 64-bit pointer anyway.
+ 64-bit pointers should always be stored on 64-bit aligned boundaries (our
+ conservative scan algorithm already depends on this assumption).
+
+ On 32-bit systems, the rounding is effectively a no-op.
+
+ * heap/ConservativeRoots.cpp:
+ (JSC::ConservativeRoots::genericAddSpan):
+ - Hardened somne assertions so that we can catch misalignment issues on
+ release builds as well.
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::gatherFromOtherThread):
+
2014-02-21 Matthew Mirman <mmir...@apple.com>
Added a GetMyArgumentsLengthSafe and added a speculation check.
Modified: trunk/Source/_javascript_Core/heap/ConservativeRoots.cpp (164499 => 164500)
--- trunk/Source/_javascript_Core/heap/ConservativeRoots.cpp 2014-02-21 22:26:37 UTC (rev 164499)
+++ trunk/Source/_javascript_Core/heap/ConservativeRoots.cpp 2014-02-21 22:27:27 UTC (rev 164500)
@@ -100,8 +100,8 @@
end = swapTemp;
}
- ASSERT(isPointerAligned(begin));
- ASSERT(isPointerAligned(end));
+ RELEASE_ASSERT(isPointerAligned(begin));
+ RELEASE_ASSERT(isPointerAligned(end));
TinyBloomFilter filter = m_blocks->filter(); // Make a local copy of filter to show the compiler it won't alias, and can be register-allocated.
for (char** it = static_cast<char**>(begin); it != static_cast<char**>(end); ++it)
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (164499 => 164500)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2014-02-21 22:26:37 UTC (rev 164499)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2014-02-21 22:27:27 UTC (rev 164500)
@@ -445,6 +445,7 @@
void* stackPointer = otherThreadStackPointer(regs);
void* stackBase = thread->stackBase;
swapIfBackwards(stackPointer, stackBase);
+ stackPointer = reinterpret_cast<void*>(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(stackPointer)));
conservativeRoots.add(stackPointer, stackBase, jitStubRoutines, codeBlocks);
freePlatformThreadRegisters(regs);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
