Bug 134514 - Debugger's breakpoint list should not be a Vector
Summary: Debugger's breakpoint list should not be a Vector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-07-01 14:50 PDT by Mark Lam
Modified: 2014-07-01 16:43 PDT (History)
11 users (show)

See Also:

Attachments
the patch (9.48 KB, patch)
2014-07-01 15:38 PDT, Mark Lam 		ggaren: review+
Details | Formatted Diff | Diff
updated patch to make the EWS bots happy + Geoff's suggested fix. (9.49 KB, patch)
2014-07-01 15:51 PDT, Mark Lam 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-07-01 14:50:25 PDT 
The debugger currently stores breakpoint data as entries in a Vector (see BreakpointsInLine).  It also keeps a fast map look up of breakpoint IDs to the breakpoint data (see m_breakpointIDToBreakpoint).  Because a Vector can compact or reallocate it backing store, this can causes all sorts of havoc.  The m_breakpointIDToBreakpoint assumes that the breakpoint data doesn't move in memory.

The fix is to replace the BreakpointsInLine Vector with a BreakpointsList doubly linked list.
Comment 1 Mark Lam 2014-07-01 14:51:00 PDT 
<rdar://problem/17487061>
Comment 2 Mark Lam 2014-07-01 15:38:59 PDT 
Created attachment 234208 [details]
the patch
Comment 3 Geoffrey Garen 2014-07-01 15:46:04 PDT 
Comment on attachment 234208 [details]
the patch

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

r=me

> Source/JavaScriptCore/debugger/Breakpoint.h:56
> +    Breakpoint(Breakpoint& other)

Should be const&.
Comment 4 Mark Lam 2014-07-01 15:49:40 PDT 
The compiler was complaining about:
Source/JavaScriptCore/debugger/Breakpoint.h:88:27: error: using the result of an assignment as a condition without parentheses [-Werror,-Wparentheses]
        while (breakpoint = removeHead())
               ~~~~~~~~~~~^~~~~~~~~~~~~~

Will add the parens.
Comment 5 Mark Lam 2014-07-01 15:51:28 PDT 
Created attachment 234209 [details]
updated patch to make the EWS bots happy + Geoff's suggested fix.
Comment 6 Mark Lam 2014-07-01 16:43:12 PDT 
Thanks.  Patch with fixes land in r170677: <http://trac.webkit.org/r170677>.