1478892 - Instantiate all Parser-related classes for UTF-8 source text

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 2

[Tooru Fujisawa [:arai]]

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

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

\o/

::: js/src/frontend/Parser.cpp
@@ +4159,5 @@
> +Parser<FullParseHandler, Utf8Unit>::asmJS(Node list)
> +{
> +    // Don't support UTF-8 asm.js: given Web Assembly is advancing rapidly,
> +    // it's probably not worth the trouble.
> +    return true;

might be nice to explain what actually happens, with "return true" for "Don't support".
This just ignores the "use asm" directive and the contents are compiled as normal JS, right?

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/185a4564afa5
Instantiate all Parser-related classes for UTF-8 source text.  r=arai

Comment 4

[Stefan Hindli [:stefan_hindli]]

Backed out 11 changesets (Bug 1478892, Bug 1478587) for build bustages in ../../../dist/bin/gdb-tests on a CLOSED TREE

Failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=190761702
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=190761702&repo=mozilla-inbound&lineNumber=32210

[task 2018-07-29T04:32:00.324Z] 04:32:00     INFO -  make[4]: *** [../../../dist/bin/gdb-tests] Error 254
[task 2018-07-29T04:32:00.325Z] 04:32:00     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/js/src/gdb'
[task 2018-07-29T04:32:00.325Z] 04:32:00     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'js/src/gdb/target' failed
[task 2018-07-29T04:32:00.326Z] 04:32:00     INFO -  make[3]: *** [js/src/gdb/target] Error 2
[task 2018-07-29T04:32:00.326Z] 04:32:00     INFO -  make[3]: *** Waiting for unfinished jobs....

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd12c0a357b3
Backed out 11 changesets (bug 1478892, bug 1478587) for build bustages in ../../../dist/bin/gdb-tests on a CLOSED TREE

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a55de03eb91d
Make all parser-related classes instantiable for UTF-8 source text.  r=arai

Comment 7

[Jeff Walden [:Waldo]]

Turns out instantiating the parser classes causes the Android linker to segfault in this one weird case.  I split up the patch here into a change-everything patch and an instantiate-stuff patch, and ^ is the former.  Not sure what the plan is going to be for the latter part just yet; we'll see after the dust clears from the first half landing.

Comment 8

[Jeff Walden [:Waldo]]

Attachment: Instantiate GeneralParser for UTF-8 source text

Per https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffeffcf9e2cf42837e965edcd6adad81ce594e48 (atop the patches in this bug that were just landed), this patch alone causes one particular Android build to go busted when linking: segfault with zero visibility into what went wrong or how.  What do I do here to see this landed?

I can keep working on UTF-8 script tokenizing without this landing, so this is not immediately blocker.  But very soon it will be, as I finish up additional work on UTF-8 script parsing and want to actually start using it and can't just keep working on an increasing stack of local patches.

Comment 9

[Jeff Walden [:Waldo]]

Attachment: Instantiate Parser for UTF-8 source text

This patch also wants to be applied after the previous one, in sequence.  It doesn't make things any worse than the linker segfault the previous patch causes.  Might as well get it out there for full testing of both patches against a fixt toolchain, I guess.

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8996163 [details] [diff] [review]
Instantiate GeneralParser for UTF-8 source text

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

Can you try changing NDK_VERSION to r17c in python/mozboot/mozboot/android.py and see what happens?

Comment 11

[Stefan Hindli [:stefan_hindli]]

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

Comment 12

[Jeff Walden [:Waldo]]

Not much of interest, it looks like:

"""
subprocess.CalledProcessError: Command '['wget', '--continue', 'https://dl.google.com/android/repository/android-ndk-r17c-linux-x86_64.zip']' returned non-zero exit status 8
"""

which happens for the most likely reason:

[jwalden@find-waldo-now after]$ wget -S --method=HEAD https://dl.google.com/android/repository/android-ndk-r17c-linux-x86_64.zip
Spider mode enabled. Check if remote file exists.
--2018-07-31 09:29:46--  https://dl.google.com/android/repository/android-ndk-r17c-linux-x86_64.zip
Resolving dl.google.com (dl.google.com)... 2607:f8b0:4005:805::200e, 216.58.194.206
Connecting to dl.google.com (dl.google.com)|2607:f8b0:4005:805::200e|:443... connected.
HTTP request sent, awaiting response... 
  HTTP/1.1 404 Not Found
  Content-Length: 1449
  Content-Type: text/html; charset=utf-8
  Server: downloads
  X-Content-Type-Options: nosniff
  X-Frame-Options: SAMEORIGIN
  X-Xss-Protection: 1; mode=block
  Date: Tue, 31 Jul 2018 16:29:47 GMT
  Alt-Svc: quic=":443"; ma=2592000; v="44,43,39,35"
Remote file does not exist -- broken link!!!

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

Comment 13

[Nathan Froyd [:froydnj]]

Comment on attachment 8996163 [details] [diff] [review]
Instantiate GeneralParser for UTF-8 source text

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

I'm not sure what sort of feedback to offer here.  It looks like the latest version of the Android NDK is r17b, not r17c as glandium suggested, so maybe change things to use r17b and see what breaks?

Comment 14

[Mike Hommey [:glandium]]

Err yeah, I meant r17b, not r17c.

Comment 15

[Jeff Walden [:Waldo]]

Still not much of interest:

[task 2018-07-31T21:02:16.284Z] 21:02:16     INFO -  checking for a shell... /bin/sh
[task 2018-07-31T21:02:16.312Z] 21:02:16     INFO -  checking for host system type... x86_64-pc-linux-gnu
[task 2018-07-31T21:02:16.319Z] 21:02:16     INFO -  checking for target system type... i386-pc-linux-android
[task 2018-07-31T21:02:16.321Z] 21:02:16     INFO -  checking for android platform directory...
[task 2018-07-31T21:02:16.321Z] 21:02:16     INFO -  ERROR: Android platform directory not found. With the current configuration, it should be in /builds/worker/workspace/build/src/android-ndk/platforms/android-9/arch-x86
[task 2018-07-31T21:02:16.337Z] 21:02:16     INFO -  *** Fix above errors and then restart with\
[task 2018-07-31T21:02:16.337Z] 21:02:16     INFO -                 "/bin/make -f client.mk build"
[task 2018-07-31T21:02:16.337Z] 21:02:16     INFO -  client.mk:123: recipe for target 'configure' failed
[task 2018-07-31T21:02:16.337Z] 21:02:16     INFO -  make: *** [configure] Error 1
[task 2018-07-31T21:02:16.389Z] 21:02:16    ERROR - Return code: 2
[task 2018-07-31T21:02:16.389Z] 21:02:16  WARNING - setting return code to 2
[task 2018-07-31T21:02:16.390Z] 21:02:16    FATAL - 'mach build -v' did not run successfully. Please check log for errors.
[task 2018-07-31T21:02:16.390Z] 21:02:16    FATAL - Running post_fatal callback...
[task 2018-07-31T21:02:16.390Z] 21:02:16    FATAL - Exiting -1
[task 2018-07-31T21:02:16.390Z] 21:02:16     INFO - [mozharness: 2018-07-31 21:02:16.390752Z] Finished build step (failed)
[task 2018-07-31T21:02:16.390Z] 21:02:16     INFO - Running post-run listener: _summarize
[task 2018-07-31T21:02:16.391Z] 21:02:16    ERROR - # TBPL FAILURE #
[task 2018-07-31T21:02:16.391Z] 21:02:16     INFO - [mozharness: 2018-07-31 21:02:16.391215Z] FxDesktopBuild summary:
[task 2018-07-31T21:02:16.391Z] 21:02:16    ERROR - # TBPL FAILURE #

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a8d8d5eff89895c70dac79a5d4f387db1b50286&selectedJob=191237583

Comment 16

[Mike Hommey [:glandium]]

Try adding ac_add_options --with-android-version=14

Comment 17

[Mike Hommey [:glandium]]

You can even try 16 instead of 14.

Comment 18

[Jeff Walden [:Waldo]]

Is https://hg.mozilla.org/try/rev/43dbb335d37f62f0e58564f0bace24c7d09d4bf9 adequate to both change NDK_VERSION and to request Android version 16?

With that change in place, https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0c9518ef5e74093030db7825bd75d60c0807078 isn't attempting an actual build of the thing that was going red before, so it feels like I'm doing something wrong but I don't know what it could be.

Comment 19

[Mike Hommey [:glandium]]

Add ac_add_options --with-android-version=16 to a less common mozconfig. Or change the default value in build/moz.configure/android-ndk.configure.

Comment 20

[Jeff Walden [:Waldo]]

So I tried the latter tack https://hg.mozilla.org/try/rev/72424c32354e4e82f6dd3633dc7e0cdbe3f947c8 and it seems like there's more than that that's necessary for our toolchains or something to support android-version=16:

...
[task 2018-08-09T21:53:42.193Z] 21:53:42     INFO -  checking for target system type... i386-pc-linux-android
[task 2018-08-09T21:53:42.193Z] 21:53:42     INFO -  checking for android platform directory... /builds/worker/workspace/build/src/android-ndk/platforms/android-16/arch-x86
[task 2018-08-09T21:53:42.194Z] 21:53:42     INFO -  checking for android sysroot directory... /builds/worker/workspace/build/src/android-ndk/sysroot
[task 2018-08-09T21:53:42.194Z] 21:53:42     INFO -  checking for android system directory...
[task 2018-08-09T21:53:42.195Z] 21:53:42     INFO -  ERROR: Android system directory not found in [u'/builds/worker/workspace/build/src/android-ndk/platforms/android-16/arch-x86/usr/include', u'/builds/worker/workspace/build/src/android-ndk/sysroot/usr/include/i386-linux-android'].
[task 2018-08-09T21:53:42.211Z] 21:53:42     INFO -  *** Fix above errors and then restart with\
[task 2018-08-09T21:53:42.211Z] 21:53:42     INFO -                 "/bin/make -f client.mk build"
[task 2018-08-09T21:53:42.211Z] 21:53:42     INFO -  client.mk:123: recipe for target 'configure' failed
[task 2018-08-09T21:53:42.211Z] 21:53:42     INFO -  make: *** [configure] Error 1
[task 2018-08-09T21:53:42.234Z] 21:53:42    ERROR - Return code: 2
[task 2018-08-09T21:53:42.234Z] 21:53:42  WARNING - setting return code to 2
[task 2018-08-09T21:53:42.234Z] 21:53:42    FATAL - 'mach build -v' did not run successfully. Please check log for errors.
[task 2018-08-09T21:53:42.234Z] 21:53:42    FATAL - Running post_fatal callback...
[task 2018-08-09T21:53:42.234Z] 21:53:42    FATAL - Exiting -1
[task 2018-08-09T21:53:42.235Z] 21:53:42     INFO - [mozharness: 2018-08-09 21:53:42.234946Z] Finished build step (failed)
[task 2018-08-09T21:53:42.235Z] 21:53:42     INFO - Running post-run listener: _summarize
[task 2018-08-09T21:53:42.235Z] 21:53:42    ERROR - # TBPL FAILURE #
[task 2018-08-09T21:53:42.235Z] 21:53:42     INFO - [mozharness: 2018-08-09 21:53:42.235233Z] FxDesktopBuild summary:
[task 2018-08-09T21:53:42.235Z] 21:53:42    ERROR - # TBPL FAILURE #
[task 2018-08-09T21:53:42.235Z] 21:53:42     INFO - Running post-run listener: copy_logs_to_upload_dir
[task 2018-08-09T21:53:42.235Z] 21:53:42     INFO - Copying logs to upload dir...

Push here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bee95db3e0f77418d081b1dd13eaa619eac29d18&selectedJob=193157042

Is there anything else I should be trying, or has this Simon-says game run its course?  :-\  I still don't know any more about what I'm doing here than when I started, and I'm not sure this game of telephone pictionary is getting much of anything done, and I don't know what we *should* be doing instead here.

Comment 21

[Jeff Walden [:Waldo]]

I also did a push setting to 14 and got similar results, FYI.

Comment 22

[Mike Hommey [:glandium]]

Change --target from i386-linux-android to i686-linux-android. I got a successful try with only that change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb05d4b142c036795f9cea112da97e51a1b6b0e4

Comment 23

[Jeff Walden [:Waldo]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f70be4ce8f63a1e1aa677ff92cb4eab16adb3dae is promising results, finally!  Bumped that to the bottom of the patch-stack, repushed -- will see what happens in the morning:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bf65e9bea6949b85971c045317348594ed1a0e5

Comment 24

[Mike Hommey [:glandium]]

Filed bug 1482330 for the NDK part.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba7c501f82f5
Instantiate GeneralParser for UTF-8 source text.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/f244ce374cd2
Instantiate Parser for UTF-8 source text.  r=arai

Comment 26

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ba7c501f82f5
https://hg.mozilla.org/mozilla-central/rev/f244ce374cd2