Changeset 166440 in webkit

Timestamp:

Mar 28, 2014 5:37:10 PM (10 years ago)

Author:

mark.lam@apple.com

Message:

mandreel throws a checksum error on 32-bit x86.
<​https://webkit.org/b/125706>

Reviewed by Filip Pizlo.

The 32-bit DFG can emit code that loads double constants from its
CodeBlock's m_constantRegisters vector. The emitted instruction will
embed the address of the constant from the vector's backing store.
Subsequently, while inserting new constants, the DFG may resize the
vector, thereby reallocating the backing store. This renders the
previously embedded constant addresses stale.

The fix is to use a dedicated doubles constant pool stored in the DFG
CommonData instead. This constant pool won't be reallocated, and
hence will not manifest this issue.

• dfg/DFGCommonData.h:
• dfg/DFGGraph.h:
• dfg/DFGJITCompiler.cpp:

(JSC::DFG::JITCompiler::link):
(JSC::DFG::JITCompiler::addressOfDoubleConstant):

• dfg/DFGJITCompiler.h:

(JSC::DFG::JITCompiler::addressOfDoubleConstant): Deleted.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGCommonData.h (modified) (2 diffs)
• dfg/DFGGraph.h (modified) (1 diff)
• dfg/DFGJITCompiler.cpp (modified) (2 diffs)
• dfg/DFGJITCompiler.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-03-28  Mark Lam  <mark.lam@apple.com>
+
+        mandreel throws a checksum error on 32-bit x86.
+        <https://webkit.org/b/125706>
+
+        Reviewed by Filip Pizlo.
+
+        The 32-bit DFG can emit code that loads double constants from its
+        CodeBlock's m_constantRegisters vector.  The emitted instruction will
+        embed the address of the constant from the vector's backing store.
+        Subsequently, while inserting new constants, the DFG may resize the
+        vector, thereby reallocating the backing store.  This renders the
+        previously embedded constant addresses stale.
+
+        The fix is to use a dedicated doubles constant pool stored in the DFG
+        CommonData instead.  This constant pool won't be reallocated, and
+        hence will not manifest this issue.
+
+        * dfg/DFGCommonData.h:
+        * dfg/DFGGraph.h:
+        * dfg/DFGJITCompiler.cpp:
+        (JSC::DFG::JITCompiler::link):
+        (JSC::DFG::JITCompiler::addressOfDoubleConstant):
+        * dfg/DFGJITCompiler.h:
+        (JSC::DFG::JITCompiler::addressOfDoubleConstant): Deleted.
+
 2014-03-28  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGCommonData.h

@@ -36 +36 @@
 #include "ProfilerCompilation.h"
 #include "SymbolTable.h"
+#include <wtf/Bag.h>
 #include <wtf/Noncopyable.h>
 
@@ -106 +107 @@
     int machineCaptureStart;
     std::unique_ptr<SlowArgument[]> slowArguments;
+
+#if USE(JSVALUE32_64)
+    std::unique_ptr<Bag<double>> doubleConstants;
+#endif
     
     unsigned frameRegisterCount;

trunk/Source/JavaScriptCore/dfg/DFGGraph.h

@@ -847 +847 @@
     int m_machineCaptureStart;
     std::unique_ptr<SlowArgument[]> m_slowArguments;
+
+#if USE(JSVALUE32_64)
+    HashMap<double, double*> m_doubleConstantsMap;
+    std::unique_ptr<Bag<double>> m_doubleConstants;
+#endif
     
     OptimizationFixpointState m_fixpointState;

trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp

@@ -160 +160 @@
     m_jitCode->common.slowArguments = std::move(m_graph.m_slowArguments);
 
+#if USE(JSVALUE32_64)
+    m_jitCode->common.doubleConstants = std::move(m_graph.m_doubleConstants);
+#endif
+
     BitVector usedJumpTables;
     for (Bag<SwitchData>::iterator iter = m_graph.m_switchData.begin(); !!iter; ++iter) {
@@ -433 +437 @@
 }
 
+#if USE(JSVALUE32_64)
+void* JITCompiler::addressOfDoubleConstant(Node* node)
+{
+    ASSERT(m_graph.isNumberConstant(node));
+    JSValue jsvalue = node->valueOfJSConstant(codeBlock());
+    ASSERT(jsvalue.isDouble());
+
+    double value = jsvalue.asDouble();
+    auto it = m_graph.m_doubleConstantsMap.find(value);
+    if (it != m_graph.m_doubleConstantsMap.end())
+        return it->value;
+
+    if (!m_graph.m_doubleConstants)
+        m_graph.m_doubleConstants = std::make_unique<Bag<double>>();
+
+    double* addressInConstantPool = m_graph.m_doubleConstants->add();
+    *addressInConstantPool = value;
+    m_graph.m_doubleConstantsMap.add(value, addressInConstantPool);
+    return addressInConstantPool;
+}
+#endif
+
 } } // namespace JSC::DFG
 

trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.h

@@ -199 +199 @@
 
 #if USE(JSVALUE32_64)
-    void* addressOfDoubleConstant(Node* node)
-    {
-        ASSERT(m_graph.isNumberConstant(node));
-        unsigned constantIndex = node->constantNumber();
-        return &(codeBlock()->constantRegister(FirstConstantRegisterIndex + constantIndex));
-    }
+    void* addressOfDoubleConstant(Node*);
 #endif