Closed Bug 832379 Opened 11 years ago Closed 11 years ago

Many JS test failures when building for ARM with -marm and gcc 4.4. gcc 4.4 with -mthumb (our build's default) and gcc 4.6 with -marm and -mthumb work properly.

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: justin.lebar+bug, Unassigned)

References

Details

Attachments

(1 file)

Patch, v1: Error out if building with -marm, --enable-optimize, and GCC <= 4.4
1.91 KB, patch
mjrosenb
: review+
Details | Diff | Splinter Review 
I'm trying to use perf to profile b2g (bug 831611).

As far as I can tell, my only hope to get readable callstacks out of perf is to compile with -marm -mapcs-frame -fno-omit-frame-pointer.

But when I compile with -marm (ac_add_options --with-thumb=no), I get "interesting" JS behavior which I can only explain as incorrect behavior in SM.  For example, one module starts with an integer x, does a few x++ and x*=2, and ends up with 1.something e-7.

Before I (or someone else) even tries to fix this bug, I still need to

 * verify that the problem goes away when I build without -marm
 * figure out how to run the JS tests on my b2g device, so I can report a specific test failure here.
I installed the b2g-python package from [1] onto my device.  I then pushed the js/src/test directory, and pushed a stripped js shell binary.

I then made a /system/bin/python wrapper which contains

> #!/system/bin/sh
> 
> export LD_LIBRARY_PATH=/data/python/lib
> export PYTHONHOME=/data/python
> /data/python/bin/python $@

Now when I run

> python jstests.py -d /system/b2g/js

I get

> Traceback (most recent call last):
>   File "jstests.py", line 10, in <module>
>     from subprocess import list2cmdline, call
>   File "/data/python/lib/python2.7/subprocess.py", line 429, in <module>
>     import select
> ImportError: Cannot load library: reloc_library[1285]:  5557 cannot locate 'PyDict_Next'...

Stefan, do you have any idea what's going on here?
With glandium's help I managed to get python working and run the JS tests.

With my ARM JS shell, I saw a number of regressions before I cancelled the test run:

> REGRESSIONS
>     js1_8/regress/regress-459389.js
>     js1_8/genexps/regress-634472.js
>     js1_7/lexical/regress-336376-01.js
>     js1_6/Array/regress-290592.js
>     js1_5/extensions/getset-003.js
>     js1_5/extensions/getset-004.js
>     js1_5/extensions/getset-005.js
>     js1_3/Script/new-001.js
>     js1_3/regress/new-001.js
>     ecma_5/RegExp/regress-617935.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-1-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-2-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-3-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-4-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-5-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-6-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-7-of-8.js
>     ecma_5/Object/15.2.3.6-dictionary-redefinition-8-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-1-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-2-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-3-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-4-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-5-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-6-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-7-of-8.js
>     ecma_5/Object/15.2.3.6-middle-redefinition-8-of-8.js
>     ecma_5/Object/15.2.3.6-redefinition-1-of-4.js
>     ecma_5/Object/15.2.3.6-redefinition-2-of-4.js
>     ecma_5/Object/15.2.3.6-redefinition-3-of-4.js
>     ecma_5/Object/15.2.3.6-redefinition-4-of-4.js
>     ecma_5/Object/add-property-non-extensible.js
>     ecma_5/misc/regress-bug632003.js
>     ecma_2/Statements/try-006.js
> FAIL (partial run -- interrupted by user)

I'll see what my thumb build looks like now.
> With glandium's help I managed to get python working and run the JS tests. 

For those following along at home, a (hacky) fix is at

https://github.com/st3fan/b2g-python/pull/1
With a thumb build (the default), I get only two test failures:

> REGRESSIONS
>    js1_8/regress/regress-471373.js
>    ecma_5/RegExp/regress-617935.js

So the next step is figuring out why these tests are failing, and fixing the problem.
Terrence suggested that the JIT tests might be less crufty and easier to debug.

-mthumb:

> FAILURES:
>     /data/local/tmp/jit-test/tests/basic/bug623859.js
>     /data/local/tmp/jit-test/tests/basic/bug698584.js
>     /data/local/tmp/jit-test/tests/basic/testBug621202.js

-marm:

