551279 - Mismatched allocators (operator new[] and free) in gfxTextRun

Status: VERIFIED DUPLICATE of bug 550805

(Core :: Graphics, defect)

Description

[Jesse Ruderman]

I don't completely understand this valgrind complaint.  Is it due to the recent Gecko-wide allocator changes?

(Not attaching a testcase, but I think one of the testcases in bug 499862 triggers this code.)

Comment 1

[Jesse Ruderman]

Attachment: valgrind complaint

Comment 2

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

I think this is a regression from bug 545989.

Comment 3

[Jonathan Kew [:jfkthame]]

The patch in bug 545989 was sound, AFAICS... it removes custom allocation (and the custom delete operator) from the gfxTextRun, and the factory Create() method now just uses operator new to create it. And for the auxiliary storage (mCharacterGlyphs array), it uses correctly-paired new (std::nothrow) [] and delete[].

However, looking at memory/mozalloc/mozalloc.h, I notice that it does not appear to redefine the (std::nothrow) variants of operator new / new[]. I think that's probably the root of the problem - and it will also apply to other callsites where we've used new (std::nothrow). There aren't many, but mxr finds a few examples in WebGL and Canvas as well as font code; and also some custom allocators in wince code that may need to be checked.

Comment 4

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

Mind fixing all those std::nothrows? :-)

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #3)
> However, looking at memory/mozalloc/mozalloc.h, I notice that it does not
> appear to redefine the (std::nothrow) variants of operator new / new[]. I think
> that's probably the root of the problem - and it will also apply to other
> callsites where we've used new (std::nothrow). There aren't many, but mxr finds
> a few examples in WebGL and Canvas as well as font code; and also some custom
> allocators in wince code that may need to be checked.

Yeah, this was my fault.  There's a patch floating around that adds (fallible) ::operator new(nothrow) to mozalloc, should land soon I hope.

Comment 6

[Jonathan Kew [:jfkthame]]

This is really a dup of bug 550805, just reported in a different form.

(TBH, I don't actually understand why we're inventing mozilla::fallible instead of just using std::nothrow. Seems like that would be the standard C++ idiom for this functionality.)