Skip to content

Commit

Permalink
Pulling TTD out of JsObject* methods
Browse files Browse the repository at this point in the history 
Any instances of VALIDATE_INCOMING_OBJECT can also marshal the object
to the current context. The record actions need to be the value prior
any of these calls to capture the original pointer.

These methods were recently added and in the process regressed the
existing methods. Partially backing out the change so we can properly
address the TTD implementation for these methods.
  • Loading branch information
@kfarnung
kfarnung committed Jan 23, 2018
1 parent cfcbe3b commit 6515838
Showing 1 changed file with 47 additions and 44 deletions.
91 changes: 47 additions & 44 deletions 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 6515838

Please sign in to comment.