505108 - Early returns in Area::ParseCoords leak "char* cp"

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Early returns in Area::ParseCoords leak "char* cp".  Regression from bug 182554, I believe.

2 bytes in 2 blocks are definitely lost in loss record 15 of 1,703
malloc (vg_replace_malloc.c:193)
NS_Alloc_P (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
ToNewCString(nsAString_internal const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
Area::ParseCoords(nsAString_internal const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
RectArea::ParseCoords(nsAString_internal const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageMap::AddArea(nsIContent*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageMap::SearchForAreas(nsIContent*, int&, int&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageMap::Init(nsIPresShell*, nsIFrame*, nsIDOMHTMLMapElement*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageFrame::GetImageMap(nsPresContext*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageFrame::PaintImage(nsIRenderingContext&, nsPoint, nsRect const&, imgIContainer*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsDisplayImage::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsDisplayClip::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)

Comment 1

[Jesse Ruderman]

http://hg.mozilla.org/mozilla-central/file/f92608198566/layout/generic/nsImageMap.cpp#l164

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Simple patch to correct this

Also corrects that it was freeing the ToNewCString() string with NS_Free() when it's supposed to use nsMemory::Free().

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 538669 [details] [diff] [review]
Simple patch to correct this

Review of attachment 538669 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 4

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/11b4b4c73f36

Comment 5

[Nicholas Nethercote [inactive]]

How significant was this leak?  Each leak is a single byte (plus heap block overhead, presumably), how often did they occur?

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Normally single byte (rounded up to allocation chunk size), plus an all-whitespace string would be leaked.

No idea how frequent; it's obviously wrong and was hit by Jesse's tracemalloc run (it appears).  I'd *guess* infrequent to rare, but that's a guess.

Now fixed in any case.  Don't count on it for anything significant; if you have testcases you can look before and after, or add a debug when it's hit.