[lldb-vscode] Fix lldb init file stdout issue

[yinghuitan]

Summary

VScode DAP client uses stdout to receive protocol message. This means any stdout/stderr from debugger would be treated as DAP packet. This means any lldb commands in init file can generate random output and break DAP client.
This diff fixes the issue by explicitly sourcing lldbinit file after stdout/stderr is redirected to dev_null instead of relying on SBDebugger::Create().
There is another issue that SBDebugger::Create() only sources the lldbinit file from home directory not debugger's cwd. This diverages from what lldb does. This diff explicitly sources init file from cwd as well.
An integration test is added.

[clayborg]

After thinking about this a bit more, it seems a mistake that we can't change the file handles _prior_ to sourcing the init files. I have proposed one solution in the inline comments to create a new SBDebugger::Create() method that takes the input output and error file handles as arguments. The other way to do this is to make it so that we _can_ run the init files later. Currently if we say "false" to "bool source_init_files", then the command interpreter marks the init files as not being able to be run again later if you call the SBCommandInterpreter::SourceInitFileInHomeDirectory(...). So we need to either be able to either specify the file handles at debugger creation time, or run them one time later. The current "run them once" means "you can only do this at debugger creation time, but never again if you specify false for source_init_files. We can change this to be "only run the init files once. so if we already run them in SBDebugger::Create(), then don't run them again if you call SBCommandInterpreter::SourceInitFileInHomeDirectory()

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

1444

So after thinking about this some more, it might be better to add a new lldb::SBDebugger::Create method that takes the input, output and error handles as SBFile objects. This will allow us to not have to duplicate the entire .lldbinit loading stuff in this patch. My proposal would be to add a new create method:

lldb::SBDebugger lldb::SBDebugger::Create(bool source_init_files, SBFile input, SBFile output, SBFile error);

SBFile has multiple constructors that will work for us:

SBFile()
SBFile(FILE *file, bool transfer_ownership);
SBFile(int fd, const char *mode, bool transfer_ownership);

If we default construct a SBFile, it will contain no valid shared pointer in the private "FileSP m_opaque_sp;" member. Any of the SBFile objects that are not valid, won't end up being set in the debugger. All valid file handles will be set _prior_ to sourcing the init files, and then we can avoid any problems.

in the SBDebugger.cpp you would do something like:

lldb::SBDebugger lldb::SBDebugger::Create(bool source_init_files, SBFile input, SBFile output, SBFile error) {

  static std::recursive_mutex g_mutex;
  std::lock_guard<std::recursive_mutex> guard(g_mutex);
  debugger.reset(Debugger::CreateInstance(callback, baton));
  if (input.IsValid())
    debugger.SetInputFile(input);
  if (output.IsValid())
    debugger.SetOutputFile(output):
  if (error.IsValid())
    debugger.SetErrorFile(error);
  SBCommandInterpreter interp = debugger.GetCommandInterpreter();
  if (source_init_files) {
    interp.get()->SkipLLDBInitFiles(false);
    interp.get()->SkipAppInitFiles(false);
    SBCommandReturnObject result;
    interp.SourceInitFileInHomeDirectory(result, false);
  } else {
    interp.get()->SkipLLDBInitFiles(true);
    interp.get()->SkipAppInitFiles(true);
  }
  return debugger;
}

[clayborg]

I added Jim and Pavel to get some more feedback.

The main issue is the Visual Studio Code debug adaptors like lldb-vscode receive and send packets on STDIN and STDOUT. The debugger is created during the "initialize" request which expects a response. If the debugger tries to run the init files when the debugger is created, it risks outputting some info to STDOUT and causing the entire debug session to fail the stdout is the default output file handle for debuggers. If you don't run the init files, they are marked as skipped in the command interpreter and calling SBCommandInterpreter::SourceInitFileInHomeDirectory(...) will do nothing if you call it later.

A further complication of this patch is that we intend to support different LLDB.framework binaries in order to support Swift debugging in the future by setting a DYLD_FRAMEWORK_PATH in the environment of lldb-vscode when it is launched so we can use the LLDB.framework (or liblldb.so for linux) that matches the swift we need. This means that if we change the API by adding the new lldb::SBDebugger::Create() method that takes the SBFile handles for in/out/error, then we can't use these older LLDB versions since it will crash when it doesn't find this new create method. The current version of this patch does actually allow us to use these older LLDB.framework binaries, though it does duplicate the running of the init files manually. So maybe this is the best option for now, but I wanted to include a few other people to get their input.

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

 
 from lldbsuite.test.lldbtest import *
 import os
 import vscode
 import time
 
 
 class VSCodeTestCaseBase(TestBase):
 
     NO_DEBUG_INFO_TESTCASE = True
 
