List scope chain levels and their variables to the selected stack frame. #2557

robertsipka · 2018-10-08T08:42:52Z

It supports to list the scope chain of the current execution context and see which variables are available.

Please note that this is not a complete list, virtual properties are not included. I plan to expand them in a follow-up patch.

An example run should look like this:

test.js

1 var c = 4;
2 var z = 5;
3 
4 function addX(x) {
5  return function(n) {
6     var b = 2;
7     return n + x;
8   }
9 }
10 addThree = addX(3);
11 d = addThree(c+z);
12 
13 function f() {
14   var x = true;
15   var g = 0;
16   (function() {
17     var a = [1,2,3]
18     a.y = "abc";
20     with (a) {
21       var h = [4,5,6]
22       with (h) {
23         h.d = "dfg"
24       }
25       a.d = x + g;
26     }
27   })();
28 }
29 
30 f();

$ ./build/bin/jerry --start-debug-server test.js

$ ./jerry-debugger/jerry_client.py

Connecting to: localhost:5001
Stopped at test.js:1
(jerry-debugger) b test.js:21
Breakpoint 1 at test.js:21 (in function() at line:16, col:4)
(jerry-debugger) c
Press enter to stop JavaScript execution.
Stopped at breakpoint:1 test.js:21 (in function() at line:16, col:4)
(jerry-debugger) scopes
level | type    
0     | with    
1     | local   
2     | closure 
3     | global  
(jerry-debugger) variables 0
name | type   | value 
y    | String | abc   
2    | Number | 3     
1    | Number | 2     
0    | Number | 1     
(jerry-debugger) variables 1
name | type  | value   
h    | Array | [4,5,6] 
a    | Array | [1,2,3] 
(jerry-debugger) variables 2
name | type    | value 
g    | Number  | 0     
x    | Boolean | true  
(jerry-debugger) variables 3
name     | type     | value                        
d        | Number   | 12                           
addThree | Function | function(){/* ecmascript */} 
f        | Function | function(){/* ecmascript */} 
addX     | Function | function(){/* ecmascript */} 
z        | Number   | 5                            
c        | Number   | 4                            
print    | Function | function(){/* ecmascript */} 
gc       | Function | function(){/* ecmascript */} 
assert   | Function | function(){/* ecmascript */} 
(jerry-debugger)

zherczeg · 2018-10-16T10:14:02Z

jerry-core/debugger/debugger.c

+      }
+    }
+
+    memcpy (message_type_p->string + buffer_pos++, &level_type, 1);


You don't need memcpy to copy 1 byte. To prepare classes ignore non-declarative and non-object-bound lex envs.

zherczeg · 2018-10-16T10:15:25Z

jerry-core/debugger/debugger.c

+  const size_t max_byte_count = JERRY_DEBUGGER_SEND_MAX (uint8_t);
+  const size_t max_message_size = JERRY_DEBUGGER_SEND_SIZE (max_byte_count, uint8_t);
+
+  ECMA_STRING_TO_UTF8_STRING (value_str, str_buff, str_buff_size);


Is this a cesu8 string or some costly conversion?

It is the name or value of the property. So, it is always returns with the cesu8 character array of a string.

zherczeg · 2018-10-16T10:16:35Z

jerry-core/debugger/debugger.c

