Log Message
A patchable GetById right after a watchpoint should have the appropriate nop padding https://bugs.webkit.org/show_bug.cgi?id=94635
Reviewed by Mark Hahnenberg. Source/_javascript_Core: * assembler/AbstractMacroAssembler.h: (JSC::AbstractMacroAssembler::padBeforePatch): (AbstractMacroAssembler): * assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::load32WithCompactAddressOffsetPatch): (JSC::MacroAssemblerARMv7::moveWithPatch): (JSC::MacroAssemblerARMv7::patchableJump): * assembler/MacroAssemblerX86.h: (JSC::MacroAssemblerX86::moveWithPatch): (JSC::MacroAssemblerX86::branchPtrWithPatch): (JSC::MacroAssemblerX86::storePtrWithPatch): * assembler/MacroAssemblerX86Common.h: (JSC::MacroAssemblerX86Common::load32WithAddressOffsetPatch): (JSC::MacroAssemblerX86Common::load32WithCompactAddressOffsetPatch): (JSC::MacroAssemblerX86Common::loadCompactWithAddressOffsetPatch): (JSC::MacroAssemblerX86Common::store32WithAddressOffsetPatch): * assembler/MacroAssemblerX86_64.h: (JSC::MacroAssemblerX86_64::loadPtrWithAddressOffsetPatch): (JSC::MacroAssemblerX86_64::loadPtrWithCompactAddressOffsetPatch): (JSC::MacroAssemblerX86_64::storePtrWithAddressOffsetPatch): (JSC::MacroAssemblerX86_64::moveWithPatch): * jit/JumpReplacementWatchpoint.cpp: (JSC::JumpReplacementWatchpoint::fireInternal): LayoutTests: * fast/js/dfg-patchable-get-by-id-after-watchpoint-expected.txt: Added. * fast/js/dfg-patchable-get-by-id-after-watchpoint.html: Added. * fast/js/script-tests/dfg-patchable-get-by-id-after-watchpoint.js: Added. (foo): (O): (O.prototype.f): (P1): (P2):
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h
- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h
- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h
- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h
- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h
- trunk/Source/_javascript_Core/jit/JumpReplacementWatchpoint.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (126213 => 126214)
--- trunk/LayoutTests/ChangeLog 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/LayoutTests/ChangeLog 2012-08-21 23:30:19 UTC (rev 126214)
@@ -1,3 +1,19 @@
+2012-08-21 Filip Pizlo <[email protected]>
+
+ A patchable GetById right after a watchpoint should have the appropriate nop padding
+ https://bugs.webkit.org/show_bug.cgi?id=94635
+
+ Reviewed by Mark Hahnenberg.
+
+ * fast/js/dfg-patchable-get-by-id-after-watchpoint-expected.txt: Added.
+ * fast/js/dfg-patchable-get-by-id-after-watchpoint.html: Added.
+ * fast/js/script-tests/dfg-patchable-get-by-id-after-watchpoint.js: Added.
+ (foo):
+ (O):
+ (O.prototype.f):
+ (P1):
+ (P2):
+
2012-08-21 Mark Hahnenberg <[email protected]>
WTF Threading leaks kernel objects on platforms that use pthreads
Added: trunk/LayoutTests/fast/js/dfg-patchable-get-by-id-after-watchpoint-expected.txt (0 => 126214)
--- trunk/LayoutTests/fast/js/dfg-patchable-get-by-id-after-watchpoint-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-patchable-get-by-id-after-watchpoint-expected.txt 2012-08-21 23:30:19 UTC (rev 126214)
@@ -0,0 +1,209 @@
+This tests that a patchable GetById right after a watchpoint has the appropriate nop padding.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS foo(o, p) is 27
+PASS foo(o, p) is 44
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/dfg-patchable-get-by-id-after-watchpoint.html (0 => 126214)
--- trunk/LayoutTests/fast/js/dfg-patchable-get-by-id-after-watchpoint.html (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-patchable-get-by-id-after-watchpoint.html 2012-08-21 23:30:19 UTC (rev 126214)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/script-tests/dfg-patchable-get-by-id-after-watchpoint.js (0 => 126214)
--- trunk/LayoutTests/fast/js/script-tests/dfg-patchable-get-by-id-after-watchpoint.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-patchable-get-by-id-after-watchpoint.js 2012-08-21 23:30:19 UTC (rev 126214)
@@ -0,0 +1,47 @@
+description(
+"This tests that a patchable GetById right after a watchpoint has the appropriate nop padding."
+);
+
+function foo(o, p) {
+ var a = p.f;
+ var b = o.f; // Watchpoint.
+ var c = p.g; // Patchable GetById.
+ return b(a + c);
+}
+
+function O() {
+}
+
+O.prototype.f = function(x) { return x + 1; };
+
+var o = new O();
+
+function P1() {
+}
+
+P1.prototype.g = 42;
+
+function P2() {
+}
+
+P2.prototype.g = 24;
+
+var p1 = new P1();
+var p2 = new P2();
+
+p1.f = 1;
+p2.f = 2;
+
+for (var i = 0; i < 200; ++i) {
+ var p = (i % 2) ? p1 : p2;
+ var expected = (i % 2) ? 44 : 27;
+ if (i == 150) {
+ // Cause first the watchpoint on o.f to fire, and then the GetById
+ // to be reset.
+ O.prototype.g = 57; // Fire the watchpoint.
+ P1.prototype.h = 58; // Reset the GetById.
+ P2.prototype.h = 59; // Not necessary, but what the heck - this resets the GetById even more.
+ }
+ shouldBe("foo(o, p)", "" + expected);
+}
+
Modified: trunk/Source/_javascript_Core/ChangeLog (126213 => 126214)
--- trunk/Source/_javascript_Core/ChangeLog 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-08-21 23:30:19 UTC (rev 126214)
@@ -1,3 +1,34 @@
+2012-08-21 Filip Pizlo <[email protected]>
+
+ A patchable GetById right after a watchpoint should have the appropriate nop padding
+ https://bugs.webkit.org/show_bug.cgi?id=94635
+
+ Reviewed by Mark Hahnenberg.
+
+ * assembler/AbstractMacroAssembler.h:
+ (JSC::AbstractMacroAssembler::padBeforePatch):
+ (AbstractMacroAssembler):
+ * assembler/MacroAssemblerARMv7.h:
+ (JSC::MacroAssemblerARMv7::load32WithCompactAddressOffsetPatch):
+ (JSC::MacroAssemblerARMv7::moveWithPatch):
+ (JSC::MacroAssemblerARMv7::patchableJump):
+ * assembler/MacroAssemblerX86.h:
+ (JSC::MacroAssemblerX86::moveWithPatch):
+ (JSC::MacroAssemblerX86::branchPtrWithPatch):
+ (JSC::MacroAssemblerX86::storePtrWithPatch):
+ * assembler/MacroAssemblerX86Common.h:
+ (JSC::MacroAssemblerX86Common::load32WithAddressOffsetPatch):
+ (JSC::MacroAssemblerX86Common::load32WithCompactAddressOffsetPatch):
+ (JSC::MacroAssemblerX86Common::loadCompactWithAddressOffsetPatch):
+ (JSC::MacroAssemblerX86Common::store32WithAddressOffsetPatch):
+ * assembler/MacroAssemblerX86_64.h:
+ (JSC::MacroAssemblerX86_64::loadPtrWithAddressOffsetPatch):
+ (JSC::MacroAssemblerX86_64::loadPtrWithCompactAddressOffsetPatch):
+ (JSC::MacroAssemblerX86_64::storePtrWithAddressOffsetPatch):
+ (JSC::MacroAssemblerX86_64::moveWithPatch):
+ * jit/JumpReplacementWatchpoint.cpp:
+ (JSC::JumpReplacementWatchpoint::fireInternal):
+
2012-08-20 Mark Lam <[email protected]>
Fix broken non-JIT build.
Modified: trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h (126213 => 126214)
--- trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h 2012-08-21 23:30:19 UTC (rev 126214)
@@ -606,6 +606,12 @@
return Label(this);
}
+ void padBeforePatch()
+ {
+ // Rely on the fact that asking for a label already does the padding.
+ (void)label();
+ }
+
Label watchpointLabel()
{
Label result;
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h (126213 => 126214)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2012-08-21 23:30:19 UTC (rev 126214)
@@ -646,6 +646,8 @@
DataLabelCompact load32WithCompactAddressOffsetPatch(Address address, RegisterID dest)
{
+ padBeforePatch();
+
RegisterID base = address.base;
DataLabelCompact label(this);
@@ -1626,12 +1628,14 @@
ALWAYS_INLINE DataLabel32 moveWithPatch(TrustedImm32 imm, RegisterID dst)
{
+ padBeforePatch();
moveFixedWidthEncoding(imm, dst);
return DataLabel32(this);
}
ALWAYS_INLINE DataLabelPtr moveWithPatch(TrustedImmPtr imm, RegisterID dst)
{
+ padBeforePatch();
moveFixedWidthEncoding(TrustedImm32(imm), dst);
return DataLabelPtr(this);
}
@@ -1659,6 +1663,7 @@
PatchableJump patchableJump()
{
+ padBeforePatch();
m_makeJumpPatchable = true;
Jump result = jump();
m_makeJumpPatchable = false;
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h (126213 => 126214)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h 2012-08-21 23:30:19 UTC (rev 126214)
@@ -175,6 +175,7 @@
DataLabelPtr moveWithPatch(TrustedImmPtr initialValue, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movl_i32r(initialValue.asIntptr(), dest);
return DataLabelPtr(this);
}
@@ -191,6 +192,7 @@
Jump branchPtrWithPatch(RelationalCondition cond, RegisterID left, DataLabelPtr& dataLabel, TrustedImmPtr initialRightValue = TrustedImmPtr(0))
{
+ padBeforePatch();
m_assembler.cmpl_ir_force32(initialRightValue.asIntptr(), left);
dataLabel = DataLabelPtr(this);
return Jump(m_assembler.jCC(x86Condition(cond)));
@@ -198,6 +200,7 @@
Jump branchPtrWithPatch(RelationalCondition cond, Address left, DataLabelPtr& dataLabel, TrustedImmPtr initialRightValue = TrustedImmPtr(0))
{
+ padBeforePatch();
m_assembler.cmpl_im_force32(initialRightValue.asIntptr(), left.offset, left.base);
dataLabel = DataLabelPtr(this);
return Jump(m_assembler.jCC(x86Condition(cond)));
@@ -205,6 +208,7 @@
DataLabelPtr storePtrWithPatch(TrustedImmPtr initialValue, ImplicitAddress address)
{
+ padBeforePatch();
m_assembler.movl_i32m(initialValue.asIntptr(), address.offset, address.base);
return DataLabelPtr(this);
}
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h (126213 => 126214)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2012-08-21 23:30:19 UTC (rev 126214)
@@ -485,12 +485,14 @@
DataLabel32 load32WithAddressOffsetPatch(Address address, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movl_mr_disp32(address.offset, address.base, dest);
return DataLabel32(this);
}
DataLabelCompact load32WithCompactAddressOffsetPatch(Address address, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movl_mr_disp8(address.offset, address.base, dest);
return DataLabelCompact(this);
}
@@ -503,6 +505,7 @@
DataLabelCompact loadCompactWithAddressOffsetPatch(Address address, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movl_mr_disp8(address.offset, address.base, dest);
return DataLabelCompact(this);
}
@@ -549,6 +552,7 @@
DataLabel32 store32WithAddressOffsetPatch(RegisterID src, Address address)
{
+ padBeforePatch();
m_assembler.movl_rm_disp32(src, address.offset, address.base);
return DataLabel32(this);
}
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h (126213 => 126214)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h 2012-08-21 23:30:19 UTC (rev 126214)
@@ -304,12 +304,14 @@
DataLabel32 loadPtrWithAddressOffsetPatch(Address address, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movq_mr_disp32(address.offset, address.base, dest);
return DataLabel32(this);
}
DataLabelCompact loadPtrWithCompactAddressOffsetPatch(Address address, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movq_mr_disp8(address.offset, address.base, dest);
return DataLabelCompact(this);
}
@@ -348,6 +350,7 @@
DataLabel32 storePtrWithAddressOffsetPatch(RegisterID src, Address address)
{
+ padBeforePatch();
m_assembler.movq_rm_disp32(src, address.offset, address.base);
return DataLabel32(this);
}
@@ -518,6 +521,7 @@
DataLabelPtr moveWithPatch(TrustedImmPtr initialValue, RegisterID dest)
{
+ padBeforePatch();
m_assembler.movq_i64r(initialValue.asIntptr(), dest);
return DataLabelPtr(this);
}
Modified: trunk/Source/_javascript_Core/jit/JumpReplacementWatchpoint.cpp (126213 => 126214)
--- trunk/Source/_javascript_Core/jit/JumpReplacementWatchpoint.cpp 2012-08-21 23:29:21 UTC (rev 126213)
+++ trunk/Source/_javascript_Core/jit/JumpReplacementWatchpoint.cpp 2012-08-21 23:30:19 UTC (rev 126214)
@@ -43,9 +43,11 @@
void JumpReplacementWatchpoint::fireInternal()
{
- MacroAssembler::replaceWithJump(
- CodeLocationLabel(bitwise_cast<void*>(m_source)),
- CodeLocationLabel(bitwise_cast<void*>(m_destination)));
+ void* source = bitwise_cast<void*>(m_source);
+ void* destination = bitwise_cast<void*>(m_destination);
+ if (Options::showDisassembly())
+ dataLog("Firing jump replacement watchpoint from %p, to %p.\n", source, destination);
+ MacroAssembler::replaceWithJump(CodeLocationLabel(source), CodeLocationLabel(destination));
if (isOnList())
remove();
}
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo/webkit-changes
