Bug 187132

Summary: The lldb vector summary provider always shows zero capacity
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, ddkilzer, jer.noble, realdawei, ryanhaddad, sam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=187229
Attachments:
Description Flags
Patch
dbates: review+
Patch for lldb_webkit.py only dbates: review+

Simon Fraser (smfr)
Reported 2018-06-27 21:02:21 PDT
The lldb vector summary provider always shows zero capacity
Attachments
Patch (3.63 KB, patch)
2018-06-27 21:03 PDT, Simon Fraser (smfr)
dbates: review+
Patch for lldb_webkit.py only (1.47 KB, patch)
2018-06-27 21:05 PDT, Simon Fraser (smfr)
dbates: review+
Simon Fraser (smfr)
Comment 1 2018-06-27 21:03:30 PDT
Simon Fraser (smfr)
Comment 2 2018-06-27 21:05:06 PDT
First patch adds a regression test based on the functionality added in bug 183744.
Simon Fraser (smfr)
Comment 3 2018-06-27 21:05:45 PDT
Created attachment 343790 [details] Patch for lldb_webkit.py only
Simon Fraser (smfr)
Comment 4 2018-06-28 08:38:10 PDT
Radar WebKit Bug Importer
Comment 5 2018-06-28 08:39:17 PDT
Simon Fraser (smfr)
Comment 6 2018-06-28 08:52:55 PDT
*** Bug 143921 has been marked as a duplicate of this bug. ***
Dawei Fenton (:realdawei)
Comment 7 2018-06-28 12:53:29 PDT
(In reply to Simon Fraser (smfr) from comment #4) > Thanks Dan! > https://trac.webkit.org/changeset/233306/webkit Looks like this patch is causing errors in Sierra debug, High Sierra debug and iOS sample run: https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/3876/steps/webkitpy-test/logs/stdio Traceback (most recent call last): File "/Volumes/Data/slave/highsierra-debug-tests-wk2/build/Tools/lldb/lldb_webkit_unittest.py", line 150, in serial_test_WTFVectorProvider_vector_size_and_capacity self.assertEqual(summary, "{ size = 1, capacity = 16 }") AssertionError: '{ size = 0, capacity = 0 }' != '{ size = 1, capacity = 16 }'
Simon Fraser (smfr)
Comment 8 2018-06-28 14:12:40 PDT
Cannot reproduce that failure locally.
Dawei Fenton (:realdawei)
Comment 9 2018-06-28 15:14:52 PDT
(In reply to Simon Fraser (smfr) from comment #8) > Cannot reproduce that failure locally. Interesting, it appears this is a new test added in this revision and has been failing since it was merged: ... def serial_test_WTFVectorProvider_vector_size_and_capacity(self): 149 summary = lldb_webkit.WTFVector_SummaryProvider(self._sbFrame.FindVariable('aVectorWithOneItem'), {}) 150 self.assertEqual(summary, "{ size = 1, capacity = 16 }") ... iOS: https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/5861/steps/webkitpy-test/logs/stdio iOS Debug https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20%28Tests%29/builds/5088/steps/webkitpy-test/logs/stdio High Sierra Debug https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/3876/steps/webkitpy-test/logs/stdio Sierra Debug: https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK2%20%28Tests%29/builds/7032/steps/webkitpy-test/logs/stdio
Ryan Haddad
Comment 10 2018-06-28 15:25:37 PDT
Looks like a fix was attempted in https://trac.webkit.org/changeset/233330/webkit
Ryan Haddad
Comment 11 2018-06-28 17:32:25 PDT
(In reply to Ryan Haddad from comment #10) > Looks like a fix was attempted in > https://trac.webkit.org/changeset/233330/webkit Still failing on the bots as of 233331: https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/builds/5876
David Kilzer (:ddkilzer)
Comment 12 2018-06-30 19:33:24 PDT
BTW, when running the test locally, it doesn't seem like Tools/lldb/lldbWebKitTester/main.cpp forces lldbWebKitTester to be rebuilt!
Daniel Bates
Comment 13 2018-06-30 20:01:51 PDT
(In reply to David Kilzer (:ddkilzer) from comment #12) > BTW, when running the test locally, it doesn't seem like > Tools/lldb/lldbWebKitTester/main.cpp forces lldbWebKitTester to be rebuilt! Filed bug #187229.
David Kilzer (:ddkilzer)
Comment 14 2018-06-30 21:49:31 PDT
(In reply to Ryan Haddad from comment #10) > Looks like a fix was attempted in > https://trac.webkit.org/changeset/233330/webkit So if I modify Tools/lldb/lldbWebKitTester/main.cpp like this: --- a/Tools/lldb/lldbWebKitTester/main.cpp +++ b/Tools/lldb/lldbWebKitTester/main.cpp @@ -60,6 +60,9 @@ static void testSummaryProviders() aVectorWithOneItem.reserveCapacity(16); aVectorWithOneItem.append(1); + fprintf(stderr, "anEmptyVector.size() = %zu capacity() = %zu\n", anEmptyVector.size(), anEmptyVector.capacity()); + fprintf(stderr, "aVectorWithOneItem.size() = %zu capacity() = %zu\n", aVectorWithOneItem.size(), aVectorWithOneItem.capacity()); + breakForTestingSummaryProviders(); } And then run it directly, I get this output: $ /var/build/Debug/lldbWebKitTester anEmptyVector.size() = 0 capacity() = 0 aVectorWithOneItem.size() = 1 capacity() = 16 This executable does nothing and is only meant for debugging lldb_webkit.py. So my guess is that the order/offset of the fields for WTF::Vector are wrong in Tools/lldb/lldb_webkit.py. Recall that WTF::Vector inherits from WTF::VectorBuffer which inherits from WTF::VectorBufferBase, and it's WTF::VectorBufferBase that defines the member variables in this order: T* m_buffer; unsigned m_capacity; unsigned m_size; // Only used by the Vector subclass, but placed here to avoid padding the struct. I tried "fixing" the order of the member variables in lldb_webkit.py, but that didn't work, and was just a blind guess anyway. (Can lldb get all these member various without casting the object to the base class first?)
David Kilzer (:ddkilzer)
Comment 15 2018-07-01 04:48:22 PDT
(In reply to Daniel Bates from comment #13) > (In reply to David Kilzer (:ddkilzer) from comment #12) > > BTW, when running the test locally, it doesn't seem like > > Tools/lldb/lldbWebKitTester/main.cpp forces lldbWebKitTester to be rebuilt! > > Filed bug #187229. So this appears to have fixed the test failures on the bots! I seem to be able to reproduce the failure locally, but I just realized (a) I'm using a Debug+ASan build and (b) I need to rebuild WebKit, too.
Daniel Bates
Comment 16 2018-07-01 06:28:10 PDT
(In reply to David Kilzer (:ddkilzer) from comment #15) > (In reply to Daniel Bates from comment #13) > > (In reply to David Kilzer (:ddkilzer) from comment #12) > > > BTW, when running the test locally, it doesn't seem like > > > Tools/lldb/lldbWebKitTester/main.cpp forces lldbWebKitTester to be rebuilt! > > > > Filed bug #187229. > > So this appears to have fixed the test failures on the bots! > Yay!
Note You need to log in before you can comment on or make changes to this bug.