Attachment 225659 Details for Bug 129429 – Patch

[patch] Patch

bug-129429-20140303180410.patch (text/plain), 13.50 KB, created by peavo on 2014-03-03 09:04:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: peavo

Created: 2014-03-03 09:04:19 PST

Size: 13.50 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 164985)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,30 @@
>+2014-03-03  peavo@outlook.com  <peavo@outlook.com>
>+
>+        [Win32][LLINT] Crash when running JSC stress tests.
>+        https://bugs.webkit.org/show_bug.cgi?id=129429
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        On Windows we need to check if the stack pointer moves over the stack guard page of size 4K.
>+        If so, we need to read or write some data on every following guard page
>+        to make sure the system commits the previously uncommitted stack memory.
>+        Otherwise we might crash later if we access uncommitted stack memory.
>+
>+        * assembler/MacroAssembler.h: Added subPtr method for destination of type Address.
>+        * JavaScriptCore.vcxproj/LLInt/LLIntAssembly/build-LLIntAssembly.sh: Enable LLINT.
>+        * jit/AssemblyHelpers.cpp:
>+        (JSC::AssemblyHelpers::moveStackGuardPage): Added method to move stack guard page.
>+        * jit/AssemblyHelpers.h:
>+        (JSC::AssemblyHelpers::moveStackPointer): Added method to move stack pointer.
>+        * jit/JIT.cpp:
>+        (JSC::JIT::privateCompile): Use moveStackPointer method, which will move the guard page.
>+        * jit/JITCall32_64.cpp:
>+        (JSC::JIT::compileLoadVarargs): Ditto.
>+        * jit/Repatch.cpp:
>+        (JSC::writeBarrier): Compile fix when DFG_JIT is not enabled.
>+        * llint/LowLevelInterpreter.asm: Added macro to move stack guard page.
>+        * offlineasm/x86.rb: Compile fix, and small simplification.
>+
> 2014-03-02  Filip Pizlo  <fpizlo@apple.com>
> 
>         PolymorphicPutByIdList should have a simpler construction API with basically a single entrypoint
>Index: Source/JavaScriptCore/JavaScriptCore.vcxproj/LLInt/LLIntAssembly/build-LLIntAssembly.sh
>===================================================================
>--- Source/JavaScriptCore/JavaScriptCore.vcxproj/LLInt/LLIntAssembly/build-LLIntAssembly.sh	(revision 164978)
>+++ Source/JavaScriptCore/JavaScriptCore.vcxproj/LLInt/LLIntAssembly/build-LLIntAssembly.sh	(working copy)
>@@ -29,4 +29,4 @@ printf "END" > LowLevelInterpreterWin.as
> 
> # When enabling LLINT and switching to the x86 backend, use "LowLevelInterpreterWin.asm" as output file when running asm.rb.
> 
>-/usr/bin/env ruby "${SRCROOT}/offlineasm/asm.rb" "${SRCROOT}/llint/LowLevelInterpreter.asm" "${BUILT_PRODUCTS_DIR}/LLIntOffsetsExtractor/LLIntOffsetsExtractor${3}.exe" "LLIntAssembly.h" || exit 1
>+/usr/bin/env ruby "${SRCROOT}/offlineasm/asm.rb" "${SRCROOT}/llint/LowLevelInterpreter.asm" "${BUILT_PRODUCTS_DIR}/LLIntOffsetsExtractor/LLIntOffsetsExtractor${3}.exe" "LowLevelInterpreterWin.asm" || exit 1
>Index: Source/JavaScriptCore/assembler/MacroAssembler.h
>===================================================================
>--- Source/JavaScriptCore/assembler/MacroAssembler.h	(revision 164978)
>+++ Source/JavaScriptCore/assembler/MacroAssembler.h	(working copy)
>@@ -533,6 +533,11 @@ public:
>         sub32(TrustedImm32(imm), dest);
>     }
> 
>+    void subPtr(TrustedImm32 imm, Address dest)
>+    {
>+        sub32(imm, dest);
>+    }
>+
>     void xorPtr(RegisterID src, RegisterID dest)
>     {
>         xor32(src, dest);
>@@ -821,6 +826,11 @@ public:
>         sub64(TrustedImm64(imm), dest);
>     }
> 
>+    void subPtr(TrustedImm32 imm, Address dest)
>+    {
>+        sub64(imm, dest);
>+    }
>+
>     void xorPtr(RegisterID src, RegisterID dest)
>     {
>         xor64(src, dest);
>Index: Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/AssemblyHelpers.cpp	(revision 164978)
>+++ Source/JavaScriptCore/jit/AssemblyHelpers.cpp	(working copy)
>@@ -70,6 +70,65 @@ void AssemblyHelpers::clearSamplingFlag(
> }
> #endif
> 
>+#if PLATFORM(WIN)
>+    // On Windows we need to check if the stack pointer moves over the stack guard page of size 4K.
>+    // If so, we need to read or write some data on every following guard page
>+    // to make sure the system commits the previously uncommitted stack memory.
>+    // Otherwise we might crash later if we access uncommitted stack memory.
>+    // See http://support.microsoft.com/kb/100775
>+
>+static int getGuardPageSize()
>+{
>+    const int defaultPageSize = 4096;
>+
>+    MEMORY_BASIC_INFORMATION stackOrigin;
>+    if (!VirtualQuery(&stackOrigin, &stackOrigin, sizeof(stackOrigin)))
>+        return defaultPageSize;
>+
>+    MEMORY_BASIC_INFORMATION uncommittedMemory;
>+    if (!VirtualQuery(stackOrigin.AllocationBase, &uncommittedMemory, sizeof(uncommittedMemory)))
>+        return defaultPageSize;
>+
>+    ASSERT(uncommittedMemory.State == MEM_RESERVE);
>+
>+    MEMORY_BASIC_INFORMATION guardPage;
>+    if (!VirtualQuery(static_cast<char*>(uncommittedMemory.BaseAddress) + uncommittedMemory.RegionSize, &guardPage, sizeof(guardPage)))
>+        return defaultPageSize;
>+
>+    ASSERT(guardPage.Protect & PAGE_GUARD);
>+
>+    return guardPage.RegionSize;
>+}
>+
>+void AssemblyHelpers::moveStackPointerAndGuardPage(int offset, RegisterID base, RegisterID scratch)
>+{
>+    static int guardPageSize = getGuardPageSize();
>+    const TrustedImm32 pageSize(guardPageSize);
>+
>+    // Align stackpointer to page boundary.
>+    andPtr(TrustedImm32(~(pageSize.m_value - 1)), stackPointerRegister);
>+
>+    move(TrustedImm32(-offset), scratch);
>+
>+    // scratch now contains the offset from base reg
>+
>+    subPtr(base, scratch);
>+    addPtr(stackPointerRegister, scratch);
>+
>+    // Now, scratch contains the allocation offset from the current stackpointer.
>+
>+    Label updateSpLoop(this);
>+    Jump almostDoneUpdateSp = branch32(LessThan, scratch, pageSize);
>+    subPtr(pageSize, scratch);
>+    subPtr(pageSize, stackPointerRegister);
>+    // Dummy operation to make sure the system commits memory, and moves the guard page.
>+    subPtr(TrustedImm32(0), Address(stackPointerRegister));
>+    jump(updateSpLoop);
>+    almostDoneUpdateSp.link(this);
>+    subPtr(scratch, stackPointerRegister);
>+}
>+#endif
>+
> #if !ASSERT_DISABLED
> #if USE(JSVALUE64)
> void AssemblyHelpers::jitAssertIsInt32(GPRReg gpr)
>Index: Source/JavaScriptCore/jit/AssemblyHelpers.h
>===================================================================
>--- Source/JavaScriptCore/jit/AssemblyHelpers.h	(revision 164978)
>+++ Source/JavaScriptCore/jit/AssemblyHelpers.h	(working copy)
>@@ -67,6 +67,20 @@ public:
> #endif
>     }
> 
>+#if PLATFORM(WIN)
>+    void moveStackPointerAndGuardPage(int offset, RegisterID base, RegisterID scratch);
>+#endif
>+
>+    ALWAYS_INLINE void moveStackPointer(int offset, RegisterID base, RegisterID scratch)
>+    {
>+#if PLATFORM(WIN)
>+        moveStackPointerAndGuardPage(offset, base, scratch);
>+#else
>+        UNUSED_PARAM(scratch);
>+        addPtr(TrustedImm32(offset), base, stackPointerRegister);
>+#endif
>+    }
>+
> #if CPU(X86_64) || CPU(X86)
>     size_t prologueStackPointerDelta()
>     {
>Index: Source/JavaScriptCore/jit/JIT.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JIT.cpp	(revision 164978)
>+++ Source/JavaScriptCore/jit/JIT.cpp	(working copy)
>@@ -522,7 +522,7 @@ CompilationResult JIT::privateCompile(JI
>         stackOverflow = branchPtr(Above, AbsoluteAddress(m_vm->addressOfStackLimit()), regT1);
>     }
> 
>-    addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
>+    moveStackPointer(stackPointerOffsetFor(m_codeBlock) * sizeof(Register), callFrameRegister, regT2);
>     checkStackPointerAlignment();
> 
>     privateCompileMainPass();
>Index: Source/JavaScriptCore/jit/JITCall32_64.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JITCall32_64.cpp	(revision 164978)
>+++ Source/JavaScriptCore/jit/JITCall32_64.cpp	(working copy)
>@@ -194,7 +194,7 @@ void JIT::compileLoadVarargs(Instruction
> 
>     emitLoad(arguments, regT1, regT0);
>     callOperation(operationSizeFrameForVarargs, regT1, regT0, firstFreeRegister, firstVarArgOffset);
>-    addPtr(TrustedImm32(-sizeof(CallerFrameAndPC)), returnValueGPR, stackPointerRegister);
>+    moveStackPointer(-sizeof(CallerFrameAndPC), returnValueGPR, regT4);
>     emitLoad(thisValue, regT1, regT4);
>     emitLoad(arguments, regT3, regT2);
>     callOperation(operationLoadVarargs, returnValueGPR, regT1, regT4, regT3, regT2, firstVarArgOffset);
>Index: Source/JavaScriptCore/jit/Repatch.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/Repatch.cpp	(revision 164978)
>+++ Source/JavaScriptCore/jit/Repatch.cpp	(working copy)
>@@ -824,9 +824,13 @@ static MacroAssembler::Call writeBarrier
>     ASSERT(owner != scratch1);
>     ASSERT(owner != scratch2);
> 
>+#if ENABLE(DFG_JIT)
>     MacroAssembler::Jump definitelyNotMarked = DFG::SpeculativeJIT::genericWriteBarrier(jit, owner);
>+#endif
>     MacroAssembler::Call call = storeToWriteBarrierBuffer(jit, owner, scratch1, scratch2, allocator);
>+#if ENABLE(DFG_JIT)
>     definitelyNotMarked.link(&jit);
>+#endif
>     return call;
> }
> #endif // ENABLE(GGC)
>Index: Source/JavaScriptCore/llint/LowLevelInterpreter.asm
>===================================================================
>--- Source/JavaScriptCore/llint/LowLevelInterpreter.asm	(revision 164978)
>+++ Source/JavaScriptCore/llint/LowLevelInterpreter.asm	(working copy)
>@@ -383,6 +383,36 @@ macro callToJavaScriptEpilogue()
>     end
> end
> 
>+# On Windows we need to check if the stack pointer moves over the stack guard page of size 4K.
>+# If so, we need to read or write some data on every following guard page
>+# to make sure the system commits the previously uncommitted stack memory.
>+# Otherwise we might crash later if we access uncommitted stack memory.
>+# See http://support.microsoft.com/kb/100775
>+
>+macro moveStackPointerAndGuardPage(base, scratch)
>+    const pageSize = 4096
>+
>+    # Align stackpointer to page boundary.
>+    andp ~(pageSize - 1), sp
>+
>+    # On entry, scratch contains the offset from base reg
>+
>+    subp base, scratch
>+    addp sp, scratch
>+
>+    # Now, scratch contains the allocation offset from the current stackpointer.
>+
>+.updateSpLoop:
>+    bilt scratch, pageSize, .almostDoneUpdateSp
>+    subp pageSize, scratch
>+    subp pageSize, sp
>+    # Dummy operation to make sure the system commits memory, and moves the guard page.
>+    subp 0, [sp]
>+    jmp .updateSpLoop
>+.almostDoneUpdateSp:
>+    subp scratch, sp
>+end
>+
> macro moveStackPointerForCodeBlock(codeBlock, scratch)
>     loadi CodeBlock::m_numCalleeRegisters[codeBlock], scratch
>     lshiftp 3, scratch
>@@ -390,6 +420,8 @@ macro moveStackPointerForCodeBlock(codeB
>     if ARMv7
>         subp cfr, scratch, scratch
>         move scratch, sp
>+    elsif X86_WIN
>+        moveStackPointerAndGuardPage(cfr, scratch)
>     else
>         subp cfr, scratch, sp
>     end
>@@ -992,6 +1024,9 @@ _llint_op_call_varargs:
>         if ARMv7
>             subp t1, CallerFrameAndPCSize, t2
>             move t2, sp
>+        elsif X86_WIN
>+            move CallerFrameAndPCSize, t2
>+            moveStackPointerAndGuardPage(t1, t2)
>         else
>             subp t1, CallerFrameAndPCSize, sp
>         end
>Index: Source/JavaScriptCore/offlineasm/x86.rb
>===================================================================
>--- Source/JavaScriptCore/offlineasm/x86.rb	(revision 164978)
>+++ Source/JavaScriptCore/offlineasm/x86.rb	(working copy)
>@@ -414,7 +414,7 @@ class BaseIndex
>     end
>     
>     def x86Operand(kind)
>-        if !isIntelSyntax || kind != :double
>+        if !isIntelSyntax
>             x86AddressOperand(:ptr)
>         else
>             "#{getSizeString(kind)}[#{offset.value} + #{base.x86Operand(:ptr)} + #{index.x86Operand(:ptr)} * #{scale}]"
>@@ -1326,7 +1326,7 @@ class Instruction
>                 }
>             end
>             op = operands[0].x86CallOperand(:ptr)
>-            if isMSVC && (/\Allint_/.match(op) || /\Aslow_path/.match(op))
>+            if isMSVC && (operands[0].is_a? LabelReference)
>                 writeSymbolToFile(op)
>             end
>             $asm.puts "call #{op}"
>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 164978)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,12 @@
>+2014-03-03  peavo@outlook.com  <peavo@outlook.com>
>+
>+        [Win32][LLINT] Crash when running JSC stress tests.
>+        https://bugs.webkit.org/show_bug.cgi?id=129429
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/Platform.h: Enable LLINT on Win32.
>+
> 2014-03-02  Jaehun Lim  <ljaehun.lim@samsung.com>
> 
>         [EFL] Remove m_initEfl from RunLoop.
>Index: Source/WTF/wtf/Platform.h
>===================================================================
>--- Source/WTF/wtf/Platform.h	(revision 164978)
>+++ Source/WTF/wtf/Platform.h	(working copy)
>@@ -640,7 +640,7 @@
> #if !defined(ENABLE_JIT) \
>     && (CPU(X86) || CPU(X86_64) || CPU(ARM) || CPU(ARM64) || CPU(MIPS)) \
>     && !OS(WINCE) \
>-    && !OS(WINDOWS)
>+    && !(OS(WINDOWS) && CPU(X86_64))
> #define ENABLE_JIT 1
> #endif
> 
>@@ -693,8 +693,8 @@
>    low-level interpreter. */
> #if !defined(ENABLE_LLINT) \
>     && ENABLE(JIT) \
>-    && (OS(DARWIN) || OS(LINUX) || OS(FREEBSD)) \
>-    && ((OS(DARWIN) && !PLATFORM(EFL)) || PLATFORM(GTK)) \
>+    && (OS(DARWIN) || OS(LINUX) || OS(FREEBSD) || OS(WINDOWS)) \
>+    && ((OS(DARWIN) && !PLATFORM(EFL)) || PLATFORM(GTK) || PLATFORM(WIN)) \
>     && (CPU(X86) || CPU(X86_64) || CPU(ARM_THUMB2) || CPU(ARM_TRADITIONAL) || CPU(ARM64) || CPU(MIPS) || CPU(SH4))
> #define ENABLE_LLINT 1
> #endif
>@@ -783,7 +783,7 @@
> #endif
> 
> /* Configure the interpreter */
>-#if COMPILER(GCC)
>+#if COMPILER(GCC) || COMPILER(MSVC)
> #define HAVE_COMPUTED_GOTO 1
> #endif
>

Actions: View | Formatted Diff | Diff

Attachments on bug 129429: 225378 | 225383 | 225659 | 225664 | 225791 | 225804 | 225818 | 225899