OSX SpiderMonkey builds on try fails

RESOLVED FIXED in Firefox 52

Status

()

Core
JavaScript Engine
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: arai, Unassigned)

Tracking

Trunk
mozilla52
Points:
---

Firefox Tracking Flags

(firefox51 fix-optional, firefox52 fixed)

Details

Attachments

(6 attachments)

(Reporter)

Description

a year ago
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
(Reporter)

Comment 1

a year ago
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.
Created attachment 8788178 [details] [diff] [review]
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.
Attachment #8788178 - Flags: review?(arai.unmht)
(Reporter)

Comment 3

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

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

thanks! :)
Attachment #8788178 - Flags: review?(arai.unmht) → review+
Keywords: leave-open

Comment 4

a year ago
Pushed by jcoppeard@mozilla.com:
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/30689f91602f
(Reporter)

Comment 6

a year ago
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?
Flags: needinfo?(bbouvier)
They should be implemented if they're enabled. I'll take a look today.
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?
Flags: needinfo?(bbouvier) → needinfo?(arai.unmht)
(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.
(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).
Flags: needinfo?(arai.unmht)
Any chance the fact that the ARM simulator is now built with SSE2 flags fixed the wasm issues?
(Reporter)

Comment 12

a year ago
(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
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
Created attachment 8793827 [details] [diff] [review]
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.
Attachment #8793827 - Flags: review?(hv1989)
Created attachment 8793829 [details] [diff] [review]
2.build-with-sse.patch

This builds the ARM simulator with sse2 under macosx (as we do under Linux).
Attachment #8793829 - Flags: review?(jdemooij)
With these two patches, all the tests under wasm/ now pass on the MacOS X ARM simulator build.
Comment on attachment 8793827 [details] [diff] [review]
1.undefined-behavior.patch

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

Good catch
Attachment #8793827 - Flags: review?(hv1989) → review+
Attachment #8793829 - Flags: review?(jdemooij) → review+
(Reporter)

Comment 18

a year ago
Created attachment 8794382 [details] [diff] [review]
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.
Attachment #8794382 - Flags: review?(bbouvier)
(Reporter)

Comment 19

a year ago
Created attachment 8794385 [details] [diff] [review]
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.
Attachment #8794385 - Flags: review?(terrence.d.cole)
(Reporter)

Comment 20

a year ago
Created attachment 8794386 [details] [diff] [review]
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.
Attachment #8794386 - Flags: review?(terrence.d.cole)
(Reporter)

Comment 21

a year ago
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.
Attachment #8794385 - Flags: review?(terrence.d.cole) → review+
Comment on attachment 8794386 [details] [diff] [review]
Fix autospider on osx.

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

Neat!
Attachment #8794386 - Flags: review?(terrence.d.cole) → review+
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!
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).
Attachment #8794382 - Flags: review?(bbouvier) → review+

Comment 25

a year ago
Pushed by bbouvier@mozilla.com:
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
(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!
(Reporter)

Comment 27

a year ago
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
(Reporter)

Updated

a year ago
Keywords: leave-open

Comment 28

a year ago
bugherder
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
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Mark 51 as fix-optional. If it's worth uplifting to 51, feel free to nominate it.
status-firefox51: affected → fix-optional
You need to log in before you can comment on or make changes to this bug.