503981 - Double free when realloc fails in JSTempVector::GrowTo

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Attachment: stack traces for realloc failure and second free

var a = [];
var s = "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456";
for (var i = 0; i < 60000000; ++i)
  a.push(s);
"" + a;

takes about 20 seconds with -j, then causes malloc to complain: "pointer being freed was not allocated".

Looks like GrowTo's realloc fails, GrowTo deletes mBegin, and then the destructor deletes mBegin again.  Which should be responsible for deleting mBegin in an OOM situation, GrowTo or the destructor?

Comment 1

[Luke Wagner [:luke]]

Attachment: remove free on realloc

Oh dear.  I tested for this, but that was before I too-hastily slapped in the POD-handling special case.  Your diagnosis is correct; growTo should not be freeing on realloc failure, since the destructor will do that instead.  Thanks!

Comment 2

[Robert Sayre]

Jesse, nice work.

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 388368 [details] [diff] [review]
remove free on realloc

Ugh, how'd I miss this...

Comment 4

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/d1b9ec46733f

with obscured commit message, not that it matters for anyone who ever takes a look at the change itself...

Comment 5

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d1b9ec46733f