In D95045#2522488, @StephenTozer wrote:

This looks good to me - does this need a test case?

Added more context to diff

The "go" method of Debugger classes in Dexter used to be called once, to launch the process. However with the new DebuggerController classes it's now used to set the target process free running until we hit a relevant source line. Alas, I'd baked the former assumption into the DbgEng driver and had the "go" method do nothing. This can lead to an infinite loop where the DebuggerController repeatedly calls "go".

LGTM. thanks for spotting this... <hangs head in shame>

@MaskRay duly noted, thanks for the info.

I'll get this committed for you shortly.

Hey Nabeel, thanks for this.

fix for any builds that don't have lldb available:

Hi Nabeel,

Hi Nabeel,

https://reviews.llvm.org/D89689

landed in rG7d8c19a4e9ed8f39eda0f0f383d5ff8f1485471b

Hi Nabeel,

First off, thanks for the feedback!

I also wondered where the best place would be for this path mapping. However, I still think DebuggerBase is a good spot as the debuggers currently rely on knowing context.source_files. So whichever tool wants to use the debuggers will also have to fill context.source_files, and therefore also run in the problem of needing to remap paths to the ones used in the compilation unit. Moving this mapping into the test tool would prevent other tools from reusing it.

Also, moving this mapping into the test tool would require to make the source_files be relative paths (to the source_root_dir) before calling the debuggers.
This leads to a future me now needing to remember what kind of path is used where: An absolute file system path (test tool), a path relative to source_root_dir (DebuggerController), or a debug info path (Debugger).

WDYT?

Hey and thanks for this!

recommitted rGc6aa829644f30d5590451b892918298f8117c985

Hi Adrian (@aprantl)

seems I've made a more mundane error of committing a diff file as part of my patch. this has been reverted for now.

@aprantl I'll take a look, thanks for pointing them out.

Landed in commit rG81e836a5a675f6a3d9d35560fddbbb87fdf66201

Landed in commit rGbf1cdc2c6c0460b7121ac653c796ef4995b1dfa9

removed and replaced LoadDebuggerException with DebuggerException in LLDB.

Updated diff to correct revision for easing future historical searches

@echristo thanks for your interest. do you need any further clarification on what dexter does or how this patch fits in with the existing behaviour?

missing note, dbgeng does not support the new DexLimitSteps commands as of right now.

I prefer the current approach for readabilities sake as well as separating the two method calls out.

no, this would cause file path comparisons that are case sensitive to potentially fail on drive letter for windows.

removed accidental new line deletion

in ./debuginfo-tests/CMakeLists.txt we have the following on line 27:

@jmorse I believe there's already a test in the cmake/config that checks the python version when generating the build.

thanks for this, LGTM.

fix for failing test reported at D76926

fix for reported test failure at:

thanks for reporting. this issue should now be fixed.

The cdb tests taking longer may be a result of the dexter regression suite running along side them. SFAIK that's not intended behaviour.

I've also added an internal ticket to add darwin support to the dexter regression suite... I don't think there'll be much appetite in-house for it but if there's a clear need to add it in the future then of course we'll consider putting in the work.

Hi Davide,

I've actually mailed these via the mailing list by accident, here's a copy of what I mailed:

@Pierre-vh and not a problem buddy, I wish you all the best in your future endeavours, don't worry about feedback on the controller patch if it's not related to your current work! :) you get on with what you need to.

The debugger controller patch is now live and awaiting review at

I hope you're keeping safe in these unprecedented times!

Hi Pierre,

I haven't attempted to generalize my work as I entirely rely on the LLDB library for this (and I work on Linux so I can't work with any other debugger supported by Dexter)
Maybe we can try to find something together? Where do you wish to discuss this?

Hi Pierre,

LGTM

Hi Reid,

Hi David,

KILL instruction is no reinstated and failing check line has been removed.

I missed adding you as a reviewer @djtodoro , my apologies. This has been rectified.

Hello Everyone,

I'll land this tomorrow as I'm off home now and don't want to risk getting buildbot emails all night.

updated diff