- Revision
- 133713
- Author
- [email protected]
- Date
- 2012-11-06 20:14:48 -0800 (Tue, 06 Nov 2012)
Log Message
Re-order variables in BidiRun and LayoutState
https://bugs.webkit.org/show_bug.cgi?id=100173
Reviewed by Eric Seidel.
The variable re-ordering and use of bitfields for bools has two benefits:
1) Size reduction. sizeof(BidiRun) goes down from 48 to 40 bytes on 64-bit. This is achieved by removing a bool member variable from BidiRun and packing it together with other bools in the BidiCharacterRun base class.
2) Security improvement. We have a lot of use-after-free in the RenderObject hierarchy, and the RenderArena class protects us from a lot of trouble by ensuring that objects of arbitrary type cannot be overlayed on top of freed RenderObjects. This change additionally makes sure that non-virtual RenderArena allocated objects do not have member variables which fully overlap the freed vtable pointer. This leaves re-used vtable pointers always pointing to either a valid vtable or an invalid address due to the freelist high-bit poisoning.
This change is exclusively about size savings; it is performance neutral as you would expect, including on Parser/html5-full-render.html
* platform/text/BidiResolver.h:
(WebCore::BidiCharacterRun::BidiCharacterRun): impact from re-ordering members.
(BidiCharacterRun): provide an efficiently packed bit of storage for BidiRun subclass to use, and re-order members to place bools adjacent.
* rendering/BidiRun.h:
(WebCore::BidiRun::BidiRun): use base class' efficiently packed bit storage for m_hasHyphen.
(BidiRun): m_hasHyphen is now stored in the base class.
* rendering/LayoutState.cpp:
(WebCore::LayoutState::LayoutState):
* rendering/LayoutState.h:
(WebCore::LayoutState::LayoutState): impact from re-ordering members.
(LayoutState): re-order members to place bools adjacently.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (133712 => 133713)
--- trunk/Source/WebCore/ChangeLog 2012-11-07 03:33:38 UTC (rev 133712)
+++ trunk/Source/WebCore/ChangeLog 2012-11-07 04:14:48 UTC (rev 133713)
@@ -1,3 +1,28 @@
+2012-11-06 Chris Evans <[email protected]>
+
+ Re-order variables in BidiRun and LayoutState
+ https://bugs.webkit.org/show_bug.cgi?id=100173
+
+ Reviewed by Eric Seidel.
+
+ The variable re-ordering and use of bitfields for bools has two benefits:
+ 1) Size reduction. sizeof(BidiRun) goes down from 48 to 40 bytes on 64-bit. This is achieved by removing a bool member variable from BidiRun and packing it together with other bools in the BidiCharacterRun base class.
+ 2) Security improvement. We have a lot of use-after-free in the RenderObject hierarchy, and the RenderArena class protects us from a lot of trouble by ensuring that objects of arbitrary type cannot be overlayed on top of freed RenderObjects. This change additionally makes sure that non-virtual RenderArena allocated objects do not have member variables which fully overlap the freed vtable pointer. This leaves re-used vtable pointers always pointing to either a valid vtable or an invalid address due to the freelist high-bit poisoning.
+
+ This change is exclusively about size savings; it is performance neutral as you would expect, including on Parser/html5-full-render.html
+
+ * platform/text/BidiResolver.h:
+ (WebCore::BidiCharacterRun::BidiCharacterRun): impact from re-ordering members.
+ (BidiCharacterRun): provide an efficiently packed bit of storage for BidiRun subclass to use, and re-order members to place bools adjacent.
+ * rendering/BidiRun.h:
+ (WebCore::BidiRun::BidiRun): use base class' efficiently packed bit storage for m_hasHyphen.
+ (BidiRun): m_hasHyphen is now stored in the base class.
+ * rendering/LayoutState.cpp:
+ (WebCore::LayoutState::LayoutState):
+ * rendering/LayoutState.h:
+ (WebCore::LayoutState::LayoutState): impact from re-ordering members.
+ (LayoutState): re-order members to place bools adjacently.
+
2012-11-06 Kent Tamura <[email protected]>
[Chromium-win] Refactor date/time format conversion code in LocaleWin
Modified: trunk/Source/WebCore/platform/text/BidiResolver.h (133712 => 133713)
--- trunk/Source/WebCore/platform/text/BidiResolver.h 2012-11-07 03:33:38 UTC (rev 133712)
+++ trunk/Source/WebCore/platform/text/BidiResolver.h 2012-11-07 04:14:48 UTC (rev 133713)
@@ -113,10 +113,10 @@
struct BidiCharacterRun {
BidiCharacterRun(int start, int stop, BidiContext* context, WTF::Unicode::Direction dir)
- : m_start(start)
+ : m_override(context->override())
+ , m_next(0)
+ , m_start(start)
, m_stop(stop)
- , m_override(context->override())
- , m_next(0)
{
if (dir == WTF::Unicode::OtherNeutral)
dir = context->dir();
@@ -146,11 +146,13 @@
BidiCharacterRun* next() const { return m_next; }
void setNext(BidiCharacterRun* next) { m_next = next; }
+ // Do not add anything apart from bitfields until after m_next. See https://bugs.webkit.org/show_bug.cgi?id=100173
+ bool m_override:1;
+ bool m_hasHyphen:1; // Used by BidiRun subclass which is a layering violation but enables us to save 8 bytes per object on 64-bit.
unsigned char m_level;
+ BidiCharacterRun* m_next;
int m_start;
int m_stop;
- bool m_override;
- BidiCharacterRun* m_next;
};
enum VisualDirectionOverride {
Modified: trunk/Source/WebCore/rendering/BidiRun.h (133712 => 133713)
--- trunk/Source/WebCore/rendering/BidiRun.h 2012-11-07 03:33:38 UTC (rev 133712)
+++ trunk/Source/WebCore/rendering/BidiRun.h 2012-11-07 04:14:48 UTC (rev 133713)
@@ -38,8 +38,8 @@
: BidiCharacterRun(start, stop, context, dir)
, m_object(object)
, m_box(0)
- , m_hasHyphen(false)
{
+ m_hasHyphen = false; // Stored in base class to save space.
}
void destroy();
@@ -60,7 +60,6 @@
public:
RenderObject* m_object;
InlineBox* m_box;
- bool m_hasHyphen;
};
}
Modified: trunk/Source/WebCore/rendering/LayoutState.cpp (133712 => 133713)
--- trunk/Source/WebCore/rendering/LayoutState.cpp 2012-11-07 03:33:38 UTC (rev 133712)
+++ trunk/Source/WebCore/rendering/LayoutState.cpp 2012-11-07 04:14:48 UTC (rev 133713)
@@ -39,12 +39,12 @@
: m_columnInfo(columnInfo)
, m_lineGrid(0)
, m_next(prev)
+#if ENABLE(CSS_EXCLUSIONS)
+ , m_exclusionShapeInsideInfo(0)
+#endif
#ifndef NDEBUG
, m_renderer(renderer)
#endif
-#if ENABLE(CSS_EXCLUSIONS)
- , m_exclusionShapeInsideInfo(0)
-#endif
{
ASSERT(m_next);
@@ -139,21 +139,21 @@
LayoutState::LayoutState(RenderObject* root)
: m_clipped(false)
, m_isPaginated(false)
+ , m_pageLogicalHeightChanged(false)
#if !ASSERT_DISABLED && ENABLE(SATURATED_LAYOUT_ARITHMETIC)
, m_layoutDeltaXSaturated(false)
, m_layoutDeltaYSaturated(false)
#endif
- , m_pageLogicalHeight(0)
- , m_pageLogicalHeightChanged(false)
, m_columnInfo(0)
, m_lineGrid(0)
, m_next(0)
+#if ENABLE(CSS_EXCLUSIONS)
+ , m_exclusionShapeInsideInfo(0)
+#endif
+ , m_pageLogicalHeight(0)
#ifndef NDEBUG
, m_renderer(root)
#endif
-#if ENABLE(CSS_EXCLUSIONS)
- , m_exclusionShapeInsideInfo(0)
-#endif
{
RenderObject* container = root->container();
FloatPoint absContentPoint = container->localToAbsolute(FloatPoint(), UseTransforms | SnapOffsetForTransforms);
Modified: trunk/Source/WebCore/rendering/LayoutState.h (133712 => 133713)
--- trunk/Source/WebCore/rendering/LayoutState.h 2012-11-07 03:33:38 UTC (rev 133712)
+++ trunk/Source/WebCore/rendering/LayoutState.h 2012-11-07 04:14:48 UTC (rev 133713)
@@ -48,21 +48,21 @@
LayoutState()
: m_clipped(false)
, m_isPaginated(false)
+ , m_pageLogicalHeightChanged(false)
#if !ASSERT_DISABLED && ENABLE(SATURATED_LAYOUT_ARITHMETIC)
, m_layoutDeltaXSaturated(false)
, m_layoutDeltaYSaturated(false)
#endif
- , m_pageLogicalHeight(0)
- , m_pageLogicalHeightChanged(false)
, m_columnInfo(0)
, m_lineGrid(0)
, m_next(0)
+#if ENABLE(CSS_EXCLUSIONS)
+ , m_exclusionShapeInsideInfo(0)
+#endif
+ , m_pageLogicalHeight(0)
#ifndef NDEBUG
, m_renderer(0)
#endif
-#if ENABLE(CSS_EXCLUSIONS)
- , m_exclusionShapeInsideInfo(0)
-#endif
{
}
@@ -111,12 +111,24 @@
void computeLineGridPaginationOrigin(RenderBox*);
public:
- bool m_clipped;
- bool m_isPaginated;
+ // Do not add anything apart from bitfields until after m_columnInfo. See https://bugs.webkit.org/show_bug.cgi?id=100173
+ bool m_clipped:1;
+ bool m_isPaginated:1;
+ // If our page height has changed, this will force all blocks to relayout.
+ bool m_pageLogicalHeightChanged:1;
#if !ASSERT_DISABLED && ENABLE(SATURATED_LAYOUT_ARITHMETIC)
- bool m_layoutDeltaXSaturated;
- bool m_layoutDeltaYSaturated;
+ bool m_layoutDeltaXSaturated:1;
+ bool m_layoutDeltaYSaturated:1;
#endif
+ // If the enclosing pagination model is a column model, then this will store column information for easy retrieval/manipulation.
+ ColumnInfo* m_columnInfo;
+ // The current line grid that we're snapping to and the offset of the start of the grid.
+ RenderBlock* m_lineGrid;
+ LayoutState* m_next;
+#if ENABLE(CSS_EXCLUSIONS)
+ ExclusionShapeInsideInfo* m_exclusionShapeInsideInfo;
+#endif
+
LayoutRect m_clipRect;
// x/y offset from container. Includes relative positioning and scroll offsets.
@@ -130,25 +142,14 @@
// The current page height for the pagination model that encloses us.
LayoutUnit m_pageLogicalHeight;
- // If our page height has changed, this will force all blocks to relayout.
- bool m_pageLogicalHeightChanged;
// The offset of the start of the first page in the nearest enclosing pagination model.
LayoutSize m_pageOffset;
- // If the enclosing pagination model is a column model, then this will store column information for easy retrieval/manipulation.
- ColumnInfo* m_columnInfo;
-
- // The current line grid that we're snapping to and the offset of the start of the grid.
- RenderBlock* m_lineGrid;
LayoutSize m_lineGridOffset;
LayoutSize m_lineGridPaginationOrigin;
- LayoutState* m_next;
#ifndef NDEBUG
RenderObject* m_renderer;
#endif
-#if ENABLE(CSS_EXCLUSIONS)
- ExclusionShapeInsideInfo* m_exclusionShapeInsideInfo;
-#endif
};
} // namespace WebCore
