1300380 - OSX SpiderMonkey builds on try fails

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=99ecf78c4e20

> + source /builds/slave/try_m64_sm-plain-0000000000000/src/js/src/devtools/automation/macbuildenv.sh
> ++ topsrcdir=
> ++ '[' -n 1 ']'
> ++ TT_SERVER=https://api.pub.build.mozilla.org/tooltool/
> ++ cd /..
> ++ ./scripts/scripts/tooltool/tooltool_wrapper.sh /browser/config/tooltool-manifests/macosx64/releng.manifest https://api.pub.build.mozilla.org/tooltool/ setup.sh /builds/tooltool.py
> /builds/slave/try_m64_sm-plain-0000000000000/src/js/src/devtools/automation/macbuildenv.sh: line 20: ./scripts/scripts/tooltool/tooltool_wrapper.sh: No such file or directory
> ++ . /build/macosx/mozconfig.common
> /builds/slave/try_m64_sm-plain-0000000000000/src/js/src/devtools/automation/macbuildenv.sh: line 27: /build/macosx/mozconfig.common: No such file or directory
> Traceback (most recent call last):
>   File "/builds/slave/try_m64_sm-plain-0000000000000/src/js/src/devtools/automation/autospider.py", line 183, in <module>
>     ['CC', 'CXX'])
>   File "/builds/slave/try_m64_sm-plain-0000000000000/src/js/src/devtools/automation/autospider.py", line 79, in set_vars_from_script
>     stdout = subprocess.check_output(['sh', '-x', '-c', script_text])
>   File "/tools/python/lib/python2.7/subprocess.py", line 544, in check_output
>     raise CalledProcessError(retcode, cmd, output=output)
> subprocess.CalledProcessError: Command '['sh', '-x', '-c', 'source /builds/slave/try_m64_sm-plain-0000000000000/src/js/src/devtools/automation/macbuildenv.sh; echo VAR SETTINGS:; echo $CC; echo $CXX']' returned non-zero exit status 1
> program finished with exit code 1

SOURCE env variable is empty, and looks like build tools are downloaded before.

> + /builds/slave/try_m64_sm-plain-0000000000000/scripts/scripts/tooltool/tooltool_wrapper.sh src/browser/config/tooltool-manifests/macosx64/releng.manifest https://api.pub.build.mozilla.org/tooltool/ setup.sh /builds/tooltool.py

We could remove tooltool_wrapper.sh part, and we need to set SOURCE env

Comment 1

[Tooru Fujisawa [:arai]]

here's try run with above changes
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec05b92ab76d
https://treeherder.mozilla.org/#/jobs?repo=try&revision=10e5ee4edc84

on opt build, 3 tests in SM(cgc) fails with timeout.
on debug build, SM(arm) fails with unused private field.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: bug1300380-macos-test-timeouts

Compacting GC zeal makes everything slower so some timeouts are expected.  Here's a patch to add the failing tests to the lists of expected timeouts.

Comment 3

[Tooru Fujisawa [:arai]]

Comment on attachment 8788178 [details] [diff] [review]
bug1300380-macos-test-timeouts

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

thanks! :)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30689f91602f
Add some more tests to the lists of expected timeouts when compacting GC zeal is enabled r=arai DONTBUILD

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/30689f91602f

Comment 6

[Tooru Fujisawa [:arai]]

current status (SM(p) is already solved tho):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4621f2ca892b&selectedJob=27267730
now wasm tests fail, I guess because of not-yet-implemented.
  js/src/jit-test/tests/wasm/conversion.js
  js/src/jit-test/tests/wasm/spec/traps.wast.js

bbouvier, can I have your opinion how we can run or disable those tests on osx arm simulator?

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

They should be implemented if they're enabled. I'll take a look today.

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

arai, not sure to understand actually: is this that we want to build the ARM simulator under macosx? If so, what is the rationale behind this? (we already have test coverage of the ARM simulator under linux, not sure how using another host OS would improve the test coverage here?)
Is it that these tests only fail under the ARM simulator build under macosx?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #8)
> is this that we want to build the ARM
> simulator under macosx? If so, what is the rationale behind this? (we
> already have test coverage of the ARM simulator under linux, not sure how
> using another host OS would improve the test coverage here?)

Some of us use the ARM simulator on OS X, it would be nice to make sure that keeps working (this bug is an example of mac-only bustage?). Also, all of our other ARM builds use GCC AFAIK, and it seems useful to compile the ARM code with Clang as well.

Comment 10

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Jan de Mooij [:jandem] from comment #9)
> Some of us use the ARM simulator on OS X, it would be nice to make sure that
> keeps working (this bug is an example of mac-only bustage?). Also, all of
> our other ARM builds use GCC AFAIK, and it seems useful to compile the ARM
> code with Clang as well.

Makes sense. All the enabled tests should pass, so there might be some issues. Unfortunately, I don't have access to an OS X machine ready for firefox development right now (could investigate later this week or next one, though).

Comment 11

[Benjamin Bouvier [:bbouvier] (inactive)]

Any chance the fact that the ARM simulator is now built with SSE2 flags fixed the wasm issues?

Comment 12