+    {
+      if (!jerry_debugger_send (max_message_size))
+      {
+        ECMA_FINALIZE_UTF8_STRING (str_buff, str_buff_size);


I have never seen such a macro used for early return. They should behave block like.

zherczeg · 2018-10-16T10:18:15Z

jerry-core/debugger/debugger.c

+      jerry_debugger_send_type (JERRY_DEBUGGER_SCOPE_VARIABLES_END);
+      return;
+    }
+    chain_index--;


Ignore non this object or declarative lex envs as well.

I do not understand exactly what you mean. It will steps through the outer references of the current lexical environment until the chain_index argument reach zero. It is always touch a declarative or this object bound lexical environment. If not, it will returns with an empty message with JERRY_DEBUGGER_SCOPE_VARIABLES_END type (e.g.: the chain_index argument is invalid / bigger than the scope chain).

In the future classes will introduces a new scope type. This type should not be shared by the debugger client side, and when encountered it should be skipped. Therefore only the two currently supported types should be handled by this loop.

Oh I see, thanks. I've updated it.

zherczeg · 2018-10-16T10:19:39Z

tests/debugger/do_variables.expected

+d        | Number   | 12                           
+addThree | Function | function(){/* ecmascript */} 
+f        | Function | function(){/* ecmascript */} 
+addX     | Function | function(){/* ecmascript */} 


We should send no values for functions.

You are right, it makes sense. I've updated it accordingly.

zherczeg · 2018-10-16T10:20:22Z

tests/debugger/do_variables.expected

+f      | Function  | function(){/* ecmascript */} 
+addX   | Function  | function(){/* ecmascript */} 
+z      | undefined | undefined                    
+c      | undefined | undefined                    


What about objects?

This really missed. I've added.

jerry-core/debugger/debugger.c

zherczeg · 2018-10-19T10:00:36Z

jerry-core/debugger/debugger.c

+      else
+      {
+        level_type = JERRY_DEBUGGER_SCOPE_WITH;
+        next_func_is_local = true;


Why this value is set to true here? I might misunderstood its purpose.

zherczeg · 2018-10-19T10:01:03Z

jerry-core/debugger/debugger.c

+
+      if (ecma_get_lex_env_outer_reference (lex_env_p) == NULL)
+      {
+        level_type = JERRY_DEBUGGER_SCOPE_GLOBAL;


A break could be added here, and change the loop to while(true)

zherczeg · 2018-10-19T10:03:56Z

jerry-core/debugger/debugger.c

+
+  if (ecma_is_value_undefined (value))
+  {
+    ret_value = JERRY_DEBUGGER_VARIABLE_UNDEFINED;


It might be nitpicking, but I think VALUE is better than VARIABLE, since the value of a variable is undefined, rather than the variable, which would mean it does not exist.

zherczeg · 2018-10-19T10:06:19Z

jerry-core/debugger/debugger.c

+      *buffer_pos = 0;
+    }
+
+    if (copied_values == 3)


This sounds like a state for a switch. Furthermore the last case is handled specially, I would take it out from this loop and done separately, since it already has a copy loop.

zherczeg · 2018-10-19T10:09:05Z

jerry-core/debugger/debugger.c

+  {
+    lex_env_p = ecma_get_lex_env_outer_reference (lex_env_p);
+
+    if (lex_env_p == NULL)


JERRY_UNLIKELY

zherczeg · 2018-10-19T10:10:36Z

jerry-core/debugger/debugger.c

+
+    for (int i = 0; i < ECMA_PROPERTY_PAIR_ITEM_COUNT; i++)
+    {
+      if (ECMA_PROPERTY_IS_NAMED_PROPERTY (prop_iter_p->types[i]))


I am not sure all names are printable. I think Promises has some internal properties with special magic string names. They should be ignored.

zherczeg · 2018-10-19T10:12:00Z

jerry-core/ecma/base/ecma-globals.h

+/**
+ * Catch clause.
+ */
+#define ECMA_OBJECT_FLAG_CATCH_CLAUSE 0x20


I would call it local scope, since we might add let/const at some day and they would be marked this flag as well.

robertsipka · 2018-10-24T12:12:34Z

@zherczeg : Thanks for the review, I've updated the patch based on your suggestions.

akosthekiss · 2018-10-26T20:42:40Z

jerry-core/vm/vm.c

          }

          ecma_object_t *catch_env_p = ecma_create_decl_lex_env (frame_ctx_p->lex_env_p);
+          catch_env_p->type_flags_refs = (uint16_t) (catch_env_p->type_flags_refs | ECMA_OBJECT_FLAG_NON_CLOSURE);


Two questions:

Is this code debugger-specific? If so, it should be wrapped into #ifdef JERRY_DEBUGGER / #endif /* JERRY_DEBUGGER */.

Isn't this equivalent to catch_env_p->type_flags_refs |= (uint16_t) ECMA_OBJECT_FLAG_NON_CLOSURE;?

akosthekiss · 2018-10-26T20:47:51Z

jerry-core/debugger/debugger.c

+  JERRY_ASSERT (ret_value != JERRY_DEBUGGER_VALUE_NONE);
+
+  return ret_value;
+} /* ecma_get_typeof_scope_variable */


The names of the new static functions are somewhat inconsistent. Most are prefixed with jerry_debugger_ but this one here is ecma_get_typeof_scope_variable while the next one is simply copy_scope_variables_to_string_message without any obvious prefix. Is this intentional?

zherczeg

Pretty good now.

zherczeg · 2018-10-29T07:44:26Z

jerry-core/debugger/debugger.c

+    {
+      if ((lex_env_p->type_flags_refs & ECMA_OBJECT_FLAG_NON_CLOSURE) != 0)
+      {
+        message_type_p->string[buffer_pos++] = JERRY_DEBUGGER_SCOPE_CATCH;


Please don\t call this catch.

akosthekiss · 2018-10-29T11:26:20Z

jerry-core/debugger/debugger.c

+ *         false - otherwise
+ */
+static bool
+ jerry_debugger_copy_variables_to_string_message (jerry_debugger_scope_variable_type_t variable_type, /**< type */


check-vera.sh is reporting a style issue here (extra space at the start of the line)

jerry-core/debugger/debugger.c

It supports to list the scope chain of the current execution context and see which variables are available. JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com

zherczeg

LGTM

akosthekiss

LGTM

robertsipka mentioned this pull request Oct 11, 2018

Support the scope and variables requests jerryscript-project/iotjs-vscode-extension#30

Merged

robertsipka force-pushed the variables_list branch 6 times, most recently from a342847 to ba01461 Compare October 12, 2018 13:05

robertsipka changed the title ~~(WIP) List scope chain levels and their variables to the selected stack frame.~~ List scope chain levels and their variables to the selected stack frame. Oct 12, 2018

robertsipka force-pushed the variables_list branch from ba01461 to aa64635 Compare October 12, 2018 13:49

zherczeg requested changes Oct 16, 2018

View reviewed changes

robertsipka force-pushed the variables_list branch 2 times, most recently from 567553b to e1ae997 Compare October 19, 2018 07:54

zherczeg requested changes Oct 19, 2018

View reviewed changes

robertsipka force-pushed the variables_list branch 3 times, most recently from 434fb3b to 3004684 Compare October 24, 2018 10:34

akosthekiss reviewed Oct 26, 2018

View reviewed changes

akosthekiss added debugger Related to the debugger development Feature implementation labels Oct 26, 2018

zherczeg requested changes Oct 29, 2018

View reviewed changes

robertsipka force-pushed the variables_list branch from 3004684 to 409fb8f Compare October 29, 2018 09:51

akosthekiss requested changes Oct 29, 2018

View reviewed changes

List scope chain levels and their variables to the selected stack frame.

87a55a4

It supports to list the scope chain of the current execution context and see which variables are available. JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com

robertsipka force-pushed the variables_list branch from 409fb8f to 87a55a4 Compare October 29, 2018 12:16

zherczeg approved these changes Oct 29, 2018

View reviewed changes

akosthekiss approved these changes Oct 29, 2018

View reviewed changes

akosthekiss merged commit 34c5e22 into jerryscript-project:master Oct 29, 2018

robertsipka deleted the variables_list branch October 27, 2021 11:07

List scope chain levels and their variables to the selected stack frame. #2557

List scope chain levels and their variables to the selected stack frame. #2557

Uh oh!

Conversation

robertsipka commented Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg Oct 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka commented Oct 24, 2018

Uh oh!

akosthekiss Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

robertsipka commented Oct 8, 2018 •

edited

Loading

robertsipka Oct 17, 2018 •

edited

Loading

robertsipka Oct 17, 2018 •

edited

Loading

zherczeg Oct 18, 2018 •

edited

Loading

akosthekiss Oct 26, 2018 •

edited

Loading