43032 2010-07-27 02:16:20 -0700 Various "selection.modify" WebCore::ContainerNode::childNodeCount NULL pointer crashes 2010-09-29 10:51:20 -0700 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) PC Windows Vista RESOLVED FIXED http://code.google.com/p/chromium/issues/detail?id=50341 P1 Normal --- 1 skylined tkent abarth eric jamesr mario mrobinson ojan rniwa tkent tony webkit.review.bot xan.lopez oldest_to_newest 256458 0 62658 skylined 2010-07-27 02:16:20 -0700 Created attachment 62658 Repro - WebCore::ContainerNode::childNodeCount ReadAV@NULL (73c50888d671e6db8bd5929edc78bd92) Attached is a repro for a NULL pointer crash in WebCore::ContainerNode::childNodeCount. I'll add a few more related crashes. It appears this bug can only be used to crash WebKit by causing a NULL pointer exception. 256459 1 62659 skylined 2010-07-27 02:18:19 -0700 Created attachment 62659 Repro - WebCore::ContainerNode::childNodeCount ReadAV@NULL (bd8706ccf27f504a18a4e8df4cbfc99f) 256460 2 62660 skylined 2010-07-27 02:18:43 -0700 Created attachment 62660 Repro - WebCore::ContainerNode::childNodeCount ReadAV@NULL (fb6d0e8b899470d4cd53d1586bb52dc3) 262007 3 63866 tkent 2010-08-09 00:53:32 -0700 Created attachment 63866 Patch 262010 4 tkent 2010-08-09 00:54:20 -0700 I confirmed all three crashed are fixed by the patch. 262580 5 tkent 2010-08-10 02:48:45 -0700 Landed as r65061. 262614 6 webkit.review.bot 2010-08-10 04:13:14 -0700 http://trac.webkit.org/changeset/65061 might have broken GTK Linux 32-bit Release and GTK Linux 64-bit Debug 262752 7 jamesr 2010-08-10 10:34:48 -0700 GTK bots have been crashing on editing/selection/selection-modify-crash.html every run since this landed (5 so far). 262788 8 rniwa 2010-08-10 11:23:57 -0700 Test is added to the skip list in http://trac.webkit.org/changeset/65078. 262790 9 rniwa 2010-08-10 11:24:42 -0700 This bug seem to require another fix on gtk. 262792 10 eric 2010-08-10 11:25:26 -0700 Um... Why are we skipping newly introduced crashers? That seems like the wrong approach. We should be fixing or rolling out, no? 262793 11 tony 2010-08-10 11:28:03 -0700 (In reply to comment #10) > Um... Why are we skipping newly introduced crashers? That seems like the wrong approach. We should be fixing or rolling out, no? I made the suggestion to skip on GTK because it passes on other platforms. I suspect that this is an existing crash in WebKit GTK that the new test is just exposing, but without a GTK build handy, I'm not certain. If you think it's better to roll out the original crash fix, I'm not opposed. 262795 12 eric 2010-08-10 11:31:23 -0700 Just so long as it's not a permenant fix. :) It sounds like from the comment which mid-air colided with mine, that rniwa is working on fixing it as we speak. So I'm fine with leaving the fix in. We've often found that when we land a patch and it fails mysteriously on gtk or qt, that there were real bugs in the patch. :) Xan or Martin should be able to hook you up with some gtk backtraces. Or you can ask on #webkit-gtk. Martin works in SF and generally super available and super helpful for this kind of thing. 262798 13 jamesr 2010-08-10 11:36:09 -0700 The test that is crashing is new. The patch didn't cause the crash, the test exposes a crash that (presumably) has always been there. Rolling out the patch would only remove the test case, it wouldn't fix anything. 262841 14 rniwa 2010-08-10 13:48:04 -0700 It seems like the corresponding crash log is http://webkit-bots.igalia.com/amd64/svn_65077.core-when_1281463861-_-who_DumpRenderTree-_-why_11.23383.trace.html +msanchez 262842 15 xan.lopez 2010-08-10 13:52:45 -0700 (In reply to comment #14) > It seems like the corresponding crash log is http://webkit-bots.igalia.com/amd64/svn_65077.core-when_1281463861-_-who_DumpRenderTree-_-why_11.23383.trace.html > > +msanchez From the trace it seems the likely cause is that we are getting a parentObject that is NULL. My poor understanding of the topic tells me that this shouldn't happen (there should always be a root object), so this would be exposing a deeper issue I suppose. If on the other hand getting a NULL parent makes sense in some cases (and for this test) then the fix is simple. 263075 16 mario 2010-08-10 23:55:37 -0700 (In reply to comment #15) > [...] > From the trace it seems the likely cause is that we are getting a parentObject > that is NULL. My poor understanding of the topic tells me that this shouldn't > happen (there should always be a root object), so this would be exposing a > deeper issue I suppose. If on the other hand getting a NULL parent makes sense > in some cases (and for this test) then the fix is simple. +1 to Xan's reasoning. Now trying to throw some additional info over the table... AFAIK, it's strange you get a null parentObject unless you directly asked for it to the root #document object, since calls to parentObject() or parentObjectUnignored() will always return a valid AccessibleObject (the one associated with the #document node, in the most xtreme case). But, in AccessibilytObjectWrapper.cpp: AccessibilityObject* objectAndOffsetUnignored(AccessibilityObject* coreObject, int& offset, bool ignoreLinks) { Node* endNode = static_cast<AccessibilityRenderObject*>(coreObject)->renderer()->node(); int endOffset = coreObject->selection().end().computeOffsetInContainerNode(); // Indication that something bogus has transpired. offset = -1; AccessibilityObject* realObject = coreObject; if (realObject->accessibilityIsIgnored()) realObject = realObject->parentObjectUnignored(); if (ignoreLinks && realObject->isLink()) <------- Guilty line realObject = realObject->parentObjectUnignored(); [...] } ... realObject can be either the coreObject (which is not null, as seen in the stacktrace) or the parent for the original coreObject *in case it did ignore accessibility*, which is not the case of the AccessibleObject associated to the #document node (the only one which in theory could return NULL through parentObject() or parentObjectUnignored()), thus that never could happen in theory. However, seeing that it crashes on that line but not in the previous ones calling to methods of coreObject and/or realObject looks to me like the confirmation of the "realObject = realObject->parentObjectUnignored()" line being executed and somehow returning NULL so I think there's definitely something wrong in there (perhaps there's no AccessibleObject for #document object -or someone else- created yet, so the a11y hierarchy is somehow broken?). Btw, any advice/tip on how to reliably reproduce this? I'd like to take a closer look to this issue to try to help as much as possible 263455 17 mrobinson 2010-08-11 14:34:12 -0700 Mario, have you been able to reproduce this issue by running the test that failed? I think the Chromium guys may have the information regarding this. 263564 18 tkent 2010-08-11 18:41:13 -0700 I couldn't reproduce the crash with Ubuntu 9.10, GTK port, amd64, Debug and Release. Mario, Martin, LayoutTests/editing/selection/selection-modify-crash.html should reproduce the crash though I couldn't. 286904 19 skylined 2010-09-29 09:17:27 -0700 The repros I attached to this bug no longer crash Chromium latest - time to close the bug? 286929 20 mrobinson 2010-09-29 09:43:29 -0700 It looks like this crash is only reproducible when run via Xvfb. Instructions for running via Xvfb are here: https://trac.webkit.org/wiki/WebKitGtkLayoutTests 286983 21 mrobinson 2010-09-29 10:51:20 -0700 The crasher on GTK+ is unrelated to this issue. I've posted a patch for that here: https://bugs.webkit.org/show_bug.cgi?id=46822 Sorry, that this has been delayed so long. 62658 2010-07-27 02:16:20 -0700 2010-07-27 02:16:20 -0700 Repro - WebCore::ContainerNode::childNodeCount ReadAV@NULL (73c50888d671e6db8bd5929edc78bd92) repro.html text/html 543 skylined PGh0bWw+CiAgPGhlYWQ+CiAgICA8c2NyaXB0PgogICAgICBmdW5jdGlvbiBnbygpIHsKICAgICAg ICBodG1sRWxlbWVudCA9IGRvY3VtZW50LmZpcnN0Q2hpbGQ7CiAgICAgICAgLy8gY3JlYXRlIGEg cmFuZ2U6CiAgICAgICAgc3RhbGVSYW5nZSA9IGRvY3VtZW50LmNyZWF0ZVJhbmdlKCk7CgogICAg ICAgIC8vIGRlbGV0ZSBkb2N1bWVudC5ib2R5OgogICAgICAgIHJhbmdlID0gZG9jdW1lbnQuY3Jl YXRlUmFuZ2UoKTsKICAgICAgICByYW5nZS5zZXRFbmRBZnRlcihodG1sRWxlbWVudCk7CiAgICAg ICAgcmFuZ2UuZGVsZXRlQ29udGVudHMoKTsKICAgICAgICAKICAgICAgICBzZWxlY3Rpb24gPSB3 aW5kb3cuZ2V0U2VsZWN0aW9uKCk7CiAgICAgICAgc2VsZWN0aW9uLmFkZFJhbmdlKHN0YWxlUmFu Z2UpOwogICAgICAgIHNlbGVjdGlvbi5tb2RpZnkoImV4dGVuZCIsICJmb3J3YXJkIiwgImRvY3Vt ZW50Ym91bmRhcnkiKTsKICAgICAgfQogICAgPC9zY3JpcHQ+CiAgPC9oZWFkPgogIDxib2R5IG9u bG9hZD0iZ28oKSI+CiAgPC9ib2R5Pgo8L2h0bWw+ 62659 2010-07-27 02:18:19 -0700 2010-07-27 02:18:19 -0700 Repro - WebCore::ContainerNode::childNodeCount ReadAV@NULL (bd8706ccf27f504a18a4e8df4cbfc99f) repro.html text/html 570 skylined PGh0bWw+CiAgPGhlYWQ+CiAgICA8c2NyaXB0PgogICAgICBmdW5jdGlvbiBnbygpIHsKICAgICAg ICBodG1sRWxlbWVudCA9IGRvY3VtZW50LmZpcnN0Q2hpbGQ7CiAgICAgICAgLy8gZGVsZXRlIEhU TUwgZWxlbWVudCBjb250ZW50czoKICAgICAgICByYW5nZSA9IGRvY3VtZW50LmNyZWF0ZVJhbmdl KCk7CiAgICAgICAgcmFuZ2Uuc2VsZWN0Tm9kZShodG1sRWxlbWVudCk7CiAgICAgICAgcmFuZ2Uu ZGVsZXRlQ29udGVudHMoKTsKICAgICAgICAvLyBzZWxlY3QgSFRNTCBlbGVtZW50IGNvbnRlbnRz OgogICAgICAgIHNlbGVjdGlvbiA9IHdpbmRvdy5nZXRTZWxlY3Rpb24oKTsKICAgICAgICBzZWxl Y3Rpb24uc2VsZWN0QWxsQ2hpbGRyZW4oZG9jdW1lbnQpOwogICAgICAgIC8vIG1vdmUgdG8gZGVs ZXRlZCBkb2N1bWVudCBib3VuZGFyeTogTlVMTCBwdHIuCiAgICAgICAgc2VsZWN0aW9uLm1vZGlm eSgibW92ZSIsICJmb3J3YXJkIiwgImRvY3VtZW50Ym91bmRhcnkiKTsKICAgICAgfQogICAgPC9z Y3JpcHQ+CiAgPC9oZWFkPgogIDxib2R5IG9ubG9hZD0iZ28oKSI+CiAgPC9ib2R5Pgo8L2h0bWw+ 62660 2010-07-27 02:18:43 -0700 2010-07-27 02:18:43 -0700 Repro - WebCore::ContainerNode::childNodeCount ReadAV@NULL (fb6d0e8b899470d4cd53d1586bb52dc3) repro.html text/html 335 skylined PGh0bWw+CiAgPGhlYWQ+CiAgICA8c2NyaXB0PgogICAgICBmdW5jdGlvbiBnbygpIHsKICAgICAg ICBzZWxlY3Rpb24gPSB3aW5kb3cuZ2V0U2VsZWN0aW9uKCk7CiAgICAgICAgcmFuZ2UgPSBkb2N1 bWVudC5jcmVhdGVSYW5nZSgpOwogICAgICAgIGRvY3VtZW50LndyaXRlKCcnKTsKICAgICAgICBz ZWxlY3Rpb24uYWRkUmFuZ2UocmFuZ2UpOwogICAgICAgIHNlbGVjdGlvbi5tb2RpZnkoImV4dGVu ZCIsImZvcndhcmQiLCJkb2N1bWVudGJvdW5kYXJ5Iik7CiAgICAgIH0KICAgIDwvc2NyaXB0Pgog IDwvaGVhZD4KICA8Ym9keSBvbmxvYWQ9ImdvKCkiPgogIDwvYm9keT4KPC9odG1sPgo= 63866 2010-08-09 00:53:32 -0700 2010-08-10 02:48:55 -0700 Patch bug-43032-20100809165330.patch text/plain 3632 tkent ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCAyMTdmZWYwNTI2NDdkYjg5Y2I0YWFhY2M4ZDVmY2ExNTIzNWZlMTBjLi5lMWNmYzAy YWE2ZGFkNDI0YWQ1ZTg0OGQ0MWUzNzEzYmQ0NjRiYjIwIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAK KzIwMTAtMDgtMDkgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0ub3JnPgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEZpeCBjcmFzaGVzIGJ5IFNl bGVjdGlvbkNvbnRyb2xsZXI6Om1vZGlmeSgpLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDMwMzIKKworICAgICAgICAqIGVkaXRpbmcvc2VsZWN0aW9u L3NlbGVjdGlvbi1tb2RpZnktY3Jhc2gtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAgICAgKiBl ZGl0aW5nL3NlbGVjdGlvbi9zZWxlY3Rpb24tbW9kaWZ5LWNyYXNoLmh0bWw6IEFkZGVkLgorCiAy MDEwLTA4LTA4ICBKYW1lcyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5vcmc+CiAKICAgICAg ICAgVW5yZXZpZXdlZC4gIFJlYmFzZWxpbmUgY2hyb21pdW0gcmVzdWx0cyBmb3IgcjY0OTU0Lgpk aWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZWRpdGluZy9zZWxlY3Rpb24vc2VsZWN0aW9uLW1vZGlm eS1jcmFzaC1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9lZGl0aW5nL3NlbGVjdGlvbi9zZWxl Y3Rpb24tbW9kaWZ5LWNyYXNoLWV4cGVjdGVkLnR4dApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRl eCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi43ZWYyMmU5YTQzMWFk MDI3MjcxM2I3MWZkYzg3OTQwMTZjOGVmMTJmCi0tLSAvZGV2L251bGwKKysrIGIvTGF5b3V0VGVz dHMvZWRpdGluZy9zZWxlY3Rpb24vc2VsZWN0aW9uLW1vZGlmeS1jcmFzaC1leHBlY3RlZC50eHQK QEAgLTAsMCArMSBAQAorUEFTUwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZWRpdGluZy9zZWxl Y3Rpb24vc2VsZWN0aW9uLW1vZGlmeS1jcmFzaC5odG1sIGIvTGF5b3V0VGVzdHMvZWRpdGluZy9z ZWxlY3Rpb24vc2VsZWN0aW9uLW1vZGlmeS1jcmFzaC5odG1sCm5ldyBmaWxlIG1vZGUgMTAwNjQ0 CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLmY1ZWJjODQw NTAyN2ExOGM5ODY0NzQ5M2QyOWQ3ZGY5YTMxOTYzYzUKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlv dXRUZXN0cy9lZGl0aW5nL3NlbGVjdGlvbi9zZWxlY3Rpb24tbW9kaWZ5LWNyYXNoLmh0bWwKQEAg LTAsMCArMSwzNCBAQAorPGh0bWw+Cis8aGVhZD4KKzxzY3JpcHQ+CitpZiAod2luZG93LmxheW91 dFRlc3RDb250cm9sbGVyKSB7CisgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIuZHVtcEFzVGV4dCgp OworICAgIGxheW91dFRlc3RDb250cm9sbGVyLndhaXRVbnRpbERvbmUoKTsKK30KKworZnVuY3Rp b24gdGVzdCgpCit7CisgICAgdmFyIGh0bWxFbGVtZW50ID0gZG9jdW1lbnQuZmlyc3RDaGlsZDsK KyAgICAvLyBDcmVhdGUgYSByYW5nZS4KKyAgICB2YXIgc3RhbGVSYW5nZSA9IGRvY3VtZW50LmNy ZWF0ZVJhbmdlKCk7CisKKyAgICAvLyBEZWxldGUgZG9jdW1lbnQuYm9keToKKyAgICB2YXIgcmFu Z2UgPSBkb2N1bWVudC5jcmVhdGVSYW5nZSgpOworICAgIHJhbmdlLnNldEVuZEFmdGVyKGh0bWxF bGVtZW50KTsKKyAgICByYW5nZS5kZWxldGVDb250ZW50cygpOworCisgICAgdmFyIHNlbGVjdGlv biA9IHdpbmRvdy5nZXRTZWxlY3Rpb24oKTsKKyAgICBzZWxlY3Rpb24uYWRkUmFuZ2Uoc3RhbGVS YW5nZSk7CisgICAgc2VsZWN0aW9uLm1vZGlmeSgiZXh0ZW5kIiwgImZvcndhcmQiLCAiZG9jdW1l bnRib3VuZGFyeSIpOyAvLyBXZWJLaXQgY3Jhc2hlZCBoZXJlLgorCisgICAgLy8gUmVjcmVhdGUg dGhlIEhUTUwgZWxlbWVudCB0byBzaG93ICdQQVNTJy4KKyAgICBkb2N1bWVudC5hcHBlbmRDaGls ZChkb2N1bWVudC5jcmVhdGVFbGVtZW50KCdodG1sJykpOworICAgIGRvY3VtZW50LmZpcnN0Q2hp bGQuaW5uZXJIVE1MID0gJzxib2R5PlBBU1M8L2JvZHk+JzsKKyAgICBpZiAod2luZG93LmxheW91 dFRlc3RDb250cm9sbGVyKQorICAgICAgICBsYXlvdXRUZXN0Q29udHJvbGxlci5ub3RpZnlEb25l KCk7Cit9Cis8L3NjcmlwdD4KKzwvaGVhZD4KKzxib2R5IG9ubG9hZD0idGVzdCgpIj4KKzwvYm9k eT4KKzwvaHRtbD4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFu Z2VMb2cKaW5kZXggNzA0YmRjOGNmM2MzZGYyNjk3YWJkMzA3NjYyZDA2Y2JlY2ViMzYwYy4uMWQ1 ZjI3Y2FiZDU2NmFkOGMzNjI2MmY3ODZkM2ZiZjA3ODk3OTBhNCAxMDA2NDQKLS0tIGEvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAx MC0wOC0wOSAgS2VudCBUYW11cmEgIDx0a2VudEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4IGNyYXNoZXMgYnkgU2VsZWN0 aW9uQ29udHJvbGxlcjo6bW9kaWZ5KCkuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD00MzAzMgorCisgICAgICAgIFRlc3Q6IGVkaXRpbmcvc2VsZWN0aW9u L3NlbGVjdGlvbi1tb2RpZnktY3Jhc2guaHRtbAorCisgICAgICAgICogZWRpdGluZy92aXNpYmxl X3VuaXRzLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmVuZE9mRG9jdW1lbnQpOiBBZGQgTlVMTC1j aGVjayBmb3IgZG9jdW1lbnQoKS0+ZG9jdW1lbnRFbGVtZW50KCkuCisKIDIwMTAtMDgtMDggIEtl dmluIE9sbGl2aWVyICA8a2V2aW5vQHRoZW9sbGl2aWVycy5jb20+CiAKICAgICAgICAgW3d4XSBC dWlsZCBmaXgsIGFkZCBtZW1iZXIgbmVlZGVkIHRvIGNvbXBpbGUgTWFjLXNwZWNpZmljIGZvbnQg Y29kZS4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvZWRpdGluZy92aXNpYmxlX3VuaXRzLmNwcCBiL1dl YkNvcmUvZWRpdGluZy92aXNpYmxlX3VuaXRzLmNwcAppbmRleCBkZDQ4NDA2MDFkZmNlM2QzNDI4 ZTZhZTAyYWI5MTBjOTI0YmRjYjRmLi4yNDY3OTM5MjZjNGNhMzQ2OTllODQ5YzhlZTI1YTM1OWQw OTUzZTIwIDEwMDY0NAotLS0gYS9XZWJDb3JlL2VkaXRpbmcvdmlzaWJsZV91bml0cy5jcHAKKysr IGIvV2ViQ29yZS9lZGl0aW5nL3Zpc2libGVfdW5pdHMuY3BwCkBAIC0xMDE5LDcgKzEwMTksNyBA QCBWaXNpYmxlUG9zaXRpb24gc3RhcnRPZkRvY3VtZW50KGNvbnN0IFZpc2libGVQb3NpdGlvbiAm YykKIAogVmlzaWJsZVBvc2l0aW9uIGVuZE9mRG9jdW1lbnQoY29uc3QgTm9kZSogbm9kZSkKIHsK LSAgICBpZiAoIW5vZGUgfHwgIW5vZGUtPmRvY3VtZW50KCkpCisgICAgaWYgKCFub2RlIHx8ICFu b2RlLT5kb2N1bWVudCgpIHx8ICFub2RlLT5kb2N1bWVudCgpLT5kb2N1bWVudEVsZW1lbnQoKSkK ICAgICAgICAgcmV0dXJuIFZpc2libGVQb3NpdGlvbigpOwogICAgIAogICAgIEVsZW1lbnQqIGRv YyA9IG5vZGUtPmRvY3VtZW50KCktPmRvY3VtZW50RWxlbWVudCgpOwo=