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

Source section fix #11

Closed Haiyang-Sun closed 5 years ago

alexjordan commented 5 years ago

@eleinadani Does graal.js not have an internal function to adapt source sections for the injected module header? E.g for stack traces or the debugger?

Haiyang-Sun commented 5 years ago

Currently, I've noticed some inconsistent source section mapping around the module header. Some source sections have excluded the module header while some not.

eleinadani commented 5 years ago

No, we don't have anything special in our parser for the module header (i.e., the wrapping function) -- if we have wrong source sections mappings they might be bugs, so better to open a separate issue, assign it to me, and I'll look into them case-by-case.

regarding the debugger support, nothing special AFAIK: the variables in the module header are closed in the scope of the module, but the module header "function" itself is not in the stack when a module is used (it is only when the module is required, which is why you might not have it in the stack depending on where you have your breakpoint)

alexjordan commented 5 years ago

@eleinadani I understand, I just assumed that the difference with Jalangi running on V8-Node was due to the runtime, but it's the source-level instrumentation, of course. I had a look at what throwing an exception in the first line looks like with Node.js:

1st-line.js:1
(function (exports, require, module, __filename, __dirname) { throw 'in first line';

So the header is definitely there, and given all that, I now think that it makes more sense for NodeProf to follow Node's convention rather than Jalangi's.

Any thoughts?

Haiyang-Sun commented 5 years ago

I think keeping the source sections as Node's convention is fine. However, we need to make sure all nodes follow the same convention so that the user can adjust the index in the analysis side. However, I've noticed that some nodes (e.g., function root nodes) have source sections that do not count the module header. If you check the failed travis build here, you will find that the root node beginning from the first line would have source sections that do not count the module's header as other nodes and lead to a negative index.

Testing exitException.js in minitests with analysis enter-exit
Fail @enter-exit
5c5
< functionExit: (src/ch.usi.inf.nodeprof.test/js/minitests/exitException.js:1:1:37:4) / 3
---
> functionExit: (src/ch.usi.inf.nodeprof.test/js/minitests/exitException.js:1:-61:37:4) / 3
11c11
< functionExit: (src/ch.usi.inf.nodeprof.test/js/minitests/exitException.js:1:2:37:2) / 3
---
> functionExit: (src/ch.usi.inf.nodeprof.test/js/minitests/exitException.js:1:-60:37:2) / 3
Haiyang-Sun commented 5 years ago

The source section issue is now fixed with Graal.js, I will close this PR.