Bug in embind's JS optimizer that affects enum registration

105 views
Skip to first unread message

arnab choudhury

unread,
Apr 26, 2016, 12:00:01 PM4/26/16
to emscripten-discuss
Hey guys

On 1.36.2, I've found a bug where Emscripten's JS optimizer seems to be removing a local function within the __embind_register_enum function in embind.js if built with -O3 optimization. This is the piece of code that's being removed:


  _embind_register_enum: function(
    rawType,
    name,
    size,
    isSigned
  ) {
    var shift = getShiftFromSize(size);
    name = readLatin1String(name);
    var bindingsIndex = (typeof(Tableau_bindingsIndex) !== 'undefined') ? Tableau_bindingsIndex : 0;

    if (typeof(Tableau_registerEnum) !== 'undefined') {
        Tableau_registerEnum(Tableau_bindingsIndex, rawType, name);
    }

    function constructor() {
    }
    constructor.values = {};

    registerType(rawType, {
        name: name,
        constructor: constructor,
        'fromWireType': function(c) {
            return this.constructor.values[c];
        },
        'toWireType': function(destructors, c) {
            return c.value;
        },
        'argPackAdvance': 8,
        'readValueFromPointer': enumReadValueFromPointer(name, shift, isSigned),
        destructorFunction: null,
    });
    exposePublicSymbol(name, constructor);
  },



I haven't yet root caused why the constructor function is being removed. But as a result, we are no longer referring to a local function object when registering the enum type, but a global value named constructor. As a result, if you end up registering a second enum, the values for the first enum get overwritten (since we set constructor.values = {}) and this results in undefined behavior later on.

Is this a known issue? I still haven't figured out where the optimizer is removing the constructor function.

Thanks,
Arnab

Alon Zakai

unread,
Apr 26, 2016, 12:04:18 PM4/26/16
to emscripten-discuss
Definitely not a known issue.

The only pass that could affect non-asm.js code like that is JSDCE is tools/js-optimizer. A simple way to test is to add that testcase to the existing test (in tests/test_other.py) and see if it does the wrong thing there.

Otherwise, closure compiler can also affect such code, make sure it isn't enabled when you see the bug.

--
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.
For more options, visit https://groups.google.com/d/optout.

arnab choudhury

unread,
Apr 26, 2016, 12:24:36 PM4/26/16
to emscripten-discuss
The other strange thing I've noticed is that debug versions of the ASM JS do not have this problem (since the constructor function is not removed). Even stranger, manually disabling the minifyNames pass also makes the problem go away. Does JSDCE get run on the entire JS file or just the ASM.JS? I've also found that renaming the constructor function to enum_constructor makes the problem go away.
To unsubscribe from this group and stop receiving emails from it, send an email to emscripten-discuss+unsub...@googlegroups.com.

arnab choudhury

unread,
Apr 26, 2016, 12:48:54 PM4/26/16
to emscripten-discuss
FYI - I've posted the repro files to: https://github.com/achoudhury85/Embind-Enum-Registration-Issue

You just have to run build.cmd (the repro was built for windows but is very easily adapted to other platforms) and then run run.cmd to see the issue.

arnab choudhury

unread,
Apr 26, 2016, 1:18:58 PM4/26/16
to emscripten-discuss
I've also logged an issue in the issue tracker for this: https://github.com/kripken/emscripten/issues/4276 
Reply all
Reply to author
Forward
0 new messages