[lldb-vscode] capture the debuggers file handles before lldbinit runs

[wallace]

Summary

We need to do this before we create the debugger because the commands run in the lldbinit might do output to these handlers as well.

This is still an issue when lldb-vscode is run in stdout/stdin mode, not sure how to address this to be honest, we would need to both capture the stdout (that comes from the script interpreter) and wrap it using the stdout category. Maybe I should post this to the lldb-dev ML.

[clayborg]

Would be good to write a test for this where the init file does "script print('hello')" which would hose up the connection

[clayborg]

lldb/tools/lldb-vscode/lldb-vscode.cpp

1207–1210

Are we allows to add extra "initialize" arguments here? How and who will ever pass this to lldb-vscode? Is there a way to make Microsoft VS Code pass this? If not, then just use an environment variable instead?

[clayborg]

Abandon this one and modify https://reviews.llvm.org/D70882?

[aadsm]

The way phabricator shows the diff is tricky, this change has nothing to do with D70882 and stands by itself. The important part here is making the g_vsc.debugger.SetOutputFileHandle(out, true); g_vsc.debugger.SetErrorFileHandle(out, false); happen before the debugger creation. Not sure how to create a test for this though since there's no mechanism to give lldb-vscode an initial file to load...

[labath]

In D70886#1765629, @aadsm wrote:

The way phabricator shows the diff is tricky, this change has nothing to do with D70882 and stands by itself. The important part here is making the g_vsc.debugger.SetOutputFileHandle(out, true); g_vsc.debugger.SetErrorFileHandle(out, false); happen before the debugger creation. Not sure how to create a test for this though since there's no mechanism to give lldb-vscode an initial file to load...

Maybe you could do something similar to LocalLLDBInit.test ?

[aadsm]

Maybe you could do something similar to LocalLLDBInit.test ?

That test uses the lldb -S (and others) flags that lldb-vscode doesn't support :(. These flags should really be added to the initialize packet but they're very specific to lldb and the DAP doesn't support it.. We could implement something like what Driver::ProcessArgs does and pass options through envs but, the logic in ProcessArgs itself is sketchy (just like the comment mentions) and I l also find it odd to pass options through env vars...

[clayborg]

We could just add a new flag to lldb-vscode like "--no-lldb-init" and always pass that when we run our test suite?

[labath]

In D70886#1767654, @aadsm wrote:

Maybe you could do something similar to LocalLLDBInit.test ?

That test uses the lldb -S (and others) flags that lldb-vscode doesn't support :(. These flags should really be added to the initialize packet but they're very specific to lldb and the DAP doesn't support it.. We could implement something like what Driver::ProcessArgs does and pass options through envs but, the logic in ProcessArgs itself is sketchy (just like the comment mentions) and I l also find it odd to pass options through env vars...

I was specifically referring to the part where the test sets up an .lldbinit in a random directory and then points the HOME environment variable there in order to "trick" lldb into executing it. It's true that the test also uses -S/--source-before-file, but nothing in there is really needed to test this functionality (i.e., making sure that lldb-vscode handles the lldbinit file reasonably -- making sure that it does not get confused by ambient lldbinit files is a different topic).

As for -no-lldbinit functionality, all of the proposed solutions (cmdline flag, environment variable, protocol flag...) seem reasonable to me. The environment variable is probably easiest to implement, and should be sufficient if we only want to use this for testing....

lldb/tools/lldb-vscode/lldb-vscode.cpp

 //       "body": {
 //         "$ref": "#/definitions/Capabilities",
 //         "description": "The capabilities of this debug adapter."
 //       }
 //     }
 //   }]
 // }
 void request_initialize(const llvm::json::Object &request) {
-  auto arguments = request.getObject("arguments");
-  const auto skipInitFiles = GetBoolean(arguments, "skipInitFiles", false);
-  g_vsc.debugger =
-      lldb::SBDebugger::Create(!skipInitFiles /*source_init_files*/);
-  // Create an empty target right away since we might get breakpoint requests
-  // before we are given an executable to launch in a "launch" request, or a
-  // executable when attaching to a process by process ID in a "attach"
-  // request.
   FILE *out = llvm::sys::RetryAfterSignal(nullptr, fopen, dev_null_path, "w");
   if (out) {
     // Set the output and error file handles to redirect into nothing otherwise
     // if any code in LLDB prints to the debugger file handles, the output and
     // error file handles are initialized to STDOUT and STDERR and any output
     // will kill our debug session.
     g_vsc.debugger.SetOutputFileHandle(out, true);
     g_vsc.debugger.SetErrorFileHandle(out, false);
   }
 
+  auto arguments = request.getObject("arguments");
+  const auto skipInitFiles = GetBoolean(arguments, "skipInitFiles", false);
+  g_vsc.debugger =
+      lldb::SBDebugger::Create(!skipInitFiles /*source_init_files*/);
+  // Create an empty target right away since we might get breakpoint requests
+  // before we are given an executable to launch in a "launch" request, or a
+  // executable when attaching to a process by process ID in a "attach"
+  // request.
   g_vsc.target = g_vsc.debugger.CreateTarget(nullptr);
   lldb::SBListener listener = g_vsc.debugger.GetListener();
   listener.StartListeningForEvents(
       g_vsc.target.GetBroadcaster(),
       lldb::SBTarget::eBroadcastBitBreakpointChanged);
   listener.StartListeningForEvents(g_vsc.broadcaster,
                                    eBroadcastBitStopEventThread);
   // Start our event thread so we can receive events from the debugger, target,