Haiyang-Sun / nodeprof.js

Instrumentation framework for Node.js compliant to ECMAScript 2020 based on GraalVM.
Apache License 2.0
53 stars 22 forks source link

Unexpected value for 'base' in invokeFunPre hook #38

Open barslev opened 5 years ago

barslev commented 5 years ago

I have created a test program and a jalangi analysis which shows an unexpected value for the argument 'base' in the invokeFunPre hook.

// Test program
new String.constructor();
// DO NOT INSTRUMENT
J$.analysis = {};

(function (sandbox) {
  function AnalysisEngine() {
    this.invokeFunPre = function (iid, f, base, args, isConstructor, isMethod, functionIid) {
      if (isConstructor) {
        console.log("Base for constructor call: ");
    console.log(base);
      }
    }
  }
  sandbox.analysis = new AnalysisEngine();
})(J$);

Running NodeProf with the above program and analysis, we get the following output:

Base for constructor call: 
undefined

where I would expect, the base object to be the function String.

alexjordan commented 5 years ago

This seems to be by-design at the moment. I'll check if we run into any issues in our extended tests when we always pass the receiver object. If not, it's a simple fix.

alexjordan commented 5 years ago

Sorry, spoke to soon (mentioning a simple fix should have given it away)! I rather seems that NodeProf has exhibits a subtle difference when it comes to the base paramater of invokeFunPre that is probably not communicated well enough.

Jalangi, sees the call new String.constructor() as a constructor call and a method call. That does not make a lot of sense IMO, because for a constructor call the base object is not the this value inside the constructor. I assume the whole thing is an artifact of Jalangi's source-code instrumentation.

For NodeProf, due to the underlying properties of the Graal.js AST, an invocation is either a method call (that has a base) or an object allocation using new (with an unknown receiver object). We could rename the argument and document this difference, I would however be in favor of changing the callback API to reflect the difference (I'll rather have your analysis break in an obvious way than in a subtle one.)

For your concrete problem of wanting to know where the constructor function came from, I'd suggest the following extension to your analysis:

(function (sandbox) {
  let baseMap = new Map(); // this map would ideally be cleared at certain points

  function AnalysisEngine() {
    this.invokeFunPre = function (iid, f, base, args, isConstructor, isMethod, functionIid) {
      console.log("ctor/method/base: ", isConstructor, isMethod, base);
      if (isConstructor) {
        console.log('constructor base is:', baseMap.get(f));
      }
    }
    this.getField = function(iid, base, offset, val, isComputed, isOpAssign, isMethodCall) {
      baseMap.set(val, base);
    }
  }
  sandbox.analysis = new AnalysisEngine();
})(J$);