514884 - TM: unit string and int string for '0', '1'...'9' must be the same JSString

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gregor Wagner]

2 copies break identity of atoms.

Comment 1

[Gregor Wagner]

Attachment: version1

js_NumberToString takes now the JSString from the unitString array for 0-9

Comment 2

[Gregor Wagner]

Attachment: NumberToStringWithBase shouldn't be called from a trace

Comment 3

[Andreas Gal :gal]

Maintaining the identity of atoms is not optional.

Comment 4

[Andreas Gal :gal]

Comment on attachment 398890 [details] [diff] [review]
NumberToStringWithBase shouldn't be called from a trace

>diff -r 45772700955a js/src/jsnum.cpp
>--- a/js/src/jsnum.cpp	Sat Sep 05 19:59:11 2009 +0400
>+++ b/js/src/jsnum.cpp	Sat Sep 05 17:20:54 2009 -0700
>@@ -593,18 +593,17 @@ num_toPrecision(JSContext *cx, uintN arg
>     if (argc == 0 || JSVAL_IS_VOID(vp[2]))
>         return num_toString(cx, 0, vp);
>     return num_to(cx, DTOSTR_STANDARD, DTOSTR_PRECISION, 1, MAX_PRECISION, 0,
>                   argc, vp);
> }
> 
> #ifdef JS_TRACER
> 
>-JS_DEFINE_TRCINFO_2(num_toString,
>-    (3, (static, STRING, NumberToStringWithBase, CONTEXT, THIS_DOUBLE, INT32, 1, 1)),
>+JS_DEFINE_TRCINFO_1(num_toString,
>     (2, (extern, STRING, js_NumberToString,      CONTEXT, THIS_DOUBLE,        1, 1)))
> 
> #endif /* JS_TRACER */
> 
> static JSFunctionSpec number_methods[] = {
> #if JS_HAS_TOSOURCE
>     JS_FN(js_toSource_str,       num_toSource,          0,JSFUN_THISP_NUMBER),
> #endif
>@@ -828,17 +827,17 @@ NumberToCString(JSContext *cx, jsdouble 
>         if (!numStr) {
>             JS_ReportOutOfMemory(cx);
>             return NULL;
>         }
>     }
>     return numStr;
> }
> 
>-static JSString * JS_FASTCALL
>+static JSString *
> NumberToStringWithBase(JSContext *cx, jsdouble d, jsint base)
> {
>     /*
>      * The longest possible result here that would need to fit in buf is
>      * (-0x80000000).toString(2), which has length 33.  (This can produce
>      * longer results, but in those cases buf is not used; see comment at
>      * NumberToCString.)
>      */
>@@ -858,16 +857,18 @@ NumberToStringWithBase(JSContext *cx, js
> }
> 
> JSString * JS_FASTCALL
> js_NumberToString(JSContext *cx, jsdouble d)
> {
>     jsint i;
> 
>     if (JSDOUBLE_IS_INT(d, i)) {

jsuint u = jsuint(i);

if (u < 10)

if (u < INT_STRING_LIMIT)

>+        if (jsuint(i) < 10) /*  to avoid 2 JSString copies of 0-9 */
>+            return &JSString::unitStringTable['0' + i];
>         if (jsuint(i) < INT_STRING_LIMIT)
>             return &JSString::intStringTable[i];
>     }
>     return NumberToStringWithBase(cx, d, 10);
> }
> 
> JSBool JS_FASTCALL
> js_NumberValueToCharBuffer(JSContext *cx, jsval v, JSCharBuffer &cb)

Comment 5

[Gregor Wagner]

Attachment: fixed jsuint

Comment 6

[Gregor Wagner]

Attachment: changed array index from i to u.

Comment 7

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/438b544f6c65

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/438b544f6c65

Comment 9

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e555739528a6