351739 - Memory leak in |JS_dtobasestr| (jsdtoa.c)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Kenneth Herron]

This is coverity ID 68. Please see the sample URL. The allocation at line 2928 is leaked if the call to |d2b| or |lshift| return null.

Comment 1

[Brendan Eich [:brendan]]

Check NSPR's prdtoa.c.  The JS version forked long ago from David M. Gay's netlib hosted dtoa.c, and I think NSPR refreshed more recently.

/be

Comment 2

[Wan-Teh Chang]

NSPR's prdtoa.c doesn't have the equivalent of the
JS_dtobasestr function.

Comment 3

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v1

Patch v1.

Simply use |free(buffer)| before the function returns.

Comment 4

[Brian Crowder]

Same leak around line 3007-ish?

Comment 5

[Ryan Jones-Ward [:sciguyryan]]

(In reply to comment #4)
> Same leak around line 3007-ish?
> 

Certainly looks like it. |buffer| isn't freed before then so I guess your right. I'll update the patch! Thanks for pointing that out :)

Comment 6

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v1.1

Updated to add a free for the same leak spotted by Brian Crowder.

Comment 7

[Igor Bukanov]

Comment on attachment 242222 [details] [diff] [review]
Patch v1.1

This is not a pacth against  the trunk. Please update it  to reflect bug 357392.

Comment 8

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v2.0

Patch v2.0

* Updated to latest trunk build.

Comment 9

[Igor Bukanov]

Comment on attachment 251760 [details] [diff] [review]
Patch v2.0

I will review=+ for the patch with changed order of free and RELEASE_DTOA_LOCK calls to follow LIFO pattern.

Comment 10

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v2.1

Patch v2.1

* Same as patch v2.0 but updated to follow the LIFO pattern as suggested by Igor Bukanov.

Comment 11

[Igor Bukanov]

I committed the patch from comment 10 to the trunk:

Checking in jsdtoa.c;
/cvsroot/mozilla/js/src/jsdtoa.c,v  <--  jsdtoa.c
new revision: 3.39; previous revision: 3.38
done

Please mark the bug fixed after verifying that the committed code is the right one. 

Comment 12

[Jay Patel [:jay]]

Comment on attachment 251764 [details] [diff] [review]
Patch v2.1

Approved for both branches, a=jay for drivers.

Comment 13

[Igor Bukanov]

I committed the patch from comment 10 to MOZILLA_1_8_0_BRANCH:

Checking in jsdtoa.c;
/cvsroot/mozilla/js/src/jsdtoa.c,v  <--  jsdtoa.c
new revision: 3.33.10.3; previous revision: 3.33.10.2
done

Comment 14

[Igor Bukanov]

I also committed yesterday the patch from comment 10 to MOZILLA_1_8_BRANCH but forgot to update the bug. The committed version was 3.33.2.4.