Bug 134514

Summary: Debugger's breakpoint list should not be a Vector
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cdumez, commit-queue, fpizlo, ggaren, gyuyoung.kim, mhahnenberg, mmirman, msaboff, oliver, sergio
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch
ggaren: review+
updated patch to make the EWS bots happy + Geoff's suggested fix. none

Mark Lam
Reported 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.
Attachments
the patch (9.48 KB, patch)
2014-07-01 15:38 PDT, Mark Lam 		ggaren: review+
DetailsFormatted DiffDiff
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
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2014-07-01 14:51:00 PDT
<rdar://problem/17487061>
Mark Lam
Comment 2 2014-07-01 15:38:59 PDT
Created attachment 234208 [details] the patch
Geoffrey Garen
Comment 3 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&.
Mark Lam
Comment 4 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.
Mark Lam
Comment 5 2014-07-01 15:51:28 PDT
Created attachment 234209 [details] updated patch to make the EWS bots happy + Geoff's suggested fix.
Mark Lam
Comment 6 2014-07-01 16:43:12 PDT
Thanks. Patch with fixes land in r170677: <http://trac.webkit.org/r170677>.
Note You need to log in before you can comment on or make changes to this bug.