Bug 187442

Summary: Fix dump-class-layout to show bit padding, and fix issues with padding offsets
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, ews-watchlist, simon.fraser, webkit-bug-importer, ysuzuki, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
dbates: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews206 for win-future none

Description Simon Fraser (smfr) 2018-07-07 17:23:18 PDT
Fix dump-class-layout to show bit padding, and fix issues with padding offsets
Comment 1 Simon Fraser (smfr) 2018-07-07 17:29:34 PDT
Created attachment 344533 [details]
Patch
Comment 2 Simon Fraser (smfr) 2018-07-07 17:30:08 PDT
Example:

 23$ $ ./Tools/Scripts/dump-class-layout WebCore InlineTextBox
  +0 < 88> InlineTextBox
  +0 < 64>     WebCore::InlineBox WebCore::InlineBox
  +0 <  8>        __vtbl_ptr_type * _vptr
  +8 <  8>       WebCore::InlineBox * m_next
 +16 <  8>       WebCore::InlineBox * m_prev
 +24 <  8>       WebCore::InlineFlowBox * m_parent
 +32 <  8>       WebCore::RenderObject & m_renderer
 +40 <  4>       float m_expansion
 +44 <  4>         WebCore::InlineBox::InlineBoxBitfields m_bitfields
 +44 <  1>           unsigned int m_firstLine : 1
 +44 <  1>           unsigned int m_constructed : 1
 +44 <  1>           unsigned int m_bidiEmbeddingLevel : 6
 +45 <  1>           unsigned int m_dirty : 1
 +45 <  1>           unsigned int m_extracted : 1
 +45 <  1>           unsigned int m_hasVirtualLogicalHeight : 1
 +45 <  1>           unsigned int m_isHorizontal : 1
 +45 <  1>           unsigned int m_endsWithBreak : 1
 +45 <  1>           unsigned int m_hasSelectedChildrenOrCanHaveLeadingExpansion : 1
 +45 <  1>           unsigned int m_canHaveTrailingExpansion : 1
 +45 <  1>           unsigned int m_knownToHaveNoOverflow : 1
 +46 <  1>           unsigned int m_hasEllipsisBoxOrHyphen : 1
 +46 <  1>           unsigned int m_dirOverride : 1
 +46 <  1>           unsigned int m_behavesLikeText : 1
 +46 <  1>           unsigned int m_forceTrailingExpansion : 1
 +46 <  1>           unsigned int m_forceLeadingExpansion : 1
 +46 <  1>           unsigned int m_determinedIfNextOnLineExists : 1
 +46 <  1>           unsigned int m_nextOnLineExists : 1
 +46 <  1>           <PADDING BITS: 1 bit>
 +47 <  1>       <PADDING: 1 byte>
 +48 <  8>         WebCore::FloatPoint m_topLeft
 +48 <  4>           float m_x
 +52 <  4>           float m_y
 +56 <  4>       float m_logicalWidth
 +60 <  4>   <PADDING: 4 bytes>
 +64 <  8>   WebCore::InlineTextBox * m_prevTextBox
 +72 <  8>   WebCore::InlineTextBox * m_nextTextBox
 +80 <  4>   unsigned int m_start
 +84 <  2>   unsigned short m_len
 +86 <  2>   unsigned short m_truncation
Total byte size: 88
Total pad bytes: 5
Padding percentage: 5.68 %
Comment 3 EWS Watchlist 2018-07-07 21:25:51 PDT
Comment on attachment 344533 [details]
Patch

Attachment 344533 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8470795

New failing tests:
http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html
http/tests/security/canvas-remote-read-remote-video-localhost.html
Comment 4 EWS Watchlist 2018-07-07 21:26:02 PDT
Created attachment 344537 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 5 Alexey Proskuryakov 2018-07-07 21:43:59 PDT
Comment on attachment 344533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344533&action=review

> Tools/lldb/lldb_dump_class_layout.py:116
> +                    str_list.append('%+4u <%3u> %s  %s<PADDING BITS: %d %s>%s' % (member_total_offset, byte_size, '    ' * depth, warn_start, padding_bits, 'bits' if padding_bits > 1 else 'bit', color_end))

Is it accurate to call these bits “padding”? Or just “unused”?
Comment 6 Simon Fraser (smfr) 2018-07-08 19:04:09 PDT
"Unused" would be better, yeah.
Comment 7 Daniel Bates 2018-07-09 13:55:58 PDT
Comment on attachment 344533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344533&action=review

> Tools/lldb/dump_class_layout_unittest.py:301
> -  +1 <  4>   unsigned int bitfield3 : 1
> +  +1 <  1>   unsigned int bitfield1 : 1

This output is a progression. I find it still confusing because the size of this bit in the bitfield is reported as 1 byte ("< 1>"). Maybe if we emitted the number of bits of the field as "1 bit", "2 bits", et cetera when the bitfield width is less than a byte it would help make this output easier to understand. If we did this then we could change "<PADDING BITS: ... >" to "<PADDING>" since we will report the number of bits of padding in the column to the left of this message.
Comment 8 Simon Fraser (smfr) 2018-07-09 16:30:20 PDT
https://trac.webkit.org/changeset/233661/webkit
Comment 9 Radar WebKit Bug Importer 2018-07-09 16:32:21 PDT
<rdar://problem/41996211>