Bug 634594 - Ensure XPCCallContext string cache is aligned (r=mrbkap,…

…a=pavlov)
Brli · Feb 17, 2011 · ac489ca · ac489ca
1 parent 8bc7242
commit ac489ca
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 46 deletions.
diff --git a/js/src/xpconnect/src/xpccallcontext.cpp b/js/src/xpconnect/src/xpccallcontext.cpp
@@ -98,18 +98,6 @@ XPCCallContext::Init(XPCContext::LangType callerLanguage,
                      jsval *argv,
                      jsval *rval)
 {
-    // Mark our internal string wrappers as not used. Make sure we do
-    // this before any early returns, as the destructor will assert
-    // based on this.
-    StringWrapperEntry *se =
-        reinterpret_cast<StringWrapperEntry*>(&mStringWrapperData);
-
-    PRUint32 i;
-    for(i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
-    {
-        se[i].mInUse = PR_FALSE;
-    }
-
     if(!mXPC)
         return;
 
@@ -424,15 +412,9 @@ XPCCallContext::~XPCCallContext()
     }
 
 #ifdef DEBUG
+    for(PRUint32 i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
     {
-        StringWrapperEntry *se =
-            reinterpret_cast<StringWrapperEntry*>(&mStringWrapperData);
-
-        PRUint32 i;
-        for(i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
-        {
-            NS_ASSERTION(!se[i].mInUse, "Uh, string wrapper still in use!");
-        }
+        NS_ASSERTION(!mScratchStrings[i].mInUse, "Uh, string wrapper still in use!");
     }
 #endif
 
@@ -443,21 +425,17 @@ XPCCallContext::~XPCCallContext()
 XPCReadableJSStringWrapper *
 XPCCallContext::NewStringWrapper(const PRUnichar *str, PRUint32 len)
 {
-    StringWrapperEntry *se =
-        reinterpret_cast<StringWrapperEntry*>(&mStringWrapperData);
-
-    PRUint32 i;
-    for(i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
+    for(PRUint32 i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
     {
-        StringWrapperEntry& ent = se[i];
+        StringWrapperEntry& ent = mScratchStrings[i];
 
         if(!ent.mInUse)
         {
             ent.mInUse = PR_TRUE;
 
             // Construct the string using placement new.
 
-            return new (&ent.mString) XPCReadableJSStringWrapper(str, len);
+            return new (ent.mString.addr()) XPCReadableJSStringWrapper(str, len);
         }
     }
 
@@ -469,20 +447,16 @@ XPCCallContext::NewStringWrapper(const PRUnichar *str, PRUint32 len)
 void
 XPCCallContext::DeleteString(nsAString *string)
 {
-    StringWrapperEntry *se =
-        reinterpret_cast<StringWrapperEntry*>(&mStringWrapperData);
-
-    PRUint32 i;
-    for(i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
+    for(PRUint32 i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i)
     {
-        StringWrapperEntry& ent = se[i];
-        if(string == &ent.mString)
+        StringWrapperEntry& ent = mScratchStrings[i];
+        if(string == ent.mString.addr())
         {
             // One of our internal strings is no longer in use, mark
             // it as such and destroy the string.
 
             ent.mInUse = PR_FALSE;
-            ent.mString.~XPCReadableJSStringWrapper();
+            ent.mString.addr()->~XPCReadableJSStringWrapper();
 
             return;
         }

diff --git a/js/src/xpconnect/src/xpcprivate.h b/js/src/xpconnect/src/xpcprivate.h
@@ -1254,23 +1254,18 @@ inline void CHECK_STATE(int s) const {NS_ASSERTION(mState >= s, "bad state");}
 
     // String wrapper entry, holds a string, and a boolean that tells
     // whether the string is in use or not.
+    //
+    // NB: The string is not stored by value so that we avoid the cost of
+    // construction/destruction.
     struct StringWrapperEntry
     {
-        StringWrapperEntry()
-            : mInUse(PR_FALSE)
-        {
-        }
+        StringWrapperEntry() : mInUse(PR_FALSE) { }
 
-        XPCReadableJSStringWrapper mString;
+        js::AlignedStorage2<XPCReadableJSStringWrapper> mString;
         PRBool mInUse;
     };
 
-    // Reserve space for XPCCCX_STRING_CACHE_SIZE string wrapper
-    // entries for use on demand. It's important to not make this be
-    // string class members since we don't want to pay the cost of
-    // calling the constructors and destructors when the strings
-    // aren't being used.
-    char mStringWrapperData[sizeof(StringWrapperEntry) * XPCCCX_STRING_CACHE_SIZE];
+    StringWrapperEntry mScratchStrings[XPCCCX_STRING_CACHE_SIZE];
 };
 
 class XPCLazyCallContext