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

See Also:

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

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
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


> 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>.