Bug 683802 - Define and restrict the semantics of PTR_IS_DATA. r=mrbkap

content/xslt/src/xpath/txXPCOMExtensionFunction.cpp

@@ -544,11 +544,10 @@ txXPCOMExtensionFunctionCall::evaluate(txIEvalContext* aContext,
         returnParam.val.p = value;
     }
     else {
-        returnParam.SetPtrIsData();
+        returnParam.SetIndirect();
         if (returnType == eNODESET || returnType == eOBJECT) {
             returnParam.SetValIsInterface();
         }
-        returnParam.ptr = &returnParam.val;
     }
 
     rv = NS_InvokeByIndex(mHelper, mMethodIndex, paramCount, invokeParams);

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -2840,13 +2840,12 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
     if (!GetOutParamSource(i, &src))
         return JS_FALSE;
 
-    // The JSVal proper is always stored within the 'val' union, regardless of
-    // in/out-ness. This is indicated with the SetPtrIsData() song + dance.
+    // The JSVal proper is always stored within the 'val' union and passed
+    // indirectly, regardless of in/out-ness.
     if(type_tag == nsXPTType::T_JSVAL)
     {
         // Indicate the storage semantics.
-        dp->SetPtrIsData();
-        dp->ptr = &dp->val;
+        dp->SetIndirect();
 
         // Assign the value
         JS_STATIC_ASSERT(sizeof(jsval) <= sizeof(dp->val));
@@ -2860,8 +2859,7 @@ CallMethodHelper::ConvertIndependentParam(uint8 i)
 
     if(paramInfo.IsOut())
     {
-        dp->SetPtrIsData();
-        dp->ptr = &dp->val;
+        dp->SetIndirect();
 
         if(type.IsPointer() &&
            type_tag != nsXPTType::T_INTERFACE &&
@@ -2994,8 +2992,7 @@ CallMethodHelper::ConvertDependentParams()
 
         if(paramInfo.IsOut())
         {
-            dp->SetPtrIsData();
-            dp->ptr = &dp->val;
+            dp->SetIndirect();
 
             if(datum_type.IsPointer() &&
                !datum_type.IsInterfacePointer() &&

xpcom/reflect/xptcall/public/xptcall.h

@@ -88,8 +88,26 @@ struct nsXPTCVariant : public nsXPTCMiniVariant
 
     enum
     {
-        // these are bitflags!
-        PTR_IS_DATA    = 0x1,  // ptr points to 'real' data in val
+        //
+        // Bitflag definitions
+        //
+
+        // Indicates that ptr (above, and distinct from val.p) is the value that
+        // should be passed on the stack.
+        //
+        // In theory, ptr could point anywhere. But in practice it always points
+        // to &val. So this flag is used to pass 'val' by reference, letting us
+        // avoid the extra allocation we would incur if we were to use val.p.
+        //
+        // Various parts of XPConnect assume that ptr==&val, so we enforce it
+        // explicitly with SetIndirect() and IsIndirect().
+        //
+        // Since ptr always points to &val, the semantics of this flag are kind of
+        // dumb, since the ptr field is unnecessary. But changing them would
+        // require changing dozens of assembly files, so they're likely to stay
+        // the way they are.
+        PTR_IS_DATA    = 0x1,
+
         VAL_IS_ALLOCD  = 0x2,  // val.p holds alloc'd ptr that must be freed
         VAL_IS_IFACE   = 0x4,  // val.p holds interface ptr that must be released
         VAL_IS_ARRAY   = 0x8,  // val.p holds a pointer to an array needing cleanup
@@ -100,7 +118,7 @@ struct nsXPTCVariant : public nsXPTCMiniVariant
     };
 
     void ClearFlags()         {flags = 0;}
-    void SetPtrIsData()       {flags |= PTR_IS_DATA;}
+    void SetIndirect()        {ptr = &val; flags |= PTR_IS_DATA;}
     void SetValIsAllocated()  {flags |= VAL_IS_ALLOCD;}
     void SetValIsInterface()  {flags |= VAL_IS_IFACE;}
     void SetValIsArray()      {flags |= VAL_IS_ARRAY;}
@@ -109,7 +127,7 @@ struct nsXPTCVariant : public nsXPTCMiniVariant
     void SetValIsCString()    {flags |= VAL_IS_CSTR;}
     void SetValIsJSRoot()     {flags |= VAL_IS_JSROOT;}
 
-    PRBool IsPtrData()       const  {return 0 != (flags & PTR_IS_DATA);}
+    PRBool IsIndirect()      const  {return 0 != (flags & PTR_IS_DATA);}
     PRBool IsValAllocated()  const  {return 0 != (flags & VAL_IS_ALLOCD);}
     PRBool IsValInterface()  const  {return 0 != (flags & VAL_IS_IFACE);}
     PRBool IsValArray()      const  {return 0 != (flags & VAL_IS_ARRAY);}
@@ -118,6 +136,9 @@ struct nsXPTCVariant : public nsXPTCMiniVariant
     PRBool IsValCString()    const  {return 0 != (flags & VAL_IS_CSTR);}    
     PRBool IsValJSRoot()     const  {return 0 != (flags & VAL_IS_JSROOT);}
 
+    // Internal use only. Use IsIndirect() instead.
+    PRBool IsPtrData()       const  {return 0 != (flags & PTR_IS_DATA);}
+
     void Init(const nsXPTCMiniVariant& mv, const nsXPTType& t, PRUint8 f)
     {
         type = t;