-    def create_debug_adaptor(self):
+    def create_debug_adaptor(self, adapter_cwd=None):
         '''Create the Visual Studio Code debug adaptor'''
         self.assertTrue(os.path.exists(self.lldbVSCodeExec),
                         'lldb-vscode must exist')
         log_file_path = self.getBuildArtifact('vscode.txt')
         self.vscode = vscode.DebugAdaptor(
             executable=self.lldbVSCodeExec, init_commands=self.setUpCommands(),
-            log_file=log_file_path)
+            log_file=log_file_path, adapter_cwd=adapter_cwd)
 
-    def build_and_create_debug_adaptor(self):
+    def build_and_create_debug_adaptor(self, adapter_cwd=None):
         self.build()
-        self.create_debug_adaptor()
+        self.create_debug_adaptor(adapter_cwd)
 
     def set_source_breakpoints(self, source_path, lines, condition=None,
                                hitCondition=None):
         '''Sets source breakpoints and returns an array of strings containing
            the breakpoint IDs ("1", "2") for each breakpoint that was set.
         '''
         response = self.vscode.request_setBreakpoints(
             source_path, lines, condition=condition, hitCondition=hitCondition)
def launch(self, program=None, args=None, cwd=None, env=None,
                             'launch failed (%s)' % (response['message']))
         # We need to trigger a request_configurationDone after we've successfully
         # attached a runInTerminal process to finish initialization.
         if runInTerminal:
             self.vscode.request_configurationDone()
         return response
 
 
-    def build_and_launch(self, program, args=None, cwd=None, env=None,
+    def build_and_launch(self, program, adapter_cwd=None, args=None, cwd=None, env=None,
                          stopOnEntry=False, disableASLR=True,
                          disableSTDIO=False, shellExpandArguments=False,
                          trace=False, initCommands=None, preRunCommands=None,
                          stopCommands=None, exitCommands=None,
                          terminateCommands=None, sourcePath=None,
                          debuggerRoot=None, runInTerminal=False):
         '''Build the default Makefile target, create the VSCode debug adaptor,
            and launch the process.
         '''
-        self.build_and_create_debug_adaptor()
+        self.build_and_create_debug_adaptor(adapter_cwd)
         self.assertTrue(os.path.exists(program), 'executable must exist')
 
         return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
                     disableSTDIO, shellExpandArguments, trace,
                     initCommands, preRunCommands, stopCommands, exitCommands,
                     terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal)

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

def validate_response(cls, command, response):
             raise ValueError('seq mismatch in response')
 
     def get_modules(self):
         module_list = self.request_modules()['body']['modules']
         modules = {}
         for module in module_list:
             modules[module['name']] = module
         return modules
-        
     def get_output(self, category, timeout=0.0, clear=True):
         self.output_condition.acquire()
         output = None
         if category in self.output:
             output = self.output[category]
             if clear:
                 del self.output[category]
         elif timeout != 0.0:
def send_recv(self, command):
             if response_or_request is None:
                 desc = 'no response for "%s"' % (command['command'])
                 raise ValueError(desc)
             if response_or_request['type'] == 'response':
                 self.validate_response(command, response_or_request)
                 return response_or_request
             else:
                 if response_or_request['command'] == 'runInTerminal':
