Changeset 165119 in webkit for trunk/Source/JavaScriptCore/jsc.cpp

Timestamp:

Mar 5, 2014, 12:26:58 PM (11 years ago)

Author:

[email protected]

Message:

FTL loadStructure always generates invalid IR
​https://bugs.webkit.org/show_bug.cgi?id=129747

Reviewed by Mark Hahnenberg.

As the comment at the top of FTL::Output states, the FTL doesn't use LLVM's notion
of pointers. LLVM's notion of pointers tries to model C, in the sense that you have
to have a pointer to a type, and you can only load things of that type from that
pointer. Pointer arithmetic is basically not possible except through the bizarre
getelementptr operator. This doesn't fit with how the JS object model works since
the JS object model doesn't consist of nice and tidy C types placed in C arrays.
Also, it would be impossible to use getelementptr and LLVM pointers for accessing
any of JSC's C or C++ objects unless we went through the exercise of redeclaring
all of our fundamental data structures in LLVM IR as LLVM types. Clang could do
this for us, but that would require that to use the FTL, JSC itself would have to
be compiled with clang. Worse, it would have to be compiled with a clang that uses
a version of LLVM that is compatible with the one against which the FTL is linked.
Yuck!

The solution is to NEVER use LLVM pointers. This has always been the case in the
FTL. But it causes some confusion.

Not using LLVM pointers means that if the FTL has a "pointer", it's actually a
pointer-wide integer (m_out.intPtr in FTL-speak). The act of "loading" and
"storing" from or to a pointer involves first bitcasting the intPtr to a real LLVM
pointer that has the type that we want. The load and store operations over pointers
are called Output::load* and Output::store*, where * is one of "8", "16", "32",
"64", "Ptr", "Float", or "Double.

There is unavoidable confusion here. It would be bizarre for the FTL to call its
"pointer-wide integers" anything other than "pointers", since they are, in all
respects that we care about, simply pointers. But they are *not* LLVM pointers and
they never will be that.

There is one exception to this "no pointers" rule. The FTL does use actual LLVM
pointers for refering to LLVM alloca's - i.e. local variables. To try to reduce
confusion, we call these "references". So an "FTL reference" is actually an "LLVM
pointer", while an "FTL pointer" is actually an "LLVM integer". FTL references have
methods for access called Output::get and Output::set. These lower to LLVM load
and store, since FTL references are just LLVM pointers.

This confusion appears to have led to incorrect code in loadStructure().
loadStructure() was using get() and set() to access FTL pointers. But those methods
don't work on FTL pointers and never will, since they are for FTL references.

The worst part of this is that it was previously impossible to have test coverage
for the relevant path (MasqueradesAsUndefined) without writing a DRT test. This
patch fixes this by introducing a Masquerader object to jsc.cpp.

• ftl/FTLAbstractHeapRepository.h: Add an abstract heap for the structure table.
• ftl/FTLLowerDFGToLLVM.cpp:

(JSC::FTL::LowerDFGToLLVM::loadStructure): This was wrong.

• ftl/FTLOutput.h: Add a comment to disuade people from using get() and set().
• jsc.cpp: Give us the power to test for MasqueradesAsUndefined.

(WTF::Masquerader::Masquerader):
(WTF::Masquerader::create):
(WTF::Masquerader::createStructure):
(GlobalObject::finishCreation):
(functionMakeMasquerader):

• tests/stress/equals-masquerader.js: Added.

(foo):
(test):

File:

• trunk/Source/JavaScriptCore/jsc.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/jsc.cpp

@@ -95 +95 @@
 class Element;
 class ElementHandleOwner;
+class Masuqerader;
 class Root;
 
@@ -143 +144 @@
 };
 
+class Masquerader : public JSNonFinalObject {
+public:
+    Masquerader(VM& vm, Structure* structure)
+        : Base(vm, structure)
+    {
+    }
+
+    typedef JSNonFinalObject Base;
+
+    static Masquerader* create(VM& vm, JSGlobalObject* globalObject)
+    {
+        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll();
+        Structure* structure = createStructure(vm, globalObject, jsNull());
+        Masquerader* result = new (NotNull, allocateCell<Masquerader>(vm.heap, sizeof(Masquerader))) Masquerader(vm, structure);
+        result->finishCreation(vm);
+        return result;
+    }
+
+    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
+    {
+        return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+    }
+
+    DECLARE_INFO;
+
+protected:
+    static const unsigned StructureFlags = JSC::MasqueradesAsUndefined | Base::StructureFlags;
+};
+
 class Root : public JSDestructibleObject {
 public:
@@ -190 +220 @@
 
 const ClassInfo Element::s_info = { "Element", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Element) };
+const ClassInfo Masquerader::s_info = { "Masquerader", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Masquerader) };
 const ClassInfo Root::s_info = { "Root", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Root) };
 
@@ -240 +271 @@
 static EncodedJSValue JSC_HOST_CALL functionFalse(ExecState*);
 static EncodedJSValue JSC_HOST_CALL functionEffectful42(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionMakeMasquerader(ExecState*);
 
 #if ENABLE(SAMPLING_FLAGS)
@@ -377 +409 @@
         
         addFunction(vm, "effectful42", functionEffectful42, 0);
+        addFunction(vm, "makeMasquerader", functionMakeMasquerader, 0);
         
         JSArray* array = constructEmptyArray(globalExec(), 0);
@@ -746 +779 @@
 {
     return JSValue::encode(jsNumber(42));
+}
+
+EncodedJSValue JSC_HOST_CALL functionMakeMasquerader(ExecState* exec)
+{
+    return JSValue::encode(Masquerader::create(exec->vm(), exec->lexicalGlobalObject()));
 }