Fix dump-class-layout to show bit padding, and fix issues with padding offsets
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2018 23:07:26 +0000 (23:07 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2018 23:07:26 +0000 (23:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187442

Reviewed by Daniel Bates.

Synthetic padding members were shown with the wrong offset because they used the
absolute offset rather than the class-relative offset. This didn't change the padding
math, but made the output confusing.

Also add support for showing empty bits in bitfields, and fix computation of padding
after bitfields. Empty bits are computed by inserting a bit padding member after
a bitfield that is not followed by another bitfield (making the assumption that bit
padding will fill to the next byte boundary).

The computation of padding after bitfields was also wrong, since lldb's member_type.GetByteSize()
just reports the size of the type without the bitfield modifier (e.g. for "unsigned : 2" it returned 4).
Fix by setting the byte size for bitfield fields to the number of bits rounded up to the next byte;
this allows byte padding following the bitfield to be computed correctly.

Add or modify test to cover these issues.

* lldb/dump_class_layout_unittest.py:
(serial_test_ClassWithPaddedBitfields):
(serial_test_MemberHasBitfieldPadding):
(serial_test_InheritsFromClassWithPaddedBitfields):
* lldb/lldbWebKitTester/DumpClassLayoutTesting.cpp:
(avoidClassDeadStripping):
* lldb/lldb_dump_class_layout.py:
(ClassLayoutBase):
(ClassLayoutBase._to_string_recursive):
(ClassLayout._parse):
(ClassLayout._compute_padding_recursive):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233661 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/lldb/dump_class_layout_unittest.py
Tools/lldb/lldbWebKitTester/DumpClassLayoutTesting.cpp
Tools/lldb/lldb_dump_class_layout.py

index a7c2dd6..ea2de1e 100644 (file)
@@ -1,3 +1,38 @@
+2018-07-09  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix dump-class-layout to show bit padding, and fix issues with padding offsets
+        https://bugs.webkit.org/show_bug.cgi?id=187442
+
+        Reviewed by Daniel Bates.
+        
+        Synthetic padding members were shown with the wrong offset because they used the
+        absolute offset rather than the class-relative offset. This didn't change the padding
+        math, but made the output confusing.
+        
+        Also add support for showing empty bits in bitfields, and fix computation of padding
+        after bitfields. Empty bits are computed by inserting a bit padding member after
+        a bitfield that is not followed by another bitfield (making the assumption that bit
+        padding will fill to the next byte boundary).
+        
+        The computation of padding after bitfields was also wrong, since lldb's member_type.GetByteSize()
+        just reports the size of the type without the bitfield modifier (e.g. for "unsigned : 2" it returned 4).
+        Fix by setting the byte size for bitfield fields to the number of bits rounded up to the next byte;
+        this allows byte padding following the bitfield to be computed correctly.
+        
+        Add or modify test to cover these issues.
+
+        * lldb/dump_class_layout_unittest.py:
+        (serial_test_ClassWithPaddedBitfields):
+        (serial_test_MemberHasBitfieldPadding):
+        (serial_test_InheritsFromClassWithPaddedBitfields):
+        * lldb/lldbWebKitTester/DumpClassLayoutTesting.cpp:
+        (avoidClassDeadStripping):
+        * lldb/lldb_dump_class_layout.py:
+        (ClassLayoutBase):
+        (ClassLayoutBase._to_string_recursive):
+        (ClassLayout._parse):
+        (ClassLayout._compute_padding_recursive):
+
 2018-07-09  Youenn Fablet  <youenn@apple.com>
 
         StringView operator==(char*) should check the length of the string
index af80ccf..10427b0 100755 (executable)
@@ -108,7 +108,7 @@ Padding percentage: 37.50 %"""
   +3 <  1>   <PADDING: 1 byte>
   +4 <  8>     BoolMemberFirst memberClass
   +4 <  1>       bool boolMember
-  +9 <  3>       <PADDING: 3 bytes>
+  +5 <  3>       <PADDING: 3 bytes>
   +8 <  4>       int intMember
 Total byte size: 12
 Total pad bytes: 4
@@ -196,7 +196,7 @@ Padding percentage: 29.17 %"""
  +24 <  8>         BasicClassLayout BasicClassLayout
  +24 <  4>           int intMember
  +28 <  1>           bool boolMember
- +45 <  3>       <PADDING: 3 bytes>
+ +29 <  3>       <PADDING: 3 bytes>
  +32 <  4>       int intMemberB
  +36 <  4>   <PADDING: 4 bytes>
  +40 <  8>   double derivedMember
@@ -221,9 +221,9 @@ Padding percentage: 28.12 %"""
  +24 <  8>         BasicClassLayout BasicClassLayout
  +24 <  4>           int intMember
  +28 <  1>           bool boolMember
- +45 <  3>       <PADDING: 3 bytes>
+ +29 <  3>       <PADDING: 3 bytes>
  +32 <  4>       int intMemberB
- +52 <  4>       <PADDING: 4 bytes>
+ +36 <  4>       <PADDING: 4 bytes>
  +40 < 16>         VirtualBase VirtualBase
  +40 <  8>            __vtbl_ptr_type * _vptr
  +48 <  1>           bool baseMember
@@ -251,7 +251,7 @@ Padding percentage: 31.25 %"""
  +24 <  8>             BasicClassLayout BasicClassLayout
  +24 <  4>               int intMember
  +28 <  1>               bool boolMember
- +45 <  3>           <PADDING: 3 bytes>
+ +29 <  3>           <PADDING: 3 bytes>
  +32 <  4>           int intMemberB
  +36 <  4>       <PADDING: 4 bytes>
  +40 <  8>       double derivedMember
@@ -298,19 +298,93 @@ Padding percentage: 38.89 %"""
     def serial_test_ClassWithBitfields(self):
         EXPECTED_RESULT = """  +0 < 12> ClassWithBitfields
   +0 <  1>   bool boolMember
-  +1 <  4>   unsigned int bitfield1 : 1
-  +1 <  4>   unsigned int bitfield2 : 2
-  +1 <  4>   unsigned int bitfield3 : 1
-  +1 <  1>   bool bitfield4 : 1
-  +1 <  1>   bool bitfield5 : 2
-  +1 <  1>   bool bitfield6 : 1
+  +1 < :1>   unsigned int bitfield1 : 1
+  +1 < :2>   unsigned int bitfield2 : 2
+  +1 < :1>   unsigned int bitfield3 : 1
+  +1 < :1>   bool bitfield4 : 1
+  +1 < :2>   bool bitfield5 : 2
+  +1 < :1>   bool bitfield6 : 1
   +2 <  2>   <PADDING: 2 bytes>
   +4 <  4>   int intMember
-  +8 <  4>   unsigned int bitfield7 : 1
-  +8 <  1>   bool bitfield8 : 1
+  +8 < :1>   unsigned int bitfield7 : 1
+  +8 < :1>   bool bitfield8 : 1
+  +8 < :6>   <UNUSED BITS: 6 bits>
   +9 <  3>   <PADDING: 3 bytes>
 Total byte size: 12
 Total pad bytes: 5
 Padding percentage: 41.67 %"""
         actual_layout = debugger_instance.layout_for_classname('ClassWithBitfields')
         self.assertEqual(EXPECTED_RESULT, actual_layout.as_string())
+
+    def serial_test_ClassWithPaddedBitfields(self):
+        EXPECTED_RESULT = """  +0 < 16> ClassWithPaddedBitfields
+  +0 <  1>   bool boolMember
+  +1 < :1>   unsigned int bitfield1 : 1
+  +1 < :1>   bool bitfield2 : 1
+  +1 < :2>   unsigned int bitfield3 : 2
+  +1 < :1>   unsigned int bitfield4 : 1
+  +1 < :2>   unsigned long bitfield5 : 2
+  +1 < :1>   <UNUSED BITS: 1 bit>
+  +2 <  2>   <PADDING: 2 bytes>
+  +4 <  4>   int intMember
+  +8 < :1>   unsigned int bitfield7 : 1
+  +8 < :9>   unsigned int bitfield8 : 9
+  +9 < :1>   bool bitfield9 : 1
+  +9 < :5>   <UNUSED BITS: 5 bits>
+ +10 <  6>   <PADDING: 6 bytes>
+Total byte size: 16
+Total pad bytes: 8
+Padding percentage: 50.00 %"""
+        actual_layout = debugger_instance.layout_for_classname('ClassWithPaddedBitfields')
+        self.assertEqual(EXPECTED_RESULT, actual_layout.as_string())
+
+    def serial_test_MemberHasBitfieldPadding(self):
+        EXPECTED_RESULT = """  +0 < 24> MemberHasBitfieldPadding
+  +0 < 16>     ClassWithPaddedBitfields bitfieldMember
+  +0 <  1>       bool boolMember
+  +1 < :1>       unsigned int bitfield1 : 1
+  +1 < :1>       bool bitfield2 : 1
+  +1 < :2>       unsigned int bitfield3 : 2
+  +1 < :1>       unsigned int bitfield4 : 1
+  +1 < :2>       unsigned long bitfield5 : 2
+  +1 < :1>       <UNUSED BITS: 1 bit>
+  +2 <  2>       <PADDING: 2 bytes>
+  +4 <  4>       int intMember
+  +8 < :1>       unsigned int bitfield7 : 1
+  +8 < :9>       unsigned int bitfield8 : 9
+  +9 < :1>       bool bitfield9 : 1
+  +9 < :5>       <UNUSED BITS: 5 bits>
+ +10 <  6>   <PADDING: 6 bytes>
+ +16 < :1>   bool bitfield1 : 1
+ +16 < :7>   <UNUSED BITS: 7 bits>
+ +17 <  7>   <PADDING: 7 bytes>
+Total byte size: 24
+Total pad bytes: 15
+Padding percentage: 62.50 %"""
+        actual_layout = debugger_instance.layout_for_classname('MemberHasBitfieldPadding')
+        self.assertEqual(EXPECTED_RESULT, actual_layout.as_string())
+
+    def serial_test_InheritsFromClassWithPaddedBitfields(self):
+        EXPECTED_RESULT = """  +0 < 16> InheritsFromClassWithPaddedBitfields
+  +0 < 16>     ClassWithPaddedBitfields ClassWithPaddedBitfields
+  +0 <  1>       bool boolMember
+  +1 < :1>       unsigned int bitfield1 : 1
+  +1 < :1>       bool bitfield2 : 1
+  +1 < :2>       unsigned int bitfield3 : 2
+  +1 < :1>       unsigned int bitfield4 : 1
+  +1 < :2>       unsigned long bitfield5 : 2
+  +1 < :1>       <UNUSED BITS: 1 bit>
+  +2 <  2>       <PADDING: 2 bytes>
+  +4 <  4>       int intMember
+  +8 < :1>       unsigned int bitfield7 : 1
+  +8 < :9>       unsigned int bitfield8 : 9
+  +9 < :1>       bool bitfield9 : 1
+  +9 < :5>       <UNUSED BITS: 5 bits>
+ +10 < :1>   bool derivedBitfield : 1
+ +10 < :7>   <UNUSED BITS: 7 bits>
+ +11 <  5>   <PADDING: 5 bytes>
+Total byte size: 16
+Total pad bytes: 7
+Padding percentage: 43.75 %"""
+        actual_layout = debugger_instance.layout_for_classname('InheritsFromClassWithPaddedBitfields')
+        self.assertEqual(EXPECTED_RESULT, actual_layout.as_string())
index ce48eca..55503ec 100644 (file)
@@ -26,7 +26,6 @@
 #include "DumpClassLayoutTesting.h"
 
 #include <memory>
-#include <wtf/Optional.h>
 
 /*
 *** Dumping AST Record Layout
@@ -437,6 +436,83 @@ class ClassWithBitfields {
     bool bitfield8 : 1;
 };
 
+/*
+*** Dumping AST Record Layout
+         0 | class ClassWithPaddedBitfields
+         0 |   _Bool boolMember
+     1:0-0 |   unsigned int bitfield1
+     1:1-1 |   _Bool bitfield2
+     1:2-3 |   unsigned int bitfield3
+     1:4-4 |   unsigned int bitfield4
+     1:5-6 |   unsigned long bitfield5
+         4 |   int intMember
+     8:0-0 |   unsigned int bitfield7
+     8:1-9 |   unsigned int bitfield8
+     9:2-2 |   _Bool bitfield9
+           | [sizeof=16, dsize=10, align=8,
+           |  nvsize=10, nvalign=8]
+*/
+class ClassWithPaddedBitfields {
+    bool boolMember;
+
+    unsigned bitfield1 : 1;
+    bool bitfield2 : 1;
+    unsigned bitfield3 : 2;
+    unsigned bitfield4 : 1;
+    unsigned long bitfield5: 2;
+
+    int intMember;
+
+    unsigned bitfield7 : 1;
+    unsigned bitfield8 : 9;
+    bool bitfield9 : 1;
+};
+
+/*
+*** Dumping AST Record Layout
+         0 | class MemberHasBitfieldPadding
+         0 |   class ClassWithPaddedBitfields bitfieldMember
+         0 |     _Bool boolMember
+     1:0-0 |     unsigned int bitfield1
+     1:1-1 |     _Bool bitfield2
+     1:2-3 |     unsigned int bitfield3
+     1:4-4 |     unsigned int bitfield4
+     1:5-6 |     unsigned long bitfield5
+         4 |     int intMember
+     8:0-0 |     unsigned int bitfield7
+     8:1-9 |     unsigned int bitfield8
+     9:2-2 |     _Bool bitfield9
+    16:0-0 |   _Bool bitfield1
+           | [sizeof=24, dsize=17, align=8,
+           |  nvsize=17, nvalign=8]
+*/
+class MemberHasBitfieldPadding {
+    ClassWithPaddedBitfields bitfieldMember;
+    bool bitfield1 : 1;
+};
+
+/*
+*** Dumping AST Record Layout
+         0 | class InheritsFromClassWithPaddedBitfields
+         0 |   class ClassWithPaddedBitfields (base)
+         0 |     _Bool boolMember
+     1:0-0 |     unsigned int bitfield1
+     1:1-1 |     _Bool bitfield2
+     1:2-3 |     unsigned int bitfield3
+     1:4-4 |     unsigned int bitfield4
+     1:5-6 |     unsigned long bitfield5
+         4 |     int intMember
+     8:0-0 |     unsigned int bitfield7
+     8:1-9 |     unsigned int bitfield8
+     9:2-2 |     _Bool bitfield9
+    10:0-0 |   _Bool derivedBitfield
+           | [sizeof=16, dsize=11, align=8,
+           |  nvsize=11, nvalign=8]
+*/
+class InheritsFromClassWithPaddedBitfields : public ClassWithPaddedBitfields {
+    bool derivedBitfield : 1;
+};
+
 void avoidClassDeadStripping()
 {
     BasicClassLayout basicClassInstance;
@@ -453,4 +529,7 @@ void avoidClassDeadStripping()
     ClassWithClassMembers classWithClassMembersInstance;
     ClassWithPointerMember classWithPointerMemberInstance;
     ClassWithBitfields classWithBitfieldsInstance;
+    ClassWithPaddedBitfields classWithPaddedBitfieldsInstance;
+    MemberHasBitfieldPadding memberHasBitfieldPaddingInstance;
+    InheritsFromClassWithPaddedBitfields inheritsFromClassWithPaddedBitfieldsInstance;
 }
index 3ffa3c1..3bebf8f 100755 (executable)
@@ -50,9 +50,12 @@ class ClassLayoutBase(object):
     MEMBER_CLASS_INSTANCE = 'class_instance'
     MEMBER_BYTE_SIZE = 'byte_size'
     MEMBER_OFFSET = 'offset'  # offset is local to this class.
+    MEMBER_OFFSET_IN_BITS = 'offset_in_bits'
     MEMBER_IS_BITFIELD = 'is_bitfield'
     MEMBER_BITFIELD_BIT_SIZE = 'bit_size'
-    PADDING_TYPE = '<PADDING>'
+    PADDING_BYTES_TYPE = 'padding'
+    PADDING_BITS_TYPE = 'padding_bits'
+    PADDING_BITS_SIZE = 'padding_bits_size'
     PADDING_NAME = ''
 
     def __init__(self, typename):
@@ -105,9 +108,13 @@ class ClassLayoutBase(object):
                 byte_size = data_member[self.MEMBER_BYTE_SIZE]
 
                 if self.MEMBER_IS_BITFIELD in data_member:
-                    str_list.append('%+4u <%3u> %s  %s %s : %d' % (member_total_offset, byte_size, '    ' * depth, data_member[self.MEMBER_TYPE_KEY], data_member[self.MEMBER_NAME_KEY], data_member[self.MEMBER_BITFIELD_BIT_SIZE]))
-                elif data_member[self.MEMBER_TYPE_KEY] == self.PADDING_TYPE:
+                    num_bits = data_member[self.MEMBER_BITFIELD_BIT_SIZE]
+                    str_list.append('%+4u < :%1u> %s  %s %s : %d' % (member_total_offset, num_bits, '    ' * depth, data_member[self.MEMBER_TYPE_KEY], data_member[self.MEMBER_NAME_KEY], num_bits))
+                elif data_member[self.MEMBER_TYPE_KEY] == self.PADDING_BYTES_TYPE:
                     str_list.append('%+4u <%3u> %s  %s<PADDING: %d %s>%s' % (member_total_offset, byte_size, '    ' * depth, warn_start, byte_size, 'bytes' if byte_size > 1 else 'byte', color_end))
+                elif data_member[self.MEMBER_TYPE_KEY] == self.PADDING_BITS_TYPE:
+                    padding_bits = data_member[self.PADDING_BITS_SIZE]
+                    str_list.append('%+4u < :%1u> %s  %s<UNUSED BITS: %d %s>%s' % (member_total_offset, padding_bits, '    ' * depth, warn_start, padding_bits, 'bits' if padding_bits > 1 else 'bit', color_end))
                 else:
                     str_list.append('%+4u <%3u> %s  %s %s' % (member_total_offset, byte_size, '    ' * depth, data_member[self.MEMBER_TYPE_KEY], data_member[self.MEMBER_NAME_KEY]))
 
@@ -231,6 +238,9 @@ class ClassLayout(ClassLayoutBase):
             if field.IsBitfield():
                 data_member[self.MEMBER_IS_BITFIELD] = True
                 data_member[self.MEMBER_BITFIELD_BIT_SIZE] = field.GetBitfieldSizeInBits()
+                data_member[self.MEMBER_OFFSET_IN_BITS] = field.GetOffsetInBits()
+                # For bitfields, member_byte_size was computed based on the field type without the bitfield modifiers, so compute from the number of bits.
+                data_member[self.MEMBER_BYTE_SIZE] = (field.GetBitfieldSizeInBits() + 7) / 8
             elif member_type_class == lldb.eTypeClassStruct or member_type_class == lldb.eTypeClassClass:
                 nested_class = ClassLayout(self.target, member_type, self)
                 data_member[self.MEMBER_CLASS_INSTANCE] = nested_class
@@ -302,15 +312,35 @@ class ClassLayout(ClassLayoutBase):
                 if padding_size > 0:
                     padding_member = {
                         self.MEMBER_NAME_KEY : self.PADDING_NAME,
-                        self.MEMBER_TYPE_KEY : self.PADDING_TYPE,
+                        self.MEMBER_TYPE_KEY : self.PADDING_BYTES_TYPE,
                         self.MEMBER_BYTE_SIZE : padding_size,
-                        self.MEMBER_OFFSET : current_offset,
+                        self.MEMBER_OFFSET : current_offset - start_offset,
                     }
 
                     self.data_members.insert(i, padding_member)
                     padding_bytes += padding_size
                     i += 1
 
+                if self.MEMBER_IS_BITFIELD in data_member:
+                    next_member_is_bitfield = False
+                    if i < len(self.data_members) - 1:
+                        next_data_member = self.data_members[i + 1]
+                        next_member_is_bitfield = self.MEMBER_IS_BITFIELD in next_data_member
+
+                    if not next_member_is_bitfield:
+                        end_bit_offset = data_member[self.MEMBER_OFFSET_IN_BITS] + data_member[self.MEMBER_BITFIELD_BIT_SIZE]
+                        unused_bits = (8 - end_bit_offset) % 8
+                        if unused_bits:
+                            bit_padding_member = {
+                                self.MEMBER_NAME_KEY : self.PADDING_NAME,
+                                self.MEMBER_TYPE_KEY : self.PADDING_BITS_TYPE,
+                                self.MEMBER_BYTE_SIZE : data_member[self.MEMBER_BYTE_SIZE],
+                                self.PADDING_BITS_SIZE : unused_bits,
+                                self.MEMBER_OFFSET : data_member[self.MEMBER_OFFSET],
+                            }
+                            self.data_members.insert(i + 1, bit_padding_member)
+                            i += 1
+
                 current_offset = start_offset + member_offset
 
             if self.MEMBER_CLASS_INSTANCE in data_member:
@@ -328,9 +358,9 @@ class ClassLayout(ClassLayoutBase):
             if padding_size > 0:
                 padding_member = {
                     self.MEMBER_NAME_KEY : self.PADDING_NAME,
-                    self.MEMBER_TYPE_KEY : self.PADDING_TYPE,
+                    self.MEMBER_TYPE_KEY : self.PADDING_BYTES_TYPE,
                     self.MEMBER_BYTE_SIZE : padding_size,
-                    self.MEMBER_OFFSET : current_offset,
+                    self.MEMBER_OFFSET : current_offset - start_offset,
                 }
                 self.data_members.append(padding_member)
                 padding_bytes += padding_size