89474 - JS_ValueToString(cx, DOUBLE_TO_JSVAL(0.123)) cores [WAS: JS Shell it.item() cores]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Darren DeRidder]

From Bugzilla Helper:
User-Agent: Mozilla/4.77 [en] (WinNT; U)
BuildID:    20010703

The standalone JS Shell interpreter cores when you type 
> it.item()

It appears that line 1576 of js.c (the call to JS_SetCallReturnValue2) is where 
things start to go really wrong.

I checked the code out via cvs on July 3rd and compiled with Sun cc.

Reproducible: Always
Steps to Reproduce:
1. Start the js shell
2. type it.item()
3. core

Comment 1

[Phil Schwartau]

Using JS shell pulled today, 2001-07-05. I do not crash by typing 
 
                           it.item(); 

however, I DO crash by typing

                           print(it.item());


WinNT stack trace:

js_ValueToString(JSContext * 0x00301df0, long -623191334) line 2385 + 6 bytes
js_ValueToStringAtom(JSContext * 0x00301df0, long -623191334) line 679 + 13 
bytes
js_Interpret(JSContext * 0x00301df0, long * 0x0012fed8) line 2726 + 159 bytes
js_Execute(JSContext * 0x00301df0, JSObject * 0x002fb340, JSScript * 0x0030a810, 
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012fed8) line 986 + 13 
bytes
JS_ExecuteScript(JSContext * 0x00301df0, JSObject * 0x002fb340, JSScript * 
0x0030a810, long * 0x0012fed8) line 3169 + 25 bytes
Process(JSContext * 0x00301df0, JSObject * 0x002fb340, char * 0x00000000) line 
371 + 22 bytes
ProcessArgs(JSContext * 0x00301df0, JSObject * 0x002fb340, char * * 0x00300074, 
int 0) line 529 + 17 bytes
main(int 0, char * * 0x00300074) line 2097 + 21 bytes
JS! mainCRTStartup + 227 bytes
KERNEL32! 77f1ba06()


FUNCTION AT CRASHPOINT: 

JSString *
js_ValueToString(JSContext *cx, jsval v)
{
    JSObject *obj;
    JSString *str;

    if (JSVAL_IS_OBJECT(v)) {
        obj = JSVAL_TO_OBJECT(v);
        if (!obj)
            return ATOM_TO_STRING(cx->runtime->atomState.nullAtom);
        if (!OBJ_DEFAULT_VALUE(cx, obj, JSTYPE_STRING, &v))
            return NULL;
    }
    if (JSVAL_IS_STRING(v)) {
        str = JSVAL_TO_STRING(v);
    } else if (JSVAL_IS_INT(v)) {
        str = js_NumberToString(cx, JSVAL_TO_INT(v));
    } else if (JSVAL_IS_DOUBLE(v)) {
        str = js_NumberToString(cx, *JSVAL_TO_DOUBLE(v)); <<<<<<<< CRASHED HERE
    } else if (JSVAL_IS_BOOLEAN(v)) {
        str = js_BooleanToString(cx, JSVAL_TO_BOOLEAN(v));
    } else {
        str = ATOM_TO_STRING(cx->runtime->atomState.typeAtoms[JSTYPE_VOID]);
    }
    return str;
}

Comment 2

[Darren DeRidder]

Some additional characterizations... the behaviour is somewhat inconsistent:

"it.item()" will not seg core if it is the first operation being performed on 
"it".  The call to jsstr.c:js_valueToString in this case evaluates parameter "v" 
as an object. If any other operation has been performed on "it" since starting 
the js shell, calling it.item() will cause a seg core. The call to 
jsstr.c:js_valueToString in this case evaluates parameter "v" as a double. Then 
the call JSVAL_TO_DOUBLE fails. 

My stack trace is quite similar.

Comment 3

[Phil Schwartau]

Darren is right; all you have to do is type it.item() twice to crash.
The first time you don't crash, but the second time you do. 

I can't find "it" as a reserved word in ECMA-262. But look at this
output from the JS shell: 

js> it
[object It]

js> typeof it
object

js> Object.prototype.toString.apply(it)
[object It]

js> Object.prototype.toString.apply(it.prototype)
[object global]

js> for (prop in it){print(prop)}
color
height
width
funny
array


js> typeof it.color
undefined

js> it.array.length
13: TypeError: it.array has no properties


js> var obj = new Object()

js> for (prop in obj){print(prop)}

js> obj.__proto__ = it
[object It]

js> for (prop in obj){print(prop)}
color
height
width
funny
array

Comment 4

[Phil Schwartau]

Seems to be a problem only with the JS shell. Trying this HTML

<SCRIPT>
it.item();
it.item();
it.item();
it.item();
</SCRIPT>


simply produces this error in the JavaScript Console:

   Error: it is not defined
   Line: 2

and there is no crash - 

Comment 5

[Phil Schwartau]

Testcase added to JS test suite - 

             js/tests/js1_5/Regress/regress-89474.js

Comment 6

[Phil Schwartau]

Reassigning to Kenton - 

Comment 7

[Darren DeRidder]

It makes sense that this is only a problem in the JS Shell, because the object 
known as "it" is only defined within the js shell (js.c).  But this bug is 
coming back to haunt me, and I think the issue may be more severe than a mere 
quirk in the JS Shell.  It appears that JS_ValueToString will always core if 
used with DOUBLE_TO_JSVAL(x) where x is a double with the whole number part 
equal to zero.

i.e.
// THIS CORES!
JS_ValueToString(cx, DOUBLE_TO_JSVAL(0.123));

Perhaps the title / severity of this bug should be modified pending verification 
of this behaviour... any thoughts?

Comment 8

[Phil Schwartau]

Updating summary; cc'ing Brendan - 

Comment 9

[Brendan Eich [:brendan]]

This bug is invalid.  DOUBLE_TO_JSVAL takes a jsdouble *, not a jsdouble.  It
casts (as it must) the parameter to (jsval), which chops 32 bits out of the
double you're passing literally.  Don't pass doubles, literal or otherwise, to
DOUBLE_TO_JSVAL.

/be

Comment 10

[Brendan Eich [:brendan]]

Sorry, I closed this prematurely.  Passing a double to DOUBLE_TO_JSVAL is
invalid, but as Darren kindly pointed out to me in email, the js shell crash in
the original comment (upon the *second* it.item() call in the shell) still needs
to be fixed.  Patch coming up.

/be

Comment 11

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 12

[Brendan Eich [:brendan]]

So the bug reported in the first comment (but not cited in the summary) was that
js.c:its_item used argv[0] without checking that argc was != 0.  That's a UMR,
in purify parlance.  The fix is trivial, and I'm going to check it in forthwith
(js.c is not part of Mozilla builds).

/be

Comment 13

[Brendan Eich [:brendan]]

Fix is in.

/be

Comment 14

[Phil Schwartau]

Verified Fixed in JS shell built 2001-07-14 on WinNT, Linux, and Mac -
js/tests/js1_5/Regress/regress-89474.js passes on all three platforms.