Bug 187132

Created attachment 343789 [details]
Patch

First patch adds a regression test based on the functionality added in bug 183744.

Created attachment 343790 [details]
Patch for lldb_webkit.py only

Thanks Dan!
https://trac.webkit.org/changeset/233306/webkit

<rdar://problem/41582463>

*** Bug 143921 has been marked as a duplicate of this bug. ***

(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 }'

Cannot reproduce that failure locally.

(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

Looks like a fix was attempted in https://trac.webkit.org/changeset/233330/webkit

(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

BTW, when running the test locally, it doesn't seem like Tools/lldb/lldbWebKitTester/main.cpp forces lldbWebKitTester to be rebuilt!

(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.

(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?)

(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.

(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!