Bug 14714 - REGRESSION: ASSERTION FAILED: i < size() in Vector.h:401 on negative -webkit-column-width
Summary: REGRESSION: ASSERTION FAILED: i < size() in Vector.h:401 on negative -webkit-...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Beth Dakin
URL:
Keywords: HasReduction, InRadar, Regression
: 14718 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-07-22 11:32 PDT by David Kilzer (:ddkilzer)
Modified: 2007-07-24 12:22 PDT (History)
5 users (show)

See Also:


Attachments
Test case (45 bytes, text/html)
2007-07-22 11:33 PDT, David Kilzer (:ddkilzer)
no flags Details
Patch v1 (3.21 KB, patch)
2007-07-22 14:43 PDT, David Kilzer (:ddkilzer)
darin: review-
Details | Formatted Diff | Diff
Patch in render tree (7.62 KB, patch)
2007-07-24 11:59 PDT, Beth Dakin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2007-07-22 11:32:59 PDT
* SUMMARY
A reproducible assertion failure occurs with some mangled HTML on debug builds of ToT WebKit.

* STEPS TO REPRODUCE
1. Launch Safari/WebKit with a debug build of WebKit.
2. Open attached test case.

* RESULTS
Safari crashes with an assertion failure.

* REGRESSION
Does NOT crash Safari 3.0.2 (522.12) public beta with its own WebKit on Mac OS X 10.4.10 (8R218).

DOES crash Safari 3.0.2 (522.12) with a local debug build of WebKit r24513 on Mac OS X 10.4.10 (8R218).
Comment 1 David Kilzer (:ddkilzer) 2007-07-22 11:33:40 PDT
Created attachment 15625 [details]
Test case
Comment 2 David Kilzer (:ddkilzer) 2007-07-22 11:36:50 PDT
(In reply to comment #0)
> * REGRESSION
> Does NOT crash Safari 3.0.2 (522.12) public beta with its own WebKit on Mac OS
> X 10.4.10 (8R218).
> 
> DOES crash Safari 3.0.2 (522.12) with a local debug build of WebKit r24513 on
> Mac OS X 10.4.10 (8R218).

DOES crash WebKit nightly r24513 with Safari 3.0 (522.12) on Mac OS X 10.4.10 (8R218).

Version: r24513 (24513)

PID:    3638
Thread: 0

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xfffffff0

Thread 0 Crashed:
0   com.apple.WebCore        	0x011437dc WebCore::RenderBlock::rightmostPosition(bool, bool) const + 508
1   com.apple.WebCore        	0x011a7aec WebCore::RenderTableSection::rightmostPosition(bool, bool) const + 188
2   com.apple.WebCore        	0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312
3   com.apple.WebCore        	0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40
4   com.apple.WebCore        	0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312
5   com.apple.WebCore        	0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40
6   com.apple.WebCore        	0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312
7   com.apple.WebCore        	0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40
8   com.apple.WebCore        	0x011622e8 WebCore::RenderFlow::rightmostPosition(bool, bool) const + 312
9   com.apple.WebCore        	0x01143608 WebCore::RenderBlock::rightmostPosition(bool, bool) const + 40
10  com.apple.WebCore        	0x011546b4 WebCore::RenderView::docWidth() const + 84
11  com.apple.WebCore        	0x011552a0 WebCore::RenderView::layout() + 288
12  com.apple.WebCore        	0x010e9344 WebCore::FrameView::layout(bool) + 1364
13  com.apple.WebCore        	0x010f3584 WebCore::Document::implicitClose() + 788
14  com.apple.WebCore        	0x0142714c WebCore::FrameLoader::checkCallImplicitClose() + 348
15  com.apple.WebCore        	0x014363d4 WebCore::FrameLoader::checkCompleted() + 228
16  com.apple.WebCore        	0x014378e8 WebCore::FrameLoader::finishedParsing() + 104
17  com.apple.WebCore        	0x01021d7c WebCore::HTMLTokenizer::end() + 188
18  com.apple.WebCore        	0x01022228 WebCore::HTMLTokenizer::finish() + 1160
19  com.apple.WebCore        	0x0143943c WebCore::FrameLoader::endIfNotLoadingMainResource() + 124
20  com.apple.WebCore        	0x0143165c WebCore::FrameLoader::finishedLoading() + 92
21  com.apple.WebCore        	0x01443448 WebCore::MainResourceLoader::didFinishLoading() + 56
22  com.apple.WebCore        	0x01415454 -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] + 84
23  com.apple.Foundation     	0x92c1489c -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback] + 188
24  com.apple.Foundation     	0x92c12b08 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 556
25  com.apple.Foundation     	0x92c12860 _sendCallbacks + 156
26  com.apple.CoreFoundation 	0x907de4fc __CFRunLoopDoSources0 + 384
27  com.apple.CoreFoundation 	0x907dda2c __CFRunLoopRun + 452
28  com.apple.CoreFoundation 	0x907dd4ac CFRunLoopRunSpecific + 268
29  com.apple.HIToolbox      	0x9329ab20 RunCurrentEventLoopInMode + 264
30  com.apple.HIToolbox      	0x9329a1b4 ReceiveNextEventCommon + 380
31  com.apple.HIToolbox      	0x9329a020 BlockUntilNextEventMatchingListInMode + 96
32  com.apple.AppKit         	0x937a0ae4 _DPSNextEvent + 384
33  com.apple.AppKit         	0x937a07a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
34  com.apple.Safari         	0x00006740 0x1000 + 22336
35  com.apple.AppKit         	0x9379ccec -[NSApplication run] + 472
36  com.apple.AppKit         	0x9388d87c NSApplicationMain + 452
37  com.apple.Safari         	0x0005c77c 0x1000 + 374652
38  com.apple.Safari         	0x0005c624 0x1000 + 374308

