emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.63k stars 3.28k forks source link

Invalid assertion in acorn-optimizer #21359

Open JakubGortychHG opened 7 months ago

JakubGortychHG commented 7 months ago

Please include the following in your bug report:

Version of emscripten/emsdk: emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.54 (a95c44ee924d02fa1498f846595485d27c31daa8) I checked 3.1.53 as well.

Failing command line in full: The following exception is thrown when compiling code with -Os

Error: /tmp/emscripten_temp_dyn6cl2g/MagicCasino.release.jso1.js:1613: expected Indentifier but found AssignmentPattern (use EMCC_DEBUG_SAVE=1 to preserve temporary inputs)
    at assertAt (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:30:11)
    at traverse (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:437:13)
    at traverse (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:430:15)
    at Object.VariableDeclarator (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:442:9)
    at c (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:92:20)
    at recursiveWalk (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:94:5)
    at file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:90:38
    at maybeChild (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:54:7)
    at Array.forEach (<anonymous>)
    at visitChildren (file:///mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs:64:15)
em++: error: '/mnt/c/emsdk/node/16.20.0_64bit/bin/node /mnt/c/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs /tmp/emscripten_temp_dyn6cl2g/MagicCasino.release.jso1.js AJSDCE minifyWhitespace -o /tmp/emscripten_temp_dyn6cl2g/MagicCasino.release.jso2.js' failed (returned 1)

https://github.com/emscripten-core/emscripten/blob/047b82506d6b471873300a5e4d1e690420b582d0/tools/acorn-optimizer.js#L437

The issue is happening after updating @sentry/browser npm package from version 6.19.7 to v7.101.0.

To reproduce the issue create the following files:

a.cpp:


int main()
{
    return 0;
}

package.json:


{
    "devDependencies": {
        "@sentry/browser": "7.101.0",
                "webpack": "^5.74.0",
                "webpack-cli": "^4.10.0"
    }
}

webpack.config.js:

path = require('path');

module.exports = env => {
    return {
        entry: path.resolve(__dirname, 'index.js'),
        output: {
            filename: 'all.js',
            path: __dirname
        },
        mode: 'production',
        resolve: {
            modules: [path.resolve(__dirname, 'node_modules')],
            extensions: ['.tsx', '.ts', '.js']
        }
    }
}

index.js:

Sentry = require('@sentry/browser');

Then run the following commands:

npm install
node_modules/.bin/webpack
emcc a.cpp -O2 --pre-js all.js
JakubGortychHG commented 5 months ago

Hi, is sth going on with the ticket? I encountered the same problem using firebaseui web sdk: https://github.com/emscripten-core/emscripten/issues/21594 I checked the newest 3.1.56 release and it din't solve the problem

sbc100 commented 5 months ago

@kripken would you have time to look into this one?

JakubGortychHG commented 5 months ago

I found another example: The compilation crashes with the same exception while using initializeApp function https://firebase.google.com/docs/reference/node/firebase#initializeapp (checked with Firebase npm package version 10.8.1)

kripken commented 5 months ago

In the steps in the first comment, where is all.js? (The problem almost certainly happens because of the content there, that we fail to handle properly.)

sbc100 commented 5 months ago

I imagine that running the webpack command produces the all.js file?

kripken commented 5 months ago

Ah, thanks. Would it be possible for you to attach that content, @JakubGortychHG ? That would save me a lot of time in setting things up here.

JakubGortychHG commented 5 months ago

all.zip Ok, @kripken

kripken commented 5 months ago

Thanks @JakubGortychHG

There is a lot of work required to handle that file, which contains many corner cases, including:

The first error is due to this:

const { beforeBreadcrumb: s = null, maxBreadcrumbs: o = Gt } = x;

And you can replace that null with arbitrary JS, nested inside a VariableDeclarator. The null is a Literal but it could be anything.

I did not look into what the null children are caused by, but in general, this input JS file has lots of JS corner cases. I actually don't think it's feasible for us to handle them all in our custom code: JS is just too large and changing of a language. I suggest instead that we either:

  1. Find a library that does what we need, that is, knows how to read a JS AST and figure out the scoping of each symbol.
  2. Allow JSDCE to "give up" when it encounters unfamiliar code, and not optimize.

As a workaround for now @JakubGortychHG , you can avoid including that code with --pre-js. Using --extern-pre-js would avoid trying to optimize it.