-                    subprocess.Popen(response_or_request['arguments']['args'], 
+                    subprocess.Popen(response_or_request['arguments']['args'],
                         env=response_or_request['arguments']['env'])
                     self.send_packet({
                         "type": "response",
                         "seq": -1,
                         "request_seq": response_or_request['seq'],
                         "success": True,
                         "command": "runInTerminal",
                         "body": {}
                     }, set_sequence=False)
                 else:
                     desc = 'unkonwn reverse request "%s"' % (response_or_request['command'])
                     raise ValueError(desc)
-            
         return None
 
     def wait_for_event(self, filter=None, timeout=None):
         while True:
             return self.recv_packet(filter_type='event', filter_event=filter,
                                     timeout=timeout)
         return None
 
def request_compileUnits(self, moduleId):
         args_dict = {'moduleId': moduleId}
         command_dict = {
             'command': 'compileUnits',
             'type': 'request',
             'arguments': args_dict
         }
         response = self.send_recv(command_dict)
         return response
-        
     def request_completions(self, text):
         args_dict = {
             'text': text,
             'column': len(text)
         }
         command_dict = {
             'command': 'completions',
             'type': 'request',
def request_testGetTargetBreakpoints(self):
         return self.send_recv(command_dict)
 
     def terminate(self):
         self.send.close()
         # self.recv.close()
 
 
 class DebugAdaptor(DebugCommunication):
-    def __init__(self, executable=None, port=None, init_commands=[], log_file=None):
+    def __init__(self, executable=None, adapter_cwd=None, port=None, init_commands=[], log_file=None):
         self.process = None
         if executable is not None:
             adaptor_env = os.environ.copy()
             if log_file:
                 adaptor_env['LLDBVSCODE_LOG'] = log_file
             self.process = subprocess.Popen([executable],
                                             stdin=subprocess.PIPE,
                                             stdout=subprocess.PIPE,
                                             stderr=subprocess.PIPE,
+                                            cwd=adapter_cwd,
                                             env=adaptor_env)
             DebugCommunication.__init__(self, self.process.stdout,
                                         self.process.stdin, init_commands)
         elif port is not None:
             s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
             s.connect(('127.0.0.1', port))
             DebugCommunication.__init__(self, s.makefile('r'), s.makefile('w'),
                 init_commands)

lldb/test/API/tools/lldb-vscode/launch/.lldbinit

+
+# Emit some output to stdout.
+help target
+
+# A unique command alias to execute/check.
+command alias __unique_custom_command__ help breakpoint

lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

def test_stopOnEntry(self):
                 if 'reason' in body:
                     reason = body['reason']
                     self.assertTrue(
                         reason != 'breakpoint',
                         'verify stop isn\'t "main" breakpoint')
 
     @skipIfWindows
     @skipIfRemote
+    def test_lldbinit(self):
+        '''
+            Tests running commands in lldbinit file with stdout output
+            won't break DAP protocol.
+        '''
+        program = self.getBuildArtifact("a.out")
+        test_dir = os.path.dirname(os.path.realpath(__file__))
+
+        custom_command_name = '__unique_custom_command__'
+
+        # Run debugger adapter in test_dir which contains a cutomized .lldbinit file.
+        self.build_and_launch(program,
+                              adapter_cwd=test_dir, stopOnEntry=True, preRunCommands=[custom_command_name])
+        self.continue_to_next_stop()
+        console_output = self.get_console()
+
+        # custom_command_name should be in console output and is a valid command alias.
+        self.assertIn(custom_command_name, console_output)
+        self.assertNotIn('not a valid command', console_output)
+        self.assertIn('Commands for operating on breakpoints', console_output)
+
+    @skipIfWindows
+    @skipIfRemote
     def test_cwd(self):
         '''
             Tests the default launch of a simple program with a current working
             directory.
         '''
         program = self.getBuildArtifact("a.out")
         program_parent_dir = os.path.realpath(
             os.path.dirname(os.path.dirname(program)))
class TestVSCode_launch(lldbvscode_testcase.VSCodeTestCaseBase):
     @skipIf(archs=["arm", "aarch64"]) # Example of a flaky run http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5540/steps/test/logs/stdio
     def test_terminate_commands(self):
         '''
             Tests that the "terminateCommands", that can be passed during
             launch, are run when the debugger is disconnected.
         '''
         self.build_and_create_debug_adaptor()
         program = self.getBuildArtifact("a.out")
-        
         terminateCommands = ['expr 4+2']
         self.launch(program=program,
                     terminateCommands=terminateCommands)
         self.get_console()
         # Once it's disconnected the console should contain the
         # "terminateCommands"
         self.vscode.request_disconnect(terminateDebuggee=True)
         output = self.collect_console(duration=1.0)
         self.verify_commands('terminateCommands', output, terminateCommands)

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

void request_modules(const llvm::json::Object &request) {
   }
 
   llvm::json::Object body;
   body.try_emplace("modules", std::move(modules));
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+static void GetHomeInitFile(llvm::SmallString<128> &init_file,
+                            llvm::StringRef suffix = {}) {
+  std::string init_file_name = ".lldbinit";
+  if (!suffix.empty()) {
+    init_file_name.append("-");
+    init_file_name.append(suffix.str());
+  }
+  if (!llvm::sys::path::home_directory(init_file)) {
+    g_vsc.SendOutput(OutputType::Stderr,
+                     llvm::StringRef("Failed to get home directory"));
+    return;
+  }
+  llvm::sys::path::append(init_file, init_file_name);
+
+  llvm::SmallString<128> init_file_real_path;
+  if (std::error_code error = llvm::sys::fs::real_path(
+          init_file.c_str(), init_file_real_path, /*expand_tilde*/ true)) {
+    auto err_msg = std::string("Failed to expand path: ") + error.message();
+    g_vsc.SendOutput(OutputType::Stderr, llvm::StringRef(err_msg.c_str()));
+    return;
+  }
+  init_file.assign(init_file_real_path);
+}
+
+/**
+ * Find and return the init file in user's home directory in \p init_file.
+ * It searches .lldbinit-<progarm-name> first, then fall back to .lldbinit.
+ */
+static void GetInitFilePathHomeDirectory(llvm::SmallString<128> &init_file) {
+  llvm::StringRef program_name =
+      lldb::SBHostOS::GetProgramFileSpec().GetFilename();
+  GetHomeInitFile(init_file, program_name);
+  if (!llvm::sys::fs::exists(init_file.c_str())) {
+    init_file.clear();
+    GetHomeInitFile(init_file);
+  }
+}
+
+/**
+ * Source the local init file in debugger's current working directory in \p
+ * init_file.
+ */
+static void
+GetInitFilePathCurrentWorkingDirectory(llvm::SmallString<128> &init_file) {
+  if (std::error_code error = llvm::sys::fs::current_path(init_file)) {
+    auto err_msg = std::string("Failed to get current working directory: ") +
+                   error.message();
+    g_vsc.SendOutput(OutputType::Stderr, llvm::StringRef(err_msg.c_str()));
+    return;
+  }
+  llvm::sys::path::append(init_file, ".lldbinit");
+
+  llvm::SmallString<128> init_file_real_path;
+  if (std::error_code error = llvm::sys::fs::real_path(
+          init_file.c_str(), init_file_real_path, /*expand_tilde*/ true)) {
+    auto err_msg = std::string("Failed to expand path: ") + error.message();
+    g_vsc.SendOutput(OutputType::Stderr, llvm::StringRef(err_msg.c_str()));
+    return;
+  }
+  init_file.assign(init_file_real_path);
+}
+
+static void SourceCommandFile(const char *command_file) {
+  if (llvm::sys::fs::exists(command_file)) {
+    lldb::SBCommandReturnObject result;
+
+    std::string source_file_cmd = "command source ";
+    source_file_cmd.append(command_file);
+    g_vsc.debugger.GetCommandInterpreter().HandleCommand(
+        source_file_cmd.c_str(), result, /*add_to_history*/ false);
+  }
+}
+
+/**
+ * Source .lldbinit files in user's home and debugger's cwd directories.
+ * This is similar to what Driver.cpp does.
+ */
+static void SourceInitFile() {
+  llvm::SmallString<128> init_file;
+  GetInitFilePathHomeDirectory(init_file);
+  SourceCommandFile(init_file.c_str());
+
+  init_file.clear();
+  GetInitFilePathCurrentWorkingDirectory(init_file);
+  SourceCommandFile(init_file.c_str());
+}
+
 // "InitializeRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 //     "type": "object",
 //     "description": "Initialize request; value of command field is
 //                     'initialize'.",
 //     "properties": {
 //       "command": {
 //         "type": "string",

 //       "body": {
 //         "$ref": "#/definitions/Capabilities",
 //         "description": "The capabilities of this debug adapter."
 //       }
 //     }
 //   }]
 // }
 void request_initialize(const llvm::json::Object &request) {
-  g_vsc.debugger = lldb::SBDebugger::Create(true /*source_init_files*/);
+  // Can't source_init_files during SBDebugger::Create because the commands in
+  // the init file may generate random output to stdout/stderr so we explicitly
+  // call SourceInitFile() below after dev_null redirection.
+  g_vsc.debugger = lldb::SBDebugger::Create(false /*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);
   }
 
+  // Soruce lldbinit files after debugger's stdout/stderr has been redirected
+  // to dev_null. Note: SourceInitFileInHomeDirectory() API can't be used here
+  // because source_init_files=false enabled the skip flags.
+  SourceInitFile();
+
   // Start our event thread so we can receive events from the debugger, target,
   // process and more.
   g_vsc.event_thread = std::thread(EventThreadFunction);
 
   llvm::json::Object response;
   FillResponse(request, response);
   llvm::json::Object body;
   // The debug adapter supports the configurationDoneRequest.