vadimcn / codelldb

A native debugger extension for VSCode based on LLDB
https://marketplace.visualstudio.com/items?itemName=vadimcn.vscode-lldb
MIT License
2.52k stars 245 forks source link

Pick(My)Process not working #915

Closed rjra100 closed 1 year ago

rjra100 commented 1 year ago

OS: Linux (RHEL7, and yeah, I know...) VSCode version: 1.77.3 CodeLLDB version: 1.90 Compiler: N/A Debuggee: N/A

With a customized Lldb: Library path, the PickProcess dialog is hanging rather than providing a list of processes to select from.

I've done a bit of diagnosis and can see where the problem arises:

If I insert an lldb symlink from the extension directory pointed at our llvm installation so that lldb can be found via the expected path, that does fix the problem. But we're in a Dockerized dev environment and I'd rather not have to redo that after every container rebuild.

Is there any chance of making that logic a little more flexible so that the process picker can work with a non-packaged lldb installation? Some options might be:

rjra100 commented 1 year ago

The following patch does resolve the problem in my environment (as a quick-fix, by falling back to the system path):

index 5799b1c..1fb8f5b 100644
--- a/extension/pickProcess.ts
+++ b/extension/pickProcess.ts
@@ -3,6 +3,7 @@ import * as path from 'path';
 import * as os from 'os';
 import * as cp from 'child_process';
 import * as adapter from './novsc/adapter';
+import * as fs from 'fs';

 type ProcessItem = QuickPickItem & { pid: number };

@@ -49,6 +50,9 @@ export async function pickProcess(context: ExtensionContext, allUsers: boolean):
 async function getProcessList(context: ExtensionContext, allUsers: boolean): Promise<ProcessItem[]> {
     let lldb = os.platform() != 'win32' ? 'lldb' : 'lldb.exe';
     let lldbPath = path.join(context.extensionPath, 'lldb', 'bin', lldb);
+    if (!fs.existsSync(lldbPath)) {
+        lldbPath = lldb
+    }
     let folder = workspace.workspaceFolders[0];
     let config = workspace.getConfiguration('lldb', folder?.uri);
     let env = adapter.getAdapterEnv(config.get('adapterEnv', {}));

However (really a separate issue), I also notice that there are some node processes with multi-line command lines, which also seem to cause niggles in the output: image The command lines in question look like

2475 ?        Ssl    0:00 /home/ralexander72/.vscode-server/bin/704ed70d4fd1c6bd6342c436f1ede30d1cff4710/node -e  ????const net = require('net'); ????const fs = require('fs'); ????process.stdin.pause(); ????const client = net.createConnection({ host: '127.0.0.1', port: 38249 }, () => { ?????console.error('Connection established'); ?????client.pipe(process.stdout); ?????process.stdin.pipe(client); ????}); ????client.on('close', function (hadError) { ?????console.error(hadError ? 'Remote close with error' : 'Remote close'); ?????process.exit(hadError ? 1 : 0); ????}); ????client.on('error', function (err) { ?????process.stderr.write(err && (err.stack || err.message) || String(err)); ????}); ????process.stdin.on('close', function (hadError) { ?????console.error(hadError ? 'Remote stdin close with error' : 'Remote stdin close'); ?????process.exit(hadError ? 1 : 0); ????}); ????process.on('uncaughtException', function (err) { ?????fs.writeSync(process.stderr.fd, `Uncaught Exception: ${String(err && (err.stack || err.message) || err)}\n`); ????}); ???

and the raw output of lldb --batch --no-lldbinit --one-line "platform process list --show-args" for one of these looks like:

PID    PARENT USER       TRIPLE                         ARGUMENTS
====== ====== ========== ============================== ============================
2475   0      ralexander72 x86_64-*-linux-gnu             /home/ralexander72/.vscode-server/bin/704ed70d4fd1c6bd6342c436f1ede30d1cff4710/node -e 
                                const net = require('net');
                                const fs = require('fs');
                                process.stdin.pause();
                                const client = net.createConnection({ host: '127.0.0.1', port: 38249 }, () => {
                                        console.error('Connection established');
                                        client.pipe(process.stdout);
                                        process.stdin.pipe(client);
                                });
                                client.on('close', function (hadError) {
                                        console.error(hadError ? 'Remote close with error' : 'Remote close');
                                        process.exit(hadError ? 1 : 0);
                                });
                                client.on('error', function (err) {
                                        process.stderr.write(err && (err.stack || err.message) || String(err));
                                });
                                process.stdin.on('close', function (hadError) {
                                        console.error(hadError ? 'Remote stdin close with error' : 'Remote stdin close');
                                        process.exit(hadError ? 1 : 0);
                                });
                                process.on('uncaughtException', function (err) {
                                        fs.writeSync(process.stderr.fd, `Uncaught Exception: ${String(err && (err.stack || err.message) || err)}\n`);
                                });
rjra100 commented 1 year ago

Here, have a patch for the multi-line niggle too :)

@@ -71,6 +75,13 @@ async function getProcessList(context: ExtensionContext, allUsers: boolean): Pro
         if (argsOffset > 0) {
             for (let line of lines.slice(i + 2)) {
                 let pid = parseInt(line);
+                if (isNaN(pid)) {
+                    let [last] = items.slice(-1);
+                    if (last) {
+                        last.description += line;
+                    }
+                    continue;
+                }
vadimcn commented 1 year ago

If I insert an lldb symlink from the extension directory pointed at our llvm installation so that lldb can be found via the expected path, that does fix the problem. But we're in a Dockerized dev environment and I'd rather not have to redo that after every container rebuild.

Not sure how your patch improves things, wouldn't you still need to delete the bundled lldb binary, so that the extension doesn't find it first?

Here, have a patch for the multi-line niggle too :)

Thanks! I'll take a look.

vadimcn commented 1 year ago

Also, if you are using PickProcess to locate the same binary over and over again, perhaps you should consider automating this step? You could use the "custom" launch mode and run a script in processCreateCommand that finds the right process and attaches to it.

rjra100 commented 1 year ago

Not sure how your patch improves things, wouldn't you still need to delete the bundled lldb binary, so that the extension doesn't find it first?

I forget the details, but I think we point LLDB_PACKAGE at our installation of lldb when building CodeLLDB; the build creates a symlink for build purposes but doesn't actually package lldb itself into the vsix. The result I'm ending up with is that there is no bundled lldb (rather than finding one that doesn't work). That's not a problem as long as we point it at our liblldb.so, except for this (and possibly any other similar niggles we haven't found!). I'm starting to wonder if we're accidentally exploiting a bug in the build system there, but it actually works well for us (we get a nice small vsix and just point it at the lldb installation we have anyway).

Also, if you are using PickProcess to locate the same binary over and over again, perhaps you should consider automating this step?

Interesting. While I'd want the general process picker to work, I think a lot of our usage of Attach is probably connecting to Python notebooks (debugging C++ libs which the notebook is calling into). While I'm not sure how easy it'd be to figure out which process to attach to, that could well be worth a bit of investigation. Thanks!

vadimcn commented 1 year ago

Implemented in v1.9.1

rjra100 commented 1 year ago

Thanks!