REGRESSION: (r255611) [ Mac ] 3 lldb tests failing related to HashMap

• Component: New Bugs
• Status: RESOLVED FIXED

[Truitt Savell]

3 lldb tests failing related to HashMap

[8/42] lldb_webkit_unittest.TestSummaryProviders.serial_test_WTFHashMap_of_vectors_tablesize_and_size failed:
  Traceback (most recent call last):
    File "/Volumes/Data/slave/mojave-release-tests-wk1/build/Tools/lldb/lldb_webkit_unittest.py", line 164, in serial_test_WTFHashMap_of_vectors_tablesize_and_size
      self.assertEqual(summary, "{ tableSize = 8, keyCount = 1 }")
  AssertionError: '{ tableSize = 0, keyCount = 0 }' != '{ tableSize = 8, keyCount = 1 }'

[31/42] lldb_webkit_unittest.TestSummaryProviders.serial_test_WTFHashSet_tablesize_and_size failed:
  Traceback (most recent call last):
    File "/Volumes/Data/slave/mojave-release-tests-wk1/build/Tools/lldb/lldb_webkit_unittest.py", line 169, in serial_test_WTFHashSet_tablesize_and_size
      self.assertEqual(summary, "{ tableSize = 8, keyCount = 1 }")
  AssertionError: '{ tableSize = 0, keyCount = 0 }' != '{ tableSize = 8, keyCount = 1 }'

[36/42] lldb_webkit_unittest.TestSummaryProviders.serial_test_WTFHashMap_tablesize_and_size failed:
  Traceback (most recent call last):
    File "/Volumes/Data/slave/mojave-release-tests-wk1/build/Tools/lldb/lldb_webkit_unittest.py", line 159, in serial_test_WTFHashMap_tablesize_and_size
      self.assertEqual(summary, "{ tableSize = 8, keyCount = 2 }")
  AssertionError: '{ tableSize = 0, keyCount = 0 }' != '{ tableSize = 8, keyCount = 2 }'

It looks like the changes in https://trac.webkit.org/changeset/255611/webkit are responsible for this.

[Radar WebKit Bug Importer]

<rdar://problem/59150608>

[Simon Fraser (smfr)]

The lldb formatters need to be fixed.

[Alex Christensen]

Created attachment 389678 [details]
Patch

[Alex Christensen]

This rebases the expectations.  I don't think it's possible to tell lldb through python to take an SBValue (from GetChildMemberWithName('m_table')) reinterpret_cast it to a uint32_t*, then read negative offsets.  To do so we would need a way to make a SBType representing uint32_t out of nothing, which seems to be impossible in their API.

[Simon Fraser (smfr)]

Are you saying that we're all going to see incorrect HashMap and HashSet summaries in Xcode now?

[Alex Christensen]

That's exactly what I'm saying.  The LLDB python API seems unable to do negative index dereferencing and reinterpret_cast equivalent.  This patch will make the bots tests fixed until we figure that out.

[Alex Christensen]

Created attachment 389725 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 389725 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 389725 [details]
Patch

Clearing flags on attachment: 389725

Committed r255780: <https://trac.webkit.org/changeset/255780>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alex Christensen]

Reopening to attach new patch.

[Alex Christensen]

Created attachment 389863 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 389863 [details]
Patch

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

> Source/WTF/wtf/HashTable.h:546
> +        union {
> +            ValueType* m_table { nullptr };
> +            unsigned* m_tableForLLDB;
> +        };

Do we really want to pollute HashTable code just to make formatters work?

[Alex Christensen]

I think this is minimal pollution.

[Alex Christensen]

Dan Bates, do you think we should re-add this, or should we just have no ability to LLDB debug HashTable-based structures?

[Daniel Bates]

(In reply to Alex Christensen from comment #15)
> Dan Bates, do you think we should re-add this, or should we just have no
> ability to LLDB debug HashTable-based structures?

🤷‍♂️ If others think it is beneficial. I'll let you decide.

[Alex Christensen]

Comment on attachment 389863 [details]
Patch

Cameron said he'd like to use this.  I'm re-requesting review.

[EWS]

Committed r274463: <https://commits.webkit.org/r274463>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 389863 [details].