Closed Bug 471540 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: JavaScript Engine, defect, P1, critical)

x86
macOS
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: jorendorff)

References

()

Details

(Keywords: assertion, testcase, verified1.9.1, Whiteboard: [fixed-in-tracemonkey])

Attachments

(1 file)

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
I'm working on a testcase.  Seems to involve Google Maps again :/
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?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9.1?
Not the fault of Google Maps code.  This was in LATimes-specific code (it had variable names like "homicides").
Taking.  Thanks once again for the minimized test case, Jesse.  Reproduces in the shell (but only with -j).
Status: NEW → ASSIGNED
Summary: "Assertion failure: cp >= buf" at homicideReport.php → TM: "Assertion failure: cp >= buf" at homicideReport.php
Assignee: general → jorendorff
Comment #2 is right.  It's just a buffer overflow.

(-Number.MIN_VALUE).toString(2).length == 1077.  Can anyone top that?
Attached patch v1Splinter Review
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).
Attachment #354983 - Flags: review?(gal)
Attachment #354983 - Flags: review?(gal) → review+
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.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Whiteboard: fixed-in-tracemonkey
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.
http://hg.mozilla.org/mozilla-central/rev/08915fd1c442
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Flags: in-litmus-
Whiteboard: fixed-in-tracemonkey → [fixed-in-tracemonkey][needs 1.9.1 landing]
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/33fc07530e05
Keywords: fixed1.9.1
Whiteboard: [fixed-in-tracemonkey][needs 1.9.1 landing] → [fixed-in-tracemonkey]
v 1.9.1, 1.9.2.
You need to log in before you can comment on or make changes to this bug.