[Lldb-commits] [PATCH] D111209: Don't push null ExecutionContext on CommandInterpreter exectx stack

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: tatyana-krasnukha.
jasonmolenda added a project: LLDB.
Herald added subscribers: JDevlieghere, GorNishanov.
jasonmolenda requested review of this revision.

Part of Tatyana's patch in https://reviews.llvm.org/D92164 added a stack of ExecutionContexts to the CommandInterpreter (pushed with OverrideExecutionContext, popped with RestoreExecutionContext) and when we need to retrieve the currently selected ExecutionContext via CommandInterpeter:: GetExecutionContext(), if the stack has an entry, we return the exe_ctx from the top of the stack, else we fall back to the Debugger::GetSelectedExecutionContext() which gives you an exe_ctx with the currently selected Target.

This patch is fixing the fact that the lldb command driver during startup, pushes the current exe_ctx on the stack here,

  -> 2874	    OverrideExecutionContext(m_debugger.GetSelectedExecutionContext());
     2875	  auto finalize = llvm::make_scope_exit([this]() {
     2876	    RestoreExecutionContext();
  0 0x00010d9fc80d    LLDB`lldb_private::CommandInterpreter::IOHandlerInputComplete + 285  CommandInterpreter.cpp:2874
  1 0x00010d81573d    LLDB`lldb_private::IOHandlerEditline::Run + 349  IOHandler.cpp:582
  2 0x00010d7ccd71    LLDB`lldb_private::Debugger::RunIOHandlers + 81  Debugger.cpp:879
  3 0x00010d9fe0d4    LLDB`lldb_private::CommandInterpreter::RunCommandInterpreter + 212  CommandInterpreter.cpp:3126
  4 0x00010d268370    LLDB`lldb::SBDebugger::RunCommandInterpreter + 544  SBDebugger.cpp:1238
  5 0x00010854b8a2    lldb`Driver::MainLoop + 3298  Driver.cpp:678

There is no Target at this point, so we have an ExecutionContext with a nullptr for a target (and all other fields).

This Target won't get popped off the stack, it remains there as the first entry. When loading a corefile, which loads a binary and dSYM, and the dSYM has Python code in it, and the SB API tries to set some settings via SBCommandInterpreter::HandleCommand, if those commands need the current Target to set something, they will silently fail to do anything.  Target settings that are set in the template target will work, but those that only take effect in the current target are ignored.  It can be a little tricky to spot.

Complicating the test case writing, API tests don't have this same "push a nullptr target on to the stack" issue, so they won't reproduce the problem.  I started writing an API test and I kept it in this patch because it might find regressions in the future. Jonas helped rewrite the test into a shell test that uses the driver program to show the issue and confirm the patch fixes it.

I can imagine we might iterate on exactly how I avoid pushing a nullptr ExecutionContext, but I think the basic safeguard is pretty straightforward - it's a pretty unusual situation to have no selected Targets, this only happens during early startup I suspect.  Passing an arg to the make_scope_exit to avoid popping the stack seems especially unpretty.  I wasn't worried about popping too many elements off the stack, the code will ignore that case, but I was worried that it might be possible to have a real ExecutionContext on the stack, then a nullptr ExecutionContext is pushed, and I didn't want to pop the real exectx later on.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111209

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/test/API/macosx/dsym-script-settings-corefile/Makefile
  lldb/test/API/macosx/dsym-script-settings-corefile/TestdSYMScriptSettingsCorefile.py
  lldb/test/API/macosx/dsym-script-settings-corefile/a_out.py
  lldb/test/API/macosx/dsym-script-settings-corefile/main.c
  lldb/test/API/macosx/dsym-script-settings-corefile/operating_system.py
  lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/a_out.py
  lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/main.c
  lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/operating_system.py
  lldb/test/Shell/ScriptInterpreter/Python/OS/os.test

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111209.377452.patch
Type: text/x-patch
Size: 15274 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20211006/873d1d94/attachment-0001.bin>