Bug 187132 - The lldb vector summary provider always shows zero capacity
Summary: The lldb vector summary provider always shows zero capacity
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
: 143921 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-06-27 21:02 PDT by Simon Fraser (smfr)
Modified: 2018-07-01 06:28 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.63 KB, patch)
2018-06-27 21:03 PDT, Simon Fraser (smfr)
dbates: review+
Details | Formatted Diff | Diff
Patch for lldb_webkit.py only (1.47 KB, patch)
2018-06-27 21:05 PDT, Simon Fraser (smfr)
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2018-06-27 21:02:21 PDT
The lldb vector summary provider always shows zero capacity
Comment 1 Simon Fraser (smfr) 2018-06-27 21:03:30 PDT
Created attachment 343789 [details]
Patch
Comment 2 Simon Fraser (smfr) 2018-06-27 21:05:06 PDT
First patch adds a regression test based on the functionality added in bug 183744.
Comment 3 Simon Fraser (smfr) 2018-06-27 21:05:45 PDT
Created attachment 343790 [details]
Patch for lldb_webkit.py only
Comment 4 Simon Fraser (smfr) 2018-06-28 08:38:10 PDT
Thanks Dan!
https://trac.webkit.org/changeset/233306/webkit
Comment 5 Radar WebKit Bug Importer 2018-06-28 08:39:17 PDT
<rdar://problem/41582463>
Comment 6 Simon Fraser (smfr) 2018-06-28 08:52:55 PDT
*** Bug 143921 has been marked as a duplicate of this bug. ***
Comment 7 Dawei Fenton (:realdawei) 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 }'
Comment 8 Simon Fraser (smfr) 2018-06-28 14:12:40 PDT
Cannot reproduce that failure locally.
Comment 9 Dawei Fenton (:realdawei) 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
Comment 10 Ryan Haddad 2018-06-28 15:25:37 PDT
Looks like a fix was attempted in https://trac.webkit.org/changeset/233330/webkit
Comment 11 Ryan Haddad 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
Comment 12 David Kilzer (:ddkilzer) 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!
Comment 13 Daniel Bates 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.
Comment 14 David Kilzer (:ddkilzer) 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?)
Comment 15 David Kilzer (:ddkilzer) 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.
Comment 16 Daniel Bates 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!