Page MenuHomePhabricator

[LLDB][MIPS] List and set hardware watchpoints for MIPS
ClosedPublic

Authored by mohit.bhakkad on Apr 21 2015, 2:06 AM.

Details

Summary

This patch contains:

  • Set H/W watchpoints
  • List H/W watchpoints
  • Check if watchpoint is hit Will roll out remaining functions if this looks good.

Diff Detail

Repository
rL LLVM

Event Timeline

mohit.bhakkad retitled this revision from to [LLDB][MIPS] List and set hardware watchpoints for MIPS.
mohit.bhakkad updated this object.
mohit.bhakkad edited the test plan for this revision. (Show Details)
mohit.bhakkad added reviewers: clayborg, jingham.
mohit.bhakkad set the repository for this revision to rL LLVM.
mohit.bhakkad added subscribers: jaydeep, dsanders, slthakur and 2 others.
clayborg requested changes to this revision.Apr 21 2015, 9:33 AM
clayborg edited edge metadata.

Move code from NativeRegisterContextLinux_mips64.h to NativeRegisterContextLinux_mips64.cpp and this will be good to go.

source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
18–70 ↗(On Diff #24104)

Do we need this content in the header file? Seems like it could move to NativeRegisterContextLinux_mips64.cpp to make sure we aren't #define'ing simple names in header files (W_BIT, R_BIT, I_BIT, etc).

This revision now requires changes to proceed.Apr 21 2015, 9:33 AM
mohit.bhakkad edited edge metadata.

Changes in this revision:

  • Addressed comment on previous revision
  • Adding functionality to clear watchpoints.
clayborg accepted this revision.May 6 2015, 9:29 AM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.May 6 2015, 9:29 AM
mohit.bhakkad edited edge metadata.

Changes in this revision:

  • some mips based changes in both client and server side
  • some code trimming.

Synced this patch with current ToT.
Should we go ahead and commit it?

mohit.bhakkad requested a review of this revision.May 20 2015, 10:59 PM
mohit.bhakkad edited edge metadata.

Hi @clayborg, could you please review this?

clayborg requested changes to this revision.May 26 2015, 3:39 PM
clayborg edited edge metadata.

Looks good except we should simplify the logic for finding a watchpoint by address using the code suggested for WatchpointList::FindByAddress() in the inlined code.

source/Breakpoint/WatchpointList.cpp
76–84 ↗(On Diff #26041)

Shouldn't this just be simpler:

lldb::addr_t watch_addr = (*pos)->GetLoadAddress();
lldb::addr_t watch_size = (*pos)->GetByteSize();
if (watch_addr <= addr && addr < watch_addr + watch_size)
...
This revision now requires changes to proceed.May 26 2015, 3:39 PM
mohit.bhakkad edited edge metadata.

Simplified WatchpointList::FindByAddress() as suggested.

clayborg accepted this revision.May 27 2015, 9:54 AM
clayborg edited edge metadata.

Much better.

This revision is now accepted and ready to land.May 27 2015, 9:54 AM
mohit.bhakkad edited edge metadata.

Changes in this patch:

  • Syncing it with rL238196
  • Adding a local buffer to get exact address (As mips watch registers conatins a masked one)
mohit.bhakkad requested a review of this revision.Jun 8 2015, 2:53 AM
mohit.bhakkad edited edge metadata.

My apologies for requesting review after acceptance, but ToT is changed significantly, can't commit directly.

mohit.bhakkad edited edge metadata.

Adding some corrections to previous patch.
Hi @clayborg, could you please review this?

clayborg accepted this revision.Jun 17 2015, 10:14 AM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Jun 17 2015, 10:14 AM
This revision was automatically updated to reflect the committed changes.