Changeset 197491 in webkit

Timestamp:

Mar 2, 2016, 9:29:16 PM (9 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC] JSCell_freeListNext and JSCell_structureID are considered not overlapping
​https://bugs.webkit.org/show_bug.cgi?id=154947

Patch by Benjamin Poulain <​bpoulain@apple.com> on 2016-03-02
Reviewed by Filip Pizlo.

This bug was discovered while testing ​https://bugs.webkit.org/show_bug.cgi?id=154894.

The problem was that JSCell_freeListNext and JSCell_structureID were
considered as disjoint. When reordering instructions, the scheduler
could move the write of the StructureID first to reduce dependencies.
This would erase half of JSCell_freeListNext before we get a chance
to load the value.

This patch changes the hierarchy to make sure nothing is written
until JSCell_freeListNext is processed.

All credits for this patch go to Filip.

• ftl/FTLAbstractHeapRepository.cpp:

(JSC::FTL::AbstractHeapRepository::AbstractHeapRepository):

• ftl/FTLAbstractHeapRepository.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• ftl/FTLAbstractHeapRepository.cpp (modified) (2 diffs)
• ftl/FTLAbstractHeapRepository.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-03-02  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] JSCell_freeListNext and JSCell_structureID are considered not overlapping
+        https://bugs.webkit.org/show_bug.cgi?id=154947
+
+        Reviewed by Filip Pizlo.
+
+        This bug was discovered while testing https://bugs.webkit.org/show_bug.cgi?id=154894.
+
+        The problem was that JSCell_freeListNext and JSCell_structureID were
+        considered as disjoint. When reordering instructions, the scheduler
+        could move the write of the StructureID first to reduce dependencies.
+        This would erase half of JSCell_freeListNext before we get a chance
+        to load the value.
+
+        This patch changes the hierarchy to make sure nothing is written
+        until JSCell_freeListNext is processed.
+
+        All credits for this patch go to Filip.
+
+        * ftl/FTLAbstractHeapRepository.cpp:
+        (JSC::FTL::AbstractHeapRepository::AbstractHeapRepository):
+        * ftl/FTLAbstractHeapRepository.h:
+
 2016-03-02  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp

@@ -58 +58 @@
 #undef ABSTRACT_FIELD_INITIALIZATION
     
-    , JSCell_freeListNext(JSCell_structureID)
+    , JSCell_freeListNext(JSCell_header)
     
 #define INDEXED_ABSTRACT_HEAP_INITIALIZATION(name, offset, size) , name(&root, #name, offset, size)
@@ -76 +76 @@
     RELEASE_ASSERT(JSCell_indexingType.offset() + 3 == JSCell_cellState.offset());
 
+    JSCell_structureID.changeParent(&JSCell_header);
+    JSCell_usefulBytes.changeParent(&JSCell_header);
     JSCell_indexingType.changeParent(&JSCell_usefulBytes);
     JSCell_typeInfoType.changeParent(&JSCell_usefulBytes);

trunk/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h

@@ -55 +55 @@
     macro(JSArrayBufferView_vector, JSArrayBufferView::offsetOfVector()) \
     macro(JSCell_cellState, JSCell::cellStateOffset()) \
+    macro(JSCell_header, 0) \
     macro(JSCell_indexingType, JSCell::indexingTypeOffset()) \
     macro(JSCell_structureID, JSCell::structureIDOffset()) \