660437 - Different output with testcase with and without -n

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

V = Int32Array([, ])
print(uneval(this))

outputs:

$ ./js-opt-64-jm-darwin 
js> V = Int32Array([, ])
({0:0})
js> print(uneval(this))
({V:{0:0}})
js> 

$ ./js-opt-64-jm-darwin -j
js> V = Int32Array([, ])
({0:0})
js> print(uneval(this))
({V:{0:0}})
js> 

$ ./js-opt-64-jm-darwin -n
js> V = Int32Array([, ])
({0:-2147483648})
js> print(uneval(this))
({V:{0:-2147483648}})
js> 

Tested on 64-bit JM changeset 96fae421af85

Comment 1

[Brian Hackett [Laid off!]]

TM bug.  Here is a testcase which illustrates the problem using a TM shell.

v = Int32Array([,,]);
print(v[0]);

a = [,,];
a.length = 10;
w = Int32Array(a);
print(w[0]);

Prints:

0
-2147483648

Presumably these should be the same.  The problem is typed array construction will behave differently on array holes depending on which of the below paths it takes in copyFrom:

        if (ar->isDenseArray() && ar->getDenseArrayCapacity() >= len) {
            JS_ASSERT(ar->getArrayLength() == len);

            Value *src = ar->getDenseArrayElements();

            for (uintN i = 0; i < len; ++i)
                *dest++ = nativeFromValue(cx, *src++);
        } else {
            // slow path
            Value v;

            for (uintN i = 0; i < len; ++i) {
                if (!ar->getProperty(cx, ::INT_TO_JSID(i), &v))
                    return false;
                *dest++ = nativeFromValue(cx, v);
            }
        }

In the TI branch the capacity check is on the array's initialized length, and it takes the slow path instead of the fast path.  For integer types, nativeFromValue will produce -1 on an undefined value and 0 on a hole value.  I don't know which is right.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

With a current js shell, the results for the comment #0 testcase are:
Interp: ({V:{0:0}})
-j:     ({V:{0:0}})
-m:     ({V:{0:0}})
-m -n:  ({V:{0:-2147483648}})

With a current js shell, the results for the comment #1 testcase are:
Interp: 0 -2147483648
-j:     0 -2147483648
-m:     0 -2147483648
-m -n:  -2147483648 -2147483648

Looks like there's still an issue here?

Comment 3

[David Mandelin [:dmandelin]]

Attachment: Patch

Comment 4

[David Mandelin [:dmandelin]]

Comment on attachment 578785 [details] [diff] [review]
Patch

Flipping review because Jeff is traveling.

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 578785 [details] [diff] [review]
Patch

Mmm conversions

>+        return ArrayTypeIsFloatingPoint()
>+             ? NativeType(js_NaN)
>+             : NativeType(int32(0));

nit: need to align ? with A in ArrayTypeIsFloatingPoint.

Comment 6

[David Mandelin [:dmandelin]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/b6ceca62f1a7

with nit fixed.

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b6ceca62f1a7