Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

for in/of support #31

Open
aralisza opened this issue Apr 7, 2019 · 13 comments
Open

for in/of support #31

aralisza opened this issue Apr 7, 2019 · 13 comments

Comments

@aralisza
Copy link

aralisza commented Apr 7, 2019

My team is interested in instrumenting for...in and for...of loops? There was a forinObject callback in jalangi, but no callback for for...of loops since they weren't supported. Ideally, for our use case, we'd have an object read/write callback for every step of any loop, but having a forinObject and forofObject would be sufficient.

Are there any plans to support these types of for loops?

@mwaldrich
Copy link
Contributor

Is there any update on this feature request? My team is hoping to use this feature soon.

@alexjordan
Copy link
Collaborator

Sorry for the late reply! We are currently scoping how to provide the basic forinObject and forofObject callbacks in NodeProf.

@mwaldrich
Copy link
Contributor

Any update on this feature?

@Haiyang-Sun
Copy link
Owner

It's in progress. Currently, there is a workaround for for-of loop. We still need some support from Graal.js part for the for-in loop.

@eleinadani
Copy link
Collaborator

Yes, the last missing bit is to be implemented on Graal.js -- I haven't had time to work on this recently, so perhaps @Haiyang-Sun can you prototype a patch to Graal.js and contribute that upstream directly (assuming you have time, of course) -- feel free to ping me on slack if you need guidance

@mwaldrich
Copy link
Contributor

Ok, thanks for the work and the update!

@Haiyang-Sun
Copy link
Owner

Haiyang-Sun commented Aug 29, 2019

@eleinadani I found a workaround without changes to Graal.js. We simply keep track of the last expression's execution result that certainly would be the object being iterated.
The patch and test example can be found in #45

@mwaldrich Please check if it works for you.

@mwaldrich
Copy link
Contributor

Thanks for the implementation! We can now easily see the object involved with the loop.

My team was planning on tracking how data flows into the loop variable. However, I'm not sure this is possible with the current callbacks.

Here's an example.

Example

Code to instrument

var a = 1;
var b = [1, 2, a];
var z = 0;

for (var k of b) {
    z = k;
}

Analysis

(function (sandbox) {
    function MyAnalysis() {
        let lastExprResult;
        this.forObject = function (iid, isForIn) {
            console.log(`forObject isForIn=${isForIn}, object=${lastExprResult}`);
        };
        this.endExpression = function (iid, type, result) {
            lastExprResult = result;
        };
        /**
         * These callbacks are called after a variable is read or written.
         **/
        this.read = function (iid, name, val, isGlobal, isScriptLocal) {
            console.log(`read name=${name}, val=${val}`);
        };
        this.write = function (iid, name, val, lhs, isGlobal, isScriptLocal) {
            console.log(`write name=${name}, val=${val}`);
        };
        this.literal = function (iid, val, hasGetterSetter, literalType) {
            console.log(`literal val=${val}`);
            if (typeof val === "object") {
                console.log("object property names:");
                for (prop in val) {
                    if (val.hasOwnProperty(prop)) {
                        console.log("- " + prop);
                    }
                }
            }
        };
    }

    sandbox.analysis = new MyAnalysis();
})(J$);

NodeProf output

After running this with NodeProf, the output is:

literal val=function (exports, require, module, __filename, __dirname) {var a = 1;
var b = [1, 2, a]
var z = 0;

for (var k of b) {
    z = k;
}
}
literal val=1
write name=a, val=1
literal val=1
literal val=2
read name=a, val=1
literal val=1,2,1
object property names:
- 0
- 1
- 2
write name=b, val=1,2,1
literal val=0
write name=z, val=0
read name=b, val=1,2,1
forObject isForIn=false, object=1,2,1
write name=k, val=1
read name=k, val=1
write name=z, val=1
write name=k, val=2
read name=k, val=2
write name=z, val=2
write name=k, val=1
read name=k, val=1
write name=z, val=1

Question

I don't think it's possible with the current callbacks to know where the value of k comes from at any given time.

Consider a single loop iteration from the example above:

write name=k, val=1
read name=k, val=1
write name=z, val=1

These callbacks don't give you any information about where the value of k is coming from. These callbacks only tell you that it has been written to.

Proposal

There are a few ways to make this possible to track.

Emitting read callbacks for each iteration (preferred)

Perhaps the most simple solution I can think of is simply emitting the read callbacks for each iteration of the for loop. A single loop iteration would emit the callbacks:

getField base=b offset=0
write name=k, val=1
read name=k, val=1
write name=z, val=1

More for loop callbacks

Another possible way to track where the value of k comes from in each iteration is to add additional callbacks about for loops. For example, if we had callbacks for:

  • the start of a for loop
  • the end of a for loop
  • the start of a single for loop iteration

we would be able to maintain a stack of in-progress for loops in our analysis. At the beginning of a for loop iteration, we can index into the forObject at the top of the stack to get the value.

I would not prefer this solution though, as it would impose lots of overhead and complication on my team's analysis.

@Haiyang-Sun
Copy link
Owner

Haiyang-Sun commented Sep 17, 2019

#48
The second solution is easy and such a callback is anyway necessary to have.
The first solution can be achieved similarly via adding a cfBlockEnter/Exit callback to report each iteration of the loop (the source section can be used to track the enclosing forIn/forOf node).

@mwaldrich
Copy link
Contributor

I am confused about how the new callbacks could be used to instrument this. What callbacks would be generated in a single iteration of a for of loop?

Also, would it be possible for NodeProf to just generate the correct read callback when iterating through the loop? This would be my preferred solution, as it wouldn't require extra work in analyses to keep records about for loops.

@alexjordan
Copy link
Collaborator

Regarding for-of loops, Graal.js does not expose the events we would need to provide a getField callback. I'm actually not certain that this could be provided given the implementation will be based on (possibly user-defined) iterators. We might be able to expose the iterator object in each iteration, but would this actually help?

@alexjordan
Copy link
Collaborator

I'd like to separate issues for for-in and for-of and merge #48 if it sufficient to support for-in.

@Haiyang-Sun can you please update #48 based on my comments and rebase/squash the commit.

@mwaldrich is anything missing from #48 to support for-in?

@Haiyang-Sun Haiyang-Sun assigned alexjordan and unassigned alexjordan Sep 26, 2019
@alexjordan
Copy link
Collaborator

I proposed changes to the callbacks in #51. The instrumentation logic itself is unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants