Support more debugger commands in the wire protocol

Support more debugger commands in the wire protocol

Also add a very simple debugger shell which at this point
is meant to be a debugging help for the debugger code rather
than a user-friendly tool.
Committed: https://code.google.com/p/dart/source/detail?r=7518

[hausner, 2012-05-09 22:49:00]

Added my homegrown debugger shell script. Should it be added?

[siva, 2012-05-10 20:08:36]

LGTM

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection.cc
File runtime/bin/dbg_connection.cc (right):

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:185: dart::JSONReader r(json_msg);
This r object appears to be unused.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:226: Dart_Handle lib_url =
Dart_NewString(lib_url_chars);
ASSERT(!Dart_IsError(lib_url));

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:241: Dart_StringToCString(lib_url, &chars);
See comments below about allocation of chars.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:254: msg.Printf("{ \"id\": %d, \"result\": {
\"urls\": [", msg_id);
Does msg.Printf handle buffer overruns?

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:255: Dart_Handle urls = Dart_GetLibraryURLs();
ASSERT(!Dart_IsError(urls));

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:262: Dart_StringToCString(lib_url, &chars);
chars is allocated in the ApiScope, do you create a new ApiScope and exit the
ApiScope every time the breakpoint handler is called?

Otherwise we will be accumulating this in the main ApiScope and could be
considered a leak.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:314: if (!is_handled) {
It appears that the loop above could handle multiple commands. If the first
command is handled and the next one
is not recognized we will not get this "unrecognized command" message.

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart
File tools/ddbg.dart (right):

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart#newcode45
tools/ddbg.dart:45: if (verbose) print("sending: '${JSON.stringify(cmd)}'");
not sure what the Dart style guide says but we usually always have a { .. }

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart#newcode119
tools/ddbg.dart:119: return;
Is this return necessary?

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart#newcode177
tools/ddbg.dart:177: StringInputStream stdinStream = new
StringInputStream(stdin);
var stdinStream = new ....

[hausner, 2012-05-10 22:36:28]

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection.cc
File runtime/bin/dbg_connection.cc (right):

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:185: dart::JSONReader r(json_msg);
On 2012/05/10 20:08:36, asiva wrote:
> This r object appears to be unused.

Done.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:226: Dart_Handle lib_url =
Dart_NewString(lib_url_chars);
On 2012/05/10 20:08:36, asiva wrote:
> ASSERT(!Dart_IsError(lib_url));

Done.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:241: Dart_StringToCString(lib_url, &chars);
On 2012/05/10 20:08:36, asiva wrote:
> See comments below about allocation of chars.

Done.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:254: msg.Printf("{ \"id\": %d, \"result\": {
\"urls\": [", msg_id);
On 2012/05/10 20:08:36, asiva wrote:
> Does msg.Printf handle buffer overruns?

Yes, it dynamically reallocs buffer space.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:255: Dart_Handle urls = Dart_GetLibraryURLs();
On 2012/05/10 20:08:36, asiva wrote:
> ASSERT(!Dart_IsError(urls));

Done.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:262: Dart_StringToCString(lib_url, &chars);
Thanks for this tip. Added a scope around the callback.

On 2012/05/10 20:08:36, asiva wrote:
> chars is allocated in the ApiScope, do you create a new ApiScope and exit the
> ApiScope every time the breakpoint handler is called?
> 
> Otherwise we will be accumulating this in the main ApiScope and could be
> considered a leak.

http://codereview.chromium.org/10392017/diff/7002/runtime/bin/dbg_connection....
runtime/bin/dbg_connection.cc:314: if (!is_handled) {
If a message is handled, the break statement in line 310 terminates the while
loop. The following message is then handled (or not handled) in a new iteration
of the outer loop. Thus, it appears to me that the case you describe can't
happen.

On 2012/05/10 20:08:36, asiva wrote:
> It appears that the loop above could handle multiple commands. If the first
> command is handled and the next one
> is not recognized we will not get this "unrecognized command" message.

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart
File tools/ddbg.dart (right):

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart#newcode45
tools/ddbg.dart:45: if (verbose) print("sending: '${JSON.stringify(cmd)}'");
On 2012/05/10 20:08:36, asiva wrote:
> not sure what the Dart style guide says but we usually always have a { .. }

Done.

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart#newcode119
tools/ddbg.dart:119: return;
On 2012/05/10 20:08:36, asiva wrote:
> Is this return necessary?
Um, no. It may have been at some time.

http://codereview.chromium.org/10392017/diff/7002/tools/ddbg.dart#newcode177
tools/ddbg.dart:177: StringInputStream stdinStream = new
StringInputStream(stdin);
On 2012/05/10 20:08:36, asiva wrote:
> var stdinStream = new ....

Slowly weaning off of using types :)