> FAILURES:
>     /data/local/tmp/jit-test/tests/basic/arrayProto.js
>     /data/local/tmp/jit-test/tests/basic/bigLoadStoreDisp.js
>     /data/local/tmp/jit-test/tests/basic/bug623859.js
>     /data/local/tmp/jit-test/tests/basic/bug630865-5.js
>     /data/local/tmp/jit-test/tests/basic/bug656261.js
>     /data/local/tmp/jit-test/tests/basic/bug679977.js
>     /data/local/tmp/jit-test/tests/basic/bug698584.js
>     /data/local/tmp/jit-test/tests/basic/new-bound-function.js
>     /data/local/tmp/jit-test/tests/basic/newTest.js
>     /data/local/tmp/jit-test/tests/basic/setprop.js
>     /data/local/tmp/jit-test/tests/basic/testArrayComp1.js
>     /data/local/tmp/jit-test/tests/basic/testArrayComp2.js
>     /data/local/tmp/jit-test/tests/basic/testBug621202.js
>     /data/local/tmp/jit-test/tests/basic/testConstructorArgs-1.js
>     /data/local/tmp/jit-test/tests/basic/testConstructorArgs-2.js
>     /data/local/tmp/jit-test/tests/basic/testConstructorArgs-3.js
>     /data/local/tmp/jit-test/tests/basic/testCustomIterator.js
>     /data/local/tmp/jit-test/tests/basic/testExistingPropToJoinedMethodAttempt-3.js
>     /data/local/tmp/jit-test/tests/basic/testExistingPropToJoinedMethodAttempt.js
>     /data/local/tmp/jit-test/tests/basic/testFloatArrayIndex.js
>     /data/local/tmp/jit-test/tests/basic/testMethodInc.js
>     /data/local/tmp/jit-test/tests/basic/testMoreClosures.js
>     /data/local/tmp/jit-test/tests/basic/testNativeSetter.js
>     /data/local/tmp/jit-test/tests/basic/testPartialFlatClosure.js
>     /data/local/tmp/jit-test/tests/basic/testRebranding2.js
>     /data/local/tmp/jit-test/tests/basic/testincops.js
>     /data/local/tmp/jit-test/tests/debug/Environment-identity-03.js
>     /data/local/tmp/jit-test/tests/debug/Object-apply-03.js
>     /data/local/tmp/jit-test/tests/e4x/bug605200.js
>     /data/local/tmp/jit-test/tests/e4x/e4x-descendants-with-arguments.js
>     /data/local/tmp/jit-test/tests/ion/bug-770309-mcall-bailout.js
>     /data/local/tmp/jit-test/tests/ion/monomorphic-property-access.js
>     /data/local/tmp/jit-test/tests/ion/new-0.js
>     /data/local/tmp/jit-test/tests/ion/new-1.js
>     /data/local/tmp/jit-test/tests/ion/new-10.js
>     /data/local/tmp/jit-test/tests/ion/new-9.js
>     /data/local/tmp/jit-test/tests/jaeger/bug588338.js
>     /data/local/tmp/jit-test/tests/jaeger/bug589108.js
>     /data/local/tmp/jit-test/tests/jaeger/bug732423.js
>     /data/local/tmp/jit-test/tests/jaeger/recompile/bug659766.js
>     /data/local/tmp/jit-test/tests/jaeger/recompile/exotic.js
>     /data/local/tmp/jit-test/tests/jaeger/recompile/incdec.js
>     /data/local/tmp/jit-test/tests/pic/callname-global1.js
>     /data/local/tmp/jit-test/tests/pic/set1.js
>     /data/local/tmp/jit-test/tests/pic/set2.js
>     /data/local/tmp/jit-test/tests/sunspider/check-3d-cube.js
>     /data/local/tmp/jit-test/tests/sunspider/check-access-binary-trees.js
>     /data/local/tmp/jit-test/tests/sunspider/check-access-nbody.js
>     /data/local/tmp/jit-test/tests/v8-v5/check-crypto.js
>     /data/local/tmp/jit-test/tests/v8-v5/check-deltablue.js
>     /data/local/tmp/jit-test/tests/v8-v5/check-earley-boyer.js
>     /data/local/tmp/jit-test/tests/v8-v5/check-raytrace.js
>     /data/local/tmp/jit-test/tests/v8-v5/check-richards.js
>     /data/local/tmp/jit-test/tests/v8-v5/check-splay.js
The following test (based on jit-test/tests/basic/newTest.js) fails with -marm.

