Bug 9317 – REGRESSION: crash in HTML tokenizer at Japanese Apple support page

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Rachael Worthington (cheers)]

loading the above URL crashes the nightly immediately.

following crash report is from identical crash in OmniWeb on same site, based on WebKit rev 13295

Thread 0 Crashed:
srr0: 0x34591f0c  srr1: 0x0200f930   cr: 0x84024222  xer: 0x20000000  lr: 0x34591f0c ctr: 0x91437800
r0: 0x34591f0c  r8: 0x91437808  r16: 0x00000000  r24: 0x0125e930
r1: 0xbfffdcd0  r9: 0x00000000  r17: 0xbfffeb90  r25: 0x0125e800
r2: 0x84024222  r10: 0xf1cb61f0  r18: 0x00006edb  r26: 0x00000001
r3: 0x00000000  r11: 0x84024222  r19: 0x011876e0  r27: 0xbfffdd7c
r4: 0x00000000  r12: 0x00000000  r20: 0x1d1d547f  r28: 0x00000000
r5: 0xbfffdd7c  r13: 0x00000000  r21: 0xc621f615  r29: 0x347d262c
r6: 0x00000001  r14: 0x00000001  r22: 0x00000001  r30: 0x17ee6968
r7: 0x44847d19  r15: 0x00000000  r23: 0xbfffde4c  r31: 0x34496f94
    0 -- 0x34591f0c -- __ZN7WebCore8NodeImpl13dispatchEventEN8KXMLCore10PassRefPtrINS_9EventImplEEERib
    1 -- 0x34591f0c -- __ZN7WebCore8NodeImpl13dispatchEventEN8KXMLCore10PassRefPtrINS_9EventImplEEERib
    2 -- 0x345922a0 -- __ZN7WebCore8NodeImpl17dispatchHTMLEventERKNS_12AtomicStringEbb
    3 -- 0x34497160 -- __ZN7WebCore13HTMLTokenizer14notifyFinishedEPNS_12CachedObjectE
    4 -- 0x345a8528 -- __ZN7WebCore12CachedScript11checkNotifyEv
    5 -- 0x345a867c -- __ZN7WebCore12CachedScript4dataERNS_5ArrayIcEEb
    6 -- 0x345aae8c -- __ZN7WebCore6Loader15receivedAllDataEPNS_11TransferJobEP6NSData
    7 -- 0x344b6c94 -- -[KWQResourceLoader finishJobAndHandle:]
    8 -- 0x005dfb60 -- -[WebSubresourceLoader didFinishLoading]
    9 -- 0x005e7abc -- -[WebLoader connectionDidFinishLoading:]
   10 -- 0x929a884c -- -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback]
   11 -- 0x929a6ab8 -- -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]
   12 -- 0x929a6810 -- __sendCallbacks
   13 -- 0x907e44cc -- ___CFRunLoopDoSources0
   14 -- 0x907e39fc -- ___CFRunLoopRun
   15 -- 0x907e347c -- _CFRunLoopRunSpecific
   16 -- 0x9321d980 -- _RunCurrentEventLoopInMode
   17 -- 0x9321d014 -- _ReceiveNextEventCommon
   18 -- 0x9321ce80 -- _BlockUntilNextEventMatchingListInMode
   19 -- 0x9371fe84 -- __DPSNextEvent
   20 -- 0x9371fb48 -- -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
   21 -- 0x9371c08c -- -[NSApplication run]
   22 -- 0x003cac54 -- -[OAApplication run]
   23 -- 0x9380cbfc -- _NSApplicationMain
   24 -- 0x00029d0c -- _main
   25 -- 0x0002a5a4 -- __start
   26 -- 0x00002a0c -- start

[Maciej Stachowiak]

This is a regression relative to the last released Safari. Marking as such and upgrading to P1.

[Alexey Proskuryakov]

Debug build gets an assertion failure:

ASSERTION FAILED: !scriptNode (/Users/ap/WebKit/WebCore/html/HTMLTokenizer.cpp:1166 WebCore::HTMLTokenizer::State WebCore::HTMLTokenizer::parseTag(WebCore::SegmentedString&, WebCore::HTMLTokenizer::State))

[Alice Liu]

<rdar://problem/4575381>

[Darin Adler]

There seem to be two halves to this crash.

Half the problem is a crash with script nesting. I've created a much-reduced test case that demonstrates this.

The other half of the problem seems to be a yen character mixup, where a \ is being used to escape the / in </script> -- the \ looks like a yen character and I think it might not be working properly. The reduced test case does not address that.

Once the crash is fixed we need to look at the page again and make sure the JavaScript is being loaded properly.

[Darin Adler]

Created attachment 8749 [details]
greatly reduced test case

(In reply to comment #4)
> There seem to be two halves to this crash.
> 
> Half the problem is a crash with script nesting. I've created a much-reduced
> test case that demonstrates this. 

That would be bug 9554. Not closing this as a duplicate, but it's the second half that needs to be reduced.

*** Bug 9554 has been marked as a duplicate of this bug. ***

Created attachment 9050 [details]
Patch for the crashing half

This fixes the crash and behaves correctly with the test case from bug 9554. It also passes all the layout tests. I haven't done much testing beyond that (in particular, with external scripts, cached and uncached).

[Maciej Stachowiak]

Comment on attachment 9050 [details]
Patch for the crashing half

looks good, please add test case and changelog

Created attachment 9061 [details]
Patch for the crashing half, now with changelog and test

[Darin Adler]

Comment on attachment 9061 [details]
Patch for the crashing half, now with changelog and test

r=me

[Darin Adler]

Committed revision 15075.