Bug 101054

Summary: [V8] Add ASSERT() to guarantee that we don't store NULL pointers to V8 internal fields
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore JavaScriptAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, japhet, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 101078, 101190, 101212    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
patch for landing none

Kentaro Hara
Reported 2012-11-02 08:12:30 PDT
I'm investigating a Chromium crash bug: http://code.google.com/p/chromium/issues/detail?id=155942 I've not yet identified the root cause (because I can't reproduce it), but it looks like we are storing NULL pointers to V8 internal fields. Just in case, we can add an ASSERT() to guarantee that NULL pointers are never stored. (Also I'm hoping that this ASSERT() will give me more debug information.)
Attachments
Patch (1.93 KB, patch)
2012-11-02 08:16 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (1.62 KB, patch)
2012-11-05 00:45 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (2.14 KB, patch)
2012-11-11 22:34 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
patch for landing (2.17 KB, patch)
2012-11-12 21:14 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2012-11-02 08:16:53 PDT
Created attachment 172063 [details] Patch
WebKit Review Bot
Comment 2 2012-11-02 10:43:13 PDT
Comment on attachment 172063 [details] Patch Clearing flags on attachment: 172063 Committed r133313: <http://trac.webkit.org/changeset/133313>
WebKit Review Bot
Comment 3 2012-11-02 10:43:16 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 4 2012-11-02 11:37:10 PDT
Re-opened since this is blocked by bug 101078
Kentaro Hara
Comment 5 2012-11-02 12:15:56 PDT
Looks strange... I'm heading for a hotel, will take a look in the weekend.
Kentaro Hara
Comment 6 2012-11-05 00:45:04 PST
Created attachment 172285 [details] Patch
Kentaro Hara
Comment 7 2012-11-11 22:34:24 PST
Created attachment 173558 [details] Patch
Kentaro Hara
Comment 8 2012-11-11 22:34:53 PST
I think now it's safe to add the ASSERT()s.
Adam Barth
Comment 9 2012-11-12 09:33:23 PST
Comment on attachment 173558 [details] Patch ok
WebKit Review Bot
Comment 10 2012-11-12 16:53:45 PST
Comment on attachment 173558 [details] Patch Rejecting attachment 173558 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: h']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Parsed 2 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/bindings/v8/V8DOMWrapper.h Hunk #1 FAILED at 66. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/bindings/v8/V8DOMWrapper.h.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/14820316
Kentaro Hara
Comment 11 2012-11-12 21:14:43 PST
Created attachment 173808 [details] patch for landing
WebKit Review Bot
Comment 12 2012-11-12 22:59:59 PST
Comment on attachment 173808 [details] patch for landing Clearing flags on attachment: 173808 Committed r134369: <http://trac.webkit.org/changeset/134369>
WebKit Review Bot
Comment 13 2012-11-12 23:00:03 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.