Calls to baselineCodeBlockForOriginAndBaselineCodeBlock in operationMaterializeObject...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2018 03:14:09 +0000 (03:14 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2018 03:14:09 +0000 (03:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189940
<rdar://problem/43640987>

Reviewed by Mark Lam.

JSTests:

* stress/use-baseline-codeblock-materialize-osr-exit.js: Added.

Source/JavaScriptCore:

We were calling baselineCodeBlockForOriginAndBaselineCodeBlock with the FTL
CodeBlock. There is nothing semantically wrong with doing that (except for
poor naming), however, the poor naming here led us to make a real semantic
mistake. We wanted the baseline CodeBlock's constant pool, but we were
accessing the FTL CodeBlock's constant pool accidentally. We need to
access the baseline CodeBlock's constant pool when we update the NewArrayBuffer
constant value.

* bytecode/InlineCallFrame.h:
(JSC::baselineCodeBlockForOriginAndBaselineCodeBlock):
* ftl/FTLOperations.cpp:
(JSC::FTL::operationMaterializeObjectInOSR):

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

JSTests/ChangeLog
JSTests/stress/use-baseline-codeblock-materialize-osr-exit.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/InlineCallFrame.h
Source/JavaScriptCore/ftl/FTLOperations.cpp

index 95ce2ed..b4da9f2 100644 (file)
@@ -1,3 +1,13 @@
+2018-09-25  Saam Barati  <sbarati@apple.com>
+
+        Calls to baselineCodeBlockForOriginAndBaselineCodeBlock in operationMaterializeObjectInOSR should actually pass in the baseline CodeBlock
+        https://bugs.webkit.org/show_bug.cgi?id=189940
+        <rdar://problem/43640987>
+
+        Reviewed by Mark Lam.
+
+        * stress/use-baseline-codeblock-materialize-osr-exit.js: Added.
+
 2018-09-24  Saam Barati  <sbarati@apple.com>
 
         Array.prototype.indexOf fast path needs to ensure the length is still valid after performing effects
diff --git a/JSTests/stress/use-baseline-codeblock-materialize-osr-exit.js b/JSTests/stress/use-baseline-codeblock-materialize-osr-exit.js
new file mode 100644 (file)
index 0000000..f86d650
--- /dev/null
@@ -0,0 +1,13 @@
+//@ runDefault("--jitPolicyScale=0")
+
+function foo() {
+    let j = 0;
+    let arr = [0];
+    arr.foo = 0;
+    for (var i = 0; i < 1024; i++) {
+        arr[0] = new Array(1024);
+    }
+}
+
+foo();
+foo();
index 24e3205..64f7509 100644 (file)
@@ -1,3 +1,24 @@
+2018-09-25  Saam Barati  <sbarati@apple.com>
+
+        Calls to baselineCodeBlockForOriginAndBaselineCodeBlock in operationMaterializeObjectInOSR should actually pass in the baseline CodeBlock
+        https://bugs.webkit.org/show_bug.cgi?id=189940
+        <rdar://problem/43640987>
+
+        Reviewed by Mark Lam.
+
+        We were calling baselineCodeBlockForOriginAndBaselineCodeBlock with the FTL
+        CodeBlock. There is nothing semantically wrong with doing that (except for
+        poor naming), however, the poor naming here led us to make a real semantic
+        mistake. We wanted the baseline CodeBlock's constant pool, but we were
+        accessing the FTL CodeBlock's constant pool accidentally. We need to
+        access the baseline CodeBlock's constant pool when we update the NewArrayBuffer
+        constant value.
+
+        * bytecode/InlineCallFrame.h:
+        (JSC::baselineCodeBlockForOriginAndBaselineCodeBlock):
+        * ftl/FTLOperations.cpp:
+        (JSC::FTL::operationMaterializeObjectInOSR):
+
 2018-09-25  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Stricter block syntax in generated ObjC protocol interfaces
index 66c3e3a..326c9ab 100644 (file)
@@ -240,6 +240,7 @@ inline CodeBlock* baselineCodeBlockForInlineCallFrame(InlineCallFrame* inlineCal
 
 inline CodeBlock* baselineCodeBlockForOriginAndBaselineCodeBlock(const CodeOrigin& codeOrigin, CodeBlock* baselineCodeBlock)
 {
+    ASSERT(baselineCodeBlock->jitType() == JITCode::BaselineJIT);
     if (codeOrigin.inlineCallFrame)
         return baselineCodeBlockForInlineCallFrame(codeOrigin.inlineCallFrame);
     return baselineCodeBlock;
index 147bb2c..16442bc 100644 (file)
@@ -225,7 +225,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
         RELEASE_ASSERT(table);
 
         CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(
-            materialization->origin(), exec->codeBlock());
+            materialization->origin(), exec->codeBlock()->baselineAlternative());
         Structure* structure = codeBlock->globalObject()->activationStructure();
 
         // It doesn't matter what values we initialize as bottom values inside the activation constructor because
@@ -286,7 +286,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
                 return ClonedArguments::createWithMachineFrame(exec, exec, ArgumentsMode::Cloned);
             case PhantomCreateRest: {
                 CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(
-                    materialization->origin(), exec->codeBlock());
+                    materialization->origin(), exec->codeBlock()->baselineAlternative());
 
                 unsigned numberOfArgumentsToSkip = codeBlock->numberOfArgumentsToSkip();
                 JSGlobalObject* globalObject = codeBlock->globalObject();
@@ -330,7 +330,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
         RELEASE_ASSERT(callee);
         
         CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(
-            materialization->origin(), exec->codeBlock());
+            materialization->origin(), exec->codeBlock()->baselineAlternative());
         
         // We have an inline frame and we have all of the data we need to recreate it.
         switch (materialization->type()) {
@@ -473,7 +473,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
 
         // For now, we use array allocation profile in the actual CodeBlock. It is OK since current NewArrayBuffer
         // and PhantomNewArrayBuffer are always bound to a specific op_new_array_buffer.
-        CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(materialization->origin(), exec->codeBlock());
+        CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(materialization->origin(), exec->codeBlock()->baselineAlternative());
         Instruction* currentInstruction = &codeBlock->instructions()[materialization->origin().bytecodeIndex];
         RELEASE_ASSERT(Interpreter::getOpcodeID(currentInstruction[0].u.opcode) == op_new_array_buffer);
         auto* newArrayBuffer = bitwise_cast<OpNewArrayBuffer*>(currentInstruction);
@@ -506,7 +506,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
 
     case PhantomNewArrayWithSpread: {
         CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(
-            materialization->origin(), exec->codeBlock());
+            materialization->origin(), exec->codeBlock()->baselineAlternative());
         JSGlobalObject* globalObject = codeBlock->globalObject();
         Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
 
@@ -585,7 +585,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
             }
         }
         RELEASE_ASSERT(regExp);
-        CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(materialization->origin(), exec->codeBlock());
+        CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(materialization->origin(), exec->codeBlock()->baselineAlternative());
         Structure* structure = codeBlock->globalObject()->regExpStructure();
         return RegExpObject::create(vm, structure, regExp);
     }