Bug 497495 (frame poisoning) part 4: Change nsPresArena to maintain separate free lists for every frame class, based on its FrameIID. Pass the necessary information in from AllocateFrame and FreeFrame.
diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -50,16 +50,17 @@
*/
/* a presentation of a document, part 2 */
#ifndef nsIPresShell_h___
#define nsIPresShell_h___
#include "nsISupports.h"
+#include "nsQueryFrame.h"
#include "nsCoord.h"
#include "nsRect.h"
#include "nsColor.h"
#include "nsEvent.h"
#include "nsCompatibility.h"
#include "nsFrameManagerBase.h"
#include "mozFlushType.h"
#include "nsWeakReference.h"
@@ -99,20 +100,20 @@ class gfxContext;
class nsPIDOMEventTarget;
class nsIDOMEvent;
class nsDisplayList;
class nsDisplayListBuilder;
typedef short SelectionType;
typedef PRUint32 nsFrameState;
-// eba51d41-68db-4dab-a57b-dc1a2704de87
+// eed2ef56-133f-4696-9eee-5fc45d816be8
#define NS_IPRESSHELL_IID \
-{ 0xeba51d41, 0x68db, 0x4dab, \
- { 0xa5, 0x7b, 0xdc, 0x1a, 0x27, 0x04, 0xde, 0x87 } }
+{ 0xeed2ef56, 0x133f, 0x4696, \
+ { 0x9e, 0xee, 0x5f, 0xc4, 0x5d, 0x81, 0x6b, 0xe8 } }
// Constants for ScrollContentIntoView() function
#define NS_PRESSHELL_SCROLL_TOP 0
#define NS_PRESSHELL_SCROLL_BOTTOM 100
#define NS_PRESSHELL_SCROLL_LEFT 0
#define NS_PRESSHELL_SCROLL_RIGHT 100
#define NS_PRESSHELL_SCROLL_CENTER 50
#define NS_PRESSHELL_SCROLL_ANYWHERE -1
@@ -167,20 +168,21 @@ public:
* times to make form controls behave nicely when printed.
*/
NS_IMETHOD Destroy() = 0;
PRBool IsDestroying() { return mIsDestroying; }
// All frames owned by the shell are allocated from an arena. They
// are also recycled using free lists. Separate free lists are
- // maintained for each combination of aSize and aCode. AllocateFrame
- // clears the memory that it returns.
- virtual void* AllocateFrame(size_t aSize, unsigned int aCode) = 0;
- virtual void FreeFrame(size_t aSize, unsigned int aCode, void* aChunk) = 0;
+ // maintained for each frame type (aCode), which must always
+ // correspond to the same aSize value. AllocateFrame clears the
+ // memory that it returns.
+ virtual void* AllocateFrame(nsQueryFrame::FrameIID aCode, size_t aSize) = 0;
+ virtual void FreeFrame(nsQueryFrame::FrameIID aCode, void* aChunk) = 0;
// Objects closely related to the frame tree, but that are not
// actual frames (subclasses of nsFrame) are also allocated from the
// arena, and recycled via a separate set of per-size free lists.
// AllocateMisc does *not* clear the memory that it returns.
virtual void* AllocateMisc(size_t aSize) = 0;
virtual void FreeMisc(size_t aSize, void* aChunk) = 0;
diff --git a/layout/base/nsPresArena.cpp b/layout/base/nsPresArena.cpp
--- a/layout/base/nsPresArena.cpp
+++ b/layout/base/nsPresArena.cpp
@@ -42,156 +42,203 @@
* ***** END LICENSE BLOCK *****
*/
/* arena allocation for the frame tree and closely-related objects */
#include "nsPresArena.h"
#include "nsCRT.h"
#include "nsDebug.h"
+#include "nsTArray.h"
+#include "nsTHashtable.h"
#include "prmem.h"
-// Uncomment this to disable arenas, instead forwarding to
-// malloc for every allocation.
-//#define DEBUG_TRACEMALLOC_PRESARENA 1
-
#ifndef DEBUG_TRACEMALLOC_PRESARENA
// Even on 32-bit systems, we allocate objects from the frame arena
// that require 8-byte alignment. The cast to PRUword is needed
// because plarena isn't as careful about mask construction as it
// ought to be.
#define ALIGN_SHIFT 3
#define PL_ARENA_CONST_ALIGN_MASK ((PRUword(1) << ALIGN_SHIFT) - 1)
#include "plarena.h"
-// Largest chunk size we recycle
-static const size_t MAX_RECYCLED_SIZE = 400;
-
-// Recycler array entry N (0 <= N < NUM_RECYCLERS) holds chunks of
-// size (N+1) << ALIGN_SHIFT, thus we need this many array entries.
-static const size_t NUM_RECYCLERS = MAX_RECYCLED_SIZE >> ALIGN_SHIFT;
-
// Size to use for PLArena block allocations.
static const size_t ARENA_PAGE_SIZE = 4096;
+// We know implicitly that the values we hash fit in 32 bits (see
+// below) and so we do not bother actually hashing them. However,
+// we must preserve our own copy of the original key, since the hash
+// library messes with the value returned from HashKey.
+class FreeList : public PLDHashEntryHdr
+{
+public:
+ typedef PRUint32 KeyType;
+ nsTArray mEntries;
+ size_t mEntrySize;
+
+protected:
+ typedef const void* KeyTypePointer;
+ KeyTypePointer mKey;
+
+ FreeList(KeyTypePointer aKey) : mKey(aKey), mEntrySize(0) {}
+ // Default copy constructor and destructor are ok.
+
+ PRBool KeyEquals(KeyTypePointer const aKey) const
+ { return mKey == aKey; }
+
+ static KeyTypePointer KeyToPointer(KeyType aKey)
+ { return NS_INT32_TO_PTR(aKey); }
+
+ static PLDHashNumber HashKey(KeyTypePointer aKey)
+ { return NS_PTR_TO_INT32(aKey); }
+
+ enum { ALLOW_MEMMOVE = PR_FALSE };
+ friend class nsTHashtable;
+};
+
struct nsPresArena::State {
- void* mRecyclers[NUM_RECYCLERS];
+ nsTHashtable mFreeLists;
PLArenaPool mPool;
State()
{
+ mFreeLists.Init();
PL_INIT_ARENA_POOL(&mPool, "PresArena", ARENA_PAGE_SIZE);
- memset(mRecyclers, 0, sizeof(mRecyclers));
}
~State()
{
PL_FinishArenaPool(&mPool);
}
- void* Allocate(size_t aSize)
+ void* Allocate(PRUint32 aCode, size_t aSize)
{
- void* result = nsnull;
+ NS_ABORT_IF_FALSE(aSize > 0, "PresArena cannot allocate zero bytes");
- // Recycler lists are indexed by aligned size
+ // We only hand out aligned sizes
aSize = PL_ARENA_ALIGN(&mPool, aSize);
- // Check recyclers first
- if (aSize <= MAX_RECYCLED_SIZE) {
- const size_t index = (aSize >> ALIGN_SHIFT) - 1;
- result = mRecyclers[index];
- if (result) {
- // Need to move to the next object
- void* next = *((void**)result);
- mRecyclers[index] = next;
- }
+ // If there is no free-list entry for this type already, we have
+ // to create one now, to record its size.
+ FreeList* list = mFreeLists.PutEntry(aCode);
+ if (!list) {
+ return nsnull;
}
- if (!result) {
- // Allocate a new chunk from the arena
- PL_ARENA_ALLOCATE(result, &mPool, aSize);
+ nsTArray::index_type len = list->mEntries.Length();
+ if (list->mEntrySize == 0) {
+ NS_ABORT_IF_FALSE(len == 0, "list with entries but no recorded size");
+ list->mEntrySize = aSize;
+ } else {
+ NS_ABORT_IF_FALSE(list->mEntrySize != aSize,
+ "different sizes for same object type code");
}
+ void* result;
+ if (len > 0) {
+ // LIFO behavior for best cache utilization
+ result = list->mEntries.ElementAt(len - 1);
+ list->mEntries.RemoveElementAt(len - 1);
+ return result;
+ }
+
+ // Allocate a new chunk from the arena
+ PL_ARENA_ALLOCATE(result, &mPool, aSize);
return result;
}
- void Free(size_t aSize, void* aPtr)
+ void Free(PRUint32 aCode, void* aPtr)
{
- // Recycler lists are indexed by aligned size
- aSize = PL_ARENA_ALIGN(&mPool, aSize);
+ NS_ABORT_IF_FALSE(aSize > 0, "PresArena cannot free zero bytes");
- // See if it's a size that we recycle
- if (aSize <= MAX_RECYCLED_SIZE) {
- const size_t index = (aSize >> ALIGN_SHIFT) - 1;
- void* currentTop = mRecyclers[index];
- mRecyclers[index] = aPtr;
- *((void**)aPtr) = currentTop;
+ // Try to recycle this entry.
+ FreeList* list = mFreeLists.PutEntry(aCode);
+ NS_ABORT_IF_FALSE(list, "no free list for pres arena object");
+
+ // Fill the freed memory with a poison value, which is believed to
+ // form a pointer to an always-unmapped region of the address
+ // space on all platforms of interest. The low 12 bits of this
+ // number are chosen to fall exactly in the middle of the typical
+ // 4096-byte page, and the address is odd.
+ //
+ // With the possible exception of PPC64, current 64-bit CPUs
+ // permit only a subset (2^48 to 2^56, depending) of the full
+ // virtual address space to be used. x86-64 has the inaccessible
+ // region in the *middle* of the address space, whereas all others
+ // are believed to have it at the highest addresses. Use an
+ // address in this region if we possibly can, as this is even
+ // stronger than its being forbidden by the OS.
+ //
+ // TODO: This is only *known* to be an always-unmapped address on
+ // x86(-32,-64) Linux and Win32. OSX, win64, winCE, and ARM Linux,
+ // at least, should be investigated.
+
+ PRUword poison;
+#if defined __x86_64
+ poison = 0x7FFFFFFFF0DEA7FF;
+#else
+ poison = (~PRUword(0x0FFFFF00) | PRUword(0x0DEA700));
+#endif
+
+ char* p = reinterpret_cast(aPtr);
+ char* limit = p + list->mEntrySize;
+ for (; p < limit; p += sizeof(PRUword)) {
+ *reinterpret_cast(p) = poison;
}
-#if defined DEBUG_dbaron || defined DEBUG_zack
- else {
- fprintf(stderr,
- "WARNING: nsPresArena::FreeFrame leaking chunk of %lu bytes.\n",
- aSize);
- }
-#endif
+
+ list->mEntries.AppendElement(aPtr);
}
};
#else
-// Stub implementation that just forwards everything to malloc.
+// Stub implementation that forwards everything to malloc and does not
+// poison.
struct nsPresArena::State
{
- void* Allocate(size_t aSize)
+ void* Allocate(PRUnit32 /* unused */, size_t aSize)
{
return PR_Malloc(aSize);
}
- void Free(size_t /*unused*/, void* aPtr)
+ void Free(PRUint32 /* unused */, void* aPtr)
{
PR_Free(aPtr);
}
};
#endif // DEBUG_TRACEMALLOC_PRESARENA
// Public interface
nsPresArena::nsPresArena()
: mState(new nsPresArena::State())
-#ifdef DEBUG
- , mAllocCount(0)
-#endif
{}
nsPresArena::~nsPresArena()
{
-#ifdef DEBUG
- NS_ASSERTION(mAllocCount == 0,
- "Some PresArena objects were not freed");
-#endif
delete mState;
}
void*
-nsPresArena::Allocate(size_t aSize)
+nsPresArena::AllocateBySize(size_t aSize)
{
- NS_ABORT_IF_FALSE(aSize > 0, "PresArena cannot allocate zero bytes");
- void* result = mState->Allocate(aSize);
-#ifdef DEBUG
- if (result)
- mAllocCount++;
-#endif
- return result;
+ return mState->Allocate(PRUint32(aSize)|nsQueryFrame::NON_FRAME_MARKER,
+ aSize);
}
void
-nsPresArena::Free(size_t aSize, void* aPtr)
+nsPresArena::FreeBySize(size_t aSize, void* aPtr)
{
- NS_ABORT_IF_FALSE(aSize > 0, "PresArena cannot free zero bytes");
-#ifdef DEBUG
- // Mark the memory with 0xdd in DEBUG builds so that there will be
- // problems if someone tries to access memory that they've freed.
- memset(aPtr, 0xdd, aSize);
- mAllocCount--;
-#endif
- mState->Free(aSize, aPtr);
+ mState->Free(PRUint32(aSize)|nsQueryFrame::NON_FRAME_MARKER, aPtr);
}
+
+void*
+nsPresArena::AllocateByCode(nsQueryFrame::FrameIID aCode, size_t aSize)
+{
+ return mState->Allocate(aCode, aSize);
+}
+
+void
+nsPresArena::FreeByCode(nsQueryFrame::FrameIID aCode, void* aPtr)
+{
+ mState->Free(aCode, aPtr);
+}
diff --git a/layout/base/nsPresArena.h b/layout/base/nsPresArena.h
--- a/layout/base/nsPresArena.h
+++ b/layout/base/nsPresArena.h
@@ -41,27 +41,42 @@
*
* ***** END LICENSE BLOCK *****
*/
#ifndef nsPresArena_h___
#define nsPresArena_h___
#include "nscore.h"
+#include "nsQueryFrame.h"
+
+// Uncomment this to disable arenas, instead forwarding to
+// malloc for every allocation.
+//#define DEBUG_TRACEMALLOC_PRESARENA 1
+
+// The debugging version of nsPresArena does not free all the memory it
+// allocated when the arena itself is destroyed.
+#ifdef DEBUG_TRACEMALLOC_PRESARENA
+#define PRESARENA_AUTOFREE_ON_DESTROY 0
+#else
+#define PRESARENA_AUTOFREE_ON_DESTROY 1
+#endif
class nsPresArena {
public:
nsPresArena();
~nsPresArena();
- // Memory management functions
- NS_HIDDEN_(void*) Allocate(size_t aSize);
- NS_HIDDEN_(void) Free(size_t aSize, void* aPtr);
+ // Pool allocation with recycler lists indexed by object size.
+ NS_HIDDEN_(void*) AllocateBySize(size_t aSize);
+ NS_HIDDEN_(void) FreeBySize(size_t aSize, void* aPtr);
+
+ // Pool allocation with recycler lists indexed by object-type code.
+ // Every type code must always be used with the same object size.
+ NS_HIDDEN_(void*) AllocateByCode(nsQueryFrame::FrameIID aCode, size_t aSize);
+ NS_HIDDEN_(void) FreeByCode(nsQueryFrame::FrameIID aCode, void* aPtr);
private:
struct State;
State* mState;
-#ifdef DEBUG
- PRUint32 mAllocCount;
-#endif
};
#endif
diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -650,18 +650,19 @@ public:
// nsIPresShell
NS_IMETHOD Init(nsIDocument* aDocument,
nsPresContext* aPresContext,
nsIViewManager* aViewManager,
nsStyleSet* aStyleSet,
nsCompatibility aCompatMode);
NS_IMETHOD Destroy();
- virtual NS_HIDDEN_(void*) AllocateFrame(size_t aSize, unsigned int aCode);
- virtual NS_HIDDEN_(void) FreeFrame(size_t aSize, unsigned int aCode,
+ virtual NS_HIDDEN_(void*) AllocateFrame(nsQueryFrame::FrameIID aCode,
+ size_t aSize);
+ virtual NS_HIDDEN_(void) FreeFrame(nsQueryFrame::FrameIID aCode,
void* aChunk);
virtual NS_HIDDEN_(void*) AllocateMisc(size_t aSize);
virtual NS_HIDDEN_(void) FreeMisc(size_t aSize, void* aChunk);
// Dynamic stack memory allocation
virtual NS_HIDDEN_(void) PushStackMemory();
virtual NS_HIDDEN_(void) PopStackMemory();
@@ -1265,16 +1266,21 @@ private:
private:
/*
* Computes the backstop color for the view: transparent if in a transparent
* widget, otherwise the PresContext default background color. This color is
* only visible if the contents of the view as a whole are translucent.
*/
nscolor ComputeBackstopColor(nsIView* aView);
+
+#ifdef DEBUG
+ // Ensure that every allocation from the PresArena is eventually freed.
+ PRUint32 mPresArenaAllocCount;
+#endif
};
class nsAutoCauseReflowNotifier
{
public:
nsAutoCauseReflowNotifier(PresShell* aShell)
: mShell(aShell)
{
@@ -1598,16 +1604,19 @@ PresShell::PresShell()
mReflowCountMgr->SetPresShell(this);
#endif
#ifdef PR_LOGGING
if (! gLog)
gLog = PR_NewLogModule("PresShell");
#endif
mSelectionFlags = nsISelectionDisplay::DISPLAY_TEXT | nsISelectionDisplay::DISPLAY_IMAGES;
mIsThemeSupportDisabled = PR_FALSE;
+#ifdef DEBUG
+ mPresArenaAllocCount = 0;
+#endif
new (this) nsFrameManager();
}
NS_IMPL_ISUPPORTS8(PresShell, nsIPresShell, nsIDocumentObserver,
nsIViewObserver, nsISelectionController,
nsISelectionDisplay, nsIObserver, nsISupportsWeakReference,
nsIMutationObserver)
@@ -1619,17 +1628,22 @@ PresShell::~PresShell()
Destroy();
}
NS_ASSERTION(mCurrentEventContentStack.Count() == 0,
"Huh, event content left on the stack in pres shell dtor!");
NS_ASSERTION(mFirstCallbackEventRequest == nsnull &&
mLastCallbackEventRequest == nsnull,
"post-reflow queues not empty. This means we're leaking");
-
+
+#ifdef DEBUG
+ NS_ASSERTION(mPresArenaAllocCount == 0,
+ "Some pres arena objects were not freed");
+#endif
+
delete mStyleSet;
delete mFrameConstructor;
mCurrentEventContent = nsnull;
NS_IF_RELEASE(mPresContext);
NS_IF_RELEASE(mDocument);
NS_IF_RELEASE(mSelection);
@@ -1954,42 +1968,56 @@ PresShell::PopStackMemory()
/* virtual */ void*
PresShell::AllocateStackMemory(size_t aSize)
{
return mStackArena.Allocate(aSize);
}
void
-PresShell::FreeFrame(size_t aSize, unsigned int /*unused*/, void* aPtr)
-{
- mFrameArena.Free(aSize, aPtr);
+PresShell::FreeFrame(nsQueryFrame::FrameIID aCode, void* aPtr)
+{
+#ifdef DEBUG
+ mPresArenaAllocCount--;
+#endif
+ if (!PRESARENA_AUTOFREE_ON_DESTROY || !mIsDestroying)
+ mFrameArena.FreeByCode(aCode, aPtr);
}
void*
-PresShell::AllocateFrame(size_t aSize, unsigned int /*unused*/)
-{
- void* result = mFrameArena.Allocate(aSize);
+PresShell::AllocateFrame(nsQueryFrame::FrameIID aCode, size_t aSize)
+{
+#ifdef DEBUG
+ mPresArenaAllocCount++;
+#endif
+ void* result = mFrameArena.AllocateByCode(aCode, aSize);
if (result) {
memset(result, 0, aSize);
}
return result;
}
void
PresShell::FreeMisc(size_t aSize, void* aPtr)
{
- mFrameArena.Free(aSize, aPtr);
+#ifdef DEBUG
+ mPresArenaAllocCount--;
+#endif
+ if (!PRESARENA_AUTOFREE_ON_DESTROY || !mIsDestroying)
+ mFrameArena.FreeBySize(aSize, aPtr);
}
void*
PresShell::AllocateMisc(size_t aSize)
{
- return mFrameArena.Allocate(aSize);
+#ifdef DEBUG
+ mPresArenaAllocCount++;
+#endif
+ return mFrameArena.AllocateBySize(aSize);
}
void
nsIPresShell::SetAuthorStyleDisabled(PRBool aStyleDisabled)
{
if (aStyleDisabled != mStyleSet->GetAuthorStyleDisabled()) {
mStyleSet->SetAuthorStyleDisabled(aStyleDisabled);
ReconstructStyleData();
diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -451,30 +451,30 @@ nsFrame::Destroy()
if (view) {
// Break association between view and frame
view->SetClientData(nsnull);
// Destroy the view
view->Destroy();
}
- // Must retrieve the object size before calling destructors, so the
+ // Must retrieve the object ID before calling destructors, so the
// vtable is still valid.
//
// Note to future tweakers: having the method that returns the
// object size call the destructor will not avoid an indirect call;
// the compiler cannot devirtualize the call to the destructor even
// if it's from a method defined in the same class.
- size_t sz = GetAllocatedSize();
+ nsQueryFrame::FrameIID id = GetFrameId();
this->~nsFrame();
// Now that we're totally cleaned out, we need to add ourselves to
// the presshell's recycler.
- shell->FreeFrame(sz, 0 /* dummy */, (void*)this);
+ shell->FreeFrame(id, this);
}
NS_IMETHODIMP
nsFrame::GetOffsets(PRInt32 &aStart, PRInt32 &aEnd) const
{
aStart = 0;
aEnd = 0;
return NS_OK;
diff --git a/layout/generic/nsFrame.h b/layout/generic/nsFrame.h
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -113,23 +113,23 @@
// Frame allocation boilerplate macros. Every subclass of nsFrame
// must define its own operator new and GetAllocatedSize. If they do
// not, the per-frame recycler lists in nsPresArena will not work
// correctly, with potentially catastrophic consequences (not enough
// memory is allocated for a frame object).
#define NS_DECL_FRAMEARENA_HELPERS \
NS_MUST_OVERRIDE void* operator new(size_t, nsIPresShell*); \
- virtual NS_MUST_OVERRIDE size_t GetAllocatedSize();
+ virtual NS_MUST_OVERRIDE nsQueryFrame::FrameIID GetFrameId();
#define NS_IMPL_FRAMEARENA_HELPERS(class) \
void* class::operator new(size_t sz, nsIPresShell* aShell) \
- { return aShell->AllocateFrame(sz, nsQueryFrame::class##_id); } \
- size_t class::GetAllocatedSize() \
- { return sizeof(class); }
+ { return aShell->AllocateFrame(nsQueryFrame::class##_id, sz); } \
+ nsQueryFrame::FrameIID class::GetFrameId() \
+ { return nsQueryFrame::class##_id; }
//----------------------------------------------------------------------
struct nsBoxLayoutMetrics;
/**
* Implementation of a simple frame that's not splittable and has no
* child frames.
diff --git a/layout/generic/nsQueryFrame.h b/layout/generic/nsQueryFrame.h
--- a/layout/generic/nsQueryFrame.h
+++ b/layout/generic/nsQueryFrame.h
@@ -246,17 +246,24 @@ public:
nsTextFrame_id,
nsTitleBarFrame_id,
nsTreeBodyFrame_id,
nsTreeColFrame_id,
nsVideoFrame_id,
nsXULLabelFrame_id,
nsXULScrollFrame_id,
SpacerFrame_id,
- ViewportFrame_id
+ ViewportFrame_id,
+
+ // The PresArena implementation uses this as a flag to distinguish
+ // objects allocated by size (that is, non-frames) from objects
+ // allocated by code (that is, frames). It should not collide
+ // with any frame ID. It is not 0x80000000 to avoid the issue of
+ // whether enumeration constants are signed.
+ NON_FRAME_MARKER = 0x40000000
};
virtual void* QueryFrame(FrameIID id) = 0;
};
class do_QueryFrame
{
public: