156809 2016-04-20 14:35:56 -0700 Crash under WebCore::TextIterator::subrange() 2016-04-20 22:02:09 -0700 1 1 1 Unclassified WebKit HTML Editing WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez commit-queue enrica rniwa webkit-bug-importer oldest_to_newest 1185544 0 cdumez 2016-04-20 14:35:56 -0700 Crash unde WebCore::TextIterator::subrange(): Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000008) [ 0] 0x00007fff9969a57e WebCore`WebCore::TextIterator::subrange(WebCore::Range*, int, int) [inlined] WTF::Ref<WebCore::Document>::get() const at Ref.h:60 0x00007fff9969a56c: movq %rbx, %rdi 0x00007fff9969a56f: callq 0x1b06d0 ; WebCore::TextIterator::advance at TextIterator.cpp:377 0x00007fff9969a574: cmpq $0x0, -0xc8(%rbp) 0x00007fff9969a57c: jne 0x2ab560 ; <+96> [inlined] WebCore::CharacterIterator::CharacterIterator(WebCore::Range const&, unsigned short) + 55 at TextIterator.cpp:1399 -> 0x00007fff9969a57e: movq 0x8(%r13), %rsi 0x00007fff9969a582: leaq -0x128(%rbp), %rbx 0x00007fff9969a589: movq %r12, %rdi 0x00007fff9969a58c: movq %rbx, %rdx 0x00007fff9969a58f: movl %r15d, %ecx [ 0] 0x00007fff9969a57e WebCore`WebCore::TextIterator::subrange(WebCore::Range*, int, int) [inlined] WebCore::Range::ownerDocument() const at Range.h:62 58 static PassRefPtr<Range> create(ScriptExecutionContext&); 59 static PassRefPtr<Range> create(Document&, const VisiblePosition&, const VisiblePosition&); 60 ~Range(); 61 -> 62 Document& ownerDocument() const { return const_cast<Document&>(m_ownerDocument.get()); } 63 64 Node* startContainer() const { return m_start.container(); } 65 int startOffset() const { return m_start.offset(); } 66 Node* endContainer() const { return m_end.container(); } [ 0] 0x00007fff9969a57e WebCore`WebCore::TextIterator::subrange(WebCore::Range*, int, int) + 126 at TextIterator.cpp:2388 2384 2385 PassRefPtr<Range> TextIterator::subrange(Range* entireRange, int characterOffset, int characterCount) 2386 { 2387 CharacterIterator entireRangeIterator(*entireRange); -> 2388 return characterSubrange(entireRange->ownerDocument(), entireRangeIterator, characterOffset, characterCount); 2389 } 2390 2391 static inline bool isInsideReplacedElement(TextIterator& iterator) 2392 { [ 1] 0x00007fff9978ac5f WebCore`WebCore::AlternativeTextController::applyAlternativeTextToRange(WebCore::Range const*, WTF::String const&, WebCore::AlternativeTextType, WTF::Vector<WebCore::DocumentMarker::MarkerType, 0ul, WTF::CrashOnOverflow> const&) + 1647 at AlternativeTextController.cpp:281 277 // Recalculate pragraphRangeContainingCorrection, since SpellingCorrectionCommand modified the DOM, such that the original paragraphRangeContainingCorrection is no longer valid. Radar: 10305315 Bugzilla: 89526 278 paragraphRangeContainingCorrection = TextIterator::rangeFromLocationAndLength(&rootNode, paragraphStartIndex, correctionStartOffsetInParagraph + alternative.length()); 279 280 setEnd(paragraphRangeContainingCorrection.get(), m_frame.selection().selection().start()); -> 281 RefPtr<Range> replacementRange = TextIterator::subrange(paragraphRangeContainingCorrection.get(), correctionStartOffsetInParagraph, alternative.length()); 282 String newText = plainText(replacementRange.get()); 283 284 // Check to see if replacement succeeded. 285 if (newText != alternative) [ 2] 0x00007fff9968af38 WebCore`WebCore::AlternativeTextController::handleAlternativeTextUIResult(WTF::String const&) + 632 at AlternativeTextController.cpp:420 [ 3] 0x00007fff911aa9d7 WebKitLegacy`-[WebView(WebViewInternal) handleAcceptedAlternativeText:] + 71 at WebView.mm:8542 1185545 1 cdumez 2016-04-20 14:36:20 -0700 rdar://problem/21102730 1185547 2 276854 cdumez 2016-04-20 14:38:55 -0700 Created attachment 276854 Patch 1185679 3 276854 achristensen 2016-04-20 21:08:00 -0700 Comment on attachment 276854 Patch paragraphRangeContainingCorrection is dereferenced before this. If this is to avoid null pointer dereferencing in this function, I think this is in the wrong place. 1185684 4 cdumez 2016-04-20 21:36:10 -0700 (In reply to comment #3) > Comment on attachment 276854 [details] > Patch > > paragraphRangeContainingCorrection is dereferenced before this. If this is > to avoid null pointer dereferencing in this function, I think this is in the > wrong place. But before this, it is initialized like so: paragraphRangeContainingCorrection = range->cloneRange(); And range->cloneRange() cannot return null. This is therefore the right place to null check. 1185686 5 cdumez 2016-04-20 21:39:00 -0700 (In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 276854 [details] > > Patch > > > > paragraphRangeContainingCorrection is dereferenced before this. If this is > > to avoid null pointer dereferencing in this function, I think this is in the > > wrong place. > > But before this, it is initialized like so: > paragraphRangeContainingCorrection = range->cloneRange(); > > And range->cloneRange() cannot return null. > > This is therefore the right place to null check. As explained in the Changelog, the issue is only with TextIterator::rangeFromLocationAndLength() potentially returning null. 1185695 6 276854 commit-queue 2016-04-20 22:02:04 -0700 Comment on attachment 276854 Patch Clearing flags on attachment: 276854 Committed r199807: <http://trac.webkit.org/changeset/199807> 1185696 7 commit-queue 2016-04-20 22:02:09 -0700 All reviewed patches have been landed. Closing bug. 276854 2016-04-20 14:38:55 -0700 2016-04-20 22:02:04 -0700 Patch bug-156809-20160420143916.patch text/plain 2224 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTk5Nzc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYTRmNWM3MTZlOGFiM2E5 ZTNkMzAzZTc4NDJkZmMwODQyZGQ2MjcwZC4uODk0MzcyMGQ3Mjk0NzA1MTgwYTI1YTdjMTRmYzM0 YWUxZTc3N2YyNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE2LTA0LTIwICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgQ3Jhc2ggdW5kZXIgV2ViQ29y ZTo6VGV4dEl0ZXJhdG9yOjpzdWJyYW5nZSgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xNTY4MDkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzIxMTAy NzMwPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRl eHRJdGVyYXRvcjo6cmFuZ2VGcm9tTG9jYXRpb25BbmRMZW5ndGgoKSBtYXkgcmV0dXJuIG51bGwu IEhvd2V2ZXIsIHdlCisgICAgICAgIGZhaWxlZCB0byBkbyBhIG51bGwgY2hlY2sgYmVmb3JlIGNh bGxpbmcgVGV4dEl0ZXJhdG9yOjpzdWJyYW5nZSgpIHdpdGgKKyAgICAgICAgdGhhdCByYW5nZS4K KworICAgICAgICBObyBuZXcgdGVzdHMsIGRvIG5vdCBrbm93IGhvdyB0byByZXByb2R1Y2UuCisK KyAgICAgICAgKiBlZGl0aW5nL0FsdGVybmF0aXZlVGV4dENvbnRyb2xsZXIuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6QWx0ZXJuYXRpdmVUZXh0Q29udHJvbGxlcjo6YXBwbHlBbHRlcm5hdGl2ZVRl eHRUb1JhbmdlKToKKwogMjAxNi0wNC0yMCAgRGF2ZSBIeWF0dCAgPGh5YXR0QGFwcGxlLmNvbT4K IAogICAgICAgICBIYW5nYWJsZSBwdW5jdHVhdGlvbiBtZWFzdXJlbWVudCB1c2luZyB0aGUgd3Jv bmcgaW5kaWNlcy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvQWx0ZXJuYXRp dmVUZXh0Q29udHJvbGxlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9lZGl0aW5nL0FsdGVybmF0aXZl VGV4dENvbnRyb2xsZXIuY3BwCmluZGV4IDVlZGYwYjZjMmQzZTllN2UyNTk5ZWIyZGRmNTAxNjdm YWQxMGNlNjQuLmVkYTUyZWEzNzA1OTVhOTZiYjM1MzBlOTMyNWZkZDc2MjRmNjczODIgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvQWx0ZXJuYXRpdmVUZXh0Q29udHJvbGxlci5j cHAKKysrIGIvU291cmNlL1dlYkNvcmUvZWRpdGluZy9BbHRlcm5hdGl2ZVRleHRDb250cm9sbGVy LmNwcApAQCAtMjczLDYgKzI3Myw4IEBAIHZvaWQgQWx0ZXJuYXRpdmVUZXh0Q29udHJvbGxlcjo6 YXBwbHlBbHRlcm5hdGl2ZVRleHRUb1JhbmdlKGNvbnN0IFJhbmdlKiByYW5nZSwKICAgICBhcHBs eUNvbW1hbmQoU3BlbGxpbmdDb3JyZWN0aW9uQ29tbWFuZDo6Y3JlYXRlKHJhbmdlV2l0aEFsdGVy bmF0aXZlLnB0cigpLCBhbHRlcm5hdGl2ZSkpOwogICAgIC8vIFJlY2FsY3VsYXRlIHByYWdyYXBo UmFuZ2VDb250YWluaW5nQ29ycmVjdGlvbiwgc2luY2UgU3BlbGxpbmdDb3JyZWN0aW9uQ29tbWFu ZCBtb2RpZmllZCB0aGUgRE9NLCBzdWNoIHRoYXQgdGhlIG9yaWdpbmFsIHBhcmFncmFwaFJhbmdl Q29udGFpbmluZ0NvcnJlY3Rpb24gaXMgbm8gbG9uZ2VyIHZhbGlkLiBSYWRhcjogMTAzMDUzMTUg QnVnemlsbGE6IDg5NTI2CiAgICAgcGFyYWdyYXBoUmFuZ2VDb250YWluaW5nQ29ycmVjdGlvbiA9 IFRleHRJdGVyYXRvcjo6cmFuZ2VGcm9tTG9jYXRpb25BbmRMZW5ndGgoJnJvb3ROb2RlLCBwYXJh Z3JhcGhTdGFydEluZGV4LCBjb3JyZWN0aW9uU3RhcnRPZmZzZXRJblBhcmFncmFwaCArIGFsdGVy bmF0aXZlLmxlbmd0aCgpKTsKKyAgICBpZiAoIXBhcmFncmFwaFJhbmdlQ29udGFpbmluZ0NvcnJl Y3Rpb24pCisgICAgICAgIHJldHVybjsKICAgICAKICAgICBzZXRFbmQocGFyYWdyYXBoUmFuZ2VD b250YWluaW5nQ29ycmVjdGlvbi5nZXQoKSwgbV9mcmFtZS5zZWxlY3Rpb24oKS5zZWxlY3Rpb24o KS5zdGFydCgpKTsKICAgICBSZWZQdHI8UmFuZ2U+IHJlcGxhY2VtZW50UmFuZ2UgPSBUZXh0SXRl cmF0b3I6OnN1YnJhbmdlKHBhcmFncmFwaFJhbmdlQ29udGFpbmluZ0NvcnJlY3Rpb24uZ2V0KCks IGNvcnJlY3Rpb25TdGFydE9mZnNldEluUGFyYWdyYXBoLCBhbHRlcm5hdGl2ZS5sZW5ndGgoKSk7 Cg==