Bug 207204 - REGRESSION: (r255611) [ Mac ] 3 lldb tests failing related to HashMap
Summary: REGRESSION: (r255611) [ Mac ] 3 lldb tests failing related to HashMap
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-04 08:58 PST by Truitt Savell
Modified: 2020-03-23 10:45 PDT (History)
10 users (show)

See Also:


Attachments
Patch (2.12 KB, patch)
2020-02-04 10:43 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (8.16 KB, patch)
2020-02-04 15:32 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (10.50 KB, patch)
2020-02-05 13:49 PST, Alex Christensen
simon.fraser: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Truitt Savell 2020-02-04 08:58:39 PST
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.
Comment 1 Radar WebKit Bug Importer 2020-02-04 08:59:05 PST
<rdar://problem/59150608>
Comment 2 Simon Fraser (smfr) 2020-02-04 09:06:47 PST
The lldb formatters need to be fixed.
Comment 3 Alex Christensen 2020-02-04 10:43:13 PST
Created attachment 389678 [details]
Patch
Comment 4 Alex Christensen 2020-02-04 12:39:40 PST
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.
Comment 5 Simon Fraser (smfr) 2020-02-04 13:10:36 PST
Are you saying that we're all going to see incorrect HashMap and HashSet summaries in Xcode now?
Comment 6 Alex Christensen 2020-02-04 13:57:24 PST
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.
Comment 7 Alex Christensen 2020-02-04 15:32:14 PST
Created attachment 389725 [details]
Patch
Comment 8 WebKit Commit Bot 2020-02-04 17:46:29 PST
The commit-queue encountered the following flaky tests while processing attachment 389725 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 9 WebKit Commit Bot 2020-02-04 17:47:04 PST
Comment on attachment 389725 [details]
Patch

Clearing flags on attachment: 389725

Committed r255780: <https://trac.webkit.org/changeset/255780>
Comment 10 WebKit Commit Bot 2020-02-04 17:47:06 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Alex Christensen 2020-02-05 13:49:07 PST
Reopening to attach new patch.
Comment 12 Alex Christensen 2020-02-05 13:49:08 PST
Created attachment 389863 [details]
Patch
Comment 13 Simon Fraser (smfr) 2020-02-05 13:58:55 PST
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?
Comment 14 Alex Christensen 2020-02-05 15:23:03 PST
I think this is minimal pollution.
Comment 15 Alex Christensen 2020-03-19 17:05:49 PDT
Dan Bates, do you think we should re-add this, or should we just have no ability to LLDB debug HashTable-based structures?
Comment 16 Daniel Bates 2020-03-20 00:08:34 PDT
(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.