Attachment 339986 Details for Bug 185421 – Patch

[patch] Patch

bug-185421-20180510030957.patch (text/plain), 12.16 KB, created by Yusuke Suzuki on 2018-05-09 11:09:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-09 11:09:58 PDT

Size: 12.16 KB

patch

obsolete

>Subversion Revision: 231573
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 3aa3d0262bb940532e6436d737d380e3d18b3a8a..a3930015f58b64b329e0479e8b3cda73bb803a38 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,38 @@
>+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/LowLevelInterpreter.asm:
>+        * 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 for `loadp g_XXXPoison, rax`.
>+
>+            mov rax, g_XXXPoison
>+            mov rax, rax
>+
>+        In favor of simplicity of the offlineasm backend implementation.
>+
>+        And we add x86Oprand for LabelReference for X86_64_WIN. So, we can do, `xorp poison, scratch`.
>+        This is necessary since X86_64_WIN does not have "t5".
>+
>+        * 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-09  Leo Balter  <leonardo.balter@gmail.com>
> 
>         [JSC] Fix ArraySpeciesCreate to return a new Array when the given object is not an array
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 09117e09fff39f98d27758b7f4952872c15ef4c4..83aa7bd31a58b5e97c0230f1ba3c256bf385cdd1 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-09  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         [WPE] Build cleanly with GCC 8 and ICU 60
>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/LowLevelInterpreter.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter.asm
>index adc1d6e656f3eecdf85e5ec66f64750d4bbf27dc..8d7804380abf333a0784e1fc40206da31537ca2b 100644
>--- a/Source/JavaScriptCore/llint/LowLevelInterpreter.asm
>+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter.asm
>@@ -785,8 +785,12 @@ end
> 
> macro unpoison(poison, field, scratch)
>     if POISON
>-        loadp poison, scratch
>-        xorp scratch, field
>+        if X86_64_WIN
>+            xorp poison, field
>+        else
>+            loadp poison, scratch
>+            xorp scratch, field
>+        end
>     end
> end
> 
>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..c54a5d34efcc5d584680ed64edff430cebb8a8f5 100644
>--- a/Source/JavaScriptCore/offlineasm/x86.rb
>+++ b/Source/JavaScriptCore/offlineasm/x86.rb
>@@ -459,14 +459,23 @@ def x86CallOperand(kind)
> end
> 
> class LabelReference
>+    def x86Operand(kind)
>+        used(isX64 ? "qword" : "dword")
>+        raise "cannot use global variables directly for x86 operands on non-Windows" unless isWin
>+        asmLabel
>+    end
>     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 +593,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 +1366,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 ad79f28837b69a8f4700f13662c6bddb33451750..729f74f6ebb6f125930b2747723229c95d96ebd9 100644
>--- a/Source/WTF/wtf/Platform.h
>+++ b/Source/WTF/wtf/Platform.h
>@@ -992,8 +992,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

Flags:

mjs: review-

Actions: View | Formatted Diff | Diff

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