Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

"all native function values should be intrinsics" - what does it mean? #1285

Closed
karelbilek opened this issue Dec 20, 2017 · 9 comments
Closed

Comments

@karelbilek
Copy link

I am trying to run prepack on an existing code, I get this error

all native function values should be intrinsics
Invariant Violation: all native function values should be intrinsics

It doesn't show me where exactly is the error in the code, so I am asking here. What does it mean, and how can I find where the error comes from?

@hermanventer
Copy link
Contributor

hermanventer commented Dec 20, 2017

An invariant violation means that something that we thought is impossible has actually happened. I.e. you've run into a programming bug and the onus is on the Prepack developers to fix the problem.

As always, if there is a simple way to reproduce this failure it will be a great help to the developers and speed up the resolution of this problem.

@karelbilek
Copy link
Author

Oh OK! I will try to make a minimal case

@jlongster
Copy link

I just ran into this too. It's very difficult to reduce down to a test case given the nature of running prepack on a generated bundle. My bundle isn't too big, it's ~1500 lines, and here it is: https://gist.github.com/jlongster/85c2f8f313bf52af1e7996b285273587

FWIW here's the screenshot of the error:

screen shot 2018-01-02 at 11 55 57 am

Any tips for how to make a reduced test case? If you hit this yourself how would you do it? I suppose I can try to invoke smaller parts of the system and see what happens.

@jlongster
Copy link

I figured it out - it's promises. How well are promises supported?

I install a global function in my VM which returns a promise: https://gist.github.com/jlongster/85c2f8f313bf52af1e7996b285273587#file-interpreter-output-js-L257-L263. This function is available to the code my VM is interpreting.

At the point where my VM implements the CALL operator which calls functions, it checks the return type to see if it's a promise, and if so, it suspends the VM: https://gist.github.com/jlongster/85c2f8f313bf52af1e7996b285273587#file-interpreter-output-js-L348-L350

If I remove these two pieces of code (make foo return a normal value, and don't check the return type) prepack successfully evaluates my VM.

Maybe this issue is known and promises aren't well-supported yet.

@sebmarkbage
Copy link
Contributor

Evaluating Promises should work but there might be bugs in the serializer, which this looks like it might be.

It is likely that Promises needs to be serialized using a special case branch here since they have to use a built-in constructor (similar to Date). https://github.com/facebook/prepack/blob/master/src/serializer/ResidualHeapSerializer.js#L1204-L1268

@NTillmann
Copy link
Contributor

Here's a small repro:

global.d = Object.getOwnPropertyDescriptor(Map.prototype, "size");

@anilanar
Copy link

anilanar commented May 8, 2018

I'm currently working on this, fixing native properties (such as Map.prototype.size). I assume promises are handled by #1306.

@anilanar
Copy link

anilanar commented May 8, 2018

Actually, prepack should not (?) replace getOwnPropertyDescriptor call for native accessors because there is no way to access native getter/setter functions without a call to getOwnPropertyDescriptor or to its variants.
For the example given above, output could be something like:

(function () {
  var _0 = Object.getOwnPropertyDescriptor(Map.prototype, "size");
  d = _0;
}).call(this);

After making isIntrinsic() return true for Map.prototype#size, current output is as follows which is obviously wrong:

(function () {
  var _$0 = this;

  var _$1 = _$0.Map;
  var _$2 = _$1.prototype;
  var _$3 = _$2.size; // Because I've set getter fn's intrinsic name to Map.prototype.size for now.
  var _4 = _$3;
  var _0 = {
    get: _4,
    set: void 0,
    enumerable: false,
    configurable: true
  };
  d = _0;
}).call(this);

@anilanar
Copy link

anilanar commented May 8, 2018

Setting intrinsicName for native accessor functions to the following fixes this bug: `Object.getOwnPropertyDescriptor(${this.intrinsicName}, "${name}").${accessorType}` where accessorType: "get" | "set".

This produces non-optimal code. E.g:

Input:

(function() {
    var desc = Object.getOwnPropertyDescriptor(Object.prototype, '__proto__');
    global.g = desc.get;
    global.s = desc.set;
    global.c = desc.configurable;
})();

Output:

(function () {
  var _$0 = Object.getOwnPropertyDescriptor(Object.prototype, "__proto__").get;
  var _$1 = Object.getOwnPropertyDescriptor(Object.prototype, "__proto__").set;
  var _2 = _$0;
  g = _2;
  var _1 = _$1;
  s = _1;
  c = true;
})();

I'm pretty sure there's a better approach but I cannot unearth it.

anilanar added a commit to anilanar/prepack that referenced this issue May 9, 2018
Release notes: fixes an issue where usage native accessor descriptors would crash

Closes facebookarchive#1285
anilanar added a commit to anilanar/prepack that referenced this issue May 9, 2018
Release notes: fixes an issue where usage native accessor descriptors would crash

Closes facebookarchive#1285
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants