LGTM, thank you for this fix!

LGTM, but I would wait for one more approval.

Added inline comment

Looks reasonable

D96952 is landed instead.

Removed unrelated changes from the patch.

Thanks for such a detailed review, David!
I would love to split off the changes into separate patches, but if I do so, this patch just will not be compileable.

clang-tidy would not be happy;)

I created D96952 - a demonstrative example of the CRTP-based version of this patch. Please, take a look.

Removed explicit conversions to StringRef from the test with respect to D96861.

CRTP was my first implementation, however, I discarded it as more bug-prone. Virtual Clone function at the interface is so common that, I believe, everyone knows it must be overridden by a new derived class. The necessity of inheriting from base_clone_helper is not so obvious.

In D93510#2499262, @jplayer-nv wrote:

I just triaged the gcc 5.4 build break. It appears that std::is_trivially_copy_constructible<T> instantiates the copy constructor (which is... strange). So we can't query a compile-time property without the compiler forcing instantiation of the deleted copy constructor in this case.

This bug exists all the way up through gcc 7.3. Here's a godbolt link which reproduces the problem:
https://godbolt.org/z/ETW7f1

In D92164#2536036, @werat wrote:

I think your test case is even better, the one in D95761 doesn't try to run commands.

Removed test since the same case was added by D95761.

It turns out that the Debugger recalculated the selected stack frame without taking the Process's run lock. I replaced the whole context evaluation with creating ExecutionContextRef which does these things right.

D92164 was intended for fixing the "settings set" issue, however, it revealed some deadlocks and data races, and had to be reverted temporarily. Currently, I'm working on those multithreading issues and will submit a patch as soon as possible.

In D92164#2463734, @labath wrote:

With this version of the patch, I am unable to reproduce the issue using the approach I described in the previous comment. However, it still reproduces when issuing the equivalent commands via the "gui" (either "by hand" or by running TestGuiBasicDebug.py).

Fixed CommandInterpreter::GetProcessOutput to avoid deadlock in Windows process plugin. This should also fix the problem @labath described above.

Removed do_select's default value.

Thanks for pointing to the nested command problem! Replaced pointer to the execution context with the stack of contexts.

Addressed comments

Removed refactoring to make the changes clearer.

I used Swig 3.0.12 + cherry-picked commit that fixes that bug (https://github.com/swig/swig/issues/1321) on Windows. It worked well with both Python 3.7 and 3.8.

Looks reasonable to me.

I'm not familiar with the z/OS target, so I cannot check the correctness of the target-specific changes. The overall patch looks good.

I see. The option "/Zc:__cplusplus" affects GTEST_LANG_CXX11 but not GTEST_HAS_TR1_TUPLE. I didn't have warnings just because my LLVM fork uses C++17.

The option "/Zc:__cplusplus" solves this problem. With this option passed, Gtest doesn't try using namespace tr1. I don't have such warnings if pass this option.

LLVM documentation says: "You will need Visual Studio 2017 or higher, with the latest Update installed". Does this mean Visual Studio 2017 version 15.9? Because the /Zc:__cplusplus option is available starting in Visual Studio 2017 version 15.7.

Thank you! I fixed the commit title and description as you suggested.

TestAddressBreakpoints.py fails on the last check - it expects hit count to be saved after re-launching the process. Removed that check.

Ups... Pushed it occationaly with the other patchs.

Addressed comments.

Un-skip TestRequireHWBreakpoints.py on Windows, now it works as expected.

It also fixes warnings about using deprecated std::tr1, which became errors in VS 2019, produced while compiling lldbUtilityHelpers project (http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/17887/steps/test/logs/stdio).

I went further than just moving m_hardware_index from StoppointLocation. Please, take a look at D84527 and let me know what you think about that.

@omjavaid, thank you for verifying this!

BreakpointSite::IntersectsRange returns false for hardware breakpoints, that's why the assertion fails.

Probably fixes llvm.org/PR44659, though I cannot check on arm/aarch64.

No updates, just to trigger rebasing onto master where tests should be already fixed.