[JSC] B3 specializeSelect must not clone Phis

An FTL optimisation handed plain JavaScript a JSValue nobody ever initialised.

Component: JSC B3 | 6dd325c

Source/JavaScriptCore/b3/B3SpecializeSelect.cpp

         for (unsigned i = selectIndex; i <= m_index; ++i) {
-            if (m_block->at(i)->kind().isCloningForbidden())
+            Value* value = m_block->at(i);
+            if (value->kind().isCloningForbidden())
+                return nullptr;
+
+            // Phi는 children이 아니라 Upsilon이 기록하는 slot을 읽으므로, children을 remap해도
+            // 복제본의 연결을 바로잡을 수 없습니다. 복제본은 아무도 기록하지 않는 slot을 읽게 됩니다.
+            // Upsilon은 복제해도 무방합니다. 각 arm이 원래 Phi에 기록하기 때문입니다.
+            if (value->opcode() == Phi)
                 return nullptr;
         }
...
         auto cloneValue = [&] (Value* value) {
             ASSERT(value != source);
+            ASSERT(value->opcode() != Phi);

B3는 FTL이 사용하는 mid-level IR입니다. 다만 고전적인 SSA 형태는 아닙니다. Phi에는 remap할 children이 없고, 대신 Upsilon 노드가 기록하는 local-state slot을 읽습니다. 이 slot은 shadow variable이며, 반드시 machine stack slot을 의미하지는 않습니다. Upsilon은 predecessor block에 있을 수도, 같은 block 안에 있을 수도 있습니다.

specializeSelect는 Check로 이어지는 Select를 branch로 변환하고, 그 사이에 있는 value들을 각 arm에 복제합니다. 이때 복제된 value의 children은 arm별 operand로 remap됩니다. 한편 Check 노드는 exit state(ColdAny rep constraint)를 갖고 있으며, 이 상태는 OSR exit 시점에 materialize됩니다.

일반 value라면 복제해도 문제가 없지만, Phi는 사정이 다릅니다. 복제된 Phi는 아무도 기록하지 않는 새 slot을 읽게 되기 때문입니다. 이 commit에서는 Select와 Check 사이에 Phi가 있으면 specializeSelect가 변환을 포기하도록 수정되었습니다. 또한 이 invariant를 보장하기 위해 cloneValue에 ASSERT가 추가되었습니다. Check 앞에 Phi와 Upsilon이 배치되는 경우는 JSTests stress test 하나와 testb3 case 두 개로 검증합니다.

Before (Phi cloned in each arm):          After (Phi blocks the transform):

BB#6 (true arm)                           Select stays as Select
  Upsilon($7, ^11)   <- correct             Check(...) sees original b@11
  b@64 = Phi()       <- clone, slot         with its real Upsilon writers
                        never written
  Upsilon(b@64, ^11) <- overwrites 7
                        with garbage
  Check(..., b@64:ColdAny)
  Jump #8
        |
        v
BB#8
  b@11 = Phi()  <- garbage from both arms
  Patchpoint(...)/CCall -> deref as cell

Significance

기존 동작에서는 어떤 Upsilon도 기록하지 않는 orphan Phi가 생성되었습니다. 그 결과 FTL로 컴파일된 JS 값이 초기화되지 않은 JSValue가 되고, 이후 cell로 dereference되었습니다. 일반 JavaScript만으로 도달 가능한 JIT miscompile입니다. 이 garbage 값은 operationValueAddProfiled까지 전달되어 cell로 취급됩니다. 이런 유형의 read는 slot에 어떤 값이 들어 있었는지에 따라 type confusion이나 arbitrary-pointer dereference로 이어질 가능성이 있습니다.

Audit directions

이번 fix는 cloning 로직을 다시 작성한 것이 아니라, 보수적으로 bail out하는 방식입니다. 그래서 주변 코드를 함께 점검할 필요가 있습니다.

좁은 범위에서는 Select에서 Check까지의 구간에 들어올 수 있는 다른 value kind를 살펴봐야 합니다. 특히 semantics가 children 이외의 요소에 의존하는 kind가 대상입니다. stackmap constraint를 갖는 Patchpoint/CCall, Check의 exit-state operand, memory effect를 갖는 value 등이 여기에 해당합니다. 이들이 isCloningForbidden에 포함되는지 확인해야 합니다. 한편 Upsilon은 의도적으로 계속 복제됩니다. 각 arm이 원래 Phi에 기록하기 때문이며, 이 경우는 testCheckSelectWithUpsilonBeforeCheck로 검증됩니다. 따라서 commit이 다루지 않는 kind에 집중하는 편이 효율적입니다.

넓은 범위에서는 tail duplication처럼 value나 block을 복제하는 다른 B3 phase를 살펴볼 필요가 있습니다. 같은 Phi-slot 가정이 숨어 있을 수 있기 때문입니다. children만 remap하고 그 외에는 아무것도 처리하지 않는 clone helper가 있다면, 그것이 의심 신호입니다.

Fuzzing을 한다면 FTL이 Select와 Check 사이에 loop-carried Phi를 생성하도록 유도하는 JS를 만들어야 합니다. 이때 함수에 대한 !g, boolean에 대한 v < 100처럼 type profile이 어긋나는 패턴을 활용할 수 있습니다.