e.g. ./rendering/RenderTable.cpp(586) : virtual void WebCore::RenderTable::layout() 1 0x3ccc632a9 WTFCrash 2 0x3ae461b8b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x3b2b51640 WebCore::RenderTable::layout() 4 0x3b287396c WebCore::RenderElement::layoutIfNeeded() 5 0x3b2b515e7 WebCore::RenderTable::layout() 6 0x3b293ede2 WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 7 0x3b293d3a4 WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) 8 0x3b293c186 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) 9 0x3b291ee99 WebCore::RenderBlock::layout() 10 0x3b293ede2 WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 11 0x3b293d3a4 WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) 12 0x3b293c186 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) 13 0x3b291ee99 WebCore::RenderBlock::layout() 14 0x3b293ede2 WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 15 0x3b293d3a4 WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) 16 0x3b293c186 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) 17 0x3b291ee99 WebCore::RenderBlock::layout() 18 0x3b2bbdb93 WebCore::RenderView::layout() 19 0x3b2141181 WebCore::FrameViewLayoutContext::layout() 20 0x3b213d683 WebCore::FrameView::forceLayout(bool) 21 0x3b213d0d3 WebCore::FrameView::forceLayoutForPagination(WebCore::FloatSize const&, WebCore::FloatSize const&, float, WebCore::AdjustViewSizeOrNot) 22 0x3b213cc61 WebCore::Frame::setPrinting(bool, WebCore::FloatSize const&, WebCore::FloatSize const&, float, WebCore::AdjustViewSizeOrNot) 23 0x3b224862c WebCore::PrintContext::begin(float, float) 24 0x3a1bada4d WebKit::WebPage::beginPrinting(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&) 25 0x3a1c5f31c void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo>, 0ul, 1ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) 26 0x3a1c5e2b0 void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo>&&, WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&)) 27 0x3a1c0f4b9 void IPC::handleMessage<Messages::WebPage::BeginPrinting, WebKit::WebPage, void (WebKit::WebPage::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&)>(IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::PrintInfo const&)) 28 0x3a1c05756 WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) 29 0x3a1bacd62 WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 30 0x3a0463591 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) 31 0x3a16e4727 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) <rdar://problem/69563902>
Created attachment 413187 [details] Reduced test case
(In reply to Ryosuke Niwa from comment #1) > Created attachment 413187 [details] > Reduced test case Are we sure the reduced test case is correct?, it isn't hitting any assertion for me.
(In reply to Sergio Villar Senin from comment #2) > (In reply to Ryosuke Niwa from comment #1) > > Created attachment 413187 [details] > > Reduced test case > > Are we sure the reduced test case is correct?, it isn't hitting any > assertion for me. Yes. Are you testing it on macOS or GTK+?
(In reply to Ryosuke Niwa from comment #3) > (In reply to Sergio Villar Senin from comment #2) > > (In reply to Ryosuke Niwa from comment #1) > > > Created attachment 413187 [details] > > > Reduced test case > > > > Are we sure the reduced test case is correct?, it isn't hitting any > > assertion for me. > > Yes. Are you testing it on macOS or GTK+? I've tried with both GTK and WPE. I'll try with macOS.
(In reply to Sergio Villar Senin from comment #4) > (In reply to Ryosuke Niwa from comment #3) > > (In reply to Sergio Villar Senin from comment #2) > > > (In reply to Ryosuke Niwa from comment #1) > > > > Created attachment 413187 [details] > > > > Reduced test case > > > > > > Are we sure the reduced test case is correct?, it isn't hitting any > > > assertion for me. > > > > Yes. Are you testing it on macOS or GTK+? > > I've tried with both GTK and WPE. I'll try with macOS. Nevermind. I was testing with MiniBrowser and it was working fine, but it looks like it reliably crashes with RWT.
(In reply to Sergio Villar Senin from comment #5) > (In reply to Sergio Villar Senin from comment #4) > > (In reply to Ryosuke Niwa from comment #3) > > > (In reply to Sergio Villar Senin from comment #2) > > > > (In reply to Ryosuke Niwa from comment #1) > > > > > Created attachment 413187 [details] > > > > > Reduced test case > > > > > > > > Are we sure the reduced test case is correct?, it isn't hitting any > > > > assertion for me. > > > > > > Yes. Are you testing it on macOS or GTK+? > > > > I've tried with both GTK and WPE. I'll try with macOS. > > Nevermind. I was testing with MiniBrowser and it was working fine, but it > looks like it reliably crashes with RWT. Ah, makes sense. Yeah, most of these fuzzer bugs require WKTR to reproduce.
I do see the crash on OS X: SHOULD NEVER BE REACHED ./rendering/RenderTable.cpp(586) : virtual void WebCore::RenderTable::layout() 1 0x558117309 WTFCrash 2 0x51d275c40 canLoad_libAccessibility__AXSIsolatedTreeMode 3 0x528fa2613 WebCore::RenderTable::layout() 4 0x528624ed0 WebCore::RenderElement::layoutIfNeeded() etc...
So apparently this is an instance of bug 174412, we are basically hitting the ASSERT added in bug 174413 to workaround infinite recursion. The patch uploaded to bug 174412 was landed in bug 176219. Would you be able to provide access to bug 176219, so that we have the full context on this issue?
(In reply to Frédéric Wang (:fredw) from comment #8) > So apparently this is an instance of bug 174412, we are basically hitting > the ASSERT added in bug 174413 to workaround infinite recursion. The patch > uploaded to bug 174412 was landed in bug 176219. Would you be able to > provide access to bug 176219, so that we have the full context on this issue? Done.
Created attachment 419275 [details] Reduce test case with table cells satisfying !isBaselineAligned() This is the same as attachment 413187 [details], but with table cells satisfying !isBaselineAligned() so that layoutBlock happens only once in RenderTableCell::layout(). It still allows to reproduce the issue.
(In reply to Ryosuke Niwa from comment #9) > (In reply to Frédéric Wang (:fredw) from comment #8) > > So apparently this is an instance of bug 174412, we are basically hitting > > the ASSERT added in bug 174413 to workaround infinite recursion. The patch > > uploaded to bug 174412 was landed in bug 176219. Would you be able to > > provide access to bug 176219, so that we have the full context on this issue? > > Done. Thanks. Bug 176219 does not seem to have much info either. Anyway, for the record this is what I've found so far for attachment 419275 [details]. Any advice would be welcome: When print() is called, WebCore::RenderTable::layout() is executed in paginated mode. It performs layout of each section at https://webkit-search.igalia.com/webkit/rev/5edc52e6a4c801295ad3cbf56569dd4a5b738f78/Source/WebCore/rendering/RenderTable.cpp#476 which ends up laying out the cell's section and setting its height at https://webkit-search.igalia.com/webkit/rev/5edc52e6a4c801295ad3cbf56569dd4a5b738f78/Source/WebCore/rendering/ComplexLineLayout.cpp#1955 The sections are then positioned at https://webkit-search.igalia.com/webkit/rev/5edc52e6a4c801295ad3cbf56569dd4a5b738f78/Source/WebCore/rendering/RenderTable.cpp#476 Since the table was previously laid out in non-paginated mode, this detects a section move and we arrive at the code I mentioned in comment 8: https://webkit-search.igalia.com/webkit/rev/5edc52e6a4c801295ad3cbf56569dd4a5b738f78/Source/WebCore/rendering/RenderTable.cpp#586 Then WebCore::RenderTable::layout() is executed a second time. A difference happens in ComplexLineLayout::determineStartPosition() : for the previous layout paginationDelta was 0 and the line's paginationStrut had been set accordingly ; for this new layout, the line's paginationStrut remains nonzero and so the calculated paginationDelta is no longer 0 : https://webkit-search.igalia.com/webkit/rev/5edc52e6a4c801295ad3cbf56569dd4a5b738f78/Source/WebCore/rendering/ComplexLineLayout.cpp#1855 The adjustBlockDirectionPosition call below then modifies the line's lineBoxBottom which at the end changes the calculated cell's height. Finally the sections are positioned again, a section move detected (due to at least one cell's height change) and the assertion failure triggered. I'm not familiar enough with the pagination code to really understand what this code is really doing. However, I see there are pagination-related FIXMEs all over the code, for example these ones (which don't seem to be involved in the current problem though): https://webkit-search.igalia.com/webkit/rev/5edc52e6a4c801295ad3cbf56569dd4a5b738f78/Source/WebCore/rendering/RenderTableSection.cpp#593 Additionally, I modified the tests to use 100 sections and noticed that the RenderTable layout does stabilize after ~50 recursions. For completeness, I can also mention that after commenting out the adjustBlockDirectionPosition call, the layout is similar to non-paginated mode and does not recurse/assert. NB: Similar behavior happens for attachment 413187 [details], but it is a slightly more difficult to debug because RenderTableCell::layout() can lay out its content twice.
The problem here is that moving one section affects the table layout, so in the next layout iteration, another section moves. I think this is valid case, it's really possible to recurse several times. The recursion check was introduced in r219394, but there's no much information nor a test case, so I don't know how this can end up in an infinite recursion. Instead of limiting to one recursion, we could allow to recurse for the amount of sections in the table?
Created attachment 421513 [details] Patch
Ping reviewers.
Committed r274627: <https://commits.webkit.org/r274627> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421513 [details].
Is this a security bug?
If it's not a security bug, can we also add a test?
It doesn't look like a security issue to me, I'll submit a test.
Reopening to attach a new patch.
Created attachment 423873 [details] Patch to add test
Committed r274854: <https://commits.webkit.org/r274854> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423873 [details].