possible problem with non-wasm minimization in 1.39.3

[Eric Mandel]

I upgraded from 1.38.37 to 1.39.3 and was very pleased to see the llvm back-end report two "multiply defined" errors in the link phase. Thanks very much! Unfortunately, we also still need to support a non-wasm version because of problems integrating into Jupyter notebooks. But when I compile with -O3, I get a runtime error in the browser (not involving Jupyter):

Uncaught TypeError: Ba is not a function

    at eC (astroem.js:64)

    at iC (astroem.js:64)

    at Wn (astroem.js:60)

    at Gf (astroem.js:40)

    at Wg (astroem.js:40)

    at Fc (astroem.js:12)

    at dk (astroem.js:20)

    at Module._openFITSFile (astroem.js:86)

    at ccall (astroem.js:86)

    at FileReader.fileReader.onload (astroem.js:86)

This error was not present in 1.38.37 (wasm and non-wasm), and it is not present when building the 1.39.3 wasm version. Surprisingly, it does not happen if I use -O2 (or -O3 -g) in the last phase of the build, which is the current work-around.

I'd appreciate help in understanding how to identify and possibly fix this. The missing routine is far too deep in the call stack to be something I would think to add to the EXPORTED_FUNCTIONS array. I didn't quite see how to use the output of the --emit-symbol-map, but I could see a number of differences in the list of routines between -O2 and -O3.

[Alon Zakai]

I suspect that is not a missing export, but could be a more serious bug in the compiler (a missing export would likely show up as "Module.X is not a function").

1.39.0 changed the default backend, and in particular the non-wasm output is now emitted by the "wasm2js" tool instead of the old asm.js backend. So I suspect this might be a wasm2js bug.

I'd suggest building with --profiling. Does the problem still happen there? If so, the function name may be more helpful.

Otherwise, bisection on versions may be useful, to verify if 1.39.0 is where this broke (which is where the default backend switched). Another option is to change the backends on the current version - install 1.39.3-fastcomp vs 1.39.3 (the default is 1.39.3-upstream, the new backend).

Regardless, if this is a wasm2js bug (as I suspect it is), if you can provide the input files for the link stage, plus the link command, so I can reproduce it locally, that would be great!

- Alon

--
You received this message because you are subscribed to the Google Groups "emscripten-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to emscripten-disc...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/emscripten-discuss/2ded49dc-940e-46a6-9bee-8091befdda43%40googlegroups.com.

[Eric Mandel]

I built with --profiling turned on and the code runs successfully ...

Tomorrow I can try to determine the version in which the problem arose, but more to the point, I made a tar file containing the input astroem.bc file, some auxiliary pre/post files, and a script with the command line to generate the output astroem.js file (also included):

https://js9.si.edu/downloads/astroembug.tar.gz

If you download the tar file and run the mkastroemjs script, it will generate an exact duplicate of the astroem.js file that fails.

If you want these files in some other way, please let me know. I wasn't sure whether a 6Mb file would attach to this discussion site ...

Eric

[Eric Mandel]

BTW, the routine that is missing with the current setup is now Aa:

Uncaught TypeError: Aa is not a function

    at _B (astroem.js:68)

    at cC (astroem.js:68)

    at Un (astroem.js:60)

    at Hf (astroem.js:28)

    at Vg (astroem.js:28)

    at Ec (astroem.js:16)

    at bk (astroem.js:20)

[Eric Mandel]

I (laboriously) added print statements to find the library routine that eventually fails and it looks like the non-asm Emscripten might indeed be trying to call a function where there is no function. Here is a snippet of the code that is being run, indicating where print statements are located:

fprintf(stdout, "2\n"); fflush(stdout);

    if (*status == NOT_IMAGE)

        *status = tstatus;    /* ignore 'unknown extension type' error */

    else if (*status > 0)

        return(*status);

fprintf(stdout, "3\n"); fflush(stdout);

    /*

       the logical end of the header is 80 bytes before the current position, 

       minus any trailing blank keywords just before the END keyword.

    */

    (fptr->Fptr)->headend = (fptr->Fptr)->nextkey - (80 * (nspace + 1));

fprintf(stdout, "4 [%d %d]\n", (int)(fptr->Fptr)->headend, (int)(fptr->Fptr)->nextkey); fflush(stdout);

    /* the data unit begins at the beginning of the next logical block */

    (fptr->Fptr)->datastart = (((fptr->Fptr)->nextkey - 80) / 2880 + 1)

                              * 2880;

fprintf(stdout, "5\n"); fflush(stdout);

and here is the output in the Chrome debugger for the non-wasm case:

The values of 400 and 480 displayed in print statement #4 are correct for this astronomical image, so things look right at that point. But the next line of code should simply set datastart to the first 2880 byte block after the image header, and it looks like it might be calling a non-existent routine instead. So we never reach print statement #5.

Note that headend, nextkey, and datastart are all declared as long long, if that is any hint.

For comparison, here is the Chrome debugger output from the wasm build:

Let me know how I can help.

[Alon Zakai]

Thanks! I think I see what's wrong. This PR should fix it, please test if you can,

https://github.com/emscripten-core/emscripten/pull/9880

- Alon

--

You received this message because you are subscribed to the Google Groups "emscripten-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to emscripten-disc...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/emscripten-discuss/39322779-c9c5-4fbc-9231-203200ffe086%40googlegroups.com.

[Eric Mandel]

Sure, is it sufficient to replace the 1.39.3 version of upstream/emscripten/tools/shared.py with the modified version? It doesn't look like the new shared.py code has been merged into incoming as yet.

And if it works, would you recommend using this modified shared.py and compiling with -O3 (until my next version update), or would you recommend using the pristine 1.39.3 with -O2?

[Alon Zakai]

I landed the PR now, so you can just get latest incoming for testing (I'm pretty sure it's the right fix, but confirmation would be good!). Or, in a few hours you can emsdk install tot-upstream (tip of tree build).

Using that with -O3 will be better for code size than -O2 with an older build.

On Wed, Nov 20, 2019 at 4:05 PM Eric Mandel <ema...@cfa.harvard.edu> wrote:

Sure, is it sufficient to replace the 1.39.3 version of upstream/emscripten/tools/shared.py with the modified version? It doesn't look like the new shared.py code has been merged into incoming as yet.

And if it works, would you recommend using this modified shared.py and compiling with -O3 (until my next version update), or would you recommend using the pristine 1.39.3 with -O2?

--
You received this message because you are subscribed to the Google Groups "emscripten-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to emscripten-disc...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/emscripten-discuss/fbe63c06-6939-4cbd-8977-f72784517113%40googlegroups.com.

[Eric Mandel]

I installed tot-upstream and can confirm that the non-wasm version now works as expected. Thanks very much!