2008-12-28 Cameron Zwarich <cwzwarich@uwaterloo.ca>
authorcwzwarich@webkit.org <cwzwarich@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 28 Dec 2008 08:52:06 +0000 (08:52 +0000)
committercwzwarich@webkit.org <cwzwarich@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 28 Dec 2008 08:52:06 +0000 (08:52 +0000)
        Reviewed by Oliver Hunt.

        Bug 22840: REGRESSION (r38349): Gmail doesn't load with profiling enabled
        <https://bugs.webkit.org/show_bug.cgi?id=22840>
        <rdar://problem/6468077>

        JavaScriptCore:

        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitNewArray): Add an assertion that the range
        of registers passed to op_new_array is sequential.
        (JSC::BytecodeGenerator::emitCall): Correct the relocation of registers
        when emitting profiler hooks so that registers aren't leaked. Also, add
        an assertion that the 'this' register is always ref'd (because it is),
        remove the needless protection of the 'this' register when relocating,
        and add an assertion that the range of registers passed to op_call for
        function call arguments is sequential.
        (JSC::BytecodeGenerator::emitConstruct): Correct the relocation of
        registers when emitting profiler hooks so that registers aren't leaked.
        Also, add an assertion that the range of registers passed to op_construct
        for function call arguments is sequential.

        LayoutTests:

        * fast/profiler/call-register-leak-expected.txt: Added.
        * fast/profiler/call-register-leak.html: Added.

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

JavaScriptCore/ChangeLog
JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
LayoutTests/ChangeLog
LayoutTests/fast/profiler/call-register-leak-expected.txt [new file with mode: 0644]
LayoutTests/fast/profiler/call-register-leak.html [new file with mode: 0644]

index f3a0996..7ca1733 100644 (file)
@@ -1,3 +1,25 @@
+2008-12-28  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Oliver Hunt.
+
+        Bug 22840: REGRESSION (r38349): Gmail doesn't load with profiling enabled
+        <https://bugs.webkit.org/show_bug.cgi?id=22840>
+        <rdar://problem/6468077>
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitNewArray): Add an assertion that the range
+        of registers passed to op_new_array is sequential.
+        (JSC::BytecodeGenerator::emitCall): Correct the relocation of registers
+        when emitting profiler hooks so that registers aren't leaked. Also, add
+        an assertion that the 'this' register is always ref'd (because it is),
+        remove the needless protection of the 'this' register when relocating,
+        and add an assertion that the range of registers passed to op_call for
+        function call arguments is sequential.
+        (JSC::BytecodeGenerator::emitConstruct): Correct the relocation of
+        registers when emitting profiler hooks so that registers aren't leaked.
+        Also, add an assertion that the range of registers passed to op_construct
+        for function call arguments is sequential.
+
 2008-12-26  Mark Rowe  <mrowe@apple.com>
 
         Reviewed by Alexey Proskuryakov.
index f7b5c9a..b3dcdd2 100644 (file)
@@ -1188,6 +1188,8 @@ RegisterID* BytecodeGenerator::emitNewArray(RegisterID* dst, ElementNode* elemen
         if (n->elision())
             break;
         argv.append(newTemporary());
+        // op_new_array requires the initial values to be a sequential range of registers
+        ASSERT(argv.size() == 1 || argv[argv.size() - 1]->index() == argv[argv.size() - 2]->index() + 1);
         emitNode(argv.last().get(), n->value());
     }
     emitOpcode(op_new_array);
