471540 - TM: "Assertion failure: cp >= buf" at homicideReport.php

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

Steps to reproduce:
1. Load http://webapp1.latimes.com/homicide/homicideReport.php
2. Wait one second.

Result:
Assertion failure: cp >= buf, at /Users/jruderman/tracemonkey/js/src/jsnum.cpp:343

Comment 1

[Jesse Ruderman]

I'm working on a testcase.  Seems to involve Google Maps again :/

Comment 2

[Jesse Ruderman]

var s = "";
for (var i = 0; i < 4; i++)
  s = (0x08000000).toString(2);

Assertion failure: cp >= buf, at ../jsnum.cpp:343

Is this a buffer overflow?  Allocating space for DTOSTR_STANDARD_BUFFER_SIZE characters here seems sketchy, since that constant is related to floating-point precision.

Why does the assertion failure go away if I remove "s = " from the third line?

Comment 3

[Jesse Ruderman]

Not the fault of Google Maps code.  This was in LATimes-specific code (it had variable names like "homicides").

Comment 4

[Jason Orendorff [:jorendorff]]

Taking.  Thanks once again for the minimized test case, Jesse.  Reproduces in the shell (but only with -j).

Comment 5

[Jason Orendorff [:jorendorff]]

Comment #2 is right.  It's just a buffer overflow.

(-Number.MIN_VALUE).toString(2).length == 1077.  Can anyone top that?

Comment 6

[Jason Orendorff [:jorendorff]]

Attachment: v1

The patch also changes a couple functions to be static, as nobody calls them outside jsnum.cpp.

The patch also fixes a memory leak (en passant, à la Brendan).

Comment 7

[Andreas Gal :gal]

I am not SM peer (whatever that is), and this is slightly outside my domain, but the patch looks simple enough. The 34 is a bit sudden. Could you calculate that in place? like 1/*sign*/+32/*digits*/+1/*\0*/. I know the comment says it but still.

Comment 8

[Jason Orendorff [:jorendorff]]

Pushed with suggested change:
http://hg.mozilla.org/tracemonkey/rev/08915fd1c442

Dumb mistake in trace-tests.js was fixed here:
http://hg.mozilla.org/tracemonkey/rev/6071ef640803

Note to self:  when using "hg rebase" instead of "hg qrefresh", keep in mind that new changes don't get committed.

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/08915fd1c442

Comment 10

[Bob Clary [:bc] (inactive)]

test updated in http://hg.mozilla.org/mozilla-central/rev/7eb907886ae7 
and  /cvsroot/mozilla/js/tests/js1_8_1/trace/trace-test.js,v  <--  trace-test.js
new revision: 1.9; previous revision: 1.8

verified 1.9.1 tracemonkey and 1.9.2

Comment 11

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/33fc07530e05

Comment 12

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2.