Different output with testcase with and without -n

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gkw, Assigned: dmandelin)

Tracking

(Blocks: 3 bugs, {testcase})

Trunk
mozilla11
x86
Mac OS X
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: js-triage-needed)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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
(Reporter)

Updated

6 years ago
Blocks: 465479
No longer blocks: 349611
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.
(Reporter)

Updated

6 years ago
Summary: TI: Different output with testcase with and without -n → TM: Different output with testcase with and without -n
(Reporter)

Updated

6 years ago
Whiteboard: js-triage-needed
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?
Summary: TM: Different output with testcase with and without -n → Different output with testcase with and without -n
(Assignee)

Comment 3

6 years ago
Created attachment 578785 [details] [diff] [review]
Patch
Assignee: general → dmandelin
Status: NEW → ASSIGNED
Attachment #578785 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 4

6 years ago
Comment on attachment 578785 [details] [diff] [review]
Patch

Flipping review because Jeff is traveling.
Attachment #578785 - Flags: review?(jwalden+bmo) → review?(luke)

Comment 5

6 years ago
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.
Attachment #578785 - Flags: review?(luke) → review+
(Assignee)

Comment 6

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

with nit fixed.
https://hg.mozilla.org/mozilla-central/rev/b6ceca62f1a7
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
(Reporter)

Updated

6 years ago
Blocks: 349611
You need to log in before you can comment on or make changes to this bug.