> function MyConstructor(i) { this.i = i; }
> 
> var a = [];
> for (var i = 0; i < 2; i++)
>   a[i] = new MyConstructor(i);
> assertEq(a[1].i, 1);

The error is

> newTest.js:6:0 Error: Assertion failed: got -1.7606544133741409e-7, expected 1

The exact floating point value changes every time I run it.
> The exact floating point value changes every time I run it.

but it's always negative, and it always has a small absolute value (less than 1e-3, it seems).
If I disable TI, the test from comment 7 passes, but plenty of other tests fail:

Failures with --no-ti and -marm:

>    /data/local/tmp/jit-test/tests/basic/arrayProto.sm.js
>    /data/local/tmp/jit-test/tests/basic/bug623859.js
>    /data/local/tmp/jit-test/tests/basic/bug656261.js
>    /data/local/tmp/jit-test/tests/basic/bug698584.js
>    /data/local/tmp/jit-test/tests/basic/testArrayComp1.js
>    /data/local/tmp/jit-test/tests/basic/testArrayComp2.js
>    /data/local/tmp/jit-test/tests/basic/testBug621202.js
>    /data/local/tmp/jit-test/tests/basic/testConstructorArgs-1.js
>    /data/local/tmp/jit-test/tests/basic/testConstructorArgs-2.js
>    /data/local/tmp/jit-test/tests/basic/testConstructorArgs-3.js
>    /data/local/tmp/jit-test/tests/basic/testExistingPropToJoinedMethodAttempt-3.js
>    /data/local/tmp/jit-test/tests/basic/testFloatArrayIndex.js
>    /data/local/tmp/jit-test/tests/basic/testMethodInc.js
>    /data/local/tmp/jit-test/tests/basic/testMoreClosures.js
>    /data/local/tmp/jit-test/tests/basic/testRebranding2.js
>    /data/local/tmp/jit-test/tests/debug/Environment-identity-03.js
>    /data/local/tmp/jit-test/tests/debug/Object-apply-03.js
>    /data/local/tmp/jit-test/tests/e4x/bug605200.js
>    /data/local/tmp/jit-test/tests/e4x/e4x-descendants-with-arguments.js
>    /data/local/tmp/jit-test/tests/ion/monomorphic-property-access.js
>    /data/local/tmp/jit-test/tests/jaeger/bug588338.js
>    /data/local/tmp/jit-test/tests/jaeger/bug589108.js
>    /data/local/tmp/jit-test/tests/jaeger/bug732423.js
>    /data/local/tmp/jit-test/tests/jaeger/recompile/exotic.js
>    /data/local/tmp/jit-test/tests/jaeger/recompile/incdec.js
>    /data/local/tmp/jit-test/tests/pic/callname-global1.js
>    /data/local/tmp/jit-test/tests/pic/set1.js
>    /data/local/tmp/jit-test/tests/pic/set2.js
>    /data/local/tmp/jit-test/tests/sunspider/check-access-nbody.js
>    /data/local/tmp/jit-test/tests/v8-v5/check-crypto.js
>    /data/local/tmp/jit-test/tests/v8-v5/check-deltablue.js
>    /data/local/tmp/jit-test/tests/v8-v5/check-earley-boyer.js
>    /data/local/tmp/jit-test/tests/v8-v5/check-raytrace.js
>    /data/local/tmp/jit-test/tests/v8-v5/check-richards.js
>    /data/local/tmp/jit-test/tests/v8-v5/check-splay.js
Hmm.  I did a debug + -marm + -O0 build, and all the test failures went away (modulo a few new timeouts which I presume are there because debug builds are slow).

I wonder if this is a compiler bug.

--enable-debug --disable-optimize --with-thumb=no jit-test results:

FAILURES:
    /data/local/tmp/jit-test/tests/basic/bug623859.js
    /data/local/tmp/jit-test/tests/basic/bug698584.js
    /data/local/tmp/jit-test/tests/basic/testBug621202.js
