Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (164204 => 164205)
--- trunk/Source/_javascript_Core/ChangeLog 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-02-17 06:25:05 UTC (rev 164205)
@@ -1,3 +1,31 @@
+2014-02-16 Filip Pizlo <[email protected]>
+
+ DFG::prepareOSREntry should be nice to the stack
+ https://bugs.webkit.org/show_bug.cgi?id=128883
+
+ Reviewed by Oliver Hunt.
+
+ Previously OSR entry had some FIXME's and some really badly commented-out code for
+ clearing stack entries to help GC. It also did some permutations on a stack frame
+ above us, in such a way that it wasn't obviously that we wouldn't clobber our own
+ stack frame. This function also crashed in ASan.
+
+ It just seems like there was too much badness to the whole idea of prepareOSREntry
+ directly editing the stack. So, I changed it to create a stack frame in a scratch
+ buffer on the side and then have some assembly code just copy it into place. This
+ works fine, fixes a FIXME, possibly fixes some stack clobbering, and might help us
+ make more progress with ASan.
+
+ * dfg/DFGOSREntry.cpp:
+ (JSC::DFG::prepareOSREntry):
+ * dfg/DFGOSREntry.h:
+ * dfg/DFGThunks.cpp:
+ (JSC::DFG::osrEntryThunkGenerator):
+ * dfg/DFGThunks.h:
+ * jit/JITOpcodes.cpp:
+ (JSC::JIT::emitSlow_op_loop_hint):
+ * jit/JITOperations.cpp:
+
2014-02-15 Filip Pizlo <[email protected]>
Vector with inline capacity should work with non-PODs
Modified: trunk/Source/_javascript_Core/dfg/DFGOSREntry.cpp (164204 => 164205)
--- trunk/Source/_javascript_Core/dfg/DFGOSREntry.cpp 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/dfg/DFGOSREntry.cpp 2014-02-17 06:25:05 UTC (rev 164205)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -188,8 +188,8 @@
// it seems silly: you'd be diverting the program to error handling when it
// would have otherwise just kept running albeit less quickly.
- unsigned frameSize = jitCode->common.requiredRegisterCountForExecutionAndExit();
- if (!vm->interpreter->stack().ensureCapacityFor(&exec->registers()[virtualRegisterForLocal(frameSize - 1).offset()])) {
+ unsigned frameSizeForCheck = jitCode->common.requiredRegisterCountForExecutionAndExit();
+ if (!vm->interpreter->stack().ensureCapacityFor(&exec->registers()[virtualRegisterForLocal(frameSizeForCheck - 1).offset()])) {
if (Options::verboseOSR())
dataLogF(" OSR failed because stack growth failed.\n");
return 0;
@@ -197,47 +197,68 @@
if (Options::verboseOSR())
dataLogF(" OSR should succeed.\n");
+
+ // At this point we're committed to entering. We will do some work to set things up,
+ // but we also rely on our caller recognizing that when we return a non-null pointer,
+ // that means that we're already past the point of no return and we must succeed at
+ // entering.
- // 3) Perform data format conversions.
- for (size_t local = 0; local < entry->m_expectedValues.numberOfLocals(); ++local) {
- if (entry->m_localsForcedDouble.get(local))
- *bitwise_cast<double*>(exec->registers() + virtualRegisterForLocal(local).offset()) = exec->registers()[virtualRegisterForLocal(local).offset()].jsValue().asNumber();
- if (entry->m_localsForcedMachineInt.get(local))
- *bitwise_cast<int64_t*>(exec->registers() + virtualRegisterForLocal(local).offset()) = exec->registers()[virtualRegisterForLocal(local).offset()].jsValue().asMachineInt() << JSValue::int52ShiftAmount;
+ // 3) Set up the data in the scratch buffer and perform data format conversions.
+
+ unsigned frameSize = jitCode->common.frameRegisterCount;
+
+ Register* scratch = bitwise_cast<Register*>(vm->scratchBufferForSize(sizeof(Register) * (2 + JSStack::CallFrameHeaderSize + frameSize))->dataBuffer());
+
+ *bitwise_cast<size_t*>(scratch + 0) = frameSize;
+
+ void* targetPC = codeBlock->jitCode()->executableAddressAtOffset(entry->m_machineCodeOffset);
+ if (Options::verboseOSR())
+ dataLogF(" OSR using target PC %p.\n", targetPC);
+ RELEASE_ASSERT(targetPC);
+ *bitwise_cast<void**>(scratch + 1) = targetPC;
+
+ Register* pivot = scratch + 2 + JSStack::CallFrameHeaderSize;
+
+ for (int index = -JSStack::CallFrameHeaderSize; index < static_cast<int>(frameSize); ++index) {
+ VirtualRegister reg(-1 - index);
+
+ if (reg.isLocal()) {
+ if (entry->m_localsForcedDouble.get(reg.toLocal())) {
+ *bitwise_cast<double*>(pivot + index) = exec->registers()[reg.offset()].jsValue().asNumber();
+ continue;
+ }
+
+ if (entry->m_localsForcedMachineInt.get(reg.toLocal())) {
+ *bitwise_cast<int64_t*>(pivot + index) = exec->registers()[reg.offset()].jsValue().asMachineInt() << JSValue::int52ShiftAmount;
+ continue;
+ }
+ }
+
+ pivot[index] = exec->registers()[reg.offset()].jsValue();
}
// 4) Reshuffle those registers that need reshuffling.
-
- Vector<EncodedJSValue> temporaryLocals(entry->m_reshufflings.size());
- EncodedJSValue* registers = bitwise_cast<EncodedJSValue*>(exec->registers());
+ Vector<JSValue> temporaryLocals(entry->m_reshufflings.size());
for (unsigned i = entry->m_reshufflings.size(); i--;)
- temporaryLocals[i] = registers[entry->m_reshufflings[i].fromOffset];
+ temporaryLocals[i] = pivot[VirtualRegister(entry->m_reshufflings[i].fromOffset).toLocal()].jsValue();
for (unsigned i = entry->m_reshufflings.size(); i--;)
- registers[entry->m_reshufflings[i].toOffset] = temporaryLocals[i];
+ pivot[VirtualRegister(entry->m_reshufflings[i].toOffset).toLocal()] = temporaryLocals[i];
- // 5) Clear those parts of the call frame that the DFG ain't using. This helps GC on some
- // programs by eliminating some stale pointer pathologies.
-
-#if 0 // FIXME: CStack - This needs to be verified before being enabled
+ // 5) Clear those parts of the call frame that the DFG ain't using. This helps GC on
+ // some programs by eliminating some stale pointer pathologies.
for (unsigned i = frameSize; i--;) {
if (entry->m_machineStackUsed.get(i))
continue;
- registers[virtualRegisterForLocal(i).offset()] = JSValue::encode(JSValue());
+ pivot[i] = JSValue();
}
-#endif
- // 6) Fix the call frame.
+ // 6) Fix the call frame to have the right code block.
- exec->setCodeBlock(codeBlock);
+ *bitwise_cast<CodeBlock**>(pivot - 1 - JSStack::CodeBlock) = codeBlock;
- // 7) Find and return the destination machine code address.
-
- void* result = codeBlock->jitCode()->executableAddressAtOffset(entry->m_machineCodeOffset);
-
if (Options::verboseOSR())
- dataLogF(" OSR returning machine code address %p.\n", result);
-
- return result;
+ dataLogF(" OSR returning data buffer %p.\n", scratch);
+ return scratch;
}
} } // namespace JSC::DFG
Modified: trunk/Source/_javascript_Core/dfg/DFGOSREntry.h (164204 => 164205)
--- trunk/Source/_javascript_Core/dfg/DFGOSREntry.h 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/dfg/DFGOSREntry.h 2014-02-17 06:25:05 UTC (rev 164205)
@@ -67,6 +67,8 @@
return osrEntryData->m_bytecodeIndex;
}
+// Returns a pointer to a data buffer that the OSR entry thunk will recognize and
+// parse. If this returns null, it means
void* prepareOSREntry(ExecState*, CodeBlock*, unsigned bytecodeIndex);
#else
inline void* prepareOSREntry(ExecState*, CodeBlock*, unsigned) { return 0; }
Modified: trunk/Source/_javascript_Core/dfg/DFGThunks.cpp (164204 => 164205)
--- trunk/Source/_javascript_Core/dfg/DFGThunks.cpp 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/dfg/DFGThunks.cpp 2014-02-17 06:25:05 UTC (rev 164205)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -96,6 +96,46 @@
return FINALIZE_CODE(patchBuffer, ("DFG OSR exit generation thunk"));
}
+MacroAssemblerCodeRef osrEntryThunkGenerator(VM* vm)
+{
+ MacroAssembler jit;
+
+ // We get passed the address of a scratch buffer. The first 8-byte slot of the buffer
+ // is the frame size. The second 8-byte slot is the pointer to where we are supposed to
+ // jump. The remaining bytes are the new call frame header followed by the locals.
+
+ ptrdiff_t offsetOfFrameSize = 0; // This is the DFG frame count.
+ ptrdiff_t offsetOfTargetPC = offsetOfFrameSize + sizeof(EncodedJSValue);
+ ptrdiff_t offsetOfPayload = offsetOfTargetPC + sizeof(EncodedJSValue);
+ ptrdiff_t offsetOfLocals = offsetOfPayload + sizeof(Register) * JSStack::CallFrameHeaderSize;
+
+ jit.move(GPRInfo::returnValueGPR2, GPRInfo::regT0);
+ jit.loadPtr(MacroAssembler::Address(GPRInfo::regT0, offsetOfFrameSize), GPRInfo::regT1); // Load the frame size.
+ jit.move(GPRInfo::regT1, GPRInfo::regT2);
+ jit.lshiftPtr(MacroAssembler::Imm32(3), GPRInfo::regT2);
+ jit.move(GPRInfo::callFrameRegister, MacroAssembler::stackPointerRegister);
+ jit.subPtr(GPRInfo::regT2, MacroAssembler::stackPointerRegister);
+
+ MacroAssembler::Label loop = jit.label();
+ jit.subPtr(MacroAssembler::TrustedImm32(1), GPRInfo::regT1);
+ jit.move(GPRInfo::regT1, GPRInfo::regT4);
+ jit.negPtr(GPRInfo::regT4);
+ jit.load32(MacroAssembler::BaseIndex(GPRInfo::regT0, GPRInfo::regT1, MacroAssembler::TimesEight, offsetOfLocals), GPRInfo::regT2);
+ jit.load32(MacroAssembler::BaseIndex(GPRInfo::regT0, GPRInfo::regT1, MacroAssembler::TimesEight, offsetOfLocals + sizeof(int32_t)), GPRInfo::regT3);
+ jit.store32(GPRInfo::regT2, MacroAssembler::BaseIndex(GPRInfo::callFrameRegister, GPRInfo::regT4, MacroAssembler::TimesEight, -static_cast<intptr_t>(sizeof(Register))));
+ jit.store32(GPRInfo::regT3, MacroAssembler::BaseIndex(GPRInfo::callFrameRegister, GPRInfo::regT4, MacroAssembler::TimesEight, -static_cast<intptr_t>(sizeof(Register)) + static_cast<intptr_t>(sizeof(int32_t))));
+ jit.branchPtr(MacroAssembler::NotEqual, GPRInfo::regT1, MacroAssembler::TrustedImmPtr(bitwise_cast<void*>(-static_cast<intptr_t>(JSStack::CallFrameHeaderSize)))).linkTo(loop, &jit);
+
+ jit.loadPtr(MacroAssembler::Address(GPRInfo::regT0, offsetOfTargetPC), GPRInfo::regT1);
+ MacroAssembler::Jump ok = jit.branchPtr(MacroAssembler::Above, GPRInfo::regT1, MacroAssembler::TrustedImmPtr(bitwise_cast<void*>(static_cast<intptr_t>(1000))));
+ jit.breakpoint();
+ ok.link(&jit);
+ jit.jump(GPRInfo::regT1);
+
+ LinkBuffer patchBuffer(*vm, &jit, GLOBAL_THUNK_ID);
+ return FINALIZE_CODE(patchBuffer, ("DFG OSR entry thunk"));
+}
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/dfg/DFGThunks.h (164204 => 164205)
--- trunk/Source/_javascript_Core/dfg/DFGThunks.h 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/dfg/DFGThunks.h 2014-02-17 06:25:05 UTC (rev 164205)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -39,6 +39,7 @@
namespace DFG {
MacroAssemblerCodeRef osrExitGenerationThunkGenerator(VM*);
+MacroAssemblerCodeRef osrEntryThunkGenerator(VM*);
} } // namespace JSC::DFG
Modified: trunk/Source/_javascript_Core/jit/JITOpcodes.cpp (164204 => 164205)
--- trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2014-02-17 06:25:05 UTC (rev 164205)
@@ -1106,7 +1106,11 @@
callOperation(operationOptimize, m_bytecodeOffset);
Jump noOptimizedEntry = branchTestPtr(Zero, returnValueGPR);
- move(returnValueGPR2, stackPointerRegister);
+ if (!ASSERT_DISABLED) {
+ Jump ok = branchPtr(MacroAssembler::Above, regT0, TrustedImmPtr(bitwise_cast<void*>(static_cast<intptr_t>(1000))));
+ breakpoint();
+ ok.link(this);
+ }
jump(returnValueGPR);
noOptimizedEntry.link(this);
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (164204 => 164205)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-02-17 03:52:02 UTC (rev 164204)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-02-17 06:25:05 UTC (rev 164205)
@@ -32,6 +32,7 @@
#include "DFGCompilationMode.h"
#include "DFGDriver.h"
#include "DFGOSREntry.h"
+#include "DFGThunks.h"
#include "DFGWorklist.h"
#include "Error.h"
#include "ErrorHandlingScope.h"
@@ -1189,16 +1190,14 @@
CodeBlock* optimizedCodeBlock = codeBlock->replacement();
ASSERT(JITCode::isOptimizingJIT(optimizedCodeBlock->jitType()));
- if (void* address = DFG::prepareOSREntry(exec, optimizedCodeBlock, bytecodeIndex)) {
+ if (void* dataBuffer = DFG::prepareOSREntry(exec, optimizedCodeBlock, bytecodeIndex)) {
if (Options::verboseOSR()) {
dataLog(
- "Performing OSR ", *codeBlock, " -> ", *optimizedCodeBlock, ", address ",
- RawPointer(OUR_RETURN_ADDRESS), " -> ", RawPointer(address), ".\n");
+ "Performing OSR ", *codeBlock, " -> ", *optimizedCodeBlock, ".\n");
}
codeBlock->optimizeSoon();
- ASSERT(exec->codeBlock() == optimizedCodeBlock);
- return encodeResult(address, exec->topOfFrame());
+ return encodeResult(vm.getCTIStub(DFG::osrEntryThunkGenerator).code().executableAddress(), dataBuffer);
}
if (Options::verboseOSR()) {