Comment 3 David Kilzer (:ddkilzer) 2007-07-22 11:37:38 PDT
<rdar://problem/5352884>
Comment 4 David Kilzer (:ddkilzer) 2007-07-22 11:38:35 PDT
Regression per Comment #2.

Comment 5 David Kilzer (:ddkilzer) 2007-07-22 12:29:23 PDT
I have a fix for this.  Basically, -webkit-column-width should not allow negative widths.

Comment 6 David Kilzer (:ddkilzer) 2007-07-22 14:43:51 PDT
Created attachment 15628 [details]
Patch v1

Proposed patch.  Don't allow negative values for -webkit-column-width.

NOTE TO REVIEWER: I did not trace through the render code to determine why negative values failed.  I simply added the FNonNeg flag when parsing -webkit-column-width values to prevent non-negative numbers from being recognized as valid.
Comment 7 David Kilzer (:ddkilzer) 2007-07-22 15:29:16 PDT
(In reply to comment #6)
> Created an attachment (id=15628) [edit]
> Patch v1
> 
> Proposed patch.  Don't allow negative values for -webkit-column-width.
> 
> NOTE TO REVIEWER: I did not trace through the render code to determine why
> negative values failed.  I simply added the FNonNeg flag when parsing
> -webkit-column-width values to prevent non-negative numbers from being
> recognized as valid.

I should also note that all layout tests passed.

Comment 8 David Kilzer (:ddkilzer) 2007-07-22 15:37:50 PDT
Note that this test case was derived from the original iExploder test case in Bug 14715.

Comment 9 David Kilzer (:ddkilzer) 2007-07-22 20:12:44 PDT
See also Bug 14718.

Comment 10 Darin Adler 2007-07-24 08:46:06 PDT
Comment on attachment 15628 [details]
Patch v1

The CSS specification doesn't say anything about column widths needing to be positive, and I also think a column width of 0 could be a problem.

I think the fix for this should be in RenderBlock::calcColumnWidth. Setting review- to keep the ball rolling on this. Hyatt should really take a look.
Comment 11 Beth Dakin 2007-07-24 11:13:32 PDT
Hyatt has mentioned that he would really like to pull our current implementation of column layout before Leopard ships. I wonder if that is how he will want to fix this bug. Looking into it in the meantime anyway.
Comment 12 Beth Dakin 2007-07-24 11:33:26 PDT
If we decide not to fix this by yanking column layout entirely, this will be easy to fix, but we still need to decide on a behavior change. As Darin mentioned, the CSS spec doesn't seem to address the issue, but a negative column width doesn't make any sense to me. I tend to think we should treat all column widths that are less than 1 as 1.
Comment 13 Beth Dakin 2007-07-24 11:46:58 PDT
I have a patch. Just putting it together with the layout tests.
Comment 14 Beth Dakin 2007-07-24 11:59:43 PDT
Created attachment 15668 [details]
Patch in render tree
Comment 15 Darin Adler 2007-07-24 12:03:46 PDT
Comment on attachment 15668 [details]
Patch in render tree

r=me
Comment 16 Beth Dakin 2007-07-24 12:06:28 PDT
Fixed with r24599.
Comment 17 Beth Dakin 2007-07-24 12:07:00 PDT
*** Bug 14718 has been marked as a duplicate of this bug. ***