diff --git a/js/xpconnect/src/XPCConvert.cpp b/js/xpconnect/src/XPCConvert.cpp index 467f81a3df77..769f94ab5b96 100644 --- a/js/xpconnect/src/XPCConvert.cpp +++ b/js/xpconnect/src/XPCConvert.cpp @@ -1668,6 +1668,11 @@ xpc::InnerCleanupValue(const nsXPTType& aType, void* aValue, uint32_t aArrayLen) free(elements); break; } + + // Clear the JS::Value to `undefined` + case nsXPTType::T_JSVAL: + ((JS::Value*)aValue)->setUndefined(); + break; } // Null out the pointer if we have it. @@ -1675,3 +1680,33 @@ xpc::InnerCleanupValue(const nsXPTType& aType, void* aValue, uint32_t aArrayLen) *(void**)aValue = nullptr; } } + +/***************************************************************************/ + +// Implementation of xpc::InitializeValue. + +void +xpc::InitializeValue(const nsXPTType& aType, void* aValue) +{ + switch (aType.Tag()) { + // Types which require custom, specific initialization. + case nsXPTType::T_JSVAL: + new (aValue) JS::Value(); + MOZ_ASSERT(reinterpret_cast(aValue)->isUndefined()); + break; + + case nsXPTType::T_ASTRING: + case nsXPTType::T_DOMSTRING: + new (aValue) nsString(); + break; + case nsXPTType::T_CSTRING: + case nsXPTType::T_UTF8STRING: + new (aValue) nsCString(); + break; + + // The remaining types all have valid states where all bytes are '0'. + default: + memset(aValue, 0, aType.Stride()); + break; + } +} diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp index 8c43cdc1b801..1a42638b8469 100644 --- a/js/xpconnect/src/XPCWrappedNative.cpp +++ b/js/xpconnect/src/XPCWrappedNative.cpp @@ -1245,10 +1245,6 @@ CallMethodHelper::Call() CallMethodHelper::~CallMethodHelper() { for (nsXPTCVariant& param : mDispatchParams) { - // Only clean up values which need cleanup. - if (!param.DoesValNeedCleanup()) - continue; - uint32_t arraylen = 0; if (!GetArraySizeFromParam(param.type, UndefinedHandleValue, &arraylen)) continue; @@ -1477,25 +1473,39 @@ CallMethodHelper::InitializeDispatchParams() mJSContextIndex = mMethodInfo->IndexOfJSContext(); - // iterate through the params to clear flags (for safe cleanup later) - for (uint8_t i = 0; i < paramCount + wantsJSContext + wantsOptArgc; i++) { - nsXPTCVariant* dp = mDispatchParams.AppendElement(); - dp->ClearFlags(); - dp->val.p = nullptr; + // Allocate enough space in mDispatchParams up-front. + if (!mDispatchParams.AppendElements(paramCount + wantsJSContext + wantsOptArgc)) { + Throw(NS_ERROR_OUT_OF_MEMORY, mCallContext); + return false; } - // Fill in the JSContext argument - if (wantsJSContext) { - nsXPTCVariant* dp = &mDispatchParams[mJSContextIndex]; - dp->type = nsXPTType::T_VOID; - dp->val.p = mCallContext; - } + // Initialize each parameter to a valid state (for safe cleanup later). + for (uint8_t i = 0, paramIdx = 0; i < mDispatchParams.Length(); i++) { + nsXPTCVariant& dp = mDispatchParams[i]; - // Fill in the optional_argc argument - if (wantsOptArgc) { - nsXPTCVariant* dp = &mDispatchParams[mOptArgcIndex]; - dp->type = nsXPTType::T_U8; - dp->val.u8 = std::min(mArgc, paramCount) - requiredArgs; + if (i == mJSContextIndex) { + // Fill in the JSContext argument + dp.type = nsXPTType::T_VOID; + dp.val.p = mCallContext; + } else if (i == mOptArgcIndex) { + // Fill in the optional_argc argument + dp.type = nsXPTType::T_U8; + dp.val.u8 = std::min(mArgc, paramCount) - requiredArgs; + } else { + // Initialize normal arguments. + const nsXPTParamInfo& param = mMethodInfo->Param(paramIdx); + dp.type = param.Type(); + xpc::InitializeValue(dp.type, &dp.val); + + // Specify the correct storage/calling semantics. This will also set + // the `ptr` field to be self-referential. + if (param.IsIndirect()) { + dp.SetIndirect(); + } + + // Advance to the next normal parameter. + paramIdx++; + } } return true; @@ -1522,40 +1532,8 @@ bool CallMethodHelper::ConvertIndependentParam(uint8_t i) { const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(i); - const nsXPTType& type = paramInfo.GetType(); + const nsXPTType& type = paramInfo.Type(); nsXPTCVariant* dp = GetDispatchParam(i); - dp->type = type; - MOZ_ASSERT(!paramInfo.IsShared(), "[shared] implies [noscript]!"); - - // Specify the correct storage/calling semantics. - if (paramInfo.IsIndirect()) - dp->SetIndirect(); - - // Some types are always stored within the nsXPTCVariant, and passed - // indirectly, regardless of in/out-ness. These types are stored in the - // nsXPTCVariant's extended value. - switch (type.Tag()) { - // Ensure that the jsval has a valid value. - case nsXPTType::T_JSVAL: - new (&dp->ext.jsval) JS::Value(); - MOZ_ASSERT(dp->ext.jsval.isUndefined()); - break; - - // Initialize our temporary string class values so they can be assigned - // to by the XPCConvert logic. - case nsXPTType::T_ASTRING: - case nsXPTType::T_DOMSTRING: - new (&dp->ext.nsstr) nsString(); - break; - case nsXPTType::T_CSTRING: - case nsXPTType::T_UTF8STRING: - new (&dp->ext.nscstr) nsCString(); - break; - } - - // Flag cleanup for anything that isn't self-contained. - if (!type.IsArithmetic()) - dp->SetValNeedsCleanup(); // Even if there's nothing to convert, we still need to examine the // JSObject container for out-params. If it's null or otherwise invalid, @@ -1641,18 +1619,8 @@ bool CallMethodHelper::ConvertDependentParam(uint8_t i) { const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(i); - const nsXPTType& type = paramInfo.GetType(); - + const nsXPTType& type = paramInfo.Type(); nsXPTCVariant* dp = GetDispatchParam(i); - dp->type = type; - - // Specify the correct storage/calling semantics. - if (paramInfo.IsIndirect()) - dp->SetIndirect(); - - // Make sure we clean up all of our dependent types. All of them require - // allocations of some kind. - dp->SetValNeedsCleanup(); // Even if there's nothing to convert, we still need to examine the // JSObject container for out-params. If it's null or otherwise invalid, @@ -1712,14 +1680,18 @@ TraceParam(JSTracer* aTrc, void* aVal, const nsXPTType& aType, if (aType.Tag() == nsXPTType::T_JSVAL) { JS::UnsafeTraceRoot(aTrc, (JS::Value*)aVal, "XPCWrappedNative::CallMethod param"); + } else if (aType.Tag() == nsXPTType::T_SEQUENCE) { + auto* sequence = (xpt::detail::UntypedSequence*)aVal; + + const nsXPTType& elty = aType.ArrayElementType(); + for (uint32_t i = 0; i < sequence->Length(); ++i) { + TraceParam(aTrc, sequence->ElementAt(elty, i), elty); + } } else if (aType.Tag() == nsXPTType::T_ARRAY && *(void**)aVal) { const nsXPTType& elty = aType.ArrayElementType(); - if (elty.Tag() != nsXPTType::T_JSVAL) { - return; - } for (uint32_t i = 0; i < aArrayLen; ++i) { - TraceParam(aTrc, elty.ElementPtr(aVal, i), elty); + TraceParam(aTrc, elty.ElementPtr(*(void**)aVal, i), elty); } } } @@ -1729,9 +1701,8 @@ CallMethodHelper::trace(JSTracer* aTrc) { // We need to note each of our initialized parameters which contain jsvals. for (nsXPTCVariant& param : mDispatchParams) { - if (!param.DoesValNeedCleanup()) { - MOZ_ASSERT(param.type.Tag() != nsXPTType::T_JSVAL, - "JSVals are marked as needing cleanup (even though they don't)"); + // We only need to trace parameters which have an innermost JSVAL. + if (param.type.InnermostType().Tag() != nsXPTType::T_JSVAL) { continue; } diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h index 658622bb670e..e73c8ede7845 100644 --- a/js/xpconnect/src/xpcprivate.h +++ b/js/xpconnect/src/xpcprivate.h @@ -3060,6 +3060,17 @@ void InnerCleanupValue(const nsXPTType& aType, void* aValue, uint32_t aArrayLen); +// In order to be able to safely call CleanupValue on a generated value, the +// data behind it needs to be initialized to a safe value. This method handles +// initializing the backing data to a safe value to use as an argument to +// XPCConvert methods, or xpc::CleanupValue. +// +// The pointer `aValue` must point to a block of memory at least aType.Stride() +// bytes large, and correctly aligned. +// +// This method accepts the same types as xpc::CleanupValue. +void InitializeValue(const nsXPTType& aType, void* aValue); + } // namespace xpc namespace mozilla { diff --git a/xpcom/reflect/xptcall/xptcall.h b/xpcom/reflect/xptcall/xptcall.h index 2085f3e9443d..d6ddd7ead9b5 100644 --- a/xpcom/reflect/xptcall/xptcall.h +++ b/xpcom/reflect/xptcall/xptcall.h @@ -45,8 +45,6 @@ static_assert(offsetof(nsXPTCMiniVariant, val) == 0, struct nsXPTCVariant { -// No ctors or dtors so that we can use arrays of these on the stack with no -// penalty. union ExtendedVal { // ExtendedVal is an extension on nsXPTCMiniVariant. It contains types @@ -79,6 +77,12 @@ struct nsXPTCVariant nsXPTType type; uint8_t flags; + // Clear to a valid, null state. + nsXPTCVariant() { + memset(this, 0, sizeof(nsXPTCVariant)); + type = nsXPTType::T_VOID; + } + enum { // @@ -88,22 +92,12 @@ struct nsXPTCVariant // Indicates that we &val.p should be passed n the stack, i.e. that // val should be passed by reference. IS_INDIRECT = 0x1, - - // Indicates that the value we hold requires some sort of cleanup (memory - // deallocation, interface release, JS::Value unrooting, etc). The precise - // cleanup that is performed depends on the 'type' field above. - // If the value is an array, this flag specifies whether the elements - // within the array require cleanup (we always clean up the array itself, - // so this flag would be redundant for that purpose). - VAL_NEEDS_CLEANUP = 0x2 }; void ClearFlags() {flags = 0;} void SetIndirect() {flags |= IS_INDIRECT;} - void SetValNeedsCleanup() {flags |= VAL_NEEDS_CLEANUP;} bool IsIndirect() const {return 0 != (flags & IS_INDIRECT);} - bool DoesValNeedCleanup() const {return 0 != (flags & VAL_NEEDS_CLEANUP);} // Implicitly convert to nsXPTCMiniVariant. operator nsXPTCMiniVariant&() { @@ -113,9 +107,8 @@ struct nsXPTCVariant return *(const nsXPTCMiniVariant*) &val; } - // As this type contains an anonymous union, we need to provide explicit - // constructors & destructors. - nsXPTCVariant() { } + // As this type contains an anonymous union, we need to provide an explicit + // destructor. ~nsXPTCVariant() { } };