Haiyang-Sun/nodeprof.js

Missing hook for read of 'this'.

mtorp opened this issue · 4 comments

mtorp commented

In Jalangi the read of this triggers the read hook. It seems like no hook is triggered in nodeprof. Are there any plans on adding this feature to nodeprof as well?

@mtorp thanks for the feedback. The bug needs some Graal.js update. Please track this PR #36 and I will merge it when necessary changes are updated. For the moment, the patch below in Graal.js could fix the problem

diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/JSFrameUtil.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/JSFrameUtil.java
index 11dc8b7062..ea934e2e21 100644
--- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/JSFrameUtil.java
+++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/JSFrameUtil.java
@@ -110,7 +110,7 @@ public final class JSFrameUtil {
             String name = (String) frameSlot.getIdentifier();
             if (name.startsWith(":")) {
                 return true;
-            } else if (name.startsWith("<") && name.endsWith(">")) {
+            } else if (name.startsWith("<") && name.endsWith(">") && !name.equals("<this>")) {
                 return true;
             }
             return false;

@eleinadani this variable is currently checked as an internal variable in Graal.js while it shouldn't. It's different from other internal variables and shouldn't be hidden from the instrumentation.

I agree this sounds like a Graal.js issue. @Haiyang-Sun your patch above is wrong: the <this> variable is internal: we use a frame slot to store a reference to this, but it should not be exposed. I think we can fix this issue in a different way: do you have a small unit test to reproduce?

    @Test
    public void readThis() {
        evalWithTag("(()=>{return this;})()", JSTags.ReadVariableExpressionTag.class);

        enter(ReadVariableExpressionTag.class, (e, r) -> {
            assertAttribute(e, NAME, "<this>");
        }).exit();
    }

Fixed in #36