[Webkit-unassigned] [Bug 102828] r134080 causes heap problem on linux systems where PAGESIZE != 4096

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 9 12:12:53 PST 2013


https://bugs.webkit.org/show_bug.cgi?id=102828





--- Comment #15 from Balazs Kilvady <kilvadyb at homejinni.com>  2013-01-09 12:14:46 PST ---
(In reply to comment #14)
> (From update of attachment 181941 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181941&action=review
> 
> > Source/JavaScriptCore/heap/BaseBlock.h:33
> > +class BaseBlock {
> 
> It seems like major overkill to define an entirely new class whose sole purpose is to make sure a static constant is the same between two classes. Why not just use a COMPILE_ASSERT? And why not just change the value in Options.cpp to be 4 KB rather than pageSize()?

That also works and we said in the comment that we can provide such a patch also if you like. In that case the 4 KB is a "magic value" and must be matching at 3 different places. With this common base class the same value is guaranteed/forced at all the 3 places. But of course we will create an other patch with a static constant.

> 
> > Source/JavaScriptCore/heap/MarkStack.cpp:-54
> > -    ASSERT(MarkStackSegment::blockSize == WeakBlock::blockSize);
> 
> Why wasn't this ASSERT firing before if the MarkStackSegment blockSize was not equal to the WeakBlock blockSize?
Because both MarkStackSegment::blockSize and WeakBlock::blockSize  are 4 KB so the ASSERT condition is matching but MarkStackArray::m_segmentCapacity == MarkStackSegment::capacityFromSize(Options::gcMarkStackSegmentSize()))
and Options::gcMarkStackSegmentSize() == WTF::pageSize() by default. So MarkStackArray believes that a MarkStackSegment block has a pageSize capacity (16 KB) while it has only a MarkStackSegment::blockSize (4 KB). And in MarkStackArray::append function it expands the segments when the currently used block's capacity is exhausted but the array objects believes that a segment block has 4 times bigger capacity then in reality:
inline void MarkStackArray::append(const JSCell* cell)
{
   if (m_top == m_segmentCapacity)
       expand();
   m_segments.head()->data()[postIncTop()] = cell;
}

when m_top reaches 102x then the m_segments.head()->data()[postIncTop()] points to m_segments.m_head + sizeof(MarkStackSegment) + 102x * sizeof(JSCell*) which is over the 4 KB size of the MarkStackSegment block. So it overwrites the next block's data even its link node part and corrupts the block list.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list