Changeset 41544 in webkit for trunk/JavaScriptCore/jit/JIT.cpp

Timestamp:

Mar 9, 2009, 6:09:44 PM (16 years ago)

Author:

[email protected]

Message:

Bug 24447: REGRESSION (r41508): Google Maps does not complete initialization
<rdar://problem/6657774>

Reviewed by Gavin Barraclough

r41508 actually exposed a pre-existing bug where we were not invalidating the result
register cache at jump targets. This causes problems when condition loads occur in an

expression -- namely through the ?: and

operators. This patch corrects these issues

by marking the target of all forward jumps as being a jump target, and then clears the
result register cache when ever it starts generating code for a targeted instruction.

I do not believe it is possible to cause this class of failure outside of a single
expression, and expressions only provide forward branches, so this should resolve this
entire class of bug. That said i've included a test case that gets as close as possible
to hitting this bug with a back branch, to hopefully prevent anyone from introducing the
problem in future.

File:

• trunk/JavaScriptCore/jit/JIT.cpp (modified) (10 diffs)

trunk/JavaScriptCore/jit/JIT.cpp

@@ -283 +283 @@
     }
 
+#define RECORD_JUMP_TARGET(targetOffset) \
+   do { m_labels[m_bytecodeIndex + (targetOffset)].used(); } while (false)
+
 void JIT::privateCompileMainPass()
 {
@@ -300 +303 @@
 #endif
 
+        if (m_labels[m_bytecodeIndex].isUsed())
+            killLastResultRegister();
+        
         m_labels[m_bytecodeIndex] = label();
         OpcodeID opcodeID = m_interpreter->getOpcodeID(currentInstruction->u.opcode);
@@ -340 +346 @@
             unsigned target = currentInstruction[1].u.operand;
             addJump(jump(), target + 1);
+            RECORD_JUMP_TARGET(target + 1);
             NEXT_OPCODE(op_jmp);
         }
@@ -746 +753 @@
                 addJump(branch32(GreaterThanOrEqual, regT0, regT1), target + 3);
             }
+            RECORD_JUMP_TARGET(target + 3);
             NEXT_OPCODE(op_jnless);
         }
@@ -767 +775 @@
 
             isNonZero.link(this);
+            RECORD_JUMP_TARGET(target + 2);
             NEXT_OPCODE(op_jfalse);
         };
@@ -787 +796 @@
 
             wasNotImmediate.link(this);
+            RECORD_JUMP_TARGET(target + 2);
             NEXT_OPCODE(op_jeq_null);
         };
@@ -807 +817 @@
 
             wasNotImmediate.link(this);
+            RECORD_JUMP_TARGET(target + 2);
             NEXT_OPCODE(op_jneq_null);
         }
@@ -825 +836 @@
             addJump(jump(), target + 2);
             m_jsrSites.append(JSRInfo(storeLocation, label()));
+            killLastResultRegister();
+            RECORD_JUMP_TARGET(target + 2);
             NEXT_OPCODE(op_jsr);
         }
         case op_sret: {
             jump(Address(callFrameRegister, sizeof(Register) * currentInstruction[1].u.operand));
+            killLastResultRegister();
             NEXT_OPCODE(op_sret);
         }
@@ -893 +907 @@
 
             isZero.link(this);
+            RECORD_JUMP_TARGET(target + 2);
             NEXT_OPCODE(op_jtrue);
         }
@@ -1032 +1047 @@
             unsigned target = currentInstruction[2].u.operand;
             addJump(jump(), target + 2);
+            RECORD_JUMP_TARGET(target + 2);
             NEXT_OPCODE(op_jmp_scopes);
         }