Created attachment 243017 [details] Test case Load this test with debug WebKit: <!DOCTYPE html> <datalist> <a> <form> <select required=""></select> </a> </datalist> Note: the closing tag of <form> is missing. Backtrace: ASSERTION FAILED: m_isValid == valid() ../../Source/WebCore/html/HTMLFormControlElement.cpp(463) : bool WebCore::HTMLFormControlElement::isValidFormControlElement() const Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fff98927700 (LWP 5586)] 0x00007fffedbca36f in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321 321 *(int *)(uintptr_t)0xbbadbeef = 0; #0 0x00007fffedbca36f in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321 #1 0x00007ffff32bf940 in WebCore::HTMLFormControlElement::isValidFormControlElement (this=0x474a70) at ../../Source/WebCore/html/HTMLFormControlElement.cpp:463 #2 0x00007ffff32be0c0 in WebCore::HTMLFormControlElement::insertedInto (this=0x474a70, insertionPoint=...) at ../../Source/WebCore/html/HTMLFormControlElement.cpp:252 #3 0x00007ffff32c0f21 in WebCore::HTMLFormControlElementWithState::insertedInto (this=0x474a70, insertionPoint=...) at ../../Source/WebCore/html/HTMLFormControlElementWithState.cpp:50 #4 0x00007ffff3327dba in WebCore::HTMLSelectElement::insertedInto (this=0x474a70, insertionPoint=...) at ../../Source/WebCore/html/HTMLSelectElement.cpp:1590 #5 0x00007ffff3046abd in WebCore::ChildNodeInsertionNotifier::notifyNodeInsertedIntoTree (this=0x7fffffffcdc0, node=...) at ../../Source/WebCore/dom/ContainerNodeAlgorithms.h:211 #6 0x00007ffff3046c02 in WebCore::ChildNodeInsertionNotifier::notify (this=0x7fffffffcdc0, node=...) at ../../Source/WebCore/dom/ContainerNodeAlgorithms.h:230 #7 0x00007ffff30444fc in WebCore::ContainerNode::parserAppendChild (this=0x471a40, newChild=...) at ../../Source/WebCore/dom/ContainerNode.cpp:755 #8 0x00007ffff3041946 in WebCore::ContainerNode::takeAllChildrenFrom (this=0x471a40, oldParent=0x712dd0) at ../../Source/WebCore/dom/ContainerNode.cpp:128 #9 0x00007ffff33c4af1 in WebCore::executeTakeAllChildrenTask (task=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:134 #10 0x00007ffff33c4b5c in WebCore::executeTask (task=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:153 #11 0x00007ffff33c4e8f in WebCore::HTMLConstructionSite::executeQueuedTasks (this=0x8175c8) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:193 #12 0x00007ffff33f6604 in WebCore::HTMLTreeBuilder::constructTree (this=0x8175b0, token=0x7fffffffd050) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:366 #13 0x00007ffff33ce663 in WebCore::HTMLDocumentParser::constructTreeFromHTMLToken (this=0x816f70, rawToken=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:352 #14 0x00007ffff33ce299 in WebCore::HTMLDocumentParser::pumpTokenizer (this=0x816f70, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:309 #15 0x00007ffff33cda31 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x816f70, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:189 #16 0x00007ffff33cebf9 in WebCore::HTMLDocumentParser::append (this=0x816f70, inputSource=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:428 #17 0x00007ffff305ac81 in WebCore::DecodedDataDocumentParser::flush (this=0x816f70, writer=...) at ../../Source/WebCore/dom/DecodedDataDocumentParser.cpp:60 #18 0x00007ffff3538b45 in WebCore::DocumentWriter::end (this=0x7d28e0) at ../../Source/WebCore/loader/DocumentWriter.cpp:243 #19 0x00007ffff35248db in WebCore::DocumentLoader::finishedLoading (this=0x7d2840, finishTime=0) at ../../Source/WebCore/loader/DocumentLoader.cpp:440 #20 0x00007ffff3524644 in WebCore::DocumentLoader::notifyFinished (this=0x7d2840, resource=0x8af2f0) at ../../Source/WebCore/loader/DocumentLoader.cpp:374 #21 0x00007ffff35d5370 in WebCore::CachedResource::checkNotify (this=0x8af2f0) at ../../Source/WebCore/loader/cache/CachedResource.cpp:293 #22 0x00007ffff35d546e in WebCore::CachedResource::finishLoading (this=0x8af2f0) at ../../Source/WebCore/loader/cache/CachedResource.cpp:309 #23 0x00007ffff35d1b63 in WebCore::CachedRawResource::finishLoading (this=0x8af2f0, data=0x8b2350) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:104 #24 0x00007ffff358594c in WebCore::SubresourceLoader::didFinishLoading (this=0x8af9c0, finishTime=0) at ../../Source/WebCore/loader/SubresourceLoader.cpp:306 #25 0x00007ffff35816e1 in WebCore::ResourceLoader::didFinishLoading (this=0x8af9c0, finishTime=0) at ../../Source/WebCore/loader/ResourceLoader.cpp:508 #26 0x00007ffff3f303e1 in WebCore::readCallback (asyncResult=0x8161d0, data=0x8b0040) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1300 #27 0x00007fffeb7ab7d6 in async_ready_callback_wrapper (source_object=0x7c66d0, res=0x8161d0, user_data=user_data@entry=0x8b0040) at ginputstream.c:523 #28 0x00007fffeb7d10d5 in g_task_return_now (task=0x8161d0) at gtask.c:1077 #29 0x00007fffeb7d10f9 in complete_in_idle_cb (task=0x8161d0) at gtask.c:1086 #30 0x00007fffeaa10a1d in g_main_dispatch (context=0x4780a0) at gmain.c:3064 #31 g_main_context_dispatch (context=context@entry=0x4780a0) at gmain.c:3663 #32 0x00007fffeaa10d88 in g_main_context_iterate (context=0x4780a0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3734 #33 0x00007fffeaa1104a in g_main_loop_run (loop=0x8eb8a0) at gmain.c:3928 #34 0x00007ffff45df9dc in WTF::RunLoop::run () at ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:59 #35 0x00007ffff2b44f82 in WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain> (argc=2, argv=0x7fffffffd978) at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61 #36 0x00007ffff2b44de7 in WebKit::WebProcessMainUnix (argc=2, argv=0x7fffffffd978) at ../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:73 #37 0x0000000000400891 in main (argc=2, argv=0x7fffffffd978) at ../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44
The way this should have worked is that parsing the node should have called parseAttribute for requiredAttr, which should have called requiredAttributeChanged, which called updateValidity, which set m_isValid to the result of valid(). Since that time, nothing should have made m_isValid get out of sync with the result of valid().
I hit this 100% in my debug build of master, loading https://github.com/WebKit/webkit
This bug reproduces for me on github.com right now
I might have a chance to fix this some time soon. It’s not clear from the assertion failure alone whether this is a super-critical problem or not. One of the most helpful things someone could do would be to create a test case demonstrating the failure that can be used as a regression test once we fix it.
(In reply to comment #4) > I might have a chance to fix this some time soon. It’s not clear from the > assertion failure alone whether this is a super-critical problem or not. One > of the most helpful things someone could do would be to create a test case > demonstrating the failure that can be used as a regression test once we fix > it. The testcase from Fuzzinator in comment #0 works for me (crashes my web process) in trunk.
Oops, overlooked that. To be clear, by “crashes” you mean hits an assertion in a debug build.
(In reply to comment #6) > Oops, overlooked that. > > To be clear, by “crashes” you mean hits an assertion in a debug build. Exactly, I should be more clear.
Created attachment 274422 [details] Patch
Comment on attachment 274422 [details] Patch This looks sane to me.
Comment on attachment 274422 [details] Patch Clearing flags on attachment: 274422 Committed r198451: <http://trac.webkit.org/changeset/198451>
All reviewed patches have been landed. Closing bug.
I'm still getting assert on github.com, maybe it deserves bug entry though: #0 WTFCrash () at ../../../Source/WTF/wtf/Assertions.cpp:322 #1 0x00007ffff40bc84d in WebCore::HTMLFormControlElement::isValidFormControlElement (this=0x7fff94f7ea50) at ../../../Source/WebCore/html/HTMLFormControlElement.cpp:495 #2 0x00007ffff4060659 in WebCore::HTMLFormControlElement::checkValidity (this=0x7fff94f7ea50, unhandledInvalidControls=0x0) at ../../../Source/WebCore/html/HTMLFormControlElement.cpp:480 #3 0x00007ffff50d9302 in WebCore::jsHTMLInputElementPrototypeFunctionCheckValidity (state=0x7fffffffb0d0) at DerivedSources/WebCore/JSHTMLInputElement.cpp:1866
Yes it seems this reduction and the bug I fixed is not the same bug seen on github then. Please do make a new bug report. I am not personally a github user and made no attempt to test it. The same issue will exist with that bug as with this: will need to make reduced test cases.
(In reply to comment #13) > Yes it seems this reduction and the bug I fixed is not the same bug seen on > github then. Please do make a new bug report. I am not personally a github > user and made no attempt to test it. The same issue will exist with that bug > as with this: will need to make reduced test cases. Another repro is available at https://bugs.webkit.org/show_bug.cgi?id=155720 (though it's not the reduction of the github issue yet).
It would be better if your fuzzer could find test case with HTMLFormControlElement::checkValidity() :)
(In reply to comment #15) > It would be better if your fuzzer could find test case with > HTMLFormControlElement::checkValidity() :) Unfortunately this is not the way how fuzzers work (at least not mine :) ), however I was able to reduce the github.com issue with an automatic test case minimizer. The test is available here: https://bugs.webkit.org/show_bug.cgi?id=155748
>Unfortunately this is not the way how fuzzers work It was a joke actually, but seriously, it should be possible to modify condition which classifies generated test cases from "crash browser" to "crash browser && output contains checkValidity" >however I was able to reduce the github.com issue with an automatic test case minimizer Great, thank you very much! Out of curiosity, what minimizer are you using?
(In reply to comment #17) > >Unfortunately this is not the way how fuzzers work > > It was a joke actually, but seriously, it should be possible to modify > condition which classifies generated test cases from "crash browser" to > "crash browser && output contains checkValidity" Aaah, I see. I can classify them this way. I thought that you meant to generate test cases that makes the browser crash in a given function :) > >however I was able to reduce the github.com issue with an automatic test case minimizer > > Great, thank you very much! Out of curiosity, what minimizer are you using? I use various private tools as part of my fuzzer framework (but I plan to release at least some of them sometime soon).