Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
deps: update ChakraCore to chakra-core/ChakraCore@2abdaab754
Browse files Browse the repository at this point in the history 
[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 <[email protected]>
  • Loading branch information
@kfarnung
kfarnung committed Jan 23, 2018
1 parent 7dbd18b commit 5bf941c
Showing 1 changed file with 47 additions and 44 deletions.
91 changes: 47 additions & 44 deletions deps/chakrashim/core/lib/Jsrt/Jsrt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -1438,14 +1435,15 @@ CHAKRA_API JsHasOwnProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert
{
return ContextAPIWrapper<true>([&] (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);
PARAM_NOT_NULL(hasOwnProperty);
*hasOwnProperty = false;

return JsHasOwnPropertyCommon(scriptContext, object,
(const Js::PropertyRecord *)propertyId, hasOwnProperty, _actionEntryPopper);
(const Js::PropertyRecord *)propertyId, hasOwnProperty);
});
}

Expand Down Expand Up @@ -1476,6 +1474,7 @@ CHAKRA_API JsObjectHasOwnProperty(_In_ JsValueRef object, _In_ JsValueRef proper
{
return ContextAPIWrapper<true>([&] (Js::ScriptContext *scriptContext,
TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

VALIDATE_INCOMING_OBJECT(object, scriptContext);
VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext);
Expand All @@ -1491,40 +1490,41 @@ 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;
}

CHAKRA_API JsGetProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propertyId, _Out_ JsValueRef *value)
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (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);
PARAM_NOT_NULL(value);
*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;
});
}

Expand All @@ -1533,6 +1533,7 @@ CHAKRA_API JsObjectGetProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (Js::ScriptContext *scriptContext,
TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

VALIDATE_INCOMING_OBJECT(object, scriptContext);
VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext);
Expand All @@ -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),
Expand All @@ -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<JSRT_MAYBE_TRUE>([&] (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;
});
}

Expand All @@ -1598,6 +1601,7 @@ CHAKRA_API JsObjectGetOwnPropertyDescriptor(_In_ JsValueRef object, _In_ JsValue
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (Js::ScriptContext *scriptContext,
TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

VALIDATE_INCOMING_OBJECT(object, scriptContext);
VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext);
Expand All @@ -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);
Expand All @@ -1638,13 +1639,14 @@ CHAKRA_API JsSetProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propertyId
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (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);
});
}

Expand All @@ -1653,6 +1655,7 @@ CHAKRA_API JsObjectSetProperty(_In_ JsValueRef object, _In_ JsValueRef propertyI
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (Js::ScriptContext *scriptContext,
TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

VALIDATE_INCOMING_OBJECT(object, scriptContext);
VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext);
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand All @@ -1760,21 +1762,16 @@ 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(),
scriptContext, useStrictRules ? Js::PropertyOperation_StrictMode : Js::PropertyOperation_None);

Assert(*result == nullptr || !Js::CrossSite::NeedMarshalVar(*result, scriptContext));

PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, result);

return JsNoError;
}

Expand All @@ -1783,14 +1780,19 @@ CHAKRA_API JsDeleteProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (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;
});
}

Expand All @@ -1800,6 +1802,7 @@ CHAKRA_API JsObjectDeleteProperty(_In_ JsValueRef object, _In_ JsValueRef proper
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (Js::ScriptContext *scriptContext,
TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

VALIDATE_INCOMING_OBJECT(object, scriptContext);
VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext);
Expand All @@ -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))
Expand All @@ -1849,6 +1850,7 @@ CHAKRA_API JsDefineProperty(_In_ JsValueRef object, _In_ JsPropertyIdRef propert
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (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);
Expand All @@ -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);
});
}

Expand All @@ -1867,6 +1869,7 @@ CHAKRA_API JsObjectDefineProperty(_In_ JsValueRef object, _In_ JsValueRef proper
{
return ContextAPIWrapper<JSRT_MAYBE_TRUE>([&] (Js::ScriptContext *scriptContext,
TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

VALIDATE_INCOMING_OBJECT(object, scriptContext);
VALIDATE_INCOMING_RECYCLABLE(propertyId, scriptContext);
Expand All @@ -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
Expand Down

0 comments on commit 5bf941c

Please sign in to comment.