[Lldb-commits] [PATCH] D109777: [lldb] [Windows] Fix continuing from breakpoints and singlestepping on ARM/AArch64

Wed Sep 15 05:26:52 PDT 2021

DavidSpickett added a comment.

> I don't have a formal source for that one, but compiling a __debugbreak() with MSVC produces that opcode. As Windows is thumb-only, we can always do a breakpoint with that single opcode (even if the targeted instruction happens to be a wide thumb2 instruction) without needing to detect betwen arm/thumb mode.

Great. (I had forgotten about being Thumb only in fact)

> You mean the duplication between the "native" and regular versions of the classes - or the arch specific switches in otherwise seemingly arch-independent code?

The first one. The switches follow the style of the generic methods so I'm cool with that.

> For the former, I'm not quite familiar enough the how the architecture of these things work in practice, but I think those two class hiearchies/families don't have much in common, so there's no immediate common place to put shared code between the two.

Looking at the cmake it builds `lldbPluginProcessWindowsCommon` which includes native and process files so I think that means you can add a file there and both modes can use it, I think. `BreakpointInfo.cpp` ?

> For the latter - I guess it'd maybe be possible to add some extra locally defined (not overriding a virtual method from the base classes) methods in the register contexts that return the breakpoint opcodes, their sizes and the right flag bits...

Now you mention the two modes, because each mode has its own register context class if you put the breakpoint info there it'll also be duplicated. Unless you add an intermediate RegisterContextWindowsBaseArm in the middle but that's blurring the register context's purpose further.

If the breakpoint info can go in a file that both modes can access that would be great, otherwise it's fine as is. Not likely to change this in a hurry anyway.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109777/new/

https://reviews.llvm.org/D109777