From 5bf941c2ac2bac92c31ff55f5fffa74f92269aaf Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Tue, 23 Jan 2018 15:37:02 -0800 Subject: [PATCH] deps: update ChakraCore to Microsoft/ChakraCore@2abdaab754 [MERGE #4591 @kfarnung] Fixing TTD regressions from JsObject* function refactor Merge pull request #4591 from kfarnung:ttdobjects When the JsObject* functions were added to JSRT there was an attempt to refactor the TTD instrumentation into a common method. The refactor caused a regression in cross-context scenarios where the record was capturing both the result of the function as well as any marshalling necessary to get the result. This change reverts the behavior of the existing methods (e.g. JsSetProperty and friends) to capture the record before any marshalling can occur (VALIDATE_INCOMING_OBJECT will marshal the object if necessary). For the new JsObject* functions I've added an assert to ensure that once they are used they will cause TTD record to fail with an actionable message. Reviewed-By: chakrabot --- deps/chakrashim/core/lib/Jsrt/Jsrt.cpp | 91 +++++++++++++------------- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/deps/chakrashim/core/lib/Jsrt/Jsrt.cpp b/deps/chakrashim/core/lib/Jsrt/Jsrt.cpp index d42212147c8..bc01e9ebbde 100644 --- a/deps/chakrashim/core/lib/Jsrt/Jsrt.cpp +++ b/deps/chakrashim/core/lib/Jsrt/Jsrt.cpp @@ -1422,11 +1422,8 @@ CHAKRA_API JsPreventExtension(_In_ JsValueRef object) } CHAKRA_API JsHasOwnPropertyCommon(Js::ScriptContext * scriptContext, _In_ JsValueRef object, - _In_ const Js::PropertyRecord * propertyRecord, _Out_ bool *hasOwnProperty, - TTDRecorder& _actionEntryPopper) + _In_ const Js::PropertyRecord * propertyRecord, _Out_ bool *hasOwnProperty) { - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTHasOwnProperty, propertyRecord, object); - *hasOwnProperty = Js::JavascriptOperators::OP_HasOwnProperty(object, propertyRecord->GetPropertyId(), scriptContext) != 0; @@ -1438,6 +1435,7 @@ CHAKRA_API JsHasOwnProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTHasOwnProperty, (const Js::PropertyRecord *)propertyId, object); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_PROPERTYID(propertyId); @@ -1445,7 +1443,7 @@ CHAKRA_API JsHasOwnProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert *hasOwnProperty = false; return JsHasOwnPropertyCommon(scriptContext, object, - (const Js::PropertyRecord *)propertyId, hasOwnProperty, _actionEntryPopper); + (const Js::PropertyRecord *)propertyId, hasOwnProperty); }); } @@ -1476,6 +1474,7 @@ CHAKRA_API JsObjectHasOwnProperty(_In_ JsValueRef object, _In_ JsValueRef proper { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); @@ -1491,24 +1490,20 @@ CHAKRA_API JsObjectHasOwnProperty(_In_ JsValueRef object, _In_ JsValueRef proper return errorValue; } - return JsHasOwnPropertyCommon(scriptContext, object, propertyRecord, hasOwnProperty, _actionEntryPopper); + return JsHasOwnPropertyCommon(scriptContext, object, propertyRecord, hasOwnProperty); }); } #endif static JsErrorCode JsGetPropertyCommon(Js::ScriptContext * scriptContext, _In_ Js::RecyclableObject * object, - _In_ const Js::PropertyRecord * propertyRecord, _Out_ JsValueRef *value, - TTDRecorder& _actionEntryPopper) + _In_ const Js::PropertyRecord * propertyRecord, _Out_ JsValueRef *value) { AssertMsg(scriptContext->GetThreadContext()->IsScriptActive(), "Caller is expected to be under ContextAPIWrapper!"); - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTGetProperty, propertyRecord, object); *value = Js::JavascriptOperators::GetPropertyNoCache(object, propertyRecord->GetPropertyId(), scriptContext); Assert(*value == nullptr || !Js::CrossSite::NeedMarshalVar(*value, scriptContext)); - PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, value); - return JsNoError; } @@ -1516,6 +1511,7 @@ CHAKRA_API JsGetProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propertyId { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTGetProperty, (const Js::PropertyRecord *)propertyId, object); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_PROPERTYID(propertyId); @@ -1523,8 +1519,12 @@ CHAKRA_API JsGetProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propertyId *value = nullptr; Js::RecyclableObject * instance = Js::RecyclableObject::FromVar(object); - return JsGetPropertyCommon(scriptContext, instance, (const Js::PropertyRecord *)propertyId, - value, _actionEntryPopper); + JsErrorCode err = JsGetPropertyCommon(scriptContext, instance, (const Js::PropertyRecord *)propertyId, + value); + + PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, value); + + return err; }); } @@ -1533,6 +1533,7 @@ CHAKRA_API JsObjectGetProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); @@ -1551,17 +1552,15 @@ CHAKRA_API JsObjectGetProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI Assert(propertyRecord != nullptr); Js::RecyclableObject * instance = Js::RecyclableObject::FromVar(object); - return JsGetPropertyCommon(scriptContext, instance, propertyRecord, value, _actionEntryPopper); + return JsGetPropertyCommon(scriptContext, instance, propertyRecord, value); }); } #endif static JsErrorCode JsGetOwnPropertyDescriptorCommon(Js::ScriptContext * scriptContext, - _In_ JsValueRef object, _In_ const Js::PropertyRecord * propertyRecord, _Out_ JsValueRef *propertyDescriptor, - TTDRecorder& _actionEntryPopper) + _In_ JsValueRef object, _In_ const Js::PropertyRecord * propertyRecord, _Out_ JsValueRef *propertyDescriptor) { AssertMsg(scriptContext->GetThreadContext()->IsScriptActive(), "Caller is expected to be under ContextAPIWrapper!"); - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTGetOwnPropertyInfo, propertyRecord, object); Js::PropertyDescriptor propertyDescriptorValue; if (Js::JavascriptOperators::GetOwnPropertyDescriptor(Js::RecyclableObject::FromVar(object), @@ -1575,21 +1574,25 @@ static JsErrorCode JsGetOwnPropertyDescriptorCommon(Js::ScriptContext * scriptCo } Assert(*propertyDescriptor == nullptr || !Js::CrossSite::NeedMarshalVar(*propertyDescriptor, scriptContext)); - PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, propertyDescriptor); - return JsNoError; } CHAKRA_API JsGetOwnPropertyDescriptor(_In_ JsValueRef object, _In_ JsPropertyIdRef propertyId, _Out_ JsValueRef *propertyDescriptor) { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTGetOwnPropertyInfo, (const Js::PropertyRecord *)propertyId, object); + VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_PROPERTYID(propertyId); PARAM_NOT_NULL(propertyDescriptor); *propertyDescriptor = nullptr; - return JsGetOwnPropertyDescriptorCommon(scriptContext, object, (const Js::PropertyRecord *)propertyId, - propertyDescriptor, _actionEntryPopper); + JsErrorCode err = JsGetOwnPropertyDescriptorCommon(scriptContext, object, (const Js::PropertyRecord *)propertyId, + propertyDescriptor); + + PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, propertyDescriptor); + + return err; }); } @@ -1598,6 +1601,7 @@ CHAKRA_API JsObjectGetOwnPropertyDescriptor(_In_ JsValueRef object, _In_ JsValue { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); @@ -1615,18 +1619,15 @@ CHAKRA_API JsObjectGetOwnPropertyDescriptor(_In_ JsValueRef object, _In_ JsValue Assert(propertyRecord != nullptr); - return JsGetOwnPropertyDescriptorCommon(scriptContext, object, propertyRecord, propertyDescriptor, _actionEntryPopper); + return JsGetOwnPropertyDescriptorCommon(scriptContext, object, propertyRecord, propertyDescriptor); }); } #endif static JsErrorCode JsSetPropertyCommon(Js::ScriptContext * scriptContext, _In_ JsValueRef object, - _In_ const Js::PropertyRecord * propertyRecord, _In_ JsValueRef value, _In_ bool useStrictRules, - TTDRecorder& _actionEntryPopper) + _In_ const Js::PropertyRecord * propertyRecord, _In_ JsValueRef value, _In_ bool useStrictRules) { AssertMsg(scriptContext->GetThreadContext()->IsScriptActive(), "Caller is expected to be under ContextAPIWrapper!"); - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTSetProperty, object, - propertyRecord, value, useStrictRules); Js::JavascriptOperators::OP_SetProperty(object, propertyRecord->GetPropertyId(), value, scriptContext, nullptr, useStrictRules ? Js::PropertyOperation_StrictMode : Js::PropertyOperation_None); @@ -1638,13 +1639,14 @@ CHAKRA_API JsSetProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propertyId { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTSetProperty, object, (const Js::PropertyRecord *)propertyId, value, useStrictRules); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_PROPERTYID(propertyId); VALIDATE_INCOMING_REFERENCE(value, scriptContext); return JsSetPropertyCommon(scriptContext, object, (const Js::PropertyRecord *)propertyId, - value, useStrictRules, _actionEntryPopper); + value, useStrictRules); }); } @@ -1653,6 +1655,7 @@ CHAKRA_API JsObjectSetProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); @@ -1669,7 +1672,7 @@ CHAKRA_API JsObjectSetProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI Assert(propertyRecord != nullptr); - return JsSetPropertyCommon(scriptContext, object, propertyRecord, value, useStrictRules, _actionEntryPopper); + return JsSetPropertyCommon(scriptContext, object, propertyRecord, value, useStrictRules); }); } #endif @@ -1718,6 +1721,7 @@ CHAKRA_API JsObjectHasProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI if (!Js::JavascriptOperators::IsObject(object)) return JsErrorArgumentNotObject; auto internalHasProperty = [&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); PARAM_NOT_NULL(hasProperty); @@ -1732,8 +1736,6 @@ CHAKRA_API JsObjectHasProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI return errorValue; } - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTHasProperty, propertyRecord, object); - Js::RecyclableObject * instance = Js::RecyclableObject::FromVar(object); *hasProperty = Js::JavascriptOperators::HasProperty(instance, propertyRecord->GetPropertyId()) != 0; @@ -1760,12 +1762,9 @@ CHAKRA_API JsObjectHasProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI #endif static JsErrorCode JsDeletePropertyCommon(Js::ScriptContext * scriptContext, _In_ JsValueRef object, - _In_ const Js::PropertyRecord * propertyRecord, _In_ bool useStrictRules, _Out_ JsValueRef *result, - TTDRecorder& _actionEntryPopper) + _In_ const Js::PropertyRecord * propertyRecord, _In_ bool useStrictRules, _Out_ JsValueRef *result) { AssertMsg(scriptContext->GetThreadContext()->IsScriptActive(), "Caller is expected to be under ContextAPIWrapper!"); - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTDeleteProperty, object, - propertyRecord, useStrictRules); *result = Js::JavascriptOperators::OP_DeleteProperty((Js::Var)object, propertyRecord->GetPropertyId(), @@ -1773,8 +1772,6 @@ static JsErrorCode JsDeletePropertyCommon(Js::ScriptContext * scriptContext, _In Assert(*result == nullptr || !Js::CrossSite::NeedMarshalVar(*result, scriptContext)); - PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, result); - return JsNoError; } @@ -1783,14 +1780,19 @@ CHAKRA_API JsDeleteProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTDeleteProperty, object, (const Js::PropertyRecord *)propertyId, useStrictRules); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_PROPERTYID(propertyId); PARAM_NOT_NULL(result); *result = nullptr; - return JsDeletePropertyCommon(scriptContext, object, (const Js::PropertyRecord *)propertyId, - useStrictRules, result, _actionEntryPopper); + JsErrorCode err = JsDeletePropertyCommon(scriptContext, object, (const Js::PropertyRecord *)propertyId, + useStrictRules, result); + + PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, result); + + return err; }); } @@ -1800,6 +1802,7 @@ CHAKRA_API JsObjectDeleteProperty(_In_ JsValueRef object, _In_ JsValueRef proper { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); @@ -1818,18 +1821,16 @@ CHAKRA_API JsObjectDeleteProperty(_In_ JsValueRef object, _In_ JsValueRef proper Assert(propertyRecord != nullptr); return JsDeletePropertyCommon(scriptContext, object, propertyRecord, - useStrictRules, result, _actionEntryPopper); + useStrictRules, result); }); } #endif static JsErrorCode JsDefinePropertyCommon(Js::ScriptContext * scriptContext, _In_ JsValueRef object, _In_ const Js::PropertyRecord *propertyRecord, _In_ JsValueRef propertyDescriptor, - _Out_ bool *result, TTDRecorder& _actionEntryPopper) + _Out_ bool *result) { AssertMsg(scriptContext->GetThreadContext()->IsScriptActive(), "Caller is expected to be under ContextAPIWrapper!"); - PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTDefineProperty, object, - propertyRecord, propertyDescriptor); Js::PropertyDescriptor propertyDescriptorValue; if (!Js::JavascriptOperators::ToPropertyDescriptor(propertyDescriptor, &propertyDescriptorValue, scriptContext)) @@ -1849,6 +1850,7 @@ CHAKRA_API JsDefineProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTDefineProperty, object, (const Js::PropertyRecord *)propertyId, propertyDescriptor); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_PROPERTYID(propertyId); @@ -1857,7 +1859,7 @@ CHAKRA_API JsDefineProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert *result = false; return JsDefinePropertyCommon(scriptContext, object, (const Js::PropertyRecord *)propertyId, - propertyDescriptor, result, _actionEntryPopper); + propertyDescriptor, result); }); } @@ -1867,6 +1869,7 @@ CHAKRA_API JsObjectDefineProperty(_In_ JsValueRef object, _In_ JsValueRef proper { return ContextAPIWrapper([&] (Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode { + PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext); VALIDATE_INCOMING_OBJECT(object, scriptContext); VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext); @@ -1883,7 +1886,7 @@ CHAKRA_API JsObjectDefineProperty(_In_ JsValueRef object, _In_ JsValueRef proper return errorValue; } - return JsDefinePropertyCommon(scriptContext, object, propertyRecord, propertyDescriptor, result, _actionEntryPopper); + return JsDefinePropertyCommon(scriptContext, object, propertyRecord, propertyDescriptor, result); }); } #endif