Drops into Simulator Debugger only in a session that is already interactive.

Drops into Simulator Debugger only in a session that is already interactive.

This change backs out an earlier change to the disassemblers that
made them return false if an instruction couldn't be decoded.
This was used in the simulator debuggers for arm and mips to avoid
executing an instruction that couldn't be decoded.

Instead, now when an unknown instruction is encountered in the simulator,
if the simulator is running interactively, we drop into
the debugger, and otherwise die. We detect that a simulator is
running interactively by setting a flag if we enter the debugger
for a user set break point.
Committed: https://code.google.com/p/dart/source/detail?r=20394

[regis, 2013-03-21 17:21:13]

LGTM

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_arm.h
File runtime/vm/simulator_arm.h (right):

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_arm.h#ne...
runtime/vm/simulator_arm.h:142: // Accesors to set and check whether the
simulator debugger is attached.
Accessors

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_mips.h
File runtime/vm/simulator_mips.h (right):

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_mips.h#n...
runtime/vm/simulator_mips.h:108: // Accesors to set and check whether the
simulator debugger is attached.
Accessors

[zra, 2013-03-21 17:48:51]

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_arm.h
File runtime/vm/simulator_arm.h (right):

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_arm.h#ne...
runtime/vm/simulator_arm.h:142: // Accesors to set and check whether the
simulator debugger is attached.
On 2013/03/21 17:21:13, regis wrote:
> Accessors

Done.

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_mips.h
File runtime/vm/simulator_mips.h (right):

https://codereview.chromium.org/12903006/diff/1/runtime/vm/simulator_mips.h#n...
runtime/vm/simulator_mips.h:108: // Accesors to set and check whether the
simulator debugger is attached.
On 2013/03/21 17:21:13, regis wrote:
> Accessors

Done.

[Ivan Posva, 2013-03-21 18:23:36]

https://codereview.chromium.org/12903006/diff/11001/runtime/vm/simulator_arm.cc
File runtime/vm/simulator_arm.cc (right):

https://codereview.chromium.org/12903006/diff/11001/runtime/vm/simulator_arm....
runtime/vm/simulator_arm.cc:1422: dbg.Attach(instr, message);
What will happen if a test with nobody watching on the terminal executes
something assembled like this:

__ stop("This case should not happen.");

[zra, 2013-03-21 18:32:08]

On 2013/03/21 18:23:36, Ivan Posva wrote:
>
https://codereview.chromium.org/12903006/diff/11001/runtime/vm/simulator_arm.cc
> File runtime/vm/simulator_arm.cc (right):
> 
>
https://codereview.chromium.org/12903006/diff/11001/runtime/vm/simulator_arm....
> runtime/vm/simulator_arm.cc:1422: dbg.Attach(instr, message);
> What will happen if a test with nobody watching on the terminal executes
> something assembled like this:
> 
> __ stop("This case should not happen.");

The test will time-out and be recorded as failing.

[zra, 2013-03-21 22:07:44]

On 2013/03/21 18:32:08, zra wrote:
> On 2013/03/21 18:23:36, Ivan Posva wrote:
> >
>
https://codereview.chromium.org/12903006/diff/11001/runtime/vm/simulator_arm.cc
> > File runtime/vm/simulator_arm.cc (right):
> > 
> >
>
https://codereview.chromium.org/12903006/diff/11001/runtime/vm/simulator_arm....
> > runtime/vm/simulator_arm.cc:1422: dbg.Attach(instr, message);
> > What will happen if a test with nobody watching on the terminal executes
> > something assembled like this:
> > 
> > __ stop("This case should not happen.");
> 
> The test will time-out and be recorded as failing.

After discussion with Ivan, we decided that it would make more sense to close
stdin in the test script so that fgets in ReadLine in the SimulatorDebugger will
fail unless we are supposed to be interactive. That obviates the "attached"
stuff in the SimulatorDebugger, and also means we don't have to wait for a
timeout if we hit a Stop while running the test script. The CL that closes stdin
is here:

[zra, 2013-03-21 22:08:24]

https://codereview.chromium.org/12521020/

[zra, 2013-03-22 17:08:00]

On 2013/03/21 22:08:24, zra wrote:
> https://codereview.chromium.org/12521020/

The change to the test script has landed, so this change should be ready, now.

[regis, 2013-03-22 17:33:55]

LGTM

[zra, 2013-03-22 18:11:12]

Committed patchset #4 manually as r20394 (presubmit successful).