From 6439e1b8b772c9352686a0dd6bc8a9a063ed0050 Mon Sep 17 00:00:00 2001 From: Dave Holoway Date: Fri, 24 Apr 2020 19:03:39 +0100 Subject: [PATCH] Version 1.1 improvements (#88) * fix 0 alignment in binary xml decoding * output reason for APK manifest read failure * try and match package name against process name when determining which pid to attach * make post launch pause user-configurable * code tidy, jsdocs and types * more types in expression parse classes * fix issue with expandable objects not evaluating * update build task example * fix package/type evaluation * improve handling of targetDevice and processID combinations * show full call stack by default * implement a queue for evaluations * improve performance of retrieving single fields * check root term identifiers against this fields --- README.md | 22 +++- extension.js | 25 ++++- package.json | 5 + src/adbclient.js | 22 +++- src/apk-decoder.js | 2 +- src/apk-file-info.js | 5 +- src/debugMain.js | 200 ++++++++++++++++++++++++++----------- src/debugger-types.js | 39 ++++++-- src/debugger.js | 86 +++++++++++----- src/expression/evaluate.js | 69 +++++++++++-- src/expression/parse.js | 17 +++- src/index.d.js | 1 + src/process-attach.js | 24 ++--- src/stack-frame.js | 2 +- 14 files changed, 381 insertions(+), 138 deletions(-) diff --git a/README.md b/README.md index ed75dd9..a1a894b 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,10 @@ The following settings are used to configure the debugger: // mutually exclusive with "amStartArgs". "launchActivity": ".MainActivity", + // Time in milliseconds to wait after launching an app before attempting to attach + // the debugger. Default: 1000ms + "postLaunchPause": 1000, + // Set to true to output debugging logs for diagnostics. "trace": false } @@ -109,6 +113,7 @@ Add a `preLaunchTask` item to the launch configuration: "request": "launch", "name": "App Build & Launch", "preLaunchTask": "run gradle", + ... } ] } @@ -123,7 +128,22 @@ Add a new task to run the build command: "label": "run gradle", "type": "shell", "command": "${workspaceFolder}/gradlew", - "args": ["assembleDebug"] + "args": [ + "assembleDebug" + ], + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": [], + "group": { + "kind": "build", + "isDefault": true + } } ] } diff --git a/extension.js b/extension.js index 4cf65c3..617c3a2 100644 --- a/extension.js +++ b/extension.js @@ -20,14 +20,33 @@ function activate(context) { openLogcatWindow(vscode); }), // add the device picker handler - used to choose a target device - vscode.commands.registerCommand('PickAndroidDevice', async () => { + vscode.commands.registerCommand('PickAndroidDevice', async (launchConfig) => { + // if the config has both PickAndroidDevice and PickAndroidProcess, ignore this + // request as PickAndroidProcess already includes chooosing a device... + if (launchConfig && launchConfig.processId === '${command:PickAndroidProcess}') { + return ''; + } const device = await selectTargetDevice(vscode, "Launch", { alwaysShow:true }); // the debugger requires a string value to be returned return JSON.stringify(device); }), // add the process picker handler - used to choose a PID to attach to - vscode.commands.registerCommand('PickAndroidProcess', async () => { - const o = await selectAndroidProcessID(vscode); + vscode.commands.registerCommand('PickAndroidProcess', async (launchConfig) => { + // if the config has a targetDevice specified, use it instead of choosing a device... + let target_device = ''; + if (launchConfig && typeof launchConfig.targetDevice === 'string') { + target_device = launchConfig.targetDevice; + } + const explicit_pick_device = target_device === '${command:PickAndroidDevice}'; + if (!target_device || explicit_pick_device) { + // no targetDevice (or it's set to ${command:PickAndroidDevice}) + const device = await selectTargetDevice(vscode, 'Attach', { alwaysShow: explicit_pick_device }); + if (!device) { + return JSON.stringify({status: 'cancelled'}); + } + target_device = device.serial; + } + const o = await selectAndroidProcessID(vscode, target_device); // the debugger requires a string value to be returned return JSON.stringify(o); }), diff --git a/package.json b/package.json index d2d5a15..b8dc30e 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,11 @@ "-r" ] }, + "postLaunchPause": { + "type": "number", + "description": "Time in milliseconds to wait after launching an app before attempting to attach the debugger. Default: 1000", + "default": 1000 + }, "staleBuild": { "type": "string", "description": "Launch behaviour if source files have been saved after the APK was built. One of: [\"ignore\" \"warn\" \"stop\"]. Default: \"warn\"", diff --git a/src/adbclient.js b/src/adbclient.js index a2c51d0..af87844 100644 --- a/src/adbclient.js +++ b/src/adbclient.js @@ -92,17 +92,31 @@ class ADBClient { return stdout.trim().split(/\s+/).filter(x => x).map(s => parseInt(s, 10)); } + /** + * Retrieve a list of named debuggable pids + * @param {number} timeout_ms + */ async named_jdwp_list(timeout_ms) { - const named_pids = (await this.jdwp_list(timeout_ms)) + const pids = await this.jdwp_list(timeout_ms); + return this.get_named_processes(pids); + } + + /** + * Convert a list of pids to named-process objects + * @param {number[]} pids + */ + async get_named_processes(pids) { + if (!pids.length) { + return []; + } + const named_pids = pids .map(pid => ({ pid, name: '', })) - if (!named_pids.length) - return []; // retrieve the list of process names from the device - const command = `for pid in ${named_pids.map(np => np.pid).join(' ')}; do cat /proc/$pid/cmdline;echo " $pid"; done`; + const command = `for pid in ${pids.join(' ')}; do cat /proc/$pid/cmdline;echo " $pid"; done`; const stdout = await this.shell_cmd({ command, untilclosed: true, diff --git a/src/apk-decoder.js b/src/apk-decoder.js index e8582fe..c567010 100644 --- a/src/apk-decoder.js +++ b/src/apk-decoder.js @@ -107,7 +107,7 @@ function decode_spec_value(o, key, value, buf, idx, main) { case /^align:\d+$/.test(value): { // used for arbitrary padding to a specified alignment const align = parseInt(value.split(':')[1], 10); - byteLength = align - (idx % align); + byteLength = idx - (Math.trunc(idx / align) * align); o[key] = buf.slice(idx, idx + byteLength); break; } diff --git a/src/apk-file-info.js b/src/apk-file-info.js index 70d92c5..fb9896a 100644 --- a/src/apk-file-info.js +++ b/src/apk-file-info.js @@ -104,8 +104,7 @@ class APKFileInfo { * 2. The decoded manifest from the APK * 3. The AndroidManifest.xml file from the root of the source tree. */ -async function getAndroidManifestXml(args) { - const {manifestFile, apkFile, appSrcRoot} = args; +async function getAndroidManifestXml({manifestFile, apkFile, appSrcRoot}) { let manifest; // a value from the manifestFile overrides the default manifest extraction @@ -121,7 +120,7 @@ async function getAndroidManifestXml(args) { manifest = await extractManifestFromAPK(apkFile); } catch(err) { // if we fail to get manifest from the APK, revert to the source file version - D(`Reading source manifest from ${appSrcRoot}`); + D(`Reading source manifest from ${appSrcRoot} (${err.message})`); manifest = await readFile(path.join(appSrcRoot, 'AndroidManifest.xml'), 'utf8'); } return manifest; diff --git a/src/debugMain.js b/src/debugMain.js index 1cd391d..4df38bb 100644 --- a/src/debugMain.js +++ b/src/debugMain.js @@ -11,7 +11,7 @@ const path = require('path'); const { ADBClient } = require('./adbclient'); const { APKFileInfo } = require('./apk-file-info'); const { Debugger } = require('./debugger'); -const { BreakpointOptions, BuildInfo, DebuggerException, DebuggerValue, JavaBreakpointEvent, JavaClassType, JavaExceptionEvent, SourceLocation } = require('./debugger-types'); +const { AttachBuildInfo, BreakpointOptions, DebuggerException, DebuggerValue, JavaBreakpointEvent, JavaClassType, JavaExceptionEvent, LaunchBuildInfo, SourceLocation } = require('./debugger-types'); const { evaluate } = require('./expression/evaluate'); const { PackageInfo } = require('./package-searcher'); const ADBSocket = require('./sockets/adbsocket'); @@ -77,7 +77,7 @@ class AndroidDebugSession extends DebugSession { this._android_sources_path = ''; // number of call stack entries to display above the project source - this.callStackDisplaySize = 1; + this.callStackDisplaySize = 0; /** * the fifo queue of evaluations (watches, hover, etc) @@ -118,26 +118,18 @@ class AndroidDebugSession extends DebugSession { /** * The 'initialize' request is the first request called by the frontend * to interrogate the features the debug adapter provides. + * @param {import('vscode-debugprotocol').DebugProtocol.InitializeResponse} response */ - initializeRequest(response/*: DebugProtocol.InitializeResponse, args: DebugProtocol.InitializeRequestArguments*/) { - - // This debug adapter implements the configurationDoneRequest. - response.body.supportsConfigurationDoneRequest = true; - - // we support some exception options + initializeRequest(response) { response.body.exceptionBreakpointFilters = [ { label:'All Exceptions', filter:'all', default:false }, { label:'Uncaught Exceptions', filter:'uncaught', default:true }, ]; - - // we support modifying variable values + response.body.supportsConfigurationDoneRequest = true; response.body.supportsSetVariable = true; - - // we support hit-count conditional breakpoints - response.body.supportsHitConditionalBreakpoints = true; - - // we support the new ExceptionInfoRequest + response.body.supportsEvaluateForHovers = true; response.body.supportsExceptionInfoRequest = true; + response.body.supportsHitConditionalBreakpoints = true; this.sendResponse(response); } @@ -167,6 +159,7 @@ class AndroidDebugSession extends DebugSession { /** * @param {string} msg + * @param {import('vscode-debugprotocol').DebugProtocol.Response} response * @param {boolean} silent */ failRequest(msg, response, silent = false) { @@ -185,7 +178,7 @@ class AndroidDebugSession extends DebugSession { /** * @param {string} requestName * @param {number} threadId - * @param {*} response + * @param {import('vscode-debugprotocol').DebugProtocol.Response} response */ failRequestNoThread(requestName, threadId, response) { this.failRequest(`${requestName} failed. Thread ${threadId} not found`, response); @@ -194,7 +187,7 @@ class AndroidDebugSession extends DebugSession { /** * @param {string} requestName * @param {number} threadId - * @param {*} response + * @param {import('vscode-debugprotocol').DebugProtocol.Response} response */ failRequestThreadNotSuspended(requestName, threadId, response) { this.failRequest(`${requestName} failed. Thread ${threadId} is not suspended`, response); @@ -203,7 +196,7 @@ class AndroidDebugSession extends DebugSession { /** * @param {string} requestName * @param {number} threadId - * @param {*} response + * @param {import('vscode-debugprotocol').DebugProtocol.Response} response */ cancelRequestThreadNotSuspended(requestName, threadId, response) { // now that vscode can resume threads before the locals,callstack,etc are retrieved, we only need to cancel the request @@ -258,10 +251,11 @@ class AndroidDebugSession extends DebugSession { // configure the thread names threadinfos.forEach(threadinfo => { const thread = this.getThread(threadinfo.threadid); - if (thread.name === null) { + if (typeof thread.name !== 'string') { thread.name = threadinfo.name; } else if (thread.name !== threadinfo.name) { // give the thread a new id for VS code + // - note: this will invalidate all current variable references for this thread delete this._threads[thread.vscode_threadid]; thread.allocateNewThreadID(); this._threads[thread.vscode_threadid] = thread; @@ -321,6 +315,17 @@ class AndroidDebugSession extends DebugSession { return s; } + /** + * @typedef AndroidAttachArguments + * @property {string} appSrcRoot + * @property {boolean} autoStartADB + * @property {number} processId + * @property {string} targetDevice + * @property {boolean} trace + * + * @param {import('vscode-debugprotocol').DebugProtocol.AttachResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.AttachRequestArguments & AndroidAttachArguments} args + */ async attachRequest(response, args) { this.debug_mode = 'attach'; if (args && args.trace) { @@ -382,7 +387,7 @@ class AndroidDebugSession extends DebugSession { // try and determine the relevant path for the API sources (based upon the API level of the connected device) await this.configureAPISourcePath(); - const build = new BuildInfo(null, new Map(this.src_packages.packages), null); + const build = new AttachBuildInfo(new Map(this.src_packages.packages)); this.LOG(`Attaching to pid ${processId} on device ${this._device.serial} [API:${this.device_api_level||'?'}]`); // try and attach to the specified pid @@ -427,12 +432,28 @@ class AndroidDebugSession extends DebugSession { } } - /** + /** + * @typedef AndroidLaunchArguments + * @property {number} adbPort + * @property {string[]} amStartArgs + * @property {string} apkFile + * @property {string} appSrcRoot + * @property {boolean} autoStartADB + * @property {number} callStackDisplaySize + * @property {string} launchActivity + * @property {string} manifestFile + * @property {string[]} pmInstallArgs + * @property {number} postLaunchPause + * @property {number} processId + * @property {StaleBuildSetting} staleBuild + * @property {string} targetDevice + * @property {boolean} trace + * The entry point to the debugger - * @param {*} response - * @param {*} args + * @param {import('vscode-debugprotocol').DebugProtocol.LaunchResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.LaunchRequestArguments & AndroidLaunchArguments} args */ - async launchRequest(response/*: DebugProtocol.LaunchResponse*/, args/*: LaunchRequestArguments*/) { + async launchRequest(response, args) { this.debug_mode = 'launch'; if (args && args.trace) { this.trace = args.trace; @@ -512,7 +533,7 @@ class AndroidDebugSession extends DebugSession { await this.configureAPISourcePath(); // launch the app - await this.startLaunchActivity(args.launchActivity); + await this.startLaunchActivity(args.launchActivity, args.postLaunchPause); this.debuggerAttached = true; @@ -552,6 +573,10 @@ class AndroidDebugSession extends DebugSession { } } + /** + * Check if the build is out of date (i.e a source file has been modified since the last build) + * @param {StaleBuildSetting} staleBuild + */ checkBuildIsUpToDate(staleBuild) { // check if any source file was modified after the apk if (this.src_packages.last_src_modified >= this.apk_file_info.app_modified) { @@ -564,7 +589,12 @@ class AndroidDebugSession extends DebugSession { } } - async startLaunchActivity(launchActivity) { + /** + * + * @param {string} launchActivity + * @param {number} postLaunchPause + */ + async startLaunchActivity(launchActivity, postLaunchPause) { if (!launchActivity) { // we're allowed no launchActivity if we have a custom am start command if (!this.am_start_args) { @@ -574,7 +604,12 @@ class AndroidDebugSession extends DebugSession { } } - const build = new BuildInfo(this.apk_file_info.manifest.package, new Map(this.src_packages.packages), launchActivity, this.am_start_args); + const build = new LaunchBuildInfo( + new Map(this.src_packages.packages), + this.apk_file_info.manifest.package, + launchActivity, + this.am_start_args, + postLaunchPause); this.LOG(`Launching on device ${this._device.serial} [API:${this.device_api_level||'?'}]`); if (this.am_start_args) { @@ -686,7 +721,11 @@ class AndroidDebugSession extends DebugSession { throw new Error(reject); } - configurationDoneRequest(response/*, args*/) { + /** + * + * @param {import('vscode-debugprotocol').DebugProtocol.ConfigurationDoneResponse} response + */ + configurationDoneRequest(response) { D('configurationDoneRequest'); this.waitForConfigurationDone(); this.sendResponse(response); @@ -702,7 +741,11 @@ class AndroidDebugSession extends DebugSession { } } - async disconnectRequest(response/*, args*/) { + /** + * + * @param {import('vscode-debugprotocol').DebugProtocol.DisconnectResponse} response + */ + async disconnectRequest(response) { D('disconnectRequest'); this._isDisconnecting = true; if (this.debuggerAttached) { @@ -733,7 +776,7 @@ class AndroidDebugSession extends DebugSession { } /** - * + * Called by the debugger in response to a JDWP breakpoint hit event * @param {JavaBreakpointEvent} e */ onBreakpointHit(e) { @@ -745,13 +788,15 @@ class AndroidDebugSession extends DebugSession { /** * Called when the user requests a change to breakpoints in a source file * Note: all breakpoints in a file are always sent in args, even if they are not changing + * @param {import('vscode-debugprotocol').DebugProtocol.SetBreakpointsResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.SetBreakpointsArguments} args */ - async setBreakPointsRequest(response/*: DebugProtocol.SetBreakpointsResponse*/, args/*: DebugProtocol.SetBreakpointsArguments*/) { + async setBreakPointsRequest(response, args) { const source_filename = args.source && args.source.path; D('setBreakPointsRequest: ' + source_filename); const unverified_breakpoint = (src_bp,reason) => { - const bp = new Breakpoint(false,src_bp.line); + const bp = new Breakpoint(false, src_bp.line); bp['id'] = ++this._breakpointId; bp['message'] = reason; return bp; @@ -814,8 +859,7 @@ class AndroidDebugSession extends DebugSession { } /** - * - * @param {*[]} breakpoints + * @param {import('vscode-debugprotocol').DebugProtocol.SourceBreakpoint[]} breakpoints * @param {string} relative_fpn */ async setupBreakpointsInFile(breakpoints, relative_fpn) { @@ -852,7 +896,11 @@ class AndroidDebugSession extends DebugSession { return java_breakpoints; }; - async setExceptionBreakPointsRequest(response /*: SetExceptionBreakpointsResponse*/, args /*: SetExceptionBreakpointsArguments*/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.SetExceptionBreakpointsResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.SetExceptionBreakpointsArguments} args + */ + async setExceptionBreakPointsRequest(response, args) { await this.dbgr.clearBreakOnExceptions(); switch(true) { case args.filters.includes('all'): @@ -865,7 +913,11 @@ class AndroidDebugSession extends DebugSession { this.sendResponse(response); } - async threadsRequest(response/*: DebugProtocol.ThreadsResponse*/) { + /** + * + * @param {import('vscode-debugprotocol').DebugProtocol.ThreadsResponse} response + */ + async threadsRequest(response) { if (!this._threads.length) { try { await this.refreshThreads(); @@ -889,8 +941,10 @@ class AndroidDebugSession extends DebugSession { /** * Returns a stack trace for the given threadId + * @param {import('vscode-debugprotocol').DebugProtocol.StackTraceResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.StackTraceArguments} args */ - async stackTraceRequest(response/*: DebugProtocol.StackTraceResponse*/, args/*: DebugProtocol.StackTraceArguments*/) { + async stackTraceRequest(response, args) { D(`stackTraceRequest thread:${args.threadId}`); // only retrieve the stack if the thread is paused const thread = this.getThread(args.threadId); @@ -972,7 +1026,11 @@ class AndroidDebugSession extends DebugSession { } } - async scopesRequest(response/*: DebugProtocol.ScopesResponse*/, args/*: DebugProtocol.ScopesArguments*/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.ScopesResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.ScopesArguments} args + */ + async scopesRequest(response, args) { D(`scopesRequest frame:${args.frameId}`); const threadId = AndroidThread.variableRefToThreadId(args.frameId); const thread = this.getThread(threadId); @@ -998,10 +1056,14 @@ class AndroidDebugSession extends DebugSession { } catch(e) { } this.sendResponse(response); -} + } - sourceRequest(response/*: DebugProtocol.SourceResponse*/, args/*: DebugProtocol.SourceArguments*/) { - D(`sourceRequest: ${args.sourceId}`); + /** + * @param {import('vscode-debugprotocol').DebugProtocol.SourceResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.SourceArguments} args + */ + sourceRequest(response, args) { + D(`sourceRequest: ${args.sourceReference}`); const content = `/* The source for this class is unavailable. @@ -1021,11 +1083,10 @@ class AndroidDebugSession extends DebugSession { } /** - * - * @param {*} response - * @param {{variablesReference:VSCVariableReference}} args + * @param {import('vscode-debugprotocol').DebugProtocol.VariablesResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.VariablesArguments} args */ - async variablesRequest(response/*: DebugProtocol.VariablesResponse*/, args/*: DebugProtocol.VariablesArguments*/) { + async variablesRequest(response, args) { D(`variablesRequest variablesReference:${args.variablesReference}`); const threadId = AndroidThread.variableRefToThreadId(args.variablesReference); const thread = this.getThread(threadId); @@ -1109,7 +1170,11 @@ class AndroidDebugSession extends DebugSession { await this.dbgr.resumeThread(thread.threadid); } - continueRequest(response/*: DebugProtocol.ContinueResponse*/, args/*: DebugProtocol.ContinueArguments*/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.ContinueResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.ContinueArguments} args + */ + continueRequest(response, args) { D(`Continue thread:${args.threadId}`); const thread = this.getThread(args.threadId); @@ -1132,8 +1197,8 @@ class AndroidDebugSession extends DebugSession { /** * Called by the user to start a step operation * @param {DebuggerStepType} which - * @param {*} response - * @param {*} args + * @param {import('vscode-debugprotocol').DebugProtocol.NextResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.NextArguments} args */ doStep(which, response, args) { D(`step ${which}`); @@ -1156,15 +1221,27 @@ class AndroidDebugSession extends DebugSession { this.dbgr.step(which, thread.threadid); } - stepInRequest(response/*: DebugProtocol.NextResponse*/, args/*: DebugProtocol.StepInArguments*/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.NextResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.StepInArguments} args + */ + stepInRequest(response, args) { this.doStep('in', response, args); } - nextRequest(response/*: DebugProtocol.NextResponse*/, args/*: DebugProtocol.NextArguments*/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.NextResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.NextArguments} args + */ + nextRequest(response, args) { this.doStep('over', response, args); } - stepOutRequest(response/*: DebugProtocol.NextResponse*/, args/*: DebugProtocol.StepOutArguments*/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.NextResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.StepOutArguments} args + */ + stepOutRequest(response, args) { this.doStep('out', response, args); } @@ -1182,7 +1259,11 @@ class AndroidDebugSession extends DebugSession { this.reportStoppedEvent("exception", e.throwlocation, last_exception); } - async exceptionInfoRequest(response /*DebugProtocol.ExceptionInfoResponse*/, args /**/) { + /** + * @param {import('vscode-debugprotocol').DebugProtocol.ExceptionInfoResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.ExceptionInfoArguments} args + */ + async exceptionInfoRequest(response, args) { D(`exceptionInfoRequest: ${args.threadId}`); const thread = this.getThread(args.threadId); if (!thread) return this.failRequestNoThread('Exception info', args.threadId, response); @@ -1252,15 +1333,10 @@ class AndroidDebugSession extends DebugSession { } /** - * @typedef SetVariableArgs - * @property {string} name - * @property {string} value - * @property {number} variablesReference - * - * @param {*} response - * @param {SetVariableArgs} args + * @param {import('vscode-debugprotocol').DebugProtocol.SetVariableResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.SetVariableArguments} args */ - async setVariableRequest(response/*: DebugProtocol.SetVariableResponse*/, args/*: DebugProtocol.SetVariableArguments*/) { + async setVariableRequest(response, args) { const threadId = AndroidThread.variableRefToThreadId(args.variablesReference); const thread = this.getThread(threadId); @@ -1289,8 +1365,10 @@ class AndroidDebugSession extends DebugSession { /** * Called by VSCode to perform watch, console and hover evaluations + * @param {import('vscode-debugprotocol').DebugProtocol.EvaluateResponse} response + * @param {import('vscode-debugprotocol').DebugProtocol.EvaluateArguments} args */ - async evaluateRequest(response/*: DebugProtocol.EvaluateResponse*/, args/*: DebugProtocol.EvaluateArguments*/) { + async evaluateRequest(response, args) { // Some notes to remember: // annoyingly, during stepping, the step can complete before the resume has called evaluateRequest on watches. diff --git a/src/debugger-types.js b/src/debugger-types.js index 8ff7568..a4b8e21 100644 --- a/src/debugger-types.js +++ b/src/debugger-types.js @@ -4,16 +4,25 @@ const { PackageInfo } = require('./package-searcher'); const { splitSourcePath } = require('./utils/source-file'); class BuildInfo { - /** - * @param {string} pkgname * @param {Map} packages - * @param {string} launchActivity - * @param {string[]} [amCommandArgs] custom arguments passed to `am start` */ - constructor(pkgname, packages, launchActivity, amCommandArgs) { - this.pkgname = pkgname; + constructor(packages) { this.packages = packages; + } +} + +class LaunchBuildInfo extends BuildInfo { + /** + * @param {Map} packages + * @param {string} pkgname + * @param {string} launchActivity + * @param {string[]} amCommandArgs custom arguments passed to `am start` + * @param {number} postLaunchPause amount of time (in ms) to wait after launch before we attempt a debugger connection + */ + constructor(packages, pkgname, launchActivity, amCommandArgs, postLaunchPause) { + super(packages); + this.pkgname = pkgname; this.launchActivity = launchActivity; /** the arguments passed to `am start` */ this.startCommandArgs = amCommandArgs || [ @@ -24,10 +33,19 @@ class BuildInfo { `-n ${pkgname}/${launchActivity}`, ]; /** - * the amount of time to wait after 'am start ...' is invoked. + * the amount of time (in millis) to wait after 'am start ...' is invoked. * We need this because invoking JDWP too soon causes a hang. */ - this.postLaunchPause = 1000; + this.postLaunchPause = ((typeof postLaunchPause === 'number') && (postLaunchPause >= 0)) ? postLaunchPause : 1000; + } +} + +class AttachBuildInfo extends BuildInfo { + /** + * @param {Map} packages + */ + constructor(packages) { + super(packages); } } @@ -670,7 +688,7 @@ class DebuggerTypeInfo { // otherwise, leave super undefined to be updated later if (info.reftype.string !== 'class' || type.signature[0] !== 'L' || type.signature === JavaType.Object.signature) { if (info.reftype.string !== 'array') { - /** @type {JavaType} */ + /** @type {JavaClassType} */ this.super = null; } } @@ -747,9 +765,9 @@ class VariableValue { } module.exports = { + AttachBuildInfo, BreakpointLocation, BreakpointOptions, - BuildInfo, DebuggerBreakpoint, DebuggerException, DebuggerFrameInfo, @@ -757,6 +775,7 @@ module.exports = { DebuggerTypeInfo, DebugSession, DebuggerValue, + LaunchBuildInfo, LiteralValue, JavaBreakpointEvent, JavaExceptionEvent, diff --git a/src/debugger.js b/src/debugger.js index 89d17b1..c6ba1e2 100644 --- a/src/debugger.js +++ b/src/debugger.js @@ -8,9 +8,9 @@ const { D } = require('./utils/print'); const { sleep } = require('./utils/thread'); const { decodeJavaStringLiteral } = require('./utils/char-decode'); const { + AttachBuildInfo, BreakpointLocation, BreakpointOptions, - BuildInfo, DebuggerBreakpoint, DebuggerFrameInfo, DebuggerMethodInfo, @@ -24,6 +24,7 @@ const { JavaTaggedValue, JavaThreadInfo, JavaType, + LaunchBuildInfo, MethodInvokeArgs, SourceLocation, TypeNotAvailable, @@ -71,7 +72,7 @@ class Debugger extends EventEmitter { }; /** - * @param {BuildInfo} build + * @param {LaunchBuildInfo} build * @param {string} deviceid */ async startDebugSession(build, deviceid) { @@ -82,19 +83,36 @@ class Debugger extends EventEmitter { const stdout = await Debugger.runApp(deviceid, build.startCommandArgs, build.postLaunchPause); // retrieve the list of debuggable processes - const pids = await Debugger.getDebuggablePIDs(this.session.deviceid, 10e3); - if (pids.length === 0) { + const named_pids = await Debugger.getDebuggableProcesses(deviceid, 10e3); + if (named_pids.length === 0) { throw new Error(`startDebugSession: No debuggable processes after app launch.`); } - // choose the last pid in the list - const pid = pids[pids.length - 1]; + // we assume the newly launched app is the last pid in the list, but try and + // validate using the process names + const matched_named_pids = build.pkgname ? named_pids.filter(np => np.name === build.pkgname) : []; + let pid; + switch (matched_named_pids.length) { + case 0: + // no name match - warn, but choose the last entry anyway + D('No process name match - choosing last jdwp pid'); + pid = named_pids[named_pids.length - 1].pid; + break; + case 1: + pid = matched_named_pids[0].pid; + break; + default: + // more than one choice - warn, but choose we'll use the last one anyway + D('Multiple process names match - choosing last matching entry'); + pid = matched_named_pids[matched_named_pids.length - 1].pid; + break; + } // after connect(), the caller must call resume() to begin await this.connect(pid); return stdout; } /** - * @param {BuildInfo} build + * @param {AttachBuildInfo} build * @param {number} pid process ID to connect to * @param {string} deviceid device ID to connect to */ @@ -110,9 +128,9 @@ class Debugger extends EventEmitter { /** * @param {string} deviceid Device ID to connect to * @param {string[]} launch_cmd_args Array of arguments to pass to 'am start' - * @param {number} [post_launch_pause] amount to time to wait after each launch attempt + * @param {number} post_launch_pause amount of time (in ms) to wait after each launch attempt */ - static async runApp(deviceid, launch_cmd_args, post_launch_pause = 1000) { + static async runApp(deviceid, launch_cmd_args, post_launch_pause) { // older (<3) versions of Android only allow target components to be specified with -n const shell_cmd = { command: `am start ${launch_cmd_args.join(' ')}`, @@ -301,7 +319,9 @@ class Debugger extends EventEmitter { if (!this.session) { return; } - return Debugger.forceStopApp(this.session.deviceid, this.session.build.pkgname); + if (this.session.build instanceof LaunchBuildInfo) { + return Debugger.forceStopApp(this.session.deviceid, this.session.build.pkgname); + } } /** @@ -682,13 +702,13 @@ class Debugger extends EventEmitter { } /** - * @param {DebuggerValue} value + * @param {string} signature */ - async getSuperType(value) { - if (value.type.signature === JavaType.Object.signature) + async getSuperType(signature) { + if (signature === JavaType.Object.signature) throw new Error('java.lang.Object has no super type'); - const typeinfo = await this.getTypeInfo(value.type.signature); + const typeinfo = await this.getTypeInfo(signature); await this._ensureSuperType(typeinfo); return typeinfo.super; } @@ -697,7 +717,7 @@ class Debugger extends EventEmitter { * @param {DebuggerValue} value */ async getSuperInstance(value) { - const supertype = await this.getSuperType(value); + const supertype = await this.getSuperType(value.type.signature); if (value.vtype === 'class') { return this.getTypeValue(supertype.signature); } @@ -743,15 +763,23 @@ class Debugger extends EventEmitter { } /** - * * @param {DebuggerValue} object_value */ async getFieldValues(object_value) { const type = await this.getTypeInfo(object_value.type.signature); await this._ensureFields(type); + return this.fetchFieldValues(object_value, type.info.typeid, type.fields); + } + + /** + * @param {DebuggerValue} object_value + * @param {JavaTypeID} typeid + * @param {JavaField[]} field_list + */ + async fetchFieldValues(object_value, typeid, field_list) { // the Android runtime now pointlessly barfs into logcat if an instance value is used // to retrieve a static field. So, we now split into two calls... - const splitfields = type.fields.reduce((z, f) => { + const splitfields = field_list.reduce((z, f) => { if (f.modbits & 8) { z.static.push(f); } else { @@ -776,7 +804,7 @@ class Debugger extends EventEmitter { let static_fieldvalues = []; if (splitfields.static.length) { static_fieldvalues = await this.session.adbclient.jdwp_command({ - cmd: JDWP.Commands.GetStaticFieldValues(type.info.typeid, splitfields.static), + cmd: JDWP.Commands.GetStaticFieldValues(typeid, splitfields.static), }); } // make sure the fields and values match up... @@ -786,7 +814,8 @@ class Debugger extends EventEmitter { res.forEach((value,i) => { value.data.field = fields[i]; value.fqname = `${object_value.fqname || object_value.name}.${value.name}`; - }) + }); + return res; } @@ -799,21 +828,24 @@ class Debugger extends EventEmitter { if (!(object_value.type instanceof JavaClassType)) { return null; } - let instance = object_value; + // retrieving field values is expensive, so we search through the class + // fields (which will be cached) until we find a match + let field, object_type = object_value.type, typeinfo; for (;;) { - // retrieve all the fields for this instance - const fields = await this.getFieldValues(instance); - const field = fields.find(f => f.name === fieldname); + typeinfo = await this.getTypeInfo(object_type.signature); + const fields = await this._ensureFields(typeinfo); + field = fields.find(f => f.name === fieldname); if (field) { - return field; + break; } - // if there's no matching field in this instance, check the super - if (!includeInherited || instance.type.signature === JavaType.Object.signature) { + if (!includeInherited || object_type.signature === JavaType.Object.signature) { const fully_qualified_typename = `${object_value.type.package}.${object_value.type.typename}`; throw new Error(`No such field '${fieldname}' in type ${fully_qualified_typename}`); } - instance = await this.getSuperInstance(instance); + object_type = await this.getSuperType(object_type.signature); } + const values = await this.fetchFieldValues(object_value, typeinfo.info.typeid, [field]); + return values[0]; } /** diff --git a/src/expression/evaluate.js b/src/expression/evaluate.js index 9002552..7ad5fdf 100644 --- a/src/expression/evaluate.js +++ b/src/expression/evaluate.js @@ -511,8 +511,20 @@ async function evaluate_identifier(dbgr, locals, identifier) { if (local) { return local; } + + // check if the identifier is an unqualified member of the current 'this' context + const this_context = locals.find(l => l.name === 'this'); + if (this_context) { + try { + const member = await evaluate_member(dbgr, new MemberExpression(identifier), this_context); + return member; + } catch { + // not a member of this - just continue + } + } + // if it's not a local, it could be the start of a package name or a type - const classes = await dbgr.getAllClasses(); + const classes = Array.from(dbgr.session.loadedClasses); return evaluate_qualified_type_name(dbgr, identifier, classes); } @@ -520,17 +532,17 @@ async function evaluate_identifier(dbgr, locals, identifier) { * * @param {Debugger} dbgr * @param {string} dotted_name - * @param {*[]} classes + * @param {string[]} classes */ async function evaluate_qualified_type_name(dbgr, dotted_name, classes) { const exact_class_matcher = new RegExp(`^L(java/lang/)?${dotted_name.replace(/\./g,'[$/]')};$`); - const exact_class = classes.find(c => exact_class_matcher.test(c.type.signature)); + const exact_class = classes.find(signature => exact_class_matcher.test(signature)); if (exact_class) { - return dbgr.getTypeValue(exact_class.type.signature); + return dbgr.getTypeValue(exact_class); } const class_matcher = new RegExp(`^L(java/lang/)?${dotted_name.replace('.','[$/]')}/`); - const matching_classes = classes.filter(c => class_matcher.test(c.type.signature)); + const matching_classes = classes.filter(signature => class_matcher.test(signature)); if (matching_classes.length === 0) { // the dotted name doesn't match any packages throw new Error(`'${dotted_name}' is not a package, type or variable name`); @@ -623,7 +635,7 @@ async function evaluate_qualifiers(dbgr, locals, thread, value, qualified_terms) i++; continue; } - value = await evaluate_member(dbgr, locals, thread, term, value); + value = await evaluate_member(dbgr, term, value); continue; } if (term instanceof ArrayIndexExpression) { @@ -822,12 +834,10 @@ async function evaluate_methodcall(dbgr, locals, thread, method_name, m, obj_loc /** * @param {Debugger} dbgr - * @param {DebuggerValue[]} locals - * @param {AndroidThread} thread * @param {MemberExpression} member * @param {DebuggerValue} value */ -async function evaluate_member(dbgr, locals, thread, member, value) { +async function evaluate_member(dbgr, member, value) { if (!JavaType.isReference(value.type)) { throw new Error('TypeError: value is not a reference type'); } @@ -952,7 +962,7 @@ async function evaluate_cast(dbgr, locals, thread, cast_type, rhs) { * @param {Debugger} dbgr * @param {{allowFormatSpecifier:boolean}} [options] */ -async function evaluate(expression, thread, locals, dbgr, options) { +async function evaluate_one_expression(expression, thread, locals, dbgr, options) { D('evaluate: ' + expression); await dbgr.ensureConnected(); @@ -994,6 +1004,45 @@ async function evaluate(expression, thread, locals, dbgr, options) { } } +/** + * + */ +const queuedExpressions = []; + +/** + * @param {string} expression + * @param {AndroidThread} thread + * @param {DebuggerValue[]} locals + * @param {Debugger} dbgr + * @param {{allowFormatSpecifier:boolean}} [options] + */ +async function evaluate(expression, thread, locals, dbgr, options) { + return new Promise(async (resolve, reject) => { + const queue_length = queuedExpressions.push({ + expression, thread, locals, dbgr, options, + resolve, reject + }); + if (queue_length > 1) { + return; + } + // run the queue + while (queuedExpressions.length) { + const { + expression, thread, locals, dbgr, options, + resolve, reject + } = queuedExpressions[0]; + try { + const res = await evaluate_one_expression(expression, thread, locals, dbgr, options); + resolve(res); + } catch (err) { + reject(err); + } + queuedExpressions.shift(); + } + }); +} + + module.exports = { evaluate, } diff --git a/src/expression/parse.js b/src/expression/parse.js index 0d3648e..495de0b 100644 --- a/src/expression/parse.js +++ b/src/expression/parse.js @@ -88,10 +88,16 @@ class UnaryOpExpression extends ParsedExpression { } class TernaryExpression extends ParsedExpression { + + /** + * @param {ParsedExpression} condition + */ constructor(condition) { super(); this.condition = condition; + /** @type {ParsedExpression} */ this.ternary_true = null; + /** @type {ParsedExpression} */ this.ternary_false = null; } } @@ -101,17 +107,24 @@ class QualifierExpression extends ParsedExpression { } class ArrayIndexExpression extends QualifierExpression { - constructor(e) { + /** + * @param {ParsedExpression} index_expression + */ + constructor(index_expression) { super(); - this.indexExpression = e; + this.indexExpression = index_expression; } } class MethodCallExpression extends QualifierExpression { + /** @type {ParsedExpression[]} */ arguments = []; } class MemberExpression extends QualifierExpression { + /** + * @param {string} name + */ constructor(name) { super(); this.name = name; diff --git a/src/index.d.js b/src/index.d.js index e397929..03655e2 100644 --- a/src/index.d.js +++ b/src/index.d.js @@ -108,6 +108,7 @@ * @typedef {number} JDWPRequestID * @typedef {JDWPRequestID} StepID * @typedef {'caught'|'uncaught'|'both'} ExceptionBreakMode + * @typedef {'ignore'|'warn'|'stop'} StaleBuildSetting * */ diff --git a/src/process-attach.js b/src/process-attach.js index 910b6f9..0fdf4b9 100644 --- a/src/process-attach.js +++ b/src/process-attach.js @@ -1,6 +1,5 @@ const os = require('os'); const { ADBClient } = require('./adbclient'); -const { selectTargetDevice } = require('./utils/device'); /** * @param {import('vscode')} vscode @@ -30,28 +29,23 @@ async function showPIDPicker(vscode, pids) { /** * @param {import('vscode')} vscode + * @param {string} device_serial */ -async function selectAndroidProcessID(vscode) { +async function selectAndroidProcessID(vscode, device_serial) { const res = { /** @type {string|'ok'|'cancelled'|'failed'} */ status: 'failed', pid: 0, serial: '', } - const err = await new ADBClient().test_adb_connection() - if (err) { - vscode.window.showWarningMessage('Attach failed. ADB is not running.'); + + let named_pids; + try { + named_pids = await new ADBClient(device_serial).named_jdwp_list(5000); + } catch { + vscode.window.showWarningMessage(`Attach failed. Check the device ${device_serial} is connected.`); return res; } - - const device = await selectTargetDevice(vscode, 'Attach'); - if (!device) { - // user cancelled picker - res.status = 'cancelled'; - return res; - } - - let named_pids = await new ADBClient(device.serial).named_jdwp_list(5000); if (named_pids.length === 0) { vscode.window.showWarningMessage( 'Attach failed. No debuggable processes are running on the device.' @@ -72,7 +66,7 @@ async function selectAndroidProcessID(vscode) { } res.pid = named_pid.pid; - res.serial = device.serial; + res.serial = device_serial; res.status = 'ok'; return res; diff --git a/src/stack-frame.js b/src/stack-frame.js index aaac662..ec0e3d0 100644 --- a/src/stack-frame.js +++ b/src/stack-frame.js @@ -137,7 +137,7 @@ class DebuggerStackFrame extends VariableManager { } async getObjectFields(varinfo) { - const supertype = await this.dbgr.getSuperType(varinfo.objvar); + const supertype = await this.dbgr.getSuperType(varinfo.objvar.type.signature); const fields = await this.dbgr.getFieldValues(varinfo.objvar); // add an extra msg field for exceptions if (varinfo.exception) {