[Tooru Fujisawa [:arai]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #11)
> Any chance the fact that the ARM simulator is now built with SSE2 flags
> fixed the wasm issues?

here's try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fef4358c352

Comment 13

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks. I've spotted and fixed one issue, but the uncanonicalized-nan patch introduced a few others.

I feel like https://giphy.com/gifs/qbXbju1ZsGwqA/html5

Comment 14

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: 1.undefined-behavior.patch

Converting a NaN to a uint64_t/uint32_t is undefined behavior; let's make it more explicit in the WasmTruncate functions.

Comment 15

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: 2.build-with-sse.patch

This builds the ARM simulator with sse2 under macosx (as we do under Linux).

Comment 16

[Benjamin Bouvier [:bbouvier] (inactive)]

With these two patches, all the tests under wasm/ now pass on the MacOS X ARM simulator build.

Comment 17

[Hannes Verschore [:h4writer]]

Comment on attachment 8793827 [details] [diff] [review]
1.undefined-behavior.patch

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

Good catch

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Suppress warning for unused private on osx arm build.

thanks :)
here's remaining patches.

OutOfLineTruncateF32OrF64ToI32.isUnsigned is not used on arm.
it hits warning, and it becomes error.
added |(void)isUnsigned;| to suppress it.

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Move deep clone test into separated file and mark it slow in cgc.

js1_8_5/extensions/clone-object.js hits timeout, because deep object clone takes too long.
that part needs to be marked as slow in cgc-jstests-slow.txt.
moved that part to clone-object-deep.js, and added it to cgc-jstests-slow.txt.

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Fix autospider on osx.

macbuildenv.sh needs SOURCE env variable, so added it.
also, tooltool things are already available before that point, so removed redundant setup code.

Comment 21

[Tooru Fujisawa [:arai]]

Comment on attachment 8793827 [details] [diff] [review]
1.undefined-behavior.patch

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

::: js/src/asmjs/WasmTypes.cpp
@@ +207,5 @@
>  TruncateDoubleToInt64(double input)
>  {
> +    // Note: INT64_MAX is not representable in double. It is actually
> +    // INT64_MAX + 1.  Therefore also sending the failure value.
> +    if (input >= double(INT64_MAX) || input < double(INT64_MIN) || IsNaN(input))

this needs

  #include "mozilla/FloatingPoint.h"
  using mozilla::IsNaN;

on non-unified build.

Comment 22

[Terrence Cole [:terrence]]

Comment on attachment 8794386 [details] [diff] [review]
Fix autospider on osx.

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

Neat!

Comment 23

[Emanuel Hoogeveen [:ehoogeveen]]

Thanks for working on this - I don't have a Mac to test with locally, and OSX browser builds are completely useless for diagnosing crashes that happen during the startup cache precompilation step (which fail the build without any meaningful output). Having access to a shell build on Try is a godsend!

Comment 24

[Emanuel Hoogeveen [:ehoogeveen]]

Well, I take back the 'completely useless' part - it seems the problem *is* in the log, it's just really hard to spot unless you know what you're looking for. Still, just being able to get a shell build should be way more efficient (I'm not sure how to request *just* the shell build though).

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20cf91d1aecd
Fix undefined behavior under WasmTruncate functions; r=h4writer
https://hg.mozilla.org/integration/mozilla-inbound/rev/a05a84121404
Build the ARM simulator under macosx with sse2 too; r=jandem

Comment 26

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Tooru Fujisawa [:arai] from comment #21)
> Comment on attachment 8793827 [details] [diff] [review]
> 1.undefined-behavior.patch
> 
> Review of attachment 8793827 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/asmjs/WasmTypes.cpp
> @@ +207,5 @@
> >  TruncateDoubleToInt64(double input)
> >  {
> > +    // Note: INT64_MAX is not representable in double. It is actually
> > +    // INT64_MAX + 1.  Therefore also sending the failure value.
> > +    if (input >= double(INT64_MAX) || input < double(INT64_MIN) || IsNaN(input))
> 
> this needs
> 
>   #include "mozilla/FloatingPoint.h"
>   using mozilla::IsNaN;
> 
> on non-unified build.

Good catch, cheers!

Comment 27

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8c06ff713115293cdd3166be5ad0481a7957a9e
Bug 1300380 - Add f64_cmp.wast.js and float_exprs.wast.js wasm jit-tests to cgc-jittest-timeouts.txt. r=bbouvier

https://hg.mozilla.org/integration/mozilla-inbound/rev/869626f3bac2ce6eec3e4ca657bbd3c056199d12
Bug 1300380 - Suppress warning for unused private on osx arm build. r=bbouvier

https://hg.mozilla.org/integration/mozilla-inbound/rev/aeab17249ad8c722544490d1afb54bb0630e24ba
Bug 1300380 - Move deep clone test into separated file and mark it slow in cgc. r=terrence

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a5b15755b6e4c61b584c0dc5949481713f297ef
Bug 1300380 - Fix autospider on osx. r=terrence

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/20cf91d1aecd
https://hg.mozilla.org/mozilla-central/rev/a05a84121404
https://hg.mozilla.org/mozilla-central/rev/a8c06ff71311
https://hg.mozilla.org/mozilla-central/rev/869626f3bac2
https://hg.mozilla.org/mozilla-central/rev/aeab17249ad8
https://hg.mozilla.org/mozilla-central/rev/2a5b15755b6e

Comment 29

[Gerry Chang [:gchang]]

Mark 51 as fix-optional. If it's worth uplifting to 51, feel free to nominate it.