e.g. Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x00000002d02a5829 WTF::CrashOnOverflow::crash() + 9 (CheckedArithmetic.h:134) 1 com.apple.WebCore 0x00000002d02a57ee WTF::CrashOnOverflow::overflowed() + 14 (CheckedArithmetic.h:127) 2 com.apple.WebCore 0x00000002d4d41332 WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::at(unsigned long) const + 50 (Vector.h:707) 3 com.apple.WebCore 0x00000002d5a463c9 WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::operator[](unsigned long) const + 9 (Vector.h:722) 4 com.apple.WebCore 0x00000002d5a45b5b WebCore::RenderLayoutState::establishLineGrid(WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::RenderBlockFlow&) + 603 (RenderLayoutState.cpp:236) 5 com.apple.WebCore 0x00000002d5a44c14 WebCore::RenderLayoutState::computePaginationInformation(WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::RenderBox&, WebCore::LayoutUnit, bool) + 2036 (RenderLayoutState.cpp:166) 6 com.apple.WebCore 0x00000002d5a439cd WebCore::RenderLayoutState::RenderLayoutState(WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit, bool) + 285 (RenderLayoutState.cpp:80) 7 com.apple.WebCore 0x00000002d5a45169 WebCore::RenderLayoutState::RenderLayoutState(WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit, bool) + 9 (RenderLayoutState.cpp:74) 8 com.apple.WebCore 0x00000002d4d20a13 std::__1::__unique_if<WebCore::RenderLayoutState>::__unique_single std::__1::make_unique<WebCore::RenderLayoutState, WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit&, bool&>(WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit&, bool&) + 179 (memory:3033) 9 com.apple.WebCore 0x00000002d4d164ce decltype(auto) WTF::makeUnique<WebCore::RenderLayoutState, WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit&, bool&>(WTF::Vector<std::__1::unique_ptr<WebCore::RenderLayoutState, std::__1::default_delete<WebCore::RenderLayoutState> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit&, bool&) + 90 (StdLibExtras.h:507) [inlined] 10 com.apple.WebCore 0x00000002d4d164ce WebCore::FrameViewLayoutContext::pushLayoutState(WebCore::RenderBox&, WebCore::LayoutSize const&, WebCore::LayoutUnit, bool) + 446 (FrameViewLayoutContext.cpp:633) 11 com.apple.WebCore 0x00000002d5a466c4 WebCore::LayoutStateMaintainer::LayoutStateMaintainer(WebCore::RenderBox&, WebCore::LayoutSize, bool, WebCore::LayoutUnit, bool) + 372 (RenderLayoutState.cpp:275) 12 com.apple.WebCore 0x00000002d5a46829 WebCore::LayoutStateMaintainer::LayoutStateMaintainer(WebCore::RenderBox&, WebCore::LayoutSize, bool, WebCore::LayoutUnit, bool) + 9 (RenderLayoutState.cpp:274) 13 com.apple.WebCore 0x00000002d57f1b04 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) + 948 (RenderBlockFlow.cpp:498) 14 com.apple.WebCore 0x00000002d57c1525 WebCore::RenderBlock::layout() + 277 (RenderBlock.cpp:598) 15 com.apple.WebCore 0x00000002d57f7075 WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) + 1461 (RenderBlockFlow.cpp:762) 16 com.apple.WebCore 0x00000002d57f3a9e WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) + 718 (RenderBlockFlow.cpp:673) 17 com.apple.WebCore 0x00000002d57f1c28 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) + 1240 (RenderBlockFlow.cpp:525) 18 com.apple.WebCore 0x00000002d57c1525 WebCore::RenderBlock::layout() + 277 (RenderBlock.cpp:598) 19 com.apple.WebCore 0x00000002d56f7dd7 WebCore::RenderElement::layoutIfNeeded() + 71 (RenderElement.h:124) 20 com.apple.WebCore 0x00000002d56f67d6 WebCore::ComplexLineLayout::layoutLineBoxes(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&) + 1030 (ComplexLineLayout.cpp:1783) 21 com.apple.WebCore 0x00000002d5810f15 WebCore::RenderBlockFlow::ensureLineBoxes() + 565 (RenderBlockFlow.cpp:3763) 22 com.apple.WebCore 0x00000002d5a9ae5d WebCore::RenderObject::setSelectionState(WebCore::RenderObject::HighlightState) + 45 (RenderObject.cpp:1784) 23 com.apple.WebCore 0x00000002d589412f WebCore::RenderBoxModelObject::setSelectionState(WebCore::RenderObject::HighlightState) + 111 24 com.apple.WebCore 0x00000002d58122c2 WebCore::RenderBlockFlow::setSelectionState(WebCore::RenderObject::HighlightState) + 34 (RenderBlockFlow.cpp:3183) 25 com.apple.WebCore 0x00000002d5894183 WebCore::RenderBoxModelObject::setSelectionState(WebCore::RenderObject::HighlightState) + 195 (RenderBoxModelObject.cpp:150) 26 com.apple.WebCore 0x00000002d58122c2 WebCore::RenderBlockFlow::setSelectionState(WebCore::RenderObject::HighlightState) + 34 (RenderBlockFlow.cpp:3183) 27 com.apple.WebCore 0x00000002d5894183 WebCore::RenderBoxModelObject::setSelectionState(WebCore::RenderObject::HighlightState) + 195 (RenderBoxModelObject.cpp:150) 28 com.apple.WebCore 0x00000002d58122c2 WebCore::RenderBlockFlow::setSelectionState(WebCore::RenderObject::HighlightState) + 34 (RenderBlockFlow.cpp:3183) 29 com.apple.WebCore 0x00000002d5b1f346 WebCore::RenderText::setSelectionState(WebCore::RenderObject::HighlightState) + 102 (RenderText.cpp:1282) 30 com.apple.WebCore 0x00000002d5ba272f WebCore::RenderObject::setSelectionStateIfNeeded(WebCore::RenderObject::HighlightState) + 79 (RenderObject.h:1028) 31 com.apple.WebCore 0x00000002d5b9f6cb WebCore::SelectionRangeData::apply(WebCore::RenderRange const&, WebCore::SelectionRangeData::RepaintMode) + 795 (SelectionRangeData.cpp:229) 32 com.apple.WebCore 0x00000002d5b9f22f WebCore::SelectionRangeData::set(WebCore::RenderRange const&, WebCore::SelectionRangeData::RepaintMode) + 415 (SelectionRangeData.cpp:132) 33 com.apple.WebCore 0x00000002d3edcb70 WebCore::FrameSelection::updateAppearance() + 1728 (FrameSelection.cpp:2181) 34 com.apple.WebCore 0x00000002d3edc0c1 WebCore::FrameSelection::updateAndRevealSelection(WebCore::AXTextStateChangeIntent const&) + 257 (FrameSelection.cpp:474) 35 com.apple.WebCore 0x00000002d3ea5743 WebCore::FrameSelection::setSelection(WebCore::VisibleSelection const&, WTF::OptionSet<WebCore::FrameSelection::SetSelectionOption>, WebCore::AXTextStateChangeIntent, WebCore::FrameSelection::CursorAlignOnScroll, WebCore::TextGranularity) + 915 (FrameSelection.cpp:440) 36 com.apple.WebCore 0x00000002d3ea450a WebCore::FrameSelection::selectAll() + 1306 (FrameSelection.cpp:2006) 37 com.apple.WebCore 0x00000002d3ef83c1 WebCore::executeSelectAll(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) + 17 (EditorCommand.cpp:1021) 38 com.apple.WebCore 0x00000002d3eba97c WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const + 220 (EditorCommand.cpp:1860) 39 com.apple.WebCore 0x00000002d3b3ada4 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) + 244 (Document.cpp:5687) 40 com.apple.WebCore 0x00000002d0d20e6a WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) + 1130 (JSDocument.cpp:5890) 41 com.apple.WebCore 0x00000002d0d2095c long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) + 252 (JSDOMOperation.h:53) 42 com.apple.WebCore 0x00000002d0d0b239 WebCore::jsDocumentPrototypeFunction_execCommand(JSC::JSGlobalObject*, JSC::CallFrame*) + 9 (JSDocument.cpp:5895) <rdar://75428270>
I can reproduce this crash with DumpRenderTree and WebKitTestRunner at r274459.
Seems this lacks a test case.
Created attachment 423989 [details] Test case Oops, sorry about that.
Created attachment 424119 [details] Patch
Comment on attachment 424119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424119&action=review > Source/WebCore/ChangeLog:8 > + Fix out-of-bound access for layoutStateStack and ensure the whole vector is browsed. Nit: two spaces between "whole" and "vector". > Source/WebCore/rendering/RenderLayoutState.cpp:235 > - for (int i = layoutStateStack.size() - 1; i <= 0; --i) { > + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix?
Looks like this might be a regression from https://trac.webkit.org/r224612? Hm... looks like this code was introduced in https://trac.webkit.org/r105176 but I guess this loop was never really tested? And line-grid spec has changed so much over the last 9 years that it's gonna be hard to figure out what this loop was supposed to do. Perhaps Alan or Simon can figure out what they ought to be doing.
Comment on attachment 424119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424119&action=review >> Source/WebCore/rendering/RenderLayoutState.cpp:235 >> + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { > > It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix? To be honest, I don't know what this code is about. But I see it was added in https://trac.webkit.org/changeset/224612/webkit as part of a refactoring with "No change in functionality".
cc'ing zalan and antti
Comment on attachment 424119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424119&action=review >>> Source/WebCore/rendering/RenderLayoutState.cpp:235 >>> + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { >> >> It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix? > > To be honest, I don't know what this code is about. But I see it was added in https://trac.webkit.org/changeset/224612/webkit as part of a refactoring with "No change in functionality". This shows how little nested line grids is used (not to confuse it with css grid layout).
Comment on attachment 424119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424119&action=review >>>> Source/WebCore/rendering/RenderLayoutState.cpp:235 >>>> + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { >>> >>> It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix? >> >> To be honest, I don't know what this code is about. But I see it was added in https://trac.webkit.org/changeset/224612/webkit as part of a refactoring with "No change in functionality". > > This shows how little nested line grids is used (not to confuse it with css grid layout). So are you able to provide some hints on how we could write a test for this? Or are you saying we should just ignore that?
(In reply to Frédéric Wang (:fredw) from comment #10) > Comment on attachment 424119 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424119&action=review > > >>>> Source/WebCore/rendering/RenderLayoutState.cpp:235 > >>>> + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { > >>> > >>> It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix? > >> > >> To be honest, I don't know what this code is about. But I see it was added in https://trac.webkit.org/changeset/224612/webkit as part of a refactoring with "No change in functionality". > > > > This shows how little nested line grids is used (not to confuse it with css grid layout). > > So are you able to provide some hints on how we could write a test for this? No, I have close to zero knowledge about this feature.
(In reply to Frédéric Wang (:fredw) from comment #10) > Comment on attachment 424119 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424119&action=review > > >>>> Source/WebCore/rendering/RenderLayoutState.cpp:235 > >>>> + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { > >>> > >>> It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix? > >> > >> To be honest, I don't know what this code is about. But I see it was added in https://trac.webkit.org/changeset/224612/webkit as part of a refactoring with "No change in functionality". > > > > This shows how little nested line grids is used (not to confuse it with css grid layout). > > So are you able to provide some hints on how we could write a test for this? > Or are you saying we should just ignore that? I initially thought this was related to grid layout but it looks like there is no way of knowing what this CSS property was supposed to do at this point so landing it without a test seems okay to me.
Created attachment 424223 [details] Patch for landing
Comment on attachment 424119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424119&action=review >> Source/WebCore/ChangeLog:8 >> + Fix out-of-bound access for layoutStateStack and ensure the whole vector is browsed. > > Nit: two spaces between "whole" and "vector". Done. >>>>>>> Source/WebCore/rendering/RenderLayoutState.cpp:235 >>>>>>> + for (int i = layoutStateStack.size() - 1; i >= 0; --i) { >>>>>> >>>>>> It seems like we should also be adding some tests demonstrating whatever grid layout bug this will fix? >>>>> >>>>> To be honest, I don't know what this code is about. But I see it was added in https://trac.webkit.org/changeset/224612/webkit as part of a refactoring with "No change in functionality". >>>> >>>> This shows how little nested line grids is used (not to confuse it with css grid layout). >>> >>> So are you able to provide some hints on how we could write a test for this? Or are you saying we should just ignore that? >> >> No, I have close to zero knowledge about this feature. > > I initially thought this was related to grid layout but it looks like there is no way of knowing what this CSS property was supposed to do at this point so landing it without a test seems okay to me. OK, let's try and land this then.
(In reply to Frédéric Wang (:fredw) from comment #14) > > > I initially thought this was related to grid layout but it looks like there is no way of knowing what this CSS property was supposed to do at this point so landing it without a test seems okay to me. > > OK, let's try and land this then. Sounds good.
Committed r275164: <https://commits.webkit.org/r275164> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424223 [details].