From 3a005f22703b9303a306bf34cbd17c3729f763aa Mon Sep 17 00:00:00 2001 From: tsepez Date: Fri, 27 May 2016 17:45:00 -0700 Subject: Workaround dubious casting between CXFA_Object and void* in FXJSE This is just a crock to get things working until we fix the underlying issue. When there's single-inheritance, it may often work in practice to C-style (reinterpret) cast a Derived* ptr to void* and then back to a Base* ptr. One place where this blows up is if Derived has virtual functions but Base does not, in which case the world will be offset by the size of a vtable ptr. Because of the use of void* types in FXJSE, the above was happening when setting a CXFA_ThisProxy (Derived, virtual) to be a global object (void*). This would then be cast back to a CFXA_Object (Base, non-virtual) and chaos is ensured. Not sure how far back this goes. Along the way, pick up some tidying which was necessary for simplicity while tracking this down. BUG=613607 Review-Url: https://codereview.chromium.org/2015143005 --- BUILD.gn | 1 - xfa.gyp | 1 - xfa/fxfa/fm2js/xfa_fm2jscontext.cpp | 3 ++- xfa/fxfa/parser/xfa_object.h | 7 +++-- xfa/fxjse/class.cpp | 1 - xfa/fxjse/context.cpp | 41 ++++++++++++++++++++++++++++- xfa/fxjse/context.h | 9 +++++++ xfa/fxjse/include/fxjse.h | 4 +-- xfa/fxjse/util_inline.h | 51 ------------------------------------- xfa/fxjse/value.cpp | 2 +- 10 files changed, 59 insertions(+), 61 deletions(-) delete mode 100644 xfa/fxjse/util_inline.h diff --git a/BUILD.gn b/BUILD.gn index 1550209ce1..580bc573d1 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1578,7 +1578,6 @@ if (pdf_enable_xfa) { "xfa/fxjse/runtime.cpp", "xfa/fxjse/runtime.h", "xfa/fxjse/scope_inline.h", - "xfa/fxjse/util_inline.h", "xfa/fxjse/value.cpp", "xfa/fxjse/value.h", ] diff --git a/xfa.gyp b/xfa.gyp index 14a813096b..82b88c9591 100644 --- a/xfa.gyp +++ b/xfa.gyp @@ -722,7 +722,6 @@ "xfa/fxjse/runtime.cpp", "xfa/fxjse/runtime.h", "xfa/fxjse/scope_inline.h", - "xfa/fxjse/util_inline.h", "xfa/fxjse/value.cpp", "xfa/fxjse/value.h" ], diff --git a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp index 40d8bcf5f4..2c8a362807 100644 --- a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp +++ b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp @@ -3347,7 +3347,8 @@ void CXFA_FM2JSContext::Eval(CFXJSE_Value* pThis, XFA_FM2JS_Translate( CFX_WideString::FromUTF8(utf8ScriptString.AsStringC()).AsStringC(), wsJavaScriptBuf, wsError); - CFXJSE_Context* pContext = FXJSE_Context_Create(pIsolate); + CFXJSE_Context* pContext = + FXJSE_Context_Create(pIsolate, nullptr, nullptr); CFXJSE_Value* returnValue = FXJSE_Value_Create(pIsolate); javaScript = wsJavaScriptBuf.AsStringC(); FXJSE_ExecuteScript( diff --git a/xfa/fxfa/parser/xfa_object.h b/xfa/fxfa/parser/xfa_object.h index 28d4712e46..8fc074c08f 100644 --- a/xfa/fxfa/parser/xfa_object.h +++ b/xfa/fxfa/parser/xfa_object.h @@ -41,9 +41,12 @@ enum XFA_OBJECTTYPE { XFA_NODEFLAG_UnusedNode = 0x08000, XFA_NODEFLAG_LayoutGeneratedNode = 0x10000, }; + class CXFA_Object { public: CXFA_Object(CXFA_Document* pDocument, uint32_t uFlags); + virtual ~CXFA_Object() {} + CXFA_Document* GetDocument() const { return m_pDocument; } uint32_t GetFlag() const { return m_uFlags; } XFA_OBJECTTYPE GetObjectType() const { @@ -590,7 +593,7 @@ class CXFA_Node : public CXFA_Object { protected: CXFA_Node(CXFA_Document* pDoc, uint16_t ePacket, XFA_ELEMENT eElement); - ~CXFA_Node(); + ~CXFA_Node() override; friend class CXFA_Document; CXFA_Node* Deprecated_GetPrevSibling(); FX_BOOL SetValue(XFA_ATTRIBUTE eAttr, @@ -669,7 +672,7 @@ class CXFA_ThisProxy : public CXFA_Object { m_pThisNode = pThisNode; m_pScriptNode = pScriptNode; } - virtual ~CXFA_ThisProxy() {} + ~CXFA_ThisProxy() override {} CXFA_Node* GetThisNode() { return m_pThisNode; } CXFA_Node* GetScriptNode() { return m_pScriptNode; } diff --git a/xfa/fxjse/class.cpp b/xfa/fxjse/class.cpp index dcc370306a..d49bafb986 100644 --- a/xfa/fxjse/class.cpp +++ b/xfa/fxjse/class.cpp @@ -9,7 +9,6 @@ #include "xfa/fxjse/cfxjse_arguments.h" #include "xfa/fxjse/context.h" #include "xfa/fxjse/scope_inline.h" -#include "xfa/fxjse/util_inline.h" #include "xfa/fxjse/value.h" static void FXJSE_V8ConstructorCallback_Wrapper( diff --git a/xfa/fxjse/context.cpp b/xfa/fxjse/context.cpp index 333b2abe57..49d0b44338 100644 --- a/xfa/fxjse/context.cpp +++ b/xfa/fxjse/context.cpp @@ -8,9 +8,48 @@ #include "xfa/fxjse/class.h" #include "xfa/fxjse/scope_inline.h" -#include "xfa/fxjse/util_inline.h" #include "xfa/fxjse/value.h" +v8::Local FXJSE_GetGlobalObjectFromContext( + const v8::Local& hContext) { + return hContext->Global()->GetPrototype().As(); +} + +void FXJSE_UpdateObjectBinding(v8::Local& hObject, + void* lpNewBinding) { + ASSERT(!hObject.IsEmpty()); + ASSERT(hObject->InternalFieldCount() > 0); + hObject->SetAlignedPointerInInternalField(0, lpNewBinding); +} + +void* FXJSE_RetrieveObjectBinding(const v8::Local& hJSObject, + CFXJSE_Class* lpClass) { + ASSERT(!hJSObject.IsEmpty()); + if (!hJSObject->IsObject()) { + return NULL; + } + v8::Local hObject = hJSObject; + if (hObject->InternalFieldCount() == 0) { + v8::Local hProtoObject = hObject->GetPrototype(); + if (hProtoObject.IsEmpty() || !hProtoObject->IsObject()) { + return NULL; + } + hObject = hProtoObject.As(); + if (hObject->InternalFieldCount() == 0) { + return NULL; + } + } + if (lpClass) { + v8::Local hClass = + v8::Local::New( + lpClass->GetContext()->GetRuntime(), lpClass->GetTemplate()); + if (!hClass->HasInstance(hObject)) { + return NULL; + } + } + return hObject->GetAlignedPointerFromInternalField(0); +} + CFXJSE_Context* FXJSE_Context_Create( v8::Isolate* pIsolate, const FXJSE_CLASS_DESCRIPTOR* lpGlobalClass, diff --git a/xfa/fxjse/context.h b/xfa/fxjse/context.h index 96b93faffd..79e5e0a740 100644 --- a/xfa/fxjse/context.h +++ b/xfa/fxjse/context.h @@ -50,4 +50,13 @@ class CFXJSE_Context { v8::Local FXJSE_CreateReturnValue(v8::Isolate* pIsolate, v8::TryCatch& trycatch); +v8::Local FXJSE_GetGlobalObjectFromContext( + const v8::Local& hContext); + +void FXJSE_UpdateObjectBinding(v8::Local& hObject, + void* lpNewBinding = nullptr); + +void* FXJSE_RetrieveObjectBinding(const v8::Local& hJSObject, + CFXJSE_Class* lpClass = nullptr); + #endif // XFA_FXJSE_CONTEXT_H_ diff --git a/xfa/fxjse/include/fxjse.h b/xfa/fxjse/include/fxjse.h index d2d0bd9014..e278935657 100644 --- a/xfa/fxjse/include/fxjse.h +++ b/xfa/fxjse/include/fxjse.h @@ -72,8 +72,8 @@ void FXJSE_Runtime_Release(v8::Isolate* pIsolate, bool bOwnedRuntime); CFXJSE_Context* FXJSE_Context_Create( v8::Isolate* pIsolate, - const FXJSE_CLASS_DESCRIPTOR* lpGlobalClass = nullptr, - void* lpGlobalObject = nullptr); + const FXJSE_CLASS_DESCRIPTOR* lpGlobalClass, + void* lpGlobalObject); void FXJSE_Context_Release(CFXJSE_Context* pContext); CFXJSE_Value* FXJSE_Context_GetGlobalObject(CFXJSE_Context* pContext); diff --git a/xfa/fxjse/util_inline.h b/xfa/fxjse/util_inline.h deleted file mode 100644 index e61dc6aa80..0000000000 --- a/xfa/fxjse/util_inline.h +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2014 PDFium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com - -#ifndef XFA_FXJSE_UTIL_INLINE_H_ -#define XFA_FXJSE_UTIL_INLINE_H_ - -#include "xfa/fxjse/context.h" - -static V8_INLINE v8::Local FXJSE_GetGlobalObjectFromContext( - const v8::Local& hContext) { - return hContext->Global()->GetPrototype().As(); -} -static V8_INLINE void FXJSE_UpdateObjectBinding(v8::Local& hObject, - void* lpNewBinding) { - ASSERT(!hObject.IsEmpty()); - ASSERT(hObject->InternalFieldCount() > 0); - hObject->SetAlignedPointerInInternalField(0, lpNewBinding); -} -static V8_INLINE void* FXJSE_RetrieveObjectBinding( - const v8::Local& hJSObject, - CFXJSE_Class* lpClass = NULL) { - ASSERT(!hJSObject.IsEmpty()); - if (!hJSObject->IsObject()) { - return NULL; - } - v8::Local hObject = hJSObject; - if (hObject->InternalFieldCount() == 0) { - v8::Local hProtoObject = hObject->GetPrototype(); - if (hProtoObject.IsEmpty() || !hProtoObject->IsObject()) { - return NULL; - } - hObject = hProtoObject.As(); - if (hObject->InternalFieldCount() == 0) { - return NULL; - } - } - if (lpClass) { - v8::Local hClass = - v8::Local::New( - lpClass->GetContext()->GetRuntime(), lpClass->GetTemplate()); - if (!hClass->HasInstance(hObject)) { - return NULL; - } - } - return hObject->GetAlignedPointerFromInternalField(0); -} - -#endif // XFA_FXJSE_UTIL_INLINE_H_ diff --git a/xfa/fxjse/value.cpp b/xfa/fxjse/value.cpp index e7a3463f28..ecd88b078d 100644 --- a/xfa/fxjse/value.cpp +++ b/xfa/fxjse/value.cpp @@ -9,7 +9,7 @@ #include #include "xfa/fxjse/class.h" -#include "xfa/fxjse/util_inline.h" +#include "xfa/fxjse/context.h" FX_BOOL FXJSE_Value_IsUndefined(CFXJSE_Value* pValue) { return pValue && pValue->IsUndefined(); -- cgit v1.2.3