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