yinghuitan created this revision.
yinghuitan added reviewers: clayborg, aadsm.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96623

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/launch/.lldbinit
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1274,6 +1274,93 @@
   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",
@@ -1351,7 +1438,11 @@
 //   }]
 // }
 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"
@@ -1366,6 +1457,11 @@
     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);
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -83,6 +83,29 @@
 
     @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
@@ -440,7 +463,7 @@
         '''
         self.build_and_create_debug_adaptor()
         program = self.getBuildArtifact("a.out")
-        
+
         terminateCommands = ['expr 4+2']
         self.launch(program=program,
                     terminateCommands=terminateCommands)
Index: lldb/test/API/tools/lldb-vscode/launch/.lldbinit
===================================================================
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/launch/.lldbinit
@@ -0,0 +1,6 @@
+
+# Emit some output to stdout.
+help target
+
+# A unique command alias to execute/check.
+command alias __unique_custom_command__ help breakpoint
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -139,7 +139,7 @@
         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
@@ -304,7 +304,7 @@
                 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",
@@ -317,7 +317,7 @@
                 else:
                     desc = 'unkonwn reverse request "%s"' % (response_or_request['command'])
                     raise ValueError(desc)
-            
+
         return None
 
     def wait_for_event(self, filter=None, timeout=None):
@@ -787,7 +787,7 @@
         }
         response = self.send_recv(command_dict)
         return response
-        
+
     def request_completions(self, text):
         args_dict = {
             'text': text,
@@ -914,7 +914,7 @@
 
 
 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()
@@ -924,6 +924,7 @@
                                             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)
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -9,18 +9,18 @@
 
     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):
@@ -334,7 +334,7 @@
         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,
@@ -344,7 +344,7 @@
         '''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,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to