Changeset 289354 in webkit for trunk/Source/JavaScriptCore/b3

Timestamp:

Feb 7, 2022, 7:00:28 PM (3 years ago)

Author:

[email protected]

Message:

Wasm crash on ​https://copy.sh/v86/?profile=dsl
​https://bugs.webkit.org/show_bug.cgi?id=236037
rdar://88358719

Reviewed by Mark Lam.

Lower stack args in Air had a bug where it was emitting a constant
materialization at the wrong instruction offset for certain types
of spill instructions. This happens when we have a stack slot that
is 8 bytes wide, but we're emitting a zero def Move32. We need to
zero the upper 4 bytes. However, there is also code inside lower
stack args that uses the temp register when encountering offsets
that are too large to encode in a single instruction. However,
this offset materialization code for the second Move32 to zero
the upper bytes was happening before the actual store. For example,
we'd end up with:
movz x16, #k
movz x16, #k2
stur x1, [x16]
stur zr, [x16]

instead of
movz x16, #k
stur x1, [x16]
movz x16, #k2
stur zr, [x16]

• b3/air/AirLowerStackArgs.cpp:

(JSC::B3::Air::lowerStackArgs):

• b3/air/testair.cpp:

Location:

trunk/Source/JavaScriptCore/b3/air

Files:

• AirLowerStackArgs.cpp (modified) (3 diffs)
• testair.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/b3/air/AirLowerStackArgs.cpp

@@ -111 +111 @@
             inst.forEachArg(
                 [&] (Arg& arg, Arg::Role role, Bank, Width width) {
-                    auto stackAddr = [&] (Value::OffsetType offsetFromFP) -> Arg {
+                    auto stackAddr = [&] (unsigned instIndex, Value::OffsetType offsetFromFP) -> Arg {
                         int32_t offsetFromSP = offsetFromFP + code.frameSize();
 
@@ -138 +138 @@
                         return result;
 #elif CPU(X86_64)
+                        UNUSED_PARAM(instIndex);
                         // Can't happen on x86: immediates are always big enough for frame size.
                         RELEASE_ASSERT_NOT_REACHED();
@@ -172 +173 @@
                             insertionSet.insert(
                                 instIndex + 1, storeOpcode, inst.origin, operand,
-                                stackAddr(arg.offset() + 4 + slot->offsetFromFP()));
+                                stackAddr(instIndex + 1, arg.offset() + 4 + slot->offsetFromFP()));
                         }
-                        arg = stackAddr(arg.offset() + slot->offsetFromFP());
+                        arg = stackAddr(instIndex, arg.offset() + slot->offsetFromFP());
                         break;
                     }
                     case Arg::CallArg:
-                        arg = stackAddr(arg.offset() - code.frameSize());
+                        arg = stackAddr(instIndex, arg.offset() - code.frameSize());
                         break;
                     default:

trunk/Source/JavaScriptCore/b3/air/testair.cpp

@@ -2359 +2359 @@
     auto runResult = compileAndRun<uint32_t>(proc);
     CHECK(runResult == 99);
+}
+
+void testZDefOfSpillSlotWithOffsetNeedingToBeMaterializedInARegister()
+{
+    if (Options::defaultB3OptLevel() == 2)
+        return;
+
+    B3::Procedure proc;
+    Code& code = proc.code();
+
+    BasicBlock* root = code.addBlock();
+
+    Vector<StackSlot*> slots;
+    for (unsigned i = 0; i < 10000; ++i)
+        slots.append(code.addStackSlot(8, StackSlotKind::Spill));
+
+    for (auto* slot : slots)
+        root->append(Move32, nullptr, Tmp(GPRInfo::argumentGPR0), Arg::stack(slot));
+
+    Tmp loadResult = code.newTmp(GP);
+    Tmp sum = code.newTmp(GP);
+    root->append(Move, nullptr, Arg::imm(0), sum);
+    for (auto* slot : slots) {
+        root->append(Move, nullptr, Arg::stack(slot), loadResult);
+        root->append(Add64, nullptr, loadResult, sum);
+    }
+    root->append(Move, nullptr, sum, Tmp(GPRInfo::returnValueGPR));
+    root->append(Ret64, nullptr, Tmp(GPRInfo::returnValueGPR));
+
+    int32_t result = compileAndRun<int>(proc, 1);
+    CHECK(result == 10000);
 }
 
@@ -2456 +2487 @@
     RUN(testLinearScanSpillRangesEarlyDef());
 
+    RUN(testZDefOfSpillSlotWithOffsetNeedingToBeMaterializedInARegister());
+
     if (tasks.isEmpty())
         usage();