Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluating a variable after a failed Step Out causes a fatal error, leaving debug session unusable #1336

Closed
gareth-rees opened this issue Aug 15, 2022 · 0 comments · Fixed by #1337

Comments

@gareth-rees
Copy link
Contributor

gareth-rees commented Aug 15, 2022

Steps to reproduce

  1. In Visual Studio Code, install the C/C++ extension from the Visual Studio Code marketplace.
  2. Create a Launch Configuration using the "(gdb) Launch" template and update "program" to point to a suitable executable with debug symbols.
  3. Use View ⟶ Run to open the Run panel, and then select "(gdb) Launch".
  4. When the program stops in main(), click on Step Out. This fails with an error message like this in the Debug Console:
    ERROR: Unexpected GDB output from command "-exec-finish". "finish" not meaningful in the outermost frame.
    
    Note that this failure is expected.
  5. In the input field below the Debug Console, type the name of a local variable that you want to evaluate.
  6. This fails with the following error message (see screenshot):
    Stopping due to fatal error: ProtocolException: Cannot evaluate expression on the specified stack frame.
    
    Now the debug session is no longer usable.

Screenshot from 2022-08-15 14-55-56

Analysis

The fatal error comes from the lookup of the stack frame in GetDebugPropertyFromExpression() here:

if (frameId == -1 && isExecInConsole)
{
// If exec in console and no stack frame, evaluate off the top frame.
success = m_frameHandles.TryGetFirst(out frame);
}
else
{
success = m_frameHandles.TryGet(frameId, out frame);
}
if (!success)
{
throw new ProtocolException(AD7Resources.Error_InvalidStackFrameOnEvaluateExpression);
}

Why does the call to m_frameHandles.TryGetFirst() or m_frameHandles.TryGet() fail? That's because m_frameHandles was reset by the call to BeforeContinue() in StepInternal() here:

BeforeContinue();
ErrorBuilder builder = new ErrorBuilder(() => errorMessage);
m_isStepping = true;
enum_STEPUNIT stepUnit = enum_STEPUNIT.STEP_STATEMENT;
switch (granularity)
{
case SteppingGranularity.Statement:
default:
break;
case SteppingGranularity.Line:
stepUnit = enum_STEPUNIT.STEP_LINE;
break;
case SteppingGranularity.Instruction:
stepUnit = enum_STEPUNIT.STEP_INSTRUCTION;
break;
}
try
{
builder.CheckHR(m_program.Step(thread, stepKind, stepUnit));
}
catch (AD7Exception)
{
m_isStopped = true;
throw;
}

The problem here is that if m_program.Step() fails without running the program, then MIEngine is left without any stack frames. The stack frames will be restored at the next point where HandleStackTraceRequestAsync() is called, but this does not happen until a subsequent successful attempt to run the program. In the mean time, any calls to GetDebugPropertyFromExpression() will result in a fatal error.

Suggested fix

In StepInternal(), wait until the program is known to be running before calling BeforeContinue() to discard the frame handles and other program state, for example:

ErrorBuilder builder = new ErrorBuilder(() => errorMessage);
m_isStepping = true;

// ... some code elided ...

try
{
    builder.CheckHR(m_program.Step(thread, stepKind, stepUnit));
}
catch (AD7Exception)
{
    m_isStopped = true;
    throw;
}
// The program should now be running, so it is safe to discard the
// cached program state.
BeforeContinue();

Software versions

C/C++ extension: 1.11.5
VSCode: 1.70.1
Commit: 6d9b74a70ca9c7733b29f0456fd8195364076dda
Date: 2022-08-10T06:09:15.055Z
Electron: 18.3.5
Chromium: 100.0.4896.160
Node.js: 16.13.2
V8: 10.0.139.17-electron.0
OS: Linux x64 5.4.0-122-generic snap

gareth-rees added a commit to undoio/MIEngine that referenced this issue Aug 15, 2022
Previously the cached state was discarded before calling `Step`, but
this meant that if `Step` failed then MIEngine was left with no cached
state, causing a subsequent call to `GetDebugPropertyFromExpression`
to fail with a fatal error.
aleks-ivanov pushed a commit to pipeline-foundation/MIEngine that referenced this issue Aug 16, 2022
…icrosoft#1337)

Previously the cached state was discarded before calling `Step`, but
this meant that if `Step` failed then MIEngine was left with no cached
state, causing a subsequent call to `GetDebugPropertyFromExpression`
to fail with a fatal error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant