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

Currently, the runInTerminal is implemented by using --wait-for underneath, 
which is error prone as it often can't attach on Linux. A reason I've found is 
that LLDB does process polling by using commands like "ps", which are not 
standard across distributions. This is a problem that doesn't happen on Mac.

For more context, the debug adapter protocol specifies that the response of the 
"runInTerminal" reverse request doesn't need to provide the pid of the process 
launched by the IDE. VSCode, for example, doesn't return the pid. Without this 
pid, the only way to attach to the process is with --wait-for.

However, IDEs like VSCode support customizations of their debuggers, allowing 
it to return the pid. In this case, lldb-vscode needs to know that the IDE will 
return the pid and therefore it shouldn't use --wait-for. Because of this, 
lldb-vscode will also require the launch request to include 
'runInTerminalRespondsWithPid: true' to work this way. This is acceptable, as 
the user/developer is already customizing the IDE, so doings the pid addition 
and the runInTerminalRespondsWithPid inclusion are simple changes.

This new approach to runInTerminal is guaranteed to attach to the process, as 
long as it doesn't finish right away, which is better than what we had before.

Tested on Mac and Linux


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93744

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/runInTerminal/TestVSCode_runInTerminal.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===================================================================
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -178,7 +178,7 @@
 							},
 							"runInTerminal": {
 								"type": "boolean",
-								"description": "Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs",
+								"description": "Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs. If the client IDE returns the \"processId\" as part of the \"runInTerminal\" reverse request, then it's suggested that the client IDE includes the field '\"runInTerminalRespondsWithPid\": true' as part of the \"launch\" request, as LLDB can guarantee attachment to the process this way.",
 								"default": false
 							}
 						}
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1449,8 +1449,16 @@
   g_vsc.is_attach = true;
   lldb::SBAttachInfo attach_info;
   lldb::SBError error;
-  attach_info.SetWaitForLaunch(true, /*async*/ true);
-  g_vsc.target.Attach(attach_info, error);
+  const llvm::json::Object *arguments = launch_request.getObject("arguments");
+  bool runInTerminalRespondsWithPid =
+      GetBoolean(*arguments, "runInTerminalRespondsWithPid", false);
+
+  // If the client IDE won't provide the pid of the debuggee, then we attach
+  // with "wait for"
+  if (!runInTerminalRespondsWithPid) {
+    attach_info.SetWaitForLaunch(true, /*async*/ true);
+    g_vsc.target.Attach(attach_info, error);
+  }
 
   llvm::json::Object reverse_request =
       CreateRunInTerminalReverseRequest(launch_request);
@@ -1461,19 +1469,28 @@
     error.SetErrorString("Process cannot be launched by IDE.");
 
   if (error.Success()) {
-    // Wait for the attach stop event to happen or for a timeout.
-    g_vsc.waiting_for_run_in_terminal = true;
-    static std::mutex mutex;
-    std::unique_lock<std::mutex> locker(mutex);
-    g_vsc.request_in_terminal_cv.wait_for(locker, std::chrono::seconds(10));
-
-    auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
-    if (attached_pid == LLDB_INVALID_PROCESS_ID)
-      error.SetErrorString("Failed to attach to a process");
-    else
-      SendProcessEvent(Attach);
+    if (runInTerminalRespondsWithPid) {
+      lldb::pid_t pid =
+          GetUnsigned(reverse_response, "processId", LLDB_INVALID_PROCESS_ID);
+      attach_info.SetProcessID(pid);
+      g_vsc.debugger.SetAsync(false);
+      g_vsc.target.Attach(attach_info, error);
+      g_vsc.debugger.SetAsync(true);
+    } else {
+      // Wait for the attach stop event to happen or for a timeout.
+      g_vsc.waiting_for_run_in_terminal = true;
+      static std::mutex mutex;
+      std::unique_lock<std::mutex> locker(mutex);
+      g_vsc.request_in_terminal_cv.wait_for(locker, std::chrono::seconds(10));
+    }
   }
 
+  auto attached_pid = g_vsc.target.GetProcess().GetProcessID();
+  if (attached_pid == LLDB_INVALID_PROCESS_ID)
+    error.SetErrorString("Failed to attach to a process");
+  else
+    SendProcessEvent(Attach);
+
   if (error.Fail()) {
     launch_response["success"] = llvm::json::Value(false);
     EmplaceSafeString(launch_response, "message",
Index: lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py
+++ lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py
@@ -46,3 +46,36 @@
         # We verify we were able to set the environment
         env = self.vscode.request_evaluate('foo')['body']['result']
         self.assertIn('bar', env)
+
+    @skipIfRemote
+    def test_runInTerminalWithPidProvidedByClient(self):
+        '''
+            Tests the "runInTerminal" reverse request when the client provides
+            the inferior's pid in the "runInTerminal" response.
+        '''
+        program = self.getBuildArtifact("a.out")
+        source = 'main.c'
+        self.build_and_launch(program, stopOnEntry=True, runInTerminal=True, args=["foobar"], env=["FOO=bar"],
+            runInTerminalRespondsWithPid=True)
+        breakpoint_line = line_number(source, '// breakpoint')
+
+        bpts = self.set_source_breakpoints(source, [breakpoint_line])
+        # We have to do this instead of continue_to_next_stop because
+        # sometimes lldb stops for a second time inside the sleep method.
+        # That's something that needs to be fixed in a different patch.
+        self.continue_until_any_of_breakpoints_is_hit(bpts)
+
+        # We verify we actually stopped inside the loop
+        counter = int(self.vscode.get_local_variable_value('counter'))
+        self.assertTrue(counter > 0)
+
+        # We verify we were able to set the launch arguments
+        argc = int(self.vscode.get_local_variable_value('argc'))
+        self.assertEqual(argc, 2)
+
+        argv1 = self.vscode.request_evaluate('argv[1]')['body']['result']
+        self.assertIn('foobar', argv1)
+
+        # We verify we were able to set the environment
+        env = self.vscode.request_evaluate('foo')['body']['result']
+        self.assertIn('bar', env)
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
@@ -309,7 +309,7 @@
                 return response_or_request
             else:
                 if response_or_request['command'] == 'runInTerminal':
-                    subprocess.Popen(response_or_request['arguments']['args'], 
+                    inferior = subprocess.Popen(response_or_request['arguments']['args'],
                         env=response_or_request['arguments']['env'])
                     self.send_packet({
                         "type": "response",
@@ -317,7 +317,9 @@
                         "request_seq": response_or_request['seq'],
                         "success": True,
                         "command": "runInTerminal",
-                        "body": {}
+                        "body": {
+                            "processId": inferior.pid
+                        }
                     }, set_sequence=False)
                 else:
                     desc = 'unkonwn reverse request "%s"' % (response_or_request['command'])
@@ -617,7 +619,7 @@
                        stopCommands=None, exitCommands=None,
                        terminateCommands=None ,sourcePath=None,
                        debuggerRoot=None, launchCommands=None, sourceMap=None,
-                       runInTerminal=False):
+                       runInTerminal=False, runInTerminalRespondsWithPid=False):
         args_dict = {
             'program': program
         }
@@ -658,6 +660,7 @@
             args_dict['sourceMap'] = sourceMap
         if runInTerminal:
             args_dict['runInTerminal'] = runInTerminal
+            args_dict['runInTerminalRespondsWithPid'] = runInTerminalRespondsWithPid
         command_dict = {
             'command': 'launch',
             'type': 'request',
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
@@ -233,6 +233,23 @@
         self.vscode.request_continue()
         self.verify_breakpoint_hit(breakpoint_ids)
 
+    '''
+        Continues successive stop points until any of the provided breakpoints is hit.
+        If an unwanted breakpoint is hit after the given timeout, this method throws an
+        exception.
+    '''
+    def continue_until_any_of_breakpoints_is_hit(self, breakpoint_ids, timeout_in_sec=10):
+        t0 = time.time()
+        while (True):
+            self.vscode.request_continue()
+            try:
+                self.verify_breakpoint_hit(breakpoint_ids)
+                break
+            except:
+                t = time.time() - t0
+                if t > timeout_in_sec:
+                    raise
+
     def continue_to_exception_breakpoint(self, filter_label):
         self.vscode.request_continue()
         self.assertTrue(self.verify_exception_breakpoint_hit(filter_label),
@@ -282,7 +299,8 @@
                trace=False, initCommands=None, preRunCommands=None,
                stopCommands=None, exitCommands=None, terminateCommands=None,
                sourcePath=None, debuggerRoot=None, launchCommands=None,
-               sourceMap=None, disconnectAutomatically=True, runInTerminal=False):
+               sourceMap=None, disconnectAutomatically=True, runInTerminal=False,
+               runInTerminalRespondsWithPid=False):
         '''Sending launch request to vscode
         '''
 
@@ -317,7 +335,8 @@
             debuggerRoot=debuggerRoot,
             launchCommands=launchCommands,
             sourceMap=sourceMap,
-            runInTerminal=runInTerminal)
+            runInTerminal=runInTerminal,
+            runInTerminalRespondsWithPid=runInTerminalRespondsWithPid)
         if not (response and response['success']):
             self.assertTrue(response['success'],
                             'launch failed (%s)' % (response['message']))
@@ -333,7 +352,8 @@
                          trace=False, initCommands=None, preRunCommands=None,
                          stopCommands=None, exitCommands=None,
                          terminateCommands=None, sourcePath=None,
-                         debuggerRoot=None, runInTerminal=False):
+                         debuggerRoot=None, runInTerminal=False,
+                         runInTerminalRespondsWithPid=False):
         '''Build the default Makefile target, create the VSCode debug adaptor,
            and launch the process.
         '''
@@ -343,4 +363,5 @@
         self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
                     disableSTDIO, shellExpandArguments, trace,
                     initCommands, preRunCommands, stopCommands, exitCommands,
-                    terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal)
+                    terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal,
+                    runInTerminalRespondsWithPid=runInTerminalRespondsWithPid)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to