Page MenuHomePhabricator

NativeProcessProtocol: Simplify breakpoint setting code
ClosedPublic

Authored by labath on Oct 5 2018, 12:37 PM.

Details

Summary

A fairly simple operation as setting a breakpoint (writing a breakpoint
opcode) at a given address was going through three classes:
NativeProcessProtocol which called NativeBreakpointList, which then
called SoftwareBrekpoint, only to end up again in NativeProcessProtocol
to do the actual writing itself. This is unnecessarily complex and can
be simplified by moving all of the logic into NativeProcessProtocol
class itself, removing a lot of boilerplate.

One of the reeasons for this complexity was that (it seems)
NativeBreakpointList class was meant to hold both software and hardware
breakpoints. However, that never materialized, and hardware breakpoints
are stored in a separate map holding only hardware breakpoints.
Essentially, this patch makes software breakpoints follow that approach
by replacing the heavy SoftwareBraekpoint with a light struct of the
same name, which holds only the data necessary to describe one
breakpoint. The rest of the logic is in the main class. As, at the
lldb-server level, handling software and hardware breakpoints is very
different, this seems like a reasonable state of things.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Oct 5 2018, 12:37 PM
krytarowski accepted this revision.Oct 8 2018, 2:13 AM

The NetBSD part looks fine. I will be out of the office soon as I will visit California for GSoC Mentor Summit and MeetBSDCa (until October 23rd).

This revision is now accepted and ready to land.Oct 8 2018, 2:13 AM
labath added a comment.Nov 3 2018, 6:40 AM

Kamil, I think you're the best qualified person to look at the lldb-server changes right now, so I'd like to hear your opinion on the other changes too. Otherwise, I'll just commit this some time next week.

This looks fine to me.

This revision was automatically updated to reflect the committed changes.