Bug 146494 - REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp
Summary: REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-30 22:35 PDT by Brady Eidson
Modified: 2015-07-01 10:14 PDT (History)
3 users (show)

See Also:


Attachments
Patch v1 (1.69 KB, patch)
2015-06-30 22:38 PDT, Brady Eidson
darin: review+
beidson: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-06-30 22:35:17 PDT
REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp

Steps to reproduce:
1 - Download FluidApp from http://fluidapp.com
2 - Create an app from a website (I used webkit.org)
3 - Launch that WebApp

Crash:
1   0x1034112e0 WTFCrash
2   0x104dbe7d9 WTF::CrashOnOverflow::crash()
3   0x104dbe7a9 WTF::CrashOnOverflow::overflowed()
4   0x104dbf471 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul>::at(unsigned long)
5   0x104e51edd WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul>::operator[](unsigned long)
6   0x104eb909b -[WebHistoryItem initFromDictionaryRepresentation:]
7   0x104eb0897 -[WebHistoryPrivate loadHistoryGutsFromURL:savedItemsCount:collectDiscardedItemsInto:error:]
8   0x104eb0a8c -[WebHistoryPrivate loadFromURL:collectDiscardedItemsInto:error:]
9   0x104eb17db -[WebHistory loadFromURL:error:]
10  0x100019c11 -[FUHistoryController setUpHistory]
11  0x1000198ca -[FUHistoryController init]
12  0x100019807 +[FUHistoryController instance]
13  0x10000e293 -[FUApplication finishLaunching]
14  0x7fff8b010a21 -[NSApplication run]
15  0x7fff8af8d354 NSApplicationMain
16  0x1000017c4 start

The code in question:
        auto redirectURLsVector = std::make_unique<Vector<String>>();

        for (NSUInteger i = 0; i < size; ++i) {
            id redirectURL = [redirectURLs objectAtIndex:i];
            if (![redirectURL isKindOfClass:[NSString class]])
                continue;

            (*redirectURLsVector)[i] = (NSString *)redirectURL;
        }

We create a Vector of size 0 then immediately reference the first element.

That's obviously wrong.

In radar as rdar://problem/21598293
Comment 1 Brady Eidson 2015-06-30 22:38:01 PDT
Created attachment 255899 [details]
Patch v1
Comment 2 Brady Eidson 2015-06-30 22:39:01 PDT
Was introduced in http://trac.webkit.org/changeset/185319
Comment 3 Darin Adler 2015-07-01 09:25:15 PDT
Comment on attachment 255899 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=255899&action=review

> Source/WebKit/mac/History/WebHistoryItem.mm:363
>          NSUInteger size = [redirectURLs count];

After making the thing below a modern Objective-C for loop this can be called right in the reserveInitialCapacity call below.

> Source/WebKit/mac/History/WebHistoryItem.mm:365
> +        redirectURLsVector->reserveCapacity(size);

This should be reserveInitialCapacity.

> Source/WebKit/mac/History/WebHistoryItem.mm:368
>          for (NSUInteger i = 0; i < size; ++i) {
>              id redirectURL = [redirectURLs objectAtIndex:i];

This should be a modern Objective-C for loop:

    for (id redirectURL in redirectURLs) {

> Source/WebKit/mac/History/WebHistoryItem.mm:372
> +            redirectURLsVector->append((NSString *)redirectURL);

This should be uncheckedAppend.
Comment 4 Brady Eidson 2015-07-01 10:14:28 PDT
https://trac.webkit.org/changeset/186179