584453 - crash [@ JS_GetFunctionArgumentCount]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Kevin Brosnan]

Install 1.6 alpha of Firebug
Open http://www.mozilla.com
Enable the console and script tabs
Reload the page
Click on new watch expression and paste in WebForm_FireDefaultButton press enter.
Watch Firefox crash.

Comment 1

[Robin Whittleton]

Hitting this. I can trigger by typing the following on to the console in latest Minefield nightly:

a==NaN

See http://crash-stats.mozilla.com/report/index/ba4e3f3a-a702-41e4-a5c5-5c7e62100811 for more info. The Firebug issue I filed is:

http://code.google.com/p/fbug/issues/detail?id=3325

Comment 2

[John J. Barton]

I did not try to reproduce this, but here is a direct link to Firebug 1.6 for your convenience
http://getfirebug.com/releases/firebug/1.6X/firebug-1.6X.0a19.xpi

This is related to code Boris added, and he is out until 22AUG10. I don't know who backs him up.

Comment 3

[Boris Zbarsky [:bzbarsky]]

JS_GetFunctionArgumentCount just returns fun->nargs.  The crash is at 0x30.  nargs is the first member of JSFunction, which inherits from JSObject.  JSObject is 6 words and 3 js::Values.  On a 32-bit system, that's 6*4 + 3*8 = 48 = 0x30 bytes.

So in other words, a null |fun| was passed to JS_GetFunctionArgumentCount.

And indeed, the code in jsdScript::GetParameterNames looks like this after the checkin for bug 549143:

  uintN nargs = JS_GetFunctionArgumentCount(cx, fun);
  if (!fun || !JS_FunctionHasLocalNames(cx, fun) || nargs == 0) { 

So the null-check comes too late.  It used to come before we tried to get nargs...

Luke, want to fix?

Comment 4

[Luke Wagner [:luke]]

Attachment: patch

For shame.  Thanks for the diagnosis.

Comment 5

[John J. Barton]

Regarding my request for blocking, this appears to be a regression from recent commits and the fix only affects users who open jsd and only applications that call the new API boris added, eg only Firebug => the risk is low and crasher is avoided.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 464932 [details] [diff] [review]
patch

r=me

Comment 7

[Luke Wagner [:luke]]

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

Comment 8

[Robert Sayre]

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