Changeset 249030 in webkit

Timestamp:

Aug 22, 2019 3:06:54 PM (5 years ago)

Author:

dbates@webkit.org

Message:

[lldb-webkit] OptionSet summary shows size 0 sometimes for non-empty set
​https://bugs.webkit.org/show_bug.cgi?id=200742

Reviewed by Simon Fraser.

The OptionSet synthetic provider must respond to requests for the value of m_storage
(i.e. GetChildMemberWithName('m_storage')) to avoid interfering with the computation
of the type summary.

Synthetic providers substitute alternative debug information (children) for the default
information for a variable. The OptionSet type summary is implemented in terms of the
OptionSet synthetic provider to maximize code reuse. If LLDB instantiates the provider
before invoking the type summary handler then evaluating GetChildMemberWithName() on
the SBValue passed to the type summary handler will access the substitute information
instead of the original debug information. As a result OptionSet's synthetic provider's
get_child_index('m_storage') returns None hence SBValue.GetChildMemberWithName('m_storage')
returned an invalid value; => WTFOptionSetProvider._bitmask() returns 0; => the size
reported in the type summary for the OptionSet is 0. Instead get_child_index('m_storage')
should return a valid value.

• lldb/lldb_webkit.py:

(FlagEnumerationProvider.init):
(FlagEnumerationProvider):
(FlagEnumerationProvider._get_child_index): Added. WTFOptionSetProvider will override.
(FlagEnumerationProvider._get_child_at_index): Added. WTFOptionSetProvider will override.
(FlagEnumerationProvider.size): Added.
(FlagEnumerationProvider.get_child_index): Modified to call _get_child_index().
(FlagEnumerationProvider.get_child_at_index): Modified to call _get_child_at_index().
(FlagEnumerationProvider.update): Moved initialization of self._elements to the constructor
and removed self.size. For the latter we can just expose a getter that returns the size of
the list self._elements.
(WTFOptionSetProvider._get_child_index): Added. Return the index for LLDB to query for the
value of m_storage.
(WTFOptionSetProvider):
(WTFOptionSetProvider._get_child_at_index): Added. Return the value for m_storage if it
matches the specified index.

• lldb/lldb_webkit_unittest.py:

(TestSummaryProviders.serial_test_WTFOptionSetProvider_empty): Update expected result now
that we return the value of m_storage as the last synthetic child.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• lldb/lldb_webkit.py (modified) (6 diffs)
• lldb/lldb_webkit_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2019-08-22  Daniel Bates  <dabates@apple.com>
+
+        [lldb-webkit] OptionSet summary shows size 0 sometimes for non-empty set
+        https://bugs.webkit.org/show_bug.cgi?id=200742
+
+        Reviewed by Simon Fraser.
+
+        The OptionSet synthetic provider must respond to requests for the value of m_storage
+        (i.e. GetChildMemberWithName('m_storage')) to avoid interfering with the computation
+        of the type summary.
+
+        Synthetic providers substitute alternative debug information (children) for the default
+        information for a variable. The OptionSet type summary is implemented in terms of the
+        OptionSet synthetic provider to maximize code reuse. If LLDB instantiates the provider
+        before invoking the type summary handler then evaluating GetChildMemberWithName() on
+        the SBValue passed to the type summary handler will access the substitute information
+        instead of the original debug information. As a result OptionSet's synthetic provider's
+        get_child_index('m_storage') returns None hence SBValue.GetChildMemberWithName('m_storage')
+        returned an invalid value; => WTFOptionSetProvider._bitmask() returns 0; => the size
+        reported in the type summary for the OptionSet is 0. Instead get_child_index('m_storage')
+        should return a valid value.
+
+        * lldb/lldb_webkit.py:
+        (FlagEnumerationProvider.__init__):
+        (FlagEnumerationProvider):
+        (FlagEnumerationProvider._get_child_index): Added. WTFOptionSetProvider will override.
+        (FlagEnumerationProvider._get_child_at_index): Added. WTFOptionSetProvider will override.
+        (FlagEnumerationProvider.size): Added.
+        (FlagEnumerationProvider.get_child_index): Modified to call _get_child_index().
+        (FlagEnumerationProvider.get_child_at_index): Modified to call _get_child_at_index().
+        (FlagEnumerationProvider.update): Moved initialization of self._elements to the constructor
+        and removed self.size. For the latter we can just expose a getter that returns the size of
+        the list self._elements.
+        (WTFOptionSetProvider._get_child_index): Added. Return the index for LLDB to query for the
+        value of m_storage.
+        (WTFOptionSetProvider):
+        (WTFOptionSetProvider._get_child_at_index): Added. Return the value for m_storage if it
+        matches the specified index.
+        * lldb/lldb_webkit_unittest.py:
+        (TestSummaryProviders.serial_test_WTFOptionSetProvider_empty): Update expected result now
+        that we return the value of m_storage as the last synthetic child.
+
 2019-08-22  Jonathan Bedard  <jbedard@apple.com>
 

trunk/Tools/lldb/lldb_webkit.py

@@ -736 +736 @@
     def __init__(self, valobj, internal_dict):
         self.valobj = valobj
+        self._elements = []
         self.update()
 
@@ -751 +752 @@
         pass
 
+    # Subclasses can override this to provide the index that corresponds to the specified name.
+    # If this method is overridden then it is also expected that _get_child_at_index() will be
+    # overridden to provide the value for the index returned by this method. Note that the
+    # returned index must be greater than or equal to self.size in order to avoid breaking
+    # printing of synthetic children.
+    def _get_child_index(self, name):
+        return None
+
+    # Subclasses can override this to provide the SBValue for the specified index. It is only
+    # meaningful to override this method if _get_child_index() is also overridden.
+    def _get_child_at_index(self, index):
+        return None
+
+    @property
+    def size(self):
+        return len(self._elements)
+
+    # LLDB overrides
     def has_children(self):
         return bool(self._elements)
@@ -758 +777 @@
 
     def get_child_index(self, name):
-        try:
-            return int(name.lstrip('[').rstrip(']'))
-        except:
-            return None
+        return self._get_child_index(name)
 
     def get_child_at_index(self, index):
@@ -769 +785 @@
             (name, value) = self._elements[index]
             return self.valobj.CreateValueFromExpression(name, str(value))
-        return None
+        return self._get_child_at_index(index)
 
     def update(self):
         self._update()
-
-        self._elements = []
-        self.size = 0
 
         enumerator_value_to_name_map = self._enumerator_value_to_name_map()
@@ -794 +807 @@
             bitmask = bitmask & (bitmask - 1)  # Turn off the rightmost set bit.
         self._elements = elements
-        self.size = len(elements)
-
 
 class WTFOptionSetProvider(FlagEnumerationProvider):
@@ -812 +823 @@
     def _update(self):
         self.storage = self.valobj.GetChildMemberWithName('m_storage')  # May be an invalid value.
+
+    def _get_child_index(self, name):
+        if name == 'm_storage':
+            return self.size
+        return None
+
+    def _get_child_at_index(self, index):
+        if index == self.size:
+            return self.storage
+        return None
 
 

trunk/Tools/lldb/lldb_webkit_unittest.py

@@ -186 +186 @@
         variable = self._sbFrame.FindVariable('exampleFlagsEmpty')
         provider = lldb_webkit.WTFOptionSetProvider(variable, {})
-        self.assertEqual(provider.get_child_at_index(0), None)
+        self.assertEqual(provider.get_child_at_index(0).GetName(), 'm_storage')
 
     def serial_test_WTFOptionSetProvider_simple(self):