Bug 44099 - REGRESSION(r65468): Crashes in StringImpl::find
Summary: REGRESSION(r65468): Crashes in StringImpl::find
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Yuta Kitamura
URL:
Keywords:
Depends on:
Blocks: 44080
  Show dependency treegraph
 
Reported: 2010-08-17 04:11 PDT by Yuta Kitamura
Modified: 2010-08-18 06:49 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.54 KB, patch)
2010-08-17 04:29 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff
Fix for second overrun (4.73 KB, patch)
2010-08-17 16:14 PDT, Gavin Barraclough
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2010-08-17 04:11:28 PDT
Since r65468, Chromium buildbots are reporting a lot of crashes inside StringImpl::find.

I guess I have found the problem; I'll upload a patch shortly.
Comment 1 Yuta Kitamura 2010-08-17 04:29:20 PDT
Created attachment 64573 [details]
Patch
Comment 2 Shinichiro Hamaji 2010-08-17 04:35:58 PDT
Comment on attachment 64573 [details]
Patch

Looks good.
Comment 3 WebKit Commit Bot 2010-08-17 04:55:11 PDT
Comment on attachment 64573 [details]
Patch

Clearing flags on attachment: 64573

Committed r65493: <http://trac.webkit.org/changeset/65493>
Comment 4 WebKit Commit Bot 2010-08-17 04:55:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Darin Adler 2010-08-17 14:16:20 PDT
Comment on attachment 64573 [details]
Patch

I would have preferred a fix that used a break inside the loop rather than repeating the logic from inside the loop.

    for (unsigned i = 0; ; ++i) {
        if (searchHash == matchHash && equal(searchCharacters + i, matchString, matchLength))
            return index + i;
        if (i >= delta)
            break;
        ...
Comment 8 Gavin Barraclough 2010-08-17 16:14:36 PDT
Created attachment 64650 [details]
Fix for second overrun

Landed in r65571
Comment 9 Gavin Barraclough 2010-08-17 16:17:35 PDT
Thanks for catching this Yuta.  I think the problem is just the overrun, but there is a second find method with the same issue, so I've landed a fix for this.  Hopefully this should resolved the remaining crashes.
Comment 10 Yuta Kitamura 2010-08-18 06:49:39 PDT
(In reply to comment #9)
> Thanks for catching this Yuta.  I think the problem is just the overrun, but there is a second find method with the same issue, so I've landed a fix for this.  Hopefully this should resolved the remaining crashes.

It seems the crashes are gone. Thanks!