@@ -1236,13 +1238,14 @@ RegisterID* BytecodeGenerator::emitCall(OpcodeID opcodeID, RegisterID* dst, Regi
 {
     ASSERT(opcodeID == op_call || opcodeID == op_call_eval);
     ASSERT(func->refCount());
+    ASSERT(thisRegister->refCount());
 
+    RegisterID* originalFunc = func;
     if (m_shouldEmitProfileHooks) {
         // If codegen decided to recycle func as this call's destination register,
         // we need to undo that optimization here so that func will still be around
         // for the sake of op_profile_did_call.
         if (dst == func) {
-            RefPtr<RegisterID> protect = thisRegister;
             RefPtr<RegisterID> movedThisRegister = emitMove(newTemporary(), thisRegister);
             RefPtr<RegisterID> movedFunc = emitMove(thisRegister, func);
             
@@ -1256,6 +1259,8 @@ RegisterID* BytecodeGenerator::emitCall(OpcodeID opcodeID, RegisterID* dst, Regi
     argv.append(thisRegister);
     for (ArgumentListNode* n = argumentsNode->m_listNode.get(); n; n = n->m_next.get()) {
         argv.append(newTemporary());
+        // op_call requires the arguments to be a sequential range of registers
+        ASSERT(argv[argv.size() - 1]->index() == argv[argv.size() - 2]->index() + 1);
         emitNode(argv.last().get(), n);
     }
 
@@ -1290,7 +1295,7 @@ RegisterID* BytecodeGenerator::emitCall(OpcodeID opcodeID, RegisterID* dst, Regi
         emitOpcode(op_profile_did_call);
         instructions().append(func->index());
 
-        if (dst == func) {
+        if (dst == originalFunc) {
             thisRegister->deref();
             func->deref();
         }
@@ -1321,6 +1326,7 @@ RegisterID* BytecodeGenerator::emitConstruct(RegisterID* dst, RegisterID* func,
 {
     ASSERT(func->refCount());
 
+    RegisterID* originalFunc = func;
     if (m_shouldEmitProfileHooks) {
         // If codegen decided to recycle func as this call's destination register,
         // we need to undo that optimization here so that func will still be around
@@ -1338,6 +1344,8 @@ RegisterID* BytecodeGenerator::emitConstruct(RegisterID* dst, RegisterID* func,
     argv.append(newTemporary()); // reserve space for "this"
     for (ArgumentListNode* n = argumentsNode ? argumentsNode->m_listNode.get() : 0; n; n = n->m_next.get()) {
         argv.append(newTemporary());
+        // op_construct requires the arguments to be a sequential range of registers
+        ASSERT(argv[argv.size() - 1]->index() == argv[argv.size() - 2]->index() + 1);
         emitNode(argv.last().get(), n);
     }
 
@@ -1378,7 +1386,7 @@ RegisterID* BytecodeGenerator::emitConstruct(RegisterID* dst, RegisterID* func,
         emitOpcode(op_profile_did_call);
         instructions().append(func->index());
         
-        if (dst == func)
+        if (dst == originalFunc)
             func->deref();
     }
 
index 88bbf47..60e5ca8 100644 (file)
@@ -1,3 +1,14 @@
+2008-12-28  Cameron Zwarich  <zwarich@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Add tests for bug 22840: REGRESSION (r38349): Gmail doesn't load with profiling enabled
+        <https://bugs.webkit.org/show_bug.cgi?id=22840>
+        <rdar://problem/6468077>
+
+        * fast/profiler/call-register-leak-expected.txt: Added.
+        * fast/profiler/call-register-leak.html: Added.
+
 2008-12-27  Alexey Proskuryakov  <ap@webkit.org>
 
         Suggested by Dave Levin.
diff --git a/LayoutTests/fast/profiler/call-register-leak-expected.txt b/LayoutTests/fast/profiler/call-register-leak-expected.txt
new file mode 100644 (file)
index 0000000..d3647bf
--- /dev/null
@@ -0,0 +1,17 @@
+This page tests that the generation of bytecode allocates registers correctly when profiling is enabled. To run the test manually, enable profiling in the web inspector and reload this page.
+
+PASS: localCallTest(1, 2) should be 2 and is.
+
+PASS: globalCallTest(1, 2) should be 2 and is.
+
+PASS: scopedCallTest(1, 2) should be 2 and is.
+
+PASS: resolveCallTest(1, 2) should be 2 and is.
+
+PASS: bracketCallTest(1, 2) should be 2 and is.
+
+PASS: dotCallTest(1, 2) should be 2 and is.
+
+PASS: newTest(1, 2) should be 2 and is.
+
+
diff --git a/LayoutTests/fast/profiler/call-register-leak.html b/LayoutTests/fast/profiler/call-register-leak.html
new file mode 100644 (file)
index 0000000..59bc6e3
--- /dev/null
@@ -0,0 +1,105 @@
+<body>
+<head>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.setJavaScriptProfilingEnabled(true);
+}
+
+function log(s)
+{
+    if (this.document)
+        document.getElementById("console").appendChild(document.createTextNode(s + "\n"));
+    else
+        print(s + "\n");
+}
+
+function shouldBe(a, aDescription, b)
+{
+    if (a === b)
+        log("PASS: " + aDescription + " should be " + b + " and is.\n");
+    else
+        log("FAIL: " + aDescription + " should be " + b + " but instead is " + a + ".\n");
+}
+
+function localCallTest(a, b)
+{
+    function localCall(o)
+    {
+        return o.toString();
+    }
+    return [localCall(a), b][1];
+}
+
+function globalCall(o)
+{
+    return o.toString();
+}
+
+function globalCallTest(a, b)
+{
+    return [globalCall(a), b][1];
+}
+
+function scopedCallTest(a, b)
+{
+    function scopedCall(o)
+    {
+        return o.toString();
+    }
+
+    function f()
+    {
+        return [scopedCall(a), b][1];
+    }
+
+    return f();
+}
+
+function resolveCallTest(a, b)
+{
+    o = { resolvedCall: function(o) { return o.toString(); }};
+    with (o) {
+        return [resolvedCall(o), b][1];
+    }
+}
+
+function bracketCallTest(a, b)
+{
+    return [a["toString"](), b][1];
+}
+
+function dotCallTest(a, b)
+{
+    return [a.toString(), b][1];
+}
+
+function testConstructor(o)
+{
+    return o.toString();
+}
+
+function newTest(a, b)
+{
+    return [new testConstructor(a), b][1];
+}
+
+function startTest()
+{
+    shouldBe(localCallTest(1, 2), "localCallTest(1, 2)", 2);
+    shouldBe(globalCallTest(1, 2), "globalCallTest(1, 2)", 2);
+    shouldBe(scopedCallTest(1, 2), "scopedCallTest(1, 2)", 2);
+    shouldBe(resolveCallTest(1, 2), "resolveCallTest(1, 2)", 2);
+    shouldBe(bracketCallTest(1, 2), "bracketCallTest(1, 2)", 2);
+    shouldBe(dotCallTest(1, 2), "dotCallTest(1, 2)", 2);
+    shouldBe(newTest(1, 2), "newTest(1, 2)", 2);
+}
+</script>
+</head>
+<body onload="startTest()">
+<p>
+This page tests that the generation of bytecode allocates registers correctly when profiling is enabled. To run the test manually, enable profiling in the web inspector and reload this page.
+</p>
+<pre id="console"></pre>
+</body>
+</html>