AccessGenerationState::emitExplicitExceptionHandler can clobber an in use register
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 May 2019 00:49:35 +0000 (00:49 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 May 2019 00:49:35 +0000 (00:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197715
<rdar://problem/50399252>

Reviewed by Filip Pizlo.

JSTests:

* stress/polymorphic-access-exception-handler-should-not-clobber-used-register.js: Added.
(foo):
(bar):

Source/JavaScriptCore:

AccessGenerationState::emitExplicitExceptionHandler was always clobbering
x86's r9 without considering if that register was needed to be preserved
by the IC. This leads to bad things when the DFG/FTL need that register when
OSR exitting after an exception from a GetById call.

* b3/air/AirCode.cpp:
(JSC::B3::Air::Code::Code):
* bytecode/PolymorphicAccess.cpp:
(JSC::AccessGenerationState::emitExplicitExceptionHandler):
* runtime/Options.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245084 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/polymorphic-access-exception-handler-should-not-clobber-used-register.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/air/AirCode.cpp
Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp
Source/JavaScriptCore/runtime/Options.h

index 1e7ed81..50c268a 100644 (file)
@@ -1,3 +1,15 @@
+2019-05-08  Saam barati  <sbarati@apple.com>
+
+        AccessGenerationState::emitExplicitExceptionHandler can clobber an in use register
+        https://bugs.webkit.org/show_bug.cgi?id=197715
+        <rdar://problem/50399252>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/polymorphic-access-exception-handler-should-not-clobber-used-register.js: Added.
+        (foo):
+        (bar):
+
 2019-05-08  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r245068.
diff --git a/JSTests/stress/polymorphic-access-exception-handler-should-not-clobber-used-register.js b/JSTests/stress/polymorphic-access-exception-handler-should-not-clobber-used-register.js
new file mode 100644 (file)
index 0000000..cc7e392
--- /dev/null
@@ -0,0 +1,22 @@
+//@ runDefault("--useConcurrentJIT=0", "--useRandomizingFuzzerAgent=1", "--airRandomizeRegs=1", "--airRandomizeRegsSeed=3421187372", "--jitPolicyScale=0")
+
+function foo() {
+    try {
+        foo.caller;
+    } catch (e) {
+        return Array.of(arguments).join();
+    }
+    throw new Error();
+}
+
+function bar() {
+'use strict';
+    try {
+        return foo();
+    } finally {
+    }
+}
+
+for (var i = 0; i < 10000; ++i) {
+    bar();
+}
index 2a4bbb3..166bdc7 100644 (file)
@@ -1,3 +1,22 @@
+2019-05-08  Saam barati  <sbarati@apple.com>
+
+        AccessGenerationState::emitExplicitExceptionHandler can clobber an in use register
+        https://bugs.webkit.org/show_bug.cgi?id=197715
+        <rdar://problem/50399252>
+
+        Reviewed by Filip Pizlo.
+
+        AccessGenerationState::emitExplicitExceptionHandler was always clobbering
+        x86's r9 without considering if that register was needed to be preserved
+        by the IC. This leads to bad things when the DFG/FTL need that register when
+        OSR exitting after an exception from a GetById call.
+
+        * b3/air/AirCode.cpp:
+        (JSC::B3::Air::Code::Code):
+        * bytecode/PolymorphicAccess.cpp:
+        (JSC::AccessGenerationState::emitExplicitExceptionHandler):
+        * runtime/Options.h:
+
 2019-05-08  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r245068.
index 2a1d486..33f8abf 100644 (file)
@@ -79,8 +79,9 @@ Code::Code(Procedure& proc)
                         calleeSaveRegs.append(reg);
                 });
             if (Options::airRandomizeRegs()) {
-                shuffleVector(volatileRegs, [&] (unsigned limit) { return m_weakRandom.getUint32(limit); });
-                shuffleVector(calleeSaveRegs, [&] (unsigned limit) { return m_weakRandom.getUint32(limit); });
+                WeakRandom random(Options::airRandomizeRegsSeed() ? Options::airRandomizeRegsSeed() : m_weakRandom.getUint32());
+                shuffleVector(volatileRegs, [&] (unsigned limit) { return random.getUint32(limit); });
+                shuffleVector(calleeSaveRegs, [&] (unsigned limit) { return random.getUint32(limit); });
             }
             Vector<Reg> result;
             result.appendVector(volatileRegs);
index dbc3923..c695f7e 100644 (file)
@@ -179,7 +179,11 @@ CallSiteIndex AccessGenerationState::originalCallSiteIndex() const { return stub
 void AccessGenerationState::emitExplicitExceptionHandler()
 {
     restoreScratch();
-    jit->copyCalleeSavesToEntryFrameCalleeSavesBuffer(m_vm.topEntryFrame);
+    jit->pushToSave(GPRInfo::regT0);
+    jit->loadPtr(&m_vm.topEntryFrame, GPRInfo::regT0);
+    jit->copyCalleeSavesToEntryFrameCalleeSavesBuffer(GPRInfo::regT0);
+    jit->popToRestore(GPRInfo::regT0);
+
     if (needsToRestoreRegistersIfException()) {
         // To the JIT that produces the original exception handling
         // call site, they will expect the OSR exit to be arrived
index 72e4413..6bc1500 100644 (file)
@@ -445,6 +445,7 @@ constexpr bool enableWebAssemblyStreamingApi = false;
     v(bool, airForceBriggsAllocator, false, Normal, nullptr) \
     v(bool, airForceIRCAllocator, false, Normal, nullptr) \
     v(bool, airRandomizeRegs, false, Normal, nullptr) \
+    v(unsigned, airRandomizeRegsSeed, 0, Normal, nullptr) \
     v(bool, coalesceSpillSlots, true, Normal, nullptr) \
     v(bool, logAirRegisterPressure, false, Normal, nullptr) \
     v(bool, useB3TailDup, true, Normal, nullptr) \