TIMEOUTS:
    /data/local/tmp/jit-test/tests/basic/bug688939.js
    /data/local/tmp/jit-test/tests/debug/Environment-variables.js
    /data/local/tmp/jit-test/tests/parallelarray/constructor-5.js
Well, mystery solved: When I compile with gcc 4.6 from NDK r8d, the failures go away.

--with-thumb=no --enable-optimize --disable-debug jit-test results, gcc 4.6 (NDK r8d):

> FAILURES:
>    /data/local/tmp/jit-test/tests/basic/bug623859.js
>    /data/local/tmp/jit-test/tests/basic/bug698584.js
>    /data/local/tmp/jit-test/tests/basic/testBug621202.js

Getting this to work was a bit of a chore, so if this is indeed the key to getting perf to work on B2G, we'll probably want to do work to either switch to the new NDK or at least add support to the tree for it.  But there is apparently no JS bug here, and I can open new work for the B2G build system stuff.

Would anyone object if I added a check to the JS configure to bail on -marm + gcc 4.4, since that apparently does not work well?
Summary: Make the ARM jit work when SM is compiled with -marm → Many JS test failures when building for ARM with -marm and gcc 4.4. gcc 4.4 with -mthumb (our build's default) and gcc 4.6 with -marm and -mthumb work properly.
See bug 832752 for my continuing trials and tribulations.
As usual, I'm asking Terrence for a review of this patch's intent and glandium for review of the actual bits.
Attachment #704331 - Flags: review?(terrence)
Attachment #704331 - Flags: review?(mh+mozilla)
Comment on attachment 704331 [details] [diff] [review]
Patch, v1: Error out if building with -marm, --enable-optimize, and GCC <= 4.4

Flipping review over to our ARM expert.
Attachment #704331 - Flags: review?(terrence) → review?(mrosenberg)
Comment on attachment 704331 [details] [diff] [review]
Patch, v1: Error out if building with -marm, --enable-optimize, and GCC <= 4.4

Review of attachment 704331 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/configure.in
@@ +4109,5 @@
> +
> +        case "$target" in
> +        arm*-*)
> +            if test -n "$MOZ_OPTIMIZE" -a -z "$MOZ_THUMB2" ; then
> +                AC_MSG_ERROR([Building for ARM with GCC <= 4.4, --enable-optimize, and --with-thumb=no is known to cause miscompilations; see bug 832379.])

I'm not sure what to think here. There are plenty of ways people can build on arm that will trigger these miscompilations that won't be caught by this test.

Also, are you sure it's not a problem with -mapcs-frame, or something else?
Attachment #704331 - Flags: review?(mh+mozilla)
> I'm not sure what to think here. There are plenty of ways people can build on arm that 
> will trigger these miscompilations that won't be caught by this test.

Indeed.

> Also, are you sure it's not a problem with -mapcs-frame, or something else?

-mapcs-frame has its own problems (bug 832752), but this was just --with-thumb=no.  :-/

I don't feel strongly about this; if you think it's better to leave it as it is, that's fine with me.
Comment on attachment 704331 [details] [diff] [review]
Patch, v1: Error out if building with -marm, --enable-optimize, and GCC <= 4.4

Review of attachment 704331 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/configure.in
@@ +4109,5 @@
> +
> +        case "$target" in
> +        arm*-*)
> +            if test -n "$MOZ_OPTIMIZE" -a -z "$MOZ_THUMB2" ; then
> +                AC_MSG_ERROR([Building for ARM with GCC <= 4.4, --enable-optimize, and --with-thumb=no is known to cause miscompilations; see bug 832379.])

I'm not too sure how this is checking for gcc <= 4.4.  I will say, however, that I have found bugs in older versions of gcc in the past, and am not horribly satisfied, but I usually like seeing the bug with my own eyes before switching compilers, since oftentimes, there is a bit of undefined behavior, and switching compilers merely changes the undefined behavior to something a bit closer to what we expect temporarily.
Attachment #704331 - Flags: review?(mrosenberg) → review+
I think I'm going to close this, based on glandium's comments above.  Glandium, if you change your mind, feel free to reopen the bug.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: