Attachment 339959 Details for Bug 185421 – Patch

[patch] Patch

bug-185421-20180509220159.patch (text/plain), 11.53 KB, created by Yusuke Suzuki on 2018-05-09 06:02:00 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-09 06:02:00 PDT

Size: 11.53 KB

patch

obsolete

>Subversion Revision: 231553
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index b129aaa3952ff5a8b321528351d057fa5e2ed885..a684a752d0badd3861aae32fb6ed2ad401b1f754 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-05-09  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [Win] Enable poisoning in 64bit Windows
>+        https://bugs.webkit.org/show_bug.cgi?id=185421
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch enables poisoning on 64bit Windows.
>+
>+        * jit/ThunkGenerators.cpp:
>+        (JSC::nativeForGenerator):
>+        X86_64 Windows version does not consider about poisoning.
>+        We merge the code for X86_64 to fix this and simplify the code.
>+
>+        * llint/LowLevelInterpreter64.asm:
>+        When we emit `mov rax, g_XXXPoison`, the value of `g_XXXPoison` is directly moved to `rax`.
>+        This is different from the other platforms: they get a pointer to `g_XXXPoison` first.
>+        Currently, we emit a bit suboptimal code like the below.
>+
>+            mov rax, g_XXXPoison
>+            mov rax, rax
>+
>+        In favor of simplicity of the offlineasm backend implementation.
>+
>+        * offlineasm/ast.rb:
>+        * offlineasm/settings.rb:
>+        Referenced labels now have types. For defined external functions, use "near".
>+        For global variables, use appropriate types like "dward", "qword" and so on.
>+
>+        * offlineasm/x86.rb:
>+
> 2018-05-06  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [JSC][GTK][JSCONLY] Use capstone disassembler
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 6dd70e202d40fbc6ad8c9549113dbc83bc9c136c..a5188742b0bbec30f2ae7d95ba03e03d59ce68b5 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-05-09  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [Win] Enable poisoning in 64bit Windows
>+        https://bugs.webkit.org/show_bug.cgi?id=185421
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/Platform.h:
>+
> 2018-05-06  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [JSC][GTK][JSCONLY] Use capstone disassembler
>diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.cpp b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>index 8832065bc2f515f2316d99ae73f3a9d24b2e13f4..30b4c1b38a2b62e0ce45796d70ed6f98466c1ac9 100644
>--- a/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>+++ b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>@@ -303,39 +303,32 @@ static MacroAssemblerCodeRef<JITThunkPtrTag> nativeForGenerator(VM* vm, ThunkFun
>     jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::stackPointerRegister);
> 
> #elif CPU(X86_64)
>-#if !OS(WINDOWS)
>-    // Calling convention:      f(edi, esi, edx, ecx, ...);
>+    // X86_64 Calling convention: f(ecx, edx, r8, r9, ...);
>+    // Win64  Calling convention: f(edi, esi, edx, ecx, ...);
>     // Host function signature: f(ExecState*);
>-    jit.move(JSInterfaceJIT::callFrameRegister, X86Registers::edi);
>-
>-    jit.emitGetFromCallFrameHeaderPtr(CallFrameSlot::callee, X86Registers::esi);
>-    if (thunkFunctionType == ThunkFunctionType::JSFunction) {
>-        jit.loadPtr(JSInterfaceJIT::Address(X86Registers::esi, JSFunction::offsetOfExecutable()), X86Registers::r9);
>-        jit.xorPtr(JSInterfaceJIT::TrustedImmPtr(JSFunctionPoison::key()), X86Registers::r9);
>-        jit.loadPtr(JSInterfaceJIT::Address(X86Registers::r9, executableOffsetToFunction), X86Registers::r9);
>-    } else
>-        jit.loadPtr(JSInterfaceJIT::Address(X86Registers::esi, InternalFunction::offsetOfNativeFunctionFor(kind)), X86Registers::r9);
>-    jit.move(JSInterfaceJIT::TrustedImm64(NativeCodePoison::key()), X86Registers::esi);
>-    jit.xor64(X86Registers::esi, X86Registers::r9);
>-    jit.call(X86Registers::r9, JSEntryPtrTag);
>-
>-#else
>-    // Calling convention:      f(ecx, edx, r8, r9, ...);
>-    // Host function signature: f(ExecState*);
>-    jit.move(JSInterfaceJIT::callFrameRegister, X86Registers::ecx);
>+    jit.move(JSInterfaceJIT::callFrameRegister, GPRInfo::argumentGPR0);
> 
>+#if OS(WINDOWS)
>     // Leave space for the callee parameter home addresses.
>     // At this point the stack is aligned to 16 bytes, but if this changes at some point, we need to emit code to align it.
>     jit.subPtr(JSInterfaceJIT::TrustedImm32(4 * sizeof(int64_t)), JSInterfaceJIT::stackPointerRegister);
>+#endif
> 
>-    jit.emitGetFromCallFrameHeaderPtr(CallFrameSlot::callee, X86Registers::edx);
>+    static_assert(GPRInfo::argumentGPR0 != GPRInfo::regT0, "");
>+    static_assert(GPRInfo::argumentGPR1 != GPRInfo::regT0, "");
>+
>+    jit.emitGetFromCallFrameHeaderPtr(CallFrameSlot::callee, GPRInfo::argumentGPR1);
>     if (thunkFunctionType == ThunkFunctionType::JSFunction) {
>-        jit.loadPtr(JSInterfaceJIT::Address(X86Registers::edx, JSFunction::offsetOfExecutable()), X86Registers::r9);
>-        jit.xorPtr(JSInterfaceJIT::TrustedImmPtr(JSFunctionPoison::key()), X86Registers::r9);
>-        jit.call(JSInterfaceJIT::Address(X86Registers::r9, executableOffsetToFunction), JSEntryPtrTag);
>+        jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::argumentGPR1, JSFunction::offsetOfExecutable()), GPRInfo::regT0);
>+        jit.xorPtr(JSInterfaceJIT::TrustedImmPtr(JSFunctionPoison::key()), GPRInfo::regT0);
>+        jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::regT0, executableOffsetToFunction), GPRInfo::regT0);
>     } else
>-        jit.call(JSInterfaceJIT::Address(X86Registers::edx, InternalFunction::offsetOfNativeFunctionFor(kind)), JSEntryPtrTag);
>+        jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::argumentGPR1, InternalFunction::offsetOfNativeFunctionFor(kind)), GPRInfo::regT0);
>+    jit.move(JSInterfaceJIT::TrustedImm64(NativeCodePoison::key()), GPRInfo::argumentGPR1);
>+    jit.xor64(GPRInfo::argumentGPR1, GPRInfo::regT0);
>+    jit.call(GPRInfo::regT0, JSEntryPtrTag);
> 
>+#if OS(WINDOWS)
>     jit.addPtr(JSInterfaceJIT::TrustedImm32(4 * sizeof(int64_t)), JSInterfaceJIT::stackPointerRegister);
> #endif
> 
>diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>index 7d2a98de1e2c65b66dd4cf791219555ad8eef3b7..b2d85fc9bd01a08cb146615c07afa86121e3cb2e 100644
>--- a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>@@ -2154,7 +2154,9 @@ macro nativeCallTrampoline(executableOffsetToFunction)
>     else
>         if X86_64_WIN
>             subp 32, sp
>-            call executableOffsetToFunction[t1], JSEntryPtrTag
>+            loadp _g_NativeCodePoison, t2
>+            xorp executableOffsetToFunction[t1], t2
>+            call t2, JSEntryPtrTag
>             addp 32, sp
>         else
>             loadp _g_NativeCodePoison, t2
>@@ -2197,7 +2199,9 @@ macro internalFunctionCallTrampoline(offsetOfFunction)
>     else
>         if X86_64_WIN
>             subp 32, sp
>-            call offsetOfFunction[t1], JSEntryPtrTag
>+            loadp _g_NativeCodePoison, t2
>+            xorp offsetOfFunction[t1], t2
>+            call t2, JSEntryPtrTag
>             addp 32, sp
>         else
>             loadp _g_NativeCodePoison, t2
>diff --git a/Source/JavaScriptCore/offlineasm/ast.rb b/Source/JavaScriptCore/offlineasm/ast.rb
>index 0ccf7b331bbb30ee11c976c08eb6b29660d8de15..1fe222294dcce787a266319f8df141123d77851a 100644
>--- a/Source/JavaScriptCore/offlineasm/ast.rb
>+++ b/Source/JavaScriptCore/offlineasm/ast.rb
>@@ -1055,8 +1055,8 @@ def self.resetReferenced
> 
>     def self.forReferencedExtern()
>         $referencedExternLabels.each {
>-            | label |
>-            yield "#{label.name}"
>+            | pair |
>+            yield "#{pair[0].name}", pair[1]
>         }
>     end
> 
>@@ -1155,9 +1155,9 @@ def extern?
>         $labelMapping[name].is_a? Label and $labelMapping[name].extern?
>     end
> 
>-    def used
>-        if !$referencedExternLabels.include?(@label) and extern?
>-            $referencedExternLabels.push(@label)
>+    def used type
>+        if !$referencedExternLabels.any? {|pair| pair[0] == @label } and extern?
>+            $referencedExternLabels.push([@label, type])
>         end
>     end
> 
>diff --git a/Source/JavaScriptCore/offlineasm/settings.rb b/Source/JavaScriptCore/offlineasm/settings.rb
>index eec092584fecf441619bd0f87de9ffa08e797d05..17a89cbe9dac4400e0a34d0ed4a05b237621e0fb 100644
>--- a/Source/JavaScriptCore/offlineasm/settings.rb
>+++ b/Source/JavaScriptCore/offlineasm/settings.rb
>@@ -221,8 +221,8 @@ def emitCodeInConfiguration(concreteSettings, ast, backend)
>         File.open("#{File.basename($output.path)}.sym", "w") {
>             | outp |
>             Label.forReferencedExtern {
>-                | name |
>-                outp.puts "EXTERN #{name[1..-1]} : near"
>+                | name, type |
>+                outp.puts "EXTERN #{name[1..-1]} : #{type}"
>             }
>         }
>     end
>diff --git a/Source/JavaScriptCore/offlineasm/x86.rb b/Source/JavaScriptCore/offlineasm/x86.rb
>index a059bc7ef802e650913ea5871423c62d1b79737f..f00437f10b88e85f2d066b0e475ba07f8811daf2 100644
>--- a/Source/JavaScriptCore/offlineasm/x86.rb
>+++ b/Source/JavaScriptCore/offlineasm/x86.rb
>@@ -64,7 +64,7 @@
> # the registers. Also, >8 return types are returned in a weird way.
> #
> # rax => t0,     r0
>-# rcx =>     a0
>+# rcx => t5, a0
> # rdx => t1, a1, r1
> #  r8 => t2, a2
> #  r9 => t3, a3
>@@ -283,8 +283,7 @@ def x86GPR
>             when "t4"
>                 isWin ? "r10" : "r8"
>             when "t5"
>-                raise "cannot use register #{name} on X86-64 Windows" unless not isWin
>-                "r10"
>+                isWin ? "ecx" : "r10"
>             when "csr0"
>                 "ebx"
>             when "csr1"
>@@ -463,10 +462,14 @@ def x86CallOperand(kind)
>         asmLabel
>     end
>     def x86LoadOperand(kind, dst)
>-        # FIXME: Implement this on platforms that aren't Mach-O.
>-        # https://bugs.webkit.org/show_bug.cgi?id=175104
>-        $asm.puts "movq #{asmLabel}@GOTPCREL(%rip), #{dst.x86Operand(:ptr)}"
>-        "#{offset}(#{dst.x86Operand(kind)})"
>+        used(isX64 ? "qword" : "dword")
>+        if isIntelSyntax
>+            $asm.puts "mov #{dst.x86Operand(:ptr)}, #{asmLabel}"
>+            dst.x86Operand(:ptr)
>+        else
>+            $asm.puts "movq #{asmLabel}@GOTPCREL(%rip), #{dst.x86Operand(:ptr)}"
>+            "#{getSizeString(kind)}#{offsetRegister(offset, dst.x86Operand(:ptr))}"
>+        end
>     end
> end
> 
>@@ -584,7 +587,8 @@ def x86Bytes(kind)
> 
>     def emitX86Lea(src, dst, kind)
>         if src.is_a? LabelReference
>-            $asm.puts "movq #{src.asmLabel}@GOTPCREL(%rip), #{dst.x86Operand(:ptr)}"
>+            raise "cannot x86 lea for global variables on X86-64 Windows" if isWin
>+            src.x86LoadOperand(kind, dst)
>         else
>             $asm.puts "lea#{x86Suffix(kind)} #{orderOperands(src.x86AddressOperand(kind), dst.x86Operand(kind))}"
>         end
>@@ -1356,7 +1360,7 @@ def lowerX86Common
>             end
>             op = operands[0].x86CallOperand(:ptr)
>             if operands[0].is_a? LabelReference
>-                operands[0].used
>+                operands[0].used("near")
>             end
>             $asm.puts "call #{op}"
>         when "ret"
>diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
>index 8aea74fdab38f725e4598a460411c1cb99e89f7b..d76f2f5d46eabe40ef553a4b38efe713f39e96c1 100644
>--- a/Source/WTF/wtf/Platform.h
>+++ b/Source/WTF/wtf/Platform.h
>@@ -996,8 +996,8 @@
> #endif
> 
> #define ENABLE_POISON 1
>-/* Not currently supported for 32-bit or OS(WINDOWS) builds (because of missing llint support). Make sure it's disabled. */
>-#if USE(JSVALUE32_64) || OS(WINDOWS)
>+/* Not currently supported for 32-bit builds (because of missing llint support). Make sure it's disabled. */
>+#if USE(JSVALUE32_64)
> #undef ENABLE_POISON
> #define ENABLE_POISON 0
> #endif

Actions: View | Formatted Diff | Diff

Attachments on bug 185421: 339815 | 339959 | 339983 | 339986